[Feat] 질문 임베딩 + search_queries 저장 - #45
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ut 5s) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough질의 임베딩 DTO와 외부 서버 연동 서비스를 추가하고, 임베딩 서버 장애 및 차원 불일치 오류를 처리한다. 검색 쿼리는 PROCESSING으로 저장한 뒤 SUCCESS 또는 FAILED로 갱신되며, 관련 단위 테스트와 설계 문서가 추가되었다. Changes검색 임베딩 및 쿼리 로깅
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/kangcheolung-`#44-search-embedding-query-logging.md:
- Line 102: 문서의 언어 식별자가 없는 네 개의 코드 펜스를 찾아 각각 ```text 또는 내용에 맞는 언어 태그를 추가하세요. 기존
코드 블록 내용과 문서 구조는 유지하고 markdownlint MD040 경고가 발생하지 않도록 수정하세요.
In
`@src/main/java/com/opensource/docgrid/domain/embedding/service/query/QueryEmbeddingService.java`:
- Around line 47-53: Update the validation before constructing EmbedResult in
QueryEmbeddingService to also reject null vectors and vectors whose length
differs from activeModel.getDimension(), alongside the existing response and
metadata-dimension checks. Ensure all mismatches log the actual vector dimension
when available and throw EMBEDDING_DIMENSION_MISMATCH; add coverage for a null
vector and a metadata/vector length mismatch.
- Around line 32-41: Update QueryEmbeddingService.embed to validate text at
method entry and reject null, empty, or whitespace-only input with the existing
400 validation mechanism before calling getActiveModel or the /embed endpoint.
Add a test covering blank input and confirming no external embedding request is
made.
In `@src/main/java/com/opensource/docgrid/domain/search/entity/SearchQuery.java`:
- Around line 105-113: Restrict the state transitions in
SearchQuery.updateToSuccess and SearchQuery.updateToFailed so they apply only
when the current status is PROCESSING; otherwise leave the status and associated
latencyMs/errorMessage unchanged. Add boundary tests covering repeated and
conflicting calls, including SUCCESS → FAILED and FAILED → SUCCESS.
In
`@src/test/java/com/opensource/docgrid/domain/search/service/command/SearchQueryCommandServiceTest.java`:
- Around line 36-46: Update the createProcessing test around
searchQueryRepository.save and SearchQueryCommandService.createProcessing to
capture the actual save argument with ArgumentCaptor<SearchQuery> instead of
asserting only the stubbed return value. Verify the captured SearchQuery has
PROCESSING status, the expected searchType, model, vector, and topK, and add the
required ArgumentCaptor and SearchType imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d80d0acb-9787-4bfe-93ac-3b9eb5b7800e
📒 Files selected for processing (15)
docs/design/kangcheolung-#44-search-embedding-query-logging.mdsrc/main/java/com/opensource/docgrid/domain/embedding/dto/EmbedResult.javasrc/main/java/com/opensource/docgrid/domain/embedding/dto/request/EmbedRequest.javasrc/main/java/com/opensource/docgrid/domain/embedding/dto/response/EmbedServerResponse.javasrc/main/java/com/opensource/docgrid/domain/embedding/service/query/QueryEmbeddingService.javasrc/main/java/com/opensource/docgrid/domain/search/entity/SearchQuery.javasrc/main/java/com/opensource/docgrid/domain/search/enums/ResultStatus.javasrc/main/java/com/opensource/docgrid/domain/search/repository/SearchQueryRepository.javasrc/main/java/com/opensource/docgrid/domain/search/service/command/SearchQueryCommandService.javasrc/main/java/com/opensource/docgrid/global/config/EmbeddingServerConfig.javasrc/main/java/com/opensource/docgrid/global/exception/ErrorCode.javasrc/main/resources/application.ymlsrc/test/java/com/opensource/docgrid/domain/embedding/service/query/QueryEmbeddingServiceTest.javasrc/test/java/com/opensource/docgrid/domain/search/fixture/SearchQueryFixture.javasrc/test/java/com/opensource/docgrid/domain/search/service/command/SearchQueryCommandServiceTest.java
|
|
||
| **`embed(String text)` 처리 흐름:** | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
코드 펜스에 언어 식별자를 추가하세요.
markdownlint-cli2의 MD040 경고가 발생한 네 블록에 text 또는 적절한 언어 태그를 지정해야 합니다.
수정 예시
-```
+```textAlso applies to: 127-127, 136-136, 144-144
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 102-102: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/kangcheolung-`#44-search-embedding-query-logging.md at line 102,
문서의 언어 식별자가 없는 네 개의 코드 펜스를 찾아 각각 ```text 또는 내용에 맞는 언어 태그를 추가하세요. 기존 코드 블록 내용과 문서
구조는 유지하고 markdownlint MD040 경고가 발생하지 않도록 수정하세요.
Source: Linters/SAST tools
| public EmbedResult embed(String text) { | ||
| EmbeddingModel activeModel = embeddingModelQueryService.getActiveModel(); | ||
|
|
||
| EmbedServerResponse response; | ||
| try { | ||
| response = restClient.post() | ||
| .uri("/embed") | ||
| .body(new EmbedRequest(text)) | ||
| .retrieve() | ||
| .body(EmbedServerResponse.class); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
빈 문자열 요청을 외부 서버 호출 전에 400으로 거부하세요.
현재 embed("") 또는 공백 문자열도 활성 모델 조회 후 /embed로 전송됩니다. 요구사항의 400 validation 계약을 충족하도록 서비스 진입 시점에 검증하고, 빈 입력 테스트도 추가해야 합니다.
수정 예시
public EmbedResult embed(String text) {
+ if (text == null || text.isBlank()) {
+ throw new DocGridException(ErrorCode.INVALID_PARAMETER);
+ }
+
EmbeddingModel activeModel = embeddingModelQueryService.getActiveModel();📝 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 EmbedResult embed(String text) { | |
| EmbeddingModel activeModel = embeddingModelQueryService.getActiveModel(); | |
| EmbedServerResponse response; | |
| try { | |
| response = restClient.post() | |
| .uri("/embed") | |
| .body(new EmbedRequest(text)) | |
| .retrieve() | |
| .body(EmbedServerResponse.class); | |
| public EmbedResult embed(String text) { | |
| if (text == null || text.isBlank()) { | |
| throw new DocGridException(ErrorCode.INVALID_PARAMETER); | |
| } | |
| EmbeddingModel activeModel = embeddingModelQueryService.getActiveModel(); | |
| EmbedServerResponse response; | |
| try { | |
| response = restClient.post() | |
| .uri("/embed") | |
| .body(new EmbedRequest(text)) | |
| .retrieve() | |
| .body(EmbedServerResponse.class); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/opensource/docgrid/domain/embedding/service/query/QueryEmbeddingService.java`
around lines 32 - 41, Update QueryEmbeddingService.embed to validate text at
method entry and reject null, empty, or whitespace-only input with the existing
400 validation mechanism before calling getActiveModel or the /embed endpoint.
Add a test covering blank input and confirming no external embedding request is
made.
| public void updateToSuccess(int latencyMs) { | ||
| this.status = ResultStatus.SUCCESS; | ||
| this.latencyMs = latencyMs; | ||
| } | ||
|
|
||
| public void updateToFailed(String errorMessage) { | ||
| this.status = ResultStatus.FAILED; | ||
| this.errorMessage = errorMessage; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
SearchQuery 상태 전이를 제한하세요.
현재 메서드는 PROCESSING 여부를 확인하지 않아 SUCCESS → FAILED, FAILED → SUCCESS 같은 재전이를 허용합니다. 재시도나 중복 호출이 발생하면 상태와 latencyMs/errorMessage가 서로 다른 실행의 값으로 저장될 수 있습니다.
PROCESSING에서만 전이하도록 방어하거나, 중복 호출을 허용한다면 필드 덮어쓰기 규칙을 명시하고 해당 경계 테스트를 추가하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/opensource/docgrid/domain/search/entity/SearchQuery.java`
around lines 105 - 113, Restrict the state transitions in
SearchQuery.updateToSuccess and SearchQuery.updateToFailed so they apply only
when the current status is PROCESSING; otherwise leave the status and associated
latencyMs/errorMessage unchanged. Add boundary tests covering repeated and
conflicting calls, including SUCCESS → FAILED and FAILED → SUCCESS.
| EmbeddingModel model = EmbeddingModelFixture.createDefaultModel(); | ||
| SearchQuery saved = SearchQueryFixture.createProcessing(); | ||
| given(searchQueryRepository.save(any(SearchQuery.class))).willReturn(saved); | ||
|
|
||
| SearchQuery result = searchQueryCommandService.createProcessing( | ||
| null, null, SearchQueryFixture.QUERY_TEXT, | ||
| model, SearchQueryFixture.VECTOR, SearchQueryFixture.TOP_K | ||
| ); | ||
|
|
||
| assertThat(result.getStatus()).isEqualTo(ResultStatus.PROCESSING); | ||
| then(searchQueryRepository).should(times(1)).save(any(SearchQuery.class)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
mock 반환값이 아니라 실제 저장 인자를 검증하세요.
현재 테스트는 SearchQueryFixture.createProcessing()을 mock 반환값으로 사용하므로, 서비스가 PROCESSING이 아닌 객체를 저장해도 테스트가 통과할 수 있습니다. ArgumentCaptor<SearchQuery>로 save()에 전달된 객체를 캡처해 상태, searchType, 모델, 벡터, topK를 검증하세요.
수정 예시
- SearchQuery saved = SearchQueryFixture.createProcessing();
- given(searchQueryRepository.save(any(SearchQuery.class))).willReturn(saved);
+ given(searchQueryRepository.save(any(SearchQuery.class)))
+ .willAnswer(invocation -> invocation.getArgument(0));
...
+ ArgumentCaptor<SearchQuery> captor = ArgumentCaptor.forClass(SearchQuery.class);
+ then(searchQueryRepository).should(times(1)).save(captor.capture());
+ SearchQuery persisted = captor.getValue();
+ assertThat(persisted.getStatus()).isEqualTo(ResultStatus.PROCESSING);
+ assertThat(persisted.getSearchType()).isEqualTo(SearchType.VECTOR);
+ assertThat(persisted.getQueryEmbeddingModel()).isSameAs(model);
+ assertThat(persisted.getTopK()).isEqualTo(SearchQueryFixture.TOP_K);필요한 ArgumentCaptor와 SearchType import도 함께 추가하세요.
📝 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.
| EmbeddingModel model = EmbeddingModelFixture.createDefaultModel(); | |
| SearchQuery saved = SearchQueryFixture.createProcessing(); | |
| given(searchQueryRepository.save(any(SearchQuery.class))).willReturn(saved); | |
| SearchQuery result = searchQueryCommandService.createProcessing( | |
| null, null, SearchQueryFixture.QUERY_TEXT, | |
| model, SearchQueryFixture.VECTOR, SearchQueryFixture.TOP_K | |
| ); | |
| assertThat(result.getStatus()).isEqualTo(ResultStatus.PROCESSING); | |
| then(searchQueryRepository).should(times(1)).save(any(SearchQuery.class)); | |
| EmbeddingModel model = EmbeddingModelFixture.createDefaultModel(); | |
| given(searchQueryRepository.save(any(SearchQuery.class))) | |
| .willAnswer(invocation -> invocation.getArgument(0)); | |
| SearchQuery result = searchQueryCommandService.createProcessing( | |
| null, null, SearchQueryFixture.QUERY_TEXT, | |
| model, SearchQueryFixture.VECTOR, SearchQueryFixture.TOP_K | |
| ); | |
| ArgumentCaptor<SearchQuery> captor = ArgumentCaptor.forClass(SearchQuery.class); | |
| then(searchQueryRepository).should(times(1)).save(captor.capture()); | |
| SearchQuery persisted = captor.getValue(); | |
| assertThat(persisted.getStatus()).isEqualTo(ResultStatus.PROCESSING); | |
| assertThat(persisted.getSearchType()).isEqualTo(SearchType.VECTOR); | |
| assertThat(persisted.getQueryEmbeddingModel()).isSameAs(model); | |
| assertThat(persisted.getTopK()).isEqualTo(SearchQueryFixture.TOP_K); | |
| assertThat(result.getStatus()).isEqualTo(ResultStatus.PROCESSING); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/opensource/docgrid/domain/search/service/command/SearchQueryCommandServiceTest.java`
around lines 36 - 46, Update the createProcessing test around
searchQueryRepository.save and SearchQueryCommandService.createProcessing to
capture the actual save argument with ArgumentCaptor<SearchQuery> instead of
asserting only the stubbed return value. Verify the captured SearchQuery has
PROCESSING status, the expected searchType, model, vector, and topK, and add the
required ArgumentCaptor and SearchType imports.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
🔍️작업 내용
✨ 상세 설명
검색 블록의 첫 번째 구현 이슈. POST /search API 조립 전 필요한 서비스 레이어 부품을 구현했습니다.
질문 임베딩 (QueryEmbeddingService)
사용자 검색어를 Python 사이드카 서버(
POST /embed)에 전달해 1024차원 벡터로 변환합니다.EmbeddingModelQueryService재사용)EmbedResult(model, vector)— Issue 5 조립 시 재사용F-SEARCH-03 — 검색 요청 로깅 (SearchQueryCommandService)
검색 요청을
search_queries테이블에 상태 흐름으로 기록합니다.createProcessing()— PROCESSING 상태로 저장 (query_id 발급)markSuccess(latencyMs)— SUCCESS + latency_ms 갱신markFailed(errorMessage)— FAILED + error_message 갱신기타 변경사항
ResultStatusPROCESSING추가ErrorCodeEMBEDDING_SERVER_UNAVAILABLE,EMBEDDING_DIMENSION_MISMATCH추가application.ymlembedding.server.base-url설정 추가SearchQueryupdateToSuccess(),updateToFailed()메서드 추가🛠 추후 리팩토링 및 고도화 계획
QueryEmbeddingService+SearchQueryCommandService를 조합해POST /searchAPI 조립 예정search_type = VECTOR고정 → Issue 명세상 HYBRID는 2단계 확장 예정latency_ms포트폴리오 성능 증빙 지표로 활용 예정 (통합 테스트 단계에서 측정)📸 스크린샷 (선택)
💬 리뷰 요구사항
QueryEmbeddingService의RestClientException단일 catch 처리가 적절한지 (타임아웃 / 4xx / 5xx를동일하게 503으로 처리)
SearchQueryCommandService.markSuccess/markFailed의 dirty checking 방식이 트랜잭션 경계 내에서 올바르게동작하는지
Summary by CodeRabbit
새 기능
오류 처리
테스트