Repository navigation
Feature/#14 report api - #15
Merged
Merged
Conversation
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough이 PR은 면접 답변 저장, AI 기반 평가·레이더 점수 계산, 그리고 사용자 코드·Q&A를 근거로 마크다운 트러블슈팅 리포트를 생성하는 엔드‑투‑엔드 API를 구현합니다. Changes면접 및 리포트 생성 통합 기능
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (5)
src/main/java/com/example/trouble_log/domain/analysis/entity/AnalysisResult.java (1)
40-54: 💤 Low value기존 필드와 타입 불일치:
intvsInteger새로 추가된 클린코드 평가 필드들은 primitive
int를 사용하지만, 기존 레이더 차트 스코어 필드들(Lines 26-38)은Integerwrapper 타입을 사용합니다. 동일 엔티티 내에서 타입이 혼재되면 getter 시그니처가 일관되지 않고, 유지보수 시 혼란을 야기할 수 있습니다.♻️ 타입 일관성을 위한 제안
// --- 클린코드 평가 스코어 영역 --- `@Column`(nullable = false) - private int scoreNaming; + private Integer scoreNaming; `@Column`(nullable = false) - private int scoreSingleResponsibility; + private Integer scoreSingleResponsibility; `@Column`(nullable = false) - private int scoreErrorHandling; + private Integer scoreErrorHandling; `@Column`(nullable = false) - private int scoreDuplication; + private Integer scoreDuplication; `@Column`(nullable = false) - private int scoreCommentQuality; + private Integer scoreCommentQuality;🤖 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/example/trouble_log/domain/analysis/entity/AnalysisResult.java` around lines 40 - 54, The new clean-code score fields in AnalysisResult (scoreNaming, scoreSingleResponsibility, scoreErrorHandling, scoreDuplication, scoreCommentQuality) use primitive int while the existing radar score fields use Integer; change these five fields to use the Integer wrapper to maintain type consistency in the entity and its getters/setters, update any constructor parameters and methods (e.g., getScoreNaming, setScoreNaming, etc.) that reference the primitive types to use Integer as well so the class API remains uniform.src/test/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java (1)
15-69: ⚡ Quick win엣지 케이스에 대한 테스트를 추가하세요.
현재 테스트는 정상적인 입력값에 대한 계산만 검증합니다. 레이더 점수 계산기의 신뢰성을 높이기 위해 다음과 같은 엣지 케이스를 추가로 검증하는 것이 좋습니다:
- 모든 점수가 최솟값(1 또는 0)인 경우
- 모든 점수가 최댓값(5 또는 20)인 경우
- 경계값 테스트 (예: 평균이 정확히 정수가 되지 않는 경우의 반올림 동작)
엣지 케이스 테스트 예시
`@Test` `@DisplayName`("모든 점수가 최솟값일 때 레이더 점수 0 반환") void calculate_withMinimumScores_returnsZero() { // given AnswerFeedbackResult.Scores scores = new AnswerFeedbackResult.Scores(); scores.setStructure(1); scores.setRelevance(1); scores.setKeyword(1); scores.setSpecificity(1); AnswerFeedbackResult feedback = new AnswerFeedbackResult(); feedback.setScores(scores); CodeEvaluationResult codeEval = new CodeEvaluationResult(); codeEval.setErrorHandling(createAxis(0)); codeEval.setSingleResponsibility(createAxis(0)); codeEval.setNaming(createAxis(0)); codeEval.setDuplication(createAxis(0)); codeEval.setCommentQuality(createAxis(0)); // when RadarScore result = calculator.calculate(feedback, codeEval); // then assertThat(result.getProblemSolving()).isEqualTo(0); assertThat(result.getTechJudgment()).isEqualTo(0); assertThat(result.getCodeReliability()).isEqualTo(0); assertThat(result.getCommunication()).isEqualTo(0); assertThat(result.getDesignThinking()).isEqualTo(0); } `@Test` `@DisplayName`("모든 점수가 최댓값일 때 레이더 점수 100 반환") void calculate_withMaximumScores_returns100() { // given AnswerFeedbackResult.Scores scores = new AnswerFeedbackResult.Scores(); scores.setStructure(5); scores.setRelevance(5); scores.setKeyword(5); scores.setSpecificity(5); AnswerFeedbackResult feedback = new AnswerFeedbackResult(); feedback.setScores(scores); CodeEvaluationResult codeEval = new CodeEvaluationResult(); codeEval.setErrorHandling(createAxis(20)); codeEval.setSingleResponsibility(createAxis(20)); codeEval.setNaming(createAxis(20)); codeEval.setDuplication(createAxis(20)); codeEval.setCommentQuality(createAxis(20)); // when RadarScore result = calculator.calculate(feedback, codeEval); // then assertThat(result.getProblemSolving()).isEqualTo(100); assertThat(result.getTechJudgment()).isEqualTo(100); assertThat(result.getCodeReliability()).isEqualTo(100); assertThat(result.getCommunication()).isEqualTo(100); assertThat(result.getDesignThinking()).isEqualTo(100); } private CodeEvaluationResult.Axis createAxis(int score) { CodeEvaluationResult.Axis axis = new CodeEvaluationResult.Axis(); axis.setScore(score); return axis; }🤖 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/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java` around lines 15 - 69, The test only validates a normal case; add edge-case unit tests to RadarScoreCalculatorTest to cover minimum, maximum, and boundary rounding behaviors by creating new `@Test` methods (e.g., calculate_withMinimumScores_returnsZero, calculate_withMaximumScores_returns100, and a boundary rounding test) that construct AnswerFeedbackResult with Scores and CodeEvaluationResult.Axis instances (use a helper like createAxis) and call calculator.calculate(feedback, codeEval) to assert expected 0/100/rounded values for getProblemSolving, getTechJudgment, getCodeReliability, getCommunication, and getDesignThinking; ensure you reference the existing calculator.calculate(...) call and the types AnswerFeedbackResult.Scores and CodeEvaluationResult.Axis when adding these tests.src/test/java/com/example/trouble_log/domain/ai/PromptIntegrationTest.java (1)
200-235: ⚡ Quick win스킵된 답변 시나리오에 대한 테스트 케이스를 추가하세요.
InterviewService.generateReport메서드는 스킵된 답변(isSkipped = true또는answer = null)을 "(스킵)"으로 처리하는 로직을 포함하고 있습니다. 하지만 현재step5_generateReport테스트는 모든 답변이 정상적으로 제공된 경우만 검증합니다.스킵된 답변이 포함된 시나리오를 추가하여 리포트 생성 로직의 엣지 케이스를 검증하는 것이 좋습니다.
테스트 케이스 추가 예시
`@Test` `@Order`(7) `@DisplayName`("STEP5-B | 스킵된 답변 포함 리포트 — (스킵) 문구 포함 확인") void step5_generateReportWithSkippedAnswers() { String qaPairsWithSkip = """ Q1. 질문 1 A1. 정상 답변 Q2. 질문 2 A2. (스킵) Q3. 질문 3 A3. 또 다른 정상 답변 """; String report = promptService.generateReport( CODE, PURPOSE, TECH, EXCEPTION, SCALE, qaPairsWithSkip ); assertThat(report).isNotBlank(); assertThat(report).contains("(스킵)"); }Based on learnings, 서비스 레이어가 스킵 처리 로직을 포함할 때 해당 경로에 대한 테스트 커버리지를 확보하는 것이 중요합니다.
🤖 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/example/trouble_log/domain/ai/PromptIntegrationTest.java` around lines 200 - 235, The test suite lacks coverage for skipped interview answers: add a new test method (e.g., step5_generateReportWithSkippedAnswers) that calls promptService.generateReport (or InterviewService.generateReport) with a QA string containing skipped answers (isSkipped=true or answer=null represented as "(스킵)"), then assert the returned report is not blank and contains the literal "(스킵)"; annotate it with `@Test`, `@Order`(7) and `@DisplayName` to mirror existing STEP5 tests and reuse CODE, PURPOSE, TECH, EXCEPTION, SCALE where appropriate.src/main/java/com/example/trouble_log/domain/interview/controller/InterviewController.java (1)
27-27: ⚡ Quick win요청 본문 검증을 위해
@Valid애너테이션 추가를 고려하세요.
@RequestBody파라미터에@Valid애너테이션이 없어,AnswerRequest에 정의된 검증 제약 조건(예:@NotNull,@NotBlank등)이 있어도 적용되지 않습니다. 입력 검증을 강화하려면@Valid를 추가하는 것이 좋습니다.✅ `@Valid` 추가 제안
+import jakarta.validation.Valid; + `@RestController` `@RequiredArgsConstructor` `@RequestMapping`("/api/projects")public ResponseEntity<AnswerResponse> saveAnswer( `@PathVariable` Long sessionId, `@PathVariable` Long questionId, - `@RequestBody` AnswerRequest request + `@RequestBody` `@Valid` AnswerRequest request ) {🤖 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/example/trouble_log/domain/interview/controller/InterviewController.java` at line 27, In InterviewController where the endpoint accepts the request parameter declared as "`@RequestBody` AnswerRequest request", add the "`@Valid`" annotation to that parameter (i.e., "`@Valid` `@RequestBody` AnswerRequest request") so Bean Validation constraints on AnswerRequest are enforced; also add the appropriate import for javax.validation.Valid or jakarta.validation.Valid consistent with the project and, if your project uses class-level validation for controllers, ensure the controller class has "`@Validated`" as needed.src/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.java (1)
36-43: 💤 Low valuenon-null 컬럼에는 boolean 원시 타입 사용을 고려하세요.
isSkipped필드가@Column(nullable = false)로 선언되어 있으므로,Boolean래퍼 타입 대신boolean원시 타입을 사용하는 것이 타입 안전성과 설계 일관성 측면에서 더 적합합니다. 현재는 생성자 파라미터로 null이 전달될 가능성이 있지만, 서비스 계층에서 항상 true/false를 전달하므로 원시 타입으로 변경해도 문제없습니다.♻️ 제안하는 리팩터링
- `@Column`(nullable = false) - private Boolean isSkipped = false; + `@Column`(nullable = false) + private boolean isSkipped = false; // 답변 저장용 생성자 public InterviewAnswer(InterviewQuestion interviewQuestion, String answer, - Boolean isSkipped, + boolean isSkipped, String feedback) {🤖 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/example/trouble_log/domain/interview/entity/InterviewAnswer.java` around lines 36 - 43, Change the nullable=false Boolean usage to a primitive boolean for type safety: update the InterviewAnswer class's isSkipped field to use the primitive type, update the InterviewAnswer(InterviewQuestion interviewQuestion, String answer, Boolean isSkipped, String feedback) constructor parameter to boolean isSkipped and assign it to the primitive field, and adjust any getter/setter or usages (e.g., getIsSkipped/setIsSkipped or isSkipped accessor) to the primitive signature to keep consistency with the `@Column`(nullable = false) contract.
🤖 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/example/trouble_log/domain/ai/service/AzureOpenAiPromptService.java`:
- Around line 271-287: The generateReport method currently forwards six raw
params to the prompt template; add input validation (either reuse
validateInterviewQuestionSource or introduce a new private validateReportSource)
that checks codeContent, codePurpose, techRationale, exceptionHandling,
projectScale, and qaPairs for null/blank (trimmed) and throws an informative
IllegalArgumentException (or returns an error) before calling
loadPromptTemplate/generate; ensure validation happens at the top of
generateReport so loadPromptTemplate is not invoked with invalid inputs and
reference the generateReport and
validateInterviewQuestionSource/validateReportSource symbols when locating the
change.
In
`@src/main/java/com/example/trouble_log/domain/interview/dto/AnswerRequest.java`:
- Around line 7-10: AnswerRequest DTO lacks setters so Jackson can't populate
memberId and answer during JSON deserialization; add setters (e.g., annotate the
class AnswerRequest with `@Setter` or replace `@Getter/`@NoArgsConstructor with
Lombok's `@Data`) or manually implement public setMemberId(...) and setAnswer(...)
methods so `@RequestBody` payloads are deserialized correctly.
In
`@src/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.java`:
- Line 44: The InterviewAnswer constructor currently sets createdAt =
LocalDateTime.now() which couples the entity to the system clock; remove that
assignment from the constructor and manage creation time with JPA/Hibernate
instead by either adding a `@PrePersist-annotated` method that sets createdAt if
null, or (recommended) annotate the createdAt field with Hibernate's
`@CreationTimestamp` and ensure the field is of type LocalDateTime; update imports
accordingly and keep the class name InterviewAnswer and the createdAt field as
the target for this change.
In
`@src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java`:
- Around line 70-74: The catch block swallowing exceptions when calling
objectMapper.writeValueAsString(feedbackResult) must log the failure so issues
can be diagnosed; update the try/catch around objectMapper.writeValueAsString in
InterviewService (the block that assigns feedbackJson) to catch Exception e and
call the class logger (e.g., logger.error or log.warn depending on logger used)
with a clear message including the feedbackResult or its id and the exception
(e), then keep the fallback feedbackJson = "{}" as before.
In `@src/main/resources/prompts/report-user.txt`:
- Around line 46-50: The Resolution section currently forbids code blocks ("코드
블록을 절대 포함하지 마세요") but the output example still contains a placeholder suggesting
code content; update prompts/report-user.txt to remove the conflicting example
placeholder (e.g. the "(코드 블록이 있으면 여기에)" sample) under the "Resolution" header
and ensure the example output matches the rule by containing only descriptive
text; confirm the "Resolution" heading and any "{codeContent}" placeholders are
either removed or changed to explicitly allow only quoted excerpts (if that was
intended) so the instructions and example are consistent.
---
Nitpick comments:
In
`@src/main/java/com/example/trouble_log/domain/analysis/entity/AnalysisResult.java`:
- Around line 40-54: The new clean-code score fields in AnalysisResult
(scoreNaming, scoreSingleResponsibility, scoreErrorHandling, scoreDuplication,
scoreCommentQuality) use primitive int while the existing radar score fields use
Integer; change these five fields to use the Integer wrapper to maintain type
consistency in the entity and its getters/setters, update any constructor
parameters and methods (e.g., getScoreNaming, setScoreNaming, etc.) that
reference the primitive types to use Integer as well so the class API remains
uniform.
In
`@src/main/java/com/example/trouble_log/domain/interview/controller/InterviewController.java`:
- Line 27: In InterviewController where the endpoint accepts the request
parameter declared as "`@RequestBody` AnswerRequest request", add the "`@Valid`"
annotation to that parameter (i.e., "`@Valid` `@RequestBody` AnswerRequest request")
so Bean Validation constraints on AnswerRequest are enforced; also add the
appropriate import for javax.validation.Valid or jakarta.validation.Valid
consistent with the project and, if your project uses class-level validation for
controllers, ensure the controller class has "`@Validated`" as needed.
In
`@src/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.java`:
- Around line 36-43: Change the nullable=false Boolean usage to a primitive
boolean for type safety: update the InterviewAnswer class's isSkipped field to
use the primitive type, update the InterviewAnswer(InterviewQuestion
interviewQuestion, String answer, Boolean isSkipped, String feedback)
constructor parameter to boolean isSkipped and assign it to the primitive field,
and adjust any getter/setter or usages (e.g., getIsSkipped/setIsSkipped or
isSkipped accessor) to the primitive signature to keep consistency with the
`@Column`(nullable = false) contract.
In `@src/test/java/com/example/trouble_log/domain/ai/PromptIntegrationTest.java`:
- Around line 200-235: The test suite lacks coverage for skipped interview
answers: add a new test method (e.g., step5_generateReportWithSkippedAnswers)
that calls promptService.generateReport (or InterviewService.generateReport)
with a QA string containing skipped answers (isSkipped=true or answer=null
represented as "(스킵)"), then assert the returned report is not blank and
contains the literal "(스킵)"; annotate it with `@Test`, `@Order`(7) and `@DisplayName`
to mirror existing STEP5 tests and reuse CODE, PURPOSE, TECH, EXCEPTION, SCALE
where appropriate.
In
`@src/test/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java`:
- Around line 15-69: The test only validates a normal case; add edge-case unit
tests to RadarScoreCalculatorTest to cover minimum, maximum, and boundary
rounding behaviors by creating new `@Test` methods (e.g.,
calculate_withMinimumScores_returnsZero, calculate_withMaximumScores_returns100,
and a boundary rounding test) that construct AnswerFeedbackResult with Scores
and CodeEvaluationResult.Axis instances (use a helper like createAxis) and call
calculator.calculate(feedback, codeEval) to assert expected 0/100/rounded values
for getProblemSolving, getTechJudgment, getCodeReliability, getCommunication,
and getDesignThinking; ensure you reference the existing
calculator.calculate(...) call and the types AnswerFeedbackResult.Scores and
CodeEvaluationResult.Axis when adding these tests.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 49f09eb3-9a1c-4f42-bbeb-f7a19d9098c1
📒 Files selected for processing (16)
src/main/java/com/example/trouble_log/domain/ai/service/AzureOpenAiPromptService.javasrc/main/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculator.javasrc/main/java/com/example/trouble_log/domain/analysis/entity/AnalysisResult.javasrc/main/java/com/example/trouble_log/domain/interview/controller/InterviewController.javasrc/main/java/com/example/trouble_log/domain/interview/dto/AnswerRequest.javasrc/main/java/com/example/trouble_log/domain/interview/dto/AnswerResponse.javasrc/main/java/com/example/trouble_log/domain/interview/dto/ReportResponse.javasrc/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewAnswerRepository.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewQuestionRepository.javasrc/main/java/com/example/trouble_log/domain/interview/service/InterviewService.javasrc/main/java/com/example/trouble_log/domain/projectSession/repository/PreContextRepository.javasrc/main/resources/prompts/report-system.txtsrc/main/resources/prompts/report-user.txtsrc/test/java/com/example/trouble_log/domain/ai/PromptIntegrationTest.javasrc/test/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java
Comment on lines
+271
to
+287
| public String generateReport( | ||
| String codeContent, String codePurpose, String techRationale, | ||
| String exceptionHandling, String projectScale, String qaPairs) { | ||
|
|
||
| String userPrompt = loadPromptTemplate(REPORT_USER_PROMPT_PATH) | ||
| .replace("{codeContent}", codeContent) | ||
| .replace("{codePurpose}", codePurpose) | ||
| .replace("{techRationale}", techRationale) | ||
| .replace("{exceptionHandling}", exceptionHandling) | ||
| .replace("{projectScale}", projectScale) | ||
| .replace("{qaPairs}", qaPairs); | ||
|
|
||
| return generate( | ||
| loadPromptTemplate(REPORT_SYSTEM_PROMPT_PATH), | ||
| userPrompt | ||
| ); | ||
| } |
There was a problem hiding this comment.
입력 값 검증 로직이 필요합니다.
generateInterviewQuestions 메서드는 validateInterviewQuestionSource를 통해 모든 입력값이 비어있지 않은지 검증하지만, generateReport는 검증 없이 6개 파라미터를 모두 프롬프트에 전달합니다. null이나 빈 문자열이 전달되면 품질이 낮은 리포트가 생성되거나 예상치 못한 동작이 발생할 수 있습니다.
🛡️ 입력 검증 추가 제안
public String generateReport(
String codeContent, String codePurpose, String techRationale,
String exceptionHandling, String projectScale, String qaPairs) {
+
+ if (isBlank(codeContent)
+ || isBlank(codePurpose)
+ || isBlank(techRationale)
+ || isBlank(exceptionHandling)
+ || isBlank(projectScale)
+ || isBlank(qaPairs)) {
+ throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "리포트 생성에 필요한 입력값이 비어 있습니다.");
+ }
String userPrompt = loadPromptTemplate(REPORT_USER_PROMPT_PATH)📝 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.
Suggested change
| public String generateReport( | |
| String codeContent, String codePurpose, String techRationale, | |
| String exceptionHandling, String projectScale, String qaPairs) { | |
| String userPrompt = loadPromptTemplate(REPORT_USER_PROMPT_PATH) | |
| .replace("{codeContent}", codeContent) | |
| .replace("{codePurpose}", codePurpose) | |
| .replace("{techRationale}", techRationale) | |
| .replace("{exceptionHandling}", exceptionHandling) | |
| .replace("{projectScale}", projectScale) | |
| .replace("{qaPairs}", qaPairs); | |
| return generate( | |
| loadPromptTemplate(REPORT_SYSTEM_PROMPT_PATH), | |
| userPrompt | |
| ); | |
| } | |
| public String generateReport( | |
| String codeContent, String codePurpose, String techRationale, | |
| String exceptionHandling, String projectScale, String qaPairs) { | |
| if (isBlank(codeContent) | |
| || isBlank(codePurpose) | |
| || isBlank(techRationale) | |
| || isBlank(exceptionHandling) | |
| || isBlank(projectScale) | |
| || isBlank(qaPairs)) { | |
| throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "리포트 생성에 필요한 입력값이 비어 있습니다."); | |
| } | |
| String userPrompt = loadPromptTemplate(REPORT_USER_PROMPT_PATH) | |
| .replace("{codeContent}", codeContent) | |
| .replace("{codePurpose}", codePurpose) | |
| .replace("{techRationale}", techRationale) | |
| .replace("{exceptionHandling}", exceptionHandling) | |
| .replace("{projectScale}", projectScale) | |
| .replace("{qaPairs}", qaPairs); | |
| return generate( | |
| loadPromptTemplate(REPORT_SYSTEM_PROMPT_PATH), | |
| userPrompt | |
| ); | |
| } |
🤖 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/example/trouble_log/domain/ai/service/AzureOpenAiPromptService.java`
around lines 271 - 287, The generateReport method currently forwards six raw
params to the prompt template; add input validation (either reuse
validateInterviewQuestionSource or introduce a new private validateReportSource)
that checks codeContent, codePurpose, techRationale, exceptionHandling,
projectScale, and qaPairs for null/blank (trimmed) and throws an informative
IllegalArgumentException (or returns an error) before calling
loadPromptTemplate/generate; ensure validation happens at the top of
generateReport so loadPromptTemplate is not invoked with invalid inputs and
reference the generateReport and
validateInterviewQuestionSource/validateReportSource symbols when locating the
change.
| this.answer = answer; | ||
| this.isSkipped = isSkipped; | ||
| this.feedback = feedback; | ||
| this.createdAt = LocalDateTime.now(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
엔티티 생성 시점을 JPA 라이프사이클 훅으로 관리하세요.
생성자에서 LocalDateTime.now()를 직접 호출하면 단위 테스트 시 시간 제어가 어렵고, 엔티티가 시스템 시계에 강결합됩니다. JPA에서 제공하는 @PrePersist 또는 Hibernate의 @CreationTimestamp를 사용하는 것이 표준 패턴입니다.
♻️ 제안하는 리팩터링
방안 1: @PrePersist 사용
+ `@PrePersist`
+ protected void onCreate() {
+ this.createdAt = LocalDateTime.now();
+ }
+
// 답변 저장용 생성자
public InterviewAnswer(InterviewQuestion interviewQuestion,
String answer,
Boolean isSkipped,
String feedback) {
this.interviewQuestion = interviewQuestion;
this.answer = answer;
this.isSkipped = isSkipped;
this.feedback = feedback;
- this.createdAt = LocalDateTime.now();
}방안 2 (권장): @CreationTimestamp 사용
+ import org.hibernate.annotations.CreationTimestamp;
+
- `@Column`(nullable = false, updatable = false)
+ `@CreationTimestamp`
+ `@Column`(nullable = false, updatable = false)
private LocalDateTime createdAt;
// 답변 저장용 생성자
public InterviewAnswer(InterviewQuestion interviewQuestion,
String answer,
Boolean isSkipped,
String feedback) {
this.interviewQuestion = interviewQuestion;
this.answer = answer;
this.isSkipped = isSkipped;
this.feedback = feedback;
- this.createdAt = LocalDateTime.now();
}📝 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.
Suggested change
| this.createdAt = LocalDateTime.now(); | |
| import org.hibernate.annotations.CreationTimestamp; | |
| `@CreationTimestamp` | |
| `@Column`(nullable = false, updatable = false) | |
| private LocalDateTime createdAt; | |
| // 답변 저장용 생성자 | |
| public InterviewAnswer(InterviewQuestion interviewQuestion, | |
| String answer, | |
| Boolean isSkipped, | |
| String feedback) { | |
| this.interviewQuestion = interviewQuestion; | |
| this.answer = answer; | |
| this.isSkipped = isSkipped; | |
| this.feedback = feedback; | |
| } |
🤖 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/example/trouble_log/domain/interview/entity/InterviewAnswer.java`
at line 44, The InterviewAnswer constructor currently sets createdAt =
LocalDateTime.now() which couples the entity to the system clock; remove that
assignment from the constructor and manage creation time with JPA/Hibernate
instead by either adding a `@PrePersist-annotated` method that sets createdAt if
null, or (recommended) annotate the createdAt field with Hibernate's
`@CreationTimestamp` and ensure the field is of type LocalDateTime; update imports
accordingly and keep the class name InterviewAnswer and the createdAt field as
the target for this change.
Comment on lines
+70
to
+74
| try { | ||
| feedbackJson = objectMapper.writeValueAsString(feedbackResult); | ||
| } catch (Exception e) { | ||
| feedbackJson = "{}"; | ||
| } |
There was a problem hiding this comment.
JSON 직렬화 실패를 로깅해야 합니다.
objectMapper.writeValueAsString 실패 시 빈 JSON("{}")으로 대체하고 있지만, 예외를 완전히 무시하고 있어 디버깅이 어렵습니다. AnswerFeedbackResult는 단순한 DTO이므로 직렬화 실패는 드물지만, 발생 시 원인 파악을 위해 최소한 로깅이 필요합니다.
📝 로깅 추가 제안
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
`@Service`
`@RequiredArgsConstructor`
`@Transactional`(readOnly = true)
public class InterviewService { try {
feedbackJson = objectMapper.writeValueAsString(feedbackResult);
} catch (Exception e) {
+ log.error("AI 피드백 JSON 직렬화 실패 - questionId: {}, sessionId: {}", questionId, sessionId, e);
feedbackJson = "{}";
}📝 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.
Suggested change
| try { | |
| feedbackJson = objectMapper.writeValueAsString(feedbackResult); | |
| } catch (Exception e) { | |
| feedbackJson = "{}"; | |
| } | |
| import lombok.extern.slf4j.Slf4j; | |
| `@Slf4j` | |
| `@Service` | |
| `@RequiredArgsConstructor` | |
| `@Transactional`(readOnly = true) | |
| public class InterviewService { | |
| // ... other code ... | |
| try { | |
| feedbackJson = objectMapper.writeValueAsString(feedbackResult); | |
| } catch (Exception e) { | |
| log.error("AI 피드백 JSON 직렬화 실패 - questionId: {}, sessionId: {}", questionId, sessionId, e); | |
| feedbackJson = "{}"; | |
| } |
🤖 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/example/trouble_log/domain/interview/service/InterviewService.java`
around lines 70 - 74, The catch block swallowing exceptions when calling
objectMapper.writeValueAsString(feedbackResult) must log the failure so issues
can be diagnosed; update the try/catch around objectMapper.writeValueAsString in
InterviewService (the block that assigns feedbackJson) to catch Exception e and
call the class logger (e.g., logger.error or log.warn depending on logger used)
with a clear message including the feedbackResult or its id and the exception
(e), then keep the fallback feedbackJson = "{}" as before.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/example/trouble_log/domain/interview/entity/InterviewAnswer.java`:
- Around line 51-53: The updateAnswer method in InterviewAnswer currently
mutates only the answer field causing state invariants with isSkipped and
feedback to break; modify InterviewAnswer.updateAnswer(String answer) to
atomically update related fields too: when a non-empty answer is set, set
isSkipped = false and clear or reset feedback; when the answer is empty/null,
set isSkipped = true and preserve/clear feedback according to domain rules, and
add validation/assertion to prevent inconsistent states (reference fields:
answer, isSkipped, feedback and method InterviewAnswer.updateAnswer).
- Around line 34-38: The InterviewAnswer constructor currently allows a
null/blank answer while leaving isSkipped=false; update the
InterviewAnswer(InterviewQuestion interviewQuestion, String answer) constructor
to validate the answer: if answer is null or blank (use StringUtils.isBlank or
answer.trim().isEmpty()), set this.answer = null and this.isSkipped = true;
otherwise set this.answer = answer.trim() and this.isSkipped = false; keep
setting createdAt as before and throw IllegalArgumentException only if
interviewQuestion is null to preserve invariants.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: fca54a50-f389-4367-9fbd-ece11e2f3484
📒 Files selected for processing (3)
src/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewAnswerRepository.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewQuestionRepository.java
💤 Files with no reviewable changes (1)
- src/main/java/com/example/trouble_log/domain/interview/repository/InterviewQuestionRepository.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/trouble_log/domain/interview/repository/InterviewAnswerRepository.java
Comment on lines
+34
to
+38
| public InterviewAnswer(InterviewQuestion interviewQuestion, String answer) { | ||
| this.interviewQuestion = interviewQuestion; | ||
| this.answer = answer; | ||
| this.createdAt = LocalDateTime.now(); | ||
| } |
There was a problem hiding this comment.
답변 전용 생성자는 null/blank 입력을 막거나 skip 상태를 함께 맞춰야 합니다.
Line 34-38 생성자는 answer가 비어 있어도 isSkipped=false 상태가 될 수 있어, 이후 리포트 조립 시 A. null 같은 잘못된 출력으로 이어질 수 있습니다.
🐛 제안 변경
public InterviewAnswer(InterviewQuestion interviewQuestion, String answer) {
+ if (answer == null || answer.isBlank()) {
+ throw new IllegalArgumentException("answer must not be blank");
+ }
this.interviewQuestion = interviewQuestion;
this.answer = answer;
+ this.isSkipped = false;
}🤖 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/example/trouble_log/domain/interview/entity/InterviewAnswer.java`
around lines 34 - 38, The InterviewAnswer constructor currently allows a
null/blank answer while leaving isSkipped=false; update the
InterviewAnswer(InterviewQuestion interviewQuestion, String answer) constructor
to validate the answer: if answer is null or blank (use StringUtils.isBlank or
answer.trim().isEmpty()), set this.answer = null and this.isSkipped = true;
otherwise set this.answer = answer.trim() and this.isSkipped = false; keep
setting createdAt as before and throw IllegalArgumentException only if
interviewQuestion is null to preserve invariants.
Comment on lines
+51
to
+53
| public void updateAnswer(String answer) { | ||
| this.answer = answer; | ||
| } |
There was a problem hiding this comment.
updateAnswer가 도메인 상태를 부분 갱신해 불변식이 깨집니다.
Line 51-53은 answer만 바꾸고 isSkipped/feedback을 동기화하지 않아 상태 모순이 생깁니다.
🐛 제안 변경
public void updateAnswer(String answer) {
- this.answer = answer;
+ boolean skipped = (answer == null || answer.isBlank());
+ this.answer = skipped ? null : answer;
+ this.isSkipped = skipped;
+ if (skipped) {
+ this.feedback = null;
+ }
}📝 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.
Suggested change
| public void updateAnswer(String answer) { | |
| this.answer = answer; | |
| } | |
| public void updateAnswer(String answer) { | |
| boolean skipped = (answer == null || answer.isBlank()); | |
| this.answer = skipped ? null : answer; | |
| this.isSkipped = skipped; | |
| if (skipped) { | |
| this.feedback = 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/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.java`
around lines 51 - 53, The updateAnswer method in InterviewAnswer currently
mutates only the answer field causing state invariants with isSkipped and
feedback to break; modify InterviewAnswer.updateAnswer(String answer) to
atomically update related fields too: when a non-empty answer is set, set
isSkipped = false and clear or reset feedback; when the answer is empty/null,
set isSkipped = true and preserve/clear feedback according to domain rules, and
add validation/assertion to prevent inconsistent states (reference fields:
answer, isSkipped, feedback and method InterviewAnswer.updateAnswer).
dl-tpdnjs
force-pushed
the
feature/#14-report-api
branch
2 times, most recently
from
June 5, 2026 15:46
b1a334a to
c903c55
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/example/trouble_log/domain/interview/entity/InterviewAnswer.java`:
- Around line 40-41: The InterviewAnswer entity is assigning to isSkipped and
feedback in its constructor but those fields are not declared; add two fields to
the InterviewAnswer class: declare a Boolean isSkipped and a String feedback
(e.g., private Boolean isSkipped; private String feedback;) directly below the
existing answer field declaration so the constructor assignments compile and the
entity holds those values.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: 7d60579e-d6f0-407c-80da-02e29f3c6373
📒 Files selected for processing (16)
src/main/java/com/example/trouble_log/domain/ai/service/AzureOpenAiPromptService.javasrc/main/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculator.javasrc/main/java/com/example/trouble_log/domain/analysis/entity/AnalysisResult.javasrc/main/java/com/example/trouble_log/domain/interview/controller/InterviewController.javasrc/main/java/com/example/trouble_log/domain/interview/dto/AnswerRequest.javasrc/main/java/com/example/trouble_log/domain/interview/dto/AnswerResponse.javasrc/main/java/com/example/trouble_log/domain/interview/dto/ReportResponse.javasrc/main/java/com/example/trouble_log/domain/interview/entity/InterviewAnswer.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewAnswerRepository.javasrc/main/java/com/example/trouble_log/domain/interview/repository/InterviewQuestionRepository.javasrc/main/java/com/example/trouble_log/domain/interview/service/InterviewService.javasrc/main/java/com/example/trouble_log/domain/projectSession/repository/PreContextRepository.javasrc/main/resources/prompts/report-system.txtsrc/main/resources/prompts/report-user.txtsrc/test/java/com/example/trouble_log/domain/ai/PromptIntegrationTest.javasrc/test/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java
✅ Files skipped from review due to trivial changes (2)
- src/main/java/com/example/trouble_log/domain/interview/repository/InterviewQuestionRepository.java
- src/main/resources/prompts/report-user.txt
🚧 Files skipped from review as they are similar to previous changes (13)
- src/main/java/com/example/trouble_log/domain/interview/repository/InterviewAnswerRepository.java
- src/main/java/com/example/trouble_log/domain/interview/dto/ReportResponse.java
- src/main/java/com/example/trouble_log/domain/interview/dto/AnswerRequest.java
- src/main/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculator.java
- src/main/resources/prompts/report-system.txt
- src/main/java/com/example/trouble_log/domain/ai/service/AzureOpenAiPromptService.java
- src/main/java/com/example/trouble_log/domain/projectSession/repository/PreContextRepository.java
- src/main/java/com/example/trouble_log/domain/analysis/entity/AnalysisResult.java
- src/main/java/com/example/trouble_log/domain/interview/dto/AnswerResponse.java
- src/main/java/com/example/trouble_log/domain/interview/controller/InterviewController.java
- src/test/java/com/example/trouble_log/domain/ai/service/RadarScoreCalculatorTest.java
- src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java
- src/test/java/com/example/trouble_log/domain/ai/PromptIntegrationTest.java
This was referenced Jun 6, 2026
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
2 participants
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.
✨ 관련된 이슈
🙌 작업 내용
📸 스크린샷 (선택)
🤔 리뷰 요구사항(선택)
Summary by CodeRabbit