feat(analysis): combo-detection PR-3 — detect mandatory-loop winner for drain-cascade combos (CR 704.5a) - #4480
Conversation
… (CR 704.5a)
When priority passes in a cycle with a non-empty stack and the loop-detect ring shows a repeating modulo game state, reconcile_terminal_result identifies the single non-falling living player and emits GameOver{winner} immediately, instead of grinding hundreds of beats to the CR 704.5a life-loss death.
All-living-player meaningful-action firewalls (CR 104) ensure a player with a meaningful out is never shortcut, and live_mandatory_loop_winner returns None for mutual-drain / can't-lose / net-zero / multi-faller boards.
Promotes Sanguine Bond + Exquisite Blood (idx 17, targeted) and Marauding Blight-Priest + Bloodthirsty Conqueror (idx 18) to DRIVEN_ROW_INDICES, each backed by a non-vacuous live apply(PassPriority) test observing GameOver at beat 6. loop_detect_ring is serde-skipped + eq-excluded (zero wire surface); reuses the existing GameOver event (no new variant). A thread-local probe guard keeps legality probes from running game-ending shortcut logic.
Assisted-by: ClaudeCode:claude-opus-4.8
There was a problem hiding this comment.
Code Review
This pull request implements live loop-shortcut detection (PR-3 Option C) for mandatory net-progress drain cascades. It introduces a persisted loop_detect_ring on GameState to track post-resolution snapshots across beats, canonicalizes volatile stack entry IDs in project_out_resources to enable modulo-equality matching, and couples the loop classifier into the live reducer via live_mandatory_loop_winner. It also adds defensive guards (SimulationProbeGuard) to prevent shortcutting during AI legality probes and ensures all living players are checked for loop-ending actions. The review feedback identifies a performance improvement opportunity in reconcile_terminal_result to avoid unnecessary heap allocation and Arc cloning when iterating over the loop detection ring.
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.
| // Clone the Arc handles (cheap refcount bumps) to release the borrow on the | ||
| // ring before the GameOver mutation below. | ||
| let priors: Vec<std::sync::Arc<GameState>> = | ||
| state.loop_detect_ring.iter().cloned().collect(); | ||
| let cur = crate::analysis::resource::ResourceVector::snapshot(state); | ||
| if let Some(winner) = priors.iter().find_map(|prior| { | ||
| let delta = crate::analysis::resource::ResourceVector::delta( | ||
| &crate::analysis::resource::ResourceVector::snapshot(prior), | ||
| &cur, | ||
| ); | ||
| crate::analysis::loop_check::live_mandatory_loop_winner(prior, state, &delta) | ||
| }) { |
There was a problem hiding this comment.
[MEDIUM] Unnecessary Heap Allocation and Arc Cloning in Hot Path
Cloning the entire loop_detect_ring into a Vec to release the borrow on state is unnecessary. Since PlayerId is a Copy type and does not borrow state, the borrow of state by find_map is completely released as soon as find_map returns.
By iterating over state.loop_detect_ring directly and storing the result of find_map in a local variable, we can avoid allocating a Vec on the heap and performing up to 16 Arc clones (refcount increments/decrements) on every priority pass check when the stack is active.
let cur = crate::analysis::resource::ResourceVector::snapshot(state);
let winner = state.loop_detect_ring.iter().find_map(|prior| {
let delta = crate::analysis::resource::ResourceVector::delta(
&crate::analysis::resource::ResourceVector::snapshot(prior),
&cur,
);
crate::analysis::loop_check::live_mandatory_loop_winner(prior, state, &delta)
});
if let Some(winner) = winner {|
Current-head maintainer review found no content blocker on the net GitHub API diff. I reviewed the reducer/analysis/GameState/SBA surfaces for the live drain-cascade shortcut: the shortcut is at the terminal reconciliation seam, samples are captured after post-action trigger placement, the winner classifier reuses the existing modulo-resource detector plus SBA can't-lose/can't-win predicates, and the production-path tests cover both live drain loops plus a victim-with-a-meaningful-out negative. I ran update-branch because the PR was behind; GitHub produced merge-only head |
matthewevans
left a comment
There was a problem hiding this comment.
Approved current head 2ecacbfe1192d9436bd27e276dfa3b4c617b713c after the earlier Tier 2 review and fresh exact-head CI.
Review basis: GitHub API diff, isolated worktree, local CR lookup for the cited 732.2a/732.5/704.5a/603.3/101.2/104.x rules, Gemini context, and the review-impl reducer/analysis/GameState/SBA lenses. No content blocker found: the live shortcut stays at terminal reconciliation, loop samples are captured after post-action trigger placement, the winner path reuses the modulo-resource detector and existing SBA can-lose/can-win predicates, and the production-path tests cover both live drain positives plus the victim-with-meaningful-out negative.
Exact-head CI is green/CLEAN, and the repush guard held after assignment reread.
🤖 AI text below 🤖
Combo-detector series
This is PR-3 of the staged, offline-first infinite-combo / loop detector (Engine A). The series goal is to incrementally build combo/loop detection toward shortcutting mandatory, game-ending loops — recognizing that a repeating modulo game state with no meaningful out must resolve to a deterministic winner, rather than grinding hundreds of beats to the CR 704.5a life-loss death.
ResourceVector+loop_states_equal_modulo_resources(modulo-resource loop equality / reuse fingerprint); additive, no behavior change.GameRunner::actthat feeds/consumesResourceVector.analysis/loop_check.rsdetect_loop→LoopCertificate) + corpus harness driving Priest+Umbral & Marwyn+Sword loop certificates.reconcile_terminal_result+live_mandatory_loop_winner): detects a repeating modulo state and emitsGameOverimmediately, ahead of the existingemit_resolution_haltrunaway backstop.Predecessors:
ResourceVector.(Foundation: PR-0 — #4092 — #4092 —
ResourceVector+ modulo-resource loop equality.)Summary
Adds a live mandatory-loop winner shortcut for drain-cascade combos. When priority passes in a cycle with a non-empty stack and the loop-detect ring shows a repeating modulo game state,
reconcile_terminal_resultidentifies the single non-falling living player and emitsGameOver{winner}immediately — instead of grinding hundreds of beats to the CR 704.5a life-loss death.What it does
pass_priority_once_with_pipeline+ a modulo-match shortcut inreconcile_terminal_result.loop_detect_ringis#[serde(skip)]+ eq-excluded (zero wire surface); reuses the existingGameOverevent (no new variant, no inventory regen).no_living_player_has_meaningful_priority_actionprobes all living players (not just the current holder);live_mandatory_loop_winnerreturnsNonefor mutual-drain / can't-lose / net-zero / multi-faller boards. A player with a meaningful out is never shortcut (CR 104).apply_as_current_for_legalityperf path (perf'd legality apply run under the guard).DRIVEN_ROW_INDICES, each backed by a non-vacuous liveapply(PassPriority)test observingGameOverat beat 6.Verification
cargo build -p engineclean;cargo clippy -p engine --lib --tests -- -D warningsclean.drive_drain_idx18_wins_live,drive_drain_idx17_targeted_wins_live, bounded-termination, victim-with-out-not-eliminated.analysis::suite 96/0. Non-vacuity measured by source revert (disabling the §2 ring sample or the §3 shortcut drops the earlyGameOver).Rebased onto current
main; the single conflict (with #4479's legality-probe perf refactor) was resolved by composing both changes and re-verified green.