[FEAT] AI 분석 요청 및 Gemini 리포트 생성 (#22, #74) - #84
Merged
Conversation
ownue
requested review from
kimyw1018,
on1yoneprivate,
p1001q and
rkdehdrbs7885-oss
July 30, 2026 15:36
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthrough분석 생성 API와 비동기 처리 흐름이 추가되었습니다. MIDI 데이터 변환, AI 결과 검증, Gemini·규칙 기반 리포트 생성, stale 작업 복구, 결과 응답 확장과 관련 설정이 포함됩니다. Changes분석 처리 흐름
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
actor Client
participant AnalysisController
participant AnalysisService
participant AnalysisRequestedEventListener
participant AnalysisProcessingService
participant AnalysisStateService
participant GeminiClient
Client->>AnalysisController: POST /api/analyses
AnalysisController->>AnalysisService: createAnalysis(userId, request)
AnalysisService->>AnalysisRequestedEventListener: AnalysisRequestedEvent 발행
AnalysisRequestedEventListener->>AnalysisProcessingService: process(analysisId)
AnalysisProcessingService->>AnalysisStateService: startProcessing(analysisId)
AnalysisProcessingService->>GeminiClient: 분석 리포트 생성 요청
AnalysisProcessingService->>AnalysisStateService: complete(analysisId, result, report)
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 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: 3
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/mr/domain/statistics/service/StatisticsEventListener.java (1)
23-28: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win재시도 주석과 실제 조건을 일치시켜 주세요.
현재 주석은 동시 upsert 충돌만 재시도한다고 설명하지만,
@Retryable은 모든DataIntegrityViolationException을 최대 3회 재시도합니다. 영구적인 NOT NULL/FK 오류까지 불필요하게 지연시킬 수 있으므로, 실제 동작에 맞게 주석을 수정하거나 동시 upsert 충돌만 선별하는 예외 분류를 추가해야 합니다.권장 수정
- // 동시 upsert 충돌 한정 재시도 + // DataIntegrityViolationException 발생 시 최대 3회 재시도🤖 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/mr/domain/statistics/service/StatisticsEventListener.java` around lines 23 - 28, StatisticsEventListener의 `@Retryable` 설정과 주석의 범위를 일치시키세요. 모든 DataIntegrityViolationException을 재시도하는 현재 동작을 유지한다면 주석을 일반적인 무결성 예외 재시도로 수정하고, 동시 upsert 충돌만 재시도해야 한다면 retryFor를 해당 예외 분류로 제한하세요.
🧹 Nitpick comments (8)
src/main/java/com/mr/global/event/PlayingCompletedEvent.java (1)
12-12: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win이벤트의 소유권 불변식을 주석에 유지해 주세요.
userId는 단순 식별자가 아니라 완료된 Playing의 소유자에서 서버가 채워야 하며, 클라이언트 입력을 신뢰하면 안 된다는 계약이 삭제되었습니다. Spring 애플리케이션 이벤트의 publisher 경계에서 이 불변식을 다시 명시해 통계가 잘못된 사용자에게 집계되지 않도록 해 주세요.🔒️ 제안
- // 완료 연주 소유자 식별자 + // 완료된 Playing의 소유자 식별자이며, 클라이언트 입력으로 설정하지 않는다🤖 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/mr/global/event/PlayingCompletedEvent.java` at line 12, PlayingCompletedEvent의 userId 주석에 해당 값이 클라이언트 입력이 아니라 완료된 Playing의 소유자를 기준으로 서버가 채우는 값이라는 소유권 불변식을 다시 명시하세요. Spring 애플리케이션 이벤트 publisher 경계에서 이 계약이 드러나도록 기존 주석을 보완하고, 통계 집계 대상이 소유자와 일치해야 한다는 의미를 유지하세요.src/main/java/com/mr/domain/analysis/service/AnalysisStateService.java (1)
128-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win중첩 삼항 대신 switch 표현식으로 정리하면 읽기 훨씬 편합니다.
이 파일은 이미
resolveGrade에서 switch 표현식을 쓰고 있으니 스타일도 통일됩니다.♻️ 제안 리팩터링
- LlmCallLog log = metadata.status() == LlmCallStatus.SUCCESS - ? LlmCallLog.success(...) - : metadata.status() == LlmCallStatus.TIMEOUT - ? LlmCallLog.timeout(...) - : LlmCallLog.failed(...); + LlmCallLog log = switch (metadata.status()) { + case SUCCESS -> LlmCallLog.success( + analysis.getUser(), analysis, report, null, + LlmPurpose.REPORT_GENERATION, metadata.modelName(), metadata.promptVersion(), + metadata.promptSnapshot(), metadata.promptTokens(), metadata.completionTokens(), + metadata.totalTokens(), metadata.temperature(), metadata.latencyMs(), + metadata.cacheHit(), metadata.inputHash() + ); + case TIMEOUT -> LlmCallLog.timeout( + analysis.getUser(), analysis, report, null, + LlmPurpose.REPORT_GENERATION, metadata.modelName(), metadata.promptVersion(), + metadata.promptSnapshot(), metadata.temperature(), metadata.latencyMs(), + metadata.inputHash(), metadata.errorMessage() + ); + default -> LlmCallLog.failed( + analysis.getUser(), analysis, report, null, + LlmPurpose.REPORT_GENERATION, metadata.modelName(), metadata.promptVersion(), + metadata.promptSnapshot(), metadata.temperature(), metadata.latencyMs(), + metadata.inputHash(), metadata.errorMessage() + ); + };🤖 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/mr/domain/analysis/service/AnalysisStateService.java` around lines 128 - 148, Replace the nested ternary that constructs LlmCallLog with a switch expression on metadata.status(), using separate SUCCESS, TIMEOUT, and default/failure branches. Preserve the existing factory methods, arguments, and resulting behavior, matching the switch-expression style already used by resolveGrade.src/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.java (1)
176-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLLM 리포트 경로 커버리지도 추가하면 좋겠습니다.
현재 픽스처는
RULE_BASED+LlmCallStatus.FAILED조합만 사용하므로AnalysisStateService의createLlmReport분기(89-96행)와LlmCallLog.success/timeout분기(128-148행)가 전혀 실행되지 않습니다.generationType/status를 파라미터로 받는 헬퍼로 바꾸고@ParameterizedTest로 세 상태를 돌리면 적은 코드로 분기 전체를 덮을 수 있습니다.🤖 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/mr/domain/analysis/service/AnalysisStateServiceTest.java` around lines 176 - 197, The generatedReport fixture only covers RULE_BASED with FAILED, leaving the LLM report and LlmCallLog success/timeout branches untested. Update generatedReport to accept generationType and status parameters, then add a parameterized test covering the three relevant LLM call statuses while preserving the existing failed-case assertions.src/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.java (1)
33-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win행복 경로 테스트가 빠져 있습니다.
세 케이스 모두 "AI를 호출하지 않는다/리포트를 만들지 않는다"는 음성 검증입니다. 유효한 AI 응답이 왔을 때
reportGenerationService.generate(...)결과가 그대로analysisStateService.complete(analysisId, result, rawJson, report)로 전달되는지 검증하는 테스트를 추가하면 이 서비스의 핵심 계약이 회귀로부터 보호됩니다. 참고로org.mockito.ArgumentMatchers.any()는 이미 정적 임포트가 가능한 형태이니import static으로 정리하면 가독성이 좋아집니다.🤖 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/mr/domain/analysis/service/AnalysisProcessingServiceTest.java` around lines 33 - 79, 분석 처리의 성공 경로 검증이 누락되어 있습니다. AnalysisProcessingService.process를 대상으로 유효한 AI 응답과 보고서 생성 결과를 설정한 테스트를 추가하고, reportGenerationService.generate(...)가 호출되며 그 결과가 원본 AI 응답 및 rawJson과 함께 analysisStateService.complete(analysisId, result, rawJson, report)에 그대로 전달되는지 검증하세요. 기존 테스트의 ArgumentMatchers.any() 사용은 가능하면 정적 임포트로 정리하세요.src/main/java/com/mr/domain/analysis/service/AnalysisRecoveryScheduler.java (2)
65-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
try/catch가 잡는 것은 "제출 실패"뿐임을 감안해 주세요.
taskExecutor.execute(...)는 큐 적재만 하므로 여기 catch는RejectedExecutionException정도만 처리합니다. 워커 스레드 내부에서process/recoverStaleProcessing이 예외를 던지면 이 로그에는 남지 않으니, 두 메서드 내부에서 반드시 상태를FAILED로 전이시키고 로깅하는지 확인해 주세요(그렇지 않으면 다음 주기에 무한 재시도됩니다). 두 반복문이 콜백만 다르므로submit(List<Long>, Consumer<Long>)형태로 합치는 것도 깔끔합니다.🤖 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/mr/domain/analysis/service/AnalysisRecoveryScheduler.java` around lines 65 - 85, Update AnalysisProcessingService.process and recoverStaleProcessing so exceptions raised during worker execution are logged and reliably transition the analysis state to FAILED, preventing stale records from being retried indefinitely. Keep the submitPending and submitStaleProcessing catches limited to task submission failures; optionally consolidate their shared executor logic through a submit helper accepting a Consumer<Long>.
31-33: 🩺 Stability & Availability | 🔵 Trivial
@ValueDuration 변환은 컨텍스트 기동 경로에서 검증하거나@ConfigurationProperties로 옮겨 주세요.Spring Boot의
1m/5m같은 Duration 표기는ApplicationConversionService기반 변환에서 다룰 수 있지만,AnalysisRecoverySchedulerTest는 실제 컨텍스트를 만든 뒤 생성자를 호출하므로 이 변환 경로가 덮어지지 않습니다. 부팅 시점의 컨텍스트 로딩 테스트로 추가하거나, 타입 안전성과 재사용성을 위해 생성자 인자를@ConfigurationProperties레코드/클래스로 그룹화하는 편이 좋습니다.🤖 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/mr/domain/analysis/service/AnalysisRecoveryScheduler.java` around lines 31 - 33, AnalysisRecoveryScheduler의 pendingThreshold 및 processingThreshold에 대한 Duration `@Value` 변환을 실제 Spring 컨텍스트 기동 경로에서 검증할 수 있도록 수정하세요. AnalysisRecoverySchedulerTest에 해당 1m/5m 프로퍼티를 포함한 컨텍스트 로딩 검증을 추가하거나, 두 Duration과 batchSize를 `@ConfigurationProperties` 레코드/클래스로 그룹화해 타입 안전하게 주입되도록 변경하세요.MR_config/local/application.example.yml (1)
53-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winpreview 모델 기본값과 빈 API 키 기본값의 운영 리스크를 한 번 검토해 주세요.
gemini-3-flash-preview는 공식 문서에 존재하는 모델이지만 preview 모델은 프로덕션 사용은 가능하나 더 엄격한 rate limit이 적용되고 최소 2주 공지 후 deprecate될 수 있습니다. 안정 버전(예: Flash 계열 GA 모델)이나gemini-flash-latest같은 별칭 사용을 검토해 보세요.또한
api-key의 최종 기본값이 빈 문자열이라 키 없이도 애플리케이션이 정상 부팅되고, 실제 실패는 첫 Gemini 호출 시점에 나타납니다. 이 PR의 규칙 기반 fallback과 맞물리면 "키 누락"이 조용히 저품질 리포트로 이어질 수 있으니,GeminiProperties에@Validated@NotBlank``를 붙여 기동 시 fail-fast 하거나 최소한 경고 로그를 남기는 편이 안전합니다.🤖 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 `@MR_config/local/application.example.yml` around lines 53 - 57, Review the Gemini defaults in the application example configuration: replace the preview model default with a stable GA Flash model or the supported latest alias, and prevent an empty api-key default from allowing silent startup. Update GeminiProperties validation to fail fast on a blank key, or add an explicit startup warning if blank keys are intentionally supported.src/main/java/com/mr/domain/analysis/service/AnalysisRequestFactory.java (1)
40-46: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
NoteType.valueOf()예외가 상위에서 캐치되지 않습니다.
AiAnalysisRequest.NoteType.valueOf(event.getType().name())은 두 enum의 상수 이름이 어긋나면IllegalArgumentException을 던지는데, 이 예외는AnalysisService.createAnalysis의catch (JsonProcessingException)블록에 걸리지 않아 제어되지 않은 500 응답으로 전파됩니다(다운스트림 영향:AnalysisService.javaLine 64-71).두 enum이 지금은 동기화되어 있어 당장 문제는 아니지만, 향후 한쪽만 값이 추가/변경되면 조용히 터질 수 있는 지점입니다.
AnalysisErrorStatus.ANALYSIS_INVALID_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/mr/domain/analysis/service/AnalysisRequestFactory.java` around lines 40 - 46, Update the analysis request creation flow around AnalysisRequestFactory and AnalysisService.createAnalysis to catch IllegalArgumentException from AiAnalysisRequest.NoteType.valueOf and map it to AnalysisErrorStatus.ANALYSIS_INVALID_REQUEST. Ensure mismatched enum values produce the established invalid-request response instead of propagating as an uncontrolled 500, while preserving normal request creation for matching values.
🤖 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/mr/domain/analysis/service/AnalysisStateService.java`:
- Around line 44-65: Update startProcessing and restartStaleProcessing to
validate analysis.getAnalysisRequestJson() before wrapping it in Optional, using
a shared requireRequestJson helper or equivalent. Throw the appropriate explicit
domain GeneralException when the value is null or blank, and preserve
Optional.empty() only for the existing status/ownership conditions.
- Around line 67-102: Update complete in AnalysisStateService to load the
analysis using the same PESSIMISTIC_WRITE-locked mechanism as startProcessing,
then proceed only when its status is PROCESSING; return without creating
reports, logs, or events for already-completed or otherwise ineligible analyses.
Apply the same state guard in fail so a completed analysis cannot be
overwritten.
In `@src/main/java/com/mr/domain/analysis/service/ReportGenerationService.java`:
- Around line 55-58: Update ReportGenerationService around
geminiClient.generateReport so the response is returned as an LLM report only
when it contains the required “# 연주 분석 리포트” heading and mandatory “##” sections
in the SYSTEM_PROMPT-defined order; otherwise use the existing rule-based
fallback, including for non-empty safety refusals. Add a regression test in
ReportGenerationServiceTest covering a non-empty Gemini response with missing
headings and asserting RULE_BASED output.
---
Outside diff comments:
In `@src/main/java/com/mr/domain/statistics/service/StatisticsEventListener.java`:
- Around line 23-28: StatisticsEventListener의 `@Retryable` 설정과 주석의 범위를 일치시키세요. 모든
DataIntegrityViolationException을 재시도하는 현재 동작을 유지한다면 주석을 일반적인 무결성 예외 재시도로 수정하고,
동시 upsert 충돌만 재시도해야 한다면 retryFor를 해당 예외 분류로 제한하세요.
---
Nitpick comments:
In `@MR_config/local/application.example.yml`:
- Around line 53-57: Review the Gemini defaults in the application example
configuration: replace the preview model default with a stable GA Flash model or
the supported latest alias, and prevent an empty api-key default from allowing
silent startup. Update GeminiProperties validation to fail fast on a blank key,
or add an explicit startup warning if blank keys are intentionally supported.
In `@src/main/java/com/mr/domain/analysis/service/AnalysisRecoveryScheduler.java`:
- Around line 65-85: Update AnalysisProcessingService.process and
recoverStaleProcessing so exceptions raised during worker execution are logged
and reliably transition the analysis state to FAILED, preventing stale records
from being retried indefinitely. Keep the submitPending and
submitStaleProcessing catches limited to task submission failures; optionally
consolidate their shared executor logic through a submit helper accepting a
Consumer<Long>.
- Around line 31-33: AnalysisRecoveryScheduler의 pendingThreshold 및
processingThreshold에 대한 Duration `@Value` 변환을 실제 Spring 컨텍스트 기동 경로에서 검증할 수 있도록
수정하세요. AnalysisRecoverySchedulerTest에 해당 1m/5m 프로퍼티를 포함한 컨텍스트 로딩 검증을 추가하거나, 두
Duration과 batchSize를 `@ConfigurationProperties` 레코드/클래스로 그룹화해 타입 안전하게 주입되도록 변경하세요.
In `@src/main/java/com/mr/domain/analysis/service/AnalysisRequestFactory.java`:
- Around line 40-46: Update the analysis request creation flow around
AnalysisRequestFactory and AnalysisService.createAnalysis to catch
IllegalArgumentException from AiAnalysisRequest.NoteType.valueOf and map it to
AnalysisErrorStatus.ANALYSIS_INVALID_REQUEST. Ensure mismatched enum values
produce the established invalid-request response instead of propagating as an
uncontrolled 500, while preserving normal request creation for matching values.
In `@src/main/java/com/mr/domain/analysis/service/AnalysisStateService.java`:
- Around line 128-148: Replace the nested ternary that constructs LlmCallLog
with a switch expression on metadata.status(), using separate SUCCESS, TIMEOUT,
and default/failure branches. Preserve the existing factory methods, arguments,
and resulting behavior, matching the switch-expression style already used by
resolveGrade.
In `@src/main/java/com/mr/global/event/PlayingCompletedEvent.java`:
- Line 12: PlayingCompletedEvent의 userId 주석에 해당 값이 클라이언트 입력이 아니라 완료된 Playing의
소유자를 기준으로 서버가 채우는 값이라는 소유권 불변식을 다시 명시하세요. Spring 애플리케이션 이벤트 publisher 경계에서 이 계약이
드러나도록 기존 주석을 보완하고, 통계 집계 대상이 소유자와 일치해야 한다는 의미를 유지하세요.
In
`@src/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.java`:
- Around line 33-79: 분석 처리의 성공 경로 검증이 누락되어 있습니다.
AnalysisProcessingService.process를 대상으로 유효한 AI 응답과 보고서 생성 결과를 설정한 테스트를 추가하고,
reportGenerationService.generate(...)가 호출되며 그 결과가 원본 AI 응답 및 rawJson과 함께
analysisStateService.complete(analysisId, result, rawJson, report)에 그대로 전달되는지
검증하세요. 기존 테스트의 ArgumentMatchers.any() 사용은 가능하면 정적 임포트로 정리하세요.
In `@src/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.java`:
- Around line 176-197: The generatedReport fixture only covers RULE_BASED with
FAILED, leaving the LLM report and LlmCallLog success/timeout branches untested.
Update generatedReport to accept generationType and status parameters, then add
a parameterized test covering the three relevant LLM call statuses while
preserving the existing failed-case assertions.
🪄 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: fd689500-f073-4046-83bc-7b63ca799616
📒 Files selected for processing (49)
MR_config/local/application.example.ymlsrc/main/java/com/mr/Application.javasrc/main/java/com/mr/domain/analysis/controller/AnalysisController.javasrc/main/java/com/mr/domain/analysis/dto/req/AnalysisCreateRequestDTO.javasrc/main/java/com/mr/domain/analysis/dto/res/AnalysisCreateResponseDTO.javasrc/main/java/com/mr/domain/analysis/entity/Analysis.javasrc/main/java/com/mr/domain/analysis/event/AnalysisRequestedEvent.javasrc/main/java/com/mr/domain/analysis/exception/AnalysisErrorStatus.javasrc/main/java/com/mr/domain/analysis/repository/AnalysisRepository.javasrc/main/java/com/mr/domain/analysis/service/AnalysisProcessingService.javasrc/main/java/com/mr/domain/analysis/service/AnalysisRecoveryScheduler.javasrc/main/java/com/mr/domain/analysis/service/AnalysisRequestFactory.javasrc/main/java/com/mr/domain/analysis/service/AnalysisRequestedEventListener.javasrc/main/java/com/mr/domain/analysis/service/AnalysisService.javasrc/main/java/com/mr/domain/analysis/service/AnalysisStateService.javasrc/main/java/com/mr/domain/analysis/service/GeneratedAnalysisReport.javasrc/main/java/com/mr/domain/analysis/service/LlmCallMetadata.javasrc/main/java/com/mr/domain/analysis/service/ReportGenerationService.javasrc/main/java/com/mr/domain/analysis/service/RuleBasedReportGenerator.javasrc/main/java/com/mr/domain/mentor/repository/LlmCallLogRepository.javasrc/main/java/com/mr/domain/playing/exception/PlayingErrorStatus.javasrc/main/java/com/mr/domain/playing/repository/PlayingRepository.javasrc/main/java/com/mr/domain/statistics/entity/PracticeStatistics.javasrc/main/java/com/mr/domain/statistics/entity/UserStatistics.javasrc/main/java/com/mr/domain/statistics/service/AnalysisSkillScoreResolver.javasrc/main/java/com/mr/domain/statistics/service/StatisticsAggregationService.javasrc/main/java/com/mr/domain/statistics/service/StatisticsEventListener.javasrc/main/java/com/mr/domain/statistics/service/StatisticsService.javasrc/main/java/com/mr/global/client/ai/AiAnalysisRequest.javasrc/main/java/com/mr/global/client/gemini/GeminiClient.javasrc/main/java/com/mr/global/client/gemini/GeminiGenerationResult.javasrc/main/java/com/mr/global/config/GeminiProperties.javasrc/main/java/com/mr/global/config/GeminiRestClientConfig.javasrc/main/java/com/mr/global/event/AnalysisCompletedEvent.javasrc/main/java/com/mr/global/event/PlayingCompletedEvent.javasrc/test/java/com/mr/domain/analysis/controller/AnalysisControllerTest.javasrc/test/java/com/mr/domain/analysis/entity/AnalysisTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRecoverySchedulerTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.javasrc/test/java/com/mr/domain/analysis/service/ReportGenerationServiceTest.javasrc/test/java/com/mr/domain/statistics/service/StatisticsAggregationServiceTest.javasrc/test/java/com/mr/domain/statistics/service/StatisticsEventListenerRetryTest.javasrc/test/java/com/mr/domain/statistics/service/StatisticsServiceTest.javasrc/test/java/com/mr/global/client/ai/AiAnalysisRequestSerializationTest.javasrc/test/java/com/mr/global/client/ai/AiServerClientTest.javasrc/test/java/com/mr/global/client/gemini/GeminiClientTest.java
💤 Files with no reviewable changes (3)
- src/main/java/com/mr/domain/statistics/service/AnalysisSkillScoreResolver.java
- src/main/java/com/mr/domain/statistics/service/StatisticsService.java
- src/test/java/com/mr/domain/statistics/service/StatisticsAggregationServiceTest.java
5 tasks
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.java (1)
35-50: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
notes.isEmpty()에INVALID_BAR_RANGE를 재사용하는 대신 전용 에러 코드를 고려하세요.범위 자체는 유효하지만 해당 구간에 실제로 연주된 노트가 없는 경우에도
INVALID_BAR_RANGE가 발생합니다. 클라이언트 입장에서는 "범위가 잘못됨"과 "범위 내 연주 데이터 없음"을 구분하기 어렵습니다. 전용 에러 코드(예:EMPTY_NOTE_RANGE)를 추가하면 원인 진단과 클라이언트 처리 분기가 쉬워집니다.🤖 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/mr/domain/analysis/factory/AnalysisRequestFactory.java` around lines 35 - 50, AnalysisRequestFactory의 notes.isEmpty() 처리에서 INVALID_BAR_RANGE를 재사용하지 말고, 노트가 없는 유효한 구간을 나타내는 전용 에러 코드(예: EMPTY_NOTE_RANGE)를 추가해 사용하세요. 범위 자체가 잘못된 경우의 INVALID_BAR_RANGE 처리는 유지하고, 새 코드를 AnalysisErrorStatus 및 관련 클라이언트 매핑에 일관되게 등록하세요.
🤖 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.
Nitpick comments:
In `@src/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.java`:
- Around line 35-50: AnalysisRequestFactory의 notes.isEmpty() 처리에서
INVALID_BAR_RANGE를 재사용하지 말고, 노트가 없는 유효한 구간을 나타내는 전용 에러 코드(예: EMPTY_NOTE_RANGE)를
추가해 사용하세요. 범위 자체가 잘못된 경우의 INVALID_BAR_RANGE 처리는 유지하고, 새 코드를 AnalysisErrorStatus
및 관련 클라이언트 매핑에 일관되게 등록하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e389eb16-45c3-413b-a067-8eae72a03b2d
📒 Files selected for processing (20)
src/main/java/com/mr/domain/analysis/entity/Analysis.javasrc/main/java/com/mr/domain/analysis/event/listener/AnalysisRequestedEventListener.javasrc/main/java/com/mr/domain/analysis/exception/AnalysisErrorStatus.javasrc/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.javasrc/main/java/com/mr/domain/analysis/generator/RuleBasedReportGenerator.javasrc/main/java/com/mr/domain/analysis/model/AnalysisProcessingClaim.javasrc/main/java/com/mr/domain/analysis/model/GeneratedAnalysisReport.javasrc/main/java/com/mr/domain/analysis/model/LlmCallMetadata.javasrc/main/java/com/mr/domain/analysis/scheduler/AnalysisRecoveryScheduler.javasrc/main/java/com/mr/domain/analysis/service/AnalysisProcessingService.javasrc/main/java/com/mr/domain/analysis/service/AnalysisService.javasrc/main/java/com/mr/domain/analysis/service/AnalysisStateService.javasrc/main/java/com/mr/domain/analysis/service/ReportGenerationService.javasrc/test/java/com/mr/domain/analysis/entity/AnalysisTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRecoverySchedulerTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.javasrc/test/java/com/mr/domain/analysis/service/ReportGenerationServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/test/java/com/mr/domain/analysis/service/AnalysisRecoverySchedulerTest.java
- src/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.java
- src/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.java
- src/main/java/com/mr/domain/analysis/service/AnalysisProcessingService.java
- src/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.java
- src/main/java/com/mr/domain/analysis/service/AnalysisService.java
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/com/mr/domain/analysis/controller/AnalysisControllerTest.java (1)
158-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win신규 필드에 대한 검증 어서션이 빠졌습니다.
genre,key,tempo, 그리고 두 번째LocalDateTime필드는 테스트 데이터에는 추가되었지만andExpect검증에는 없습니다. 이 필드들의 매핑이 잘못되어도 테스트가 통과합니다.
genre,key,tempo값에 대한jsonPath어서션을 추가하세요. 사소해 보여도, 나중에 필드 순서가 바뀌면 이 테스트가 안전망 역할을 못 할 수 있습니다.♻️ 제안하는 추가 어서션
.andExpect(jsonPath("$.data.title").value("Jazz Standard Practice")) + .andExpect(jsonPath("$.data.genre").value("jazz")) + .andExpect(jsonPath("$.data.key").value("C Major")) + .andExpect(jsonPath("$.data.tempo").value(120)) .andExpect(jsonPath("$.data.status").value("COMPLETED"))🤖 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/mr/domain/analysis/controller/AnalysisControllerTest.java` around lines 158 - 165, Update the response assertions in AnalysisControllerTest to validate the newly added genre, key, and tempo fields using their expected JSON paths and test values. Add coverage for the second LocalDateTime field as well if it has a distinct response mapping, while preserving the existing assertions.src/main/java/com/mr/domain/analysis/generator/AnalysisResultEnricher.java (1)
35-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value도메인 키와 요약 문구를 함께 상수로 관리하세요.
AnalysisStateService는 "스케일", "텐션", "진행", "코드 연결"을SCALE,TENSION,PROGRESSION,VOICE_LEADING상수로 검사하지만,AnalysisResultEnricher는 이 값을 또 다른 문자열 리터럴로 하드코딩합니다. 공유 문자열 상수를 도입해 요약switch case에 이 상수를 사용하면 도메인 키 변경 또는 요약 문구 변경 시 드리프트를 함께 잡을 수 있습니다. 코드가 깔끔하게 정리되는 방향이네요.🤖 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/mr/domain/analysis/generator/AnalysisResultEnricher.java` around lines 35 - 41, Update the summary mapping in AnalysisResultEnricher to use shared domain-key constants for SCALE, TENSION, PROGRESSION, and VOICE_LEADING instead of string literals, and manage each key with its associated summary text as a shared constant. Reuse the existing AnalysisStateService constants where appropriate, preserving the current messages and default behavior.
🤖 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/mr/domain/analysis/dto/res/AnalysisResultResponseDTO.java`:
- Around line 67-72: Update formatKey in AnalysisResultResponseDTO to perform
the scale conversion with Locale.ROOT instead of the JVM default locale, while
preserving the existing capitalization and key-formatting behavior.
In
`@src/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.java`:
- Around line 92-147: In process_passesGeneratedReportToFencedCompletion,
replace the Mockito mock created for GeneratedAnalysisReport with a concrete
record instance containing valid test values. Continue stubbing
reportGenerationService.generate(enrichedResult) to return that instance and
verify the same instance is passed to analysisStateService.complete.
In
`@src/test/java/com/mr/domain/analysis/service/ReportGenerationServiceTest.java`:
- Around line 128-155: Update validateMarkdownStructure to validate the content
length of every required Markdown section, not only heading presence and total
report length. Enforce the existing or intended minimum section-length rule so
the short sections used by
generate_fallsBackWhenGeminiReportIsStructurallyValidButTooShort are rejected
and generate returns the RULE_BASED fallback with a FAILED LlmCallStatus.
---
Nitpick comments:
In `@src/main/java/com/mr/domain/analysis/generator/AnalysisResultEnricher.java`:
- Around line 35-41: Update the summary mapping in AnalysisResultEnricher to use
shared domain-key constants for SCALE, TENSION, PROGRESSION, and VOICE_LEADING
instead of string literals, and manage each key with its associated summary text
as a shared constant. Reuse the existing AnalysisStateService constants where
appropriate, preserving the current messages and default behavior.
In `@src/test/java/com/mr/domain/analysis/controller/AnalysisControllerTest.java`:
- Around line 158-165: Update the response assertions in AnalysisControllerTest
to validate the newly added genre, key, and tempo fields using their expected
JSON paths and test values. Add coverage for the second LocalDateTime field as
well if it has a distinct response mapping, while preserving the existing
assertions.
🪄 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: 3ec2eb93-8d50-44f7-9a4e-27eda477efce
📒 Files selected for processing (18)
src/main/java/com/mr/domain/analysis/dto/res/AnalysisResultResponseDTO.javasrc/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.javasrc/main/java/com/mr/domain/analysis/generator/AnalysisResultEnricher.javasrc/main/java/com/mr/domain/analysis/generator/RuleBasedReportGenerator.javasrc/main/java/com/mr/domain/analysis/service/AnalysisProcessingService.javasrc/main/java/com/mr/domain/analysis/service/AnalysisStateService.javasrc/main/java/com/mr/domain/analysis/service/ReportGenerationService.javasrc/main/java/com/mr/global/client/gemini/GeminiClient.javasrc/main/java/com/mr/global/event/PlayingCompletedEvent.javasrc/test/java/com/mr/domain/analysis/controller/AnalysisControllerTest.javasrc/test/java/com/mr/domain/analysis/generator/AnalysisResultEnricherTest.javasrc/test/java/com/mr/domain/analysis/generator/RuleBasedReportGeneratorTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisProcessingServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.javasrc/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.javasrc/test/java/com/mr/domain/analysis/service/ReportGenerationServiceTest.javasrc/test/java/com/mr/global/client/gemini/GeminiClientTest.java
🚧 Files skipped from review as they are similar to previous changes (10)
- src/main/java/com/mr/global/event/PlayingCompletedEvent.java
- src/test/java/com/mr/domain/analysis/service/AnalysisRequestFactoryTest.java
- src/test/java/com/mr/global/client/gemini/GeminiClientTest.java
- src/main/java/com/mr/global/client/gemini/GeminiClient.java
- src/test/java/com/mr/domain/analysis/service/AnalysisServiceTest.java
- src/main/java/com/mr/domain/analysis/service/AnalysisProcessingService.java
- src/test/java/com/mr/domain/analysis/service/AnalysisStateServiceTest.java
- src/main/java/com/mr/domain/analysis/service/AnalysisStateService.java
- src/main/java/com/mr/domain/analysis/service/ReportGenerationService.java
- src/main/java/com/mr/domain/analysis/factory/AnalysisRequestFactory.java
Comment on lines
+128
to
+155
| @Test | ||
| void generate_fallsBackWhenGeminiReportIsStructurallyValidButTooShort() { | ||
| given(geminiClient.generateReport( | ||
| org.mockito.ArgumentMatchers.anyString(), | ||
| org.mockito.ArgumentMatchers.anyString() | ||
| )).willReturn(new GeminiGenerationResult( | ||
| """ | ||
| # 연주 분석 리포트 | ||
| ## 총평 | ||
| 짧은 총평 | ||
| ## 잘한 점 | ||
| 짧은 강점 | ||
| ## 진행 맥락 | ||
| 짧은 맥락 | ||
| ## 개선 제안 | ||
| 짧은 제안 | ||
| ## 점수 요약 | ||
| 80점 | ||
| """, | ||
| 100, 50, 150, false | ||
| )); | ||
|
|
||
| GeneratedAnalysisReport report = service.generate(result); | ||
|
|
||
| assertThat(report.generationType()).isEqualTo(ReportGenerationType.RULE_BASED); | ||
| assertThat(report.llmCall().status()) | ||
| .isEqualTo(com.mr.domain.mentor.entity.enums.LlmCallStatus.FAILED); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: validateMarkdownStructure 구현에서 섹션별 길이 검사가 있는지 확인합니다.
ast-grep run --pattern 'private void validateMarkdownStructure($_) { $$$ }' --lang java src/main/java/com/mr/domain/analysis/service/ReportGenerationService.javaRepository: Musereview/BE
Length of output: 2146
🏁 Script executed:
#!/bin/bash
# Inspect the relevant service methods and test expectations without running repository code.
sed -n '1,220p' src/main/java/com/mr/domain/analysis/service/ReportGenerationService.java
printf '\n--- test excerpt ---\n'
sed -n '110,170p' src/test/java/com/mr/domain/analysis/service/ReportGenerationServiceTest.javaRepository: Musereview/BE
Length of output: 9523
🏁 Script executed:
#!/bin/bash
# Calculate character counts and demonstrate parse behavior for the test fixture and current logic.
python3 - <<'PY'
fixture = """# 연주 분석 리포트
## 총평
짧은 총평
## 잘한 점
짧은 강점
## 진행 맥락
짧은 맥락
## 개선 제안
짧은 제안
## 점수 요약
80점
"""
print("fixture_chars_without_newlines=", len("".join(fixture.splitlines())))
print("fixture_unicode_chars=", len(fixture))
print("fixture_content_between_headings:")
lines = [line.rstrip() for line in fixture.splitlines()]
for i, h in enumerate(["## 총평", "## 잘한 점", "## 진행 맥락", "## 개선 제안", "## 점수 요약"]):
try:
idx = lines.index(h)
next_h, rest = lines[idx+1], lines[idx+2:]
text = ["", next_h][0]
j = rest.index(next_h) if next_h in rest else len(rest)
print(h, [line.strip() for line in rest[:j]], "{", len("".join(rest[:j].split())), "}")
except Exception as e:
print(h, "error", e)
PYRepository: Musereview/BE
Length of output: 512
🏁 Script executed:
#!/bin/bash
# Demonstrate current parse behavior and character-length gate for the test fixture.
python3 - <<'PY'
fixture = """# 연주 분석 리포트
## 총평
짧은 총평
## 잘한 점
짧은 강점
## 진행 맥락
짧은 맥락
## 개선 제안
짧은 제안
## 점수 요약
80점
"""
print("fixture_chars_without_newlines=", len("".join(fixture.splitlines())))
print("fixture_total_chars=", len(fixture))
lines = [line.rstrip() for line in fixture.splitlines()]
for heading in ["# 연주 분석 리포트", "## 총평", "## 잘한 점", "## 진행 맥락", "## 개선 제안", "## 점수 요약"]:
try:
idx = lines.index(heading)
print(f"{heading}: line index {idx}")
except ValueError:
print(f"{heading}: missing")
next_required = ["## 총평", "## 잘한 점", "## 진행 맥락", "## 개선 제안", "## 점수 요약"]
remaining = lines[1:]
for heading in next_required:
idx = remaining.index(heading)
remaining = remaining[idx+1:]
PYRepository: Musereview/BE
Length of output: 347
validateMarkdownStructure에는 섹션별 최소 분량 검증이 빠져 있습니다.
헤딩이 있고 전체 길이만 검사하므로, 현재 테스트의 짧은 각 섹션 응답은 LLM 리포트로 바로 반환될 수 있습니다. generate_fallsBackWhenGeminiReportIsStructurallyValidButTooShort가 기대하는 폴백 동작을 보장하려면 validateMarkdownStructure에서 각 섹션 콘텐츠 길이도 검증하세요. 좋은 테스트 케이스 선택인데, 기대 동작과 구현을 함께 맞추는 게 좋습니다.
🤖 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/mr/domain/analysis/service/ReportGenerationServiceTest.java`
around lines 128 - 155, Update validateMarkdownStructure to validate the content
length of every required Markdown section, not only heading presence and total
report length. Enforce the existing or intended minimum section-length rule so
the short sections used by
generate_fallsBackWhenGeminiReportIsStructurallyValidButTooShort are rejected
and generate returns the RULE_BASED fallback with a FAILED LlmCallStatus.
p1001q
approved these changes
Jul 31, 2026
p1001q
left a comment
p1001q
left a comment
Contributor
There was a problem hiding this comment.
리뷰 잘 봤습니다! stale 복구 시 이중완료 막는 부분 타임스탬프 펜싱으로 처리하신 거 좋네요, 코드래빗 지적보다 더 탄탄하게 짜신 것 같아요. 통계 이벤트/Gemini fallback/타임아웃 처리도 다 확인했는데 문제 없어 보입니다. 승인할게용!
(참고로
Analysis.java의LocalDateTime.now()쓰는 부분들, 통계 쪽에서 이미 만들어두신Clock빈으로 바꾸면 테스트 시간 고정하기 편해질 것 같은데 이건 급한 건 아니에요.)
| } | ||
| if (analysisRepository.existsByPlayingIdAndStatusIn( | ||
| playing.getId(), | ||
| List.of(AnalysisStatus.PENDING, AnalysisStatus.PROCESSING) |
Contributor
There was a problem hiding this comment.
P2: 동일 연주에 대한 재분석 허용 정책에 대한 질문
현재 PENDING, PROCESSING 상태의 분석만 중복 요청으로 차단하고 있는데, 기존 분석이 COMPLETED 또는 FAILED 상태라면 동일한 playingId로 분석을 계속 새로 생성할 수 있지 않나요??
완료된 연주도 재분석을 허용하는 것이 의도된 정책인지 궁금합니다!
재분석을 허용한다면 기존 분석 결과를 이력으로 모두 유지할지, 조회 화면에서는 어떤 분석을 대표 결과로 사용할지도 함께 정해두면 좋을 것 같아요
Contributor
Author
There was a problem hiding this comment.
안녕하세요! COMPLETED 상태는 막아야 할 것 같네용.
다만 제가 명세한 API 중에서는 FAILED 상태인 분석만 analysisId로 재분석 요청을 할 수 있게 하는 재분석 API가 있었어서, FAILED는 막지 않아야 할 것 같습니다.
정책으로 결정된 부분이 없어서, 프론트 쪽의 재분석 요청에 대한 편의를 위해 준비한 API였는데요. 이 부분은 프론트 쪽 담당자인 다현님과 더 논의해 보고 알려드려도 괜찮을까요?
on1yoneprivate
approved these changes
Jul 31, 2026
on1yoneprivate
left a comment
on1yoneprivate
left a comment
Contributor
There was a problem hiding this comment.
수고하셨습니다! 코멘트 참고해 주세요
This was referenced Aug 1, 2026
Merged
This was referenced Aug 6, 2026
Merged
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
3 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.
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
🔥 리뷰 요청 사항
✅ 체크리스트
📎 참고 사항
PlayingCompletedEvent를 발행해 통계 집계 리스너와 연결해 주세요. (개인적으로 연락 드렸으나 해당 코드 확인 후에도 감이 안 잡히시거나 제가 설명을 이상하게 했다면 언제든... 연락 부탁드려용 ///)analysis.processing_started_at컬럼 추가가 필요합니다. (연락 드려서 해결 완)Summary by CodeRabbit
새로운 기능
버그 수정