카드 표시값을 파생 조회로: 최신 기계 READY 우선 + start 시점 박제 - #858
Conversation
- 위시(목록·단건)·토너먼트 대기실(상세·아이템 단건)의 표시값을 포인터 소유에서 파생으로 전환한다. 규칙은 ItemDisplayService 단일 지점: 최신 기계(SERVER/SERVER_LLM) READY 는 절대 지지 않는다 / 포인터가 수기(MANUAL)이고 그보다 새로운 기계 READY 가 없으면 그 맥락만 수기값(놓은 사람의 의도 존중) / 진행 중 포인터는 그대로(등록·갱신 UX 신호) / 기계 READY 가 없으면 포인터 fallback(도입 전 데이터·수기 복구) - 같은 상품을 담은 다른 위시가 갱신해도 모두가 최신 값을 본다 - 포인터는 정체성 도달·수기 존중 판정의 표식으로 내려간다 - 토너먼트 start 를 "겨루는 값 확정" 순간으로: 파생 표시 버전을 포인터에 박제(repin)해 겨룬 값 = 진행·완료 화면 값 = 히스토리 값을 고정한다. 시작 후 화면·히스토리·CLONE 플레이는 파생 없이 박제 값을 읽는다(히스토리는 당시를 보는 것이 확정) - 배치 파생은 item 별 max(id) 서브쿼리 1회 - 목록 크기와 무관하게 추가 쿼리 1개
- WishDisplayIntegrationTest 신규: 포인터가 옛 버전이어도 목록·단건이 마지막 기계값 표시 / 수기 최신이면 놓은 맥락만 수기값·다른 위시는 기계값 / 수기 뒤 새 기계가 생기면 수기 맥락도 기계값 복귀 / 기계 없으면 포인터 fallback - TournamentIntegrationTest: 대기실 파생 표시 → start 가 표시 버전으로 repin(결과 값·포인터 단언) → 시작 후 새 기계 버전이 생겨도 진행 화면은 박제 값 유지
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough최신 기계 Changes표시 스냅샷 파생 조회
Estimated code review effort: 4 (Complex) | ~45 minutes Assessment against linked issues
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 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.
🧹 Nitpick comments (2)
src/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.kt (1)
43-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"진행 중 유지" 정책이 위시·토너먼트 어느 통합 테스트에서도 검증되지 않습니다.
ItemDisplayService.displayOf의 48행(pointer.isInProgress()분기)은 포인터가 PENDING/PROCESSING 이면서 같은 item 에 이미 기계 READY 버전이 존재할 때만 실행됩니다. 기존 PROCESSING 관련 테스트들은 기계 READY 버전이 아예 없는 상태만 다뤄 47행의 null-fallback 으로 빠지므로, 이 분기는 회귀에 무방비 상태입니다.
src/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.kt#L43-L51: 로직 자체는 정확합니다. 수정 불필요, 테스트로만 잠그면 됩니다.src/test/kotlin/com/depromeet/piki/wishlist/controller/WishDisplayIntegrationTest.kt#L49-L133:saveMachineVersion류로 READY 버전을 먼저 만들고, 같은 item 에 PENDING/PROCESSING 포인터를 가진 wish 를 추가해 응답의 status 가 여전히 PENDING/PROCESSING 인지 검증하는 테스트를 추가하세요.src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.kt#L477-L519: 동일한 조합(기계 READY 존재 + 출전 아이템 포인터가 PROCESSING)을 대기실 조회에 대해 검증하는 테스트를 추가하세요.🤖 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/depromeet/piki/item/service/ItemDisplayService.kt` around lines 43 - 51, ItemDisplayService.displayOf requires regression coverage for preserving PENDING/PROCESSING pointers when a same-item machine READY version exists. In src/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.kt lines 43-51, make no direct change; in src/test/kotlin/com/depromeet/piki/wishlist/controller/WishDisplayIntegrationTest.kt lines 49-133, add tests that create a READY machine version, attach PENDING and PROCESSING pointers to the same item, and assert the response status remains unchanged; in src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.kt lines 477-519, add the corresponding waiting-room test for a PROCESSING participant item.src/main/kotlin/com/depromeet/piki/item/repository/ItemSnapshotJpaRepository.kt (1)
72-85: 🚀 Performance & Scalability | 🔵 Trivial배치 조회에 맞는 복합 인덱스를 추가하세요.
findLatestMachineReadyByItemIds()는item_id in (:itemIds),status=READY,source in (SERVER, SERVER_LLM),deletedAt is null로GROUP BY item_id를 수행합니다. 현재item_snapshots인덱스는(item_id),(status, created_at),(status, updated_at)뿐이라, 이 조건이idx_item_snapshots_item_id(20, V20260603171111)만으로 최적화됩니다. 스냅샷 이력이 item당 많이 쌓이면 위시 목록·포인터 매핑 경로에서 테이블 스캔 비용이 커집니다.(item_id, status, source, deleted_at)등의 복합 인덱스를 고려해 주세요.🤖 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/depromeet/piki/item/repository/ItemSnapshotJpaRepository.kt` around lines 72 - 85, Update the database schema migrations for item_snapshots to add a composite index covering item_id, status, source, and deleted_at, aligned with the predicates and GROUP BY used by findLatestMachineReadyByItemIds(). Keep the existing indexes unchanged and use the project’s established migration naming and index conventions.Source: Path instructions
🤖 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/kotlin/com/depromeet/piki/item/repository/ItemSnapshotJpaRepository.kt`:
- Around line 72-85: Update the database schema migrations for item_snapshots to
add a composite index covering item_id, status, source, and deleted_at, aligned
with the predicates and GROUP BY used by findLatestMachineReadyByItemIds(). Keep
the existing indexes unchanged and use the project’s established migration
naming and index conventions.
In `@src/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.kt`:
- Around line 43-51: ItemDisplayService.displayOf requires regression coverage
for preserving PENDING/PROCESSING pointers when a same-item machine READY
version exists. In
src/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.kt lines
43-51, make no direct change; in
src/test/kotlin/com/depromeet/piki/wishlist/controller/WishDisplayIntegrationTest.kt
lines 49-133, add tests that create a READY machine version, attach PENDING and
PROCESSING pointers to the same item, and assert the response status remains
unchanged; in
src/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.kt
lines 477-519, add the corresponding waiting-room test for a PROCESSING
participant item.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: ef2580ee-2f13-40fb-8c90-3df70f9257d3
📒 Files selected for processing (8)
src/main/kotlin/com/depromeet/piki/item/repository/ItemSnapshotJpaRepository.ktsrc/main/kotlin/com/depromeet/piki/item/repository/ItemSnapshotRepository.ktsrc/main/kotlin/com/depromeet/piki/item/repository/ItemSnapshotRepositoryImpl.ktsrc/main/kotlin/com/depromeet/piki/item/service/ItemDisplayService.ktsrc/main/kotlin/com/depromeet/piki/tournament/service/TournamentService.ktsrc/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.ktsrc/test/kotlin/com/depromeet/piki/tournament/controller/TournamentIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/wishlist/controller/WishDisplayIntegrationTest.kt
- 기계 READY 가 있어도 진행 중(PENDING) 포인터는 파생에 덮이지 않고 프로세싱 상태로 내려간다 - 자기가 시작한 갱신의 UX 신호 보존 (CodeRabbit nitpick 수용). 기존 refresh 테스트는 쓰기 반환값만 단언해 파생 조회 경로의 이 분기를 못 덮었다
|
CodeRabbit nitpick 처리:
|
- 같은 링크는 한 item 을 공유하고 카드는 최신 기계 READY 를 보여준다(#858 파생). 담기 게이트만 위시 포인터를 봐서, 남이 같은 링크를 담아 추출이 성공하면 화면엔 값이 다 떠 있는데 "채운 뒤 담아 주세요"(TOURNAMENT-039)가 나가고 같은 아이템으로 시작은 되는 어긋남이 있었다. 사용자가 따를 수 없는 지시다 - 방아쇠는 둘이다. 다른 사용자가 같은 링크를 위시에 담거나 토너먼트에 URL 로 담을 때. 둘 다 attachOrNull 을 타는데 미완성·실패는 재사용 대상이 아니라 새 PENDING 을 만들고, 그게 성공하면 이 상태가 된다 - INCOMPLETE 만이 아니라 FAILED 도 같은 함정이라 준비됨 검사까지 함께 옮긴다 - 판정을 requireEntryEligible 한 곳으로 합쳤다. 담기와 시작은 같은 질문에 답하는데 코드가 두 벌이라 갈라졌다. 한 벌이면 갈라지는 것 자체가 불가능하다. 미완성을 먼저 보는 순서도 여기 한 곳에만 남아 리팩터링에 흩어지지 않는다 - 박제는 포인터 그대로 둔다. 겨루는 값 확정은 start 의 몫이고(#858) 대기실 표시도 파생으로 움직여, 판정만 옮겨도 결과가 맞아떨어진다 - 회귀 테스트 2개(성공값 있을 때 통과 / 없을 때 그대로 거부). 대조군을 함께 두는 이유는 앞 테스트만으로는 게이트를 통째로 지워도 초록불이기 때문이다. 수정을 되돌리면 앞 테스트만 실패하는 것까지 확인했다 - 픽스처에 extractionMethod 를 실었다. 이걸 빼면 source 가 null 이라 표시값 파생의 후보(기계 READY)에 안 걸려, 재현하려던 상황이 재현되지 않는다
* fix: 사용자 행동에 관여하는 스냅샷 읽기를 전부 표시값으로 정합한다 - #1006(담기 게이트)과 같은 축의 어긋남을 전수 조사했다. 원본 저장소 소비 23군데를 갈래로 가르니 쓰기·정체성·버전 자체(알림·이력)·표시값 입력은 원본이 맞고, 사용자가 화면에서 보고 행동한 결과에 관여하는 읽기만 표시값이어야 했다 - 어긋난 곳 셋을 고쳤다 - 수기 수정 base(위시·토너먼트): 카드는 표시값을 그리는데 병합 base 만 포인터라, 남이 채운 가격이 화면에 떠 있는데 이름만 고치면 "가격이 필요하다"(400)로 튕겼다. base 를 표시값으로 바꿔 카드에 보인 값 위에 수정이 얹힌다. dry-run(업로드 전 검증)도 같은 base 로 맞췄다 - 새로고침 판정: 포인터가 FAILED 여도 남의 성공(기계 READY)이 있으면 item 이 추출 가능하다는 증거라 막을 이유가 없다. 표시값까지 FAILED 일 때만 보정 유도(409)가 남는다. recover 와의 직렬화는 원래 wish 행 락이 지고 있었다 - 기존 주석의 "상태로 갈려 침범하지 않는다"는 근거를 실물로 재검증한 결과다 - 토너먼트 수정의 "pin 을 base 로" 주석은 #858 이전 유물이었다. 수정은 PENDING 전용이고 PENDING 카드는 표시값을 그리므로, "이 카드가 보던 버전 위에 수정" 이라는 그 주석의 의도 자체가 지금은 표시값을 가리킨다 - SnapshotAccessConventionTest 로 원본 소비자 집합을 동결했다. 포인터와 표시값이 같은 타입이라 컴파일러가 못 잡고 단일 사용자 테스트에선 둘이 같아 테스트도 못 잡는 함정이라, 새 소비 클래스가 목록에 자기 갈래를 적게 강제해 규칙을 읽게 한다. 갈래 판정 자체는 사람 리뷰 몫이다 - 회귀 테스트 3쌍(위시 수정·토너먼트 수정·새로고침, 각각 대조군 포함). 세 수정을 일시 되돌리면 정확히 새 테스트 3개만 실패하는 것을 실측했다 * refactor: 수기 수정 dry-run 을 persistence 소유로 접고 낭비 쿼리를 줄인다 /simplify 4각(재사용·단순화·효율·깊이) 리뷰 반영. - dry-run(업로드 전 검증)을 각 persistence 서비스의 validateManualEdit 으로 이동해 병합 base 선택(editBasisOf)이 실제 저장과 같은 코드를 탄다. 이 PR 이 "dry-run 과 실저장은 같은 base" 를 우연에서 규칙으로 승격시켰는데, 그 규칙을 주석 두 벌이 지키는 구조였다 - 한 벌이면 갈라지는 것 자체가 불가능하다 - dry-run 은 이미지가 있을 때만 부른다. 유일한 존재 이유가 S3 orphan 방지라, 업로드 없는 수정(지배적 케이스)은 manualEdit(락 안) 최종 판정 하나로 충분하다. 예외·응답 동일, 수기 수정 요청당 표시값 파생 쿼리 2회 -> 이미지 없으면 0회 - 새로고침 판정을 단락 평가로: 포인터가 FAILED 가 아니면 표시값도 FAILED 일 수 없어(파생 후보가 READY 뿐) 지배적 READY 경로의 쿼리 1회가 결과 무관 낭비였다 - TournamentItemService 의 사전 권한 검사·스냅샷 의존이 위임으로 사라져 주입 3개(스냅샷 저장소·표시값 서비스·아이템 저장소)를 걷어냈다. 원본 소비자가 9곳 에서 8곳으로 줄었고 동결 목록의 stale 검사가 이 제거를 실제로 강제했다 - SnapshotAccessConventionTest: JUnit 이 테스트마다 인스턴스를 새로 만들어 인스턴스 lazy 로는 1회 스캔이 안 됐다(전체 트리 2회 walk). companion 으로 올려 1회로 만들고, 두 테스트가 각자 돌리던 같은 필터를 공유 집합으로 합쳤다. 파일명 키는 동명 파일이 생기면 무단 통과라 경로 접미사 키로 바꿨다 - 테스트 시딩 중복 제거: 위시는 기존 seedReadyWish 에 extractionMethod 파라미터 (기본 null 이라 기존 호출 무변화)를 더해 인라인 3블록을 대체, 새로고침은 seedFailedWish 추출, 토너먼트는 saveTournamentItemFor 재사용. 그 과정에서 saveTournamentItemFor 의 extractedAt 규칙이 saveSnapshot 과 어긋나 있던 것 (READY 만 vs READY·INCOMPLETE)도 정정했다
Situation
Task
Action
파생 규칙 (단일 지점: ItemDisplayService)
적용 지점과 박제
성능
검증
Result
연관 이슈
Summary by CodeRabbit
개선 사항
테스트