-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat : adds ollama-llama2 RAG sample #26
Conversation
WalkthroughThe update enriches a Spring Boot application for AI chat functionality, emphasizing the integration of the Ollama model with pgvector store support. It encompasses Maven configuration updates, guidance on pgvector usage, and various application enhancements like global exception handling, Swagger setup, and improvements to the AI chat service. Moreover, the testing infrastructure is reinforced with integration tests and Testcontainers setup for Ollama and Redis. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
rag-springai-ollama-llm/pom.xml
is excluded by!**/*.xml
rag-springai-ollama-llm/src/main/resources/Rohit_Gurunath_Sharma.pdf
is excluded by!**/*.pdf
Files selected for processing (17)
- .github/workflows/rag-springai-ollama-llm.yml (1 hunks)
- rag-springai-ollama-llm/.gitignore (1 hunks)
- rag-springai-ollama-llm/.mvn/wrapper/maven-wrapper.properties (1 hunks)
- rag-springai-ollama-llm/ReadMe.md (1 hunks)
- rag-springai-ollama-llm/mvnw (1 hunks)
- rag-springai-ollama-llm/mvnw.cmd (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplication.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/AppConfig.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/GlobalExceptionHandler.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/ResponseHeadersModification.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/controller/AiController.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (1 hunks)
- rag-springai-ollama-llm/src/main/resources/application.properties (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/TestLlmRagWithSpringAiApplication.java (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/AbstractIntegrationTest.java (1 hunks)
Files skipped from review due to trivial changes (2)
- rag-springai-ollama-llm/.mvn/wrapper/maven-wrapper.properties
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java
Additional comments (26)
rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplication.java (1)
- 1-12: The addition of the main application class follows the standard Spring Boot setup and is correctly implemented.
rag-springai-ollama-llm/src/main/resources/application.properties (1)
- 1-12: The configurations in
application.properties
are well-defined and align with the objectives of integrating the ollama-llama2 RAG model and Redis for vector storage.rag-springai-ollama-llm/.gitignore (1)
- 1-33: The updates to the
.gitignore
file correctly exclude build artifacts, IDE-specific files, and other non-source code files, ensuring a clean repository.rag-springai-ollama-llm/ReadMe.md (1)
- 1-13: The
ReadMe.md
file provides clear guidance on using pgvector with the ollama model and Testcontainers support. Ensure that all Docker image tags used in development are consistent with those in production to avoid environment discrepancies.rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/config/AbstractIntegrationTest.java (1)
- 1-23: The setup in
AbstractIntegrationTest.java
correctly configures the Spring Boot test environment and provides necessary beans for integration testing, following best practices..github/workflows/rag-springai-ollama-llm.yml (1)
- 1-39: The GitHub Actions workflow is well-structured and aligns with CI/CD best practices, ensuring code quality and functionality with each push or pull request.
rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/controller/AiController.java (1)
- 1-35: The
AiController
is well-implemented, correctly handling AI chat queries with proper input validation and response formatting, following REST API design best practices.rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/TestLlmRagWithSpringAiApplication.java (1)
- 1-38: The configuration in
TestLlmRagWithSpringAiApplication.java
correctly sets up test containers for Ollama and Redis, ensuring a robust testing environment.rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/AppConfig.java (3)
- 20-21: Consider externalizing the PDF resource path to a properties file or environment variable for easier configuration and to avoid recompilation for changes.
- 28-42: Ensure proper error handling for the PDF document reading and vector store loading process. Consider catching and logging potential exceptions to avoid application crashes.
- 32-38: The configuration for
PdfDocumentReaderConfig
is hardcoded. Consider externalizing these values to a configuration file or environment variables for greater flexibility and easier adjustments.rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/ResponseHeadersModification.java (2)
- 18-18: Ensure that the conditional property
spring.ai.openai.api-key
is correctly set to "demo" for this configuration to be applied. If this is for testing purposes, consider documenting or providing alternative configurations for production.- 22-26: This custom response wrapper modifies all responses to have a content type of
application/json
. Verify that this behavior is intended for all endpoints, as it might not be suitable for endpoints returning different content types.rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (3)
- 19-37: The template string for the chat prompt is hardcoded. Consider externalizing this template to a configuration file or database to allow for easy updates without code changes.
- 47-60: Ensure that the input
message
is sanitized or validated before being used in the similarity search to prevent potential injection attacks or unexpected behavior.- 49-52: When concatenating document contents, ensure that the separator (system line separator) is appropriate for the model's expected input format. Inconsistent formatting may affect the model's ability to understand the context.
rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/GlobalExceptionHandler.java (2)
- 22-41: The method
onException
for handlingMethodArgumentNotValidException
constructs aProblemDetail
object with hardcoded status and detail messages. Consider using a more dynamic approach or externalizing these messages for localization or customization.- 43-58: Similar to the previous comment, the handling of
ConstraintViolationException
also uses hardcoded messages. Additionally, ensure that the field names and messages inApiValidationError
are meaningful and user-friendly.rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java (4)
- 26-33: This test case asserts that a specific response is returned for a given query. Ensure that the test data setup is consistent and that the response is not overly specific to avoid flakiness.
- 36-49: The test case for an empty query correctly expects a 400 status code. However, ensure that the validation logic for the query parameter is implemented in the controller to match this expectation.
- 52-66: For the test case handling long query strings, ensure that there's a defined maximum length for queries in the application logic and that it's documented for API consumers.
- 69-82: The test for special characters in the query checks for a 400 status code. Verify that the application has appropriate sanitization or validation rules for query parameters to prevent injection attacks or processing errors.
rag-springai-ollama-llm/mvnw.cmd (2)
- 21-21: The Maven Wrapper version is specified as 3.2.0. Ensure this version is compatible with the project's Maven version and requirements.
- 122-122: The URL for downloading the Maven Wrapper jar is hardcoded. Ensure that this URL points to a trusted and reliable Maven repository.
rag-springai-ollama-llm/mvnw (2)
- 22-22: The Maven Wrapper version is specified as 3.2.0. As with the Windows version, ensure this version is compatible with the project's Maven version and requirements.
- 199-199: The URL for downloading the Maven Wrapper jar is hardcoded. Confirm that this URL points to a trusted and reliable Maven repository, similar to the Windows script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- rag-springai-ollama-llm/src/main/resources/application.properties (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/TestLlmRagWithSpringAiApplication.java (1 hunks)
- rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java (1 hunks)
- rag-springai-openai-llm/src/main/resources/application.properties (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- rag-springai-ollama-llm/src/main/resources/application.properties
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/TestLlmRagWithSpringAiApplication.java
Additional comments (2)
rag-springai-openai-llm/src/main/resources/application.properties (1)
- 1-1: The application name change is clear and aligns with the PR's objectives. Ensure that the
spring.ai.openai.api-key
is securely managed and not hardcoded in production environments.rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/config/SwaggerConfig.java (1)
- 9-9: The updates to the Swagger configuration align with the application's renaming. It's recommended to verify the Swagger UI to ensure the documentation accurately reflects the new title and version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/controller/AiController.java (1 hunks)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (1 hunks)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/controller/AiController.java
- rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java
Summary by CodeRabbit
Content-Type
toapplication/json
.