-
Notifications
You must be signed in to change notification settings - Fork 67
fix: add ai/search endpoint #281
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
Conversation
WalkthroughAdds a vector search API endpoint Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AiChatController
participant StorageService
participant VectorStore as "Vector DB / Store"
rect rgb(230,248,255)
Client->>AiChatController: POST /ai/search (SearchRequest)
end
AiChatController->>StorageService: search(SearchRequest)
StorageService->>VectorStore: vector similarity query
VectorStore-->>StorageService: List<EmbeddingMatchDto>
StorageService-->>AiChatController: List<EmbeddingMatchDto>
AiChatController-->>Client: Result.success(List<EmbeddingMatchDto>)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
🧹 Nitpick comments (1)
base/src/main/java/com/tinyengine/it/controller/AiChatController.java (1)
59-63: Field injection is acceptable, but constructor injection is preferred.The field injection works correctly. However, as an optional improvement, consider using constructor injection instead, which makes dependencies explicit, facilitates testing, and prevents potential issues with field injection in certain frameworks.
base/src/main/java/com/tinyengine/it/controller/AiChatController.java
Outdated
Show resolved
Hide resolved
… into feat/ai-chat
base/src/main/java/com/tinyengine/it/controller/AiChatController.java
Outdated
Show resolved
Hide resolved
base/src/main/java/com/tinyengine/it/controller/AiChatController.java
Outdated
Show resolved
Hide resolved
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
♻️ Duplicate comments (2)
base/src/main/java/com/tinyengine/it/controller/AiChatController.java (2)
74-76: Note: Formatting consistency already flagged.The formatting changes to these
@ApiResponseannotations have already been noted by another reviewer regarding code indentation style consistency with the project.Also applies to: 113-115, 148-150
179-181: Authorization header concern already raised.A previous review comment already noted that this endpoint is missing the
Authorizationheader parameter that other endpoints in this controller accept. Please address that feedback.
🧹 Nitpick comments (1)
base/src/main/java/com/tinyengine/it/controller/AiChatController.java (1)
179-181: Addthrows Exceptionfor consistency with other endpoints.All other endpoints in this controller declare
throws Exceptionin their method signatures (lines 81, 120, 154). Consider adding it here for consistency:public Result<List<EmbeddingMatchDto>> searchInCollection(@RequestBody SearchRequest searchDto) throws Exception {This maintains a uniform error-handling contract across the controller.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.