perf(test): cap the 3p loop-shortcut grind drives at the measured priming point

[lgray]

🤖 AI text below 🤖

Summary

The three longest-running tests in the entire 21621-test corpus all live in loop_shortcut.rs, and all three spent ~99.9% of their wall clock in a drive_collect grind that runs before their assertions and never exits early. A cap sweep proves the grind buys zero discrimination past beat ~4, so this replaces the three cap literals with one measured shared constant — 49.7s/49.8s/56.1s → 2.4s/2.5s/4.8s, with every assertion, fixture, and revert-probe flip preserved.

Files changed

• crates/engine/tests/integration/loop_shortcut.rs — new PRIMED_LOOP_BEATS const + 3 cap substitutions + doc corrections

Track

Developer

LLM

Model: claude-opus-4-5-20260514
Tier: Frontier
Thinking: high

Implementation method (required)

Method: /engine-implementer

CR references

None. No rules logic changes — test-runtime only. The three existing citations in touched doc comments (CR 119.8, CR 104.2a, CR 704.5a) survive verbatim; zero CR numbers added or removed (git diff | grep -oE 'CR [0-9.]+' on both + and - lines is empty). docs/MagicCompRules.txt is gitignored and absent from this worktree, so no new CR number could be verified — per CLAUDE.md, none was written.

Verification

• Required checks ran clean, or the exact CI-owned alternative is stated below.
• Gate A output below is for the current committed head.
• Final review-impl below is clean for the current committed head.
• Both anchors cite existing analogous code at the same seam.

Where the time went (measured, not estimated)

probe	drive_collect	everything else
subset-lethal, 500 beats	19.30s	—
subset-lethal 500 + inject + declare + E1 drive	19.32s	declare 3.3ms + accept/E1 9.4ms
both_fall(1000,1050), 200 beats + E1	22.19s	declare 7.1ms + E1 26.5ms

The behaviour actually under test costs 10–35ms. Per-beat cost is record_loop_detect_sample() (a full normalize_for_loop deep GameState clone, ring cap 16) plus interactive_loop_bridge rescanning all ≤16 priors twice — paid every beat, forever, because the subset-lethal bridge deliberately falls through to the pre-feature grind and so never reaches drive_collect's terminal-state exit.

Before / after

test	before	after	speedup
interactive_3p_subset_lethal_does_not_crown	49.672s	2.389s	20.8×
injected_3p_one_faller_no_crown	49.814s	2.522s	19.8×
injected_3p_unequal_life_pin_all_no_crown	56.062s	4.813s	11.6×

Debug build, contended (Tilt building concurrently) — upper bounds, and deliberately not encoded as a timing assertion.

Why cap 24 preserves discrimination

Cap sweep over {4, 8, 12, 16, 24, 32, 48, 64, 100}, run on both a clean tree and one tree per defect:

• Clean: every assertion and every reach-guard PASS at every cap — including test 3's eq_crowns reach-guard and test 2's rolled-back P2 == 20.
• Defective: each test's revert-probe flips it to FAIL at every cap ≥ 4.

Behaviour is flat from beat 4 to beat 500, so the old 500/500/200 caps bought nothing. 24 is itself a swept value (6× margin over the measured ≤4 priming point), not an interpolation.

Flip-proofs on the SHIPPED tests (not on probe copies)

The sweep above ran on temporary probe clones, so it was re-run against the real committed tests, one defect per build:

defect (single mutation, reverted after)	shipped test	result
loop_check.rs — nonfallers.len() != 1 weakened to any-faller-wins	interactive_3p_subset_lethal_does_not_crown	FAILED 0.153s — :1165 "subset-lethal loop must NOT crown a winner (CR 104.2a), got GameOver { winner: Some(PlayerId(0)) }"
engine.rs — E1-measure live_mandatory_loop_winner gate bypassed	injected_3p_one_faller_no_crown	FAILED 2.481s — :1338 "subset-lethal loop must NOT crown (CR 104.2a), got GameOver { winner: Some(PlayerId(0)) }"
engine.rs — F2 fallers_lives_pairwise_equal ≥2-faller re-check removed	injected_3p_unequal_life_pin_all_no_crown	FAILED 4.634s — :1544 "unequal-life ≥2-faller drain must NOT crown (CR 704.3 simultaneity), got GameOver { winner: Some(PlayerId(0)) }"

Every panic lands on the test's intended discriminator, not on a reach-guard — the optimized tests fail for the right reason, not incidentally. The production tree was verified pristine (git diff --stat crates/engine/src/ empty) after each mutation.

Non-vacuity

Stripping all comments from both HEAD: and the new file and diffing shows the complete non-comment delta is the new const plus three cap substitutions. Every assertion body and every fixture value (1000 / 1050 / 20, gap 50) is byte-identical.

Commands

