변경된 요구사항에 따른 톡픽 본문 요약 기능 구현 #829
Conversation
Hanjaemo
added
⚙️ chore
개요Walkthrough이 풀 리퀘스트는 톡픽 요약 기능을 전면적으로 재설계했습니다. Changes
Assessment against linked issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
# Conflicts: # src/main/java/balancetalk/talkpick/domain/TalkPick.java
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java (2)
57-82: ChatClient 활용 시 예외 처리 강화 권장
getSummary 메서드에서 ChatClient를 호출할 때 네트워크 장애, 응답 포맷 이상 등의 예외가 발생할 가능성이 있습니다. 필요한 경우 별도의 예외 처리를 통해 사용자에게 적절한 오류 정보를 전달하도록 방어 로직을 보강해 보세요.
98-103: Recover 메서드에서 민감 정보 로그 유출에 유의
에러 로그를 남길 때 스택 트레이스 전체가 노출되어 민감한 정보가 포함될 수 있습니다. 개발 단계에서는 유용하지만 운영 환경에서는 민감도가 높은 정보를 마스크 처리하는 방안을 검토해 주세요.src/main/java/balancetalk/talkpick/application/TalkPickService.java (1)
90-90: 톡픽 수정 후 요약 재실행
updateTalkPick 이후에 별도 로직이 필요한지 확인이 중요합니다. 사용자가 번복해서 여러 차례 수정을 시도하면 불필요한 요약 요청이 누적될 수 있으므로, 필요 시 중복 요약 호출 방지 메커니즘을 고려해 보세요.src/main/java/balancetalk/talkpick/domain/TalkPick.java (2)
44-44: MIN_CONTENT_LENGTH_FOR_SUMMARY 상수 유지보수
본문 최소 길이를 200으로 고정했는데, 추후 정책 변경 가능성이 있다면 설정 파일이나 별도 환경 변수로 분리하여 유연하게 관리하는 것이 좋습니다.
159-161: 메서드명 명확성
updateSummaryStatus는 업무적으로 중요한 상태 전이를 처리하므로, updateSummaryStatus나 setSummaryStatus 대신 좀 더 의미 있는 메서드명을 고민해 볼 수 있습니다. 예: markSummaryAs()src/main/java/balancetalk/talkpick/dto/TalkPickDto.java (1)
118-119: 상세 응답에서 summaryStatus 노출
사용자나 클라이언트가 요약 상태를 확인하기 용이해집니다. 보안적으로 문제가 될 만한 정보는 아니지만, 상태가 노출됨으로써 사용자가 중간 상태(예: PENDING, FAIL 등)를 어떻게 인식하고 조치해야 하는지 가이드가 필요할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/balancetalk/talkpick/application/SummaryContentService.java(0 hunks)src/main/java/balancetalk/talkpick/application/TalkPickService.java(3 hunks)src/main/java/balancetalk/talkpick/domain/SummaryStatus.java(1 hunks)src/main/java/balancetalk/talkpick/domain/TalkPick.java(4 hunks)src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java(1 hunks)src/main/java/balancetalk/talkpick/dto/TalkPickDto.java(5 hunks)src/main/java/balancetalk/talkpick/dto/fields/BaseTalkPickFields.java(3 hunks)src/main/java/balancetalk/talkpick/presentation/SummaryContentController.java(0 hunks)src/main/resources/config(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/balancetalk/talkpick/presentation/SummaryContentController.java
- src/main/java/balancetalk/talkpick/application/SummaryContentService.java
✅ Files skipped from review due to trivial changes (2)
- src/main/resources/config
- src/main/java/balancetalk/talkpick/domain/SummaryStatus.java
🧰 Additional context used
📓 Learnings (1)
src/main/java/balancetalk/talkpick/application/TalkPickService.java (1)
Learnt from: Hanjaemo
PR: JECT-Study/PICK-O-Server#708
File: src/main/java/balancetalk/vote/application/VoteTalkPickService.java:55-63
Timestamp: 2024-11-12T17:26:42.341Z
Learning: 일부 API에서는 비회원 접근을 허용해야 하므로 Java 컨트롤러와 서비스에서 `GuestOrApiMember`를 계속 사용해야 한다.
🔇 Additional comments (7)
src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java (1)
39-42: 짧은 본문 처리 로직이 명확합니다.
본문 길이가 최소 길이에 미치지 못할 때 요약이 불필요함을 즉시 반환하는 로직이 깔끔합니다.
src/main/java/balancetalk/talkpick/dto/fields/BaseTalkPickFields.java (2)
32-32: null 필드 직렬화 제외는 적절합니다.
@JsonInclude(JsonInclude.Include.NON_NULL)을 통해 null 필드를 자동 제외함으로써 불필요한 정보 노출을 막을 수 있습니다.
47-55: sourceUrl 없이도 객체를 생성할 수 있어 유연성이 향상됩니다.
from 메서드의 오버로드로 sourceUrl 대신 기본 필드만 사용하는 경우를 지원하므로, 다양한 요청 상황을 잘 처리할 수 있습니다.
src/main/java/balancetalk/talkpick/application/TalkPickService.java (2)
46-47: 톡픽 생성 직후 요약 처리
createTalkPick 메서드 내에서, 저장이 완료된 TalkPick에 대해 즉시 비동기 요약을 수행합니다. 저장 시점 기준으로 필드가 정상적으로 확정되었는지(특히 DB에서 자동 증가 키 사용 시) 점검해 주세요.
34-34: Summarizer DI 주입 확인
TalkPickSummarizer가 정상적으로 빈 등록이 되었는지 확인이 필요합니다. IDE나 테스트에서 의존성 주입 오류가 없는지 검증해 주세요.
✅ Verification successful
Let me search for the bean registration and related configuration files for TalkPickSummarizer.
Let me check for Spring annotations on the TalkPickSummarizer class and its constructor usage.
의존성 주입 정상 확인 완료
TalkPickSummarizer 클래스에 @Component 어노테이션이 정상적으로 적용되어 있고, TalkPickService에서 @RequiredArgsConstructor를 통해 final 필드로 의존성 주입이 올바르게 구성되어 있습니다. 빈 등록과 의존성 주입에 문제가 없습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for TalkPickSummarizer class definition and configuration
ast-grep --pattern 'class TalkPickSummarizer'
# Search for @Component, @Service, or @Configuration annotations related to TalkPickSummarizer
rg -l "@(Component|Service|Configuration).*TalkPickSummarizer"
# Search for bean configuration in XML or Java config files
fd -e xml -e java | xargs rg "TalkPickSummarizer"
Length of output: 510
Script:
#!/bin/bash
# Check TalkPickSummarizer class content for Spring annotations
cat src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java
# Check for constructor injection in TalkPickService
cat src/main/java/balancetalk/talkpick/application/TalkPickService.java
Length of output: 8381
src/main/java/balancetalk/talkpick/dto/TalkPickDto.java (2)
51-51: 생성 시 summaryStatus 설정
CreateTalkPickRequest에서 summaryStatus를 PENDING으로 초기화하는 로직은 적절합니다. 다만, PENDING 상태의 TalkPick이 지나치게 많아지지 않도록 후속 작업(언제 FAIL, NOT_REQUIRED 등으로 업데이트되는지) 관리가 필요해 보입니다.
172-172: from 메서드와 동일하게 summaryStatus를 매핑
이미 TalkPick 엔티티에 summaryStatus가 세팅되어 있으므로, TalkPickDetailResponse 생성 시에도 일관성 있게 반영하고 있어 좋습니다.
| @Retryable(backoff = @Backoff(delay = 1000), maxAttempts = 3) | ||
| @Transactional | ||
| public void summary(TalkPick talkPick) { |
There was a problem hiding this comment.
💡 Codebase verification
@async와 @transactional 어노테이션의 동시 사용이 확인되었습니다.
현재 코드에서 다음과 같은 잠재적인 문제가 있을 수 있습니다:
TalkPickSummarizer.summary()메서드에 @async와 @transactional이 함께 사용되고 있어, 비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행됩니다.- 부모 트랜잭션과의 전파나 롤백이 예상대로 동작하지 않을 수 있습니다.
권장 사항:
- 비동기 메서드와 트랜잭션 메서드를 분리하여 구현하는 것을 고려해보세요.
- 트랜잭션 범위를 비동기 메서드 내부의 별도 메서드로 이동하는 것이 안전합니다.
🔗 Analysis chain
비동기와 트랜잭션 동시 사용에 유의하세요.
요약 메서드에 @async와 @transactional이 함께 사용될 경우, 예상치 못한 트랜잭션 범위 문제가 발생할 수 있습니다. 트랜잭션 롤백 범위나 프로그래시(proxy) 동작을 다시 한 번 점검해 보시는 것을 권장합니다.
Let me gather information about the usage of @async annotation in this class and related configurations.
Let me check the actual implementation and configuration of async and transaction handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check if @Async is used in TalkPickSummarizer
ast-grep --pattern 'class TalkPickSummarizer {
$$$
@Async
$$$
}'
# Search for @Async configuration or AsyncConfig files
fd -e java -e xml -e properties -e yml | xargs rg -l "(@EnableAsync|@Async|AsyncConfig)"
# Check for any async-related configuration in application properties
fd "application.*properties" -e yml -e yaml | xargs cat
# Look for transaction configuration
fd -e java | xargs rg "@EnableTransactionManagement|@TransactionConfiguration"
Length of output: 492
Script:
#!/bin/bash
# Check AsyncConfig implementation
cat src/main/java/balancetalk/global/config/AsyncConfig.java
# Check TalkPickSummarizer implementation for @Async usage
cat src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java
# Check TalkPickFileHandler implementation
cat src/main/java/balancetalk/talkpick/domain/TalkPickFileHandler.java
Length of output: 7755
There was a problem hiding this comment.
비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행되는 것을 인지하고 작성했습니다.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@coderabbitai 비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행되는 것을 인지하고 작성했습니다.
| public boolean hasShortContent() { | ||
| return content.length() < MIN_CONTENT_LENGTH_FOR_SUMMARY; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
hasShortContent 메서드의 간소화
content가 null인 경우의 예외 처리가 필요할 수 있습니다. 외부나 다른 서비스에서 content를 누락하여 저장할 가능성이 있다면 방어 로직을 추가해주세요.
| @NotNull | ||
| private SummaryStatus summaryStatus; |
There was a problem hiding this comment.
💡 Codebase verification
summaryStatus 필드의 초기화가 누락되었습니다
TalkPick 클래스의 코드를 검토한 결과:
@Builder패턴을 사용하고 있으나summaryStatus필드의 기본값이 설정되어 있지 않습니다hasShortContent()메서드와updateSummaryStatus()메서드가 있어 요약 상태를 관리하고 있으나, 객체 생성 시점의 초기화는 보장되지 않습니다
다음 사항들을 수정해주세요:
@Builder.Default를 사용하여summaryStatus필드에 기본값PENDING또는 적절한 초기 상태를 지정하거나- 생성자나 빌더에서
hasShortContent()를 확인하여 적절한 초기 상태값을 설정하도록 구현해주세요
🔗 Analysis chain
summaryStatus 필드@NotNull 적용
summaryStatus는 반드시 null이 아니어야 하므로, TalkPick 생성 시점에 적절한 기본값으로 초기화하는 로직을 점검해 주세요.
Let me gather information about the initialization of summaryStatus field in TalkPick class.
Let me try a different approach to search for the relevant code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for TalkPick class implementation to check constructor and initialization
ast-grep --pattern 'class TalkPick {
$$$
}'
# Also search for SummaryStatus enum definition
ast-grep --pattern 'enum SummaryStatus {
$$$
}'
Length of output: 99
Script:
#!/bin/bash
# Search for TalkPick.java file content
fd -e java "TalkPick.java" --exec cat {}
# Search for SummaryStatus.java file content
fd -e java "SummaryStatus.java" --exec cat {}
Length of output: 7344
| @@ -82,6 +84,7 @@ public TalkPick toEntity(Member member) { | |||
| .optionA(baseFields.getOptionA()) | |||
| .optionB(baseFields.getOptionB()) | |||
| .sourceUrl(baseFields.getSourceUrl()) | |||
| .summaryStatus(SummaryStatus.PENDING) | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the code analysis, I can now provide a final response:
요약 상태 초기화 문제 확인됨
톡픽 수정 시 update() 메서드에서 summaryStatus를 변경하지 않고 있으나, TalkPickDto에서 항상 PENDING 상태로 설정하고 있습니다. 이는 다음과 같은 문제를 야기할 수 있습니다:
- 이미 요약이 완료(
SUCCESS)되거나 실패(FAIL)한 톡픽의 내용을 수정할 때 불필요하게 다시 요약 프로세스가 시작될 수 있음 - 요약이 불필요(
NOT_REQUIRED)하다고 판단된 톡픽도 다시PENDING상태가 되어 혼란을 야기할 수 있음
수정 제안:
TalkPickDto에서 기존 톡픽의summaryStatus를 유지하도록 수정- 내용이 변경된 경우에만 선택적으로
summaryStatus를PENDING으로 변경하는 로직 추가
🔗 Analysis chain
수정 시에도 summaryStatus를 PENDING으로
UpdateTalkPickRequest에서도 요약 상태가 계속 PENDING으로 되돌아갈 수 있으니, 이미 실패하거나 요약 완료된 톡픽에 대해 다시 PENDING으로 세팅되는 것이 의도된 것인지 사용 시나리오를 재검토해보세요.
Let me gather more information about the usage of SummaryStatus and the update logic.
Let me check the SummaryStatus enum and any logic related to status transitions.
Let me check the update logic in TalkPick entity and how it handles the summary status during updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find files containing SummaryStatus
rg "SummaryStatus" -l
# Search for UpdateTalkPickRequest class definition and usage
ast-grep --pattern 'class UpdateTalkPickRequest {
$$$
}'
# Search for methods that handle talk pick updates
rg "update.*TalkPick" -A 5
# Look for any status transition logic
rg "summaryStatus" -A 3
Length of output: 8089
Script:
#!/bin/bash
# Check SummaryStatus enum definition
cat src/main/java/balancetalk/talkpick/domain/SummaryStatus.java
# Look for TalkPickSummarizer implementation
cat src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java
# Search for any update logic that changes summary status
rg "updateSummaryStatus" -B 5 -A 5
Length of output: 8219
Script:
#!/bin/bash
# Check TalkPick entity update method
rg "update.*\(" src/main/java/balancetalk/talkpick/domain/TalkPick.java -A 10
# Check UpdateTalkPickRequest implementation
rg -g "UpdateTalkPickRequest.java" "class|toEntity"
Length of output: 1335
There was a problem hiding this comment.
톡픽이 생성된 직후에는 PENDING으로 설정되고, 요약이 수행되는 시점에 NOT_REQUIRED, SUCCESS, 혹은 FAIL로 설정되는 방식이에요.
There was a problem hiding this comment.
@Hanjaemo 감사합니다. 말씀해주신 것처럼 톡픽이 생성될 때 PENDING 상태로 시작하고, 이후 실제 요약 작업에서 NOT_REQUIRED, SUCCESS, 혹은 FAIL 상태로 변경되는 구조로 이해했습니다. 현재 구조라면, 톡픽 수정 시에도 새롭게 요약 과정을 진행하려는 의도라면 문제없이 동작하겠네요. 추가로 공유해주실 내용이 있으면 말씀 부탁드립니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java (3)
55-79: 비동기 요약 로직에서 예외 발생 시 복구 경로를 고려하세요.
@Retryable을 적용하여 자동 재시도를 수행하고 있으나, 예외 상황에서 트랜잭션 롤백 처리가 어떻게 이뤄지는지 충분히 검토하시기 바랍니다. 특히 재시도 실패 후에는TALK_PICK_SUMMARY_FAILED예외가 발생하므로, 로그 모니터링과 알림을 적절히 연동하면 좋겠습니다.
65-69: 중간 산출물 검증 로직 보강을 권장합니다.
getSummary(talkPick)메서드가null을 반환하는 경우에 대해, 로그 남기기 등 추가 진단 정보를 확보하는 방법을 고려해 보세요.
71-74: 요약 글자 수 검증 시 범위 정보 제공을 고려해 보세요.
단순히 예외만 던지는 것보다, 어느 정도 길이를 초과했는지에 대한 정보를 재시도 로직 또는 운영 로그에 함께 남기면, 문제 파악이 빨라질 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/balancetalk/global/exception/ErrorCode.java(1 hunks)src/main/java/balancetalk/talkpick/domain/TalkPick.java(6 hunks)src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/balancetalk/talkpick/domain/TalkPick.java
🔇 Additional comments (4)
src/main/java/balancetalk/global/exception/ErrorCode.java (1)
91-92: 에러 코드 식별이 더 명확해졌습니다.
TALK_PICK_SUMMARY_SIZE_IS_OVER와 TALK_PICK_SUMMARY_FAILED로 이름을 구체화함으로써 톡픽 요약 시나리오에서 발생하는 오류를 훨씬 명확하게 표현할 수 있게 되었습니다. 다만, 기존 오류 처리 로직과의 호환성 여부도 한 번 더 점검해 보시면 좋겠습니다.
src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java (3)
22-24: @async와 @transactional 동시 사용 시 주의가 필요합니다.
다른 스레드에서 트랜잭션이 독립적으로 동작할 수 있으므로, 트랜잭션 경계가 예상과 다르게 동작할 가능성을 유의하세요.
60-63: 콘텐츠 길이가 짧은 경우 요약 상태를 NOT_REQUIRED로 즉시 반환하는 로직이 유효합니다.
짧은 본문에 대해서는 불필요한 자원 소모를 줄일 수 있어, 적절한 방어적 프로그래밍으로 보입니다.
105-110: 에러 복구 로직에서의 예외 메시지 처리 방식을 재점검하세요.
ex.getMessage() 및 ex.getStackTrace() 출력이 민감 정보 유출 가능성을 높일 수 있으므로, 운영 환경에서는 로그 레벨 조정 혹은 예외 요약 정보만 노출하는 방안을 고려해보세요.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/balancetalk/global/exception/ErrorCode.java (1)
92-93: 에러 코드 추가가 적절하며, 메시지 개선을 제안드립니다.변경사항이 톡픽 요약 기능 구현에 잘 부합합니다. 다만 에러 메시지를 조금 더 구체적으로 개선하면 좋을 것 같습니다.
다음과 같이 에러 메시지를 더 상세하게 수정하는 것을 제안드립니다:
- TALK_PICK_SUMMARY_SIZE_IS_OVER(INTERNAL_SERVER_ERROR, "요약 내용의 길이가 적정 기준을 초과했습니다."), + TALK_PICK_SUMMARY_SIZE_IS_OVER(INTERNAL_SERVER_ERROR, "톡픽 요약 내용이 최대 길이(n자)를 초과했습니다."), - TALK_PICK_SUMMARY_FAILED(INTERNAL_SERVER_ERROR, "톡픽 요약에 실패했습니다."); + TALK_PICK_SUMMARY_FAILED(INTERNAL_SERVER_ERROR, "톡픽 요약 생성 중 오류가 발생했습니다. 잠시 후 다시 시도해 주세요.");이렇게 수정하면:
- 첫 번째 메시지에서 실제 길이 제한을 명시하여 사용자에게 더 명확한 정보를 제공합니다.
- 두 번째 메시지에서는 사용자에게 재시도 가능성을 안내하여 더 나은 사용자 경험을 제공합니다.
💡 작업 내용
💡 자세한 설명
톡픽 생성~요약 플로우
자세한 내용은 팀 노션을 참고해 주세요.
local 환경에 Spring AI 관련 로깅 추가
로컬 환경에 Spring AI 관련 로깅을 설정하기 위해 다음과 같이
org.springframework.ai.chat.client.advisor: DEBUG를 추가했습니다.톡픽 요약 상태값 추가
요약 상태값 관리를 위해 TALK_PICK 테이블에 SUMMARY_STATUS라는 컬럼을 추가했습니다.
각 상태값의 의미는 다음과 같습니다.
PENDING: 톡픽이 생성되거나 수정되어 요약을 수행중인 상태SUCCESS: 톡픽 요약이 완료된 상태FAIL: 톡픽 요약이 최초 1번, 3번의 재시도 끝에 최종적으로 실패한 상태NOT_REQUIRED: 톡픽의 본문 글자수가 200자 미만이라 요약을 제공하지 않는 상태재시도 처리
Spring Retry 라이브러리를 사용하여 톡픽 요약 실패 시 최대 3번까지 재시도하도록 구현했습니다.
아래는 3번의 재시도가 실패했을 때 최종적으로 실행할 코드입니다.
요약 프롬프트 지시문
가장 최근 업데이트된 지시문을 사용했습니다.
요약 프롬프트 지시문은 계속해서 디벨롭중입니다.
TypeReference
톡픽 기본 필드 DTO를 Map 컬렉션으로 변환할 때 제네릭 타입의 타입 정보를 유지하기 위해 TypeReference를 사용했습니다.
참고 자료
🚩 후속 작업
✅ 셀프 체크리스트
closes #826
Summary by CodeRabbit
새로운 기능
개선 사항
버그 수정