-
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 : polish spring chat model #36
Conversation
WalkthroughThe project saw a comprehensive overhaul to improve code quality and adapt to a new data handling approach. Key changes include adopting the 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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (7)
- chatmodel-springai/pom.xml (2 hunks)
- chatmodel-springai/src/main/java/com/example/ai/controller/ChatController.java (2 hunks)
- chatmodel-springai/src/main/java/com/example/ai/model/request/AIChatRequest.java (1 hunks)
- chatmodel-springai/src/main/resources/application.properties (1 hunks)
- chatmodel-springai/src/test/java/com/example/ai/controller/ChatControllerTest.java (2 hunks)
- rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (2 hunks)
- rag/rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (2 hunks)
Additional comments not posted (8)
chatmodel-springai/src/main/java/com/example/ai/model/request/AIChatRequest.java (1)
1-3
: LGTM! Using a record forAIChatRequest
is a suitable choice for immutable data representation.chatmodel-springai/src/main/resources/application.properties (1)
9-9
: LGTM! Enabling thespring.ai.openai.embedding
feature aligns with the PR objectives.chatmodel-springai/src/test/java/com/example/ai/controller/ChatControllerTest.java (2)
6-8
: LGTM! The addition ofAIChatRequest
andContentType
imports supports the updated testing approach.
29-54
: LGTM! Transitioning to POST methods and usingAIChatRequest
for request bodies are correctly implemented. Consider adding more comprehensive tests to cover edge cases if not already present.chatmodel-springai/src/main/java/com/example/ai/controller/ChatController.java (1)
23-64
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [3-63]
LGTM! The transition to POST methods, use of
AIChatRequest
, and introduction ofEmbeddingClient
are well-implemented. EnsureEmbeddingClient
is properly configured and injected.rag/rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (1)
56-62
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [8-60]
LGTM! The use of
Generation
for output content retrieval standardizes AI-generated content handling across the application.rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/service/AIChatService.java (1)
65-71
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [10-69]
LGTM! Consistent use of
Generation
for output content retrieval across different modules enhances the standardization of AI-generated content handling.chatmodel-springai/pom.xml (1)
76-98
: LGTM! The addition ofspotless-maven-plugin
for code formatting is a positive change. Ensure the chosen formatting rules align with the project's coding standards.
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/rag-springai-ollama-llm.yml (1 hunks)
Additional comments not posted (1)
.github/workflows/rag-springai-ollama-llm.yml (1)
28-28
: Consider the implications of restricting CI builds toubuntu-latest
.Restricting the operating systems to
ubuntu-latest
might simplify the CI pipeline and reduce costs, but it could also impact the cross-platform compatibility testing of the application. Ensure this change aligns with the project's requirements and consider documenting the rationale behind this decision for future reference.
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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (3)
- chatmodel-springai/src/main/java/com/example/ai/config/LoggingConfig.java (3 hunks)
- rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java (1 hunks)
- rag/rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/config/ResponseHeadersModification.java (2 hunks)
Additional comments not posted (3)
rag/rag-springai-ollama-llm/src/main/java/com/learning/ai/llmragwithspringai/config/RestClientBuilderConfig.java (1)
14-18
: Consider making the timeout values configurable through application properties to accommodate different environments or use cases.rag/rag-springai-openai-llm/src/main/java/com/learning/ai/llmragwithspringai/config/ResponseHeadersModification.java (1)
22-23
: LGTM! Consider adding a comment to clarify the intended use case for modifying theContent-Type
header, ensuring it aligns with application requirements.chatmodel-springai/src/main/java/com/example/ai/config/LoggingConfig.java (1)
27-28
: LGTM! Ensure that logging does not inadvertently expose sensitive information, such as API keys or personal data.
No description provided.