• cargo nextest run -p engine -E 'test(interactive_3p_subset_lethal_does_not_crown) | test(injected_3p_one_faller_no_crown) | test(injected_3p_unequal_life_pin_all_no_crown)' — 3 passed, 4.828s total (2.389 / 2.522 / 4.813)
• cargo nextest run -p engine -E 'binary(integration) and test(loop_shortcut)' — 69 passed, 8.578s (also covers loop_shortcut_activation + loop_shortcut_mana_engine)
• cargo clippy -p engine --all-targets -- -D warnings — clean, 0 warnings
• cargo fmt --all — no reformat

All run in a dedicated worktree with its own target/ (Tilt watches a different checkout, so scripts/tilt-wait.sh would return 3 = cannot-answer, not a failure).

Gate A

Gate A PASS head=345b34be61ca53eeff81eab32698a2108c0e8904 base=2c6596de9360ef3e8d5df1c20e863631201253aa

Anchored on

• crates/engine/tests/integration/loop_shortcut.rs:1954 — POISON_RIDER, the file's existing convention of a file-scope const placed after a section banner, above its consuming tests (also :2079, :2532, :2703, :4077)
• crates/engine/tests/integration/loop_shortcut.rs:253 — drive_collect, the unchanged shared driver whose cap argument is the only thing this PR re-values

Final review-impl

Final review-impl PASS head=345b34be61ca53eeff81eab32698a2108c0e8904

Claimed parse impact

None. No parser file touched, so no Gate A parser-combinator concern applies beyond the pre-commit gate output above.

Scope Expansion

None. Two pre-existing doc errors surfaced during measurement and are corrected in place (no assertion body changed):

1. The M1 REVERT-FAIL note claimed the mutation flips "the two no-crown assertions". Measured: only the wf assertion discriminates — the event-scan stays true under that mutation at every cap 4…100. Now stated accurately. Fixing the assertion itself would require editing an assertion body, so it is disclosed rather than changed.
2. The equal-life half of setup_3p_both_fall consumes 0 beats — its mandatory single-non-faller loop is already crowned by the natural bridge while the kick-off resolves. This is cap-independent, so it was equally true at the old cap 200. The const doc now carries measured per-drive beat counts (24/24, 24/24, 0/24) instead of a blanket claim.

Validation Failures

None.

CI Failures

None.

Summary by CodeRabbit

• Tests
  • Updated loop-driving scenarios to use a consistent priming interval and a fixed priming beat cap.
  • Added stronger assertions to ensure priming/loop reachability occurs within the cap, preventing vacuous no-crown outcomes.
  • Updated related injected-soundness test flows to be priming-aware and reflect the intended reachability behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 072638c7-e31d-44cb-9bc4-8b11fc17c82f

📥 Commits

Reviewing files that changed from the base of the PR and between 345b34b and f69974a.

📒 Files selected for processing (1)

• crates/engine/tests/integration/loop_shortcut.rs

---

📝 Walkthrough

Walkthrough

Changes

The integration test harness now detects loop recurrence during capped drives. Subset-lethal and injected soundness tests use a shared 24-beat cap and assert priming before evaluating no-crown outcomes.

Subset-lethal loop test validation

