upgrade to SB 3.4 and polish observability #130
Conversation
WalkthroughThis pull request introduces significant changes to the RAG (Retrieval-Augmented Generation) Spring AI project, focusing on shifting from Redis to PgVector for vector storage, updating configuration management, and refactoring the AI chat service. The modifications include removing the REST client configuration, updating the AI chat service logic, modifying application properties, and adjusting the Testcontainers configuration to support PostgreSQL instead of Redis. Changes
Sequence DiagramsequenceDiagram
participant User
participant AIChatService
participant VectorStoreDocumentRetriever
participant ContextualQueryAugmenter
participant RetrievalAugmentationAdvisor
participant AIClient
User->>AIChatService: Send query
AIChatService->>VectorStoreDocumentRetriever: Retrieve relevant documents
AIChatService->>ContextualQueryAugmenter: Augment query
AIChatService->>RetrievalAugmentationAdvisor: Create advisor
AIChatService->>AIClient: Process augmented query
AIClient-->>AIChatService: Return response
AIChatService-->>User: Provide chat response
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java (1)
29-31: Consider customizing the container configuration for test isolation.By default,
PostgreSQLContainerprovides ephemeral credentials and port mappings, which is often sufficient. However, if advanced scenarios (like test parallelization or custom credentials) are needed, you can further configure the container via environment variables or constructor parameters as needed.rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (2)
32-37: Parameterize the similarity threshold for flexibility.
While0.50might be a sensible default, you may wish to expose this as a configuration property, enabling easier tuning.
54-54: Clean return value but consider null or empty responses.
Check whether theaiClientmight return null or an empty string, and guard accordingly to provide better user feedback.rag/rag-springai-ollama-llm/src/main/resources/application.properties (2)
18-19: Consider shorter timeouts with retry mechanismThe current timeout values (1m connect, 5m read) are quite long and might impact user experience. Consider:
- Reducing timeouts to more reasonable values (e.g., 10s connect, 30s read)
- Implementing a retry mechanism for failed requests
-spring.http.client.connect-timeout=PT1M -spring.http.client.read-timeout=PT5M +spring.http.client.connect-timeout=PT10S +spring.http.client.read-timeout=PT30S +spring.cloud.httpclient.ok-http.retry.max-attempts=3 +spring.cloud.httpclient.ok-http.retry.initial-interval=1000
34-34: Consider environment-specific logging levelsDebug logging for
org.springframework.ai.ragmight generate excessive logs in production.Consider moving this to an environment-specific properties file (e.g.,
application-local.properties):-logging.level.org.springframework.ai.rag=debug +# In application-local.properties only: +logging.level.org.springframework.ai.rag=debug +# In application.properties: +logging.level.org.springframework.ai.rag=info
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
rag/rag-springai-ollama-llm/pom.xmlis excluded by!**/*.xmlrag/rag-springai-ollama-llm/src/main/resources/logback-spring.xmlis excluded by!**/*.xml
📒 Files selected for processing (4)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java(0 hunks)rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java(2 hunks)rag/rag-springai-ollama-llm/src/main/resources/application.properties(1 hunks)rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java(2 hunks)
💤 Files with no reviewable changes (1)
- rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java
🔇 Additional comments (9)
rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java (2)
8-8: Verify that the imported container library versions align with your project's dependencies.
The PostgreSQLContainer import looks good. However, ensure that the testcontainers library version used in this project matches the new container references, including pgvector.
35-37: Ensure the updated image tag meets your observability requirements.
You've updated the LgtmStackContainer to tag 0.8.1. Double-check the plugin or feature support in 0.8.1 to confirm that it satisfies any new observability or tracing requirements introduced in this PR. Also verify that the 2 minute startup timeout accommodates potential slow resource allocation in CI environments.
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (6)
6-9: Imports are well-organized and consistent with the new retrieval-augmented approach.
These imports cleanly reference the new classes, aligning with the RAG (Retrieval-Augmented Generation) workflow. No issues identified.
25-25: Updated placeholder in the template ensures clarity.
Replacing the placeholder with {query} is straightforward and improves readability. Ensure that any code referencing the older placeholder is updated to avoid mismatches.
38-42: Allowing empty context can lead to greater adaptability but may increase hallucinations.
Be mindful that enabling an empty context might lower the model’s grounding. Consider employing guardrails or fallback strategies when no relevant context is found.
43-47: Advisor creation is consistent with the new RAG flow.
This chain of configuring the retriever and query augmenter is cohesive. No issues identified.
48-48: Cloning the builder to inject the new advisors is a clean pattern.
This approach avoids side effects on the original builder instance and fosters a well-contained configuration.
52-52: Consider error handling when calling the chat client.
The .call() method could potentially fail. You may want to add try-catch or handle error responses to avoid uncaught exceptions.
rag/rag-springai-ollama-llm/src/main/resources/application.properties (1)
6-11: LGTM: Ollama AI configuration is well-tuned for RAG
The configuration uses appropriate values for RAG implementation:
- Pull strategy prevents unnecessary model downloads
- Temperature (0.3) and sampling parameters are well-balanced for deterministic responses
| management.endpoints.web.exposure.include=* | ||
| management.metrics.tags.service.name=${spring.application.name} | ||
| management.tracing.sampling.probability=1.0 | ||
| management.otlp.tracing.endpoint=http://localhost:4318/v1/traces | ||
| management.otlp.logging.endpoint=http://localhost:4318/v1/logs |
There was a problem hiding this comment.
Security: Restrict management endpoint exposure
Exposing all management endpoints (management.endpoints.web.exposure.include=*) could pose a security risk in production environments.
Consider:
- Limiting exposed endpoints to necessary ones only
- Securing the endpoints with appropriate authentication
-management.endpoints.web.exposure.include=*
+management.endpoints.web.exposure.include=health,metrics,prometheus,info
+management.endpoints.web.exposure.exclude=env,beans📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| management.endpoints.web.exposure.include=* | |
| management.metrics.tags.service.name=${spring.application.name} | |
| management.tracing.sampling.probability=1.0 | |
| management.otlp.tracing.endpoint=http://localhost:4318/v1/traces | |
| management.otlp.logging.endpoint=http://localhost:4318/v1/logs | |
| management.endpoints.web.exposure.include=health,metrics,prometheus,info | |
| management.endpoints.web.exposure.exclude=env,beans | |
| management.metrics.tags.service.name=${spring.application.name} | |
| management.tracing.sampling.probability=1.0 | |
| management.otlp.tracing.endpoint=http://localhost:4318/v1/traces | |
| management.otlp.logging.endpoint=http://localhost:4318/v1/logs |
| ## only for development | ||
| spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true | ||
| spring.ai.ollama.baseUrl=http://localhost:11434 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Separate development configuration
Development-specific settings should be moved to a dedicated profile configuration file.
Create application-local.properties and move these settings there:
-## only for development
-spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true
-spring.ai.ollama.baseUrl=http://localhost:11434This helps prevent accidental activation of development settings in other environments.
Committable suggestion skipped: line range outside the PR's diff.
| #PgVector | ||
| spring.ai.vectorstore.observations.include-query-response=true | ||
| spring.ai.vectorstore.pgvector.initialize-schema=true |
There was a problem hiding this comment.
💡 Codebase verification
Missing required PostgreSQL connection properties for PgVector
The PgVector configuration is incomplete. Based on the example from pgvector-springai, the following essential properties are missing:
spring.datasource.urlspring.datasource.usernamespring.datasource.password
These properties should be added to either application.properties or application-local.properties (which currently doesn't exist) to ensure proper connection to the PostgreSQL database with PgVector extension.
Example of required properties:
spring.datasource.url=jdbc:postgresql://localhost/appdb
spring.datasource.username=appuser
spring.datasource.password=secret🔗 Analysis chain
Verify PgVector connection properties
While schema initialization is configured, the PostgreSQL connection properties appear to be missing. These are typically required for PgVector to function.
Let's check if these properties are defined elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PostgreSQL connection properties in all property files
fd -e properties -e yaml | xargs rg -l "spring\.datasource\.(url|username|password)"
Length of output: 403
Script:
#!/bin/bash
# Let's check the content of application-local.properties in the same directory
cat rag/rag-springai-ollama-llm/src/main/resources/application-local.properties 2>/dev/null || echo "File not found"
# Also check if there are any other property files in the same directory
ls rag/rag-springai-ollama-llm/src/main/resources/
# Let's also examine one of the found PgVector examples to see the required properties
cat embeddingstores/pgvector-springai/src/main/resources/application-local.properties
Length of output: 428
rajadilipkolli
force-pushed
the
rag-ollama
branch
from
8590993 to
ded82f7
Compare
rajadilipkolli
force-pushed
the
rag-ollama
branch
from
ded82f7 to
cbe0c01
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
rag/rag-springai-ollama-llm/src/main/resources/application.properties (2)
14-16:⚠️ Potential issueCritical: Missing PostgreSQL connection configuration
While PgVector initialization is configured, the essential PostgreSQL connection properties are missing:
spring.datasource.url=jdbc:postgresql://localhost:5432/your_database spring.datasource.username=your_username spring.datasource.password=your_password
28-28:⚠️ Potential issueSecurity: Restrict management endpoint exposure
Exposing all management endpoints (
management.endpoints.web.exposure.include=*) could pose a security risk in production environments.
🧹 Nitpick comments (5)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java (1)
67-67: Consider providing a custom error message or fallback
Objects.requireNonNull(this.vectorStore.similaritySearch("*"))will throw aNullPointerExceptionwith a generic message ifsimilaritySearch("*")happens to returnnull. If that scenario is possible, a custom message or a fallback approach can make it easier to debug and handle.rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (2)
1-2: Add a warning comment for the destructive configurationThe
removeExistingVectorStoreTable=truesetting is destructive and should be clearly marked as development-only. Consider adding a prominent warning comment.-## only for development +## WARNING: Development-only configuration +## The following setting will DELETE existing vector store data on startup spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true
3-3: Consider parameterizing the Ollama service URLThe Ollama service URL is hardcoded. Consider making it configurable through environment variables for better flexibility across different environments.
-spring.ai.ollama.baseUrl=http://localhost:11434 +spring.ai.ollama.baseUrl=${OLLAMA_SERVICE_URL:http://localhost:11434}rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (1)
21-24: Consider making similarity threshold configurable.The hardcoded similarity threshold of 0.50 might need adjustment based on your specific use case. Consider making this configurable through application properties for easier tuning in different environments.
- .similarityThreshold(0.50) + @Value("${ai.vector.similarity-threshold:0.50}") + private double similarityThreshold; + // In constructor: + .similarityThreshold(similarityThreshold)rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java (1)
17-18: Consider pinning the Ollama container version for test stability.The current configuration uses the latest tag implicitly. To ensure consistent test environments across different machines and CI runs, consider pinning to a specific version.
- return new OllamaContainer(DockerImageName.parse("ollama/ollama")); + return new OllamaContainer(DockerImageName.parse("ollama/ollama:0.1.29"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
rag/rag-springai-ollama-llm/docker/docker-compose.ymlis excluded by!**/*.ymlrag/rag-springai-ollama-llm/docker/docker_pgadmin_servers.jsonis excluded by!**/*.jsonrag/rag-springai-ollama-llm/pom.xmlis excluded by!**/*.xmlrag/rag-springai-ollama-llm/src/main/resources/logback-spring.xmlis excluded by!**/*.xml
📒 Files selected for processing (6)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java(0 hunks)rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java(2 hunks)rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java(2 hunks)rag/rag-springai-ollama-llm/src/main/resources/application-local.properties(1 hunks)rag/rag-springai-ollama-llm/src/main/resources/application.properties(1 hunks)rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java(2 hunks)
💤 Files with no reviewable changes (1)
- rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java
🔇 Additional comments (9)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java (1)
4-4: Good use of Objects for null safety
Using the Objects class ensures readability and self-documenting code around null checks.
rag/rag-springai-ollama-llm/src/main/resources/application.properties (3)
6-11: LGTM: Efficient model management strategy
The WHEN_MISSING pull strategy is a good choice for optimizing startup time while ensuring model availability.
18-19: LGTM: Well-configured timeout settings
The timeout settings are appropriate for LLM operations and properly use the ISO-8601 duration format.
24-34: LGTM: Comprehensive observability configuration
The observability settings are well-configured with:
- Detailed chat observations
- OpenTelemetry logging endpoint
- Appropriate logging levels
These align perfectly with the PR objective of polishing observability.
rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (1)
6-6: Review debug logging for production readiness
Debug logging is enabled for the Spring AI RAG package. While appropriate for local development, ensure this level of logging is adjusted appropriately in production configurations to prevent performance impact and potential exposure of sensitive information.
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (3)
6-8: LGTM! Clean import structure for RAG components.
The new imports properly reflect the shift to a retrieval-augmented generation approach using vector store and contextual query augmentation.
19-34: Verify Spring AI compatibility with Spring Boot 3.4.
Since this PR involves upgrading to Spring Boot 3.4, ensure that the Spring AI dependencies and their features (especially the new RAG components) are compatible with this version.
✅ Verification successful
Spring AI 1.0.0-M5 is compatible with Spring Boot 3.4.1
The project is correctly configured with:
- Spring Boot 3.4.1
- Spring AI 1.0.0-M5 (latest milestone release)
- Required Spring AI dependencies are properly managed through the BOM
- Spring Milestones repository is correctly configured
The RAG components being used (VectorStoreDocumentRetriever, ContextualQueryAugmenter, RetrievalAugmentationAdvisor) are part of the stable API in Spring AI 1.0.0-M5 and are fully compatible with Spring Boot 3.4.1.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Spring AI version compatibility
grep -r "spring-ai.version\|spring-boot.version" pom.xml
Length of output: 99
Script:
#!/bin/bash
# Let's try to find the pom.xml file first
fd pom.xml
# Also search for gradle files as an alternative
fd "build.gradle|gradle.properties"
Length of output: 459
Script:
#!/bin/bash
# Check the specific project's pom.xml for Spring Boot and Spring AI versions
cat rag/rag-springai-ollama-llm/pom.xml
Length of output: 9882
26-27: Verify empty context handling.
Setting allowEmptyContext=true means the system will proceed even when no relevant context is found. This could potentially lead to less accurate or relevant responses. Consider logging when empty contexts occur to monitor this behavior.
- ContextualQueryAugmenter.builder().allowEmptyContext(true).build();
+ ContextualQueryAugmenter.builder()
+ .allowEmptyContext(true)
+ .onEmptyContext(context ->
+ LOGGER.warn("No relevant context found for query"))
+ .build();✅ Verification successful
Let me verify the handling of empty contexts by checking the codebase for any existing logging or handling mechanisms.
Let me check the complete implementation of AIChatService to understand the context better.
Empty context handling is acceptable with existing logging
The current implementation already includes logging through SLF4J, and the service logs all AI responses using LOGGER.info(). Since the VectorStoreDocumentRetriever is configured with a similarity threshold of 0.50, and the responses are logged, we can effectively monitor the quality of responses, including cases where no context is found.
- The service uses
similarityThreshold(0.50)which helps filter out irrelevant contexts - All AI responses are logged via
LOGGER.info("Response received from call :{}", aiResponse) - Setting
allowEmptyContext(true)is reasonable as it prevents request failures when no context is found
While adding specific empty context logging could be helpful, the current implementation provides sufficient monitoring capabilities through response logging.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for ContextualQueryAugmenter usage and configuration
rg -l "ContextualQueryAugmenter"
# Check for any logging related to empty contexts
rg -i "empty.?context"
# Look for the specific class implementation
ast-grep --pattern 'class AIChatService {
$$$
}'
Length of output: 402
Script:
#!/bin/bash
# Get the complete implementation of AIChatService
cat rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java
Length of output: 1737
rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java (1)
28-31: LGTM stack upgrade looks good.
The upgrade to version 0.8.1 aligns with the PR objective of polishing observability. The configuration is properly annotated with @Serviceconnection and maintains a reasonable startup timeout.
Let's verify the compatibility of the new version:
✅ Verification successful
LGTM stack version upgrade is consistently applied across the codebase
The version upgrade to 0.8.1 has been consistently applied in all relevant configurations:
- Test container configuration in
rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java - Docker compose configuration in
rag/rag-springai-ollama-llm/docker/docker-compose.yml - Docker compose configuration in
chatbot/chatbot-ollama-springai/docker/docker-compose.yml
All configurations maintain consistent settings including the OTEL metric export interval of 500ms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any breaking changes in LGTM stack configuration
rg -A 5 "grafana/otel-lgtm" || echo "No other LGTM stack references found"
# Check application properties for related observability settings
fd -e properties -e yaml | xargs grep -l "otel" || echo "No OpenTelemetry configuration files found"
Length of output: 1883
| spring.datasource.url=jdbc:postgresql://localhost/appdb | ||
| spring.datasource.username=appuser | ||
| spring.datasource.password=secret |
There was a problem hiding this comment.
Secure database credentials and add connection pool settings
Several security and configuration concerns:
- Database credentials should not be stored in properties files. Use environment variables or a secure configuration management system.
- Connection pool settings are missing, which could affect application performance.
Apply these changes:
-spring.datasource.url=jdbc:postgresql://localhost/appdb
-spring.datasource.username=appuser
-spring.datasource.password=secret
+spring.datasource.url=${SPRING_DATASOURCE_URL:jdbc:postgresql://localhost/appdb}
+spring.datasource.username=${SPRING_DATASOURCE_USERNAME}
+spring.datasource.password=${SPRING_DATASOURCE_PASSWORD}
+
+# Connection pool settings
+spring.datasource.hikari.maximum-pool-size=10
+spring.datasource.hikari.minimum-idle=5
+spring.datasource.hikari.idle-timeout=300000Also, consider using Spring Cloud Config Server or HashiCorp Vault for secure credential management in a production environment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spring.datasource.url=jdbc:postgresql://localhost/appdb | |
| spring.datasource.username=appuser | |
| spring.datasource.password=secret | |
| spring.datasource.url=${SPRING_DATASOURCE_URL:jdbc:postgresql://localhost/appdb} | |
| spring.datasource.username=${SPRING_DATASOURCE_USERNAME} | |
| spring.datasource.password=${SPRING_DATASOURCE_PASSWORD} | |
| # Connection pool settings | |
| spring.datasource.hikari.maximum-pool-size=10 | |
| spring.datasource.hikari.minimum-idle=5 | |
| spring.datasource.hikari.idle-timeout=300000 |
| public String chat(String query) { | ||
| ChatResponse aiResponse = aiClient.prompt() | ||
| .advisors(new QuestionAnswerAdvisor(vectorStore, SearchRequest.query(query), template)) | ||
| .user(query) | ||
| .call() | ||
| .chatResponse(); | ||
| String aiResponse = aiClient.prompt().user(query).call().content(); | ||
| LOGGER.info("Response received from call :{}", aiResponse); | ||
| Generation generation = aiResponse.getResult(); | ||
| return (generation != null) ? generation.getOutput().getContent() : ""; | ||
| return aiResponse; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and observability.
The current implementation lacks proper error handling and could benefit from enhanced observability, especially given the PR's objective to "polish observability".
Consider implementing these improvements:
public String chat(String query) {
+ if (query == null || query.trim().isEmpty()) {
+ throw new IllegalArgumentException("Query cannot be empty");
+ }
+ LOGGER.debug("Processing chat query: {}", query);
+ try {
+ long startTime = System.currentTimeMillis();
String aiResponse = aiClient.prompt().user(query).call().content();
- LOGGER.info("Response received from call :{}", aiResponse);
+ long duration = System.currentTimeMillis() - startTime;
+ LOGGER.info("Chat response received in {}ms. Query: '{}', Response: '{}'",
+ duration, query, aiResponse);
return aiResponse;
+ } catch (Exception e) {
+ LOGGER.error("Error processing chat query: '{}'. Error: {}",
+ query, e.getMessage(), e);
+ throw new RuntimeException("Failed to process chat query", e);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String chat(String query) { | |
| ChatResponse aiResponse = aiClient.prompt() | |
| .advisors(new QuestionAnswerAdvisor(vectorStore, SearchRequest.query(query), template)) | |
| .user(query) | |
| .call() | |
| .chatResponse(); | |
| String aiResponse = aiClient.prompt().user(query).call().content(); | |
| LOGGER.info("Response received from call :{}", aiResponse); | |
| Generation generation = aiResponse.getResult(); | |
| return (generation != null) ? generation.getOutput().getContent() : ""; | |
| return aiResponse; | |
| public String chat(String query) { | |
| if (query == null || query.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Query cannot be empty"); | |
| } | |
| LOGGER.debug("Processing chat query: {}", query); | |
| try { | |
| long startTime = System.currentTimeMillis(); | |
| String aiResponse = aiClient.prompt().user(query).call().content(); | |
| long duration = System.currentTimeMillis() - startTime; | |
| LOGGER.info("Chat response received in {}ms. Query: '{}', Response: '{}'", | |
| duration, query, aiResponse); | |
| return aiResponse; | |
| } catch (Exception e) { | |
| LOGGER.error("Error processing chat query: '{}'. Error: {}", | |
| query, e.getMessage(), e); | |
| throw new RuntimeException("Failed to process chat query", e); | |
| } | |
| } |
| @ServiceConnection | ||
| PostgreSQLContainer<?> postgreSQLContainer() { | ||
| return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17")); |
There was a problem hiding this comment.
💡 Codebase verification
PostgreSQL 17 is compatible, but timeout setting recommended
The verification shows:
- The project uses Spring Boot 3.4.1 which is compatible with PostgreSQL 17
- PgVector extension is properly configured with
spring.ai.vectorstore.pgvector.initialize-schema=true - The container configuration is correct, but lacks a startup timeout setting
Adding a startup timeout is recommended for consistent behavior in CI environments:
- return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"));
+ return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"))
+ .withStartupTimeout(Duration.ofMinutes(2));Note that other containers in the same configuration file (like LGTM) already use timeout settings for consistency.
🔗 Analysis chain
Verify PostgreSQL 17 compatibility and consider adding startup timeout.
Two suggestions for robustness:
- PostgreSQL 17 is very recent. Verify compatibility with your Spring Boot 3.4 upgrade and pgvector extension.
- Consider adding a startup timeout similar to the LGTM container for consistent behavior in CI environments.
- return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"));
+ return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"))
+ .withStartupTimeout(Duration.ofMinutes(2));Let's verify the Spring Boot and pgvector compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Spring Boot's default PostgreSQL driver version
cat pom.xml | grep -A 5 postgresql || echo "PostgreSQL dependency not found in pom.xml"
# Check if pgvector extension is properly initialized in tests
rg -l "CREATE EXTENSION IF NOT EXISTS vector"
Length of output: 219
Script:
#!/bin/bash
# Let's first check if we're in the right directory and locate the pom.xml
fd pom.xml
# Check for any SQL files or Java files that might contain vector extension initialization
rg -t sql -t java "vector" -l
# Look for Spring Boot version in any pom.xml files
fd pom.xml --exec grep -A 5 "spring-boot"
# Check for PostgreSQL related configuration or beans
rg -t java "PostgreSQL|postgres" -l
Length of output: 19096
Script:
#!/bin/bash
# Let's check the specific project's pom.xml for PostgreSQL driver version
cat rag/rag-springai-ollama-llm/pom.xml | grep -A 5 "postgresql"
# Check application properties for any PostgreSQL configuration
cat rag/rag-springai-ollama-llm/src/main/resources/application.properties
cat rag/rag-springai-ollama-llm/src/main/resources/application-local.properties
# Look for vector extension initialization in the test configuration
cat rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java
Length of output: 3235
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (1)
9-11:⚠️ Potential issueSecure database credentials and add connection pool settings
Several security and configuration concerns remain unaddressed from the previous review.
Please refer to the previous review comment for the recommended changes regarding:
- Externalizing database credentials
- Adding connection pool settings
🧹 Nitpick comments (1)
rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (1)
4-4: Externalize Ollama base URL configurationThe Ollama base URL should be externalized to support different environments.
-spring.ai.ollama.baseUrl=http://localhost:11434 +spring.ai.ollama.baseUrl=${OLLAMA_BASE_URL:http://localhost:11434}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java(2 hunks)rag/rag-springai-ollama-llm/src/main/resources/application-local.properties(1 hunks)rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/DataIndexerService.java
🔇 Additional comments (4)
rag/rag-springai-ollama-llm/src/main/resources/application-local.properties (1)
7-7: Verify debug logging is environment-specific
Debug-level logging can impact performance and generate excessive logs in production.
Let's verify the logging configuration across environments:
✅ Verification successful
Debug logging is properly configured for environments
The verification shows that debug logging is correctly configured:
- Debug level is only set in
application-local.properties - The default logging level in
application.propertiesis appropriately set toinfo
This confirms that debug logging won't impact production performance as it's specifically scoped to the local profile.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for RAG logging configuration in all property files
fd -e properties -e yml -e yaml | xargs rg "logging.level.org.springframework.ai.rag"
Length of output: 324
rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java (3)
23-25: PostgreSQL container configuration looks good!
The implementation follows best practices with:
- Correct pgvector image selection
- Appropriate startup timeout configuration
31-32: Verify LGTM stack version compatibility
The LGTM stack version has been upgraded from 0.7.1 to 0.8.1. Please verify compatibility with Spring Boot 3.4.
✅ Verification successful
LGTM stack version 0.8.1 is compatible with Spring Boot 3.4
The version upgrade from 0.7.1 to 0.8.1 of the LGTM stack (Grafana/OpenTelemetry) is compatible with the project's configuration:
- The project uses OpenTelemetry instrumentation version 2.10.0-alpha which is compatible with LGTM stack 0.8.1
- The same LGTM stack version (0.8.1) is consistently used in both test configuration and docker-compose
- The configuration is properly set up with Spring Boot's
@ServiceConnectionfor testcontainers integration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any LGTM-related configuration or dependencies that might need updates
rg -t java -t yaml -t properties "lgtm|grafana|otel"
# Check Spring Boot version
fd pom.xml --exec grep -A 5 "spring-boot"
Length of output: 15168
Script:
#!/bin/bash
# Let's check for LGTM-related configurations with proper file type flags
rg -t java -t xml -t yaml "lgtm|grafana|otel"
# Let's also check the specific file for any LGTM configuration
cat rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/TestcontainersConfiguration.java
Length of output: 3232
17-18: Verify the impact of simplified Ollama container initialization
The container initialization has been simplified by removing exception handling. Please verify that this doesn't impact error reporting during container startup failures.
✅ Verification successful
Simplified Ollama container initialization is safe to use
Based on the search results, the simplified Ollama container initialization is consistent with standard Testcontainers usage patterns. The @ServiceConnection annotation indicates that Spring Boot's test container support is handling the container lifecycle, which includes proper error handling at the framework level. The removal of explicit exception handling is actually an improvement as it follows the framework's conventions.
Key observations:
- Both implementations in the codebase (
rag-springai-ollama-llmandchatbot-ollama-springai) use the same simplified pattern - Spring Boot's test container support (
@ServiceConnection) manages container lifecycle - No custom exception handling was found in the codebase for Ollama containers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any existing error handling or startup validation in test classes
rg -t java "ollama.*catch|ollama.*throws"
Length of output: 43
Script:
#!/bin/bash
# Let's check the broader context of Ollama container usage in tests
rg -t java -B 3 -A 3 "OllamaContainer"
# Also check for any test-related exception handling patterns
rg -t java -B 2 -A 2 "@Test.*throws|catch.*Exception" test/
Length of output: 4299
Script:
#!/bin/bash
# Let's check the Testcontainers library's OllamaContainer implementation
ast-grep --pattern 'class OllamaContainer {
$$$
}'
# Check for any test classes that might be handling container startup
rg -t java -B 3 -A 3 "@SpringBootTest|@TestConfiguration" src/test/
Length of output: 174
| ## WARNING: Development-only configuration | ||
| ## The following setting will DELETE existing vector store data on startup | ||
| spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=true |
There was a problem hiding this comment.
💡 Codebase verification
The dangerous vector store table removal setting is not properly isolated. It's currently set in:
rag/rag-springai-ollama-llm/src/main/resources/application-local.properties(local profile)embeddingstores/pgvector-springai/src/main/resources/application.properties(default configuration)
Having this setting in the default application.properties is risky as it could lead to unintended data loss in production. This setting should only exist in local development profiles.
Recommendations:
- Remove
spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable=truefromembeddingstores/pgvector-springai/src/main/resources/application.properties - Keep it only in
*-local.propertiesfiles - Consider adding a warning comment in the default properties file about this setting being development-only
🔗 Analysis chain
Verify configuration isolation for data deletion setting
The vector store table removal setting is dangerous and should be strictly isolated to local development.
Let's verify this setting isn't present in other profiles:
Let me verify if this setting is properly isolated by checking the context of its usage in the other file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for vector store removal setting in all property files
fd -e properties -e yml -e yaml | xargs rg "spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable"
Length of output: 392
Script:
#!/bin/bash
# Get context for the other occurrence
rg -B2 -A2 "spring.ai.vectorstore.pgvector.removeExistingVectorStoreTable" embeddingstores/pgvector-springai/src/main/resources/application.properties
Length of output: 342
No description provided.