fix: 优先用确定来源暗牌做身份交换并修正牌顶候选#34
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough本次更新扩展了模糊暗手覆盖计算、玩家来源占位匹配与身份置换逻辑,并补充公共位置归一化及相关测试,覆盖随机手牌转移、约束组同步和牌堆顶候选描述。 Changes匿名手牌与模糊覆盖
玩家来源占位与身份置换
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Code Review
This pull request refactors the card tracker logic to prevent premature convergence of random hand transfer constraints and avoid duplicate anonymous card creation. It introduces tracking for ambiguous hidden hand coverage and refactors source card retrieval to prioritize exact unknown cards. Feedback on these changes suggests optimizing collectAmbiguousHiddenHandCoverage to reduce garbage collection pressure in a hot path, ensuring strict type safety for combinationID assignments in replaceCardInConstraintGroups, and avoiding redundant Set instantiations in getUnknownPlayerSourceCards.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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/tracker/roomConstraints.ts`:
- Around line 147-223: 修正 collectAmbiguousHiddenHandCoverage 中按任意 hiddenCards
交集直接跳过整个 coveragePackage 的逻辑。将共享实体的覆盖包按重叠连通分量合并,并结合各包的 coverageBySeat
计算满足不同座位需求的最大可行覆盖,确保同一实体不能同时覆盖多个槽位但不丢弃不冲突的席位覆盖;保留互不相交分量可累加的行为,避免低估覆盖量或重复创建匿名实体。
In `@src/tracker/roomMovement/sources.ts`:
- Around line 73-74: Update the source spell ID handling near
getCompatibleMarkSpellIDs so every value in the spellIDs array is preserved and
evaluated for compatibility, rather than only spellIDs[0]. Keep scalar spellIDs
supported, and ensure markSpellIDs includes all compatible IDs so existing mark
entities are selected before fallback handling.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f2ad573-0d86-45e3-a4b8-95ad72e0ef86
📒 Files selected for processing (10)
docs/agents/card_tracker.mdsrc/tracker/Room.tssrc/tracker/candidate/publicCandidate.tssrc/tracker/roomConstraints.tssrc/tracker/roomMovement.tssrc/tracker/roomMovement/sources.tstests/tracker/ambiguousKnownIndexIncremental.test.tstests/tracker/handCountObservation.test.tstests/tracker/publicCandidates.test.tstests/tracker/trackerController.test.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 (2)
src/tracker/roomMovement/sources.ts (1)
369-392: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win不要撤销迁移后的约束组身份。
Line 370 的
replaceCardInConstraintGroups()已将placeholder.combinationID同步为命中的组 ID,但 Line 392 随即无条件恢复oldCombinationID,会让实体的组合身份与实际组成员关系不一致。仅在没有迁移任何约束组时恢复旧值。建议修改
+ let migratedConstraintIdentity = false if (preserveAmbiguousIdentity) { - this.room.constraints.replaceCardInConstraintGroups(card, placeholder) + migratedConstraintIdentity = + this.room.constraints.replaceCardInConstraintGroups(card, placeholder) } ... - placeholder.combinationID = oldCombinationID + if (!migratedConstraintIdentity) { + placeholder.combinationID = oldCombinationID + }🤖 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/tracker/roomMovement/sources.ts` around lines 369 - 392, Update the placeholder restoration logic in swapKnownCardWithPlayerSourcePlaceholder so oldCombinationID is restored only when no constraint groups were migrated. Preserve the combination ID assigned by replaceCardInConstraintGroups when preserveAmbiguousIdentity performs a migration, while retaining the existing old-value restoration behavior otherwise.src/tracker/roomConstraints.ts (1)
288-293: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win合并同一席位的候选实体,不要覆盖先前集合。
Line 288 会累加同席位多个 hand location key 的覆盖量,但 Line 292 仅保留最后一个 key 的实体。若这些 key 的
spellID不同且候选不重叠,匹配会错误少算覆盖量。建议修改
- hiddenCardsBySeat.set(handCandidate.seatID, new Set(ambiguousHiddenCards)) + const seatHiddenCards = + hiddenCardsBySeat.get(handCandidate.seatID) ?? new Set<Card>() + ambiguousHiddenCards.forEach((card) => seatHiddenCards.add(card)) + hiddenCardsBySeat.set(handCandidate.seatID, seatHiddenCards) ambiguousHiddenCards.forEach((card) => hiddenCards.add(card))🤖 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/tracker/roomConstraints.ts` around lines 288 - 293, 在处理 handCandidate 的席位聚合逻辑中,更新 hiddenCardsBySeat 时合并该席位已有集合与 ambiguousHiddenCards,而不是用新集合覆盖旧集合;保留 coverageBySeat 的累加行为,并确保合并后的集合继续用于后续匹配。
🤖 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/tracker/roomConstraints.ts`:
- Around line 288-293: 在处理 handCandidate 的席位聚合逻辑中,更新 hiddenCardsBySeat
时合并该席位已有集合与 ambiguousHiddenCards,而不是用新集合覆盖旧集合;保留 coverageBySeat
的累加行为,并确保合并后的集合继续用于后续匹配。
In `@src/tracker/roomMovement/sources.ts`:
- Around line 369-392: Update the placeholder restoration logic in
swapKnownCardWithPlayerSourcePlaceholder so oldCombinationID is restored only
when no constraint groups were migrated. Preserve the combination ID assigned by
replaceCardInConstraintGroups when preserveAmbiguousIdentity performs a
migration, while retaining the existing old-value restoration behavior
otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 222d34ba-de92-4fdd-ae06-ad9ecbe2c59d
📒 Files selected for processing (4)
src/tracker/roomConstraints.tssrc/tracker/roomMovement/sources.tstests/tracker/handCountObservation.test.tstests/tracker/hiddenMarkCandidates.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/tracker/handCountObservation.test.ts
Summary
position: 'top' | 'bottom'字符串,修正暗手牌置于牌堆顶时的公共候选文案。Why
随机获得后使用自身暗牌时,内部暗实体 ID 可能碰巧命中转移候选;旧逻辑会按实体顺序过早确认来源,导致候选明牌被错误排除,约束组座位名额也提前坍缩。
Impact
Validation
pnpm test:tracker(189 passed)pnpm lintpnpm typecheck:trackerSummary by CodeRabbit