Repository navigation
[FEAT] 레이더 차트 전체 답변 평균 계산 및 DB 피드백 파싱 적용 - #18
Merged
Merged
Conversation
|
Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInterviewService의 generateReport가 DB에 저장된 모든 답변의 feedback JSON을 파싱해 유효 피드백을 평균화하고, 그 평균을 기반으로 레이더 점수를 계산하도록 변경되었습니다. 로깅( Changes레이더 차트 평균 피드백 적용
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 시 (詩)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 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/service/InterviewService.java`:
- Around line 140-155: The current stream in InterviewService that builds
feedbackList deserializes AnswerFeedbackResult but still includes entries with
null scores, causing NPE later; update the mapping/filtering so after
objectMapper.readValue returns an AnswerFeedbackResult you also check that its
getScores() != null (and optionally that required score fields are present)
before returning it, i.e., inside the map catch block return null for invalid or
score-less results and keep the existing filter(f -> f != null), and similarly
ensure any downstream code around f.getScores() (used at lines ~198-205) only
processes entries with non-null scores.
🪄 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: 82f6426f-ba44-491a-8f5c-b6901e025bb1
📒 Files selected for processing (1)
src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java
There was a problem hiding this comment.
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/example/trouble_log/domain/interview/service/InterviewService.java (1)
213-220:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winvalidList 대신 feedbackList를 사용하여 NPE가 발생합니다.
Line 207-209에서 scores가 null인 항목을 제외한
validList를 생성했지만, Line 213-220의 평균 계산에서는 원래feedbackList를 사용하고 있습니다. 이로 인해 scores가 null인 피드백이 하나라도 있으면.getScores().getSpecificity()호출 시 NPE가 발생합니다.
saveAnswer메서드의 Line 88에서 파싱 실패 시"{}"를 저장하는데, 이는 scores가 null인 AnswerFeedbackResult로 역직렬화됩니다. 따라서 이 버그는 실제 프로덕션에서 발생할 수 있습니다.🐛 제안하는 수정
- int specificity = (int) Math.round(feedbackList.stream() + int specificity = (int) Math.round(validList.stream() .mapToInt(f -> f.getScores().getSpecificity()).average().orElse(0)); - int structure = (int) Math.round(feedbackList.stream() + int structure = (int) Math.round(validList.stream() .mapToInt(f -> f.getScores().getStructure()).average().orElse(0)); - int relevance = (int) Math.round(feedbackList.stream() + int relevance = (int) Math.round(validList.stream() .mapToInt(f -> f.getScores().getRelevance()).average().orElse(0)); - int keyword = (int) Math.round(feedbackList.stream() + int keyword = (int) Math.round(validList.stream() .mapToInt(f -> f.getScores().getKeyword()).average().orElse(0));🤖 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 213 - 220, The averages are computed from feedbackList (causing NPE when some Feedback have null scores) even though you already built validList excluding null scores; update the four average calculations (the computations that assign specificity, structure, relevance, keyword) in InterviewService.saveAnswer to stream over validList instead of feedbackList so .getScores() is only called on non-null score entries, keeping the existing .average().orElse(0) handling for empty validList cases.
🧹 Nitpick comments (2)
src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java (2)
150-150: 💤 Low value불필요한 null 체크입니다.
objectMapper.readValue()는 파싱 실패 시 예외를 던지며 null을 반환하지 않으므로,result == null체크는 불필요합니다.result.getScores() == null체크만으로 충분합니다.♻️ 제안하는 수정
- if (result == null || result.getScores() == null) { + if (result.getScores() == null) { log.warn("피드백 scores 없음. questionId={}", q.getId()); return 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/service/InterviewService.java` at line 150, 조건문에서 result == null 검사는 불필요하므로 제거하세요: InterviewService 내에서 objectMapper.readValue(...)로 파싱한 후 사용하는 변수 result에 대해 예외가 발생하면 readValue가 예외를 던지므로 null이 돌아오지 않습니다. 해당 if 문을 result.getScores() == null 검사만 하도록 수정하고, 필요하면 readValue 호출부를 try/catch로 감싸 예외를 처리하거나 호출 흐름에서 이미 예외를 처리하고 있음을 확인하세요; 참조 심볼: result, objectMapper.readValue(...), InterviewService.
156-156: ⚡ Quick win파싱 실패 로그에 예외 정보를 포함하세요.
디버깅을 용이하게 하기 위해 예외 객체를 로그에 포함하는 것이 좋습니다.
♻️ 제안하는 수정
} catch (Exception e) { - log.warn("피드백 파싱 실패. questionId={}", q.getId()); + log.warn("피드백 파싱 실패. questionId={}", q.getId(), e); return 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/service/InterviewService.java` at line 156, The warning log in InterviewService (the log.warn("피드백 파싱 실패. questionId={}", q.getId()) call) omits the caught exception—update the catch block that surrounds the feedback parsing to pass the exception to the logger so the stacktrace is recorded (e.g., include the caught Exception variable, commonly named e, as the final argument to log.warn). Locate the try/catch around the parsing logic in InterviewService and change the log.warn call to include the exception object (keeping the questionId argument) so the log shows both context and the throwable.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java`:
- Around line 213-220: The averages are computed from feedbackList (causing NPE
when some Feedback have null scores) even though you already built validList
excluding null scores; update the four average calculations (the computations
that assign specificity, structure, relevance, keyword) in
InterviewService.saveAnswer to stream over validList instead of feedbackList so
.getScores() is only called on non-null score entries, keeping the existing
.average().orElse(0) handling for empty validList cases.
---
Nitpick comments:
In
`@src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.java`:
- Line 150: 조건문에서 result == null 검사는 불필요하므로 제거하세요: InterviewService 내에서
objectMapper.readValue(...)로 파싱한 후 사용하는 변수 result에 대해 예외가 발생하면 readValue가 예외를
던지므로 null이 돌아오지 않습니다. 해당 if 문을 result.getScores() == null 검사만 하도록 수정하고, 필요하면
readValue 호출부를 try/catch로 감싸 예외를 처리하거나 호출 흐름에서 이미 예외를 처리하고 있음을 확인하세요; 참조 심볼:
result, objectMapper.readValue(...), InterviewService.
- Line 156: The warning log in InterviewService (the log.warn("피드백 파싱 실패.
questionId={}", q.getId()) call) omits the caught exception—update the catch
block that surrounds the feedback parsing to pass the exception to the logger so
the stacktrace is recorded (e.g., include the caught Exception variable,
commonly named e, as the final argument to log.warn). Locate the try/catch
around the parsing logic in InterviewService and change the log.warn call to
include the exception object (keeping the questionId argument) so the log shows
both context and the throwable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9e5a6526-9ab1-4e05-bb64-e885789b3150
📒 Files selected for processing (1)
src/main/java/com/example/trouble_log/domain/interview/service/InterviewService.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.
✨ 관련된 이슈
🙌 작업 내용
📸 스크린샷 (선택)
🤔 리뷰 요구사항(선택)
Summary by CodeRabbit
버그 수정
리팩토링