Conversation
9 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughKEYWORD_ANALYSIS LLM Job을 처리하는 OpenAI client, worker, transaction service, 이벤트 리스너를 추가했다. 최신 조회는 COMPLETED 상태에서만 키워드를 반환하도록 바뀌었고, 관련 문서와 테스트가 함께 갱신되었다. Changes키워드 분석 worker 구현
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Backend CI Report
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (5)
src/test/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorkerTest.java (1)
192-204: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value이미 완료된 Job 재실행 방지 검증에 상태 불변 확인 추가 고려.
keywordAnalysisClient.shouldHaveNoInteractions()만 검증하고 있어 skip 경로는 확인되지만,KeywordAnalysis/LlmJob상태가 그대로COMPLETED로 유지되는지는 별도로 검증하지 않습니다.transactionService.start()가 null을 반환하는 경로 자체는 이 테스트로 충분히 검증되므로 우선순위는 낮습니다.🤖 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/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorkerTest.java` around lines 192 - 204, In executeSkipsAlreadyCompletedJob, the test only verifies keywordAnalysisClient is untouched, but it should also assert that the existing LlmJob/KeywordAnalysis state remains COMPLETED after worker.execute("job_1"). Update this test to reload the job from llmJobRepository after execution and verify its status/result fields are unchanged, so the skip path is covered beyond just no client interaction.src/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobTransactionService.java (2)
148-166: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
fail()에 keywordAnalysis 상태 검증 누락
complete()는keywordAnalysis.getStatus() != PROCESSING일 때INTERNAL_ERROR를 던지는 반면(Line 118-120),fail()은 동일한 검증 없이 무조건keywordAnalysis.fail(now)를 호출합니다. job 상태 가드로 인해 현재 구조에서는 실제 불일치가 발생하기 어렵지만, 향후 코드 변경(예: 동시 실행 경로 추가) 시 이미COMPLETED인 분석을FAILED로 되돌릴 위험이 있어 대칭적으로 방어 검증을 추가하는 것이 안전합니다.♻️ 제안
KeywordAnalysis keywordAnalysis = keywordAnalysisRepository.findByCoverLetterId(job.getTargetId()) .orElseThrow(() -> new BusinessException(ErrorCode.INTERNAL_ERROR)); + if (keywordAnalysis.getStatus() != KeywordAnalysisStatus.PROCESSING) { + throw new BusinessException(ErrorCode.INTERNAL_ERROR); + } Instant now = Instant.now(clock);
178-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM, 사소한 중복 제안
errorCode/errorMessage는 동일한 조건 분기를 반복합니다. 필요 시Reason을 키로 하는 맵이나 record 하나로 묶어 중복을 줄일 수 있으나, 현재 크기에서는 실익이 크지 않습니다.🤖 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/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobTransactionService.java` around lines 178 - 190, Both errorCode and errorMessage repeat the same Reason check, so if you choose to refactor, centralize the mapping in KeywordAnalysisJobTransactionService by using a single helper or a Reason-to-result structure instead of duplicating the OUTPUT_VALIDATION_FAILED branch in two methods. Keep the existing behavior for KeywordAnalysisClientException.Reason.OUTPUT_VALIDATION_FAILED versus the default provider case, but make the lookup shared so future changes only need to be updated in one place.src/main/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.java (2)
43-52: 🩺 Stability & Availability | 🔵 Trivial외부 OpenAI 호출에 대한 타임아웃 설정 확인 필요.
chatClient.prompt()...call()은 OpenAI 응답을 동기적으로 기다리는 블로킹 호출입니다. 이 파일에서는 타임아웃 설정이 보이지 않으므로, worker 스레드가 provider 응답 지연 시 무한정 블로킹되지 않도록application.yml(또는RestClient/WebClient설정)에 명시적인 connect/read timeout이 구성되어 있는지 확인해 주세요.🤖 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/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.java` around lines 43 - 52, The OpenAI call in OpenAiKeywordAnalysisClient is a blocking chatClient.prompt().call() and currently has no visible timeout protection. Verify that the underlying client configuration adds explicit connect/read timeouts in application.yml or the RestClient/WebClient setup used by OpenAiKeywordAnalysisClient, and ensure the provider call cannot block worker threads indefinitely. If no timeout is configured, add one at the client configuration layer rather than in the response handling code.
33-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSpring AI 2.0.0 GA와
validateSchema()적용 검토
spring-ai-starter-model-openai가 아직2.0.0-M8이므로 GA로 올리고,entity(OpenAiKeywordAnalysisResponse.class, spec -> spec.validateSchema())를 쓰면 스키마 불일치 시 모델 재시도를 유도해 지금처럼 곧바로OUTPUT_VALIDATION_FAILED로 떨어지는 빈도를 줄일 수 있습니다.🤖 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/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.java` around lines 33 - 53, The OpenAI response parsing in OpenAiKeywordAnalysisClient.analyze() is still using the plain entity(OpenAiKeywordAnalysisResponse.class) path, so schema mismatches fail immediately and the dependency is on the M8 starter. Update the Spring AI OpenAI starter to the GA 2.0.0 release, then change the ChatClient call in analyze() to use entity(..., spec -> spec.validateSchema()) so schema validation can trigger model retry behavior before falling back to KeywordAnalysisClientException.outputValidationFailed.
🤖 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
`@src/main/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.java`:
- Around line 62-77: The validateAndNormalize method in
OpenAiKeywordAnalysisClient is rejecting responses with more than
MAX_KEYWORD_COUNT keywords instead of trimming them. Remove the size-based
outputValidationFailed check for oversized keyword lists, keep the null/empty
validation, and after normalizing/sorting by KeywordAnalysisResult.importance in
descending order, apply limit(MAX_KEYWORD_COUNT) so only the top 20 are
returned.
In
`@src/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobEventListener.java`:
- Around line 20-24: `KeywordAnalysisJobEventListener.handle()` currently lets
exceptions from `dispatch`/`KeywordAnalysisJobWorker.execute()` escape the
`@Async` boundary, so they are only handled by
`SimpleAsyncUncaughtExceptionHandler` and the `LlmJob` can remain stuck. Update
`handle()` to catch unexpected runtime exceptions around the dispatch path, then
route them through the same failure handling used for job execution: mark the
`LlmJob` as failed (or otherwise transition it out of `PENDING`/`PROCESSING`)
and add a shared logging/alerting path. Use the `handle`, `dispatch`, and
`KeywordAnalysisJobWorker.execute` symbols to keep the fix aligned with the
async event flow.
In
`@src/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobTransactionService.java`:
- Line 59: The `KeywordAnalysisJobWorker.execute()` flow only handles
`KeywordAnalysisClientException`, so `BusinessException` and other unexpected
errors can escape and leave the job in `PENDING`. Update `execute()` to catch
generic exceptions as well, then route them through the existing `fail()` path
or at least log them clearly and mark the job as failed. Use the existing
`fail()` logic and `KeywordAnalysisJobWorker`/`start()` entry points to keep the
status transition consistent.
In
`@src/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorker.java`:
- Around line 18-30: `KeywordAnalysisJobWorker.execute` only handles
`KeywordAnalysisClientException`, so failures from
`transactionService.start(jobId)` or other runtime exceptions can escape and
leave the job stuck. Move the start/analysis/complete flow into a single `try`
block in `execute`, and add a catch path that routes any `BusinessException` or
unexpected exception through `transactionService.fail(jobId, ...)` as well,
using the existing `KeywordAnalysisJobWorker`, `transactionService`, and
`keywordAnalysisClient` flow to keep all errors on the same failure path.
---
Nitpick comments:
In
`@src/main/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.java`:
- Around line 43-52: The OpenAI call in OpenAiKeywordAnalysisClient is a
blocking chatClient.prompt().call() and currently has no visible timeout
protection. Verify that the underlying client configuration adds explicit
connect/read timeouts in application.yml or the RestClient/WebClient setup used
by OpenAiKeywordAnalysisClient, and ensure the provider call cannot block worker
threads indefinitely. If no timeout is configured, add one at the client
configuration layer rather than in the response handling code.
- Around line 33-53: The OpenAI response parsing in
OpenAiKeywordAnalysisClient.analyze() is still using the plain
entity(OpenAiKeywordAnalysisResponse.class) path, so schema mismatches fail
immediately and the dependency is on the M8 starter. Update the Spring AI OpenAI
starter to the GA 2.0.0 release, then change the ChatClient call in analyze() to
use entity(..., spec -> spec.validateSchema()) so schema validation can trigger
model retry behavior before falling back to
KeywordAnalysisClientException.outputValidationFailed.
In
`@src/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobTransactionService.java`:
- Around line 178-190: Both errorCode and errorMessage repeat the same Reason
check, so if you choose to refactor, centralize the mapping in
KeywordAnalysisJobTransactionService by using a single helper or a
Reason-to-result structure instead of duplicating the OUTPUT_VALIDATION_FAILED
branch in two methods. Keep the existing behavior for
KeywordAnalysisClientException.Reason.OUTPUT_VALIDATION_FAILED versus the
default provider case, but make the lookup shared so future changes only need to
be updated in one place.
In
`@src/test/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorkerTest.java`:
- Around line 192-204: In executeSkipsAlreadyCompletedJob, the test only
verifies keywordAnalysisClient is untouched, but it should also assert that the
existing LlmJob/KeywordAnalysis state remains COMPLETED after
worker.execute("job_1"). Update this test to reload the job from
llmJobRepository after execution and verify its status/result fields are
unchanged, so the skip path is covered beyond just no client interaction.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 617623ae-e0ab-4743-a556-0eccf02fe1da
📒 Files selected for processing (19)
docs/api/keyword-analysis.mddocs/status.mdsrc/main/java/com/daon/rewrite/keywordanalysis/client/KeywordAnalysisAnswer.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/KeywordAnalysisClient.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/KeywordAnalysisClientException.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/KeywordAnalysisRequest.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/KeywordAnalysisResult.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClient.javasrc/main/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisResponse.javasrc/main/java/com/daon/rewrite/keywordanalysis/repository/KeywordAnalysisKeywordRepository.javasrc/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobEventListener.javasrc/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobTransactionService.javasrc/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorker.javasrc/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisService.javasrc/main/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisWork.javasrc/test/java/com/daon/rewrite/keywordanalysis/client/OpenAiKeywordAnalysisClientTest.javasrc/test/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobEventIntegrationTest.javasrc/test/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisJobWorkerTest.javasrc/test/java/com/daon/rewrite/keywordanalysis/service/KeywordAnalysisServiceTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 내용
KEYWORD_ANALYSISLLM Job을 after-commit 비동기 worker로 실행하도록 연결했습니다.finalAnswer)을 입력으로 키워드를 추출하도록 구현했습니다.KeywordAnalysisKeyword에 결과를 저장하고, Job은COMPLETED및KEYWORD_ANALYSISresultRef로 마킹합니다.LlmJoberror 정보로 기록하고,KeywordAnalysis는FAILED상태로 전환합니다.COMPLETED상태일 때만 keywords를 반환하도록 보강했습니다.관련 이슈
문서 반영
docs/api/keyword-analysis.mddocs/status.md확인 결과
./gradlew test./gradlew checkgit diff --checkSummary by CodeRabbit