[feat] 대화 저장 + LLM 댓글 생성 API 통합 - #48
Conversation
답장 대상이 같은 채팅방에 속하는지만 검증하고 캐릭터 메시지인지는 확인하지 않았다. 저장과 생성이 분리되어 있을 때는 이어지는 생성 호출에서 INVALID_COMMENT_TARGET으로 걸러졌지만, 저장 직후 같은 요청에서 생성까지 이어지는 흐름에서는 저장은 이미 커밋된 채 응답만 400을 반환해 "실패로 보이는데 실제로는 저장된" 상태가 될 수 있었다. 저장 시점 (validateReplyTarget)에 미리 막아 이 상태 자체가 생기지 않게 한다.
LLM 호출(재시도 최대 2회)을 저장과 한 요청으로 묶으면 nginx proxy_read_timeout(120s)이 하드 리밋이 되는데, 기존엔 호출별 타임아웃이 없었다. GeminiProperties.requestTimeout(PT30S)을 GenerateContentConfig의 HttpOptions에 적용해 최악의 경우에도 재시도 포함 ~60초 안에 들어오게 한다.
- POST /messages가 저장 직후 같은 요청 안에서 CommentGenerationService의 댓글(일기)·재응답(답글) 생성까지 동기로 처리해 함께 반환한다(폴링 제거) - 새로 저장된 메시지 행은 이번 요청에서 최초로 CAS 선점을 시도하므로 GENERATING은 나오지 않고 DONE/FAILED만 나온다 - LLM 생성이 재시도까지 실패해도 저장은 유지되고 commentStatus=FAILED로 구분해서 응답한다 - 기존 폴링 엔드포인트(/messages/comments류)는 삭제하지 않고 FAILED 이후 수동 재시도용으로 문서만 좁혔다 - 크래시 복구는 기존 PendingCommentCleanupScheduler가 그대로 커버해 별도 작업이 필요 없었다
…m-diary # Conflicts: # src/main/kotlin/com/nexters/gamss/conversation/controller/ConversationController.kt # src/test/kotlin/com/nexters/gamss/conversation/service/ConversationServiceTest.kt
POST /messages가 저장과 함께 댓글·답글 생성을 동기로 처리하게 되면서
/messages/comments, /messages/comments/{messageId}는 실사용 경로가
아니게 됐다. 실패 후 수동 재시도용으로 문서만 좁혀 남겨뒀었는데, 쓰이지
않는 API 표면을 유지할 이유가 없어 엔드포인트와 전용 DTO
(GenerateCommentsRequest, CommentGenerationResponse,
ReplyGenerationResponse)를 완전히 제거한다. 삭제된 채팅방에서 생성이
막히는지 검증하던 관련 통합 테스트 2건도 함께 정리한다(같은 보호 로직은
CommentGenerationServiceTest에서 단위 테스트로 계속 커버).
This reverts commit 596c3e9.
Test Results276 tests +21 276 ✅ +21 1m 20s ⏱️ +17s Results for commit f58ceb9. ± Comparison against base commit 1a55cdf. This pull request removes 2 and adds 23 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Test Coverage
|
Walkthrough메시지 저장 API가 저장된 메시지와 동기 댓글·답글 생성 결과를 함께 반환하도록 변경되었습니다. 생성 실패 시 저장 결과를 유지하고 실패 상태를 반환하며, 답장 대상 검증과 Gemini 요청 타임아웃 및 재시도 예산 검증이 추가되었습니다. Changes대화 저장 및 생성 흐름
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ConversationController
participant ConversationService
participant CommentGenerationService
participant GeminiCommentGenerator
Client->>ConversationController: saveMessage 요청
ConversationController->>ConversationService: 메시지 저장
ConversationService-->>ConversationController: 저장된 Message
ConversationController->>CommentGenerationService: generateFor 호출
CommentGenerationService->>GeminiCommentGenerator: 댓글 또는 답글 생성 요청
GeminiCommentGenerator-->>CommentGenerationService: 생성 결과 또는 실패
CommentGenerationService-->>ConversationController: GenerationResult 반환
ConversationController-->>Client: SaveMessageResponse 반환
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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/kotlin/com/nexters/gamss/conversation/controller/ConversationController.kt`:
- Around line 69-74: 동기 저장 흐름이 CAS 기반 생성 결과의 GENERATING 상태를 그대로 응답하지 않도록 수정하세요.
ConversationController의 저장 분기에서 DONE 또는 FAILED까지 bounded wait/retrieve로 해소하는 동기
서비스 경로를 사용하고, SaveMessageResponse.from의 입력도 종단 결과로 제한하거나 비종단 결과가 전달되지 않는 불변식을
검증하도록 보장하세요.
src/main/kotlin/com/nexters/gamss/conversation/controller/ConversationController.kt
69-74와
src/main/kotlin/com/nexters/gamss/conversation/controller/dto/SaveMessageResponse.kt
29-42를 모두 반영하세요.
In `@src/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.kt`:
- Around line 8-12: GeminiProperties의 requestTimeout을 0보다 크고 Int.MAX_VALUE 밀리초
이하로 생성 시 검증하고, 검증된 requestTimeoutMillis를 노출해 SDK의 HttpOptions.Builder.timeout에
안전하게 전달하세요. GeminiCommentGenerator의 requestTimeout 변환 지점도 동일한 검증된 정수 값을 재사용하도록
수정하세요.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: eaa7498a-16e0-4617-a194-0141841794a5
📒 Files selected for processing (13)
src/main/kotlin/com/nexters/gamss/conversation/controller/ConversationController.ktsrc/main/kotlin/com/nexters/gamss/conversation/controller/dto/CommentGenerationStatus.ktsrc/main/kotlin/com/nexters/gamss/conversation/controller/dto/SaveMessageResponse.ktsrc/main/kotlin/com/nexters/gamss/conversation/service/ConversationService.ktsrc/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.ktsrc/main/kotlin/com/nexters/gamss/llm/generation/GeminiCommentGenerator.ktsrc/main/resources/application.ymlsrc/test/kotlin/com/nexters/gamss/conversation/controller/ConversationControllerIntegrationTest.ktsrc/test/kotlin/com/nexters/gamss/conversation/controller/ConversationControllerTest.ktsrc/test/kotlin/com/nexters/gamss/conversation/service/ConversationServiceTest.ktsrc/test/kotlin/com/nexters/gamss/llm/settings/LlmSettingsServiceTest.ktsrc/test/kotlin/com/nexters/gamss/support/FakeCommentGeneratorConfig.ktsrc/test/resources/application.yml
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/test/kotlin/com/nexters/gamss/conversation/controller/ConversationControllerTest.kt`:
- Around line 122-132: Update the reply-generation test around
controller.saveMessage to cover ReplyGenerationOutcome.FAILED, asserting the
saved message is returned with FAILED status, an empty comments collection, and
usedTokens == null. Keep the existing exception assertion for GENERATING
unchanged and add this as a separate reply failure-path test.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 7cee8c6c-83ad-49ce-ad4f-e94731b11c07
📒 Files selected for processing (2)
src/main/kotlin/com/nexters/gamss/conversation/controller/dto/SaveMessageResponse.ktsrc/test/kotlin/com/nexters/gamss/conversation/controller/ConversationControllerTest.kt
04fb598 to
d26d23d
Compare
theminjunchoi
left a comment
There was a problem hiding this comment.
수고하셨습니다! 코드 잘 작성해주셨네용
아래 리뷰 한 번씩 봐주세요!
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.kt`:
- Around line 47-52: Update the reply path in generateFor and
generateReplyInternal to validate conversation ownership before loading the
referenced character and diary messages. Scope the repliesToMessageId lookup,
including its rootMessageId, to the requesting owner and the current
conversation (or verify both after lookup), and reject mismatches before
generation.
In `@src/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.kt`:
- Around line 21-23: Update the timeout validation in GeminiProperties to
account for the complete LLM retry budget, including the configured maximum
retry attempts and backoff, rather than checking only Int.MAX_VALUE
milliseconds. Define or reuse maintainable shared constants for retry count,
backoff, and the nginx-compatible total budget, and ensure requestTimeout
multiplied across all attempts remains within that budget while preserving the
existing upper-bound validation.
- Around line 14-21: The timeout validation in GeminiProperties must enforce the
integer-millisecond contract used by buildConfig: replace the Duration.ZERO
check with validation that requestTimeout.toMillis() is within 1..Int.MAX_VALUE,
while retaining the upper-bound validation behavior. In
src/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.kt lines 14-21,
update the init validation; in
src/test/kotlin/com/nexters/gamss/llm/config/GeminiPropertiesTest.kt lines
16-21, add a boundary test covering a positive sub-millisecond Duration and its
rejection.
In `@src/test/kotlin/com/nexters/gamss/llm/config/GeminiPropertiesTest.kt`:
- Around line 16-21: GeminiProperties의 requestTimeout 검증 및 밀리초 변환을 조정해 1ms 미만의
양수 Duration이 0으로 전달되지 않게 하세요. 해당 값을 0ms와 동일하게 거부하거나 명확한 반올림 규칙으로 정수화하고, 선택한 기준을
GeminiPropertiesTest에 테스트로 고정하세요.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b7ccb577-37fe-41c3-873a-bd8235b127b2
📒 Files selected for processing (10)
src/main/kotlin/com/nexters/gamss/conversation/controller/ConversationController.ktsrc/main/kotlin/com/nexters/gamss/conversation/controller/dto/SaveMessageResponse.ktsrc/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationResult.ktsrc/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.ktsrc/main/kotlin/com/nexters/gamss/conversation/service/GenerationResult.ktsrc/main/kotlin/com/nexters/gamss/conversation/service/ReplyGenerationResult.ktsrc/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.ktsrc/test/kotlin/com/nexters/gamss/conversation/controller/ConversationControllerTest.ktsrc/test/kotlin/com/nexters/gamss/conversation/service/CommentGenerationServiceTest.ktsrc/test/kotlin/com/nexters/gamss/llm/config/GeminiPropertiesTest.kt
💤 Files with no reviewable changes (2)
- src/main/kotlin/com/nexters/gamss/conversation/service/ReplyGenerationResult.kt
- src/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationResult.kt
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/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.kt (1)
48-53: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift통합 저장 경로에서
GENERATING응답을 차단하세요.
generateFor도 기존 CAS 선점 로직을 재사용하므로, 다른 재시도 요청이 먼저PENDING을 선점하면currentStatusResult/currentReplyStatusResult를 통해GENERATING을 반환할 수 있습니다. 이는 동기 통합 흐름에서GENERATING상태가 발생하지 않아야 한다는 계약을 깨뜨립니다.통합 경로는 선점 충돌 시 완료 상태를 대기·조회해
DONE/FAILED만 반환하거나, 저장 직후 생성 권한을 별도로 원자적으로 확보하도록 분리하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.kt` around lines 48 - 53, Update generateFor and its generateCommentsInternal/generateReplyInternal CAS flow so the synchronous integrated path never returns GENERATING after a claim conflict. When another retry has claimed PENDING, wait for and reload the completed result, returning only DONE or FAILED; alternatively, atomically acquire generation ownership separately after persistence while preserving the existing retry 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.
Outside diff comments:
In
`@src/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.kt`:
- Around line 48-53: Update generateFor and its
generateCommentsInternal/generateReplyInternal CAS flow so the synchronous
integrated path never returns GENERATING after a claim conflict. When another
retry has claimed PENDING, wait for and reload the completed result, returning
only DONE or FAILED; alternatively, atomically acquire generation ownership
separately after persistence while preserving the existing retry behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8e03dddd-290c-4b70-8d4c-283f4aceb543
📒 Files selected for processing (6)
src/main/kotlin/com/nexters/gamss/conversation/service/CommentGenerationService.ktsrc/main/kotlin/com/nexters/gamss/llm/config/GeminiProperties.ktsrc/main/kotlin/com/nexters/gamss/llm/generation/GeminiCommentGenerator.ktsrc/main/kotlin/com/nexters/gamss/llm/generation/LlmRetryPolicy.ktsrc/test/kotlin/com/nexters/gamss/conversation/service/CommentGenerationServiceTest.ktsrc/test/kotlin/com/nexters/gamss/llm/config/GeminiPropertiesTest.kt
🔗 연관 이슈
📌 개요
일기 저장(
POST /messages) → 캐릭터 댓글 생성, 답글 저장 → 캐릭터 재응답 생성으로 나뉘어 있던 두 흐름을 각각 저장+생성 동기 1콜로 합쳤습니다. 폴링 없이 한 번의 요청으로 저장 결과와 LLM 생성 결과를 함께 받습니다.🔧 주요 변경사항
저장+생성 통합
POST /messages가 저장 직후 같은 요청 안에서CommentGenerationService.generateComments(일기) 또는generateReplyComment(답글)를 동기로 호출해 함께 반환 — 새 엔드포인트를 만들지 않고 기존 저장 엔드포인트 하나를 확장SaveMessageResponse(신규) —message+commentStatus(DONE/FAILED만 발생) +comments+usedTokensCommentGenerationService의 CAS 선점 → LLM 재시도(최대 2회) → 저장 로직은 그대로 재사용(변경 없음)Gemini 호출 타임아웃
GeminiProperties.requestTimeout(기본PT30S)을GenerateContentConfig의HttpOptions에 적용proxy_read_timeout(120s)에 걸릴 위험이 있어 추가답장 대상 검증 강화 (리뷰 중 발견한 버그 수정)
ConversationService.validateReplyTarget에 답장 대상이 캐릭터 메시지인지 확인하는 검증 추가🌐 API · DB 영향
POST /api/conversations/messages응답 스키마 변경 — 기존MessageResponse평평한 구조에서{ message, commentStatus, comments, usedTokens }로 변경. 그 외 신규 엔드포인트 없음💬 리뷰 포인트
1. 새 엔드포인트 대신 기존
POST /messages를 확장한 이유 — 이슈 DoD가 "저장 요청 한 번으로 저장+생성 결과를 함께 받는다"였고, 실제로 두 폴링 엔드포인트(/messages/comments류)를 그대로 남겨 "실패 후 수동 재시도" 용도로만 쓰이게 하는 게 API 표면을 늘리지 않으면서 이슈 의도에 가장 맞다고 판단했습니다. 다만 응답 스키마가 바뀌는 breaking change라 클라와의 연동 타이밍 맞춰야 할 것 같습니다.2. 답장 대상 검증 버그 — 원래 답장 대상이 같은 채팅방에 있는지만 확인하고 캐릭터 메시지인지는 안 봐서, 일기 등 다른 메시지에도 답장을 저장할 수 있었습니다. 저장과 생성이 분리돼 있을 땐 이어지는 생성 호출에서 걸러졌지만, 한 요청으로 합치면서 "저장은 커밋됐는데 응답은 400" 상태가 생길 뻔해서 저장 시점에 미리 막았습니다.
Summary by CodeRabbit
commentStatus, 생성 댓글/답글 목록, 사용 토큰(해당 시)이 포함됩니다.