[Refactor] evaluation analysis 경계 분리 및 의존 고정 (#270) - #277
Merged
Conversation
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthrough평가 분석 결과가 외부 DTO에서 평가 전용 스냅샷과 JSON 문자열로 전환되었습니다. 후보 검토 결정과 sanitization 결과 모델이 추가되었습니다. 분석 집계, 재생, NLG 흐름이 새 모델과 정책 상수를 사용합니다. Changes평가 분석 스냅샷 전환
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AI as AnalysisAiEvaluationAnalysisGenerator
participant Parser as EvaluationCandidateSnapshotParser
participant Batch as EvaluationAnalysisBatchService
participant Sanitizer as EvaluationSanitizationService
participant Replay as MissingKeywordSanitizerReplayService
AI->>Parser: 후보 JSON 파싱
Parser-->>AI: EvaluationCandidateSnapshot
AI->>Batch: EvaluationGeneratedResult 전달
Batch->>Sanitizer: 누락 키워드 후보 sanitization
Sanitizer-->>Batch: 승인 후보와 sanitization 결정
Replay->>Parser: 저장된 후보 JSON 재파싱
Parser-->>Replay: EvaluationCandidateSnapshot
Replay->>Sanitizer: 후보 재검증
Sanitizer-->>Replay: 재생 결과와 결정
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/MissingKeywordSanitizerReplayService.java (1)
121-138: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win잘못된 후보를 조용히 제외하지 않도록 재생 검증을 보강하세요.
EvaluationCandidateSnapshotParser는rawCandidateResponseJson와sanitizedCandidateResponseJson모두에서 빈keyword와 알 수 없는source를 제거합니다. 따라서 잘못된 후보가 있어도validateExistingSanitized가 동일한 목록으로 판정할 수 있습니다.
EvaluationMissingKeywordCandidate의Objects.equals비교에는relatedRequirement도 포함됩니다. 기존 회귀 데이터가 이 필드를 저장하지 않으면 정상 후보도 불일치할 수 있습니다.잘못된 후보를 파싱 오류로 처리하거나 후보 손실을 별도로 검증하세요.
MissingKeywordSanitizerReplayServiceTest에 빈keyword, 알 수 없는source,relatedRequirement불일치 사례를 추가하세요.🤖 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/jobdri/jobdri_api/domain/evaluation/analysis/MissingKeywordSanitizerReplayService.java` around lines 121 - 138, Strengthen replay validation in MissingKeywordSanitizerReplayService around readCandidateResponse and validateExistingSanitized so candidates removed by EvaluationCandidateSnapshotParser for blank keyword or unknown source are reported as parse/validation errors rather than silently omitted; preserve relatedRequirement in candidate comparisons, while handling legacy missing values according to the existing replay contract to avoid false mismatches. Add MissingKeywordSanitizerReplayServiceTest coverage for blank keyword, unknown source, and relatedRequirement mismatches.Source: Path instructions
🤖 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/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateReviewSnapshotParser.java`:
- Around line 20-30: Update EvaluationCandidateReviewSnapshotParser.parse to use
the same fail-fast malformed-JSON policy as
EvaluationCandidateSnapshotParser.parse: do not return emptySnapshot() from the
JsonProcessingException path, and instead throw IllegalArgumentException
containing the available fieldName and caseId identifiers. Preserve the existing
empty-input handling and successful decisions parsing.
In
`@src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotParser.java`:
- Around line 41-48: Update readOpaqueItems so it no longer returns List<Object>
containing JsonNode values. If callers only use the collection size, change the
parsing flow to store and return an int count; otherwise change the return type
and backing list to List<JsonNode>, updating callers and domain fields
accordingly to avoid opaque Jackson values.
In
`@src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationCandidateSnapshot.java`:
- Around line 6-7: Replace the List<Object> fields in
EvaluationCandidateSnapshot with a dedicated evaluation snapshot candidate type
that explicitly defines the allowed value shape. Update
EvaluationCandidateSnapshotParser.readOpaqueItems and its consumers to produce
and use this type instead of exposing raw JSON objects or requiring runtime
casts. If preserving arbitrary JSON is required, encapsulate it in a dedicated
value type rather than retaining Object.
In
`@src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/sanitization/EvaluationSanitizationService.java`:
- Around line 170-174: Normalize immutable term collections once during static
initialization instead of on each evaluation: in
EvaluationSanitizationService.java:170-174, replace
STRUCTURED_QUALIFICATION_TERMS with a pre-normalized Set and remove
normalize(term); at 304-308, pre-normalize FABRICATED_DIRECT_CONFLICT_TERMS and
promote the literal normalize("팀 프로젝트") values at 309-313 to constants; at
322-327, precompute whitespace-stripped BANNED_IMPROVEMENT_PHRASES and
IMPERATIVE_ENDINGS and remove the stream replaceAll mapping; at 420-424, replace
META_IMPROVEMENT_TERMS with a pre-normalized collection. Preserve all existing
matching behavior while reusing the static values.
- Around line 372-391: Update EvaluationSanitizationService around
stripKoreanSuffix by hoisting the Korean-character regex into a reusable static
Pattern and the suffix list into a reusable static constant. Replace
token.matches and the per-call suffixes allocation with those constants,
preserving all existing matching and suffix-stripping behavior.
- Around line 165-176: isStructuredQualificationKeyword에서 value가 null일 때
CAREER_YEAR_PATTERN.matcher 호출 전에 false를 반환하도록 null 처리를 일관되게 추가하세요. 이후의
normalize 및 구조화 자격 용어 검사 흐름은 비null 입력에서 그대로 유지하세요.
In
`@src/main/java/com/jobdri/jobdri_api/domain/evaluation/infrastructure/analysis/AnalysisAiEvaluationAnalysisGenerator.java`:
- Around line 69-71: Promote the three stateless parsers in
AnalysisAiEvaluationAnalysisGenerator from local variables inside generate to
final fields initialized once in an explicit constructor with the shared
ObjectMapper. Reuse those fields on every generate call, matching the
construction pattern used by MissingKeywordSanitizerReplayService and
NlgEvaluationBatchService, and do not use `@RequiredArgsConstructor`.
- Around line 98-104: Update writeJson in AnalysisAiEvaluationAnalysisGenerator
so null object responses are serialized as a JSON null literal or object-shaped
{} instead of the array []. Preserve normal serialization for non-null values
and keep the existing exception handling, ensuring stored response JSON matches
the object response shape.
In
`@src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/EvaluationAnalysisPackageDependencyTest.java`:
- Around line 19-22: Update EvaluationAnalysisPackageDependencyTest’s
FORBIDDEN_IMPORT_PREFIXES and collectViolations logic to detect both regular and
static imports, including lines with leading whitespace. Strip leading
whitespace before applying startsWith checks, and register matching prefixes for
both import forms.
In
`@src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateReviewSnapshotParserTest.java`:
- Around line 15-33: Expand parsesReviewJson tests to cover parser boundaries:
null and empty input, malformed JSON, missing decisions, and non-array decisions
must produce an empty snapshot/list under the current contract; non-boolean
accepted must produce a null accepted() value. Use the existing parser and
assertion style, preserving the current valid JSON behavior.
In
`@src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotParserTest.java`:
- Around line 31-35: 보강된 테스트에서 `strengthCandidates`와 `analysisCandidates`의 크기만
확인하지 말고 각 후보의 `quote`와 `candidateId` 값이 원본 JSON과 동일하게 보존되는지 직접 검증하세요. 또한
`EvaluationCandidateSnapshotParser.readOpaqueItems`에 배열 필드가 없거나 배열이 아닌 입력을 전달했을
때 해당 후보 목록이 빈 목록이 되는 경계 계약을 테스트로 추가하세요.
In
`@src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotParserTest.java`:
- Around line 30-31: EvaluationLlmSnapshotParserTest의 누락 점수 검증을 보강하세요. jobFit뿐
아니라 impact와 completeness도 모두 isNull()인지 확인하고, 숫자 점수 입력에 대해 세 필드가 올바르게 파싱되는 별도
테스트를 추가하세요.
---
Outside diff comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/MissingKeywordSanitizerReplayService.java`:
- Around line 121-138: Strengthen replay validation in
MissingKeywordSanitizerReplayService around readCandidateResponse and
validateExistingSanitized so candidates removed by
EvaluationCandidateSnapshotParser for blank keyword or unknown source are
reported as parse/validation errors rather than silently omitted; preserve
relatedRequirement in candidate comparisons, while handling legacy missing
values according to the existing replay contract to avoid false mismatches. Add
MissingKeywordSanitizerReplayServiceTest coverage for blank keyword, unknown
source, and relatedRequirement mismatches.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 648a5926-9541-4170-b170-3a322e8d3084
📒 Files selected for processing (29)
src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/EvaluationAnalysisBatchService.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/MissingKeywordSanitizerReplayService.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/NlgEvaluationBatchService.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateReviewSnapshotParser.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotMapper.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotParser.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotMapper.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotParser.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationMissingKeywordMapper.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationMissingKeywordSourceMapper.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationQuestionAnalysisMapper.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationCandidateReviewDecision.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationCandidateReviewSnapshot.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationCandidateSnapshot.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationGeneratedResult.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationLlmSnapshot.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationMissingKeywordRejectionReason.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationMissingKeywordSanitizationDecision.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationMissingKeywordSanitizationResult.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/policy/EvaluationAnalysisPolicyConstants.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/sanitization/EvaluationSanitizationService.javasrc/main/java/com/jobdri/jobdri_api/domain/evaluation/infrastructure/analysis/AnalysisAiEvaluationAnalysisGenerator.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/EvaluationAnalysisBatchServiceTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/EvaluationAnalysisPackageDependencyTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateReviewSnapshotParserTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotMapperTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotParserTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotMapperTest.javasrc/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotParserTest.java
💤 Files with no reviewable changes (7)
- src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationMissingKeywordMapper.java
- src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotMapperTest.java
- src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotMapper.java
- src/test/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateSnapshotMapperTest.java
- src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationMissingKeywordSourceMapper.java
- src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationLlmSnapshotMapper.java
- src/main/java/com/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationQuestionAnalysisMapper.java
Comment on lines
+20
to
+30
| public EvaluationCandidateReviewSnapshot parse(String json) { | ||
| if (!StringUtils.hasText(json)) { | ||
| return emptySnapshot(); | ||
| } | ||
| try { | ||
| JsonNode root = objectMapper.readTree(json); | ||
| return new EvaluationCandidateReviewSnapshot(readDecisions(root.path("decisions"))); | ||
| } catch (JsonProcessingException e) { | ||
| return emptySnapshot(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
malformed JSON을 조용히 삼키는 동작을 재검토하세요.
parse는 JSON 파싱 실패 시 빈 스냅샷을 반환합니다. 형제 파서인 EvaluationCandidateSnapshotParser.parse는 같은 상황에서 fieldName과 caseId를 포함해 IllegalArgumentException을 던집니다. 예외 처리 정책이 파서마다 다릅니다.
이 동작에서는 리뷰 응답이 손상되어도 "결정 0건"과 구분되지 않습니다. 배치 집계나 재생 결과가 조용히 왜곡될 수 있고, 추적 가능한 식별자도 남지 않습니다.
두 가지 중 하나를 선택하세요. 리뷰 스냅샷을 선택적 데이터로 본다면 최소한 경고 로그를 남기고 caseId를 파라미터로 받으세요. 필수 데이터로 본다면 후보 파서와 동일하게 fail-fast 하세요.
♻️ 로그와 식별자를 추가하는 예시
-public class EvaluationCandidateReviewSnapshotParser {
+@Slf4j
+public class EvaluationCandidateReviewSnapshotParser {
private final ObjectMapper objectMapper;
public EvaluationCandidateReviewSnapshotParser(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}
- public EvaluationCandidateReviewSnapshot parse(String json) {
+ public EvaluationCandidateReviewSnapshot parse(String json, String caseId) {
if (!StringUtils.hasText(json)) {
return emptySnapshot();
}
try {
JsonNode root = objectMapper.readTree(json);
return new EvaluationCandidateReviewSnapshot(readDecisions(root.path("decisions")));
} catch (JsonProcessingException e) {
+ log.warn("candidateReviewResponseJson is not valid JSON. caseId={}", caseId, e);
return emptySnapshot();
}
}As per path instructions: "코드 완성도: ... 예외 처리 일관성".
🤖 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/jobdri/jobdri_api/domain/evaluation/analysis/mapper/EvaluationCandidateReviewSnapshotParser.java`
around lines 20 - 30, Update EvaluationCandidateReviewSnapshotParser.parse to
use the same fail-fast malformed-JSON policy as
EvaluationCandidateSnapshotParser.parse: do not return emptySnapshot() from the
JsonProcessingException path, and instead throw IllegalArgumentException
containing the available fieldName and caseId identifiers. Preserve the existing
empty-input handling and successful decisions parsing.
Source: Path instructions
Comment on lines
+6
to
+7
| List<Object> strengthCandidates, | ||
| List<Object> analysisCandidates, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift
후보 스냅샷의 타입 계약을 명시하세요.
List<Object>는 EvaluationCandidateSnapshot이 허용하는 값의 형태를 정의하지 않습니다. EvaluationCandidateSnapshotParser.java의 readOpaqueItems(Line 14-71)는 JSON 원소 객체를 그대로 저장하므로, 소비자가 런타임 캐스팅과 원시 JSON 표현에 의존하게 됩니다. 후보 항목용 평가 스냅샷 타입을 도입하세요. 원시 JSON 보존이 필요하면 전용 값 타입으로 그 의도를 명시하세요.
🤖 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/jobdri/jobdri_api/domain/evaluation/analysis/model/EvaluationCandidateSnapshot.java`
around lines 6 - 7, Replace the List<Object> fields in
EvaluationCandidateSnapshot with a dedicated evaluation snapshot candidate type
that explicitly defines the allowed value shape. Update
EvaluationCandidateSnapshotParser.readOpaqueItems and its consumers to produce
and use this type instead of exposing raw JSON objects or requiring runtime
casts. If preserving arbitrary JSON is required, encapsulate it in a dedicated
value type rather than retaining Object.
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.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
EvaluationAnalysisBatchService가 runtime 상수에 직접 의존하던 값을EvaluationAnalysisPolicyConstants로 분리해 evaluation 경계를 명확히 했습니다.MissingKeywordSanitizerReplayService가 runtime DTO와 sanitizer 결과 타입을 직접 참조하던 구조를 evaluation snapshot/parser와 로컬 sanitizer 결과 모델로 전환했습니다.domain/evaluation/analysis내부에서 runtime analysisdto/serviceimport가 다시 생기지 않도록 obsolete mapper를 제거하고 패키지 규칙 테스트를 추가했습니다.📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [ #270 ]
검증
./gradlew test --tests 'com.jobdri.jobdri_api.domain.evaluation.analysis.*' --tests 'com.jobdri.jobdri_api.domain.evaluation.analysis.mapper.*'Summary by CodeRabbit
개선 사항
버그 수정