Layer / File(s)	Summary
Add recurrence-aware primed drive crates/engine/tests/integration/loop_shortcut.rs	Adds a helper that collects drive events and reports whether loop states recur modulo resources.
Apply priming guards to soundness tests crates/engine/tests/integration/loop_shortcut.rs	Uses the documented 24-beat cap across relevant tests and asserts recurrence before checking crown outcomes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: matthewevans

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: reducing 3p loop-shortcut test drive caps to the measured priming point.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@crates/engine/tests/integration/loop_shortcut.rs`:
- Around line 1105-1131: Add a positive reach-guard in the no-crown test flows
around the natural loop-detection/no-crown decision in drive_collect, proving
that terminal-state evaluation occurred before asserting !GameOver and no
GameOver event. Ensure upstream exhaustion or the PRIMED_LOOP_BEATS cap cannot
make these negative assertions pass without reaching that branch; retain or
adjust the cap only if necessary to guarantee the guard is reached.

🪄 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: CHILL

Plan: Pro Plus

Run ID: 1d882964-8ffd-4488-ac5d-d986d83bf6e1

📥 Commits

Reviewing files that changed from the base of the PR and between 2c6596d and 345b34b.

📒 Files selected for processing (1)

• crates/engine/tests/integration/loop_shortcut.rs

[matthewevans]

Request changes — the cap reduction needs a decision-point reach guard.

🔴 Blocker

crates/engine/tests/integration/loop_shortcut.rs:1149 now drives only PRIMED_LOOP_BEATS, while drive_collect returns the current WaitingFor state when its loop simply exhausts (crates/engine/tests/integration/loop_shortcut.rs:256). The new documentation explicitly says the subset-lethal and unequal-life runs consume all 24 beats without hitting a terminal exit (crates/engine/tests/integration/loop_shortcut.rs:1104), and their reach guards establish only that players lost life (:1153, :1516). That does not show that the loop-classification decision was evaluated: a regression which delays sampling/classification beyond beat 24 still leaves the runner in Priority, makes the no-crown negative assertions pass, and satisfies the life-loss guards.

CodeRabbit raised the same current-head finding; tracing the production path confirms it: the actual classification is in interactive_loop_bridge / find_live_loop_winner (crates/engine/src/game/engine.rs:604, :784), not in drive_collect.

Suggested fix: add a positive, externally observable reach guard for the classification/sampling decision that the three no-crown tests exercise (or retain a cap that demonstrably reaches it). The guard must fail when the cap ends before that decision, rather than relying on the documented manual sweep.

✅ Clean

The change is tightly scoped to the integration test and retains the existing fixtures and assertions.

Recommendation: request changes; re-review once the cap has a discriminating decision-point guard.

[lgray]

🤖 AI text below 🤖

Guard added in f69974a — you were right, and the measurement went further than the review did.

What the guard is. drive_collect_primed drives through the existing drive_collect beat by beat and reports whether any prior in GameState::loop_detect_ring has compared equal to the live state modulo resources, using loop_states_equal_modulo_resources — the engine's own predicate, publicly re-exported at analysis/mod.rs, so no recurrence logic is reimplemented test-side. A ring hit forces the §3 bridge conjuncts to have held at that state (the sampler only pushes with a non-empty stack at a Priority beat, and GameState's PartialEq compares both), so find_live_loop_winner → live_mandatory_loop_winner demonstrably ran on it.

It fails when the cap ends before the decision, as you required. Forcing PRIMED_LOOP_BEATS to 1 — below the measured beat-2 priming point — flips all three tests to FAIL on this guard:

interactive_3p_subset_lethal_does_not_crown   FAIL  the loop never reached a board-recurrent state within 1 beats
injected_3p_one_faller_no_crown               FAIL  (same)
injected_3p_unequal_life_pin_all_no_crown     FAIL  (same)

Three measurements shaped it, and one of them contradicts the obvious implementation.

1. Recurrence is phase-dependent, not monotone. Swept over caps {1, 2, 3, 4, 8, 24}, ring hits occur at 2 and 8 and not at 1/3/4/24 — period 6. A guard that inspected the terminal state (the natural first cut) would have reported false on a perfectly primed loop and blocked the PR for the wrong reason. Hence the per-beat scan, short-circuited at the first hit: 5.6–17.5 ms per drive, suite still 69/69 in 8.1 s.
2. Guard placement is load-bearing in interactive_3p_subset_lethal_does_not_crown. Placed first, it steals the M1 panic — under the weakened nonfallers.len() != 1 defect the crown fires at beat ~1, before recurrence, so the test failed with "never became reachable" instead of reporting the wrongful crown. Placed last, M1 lands on the intended wf no-crown assertion (line 1212). Both orderings were measured; the comment records why.
3. In the F2 test the guard is asserted on the unequal half only: the equal half consumes 0 beats (already crowned as the kick-off resolved), so it has no drive in which to recur — and the unequal half is the one the F2 revert-probe flips, so the whole discriminator sits on the guarded side.

M1/M2/M3 were re-run on the shipped text: each still fails at its intended discriminator (lines 1212 / 1425 / 1640), never at a guard. cargo clippy -p engine --all-targets -D warnings clean.

One correction to the record, in your favour. I also measured whether any cap could have satisfied the alternative you offered ("or retain a cap that demonstrably reaches it"). It cannot. Making the bridge's !state.loop_detect_ring.is_empty() conjunct unreachable — i.e. the classification never runs at all — leaves all three tests green at cap 24 and equally green at cap 500, while vito_2p_optional_offer_declare_crowns and interactive_queued_opponent_concede_no_deadlock fail (which is what proves the suppression took effect). The old caps were not buying the reach either: at 500 the drive also consumed every beat and returned mid-drive in Priority.

And the new guard does not close that particular hole either — the ring sampler is a separate gate that the suppression doesn't touch, so the ring still fills and primed still reports true. I had claimed otherwise in a first draft of the doc; it was wrong and is now stated accurately. The guard closes the cap-adequacy question, which is the one the cap reduction actually opened. The bridge-suppression blind spot is inherent to a negative "did not crown" test and is covered by the positive-side tests named above.

Two smaller doc corrections the measurements forced, both flagged rather than buried: the guard checks only the equality disjunct of the engine's equal_modulo_resources || cover_modulo_growth gate (loop_states_cover_modulo_growth is pub(crate), so an integration test cannot call it — noted in the doc as the thing to widen if a fixture ever drifts into the coverability regime), and an interpolated "holds at beats 14, 20" claim has been cut back to the beats actually probed.

[matthewevans]

Approved on 91e687b0aede0d8e97412dd286cdfbbe86d8d639.

The recurrence guard uses the engine's loop_states_equal_modulo_resources predicate at every driven beat, so the capped no-crown cases now fail if they never reach the live ring/bridge regime. The change is test-only, scoped to the existing integration seam, and the current CodeRabbit review has no actionable finding.