From 0858ca9c7cb1c68eac25790a2e39e0810771d3b0 Mon Sep 17 00:00:00 2001 From: lgray Date: Tue, 4 Aug 2026 23:02:18 -0500 Subject: [PATCH 1/5] fix(engine): cite basis A, not `ring_delta_signature`, for answer-beat frame homogeneity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment/test-prose correction on top of `f144bb374`. NO production behaviour change: the `sync_waiting_for(state, &wf)` hoist that commit made is correct and is untouched. What it shipped WRONG was the justification. `f144bb374` claimed, in its message and in three in-tree sites, that a heterogeneous ring "breaks `ring_delta_signature`'s turn-position conjunct, because `impl PartialEq for GameState` compares `waiting_for`/`priority_player`". That is false at source. `ring_delta_signature`'s body reads exactly two things: `ResourceVector::snapshot(&f.normalized)` and `window_scope_from_cover_frames(..).phase_invariant`, and `phase_invariant` is `turn_number` + `phase` + `extra_phases.is_empty()` on both frames. Measured: `grep -cP "waiting_for|priority_player|loop_states_equal"` over the function body returns 1, and that single hit is a prose comment, not code. The real sensitivity is BASIS A. The ring scans call `analysis::resource::loop_states_equal_modulo_resources(prior, state)` with `prior` a ring frame's `normalized` half and `state` the LIVE board; that chains to `types::game_state::loop_states_equal` => `impl PartialEq for GameState`, which DOES compare both fields, and neither `normalize_for_loop` nor `project_out_resources` neutralizes either. An un-synced answer-beat frame therefore compares UNEQUAL against a synced live board and basis A misses the recurrence the frame was minted to certify. Each link verified at source. Corrected, in order of harm: * `tests/integration/loop_shortcut.rs` arm-3 failure message — it printed the false reason ON FAILURE, the worst placement of the four. Now states that `ring_delta_signature` is INSENSITIVE to the two fields and that arm 3 is the blast-radius pin, not a restatement of arms (1)/(2). * the same file's row doc, and `game/engine.rs`'s permanent comment at the sampler, both re-cited to basis A. The comment quotes and names the false claim rather than silently replacing it, because a later reader would otherwise re-derive it. * `analysis/resource.rs`'s `ring_delta_signature` doc gains a minimal SCOPE note separating what the function READS from where its frames' homogeneity comes from. The pre-existing turn-position paragraph is TRUE (via `phase_invariant`) and is left standing. Also corrected: * BLAST RADIUS was justified by "`apply_action_boundary` re-syncs". Three production routes reach `apply_action` without that boundary: `inject_pinned_answer`'s three dispatches, `drive_loop_action_iteration`'s ten, and `apply_interaction_pre_reconciliation_for_life_safety`, which returns `raw.result` without ever calling `finish_action_boundary` (recorded in `apply_action_boundary_core`'s own comment). The claim is now an argument about RE-DERIVATION (`finish_action_boundary` runs the same `sync_waiting_for` over `result.waiting_for` and copies it back; the reorder does not change `ActionResult.waiting_for`) plus the measured fact that all three non-boundary routes drive a CLONE (`drive_one_shortcut_cycle`'s `work`, the drive's `clone`, `preview_candidate_life_safety`'s `preview`), never the settled board. * "PRE-pipeline pair" was the wrong label for what the pre-fix snapshot carried: `run_post_action_pipeline_from` itself writes `state.waiting_for` at five sites. Relabelled "UN-SYNCED pair". * the permanent comment asserted, present tense, "0 divergences over 18,486 lib + 4,487 integration rows" — figures from a probe tree that this tree already exceeds. Restated as a historical measurement attributed to `f144bb374`, with no transcribed row count that can go stale. * one sentence added for a consequence `f144bb374` left unmentioned: because the synchronizer runs `normalize_legacy_attach_waiting_for`, `state.waiting_for` can now differ from the returned `ActionResult.waiting_for` on this path, where the old raw clone made them exactly equal. Benign, and why. ARM 2 ASSERTED THE WRONG COMPARAND — the one code-affecting fix here. `frame.priority_player == frame.active_player` is not what `sync_waiting_for` establishes: it sets `priority_player = turn_control::authorized_submitter_for_player(state, wf.acting_player())`, which re-routes to a DIFFERENT seat under a turn-decision controller (Mindslaver) or a latched search-decision controller. A correctly-synced frame on any future turn-control fixture would false-fail this standing pin. The comparand is now the authority function applied to the frame's own window. That change does NOT weaken the arm, measured rather than argued. Neither `effective_authority_for_player` nor `search_decision_authority` reads `priority_player`, so the recomputation cannot be tainted by the mutant it must catch. Re-run at this tree, predicate `cargo test -p phase-engine --test integration loop_shortcut::answer_beat_frames_carry_the_synced_window_and_the_ offer_certificate_is_exact -- --exact`, one unit = one test row: * clean: 1 passed. * MUTANT-A (`state.priority_player = PlayerId(3);` after the sync): FAILED at `loop_shortcut.rs:11335`, `left: PlayerId(3)` / `right: PlayerId(0)`, beat 5 — the first answer-beat mint. Arm 2 still fires. * PURE REVERT of the reorder (drop the hoisted sync, restore the raw clone below the record): 1 passed — so the row's own "a pure revert leaves all three arms GREEN" statement remains true under the new comparand, and the arm is still a standing pin rather than a live catch. `engine.rs` restored byte-identically afterwards, verified by sha256. Assisted-by: ClaudeCode:claude-opus-5 CR 603.5 CENSUS PIN, moved because this commit moved it. The line-exact pin in `the_cr_603_5_prompt_census_is_pinned_so_a_sixth_producer_is_a_counted_event` goes `engine.rs:11549 ⇒ :11583`, and the drift-log entry carries the evidence the row's own convention demands: engine.rs's entire delta this round is ONE comment hunk (`@@ -11177,8 +11177,42 @@ fn apply_action`, numstat +42/-8 ⇒ net +34) sitting above the producer, 11549 + 34 = 11583 exactly, the producer line is sha256-identical to `a6d1a0e62:engine.rs:11549` (prefix `8a544e878d3e77fb…`, the same prefix the log already recorded for `:11549`), it is still inside `begin_pending_trigger_target_selection` (which moved by the same +34, :11400 ⇒ :11434), the census total (37) and partition (5/7/25) asserts fired GREEN on the run that caught this, and the other four entries did not move. Not a sixth producer. ONE MORE ROTTED CITATION, same class, fixed LINE-NEUTRALLY. `bounded_cycle_offer`'s basis-A note cited the four sibling ring `find_map` scans as `:481`/`:668`/`:710`/`:808`. Measured at this tree those lines hold `);`, `block,`, `}` and `state,` — meaningless fragments; the real scans are the `On` reconcile, Path B, Path C and `find_live_loop_winner`. That stale citation is what propagated the bogus coordinates into this round's own brief, through two agents, before anyone re-derived them — so the fix is to NAME the four and delete the numbers, which cannot rot the same way. The block grows by one line and the sampler comment was tightened by one to pay for it, keeping the CR 603.5 producer exactly on `:11583`: engine.rs's hunks above it are now `@@ -1924,5 +1924,6 @@` (+1) and `@@ -11177,8 +11178,41 @@` (+33), still 11549 + 34. COUNT RECONCILED, and my first figure was wrong. `drive_loop_action_iteration` dispatches `apply_action` TEN times, not nine. Predicate: `awk` the function body (`fn drive_loop_action_iteration(` at :4062 to the next column-0 `}` at :4319) and `grep -cP 'apply_action\('` it; one unit = one call-expression opening line. The ten are :4087 :4118 :4150 :4173 :4200 :4222 :4231 :4256 :4271 :4303. My "nine" was a transcription miscount off a whole-file grep that had listed :4231 — not a different predicate. Corrected above and in the comment. --- crates/engine/src/analysis/resource.rs | 9 ++ crates/engine/src/game/engine.rs | 82 +++++++++++++++---- .../engine/tests/integration/loop_shortcut.rs | 67 +++++++++++---- 3 files changed, 126 insertions(+), 32 deletions(-) diff --git a/crates/engine/src/analysis/resource.rs b/crates/engine/src/analysis/resource.rs index 6ec4dac558..3af92f7c4a 100644 --- a/crates/engine/src/analysis/resource.rs +++ b/crates/engine/src/analysis/resource.rs @@ -1325,6 +1325,15 @@ fn map_delta( /// normalized. It does not consult a board predicate, but it DOES require the frames it /// compares to be homogeneous in turn position, which is what "homogeneous" above now means /// in full. +/// +/// ⚠ SCOPE OF THAT REQUIREMENT — what this function READS, versus where the frames' sameness +/// comes from. It reads exactly two things: `ResourceVector::snapshot(&f.normalized)` and +/// `window_scope_from_cover_frames(..).phase_invariant`, and `phase_invariant` is +/// `turn_number` + `phase` + `extra_phases.is_empty()`. The sampler gate that mints the frames +/// also makes them homogeneous in `waiting_for`/`priority_player`, but THIS function never +/// looks at those two — basis A does, via `loop_states_equal_modulo_resources` ⇒ +/// `loop_states_equal` ⇒ `impl PartialEq for GameState`. Do not cite `ring_delta_signature` as +/// the consumer of either field. pub(crate) fn ring_delta_signature(state: &GameState) -> Option<(u32, ResourceVector)> { let frames = state.loop_detect_ring.len(); // 2k + 1 with k >= 1. diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 6911c2d464..4ffda22570 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -1921,11 +1921,12 @@ fn bounded_cycle_offer( // (4) CERTIFICATION — two bases, first match wins, NEVER combined. // - // Basis A is a fifth copy of the ring `find_map` scan (`:481` the `On` reconcile, `:668` - // Path B, `:710` Path C, `:808` `find_live_loop_winner`). Recorded, not hidden: the repo - // already made this call at `find_live_loop_winner`'s own doc — "a deliberate, isolated - // copy … the `On` arm stays VERBATIM (byte-identity gate)" — and retargeting the four - // shipped walks would edit byte-identity-gated paths inside a feature commit. Newest + // Basis A is a fifth copy of the ring `find_map` scan. The other four are NAMED, not + // numbered — the `On` reconcile, Path B, Path C, `find_live_loop_winner` — because the + // coordinates that stood here had ROTTED onto unrelated code. Grep the symbols. Recorded, + // not hidden: `find_live_loop_winner`'s own doc already made this call — "a deliberate, + // isolated copy … the `On` arm stays VERBATIM (byte-identity gate)" — and retargeting the + // four shipped walks would edit byte-identity-gated paths inside a feature commit. Newest // prior first: the most recent recurrence is the least extrapolation. // // TWO PARALLEL VECS OVER ONE INDEX SPACE. They are built from the same `VecDeque` in the @@ -11174,14 +11175,47 @@ fn apply_action( // and recomputes `priority_player` through `turn_control`'s authorized-submitter // resolver — and it now runs BEFORE the record, exactly as the settle sampler in // `pass_priority_once_with_pipeline` does. So a frame minted here carries the same - // `waiting_for`/`priority_player` pair a settle frame carries, and that homogeneity - // is load-bearing: `impl PartialEq for GameState` compares both fields and - // `normalize_for_loop` neutralizes neither, so a mixed ring would break - // `ring_delta_signature`'s turn-position conjunct. BLAST RADIUS IS THE RING ONLY — - // `apply_action_boundary` re-syncs the returned `wf` before the result leaves the - // engine, so the settled state is unchanged. MEASURED before the reorder: a - // `debug_assert_eq!` census on both fields reported 0 divergences over 18,486 lib + - // 4,487 integration rows, so this replaces a coincidence with a guarantee. + // `waiting_for`/`priority_player` pair a settle frame carries. + // + // WHICH CONSUMER THAT HOMOGENEITY IS FOR — and it is NOT `ring_delta_signature`. The + // claim shipped in `f144bb374` ("a heterogeneous ring breaks `ring_delta_signature`'s + // turn-position conjunct, because `impl PartialEq for GameState` compares both + // fields") is FALSE at source: that function's body reads only + // `ResourceVector::snapshot(&f.normalized)` and + // `window_scope_from_cover_frames(..).phase_invariant`, and `phase_invariant` is + // `turn_number` + `phase` + `extra_phases.is_empty()`. Neither field is in it. + // + // The consumer that DOES read them is BASIS A — the ring scans that call + // `analysis::resource::loop_states_equal_modulo_resources(prior, state)` with `prior` + // a ring frame's `normalized` half and `state` the LIVE board. That chains to + // `types::game_state::loop_states_equal` ⇒ `impl PartialEq for GameState`, which + // compares BOTH `waiting_for` and `priority_player`, and neither `normalize_for_loop` + // nor `project_out_resources` neutralizes either. An answer-beat frame carrying an + // un-synced pair therefore compares UNEQUAL against a live board that has been + // synced, and basis A silently misses the recurrence the frame was minted to certify. + // + // BLAST RADIUS. Nothing this leaves in `state` survives to a consumer unrecomputed: + // `finish_action_boundary` runs the SAME `sync_waiting_for` over `result.waiting_for` + // (`:1171`) and copies the outcome back into the result (`:1189`), and the reorder + // never changes `ActionResult.waiting_for` itself. That is an argument about + // RE-DERIVATION, not reachability, because `apply_action_boundary` is not the only + // route: `inject_pinned_answer`'s three dispatches and `drive_loop_action_iteration`'s + // ten reach `apply_action` directly, and + // `apply_interaction_pre_reconciliation_for_life_safety` returns `raw.result` without + // ever calling `finish_action_boundary` (`apply_action_boundary_core`'s own comment at + // `:1119` records it). All three drive a CLONE — `drive_one_shortcut_cycle`'s `work`, + // the drive's `clone`, `preview_candidate_life_safety`'s `preview` — never the settled + // board. MEASURED pre-reorder by an instrumented `debug_assert_eq!` census over the + // full lib + integration corpus (per-site counts in `f144bb374`'s message; one unit = + // one `record_loop_detect_sample` invocation): 0 at EITHER sampler where the sync + // changed either field, so this replaces a coincidence with a guarantee. + // + // ONE CONSEQUENCE OF USING THE SYNCHRONIZER RATHER THAN A RAW CLONE: + // `normalize_legacy_attach_waiting_for` can now edit `state.waiting_for` on this + // path, so it may differ from the returned `ActionResult.waiting_for`, where the raw + // clone made the two exactly equal. Benign — the boundary re-normalizes at `:1171` + // and copies back at `:1189`, and `inject_pinned_answer` fails closed on every prompt + // kind it has no pin producer for. sync_waiting_for(state, &wf); if answering_forced_window && !in_simulation_probe() @@ -15457,7 +15491,27 @@ mod stage2_injector_tests { // sha256 `a6d7f2f9d1e15de5…5cb032`, matching `117baa6a1:engine.rs:11549`, and // still inside `begin_pending_trigger_target_selection` (opens :11400 here, // :11271 at `b654513cb`). - "game/engine.rs:11549".to_string(), + // + // THIS PR (the basis-A prose correction), ON TOP OF `a6d1a0e62`: `:11549 ⇒ + // :11583`, +34. LOCAL, not upstream — the CI-vs-local diagnosis in the header + // does not apply. engine.rs's entire delta this round is THREE COMMENT HUNKS + // and nothing else. Two sit ABOVE this producer — `@@ -1924,5 +1924,6 @@` in + // `bounded_cycle_offer` (+1: the rotted `:481/:668/:710/:808` sibling-scan + // coordinates replaced by symbol names) and `@@ -11177,8 +11178,41 @@` in + // `apply_action` (+33) — summing to +34, and 11549 + 34 = 11583 exactly. The + // third is THIS drift entry, below the producer, which therefore cannot move + // it (its own size is deliberately not arithmetic here — a self-counting + // entry restates itself every edit). A comment round cannot mint a prompt, + // and the census agrees — the two asserts above this one fired GREEN on the + // run that caught this (total still 37, partition still 5/7/25) and the other + // four entries did not move (`effects/` is untouched by this commit; + // `scoped_library_search.rs:452` re-read and sha256-confirmed in place). + // Identity re-established rather than assumed: line :11583 is byte-identical + // by sha256 (`8a544e878d3e77fb…`, the SAME prefix this log recorded for + // `:11549`) to `a6d1a0e62:engine.rs:11549`, and it is still inside + // `begin_pending_trigger_target_selection`, which moved by the same +34 + // (opens :11400 ⇒ :11434). + "game/engine.rs:11583".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/tests/integration/loop_shortcut.rs b/crates/engine/tests/integration/loop_shortcut.rs index f716c86b29..8176e2e6da 100644 --- a/crates/engine/tests/integration/loop_shortcut.rs +++ b/crates/engine/tests/integration/loop_shortcut.rs @@ -11163,13 +11163,27 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// `answering_forced_window`) called `record_loop_detect_sample` BEFORE installing the /// pipeline's returned `wf`, while the settle sampler in `pass_priority_once_with_pipeline` /// records AFTER its `sync_waiting_for`. A frame minted at the answer site therefore carried -/// the PRE-pipeline `waiting_for`/`priority_player` pair and a settle frame carried the synced -/// one. That is a detection hazard, not cosmetics: `impl PartialEq for GameState` compares both -/// fields and `normalize_for_loop` neutralizes neither, so a heterogeneous ring breaks -/// `analysis::resource::ring_delta_signature`'s turn-position conjunct. The fix routes `wf` -/// through `game::public_state::sync_waiting_for` — the canonical synchronizer, which also -/// recomputes `priority_player` via `turn_control::authorized_submitter_for_player` — before -/// the record, so both producers mint the same shape. +/// the UN-SYNCED pair — whatever the reducer or `run_post_action_pipeline` last wrote straight +/// into `state.waiting_for`, with `priority_player` never recomputed — while a settle frame +/// carried the synced one. (NOT the "pre-pipeline" pair: the pipeline itself writes +/// `state.waiting_for` at five sites inside `run_post_action_pipeline_from`.) +/// +/// That is a detection hazard, not cosmetics — but the consumer is BASIS A, not +/// `ring_delta_signature`. `f144bb374`'s message and the comments it shipped said a +/// heterogeneous ring breaks `ring_delta_signature`'s turn-position conjunct "because +/// `impl PartialEq for GameState` compares both fields"; that is false at source. That +/// function reads only `ResourceVector::snapshot(&f.normalized)` and +/// `window_scope_from_cover_frames(..).phase_invariant` (= `turn_number` + `phase` + +/// `extra_phases.is_empty()`). The real sensitivity is the ring scans that call +/// `analysis::resource::loop_states_equal_modulo_resources(prior, state)` with `prior` a ring +/// frame's `normalized` half and `state` the live board: that chains to `loop_states_equal` ⇒ +/// `impl PartialEq for GameState`, which DOES compare `waiting_for` and `priority_player`, and +/// neither `normalize_for_loop` nor `project_out_resources` neutralizes either — so an +/// un-synced frame compares UNEQUAL against a synced live board and basis A misses the +/// recurrence. The fix routes `wf` through `game::public_state::sync_waiting_for` — the +/// canonical synchronizer, which also recomputes `priority_player` via +/// `turn_control::authorized_submitter_for_player` — before the record, so both producers mint +/// the same shape. /// /// FIXTURE: the tracked `dina_conqueror_4p` dump, driven through the production `apply()` path, /// so every frame asserted below was minted by `record_loop_detect_sample` itself rather than @@ -11183,10 +11197,11 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// mints (beats 5 and 14), 3 settle mints, offer at beat 19 over a 5-frame ring. /// /// ⚠ WHAT THIS ROW DOES **NOT** CATCH, stated rather than implied. A PURE REVERT of the reorder -/// leaves all three arms GREEN, and that is a measurement rather than an oversight: a -/// `debug_assert_eq!` census on both fields at that position reported 0 divergences over 18,486 -/// lib + 4,487 integration rows, so no fixture in the corpus reaches the divergence. The row is -/// consequently the STANDING pin — it fires the first time a beat does diverge — and its +/// leaves all three arms GREEN, and that is a measurement rather than an oversight: an +/// instrumented `debug_assert_eq!` census on both fields at that position, run on the +/// pre-reorder tree over the full lib + integration corpus, reported 0 divergences (per-site +/// counts in `f144bb374`'s message), so no fixture in the corpus reaches the divergence. The +/// row is consequently the STANDING pin — it fires the first time a beat does diverge — and its /// instrument is proved live by MUTANTS at the sampler instead of by the revert: /// * `state.priority_player = PlayerId(3);` after the sync ⇒ arm (2) FAILS at the first mint. /// * `state.waiting_for = WaitingFor::GameOver { winner: None };` after the sync ⇒ arm (1) @@ -11251,12 +11266,25 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact .live; // ── (2) ITS PRIORITY PLAYER. Asserted FIRST so a mutation that touches only // `priority_player` is caught by its own arm instead of being masked by arm (1). + // + // The comparand is the AUTHORITY FUNCTION, not `frame.active_player`. `sync_waiting_for` + // sets `priority_player = turn_control::authorized_submitter_for_player(state, + // waiting_for.acting_player())`, which re-routes to a DIFFERENT seat whenever a + // turn-decision controller (Mindslaver) or a latched search-decision controller is in + // play. Pinning the seat itself would false-fail a correctly synced frame on any future + // turn-control fixture. The recomputation is not circular: neither + // `effective_authority_for_player` nor `search_decision_authority` reads + // `priority_player`, so a mutant that clobbers only that field still fails here. + let semantic_player = frame + .waiting_for + .acting_player() + .expect("the sampler's gate admits only `Priority{player}`, which has an actor"); assert_eq!( - frame.priority_player, frame.active_player, + frame.priority_player, + engine::game::turn_control::authorized_submitter_for_player(frame, semantic_player), "beat {beat}: `sync_waiting_for` recomputes `priority_player` from the window it \ - installs, so an answer-beat frame must carry the post-sync submitter for the \ - returned `Priority{{active_player}}` window, not whatever the pre-pipeline window \ - left behind" + installs, so an answer-beat frame must carry that window's AUTHORIZED SUBMITTER, \ + not whatever the un-synced state left behind" ); // ── (1) THE NEWEST SAMPLED STATE: the window the action RETURNS, never the forced one // it answered. @@ -11350,8 +11378,11 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact assert_eq!( *delta, expected_delta, "EXACT per-period signature: +1 to the controller, -1 to each opponent, and every \ - other axis at rest. A ring whose frames disagreed on `waiting_for`/`priority_player` \ - could not produce this signature at all, because `ring_delta_signature` compares the \ - frames with `impl PartialEq for GameState`" + other axis at rest. `ring_delta_signature` is INSENSITIVE to \ + `waiting_for`/`priority_player` — it reads resource snapshots plus \ + `phase_invariant` (turn/phase/extra-phases) — so this arm is the blast-radius pin \ + for the reorder, not a restatement of arms (1)/(2). The frame homogeneity those two \ + arms pin is basis A's concern (`loop_states_equal_modulo_resources` ⇒ \ + `impl PartialEq for GameState`)" ); } From 5b4393ee35856d56b6c24916da66a9738ba95176 Mon Sep 17 00:00:00 2001 From: lgray Date: Tue, 4 Aug 2026 23:50:40 -0500 Subject: [PATCH 2/5] test(engine): enforce the answer-beat row's frame attribution instead of measuring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit (Major) on `answer_beat_frames_carry_the_synced_window_and_the_offer_ certificate_is_exact`. The row is a STANDING pin — it exists to fire the first time an answer beat diverges — but its mint detector could hand arms (1)/(2) the wrong frame, in which case the pin goes quietly vacuous instead of firing. Two independent ways, both closed here. No production change; test-only. (1) ATTRIBUTION. A single beat can reach BOTH samplers — `apply_action`'s forced-window answer site, then the settle site in `pass_priority_once_with_pipeline`. The ring then grows by 2, `answered_forced_ window` is true, and `back()` holds the SETTLE frame; arms (1)/(2) pass while inspecting a frame the reorder never touched. `assert_eq!(grew, 1, ..)` on the answer path is the property that makes `back()` the answer-site frame, and `settle_mints += grew` stops the settle path under-counting a double mint. (2) EVICTION. `record_loop_detect_sample` pops the front before pushing once the ring is at `LOOP_DETECT_RING_CAP`, so a full ring mints WITHOUT growing and the old `len() == before ⇒ continue` read that as "no mint" — `answer_mints` would then measure a smaller set than the reach-guard claims. Fixed by detecting a mint through `Arc` identity of the ring's back, which is what `game::engine::drive_one_shortcut_cycle` already does and for this exact reason (its own comment: "a length delta reads 0 once it is full"). Reusing the shipped instrument rather than adding a second one. DELIBERATE DEVIATION, stated rather than slipped in: the brief asked for a headroom assertion against the ring's capacity. The capacity is real and fixed — `LOOP_DETECT_RING_CAP: usize = 16`, `types/game_state.rs:20025` — but it is PRIVATE to that module, so an integration row can only transcribe the literal. A transcribed cap fails in the dangerous direction: if the const ever SHRINKS below the drive's ring length, `len() < 16` still passes while eviction silently resumes. Identity detection has no such failure mode and makes the capacity irrelevant to this row, so the residual `assert!(grew >= 1, ..)` needs no constant: back-changed-but-length-did-not IS "at capacity and evicting", named as such in the message and failing closed. MEASURED, one unit = one emitted probe line = one beat on which the ring's back changed. Temporary `eprintln!` on the line after `grew` is computed, whole drive, `--nocapture`: beat=0 answered_forced=false grew=1 len=1 beat=5 answered_forced=true grew=1 len=2 beat=9 answered_forced=false grew=1 len=3 beat=14 answered_forced=true grew=1 len=4 beat=18 answered_forced=false grew=1 len=5 So `assert_eq!(grew, 1)` is REACHED on both answer beats (5 and 14) and true there — not vacuously skipped — and the 2/3 answer/settle split the row's doc claims is reproduced by the new counters. The probe was removed afterwards and the file restored byte-identically (sha256 `45b33588f08cc6cf…`). HONEST LIMIT on the second assertion: max ring length on this drive is 5 against a cap of 16, so `assert!(grew >= 1)` never trips here and is NOT discriminating on this fixture. That is its job — it is a fail-closed guard against a future fixture whose drive fills the ring, which is exactly the case where the old detector would have gone silent. Assisted-by: ClaudeCode:claude-opus-5 ARM SEPARATION, a defect the arm-1 mutant caught in MY OWN prior round. The `priority_player` comparand added in `e0da32fca` derived its semantic player with `frame.waiting_for.acting_player().expect(..)` placed BEFORE arm (1). A window with no acting player is precisely what arm (1) exists to catch, so the `waiting_for = GameOver { winner: None }` mutant died on the unwrap at `loop_shortcut.rs:11321:14` — arm (1)'s explanation replaced by an unwrap string, and the row's own doc claim that the two arms are separately live silently false. Fixed by making arm (2) an `if let`. The pair is TOTAL, so the skip opens no hole: either arm (2) runs, or the window has no actor and arm (1) fails on that same frame. Both mutants re-run at the final tree, predicate `cargo test -p phase-engine --test integration loop_shortcut::answer_beat_frames_carry_the_synced_window_and_ the_offer_certificate_is_exact -- --exact`, one unit = one test row: * `state.priority_player = PlayerId(3);` ⇒ FAILS ARM (2), `:11327`, beat 5, `left: PlayerId(3)` / `right: PlayerId(0)`, arm (2)'s own message. * `state.waiting_for = WaitingFor::GameOver { winner: None };` ⇒ FAILS ARM (1), `:11337`, beat 5, `left: GameOver { winner: None }` / `right: Priority { player: PlayerId(0) }`, arm (1)'s own message. * clean: 1 passed. `engine.rs` restored byte-identically after both, sha256 `2393d6a2597c1e33…`. A DIFFERENT arm answers each mutant, which is the property "separately live" names; the doc bullet is corrected to say so and to record why arm (2) is an `if let`. CR 603.5 census pin re-derived, not assumed: this commit touches only `tests/integration/loop_shortcut.rs`, and the producer is still `engine.rs:11583` with sha256 prefix `8a544e878d3e77fb` unchanged. --- .../engine/tests/integration/loop_shortcut.rs | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/crates/engine/tests/integration/loop_shortcut.rs b/crates/engine/tests/integration/loop_shortcut.rs index 8176e2e6da..3369a16de0 100644 --- a/crates/engine/tests/integration/loop_shortcut.rs +++ b/crates/engine/tests/integration/loop_shortcut.rs @@ -11196,6 +11196,14 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// ring grew" names the answer site and nothing else. MEASURED on this drive: 2 answer-beat /// mints (beats 5 and 14), 3 settle mints, offer at beat 19 over a 5-frame ring. /// +/// Both halves of that attribution are now ENFORCED in the loop rather than left to today's +/// fixture, because a standing pin that rests on a measurement goes vacuous the moment the +/// measurement changes. A mint is detected by `Arc` identity of the ring's back, so an +/// evicting push at `LOOP_DETECT_RING_CAP` cannot masquerade as "no mint" and shrink the set +/// `answer_mints` claims to cover; and an answer beat must mint EXACTLY ONE frame, because a +/// beat that reaches both samplers leaves `back()` holding the SETTLE frame and arms (1)/(2) +/// would then pass while inspecting a frame the reorder never touched. +/// /// ⚠ WHAT THIS ROW DOES **NOT** CATCH, stated rather than implied. A PURE REVERT of the reorder /// leaves all three arms GREEN, and that is a measurement rather than an oversight: an /// instrumented `debug_assert_eq!` census on both fields at that position, run on the @@ -11203,9 +11211,15 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// counts in `f144bb374`'s message), so no fixture in the corpus reaches the divergence. The /// row is consequently the STANDING pin — it fires the first time a beat does diverge — and its /// instrument is proved live by MUTANTS at the sampler instead of by the revert: -/// * `state.priority_player = PlayerId(3);` after the sync ⇒ arm (2) FAILS at the first mint. +/// * `state.priority_player = PlayerId(3);` after the sync ⇒ arm (2) FAILS at the first mint +/// (beat 5), `PlayerId(3)` vs `PlayerId(0)`, with arm (2)'s own message; arm (1) is never +/// reached. /// * `state.waiting_for = WaitingFor::GameOver { winner: None };` after the sync ⇒ arm (1) -/// FAILS at the first mint (arm (2) still passes, so the two arms are separately live). +/// FAILS at the same beat, `GameOver { winner: None }` vs `Priority { player: PlayerId(0) }`, +/// with arm (1)'s own message. Arm (2) is SKIPPED there rather than passed — `GameOver` has +/// no acting player — which is exactly why arm (2) is an `if let` and not an unwrap: an +/// unwrap would panic on the `None` and replace arm (1)'s explanation with its own. Each +/// mutant is answered by a DIFFERENT arm, which is what "separately live" means. /// /// Arm (3) is the BLAST-RADIUS pin: the certificate is byte-exact under the pure revert, which /// is what makes "this reorder does not perturb detection" a measurement. @@ -11248,16 +11262,48 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact } let answered_forced_window = state.waiting_for.is_forced_cascade_window(); let before = state.loop_detect_ring.len(); + // A MINT IS DETECTED BY `Arc` IDENTITY OF THE RING'S BACK, NOT BY A LENGTH DELTA. + // `record_loop_detect_sample` pops the front before pushing once the ring is at + // `LOOP_DETECT_RING_CAP`, so a full ring mints WITHOUT growing and a length-only + // detector reads that as "no mint" — the beat would take the `continue` below and + // `answer_mints` would silently under-count the set this row claims to have checked. + // `game::engine::drive_one_shortcut_cycle` uses identity here for exactly this reason; + // this is that instrument, not a second one. Identity also makes the ring's capacity + // irrelevant to the row, which is why no literal cap is transcribed here (the const is + // private to `types::game_state`, so a transcribed copy could only rot). + let back_before = state.loop_detect_ring.back().map(std::sync::Arc::as_ptr); if dump_drive_one_beat(&mut state, pin).is_err() { break; } - if state.loop_detect_ring.len() == before { + if state.loop_detect_ring.back().map(std::sync::Arc::as_ptr) == back_before { continue; } + let grew = state.loop_detect_ring.len() - before; + assert!( + grew >= 1, + "beat {beat}: the ring's back changed while its length did not, which means the \ + ring is AT CAPACITY and evicting. `grew` can no longer attribute frames to a \ + sampling site, so every count below would be measuring a smaller set than it \ + names. Fail closed rather than report a number this instrument cannot support" + ); if !answered_forced_window { - settle_mints += 1; + settle_mints += grew; continue; } + // ATTRIBUTION, ENFORCED RATHER THAN OBSERVED. A single beat can reach BOTH samplers — + // `apply_action`'s forced-window answer site and then the settle site in + // `pass_priority_once_with_pipeline` — in which case the ring grows by 2 and `back()` + // is the SETTLE frame. Arms (1) and (2) would then pass while inspecting a frame the + // reorder never touched, i.e. this standing pin would go quietly vacuous. `grew == 1` + // is the property that makes `back()` the answer-beat frame; nothing else here asserts + // it, and "measured as 1 on today's fixture" is not what a standing pin rests on. + assert_eq!( + grew, 1, + "beat {beat}: this beat answered a forced window AND minted {grew} frames, so \ + `back()` is not necessarily the answer-site frame — arms (1)/(2) below would be \ + inspecting whichever sampler recorded LAST. Attribute per site before asserting \ + on a frame; do not relax this to `>= 1`" + ); answer_mints += 1; let frame = &state .loop_detect_ring @@ -11275,17 +11321,23 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact // turn-control fixture. The recomputation is not circular: neither // `effective_authority_for_player` nor `search_decision_authority` reads // `priority_player`, so a mutant that clobbers only that field still fails here. - let semantic_player = frame - .waiting_for - .acting_player() - .expect("the sampler's gate admits only `Priority{player}`, which has an actor"); - assert_eq!( - frame.priority_player, - engine::game::turn_control::authorized_submitter_for_player(frame, semantic_player), - "beat {beat}: `sync_waiting_for` recomputes `priority_player` from the window it \ - installs, so an answer-beat frame must carry that window's AUTHORIZED SUBMITTER, \ - not whatever the un-synced state left behind" - ); + // + // `if let` rather than `expect`, and the difference is MEASURED: an `expect` here + // pre-empts arm (1). A window with no acting player (`GameOver`) is precisely what + // arm (1) exists to catch, so panicking on the `None` before reaching it replaces + // arm (1)'s explanation with an unwrap message and destroys the arms' separation — + // the `waiting_for = GameOver` mutant died on the unwrap instead of on arm (1). The + // pair stays TOTAL, so this skip opens no hole: either arm (2) runs, or the window + // had no actor and arm (1) below fails on that same frame. + if let Some(semantic_player) = frame.waiting_for.acting_player() { + assert_eq!( + frame.priority_player, + engine::game::turn_control::authorized_submitter_for_player(frame, semantic_player), + "beat {beat}: `sync_waiting_for` recomputes `priority_player` from the window \ + it installs, so an answer-beat frame must carry that window's AUTHORIZED \ + SUBMITTER, not whatever the un-synced state left behind" + ); + } // ── (1) THE NEWEST SAMPLED STATE: the window the action RETURNS, never the forced one // it answered. assert_eq!( From 04685ebe6f35003784be9abf5800ace39d6cd4ba Mon Sep 17 00:00:00 2001 From: lgray Date: Wed, 5 Aug 2026 00:49:57 -0500 Subject: [PATCH 3/5] test(engine): assert the answer-beat arms over every frame a beat mints, not over `back()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up review on `e7e50229b`. That commit fixed a vacuity in this row and introduced a NEW false claim of the same shape as the one the branch exists to correct — a locally true statement generalised one step too far. Test-only; no production behaviour change. THE FALSE CLAIM, stated before it is fixed. `e7e50229b`'s comment said `grew == 1` "is the property that makes `back()` the answer-beat frame". It is NECESSARY, not sufficient. The row's `answered_forced_window` is `is_forced_cascade_window()` read BEFORE the beat, i.e. ONE conjunct of the production answer-site gate, which also requires `!in_simulation_probe()`, `loop_detection.samples()`, `!stack.is_empty()`, `stack.len() >= stack_len_before_action`, and `Priority{player == active_player}`. One test beat is one `apply()`, and `apply()` reaches the SETTLE sampler after `apply_action` returns. So on a beat where answering the forced window resolves the last stack entry — `!stack.is_empty()` false, answer site gated OFF — and the refill cascade mints one settle frame, `grew == 1` holds, `answer_mints` increments, and arms (1)/(2) inspect a SETTLE frame while the row claims an answer frame. That is precisely the class this row targets: a self-refilling drain cascade. The predecessor paragraph "SITE ATTRIBUTION IS EXACT, not assumed" is wrong for the same reason and is corrected in place, quoting itself. FIXED AT THE ROOT rather than by asserting harder. Arms (1) and (2) now run over `loop_detect_ring.iter().rev().take(grew)` — every frame the beat added, on every minting beat — so attribution stops mattering instead of getting sharper. Sound because BOTH samplers gate their record on `Priority{player == active_player}` (`engine.rs` answer site; `pass_priority_once_with_pipeline` settle site) and both record after their own `sync_waiting_for`: a frame failing either arm is a real defect whichever site minted it. `grew == 1` is therefore no longer asserted at all — what remains of the mint accounting is `Arc`-identity detection and `grew >= 1`, and the comment now says only what each actually rules out. MINT DETECTOR: the production `is_some()` guard restored. `drive_one_shortcut_cycle` uses `ring_back_after.is_some() && ring_back_after != ring_back_before`; `e7e50229b` kept only the inequality. The settle sampler's `else` arm calls `loop_detect_ring.clear()`, so `back()` can go to `None` inside a beat — then `None != Some(..)` fell through to `len() - before`, which underflows and panics "attempt to subtract with overflow" (dev profile leaves `overflow-checks` at its `true` default) instead of this row's own explanation. Now: `is_some()` skips a cleared-and-not-repushed beat as the non-mint it is, `saturating_sub` cannot underflow, and the `grew >= 1` message names BOTH causes — at-capacity eviction AND clear-then-repush — because `e7e50229b`'s message asserted "AT CAPACITY", which is false for the clear case. `settle_mints` NOW FEEDS AN ASSERTION. `e7e50229b` incremented it and reached only an interpolated message while its commit message listed it among vacuities "both closed here" — an overclaim. It is now the reach-guard for the widening (`settle_mints > 0`), which is the conjunct that proves the settle-frame coverage was actually exercised. The `answer_mints` guard's message is corrected to say it is a reach signal over ONE gate conjunct, not proof of which sampler minted. CITATIONS DE-ROTTED, same class as this branch's `bounded_cycle_offer` fix two hunks away. Four in-tree references named `f144bb374`, which the user's rebase orphaned: `git merge-base --is-ancestor f144bb374 HEAD` exits 1. Two of the four sat in a PERMANENT PRODUCTION COMMENT, and under squash-merge neither that SHA nor `a6d1a0e62` survives into main. All four now cite `#7005`, which is durable and was already named alongside them. EVIDENCE. Predicate for every row below: `cargo test -p phase-engine --test integration loop_shortcut::answer_beat_frames_carry_the_synced_window_and_the_ offer_certificate_is_exact -- --exact`; one unit = one test row. Reachability probe, temporary `eprintln!` after `grew`, `--nocapture`, one unit = one minting beat: beat=0 forced=false grew=1 len=1 inspected=1 beat=5 forced=true grew=1 len=2 inspected=1 beat=9 forced=false grew=1 len=3 inspected=1 beat=14 forced=true grew=1 len=4 inspected=1 beat=18 forced=false grew=1 len=5 inspected=1 HONEST LIMIT: `grew > 1` never occurs on this fixture, so the `take(grew)` slice is always ONE frame and the widened loop CANNOT be shown here inspecting several frames from a single beat. What IS measurable is the other half of the widening — arms now run on the 3 non-forced beats as well as the 2 forced ones, 5 inspected frames against 2 before — and that half is proved two-sided below. Mutants at the final tree, `engine.rs` restored byte-identically after each (file sha256 `ed92a21b6715739f…`; `loop_shortcut.rs` `9684389b859345c0…`, both re-measured against the COMMITTED content after the `saturating_sub` fix, not a WIP tree): * `state.priority_player = PlayerId(3);` after the answer-site sync ⇒ ARM (2), `loop_shortcut.rs:11352`, beat 5, `left: PlayerId(3)` / `right: PlayerId(0)`. * `state.waiting_for = WaitingFor::GameOver { winner: None };` same place ⇒ ARM (1), `:11365`, beat 5, `left: GameOver { winner: None }` / `right: Priority { player: PlayerId(0) }`. A DIFFERENT arm answers each. * NEW, the widening's own two-sided control — `state.priority_player = PlayerId(3);` immediately before the SETTLE sampler's `record_loop_detect_sample()` ⇒ ARM (2), `:11352`, BEAT 0, a beat the probe measured as `forced=false`. TRIVIALIZE arm: the same mutant with the arms narrowed back to forced beats only (one-line `continue`, the pre-widening scope) PASSES. So the widening is measured coverage, not decoration — it catches a settle-frame defect that the predecessor could not see. Both files restored byte-identically after every run. CENSUS PIN re-derived, not assumed: `engine.rs`'s two citation edits are 1:1 line substitutions, the producer is still `engine.rs:11583`, sha256 prefix `8a544e878d3e77fb` unchanged, pin literal unchanged. THREE CORRECTIONS TO `e7e50229b`'s OWN MESSAGE, since under squash-merge that text lands. They are corrections, not restatements: 1. It cited the mutant failures at `loop_shortcut.rs:11327` and `:11337`. Those were measured BEFORE a subsequent doc edit inserted 6 lines above them, and the message was written after. The committed positions were `:11333`/`:11343`; at THIS tree they are `:11352`/`:11365` as measured above. The lesson is the one this branch keeps re-learning: a coordinate measured before a later edit is not a coordinate. 2. It listed `drive_loop_action_iteration`'s ten dispatches as `:4087 :4118 :4150 :4173 :4200 :4222 :4231 :4256 :4271 :4303`. Each is exactly 1 low — the same commit's `bounded_cycle_offer` fix added one line above the function. Re-derived at this tree (`fn` spans `:4063`..`:4320`): `:4088 :4119 :4151 :4174 :4201 :4223 :4232 :4257 :4272 :4304`. The COUNT, 10, re-derives exactly; only the coordinates were wrong. 3. It gave the restored `loop_shortcut.rs` as sha256 `45b33588f08cc6cf…` without saying what that digest was of. `git cat-file -t` reports it is not a valid object, and it never could be: it is `sha256sum` of the working-tree FILE, not a git blob id (git hashes a header plus content). It also described a WIP tree — the file was edited again before the commit — so it corresponds to no committed state either. Restoration digests in this message are labelled as file digests and were taken against the committed content. CLIPPY CAUGHT ONE OF MINE, recorded because the process point matters more than the token. The first cut of the underflow guard was `after_len.checked_sub(before).unwrap_or(0)`, which `cargo test` compiles happily and `clippy::manual_saturating_arithmetic` rejects under `-D warnings` (`loop_shortcut.rs:11297`). I had verified the change with targeted `cargo test` runs before committing and let the battery find it — the battery did its job, but the cheap gate belonged before the commit, not after. Now `saturating_sub`, which is what the guard meant: 0 on underflow, and the `grew >= 1` assertion below turns that 0 into a named failure. Assisted-by: ClaudeCode:claude-opus-5 --- crates/engine/src/game/engine.rs | 4 +- .../engine/tests/integration/loop_shortcut.rs | 193 ++++++++++-------- 2 files changed, 115 insertions(+), 82 deletions(-) diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 4ffda22570..bcd4597b7a 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -11178,7 +11178,7 @@ fn apply_action( // `waiting_for`/`priority_player` pair a settle frame carries. // // WHICH CONSUMER THAT HOMOGENEITY IS FOR — and it is NOT `ring_delta_signature`. The - // claim shipped in `f144bb374` ("a heterogeneous ring breaks `ring_delta_signature`'s + // claim #7005's first commit shipped ("a heterogeneous ring breaks `ring_delta_signature`'s // turn-position conjunct, because `impl PartialEq for GameState` compares both // fields") is FALSE at source: that function's body reads only // `ResourceVector::snapshot(&f.normalized)` and @@ -11206,7 +11206,7 @@ fn apply_action( // `:1119` records it). All three drive a CLONE — `drive_one_shortcut_cycle`'s `work`, // the drive's `clone`, `preview_candidate_life_safety`'s `preview` — never the settled // board. MEASURED pre-reorder by an instrumented `debug_assert_eq!` census over the - // full lib + integration corpus (per-site counts in `f144bb374`'s message; one unit = + // full lib + integration corpus (per-site counts in PR #7005's history; one unit = // one `record_loop_detect_sample` invocation): 0 at EITHER sampler where the sync // changed either field, so this replaces a coincidence with a guarantee. // diff --git a/crates/engine/tests/integration/loop_shortcut.rs b/crates/engine/tests/integration/loop_shortcut.rs index 3369a16de0..9bb8fd4241 100644 --- a/crates/engine/tests/integration/loop_shortcut.rs +++ b/crates/engine/tests/integration/loop_shortcut.rs @@ -11169,7 +11169,7 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// `state.waiting_for` at five sites inside `run_post_action_pipeline_from`.) /// /// That is a detection hazard, not cosmetics — but the consumer is BASIS A, not -/// `ring_delta_signature`. `f144bb374`'s message and the comments it shipped said a +/// `ring_delta_signature`. PR #7005's first commit and the comments it shipped said a /// heterogeneous ring breaks `ring_delta_signature`'s turn-position conjunct "because /// `impl PartialEq for GameState` compares both fields"; that is false at source. That /// function reads only `ResourceVector::snapshot(&f.normalized)` and @@ -11189,26 +11189,35 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// so every frame asserted below was minted by `record_loop_detect_sample` itself rather than /// staged by the test. /// -/// SITE ATTRIBUTION IS EXACT, not assumed. The answer site's own gate conjunct is -/// `state.waiting_for.is_forced_cascade_window()` read BEFORE the action; the settle sampler is -/// reachable only from `pass_priority_once_with_pipeline`, i.e. from a `Priority` window, which -/// `is_forced_cascade_window` deliberately excludes. So "the pre-beat window was forced AND the -/// ring grew" names the answer site and nothing else. MEASURED on this drive: 2 answer-beat -/// mints (beats 5 and 14), 3 settle mints, offer at beat 19 over a 5-frame ring. -/// -/// Both halves of that attribution are now ENFORCED in the loop rather than left to today's -/// fixture, because a standing pin that rests on a measurement goes vacuous the moment the -/// measurement changes. A mint is detected by `Arc` identity of the ring's back, so an -/// evicting push at `LOOP_DETECT_RING_CAP` cannot masquerade as "no mint" and shrink the set -/// `answer_mints` claims to cover; and an answer beat must mint EXACTLY ONE frame, because a -/// beat that reaches both samplers leaves `back()` holding the SETTLE frame and arms (1)/(2) -/// would then pass while inspecting a frame the reorder never touched. +/// SITE ATTRIBUTION IS DELIBERATELY NOT ATTEMPTED, and the paragraph that stood here claiming +/// it was "exact" was WRONG. It argued that because the settle sampler is reached only from +/// `pass_priority_once_with_pipeline` — i.e. from a `Priority` window, which +/// `is_forced_cascade_window` excludes — "the pre-beat window was forced AND the ring grew" +/// names the answer site and nothing else. That conflates the window BEFORE the beat with the +/// window at the settle sampler's own moment. One beat here is one `apply()`, which reaches the +/// settle sampler AFTER `apply_action` returns, so a beat whose pre-beat window was forced can +/// perfectly well mint at the SETTLE site — e.g. when answering it resolves the last stack +/// entry, gating the answer site off on `!stack.is_empty()` while the refill cascade settles. +/// `answered_forced_window` is ONE conjunct of the production answer-site gate (which also +/// demands `!in_simulation_probe()`, `loop_detection.samples()`, `!stack.is_empty()`, a +/// non-shrinking stack, and `Priority{player == active_player}`), so it is a REACH signal and +/// never an attribution. +/// +/// The row therefore asserts arms (1) and (2) over EVERY frame a beat added +/// (`loop_detect_ring.iter().rev().take(grew)`), which makes attribution irrelevant rather than +/// sharper: both samplers gate their record on `Priority{player == active_player}` and both +/// record after their own `sync_waiting_for`, so a frame that fails either arm is a real defect +/// whichever site minted it. What survives of the mint accounting is narrow and stated as such: +/// `Arc`-identity detection so an evicting push at `LOOP_DETECT_RING_CAP` cannot masquerade as +/// "no mint", and `grew >= 1` so a beat whose length did not grow fails closed instead of +/// mis-sizing the slice. MEASURED on this drive: 2 forced-window minting beats (5, 14), 3 +/// non-forced ones (0, 9, 18), every beat `grew == 1`, offer at beat 19 over a 5-frame ring. /// /// ⚠ WHAT THIS ROW DOES **NOT** CATCH, stated rather than implied. A PURE REVERT of the reorder /// leaves all three arms GREEN, and that is a measurement rather than an oversight: an /// instrumented `debug_assert_eq!` census on both fields at that position, run on the /// pre-reorder tree over the full lib + integration corpus, reported 0 divergences (per-site -/// counts in `f144bb374`'s message), so no fixture in the corpus reaches the divergence. The +/// counts in PR #7005's history), so no fixture in the corpus reaches the divergence. The /// row is consequently the STANDING pin — it fires the first time a beat does diverge — and its /// instrument is proved live by MUTANTS at the sampler instead of by the revert: /// * `state.priority_player = PlayerId(3);` after the sync ⇒ arm (2) FAILS at the first mint @@ -11275,87 +11284,111 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact if dump_drive_one_beat(&mut state, pin).is_err() { break; } - if state.loop_detect_ring.back().map(std::sync::Arc::as_ptr) == back_before { + // `is_some() &&` is the production instrument's guard, kept rather than trimmed: the + // settle sampler's `else` arm calls `loop_detect_ring.clear()`, so `back()` can go to + // `None` within a beat. Without it, `None != Some(..)` falls through to a `usize` + // subtraction that panics with "attempt to subtract with overflow" instead of this + // row's own explanation. A cleared ring minted nothing, so the beat is skipped. + let back_after = state.loop_detect_ring.back().map(std::sync::Arc::as_ptr); + if back_after.is_none() || back_after == back_before { continue; } - let grew = state.loop_detect_ring.len() - before; + let after_len = state.loop_detect_ring.len(); + let grew = after_len.saturating_sub(before); assert!( grew >= 1, - "beat {beat}: the ring's back changed while its length did not, which means the \ - ring is AT CAPACITY and evicting. `grew` can no longer attribute frames to a \ - sampling site, so every count below would be measuring a smaller set than it \ - names. Fail closed rather than report a number this instrument cannot support" + "beat {beat}: the ring's back changed but its length did not grow ({before} -> \ + {after_len}). Either the ring is AT CAPACITY and evicting, or it was CLEARED and \ + re-pushed inside the beat (the settle sampler's `else` arm does exactly that). \ + Either way `grew` cannot say how many frames THIS beat added, so the arms below \ + would inspect the wrong set. Fail closed rather than report a number this \ + instrument cannot support" ); - if !answered_forced_window { + if answered_forced_window { + answer_mints += 1; + } else { settle_mints += grew; - continue; } - // ATTRIBUTION, ENFORCED RATHER THAN OBSERVED. A single beat can reach BOTH samplers — - // `apply_action`'s forced-window answer site and then the settle site in - // `pass_priority_once_with_pipeline` — in which case the ring grows by 2 and `back()` - // is the SETTLE frame. Arms (1) and (2) would then pass while inspecting a frame the - // reorder never touched, i.e. this standing pin would go quietly vacuous. `grew == 1` - // is the property that makes `back()` the answer-beat frame; nothing else here asserts - // it, and "measured as 1 on today's fixture" is not what a standing pin rests on. - assert_eq!( - grew, 1, - "beat {beat}: this beat answered a forced window AND minted {grew} frames, so \ - `back()` is not necessarily the answer-site frame — arms (1)/(2) below would be \ - inspecting whichever sampler recorded LAST. Attribute per site before asserting \ - on a frame; do not relax this to `>= 1`" - ); - answer_mints += 1; - let frame = &state - .loop_detect_ring - .back() - .expect("the ring just grew, so it has a back element") - .live; - // ── (2) ITS PRIORITY PLAYER. Asserted FIRST so a mutation that touches only - // `priority_player` is caught by its own arm instead of being masked by arm (1). + // BOTH ARMS RUN OVER EVERY FRAME THIS BEAT ADDED, which is what makes site attribution + // stop mattering — and the row deliberately does NOT try to attribute. // - // The comparand is the AUTHORITY FUNCTION, not `frame.active_player`. `sync_waiting_for` - // sets `priority_player = turn_control::authorized_submitter_for_player(state, - // waiting_for.acting_player())`, which re-routes to a DIFFERENT seat whenever a - // turn-decision controller (Mindslaver) or a latched search-decision controller is in - // play. Pinning the seat itself would false-fail a correctly synced frame on any future - // turn-control fixture. The recomputation is not circular: neither - // `effective_authority_for_player` nor `search_decision_authority` reads - // `priority_player`, so a mutant that clobbers only that field still fails here. + // `answered_forced_window` is `is_forced_cascade_window()` read BEFORE the beat, i.e. + // strictly ONE conjunct of the production answer-site gate; that gate also demands + // `!in_simulation_probe()`, `loop_detection.samples()`, `!stack.is_empty()`, a + // non-shrinking stack, and `Priority{player == active_player}`. One beat here is one + // `apply()`, which reaches the SETTLE sampler after `apply_action` returns. So a beat + // that answers a forced window resolving the LAST stack entry gates the answer site + // off (`!stack.is_empty()` false) while the refill cascade mints a settle frame: + // `grew == 1`, `answer_mints` increments, and a `back()`-only check would inspect a + // settle frame while claiming an answer frame. `grew == 1` rules out the DOUBLE-MINT + // beat and nothing more — it was never evidence about WHICH sampler minted. // - // `if let` rather than `expect`, and the difference is MEASURED: an `expect` here - // pre-empts arm (1). A window with no acting player (`GameOver`) is precisely what - // arm (1) exists to catch, so panicking on the `None` before reaching it replaces - // arm (1)'s explanation with an unwrap message and destroys the arms' separation — - // the `waiting_for = GameOver` mutant died on the unwrap instead of on arm (1). The - // pair stays TOTAL, so this skip opens no hole: either arm (2) runs, or the window - // had no actor and arm (1) below fails on that same frame. - if let Some(semantic_player) = frame.waiting_for.acting_player() { - assert_eq!( - frame.priority_player, - engine::game::turn_control::authorized_submitter_for_player(frame, semantic_player), - "beat {beat}: `sync_waiting_for` recomputes `priority_player` from the window \ + // Iterating the added frames removes the question instead of sharpening it. Both + // samplers gate their record on `Priority{player == active_player}` and both record + // after their `sync_waiting_for`, so both arms hold for EITHER site's frame; a frame + // that fails one is a real defect no matter which sampler produced it. + for sample in state.loop_detect_ring.iter().rev().take(grew) { + let frame = &sample.live; + // ── (2) ITS PRIORITY PLAYER. Asserted FIRST so a mutation that touches only + // `priority_player` is caught by its own arm instead of being masked by arm (1). + // + // The comparand is the AUTHORITY FUNCTION, not `frame.active_player`. `sync_waiting_for` + // sets `priority_player = turn_control::authorized_submitter_for_player(state, + // waiting_for.acting_player())`, which re-routes to a DIFFERENT seat whenever a + // turn-decision controller (Mindslaver) or a latched search-decision controller is in + // play. Pinning the seat itself would false-fail a correctly synced frame on any future + // turn-control fixture. The recomputation is not circular: neither + // `effective_authority_for_player` nor `search_decision_authority` reads + // `priority_player`, so a mutant that clobbers only that field still fails here. + // + // `if let` rather than `expect`, and the difference is MEASURED: an `expect` here + // pre-empts arm (1). A window with no acting player (`GameOver`) is precisely what + // arm (1) exists to catch, so panicking on the `None` before reaching it replaces + // arm (1)'s explanation with an unwrap message and destroys the arms' separation — + // the `waiting_for = GameOver` mutant died on the unwrap instead of on arm (1). The + // pair stays TOTAL, so this skip opens no hole: either arm (2) runs, or the window + // had no actor and arm (1) below fails on that same frame. + if let Some(semantic_player) = frame.waiting_for.acting_player() { + assert_eq!( + frame.priority_player, + engine::game::turn_control::authorized_submitter_for_player( + frame, + semantic_player + ), + "beat {beat}: `sync_waiting_for` recomputes `priority_player` from the window \ it installs, so an answer-beat frame must carry that window's AUTHORIZED \ SUBMITTER, not whatever the un-synced state left behind" - ); - } - // ── (1) THE NEWEST SAMPLED STATE: the window the action RETURNS, never the forced one - // it answered. - assert_eq!( - frame.waiting_for, - WaitingFor::Priority { - player: frame.active_player - }, - "beat {beat}: the sampler's own gate requires the RETURNED `wf` to be \ + ); + } + // ── (1) THE NEWEST SAMPLED STATE: the window the action RETURNS, never the forced one + // it answered. + assert_eq!( + frame.waiting_for, + WaitingFor::Priority { + player: frame.active_player + }, + "beat {beat}: the sampler's own gate requires the RETURNED `wf` to be \ `Priority{{active_player}}`, so recording before the sync is the only way the \ frame can carry a different window — and `impl PartialEq for GameState` compares it" - ); + ); + } } assert!( answer_mints > 0, - "reach-guard: the drive must mint at least one frame at the FORCED-WINDOW ANSWER site, \ - else arms (1)/(2) never ran and this row passes vacuously; got answer={answer_mints} \ - settle={settle_mints}" + "reach-guard: the drive must reach at least one MINTING beat whose pre-beat window was \ + FORCED, else the answer-site path this row exists for was never exercised and it \ + passes vacuously. Stated exactly: this counts beats whose PRE-beat window satisfied \ + `is_forced_cascade_window()`, which is ONE conjunct of the production answer-site \ + gate, so it is a reach guard and NOT proof that the answer sampler is what minted; \ + got answer={answer_mints} settle={settle_mints}" + ); + assert!( + settle_mints > 0, + "reach-guard for the WIDENING: arms (1)/(2) now run over every frame a beat added, \ + from either sampler, so the drive must also mint at a non-forced (settle) beat — \ + otherwise the settle-frame coverage this row claims is untested. got \ + answer={answer_mints} settle={settle_mints}" ); let offer_beat = offer_beat.expect( "reach-guard: the bounded offer must FIRE on this real 4p drain, else arm (3) asserts \ From b90042d66e5badf3fa9837c3bd6c04488f984ad6 Mon Sep 17 00:00:00 2001 From: lgray Date: Wed, 5 Aug 2026 06:19:37 -0500 Subject: [PATCH 4/5] test(engine): read a beat's minted frames as ring membership, and reach both routes a scalar cannot name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer review on #7023 (matthewevans, CHANGES_REQUESTED at `483ce27d3`), confirming CodeRabbit's still-open Major. Test-only; no production behaviour change. Both claims were re-verified at source before any edit, and both are true. THE DEFECT, stated before it is fixed. The row inferred a SET of minted frames from a SCALAR: it detected a changed `Arc::as_ptr` on the ring's back, computed `grew = after_len - before_len`, and validated `rev().take(grew)`. Two production routes break that inference. (1) EVICTION AT CAPACITY. `GameState::record_loop_detect_sample` calls `pop_front()` and THEN `push_back()` once `loop_detect_ring.len() == LOOP_DETECT_RING_CAP`. The back changes while the length stays equal, so `grew == 0` and the row's `assert!(grew >= 1, ..)` FAILED THE TEST on a legitimate beat. That guard was labelled "fail closed"; the label was wrong about which side of the line the beat is on, and it has gone with the code. (2) CLEAR-AND-REBUILD. `apply_action` clears the ring at the TOP of the beat for any action that is neither `PassPriority` nor `OrderTriggers` answering a non-forced window, and a sampler then mints LATER in the same `apply()`. Net growth is `minted - cleared`, so `take(grew)` silently skips frames the row claims to validate. The sentence "a cleared ring minted nothing, so the beat is skipped" was untrue for exactly this beat and is also gone. FIXED BY DELETING THE SCALAR, not by re-deriving it more carefully. The row snapshots the COMPLETE pre-beat membership of `loop_detect_ring` and validates every remaining frame whose identity was absent from that snapshot. Append, eviction and clear-and-rebuild are then covered uniformly, the ring's capacity stops mattering to the row, and `grew` appears in no assertion. The per-frame validation logic (arms (1)/(2)) is untouched — only how its frame set is obtained. The snapshot holds `Arc` CLONES, not raw addresses, and that is load-bearing rather than defensive: `pop_front` DROPS the evicted allocation before `Arc::new` claims a new one of identical layout, so an address-keyed snapshot can be aliased by the allocator handing the freed block straight back, and a genuinely new frame would then read as an old one. A retained strong reference makes every snapshotted address un-reusable for the beat, so `ptr_eq` is exact by construction. (Aliasing was not observed on these drives — 0 over the `dellian` drive — which is why the reason is construction and not a measured near-miss.) REACHABILITY SHIPS WITH THE FIX rather than after it. The row's own `dina_conqueror_4p` drive reaches NEITHER route — measured: max ring 5 against a capacity of 16, 0 evicting beats, 0 clearing beats — so correcting the detector alone would have re-opened the evidential hole that sent the PR back. Two new rows put each route on a board where it is live, both driven through production `apply()`: * `an_evicting_beat_mints_without_growing_the_ring` — `dellian_emblem_conqueror_4p`, ordinary pass-the-drain drive. MEASURED: beat 72, ring 16 -> 16, 1 minted, 1 dropped, net growth 0; three more evicting beats inside the first 90. * `a_clearing_beat_rebuilds_the_ring_inside_the_same_beat` — `dina_conqueror_4p`, the same board plus one `GameAction::SetAutoPass { UntilStackEmpty }`, which is the exact payload the client's Arena-style "Resolve All" control dispatches. MEASURED: beat 6, ring 2 -> 1, 2 dropped, 1 minted, net growth 0 against a real mint. `SetAutoPass` has to be dispatched by name because it is absent from `ai_support::legal_actions_for_viewer` (`classify_flat_priority_action` files it with the preference-propagation actions), so the generic dump driver never picks it. A survey of the tracked 4p dumps under the driver's own policy found no natural clear-and-rebuild beat: on `fantastic_four_bounded_loop_4p` every non-`Priority` window met with a non-empty ring is a FORCED one, which is precisely the class `apply_action`'s clear exempts. Both new rows SEARCH ON A STRUCTURAL PREDICATE AND ASSERT THE CONSEQUENCE. The witness is the first beat that lost a pre-beat frame without the ring shrinking (ⓔ) or that lost every pre-beat frame (ⓒ) — statements about membership alone — and what is then asserted is what the beat minted and what its length did. A witness selected on "minted while the length stood still" would have carried its own conclusion into the arm that claims to test it. ALSO ADDED, and it is a reach guard rather than decoration: `frames_validated == ring_at_offer` in the answer-beat row. Every frame the offer's ring holds must have reached arms (1)/(2) as a member of some beat's minted set, so a detector that silently returned the EMPTY set on a minting beat — exactly what a length delta returns at capacity — can no longer pass quietly. DRIVE COST. `drive_one_beat_passing_fast` takes `dump_drive_one_beat`'s `Priority` arm directly: that policy is unconditionally "pass", and routing it through the per-viewer enumerator is the dominant cost of a long drive on the 152-entry `dellian` stack. `apply` performs the real legality check itself, so nothing is skipped but the enumeration. Measured equivalence, not assumed: the eviction row's own mutant panics name beat 72 — the same first evicting beat the enumerator-driven probe measured. The 120-beat dellian drive fell from ~9 min to ~2 min. MUTANTS, per assertion, two-sided, each pair flipping its own named assertion. PRISTINE controls (comment-only rebuild) pass for all three rows; the harness reports SETUP-ERROR unless libtest executed exactly one test, after a first run produced two vacuous PASSes from `--exact` against an unqualified test name. frames_validated DROP `.skip(1)` on the post-beat ring -> FAIL 4 vs 5 TRIV minted filter `true` -> FAIL 49 vs 5 arm (1) prod `waiting_for = GameOver` -> FAIL beat 5, GameOver vs Priority{P0} arm (2) prod `priority_player = PlayerId(3)` -> FAIL beat 5, PlayerId(3) vs PlayerId(0) ⓔ reach prod no pop at capacity -> FAIL reach-guard, max ring 28 prod `LOOP_DETECT_RING_CAP = 4096` -> FAIL reach-guard ⓔ shape prod eviction made 2-for-2 -> FAIL beat 67, (2,16) vs (1,16) TRIV minted filter `false` -> FAIL beat 72, (0,16) vs (1,16) ⓒ reach prod top-of-beat clear neutered -> FAIL reach-guard prod `SetAutoPass` added to exemption -> FAIL reach-guard ⓒ shape prod sampler pushes twice -> FAIL beat 1, (0,0) vs (1,1) TRIV minted filter `false` -> FAIL beat 6, (0,1) vs (1,1) ONE MUTANT MISSED ITS TARGET AND IS RECORDED RATHER THAN DROPPED: making the sampler push twice flips ⓔ's REACH guard, not its shape assertion, because the doubled push steps the length 15 -> 17 so `len == LOOP_DETECT_RING_CAP` never holds and the ring never evicts at all. ⓔ's DROP arm is the 2-for-2 eviction instead. Assisted-by: ClaudeCode:claude-opus-5 --- .../engine/tests/integration/loop_shortcut.rs | 375 +++++++++++++++--- 1 file changed, 327 insertions(+), 48 deletions(-) diff --git a/crates/engine/tests/integration/loop_shortcut.rs b/crates/engine/tests/integration/loop_shortcut.rs index 9bb8fd4241..515f7a42f9 100644 --- a/crates/engine/tests/integration/loop_shortcut.rs +++ b/crates/engine/tests/integration/loop_shortcut.rs @@ -26,7 +26,8 @@ use engine::types::ability::{Effect, TargetRef}; use engine::types::actions::GameAction; use engine::types::events::GameEvent; use engine::types::game_state::{ - CastPaymentMode, GameState, LoopDetectionMode, StackEntryKind, WaitingFor, YieldTarget, + AutoPassRequest, CastPaymentMode, GameState, LoopDetectionMode, StackEntryKind, WaitingFor, + YieldTarget, }; use engine::types::identifiers::ObjectId; use engine::types::mana::{ManaColor, ManaCost, ManaCostShard, ManaType, ManaUnit}; @@ -11203,15 +11204,22 @@ fn ai1_the_bounded_declare_candidate_withdraws_when_the_offer_publishes_a_pin() /// non-shrinking stack, and `Priority{player == active_player}`), so it is a REACH signal and /// never an attribution. /// -/// The row therefore asserts arms (1) and (2) over EVERY frame a beat added -/// (`loop_detect_ring.iter().rev().take(grew)`), which makes attribution irrelevant rather than -/// sharper: both samplers gate their record on `Priority{player == active_player}` and both -/// record after their own `sync_waiting_for`, so a frame that fails either arm is a real defect -/// whichever site minted it. What survives of the mint accounting is narrow and stated as such: -/// `Arc`-identity detection so an evicting push at `LOOP_DETECT_RING_CAP` cannot masquerade as -/// "no mint", and `grew >= 1` so a beat whose length did not grow fails closed instead of -/// mis-sizing the slice. MEASURED on this drive: 2 forced-window minting beats (5, 14), 3 -/// non-forced ones (0, 9, 18), every beat `grew == 1`, offer at beat 19 over a 5-frame ring. +/// The row therefore asserts arms (1) and (2) over EVERY frame a beat MINTED, which makes +/// attribution irrelevant rather than sharper: both samplers gate their record on +/// `Priority{player == active_player}` and both record after their own `sync_waiting_for`, so a +/// frame that fails either arm is a real defect whichever site minted it. The minted set is the +/// ring's pre-beat/post-beat `Arc`-identity MEMBERSHIP DIFFERENCE. No scalar is derived from the +/// ring's length, and the length-delta accounting that stood here is deleted rather than +/// sharpened — a scalar cannot name that set on either of the two production paths the loop body +/// documents. MEASURED on this drive: 2 forced-window minting beats (5, 14), 3 non-forced ones +/// (0, 9, 18), 5 frames minted and validated in total, offer at beat 19 over a 5-frame ring. +/// +/// THIS FIXTURE REACHES NEITHER of the two paths that break a length delta, measured rather than +/// assumed: 0 evicting beats and 0 clear-and-rebuild beats over the drive, max ring 5 against a +/// capacity of 16. Both are reached — and the deleted scalar shown wrong on each — by +/// `an_evicting_beat_mints_without_growing_the_ring` and +/// `a_clearing_beat_rebuilds_the_ring_inside_the_same_beat`, on boards driven through the same +/// production `apply()`. /// /// ⚠ WHAT THIS ROW DOES **NOT** CATCH, stated rather than implied. A PURE REVERT of the reorder /// leaves all three arms GREEN, and that is a measurement rather than an oversight: an @@ -11257,6 +11265,7 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact let pin = engine_live_opponents(&state, P0).first().copied(); let mut answer_mints = 0usize; let mut settle_mints = 0usize; + let mut frames_validated = 0usize; let mut offer_beat = None; for beat in 0..400usize { if matches!( @@ -11270,46 +11279,51 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact break; } let answered_forced_window = state.waiting_for.is_forced_cascade_window(); - let before = state.loop_detect_ring.len(); - // A MINT IS DETECTED BY `Arc` IDENTITY OF THE RING'S BACK, NOT BY A LENGTH DELTA. - // `record_loop_detect_sample` pops the front before pushing once the ring is at - // `LOOP_DETECT_RING_CAP`, so a full ring mints WITHOUT growing and a length-only - // detector reads that as "no mint" — the beat would take the `continue` below and - // `answer_mints` would silently under-count the set this row claims to have checked. - // `game::engine::drive_one_shortcut_cycle` uses identity here for exactly this reason; - // this is that instrument, not a second one. Identity also makes the ring's capacity - // irrelevant to the row, which is why no literal cap is transcribed here (the const is - // private to `types::game_state`, so a transcribed copy could only rot). - let back_before = state.loop_detect_ring.back().map(std::sync::Arc::as_ptr); + // THE BEAT'S MINTED SET IS A MEMBERSHIP DIFFERENCE. Nothing about the ring's LENGTH, + // and nothing about its `back()`, can name it — both are unsound against production: + // + // * `GameState::record_loop_detect_sample` pops the front before pushing once the ring + // is at `LOOP_DETECT_RING_CAP`, so an evicting push mints while the length stays + // EQUAL. A `back()`-changed + `after - before` detector reads that as a mint of size + // zero and fails on a legitimate beat. + // * `apply_action` clears the ring at its top for any non-`PassPriority`, + // non-`OrderTriggers` action answering a non-forced window, and the settle sampler in + // `pass_priority_once_with_pipeline` mints LATER in the same beat. Net growth is then + // `minted - cleared`, which is smaller than `minted` — a `take(net)` silently skips + // frames the row claims to have checked. + // + // The full pre-beat membership covers append, eviction and clear-and-rebuild uniformly, + // and it makes the ring's capacity irrelevant to the row (no literal cap is transcribed + // here; the const is private to `types::game_state`, so a copy could only rot). + // + // The snapshot holds `Arc` CLONES, not raw addresses, and that is load-bearing rather + // than incidental: `pop_front` DROPS the evicted `Arc` before `Arc::new` allocates the + // replacement, so an address-keyed snapshot can be aliased by the allocator handing the + // freed block straight back, and a genuinely new frame would then read as an old one. + // A retained strong reference makes every snapshotted address un-reusable for the whole + // beat, so `Arc::ptr_eq` is exact by construction instead of by luck. + let before: Vec<_> = state.loop_detect_ring.iter().cloned().collect(); if dump_drive_one_beat(&mut state, pin).is_err() { break; } - // `is_some() &&` is the production instrument's guard, kept rather than trimmed: the - // settle sampler's `else` arm calls `loop_detect_ring.clear()`, so `back()` can go to - // `None` within a beat. Without it, `None != Some(..)` falls through to a `usize` - // subtraction that panics with "attempt to subtract with overflow" instead of this - // row's own explanation. A cleared ring minted nothing, so the beat is skipped. - let back_after = state.loop_detect_ring.back().map(std::sync::Arc::as_ptr); - if back_after.is_none() || back_after == back_before { + // Frames minted and then evicted WITHIN one beat are absent here by construction — + // they are gone from the ring. "Every frame this beat added that the ring still holds" + // is exactly the set the arms below claim, and exactly the set any consumer can read. + let minted: Vec<_> = state + .loop_detect_ring + .iter() + .filter(|f| !before.iter().any(|b| std::sync::Arc::ptr_eq(b, f))) + .collect(); + if minted.is_empty() { continue; } - let after_len = state.loop_detect_ring.len(); - let grew = after_len.saturating_sub(before); - assert!( - grew >= 1, - "beat {beat}: the ring's back changed but its length did not grow ({before} -> \ - {after_len}). Either the ring is AT CAPACITY and evicting, or it was CLEARED and \ - re-pushed inside the beat (the settle sampler's `else` arm does exactly that). \ - Either way `grew` cannot say how many frames THIS beat added, so the arms below \ - would inspect the wrong set. Fail closed rather than report a number this \ - instrument cannot support" - ); + frames_validated += minted.len(); if answered_forced_window { answer_mints += 1; } else { - settle_mints += grew; + settle_mints += minted.len(); } - // BOTH ARMS RUN OVER EVERY FRAME THIS BEAT ADDED, which is what makes site attribution + // BOTH ARMS RUN OVER EVERY FRAME THIS BEAT MINTED, which is what makes site attribution // stop mattering — and the row deliberately does NOT try to attribute. // // `answered_forced_window` is `is_forced_cascade_window()` read BEFORE the beat, i.e. @@ -11318,16 +11332,14 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact // non-shrinking stack, and `Priority{player == active_player}`. One beat here is one // `apply()`, which reaches the SETTLE sampler after `apply_action` returns. So a beat // that answers a forced window resolving the LAST stack entry gates the answer site - // off (`!stack.is_empty()` false) while the refill cascade mints a settle frame: - // `grew == 1`, `answer_mints` increments, and a `back()`-only check would inspect a - // settle frame while claiming an answer frame. `grew == 1` rules out the DOUBLE-MINT - // beat and nothing more — it was never evidence about WHICH sampler minted. + // off (`!stack.is_empty()` false) while the refill cascade mints a settle frame, and + // `answer_mints` increments over a frame the SETTLE site produced. // - // Iterating the added frames removes the question instead of sharpening it. Both + // Iterating the minted frames removes the question instead of sharpening it. Both // samplers gate their record on `Priority{player == active_player}` and both record // after their `sync_waiting_for`, so both arms hold for EITHER site's frame; a frame // that fails one is a real defect no matter which sampler produced it. - for sample in state.loop_detect_ring.iter().rev().take(grew) { + for sample in minted { let frame = &sample.live; // ── (2) ITS PRIORITY PLAYER. Asserted FIRST so a mutation that touches only // `priority_player` is caught by its own arm instead of being masked by arm (1). @@ -11385,11 +11397,25 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact ); assert!( settle_mints > 0, - "reach-guard for the WIDENING: arms (1)/(2) now run over every frame a beat added, \ + "reach-guard for the WIDENING: arms (1)/(2) now run over every frame a beat minted, \ from either sampler, so the drive must also mint at a non-forced (settle) beat — \ otherwise the settle-frame coverage this row claims is untested. got \ answer={answer_mints} settle={settle_mints}" ); + let ring_at_offer = state.loop_detect_ring.len(); + assert_eq!( + frames_validated, ring_at_offer, + "reach-guard on the MEMBERSHIP DIFFERENCE ITSELF, which is what makes 'every frame the \ + beat minted' a measurement instead of a claim: every frame the offer's ring holds must \ + have reached arms (1)/(2) as a member of some beat's minted set. A detector that \ + silently returned the EMPTY set on a minting beat — exactly what a length delta returns \ + at capacity — leaves frames in the ring that no arm ever read, and that lands here as \ + validated {frames_validated} against ring {ring_at_offer}. Equality rather than `>=` \ + because \ + this drive neither evicts nor clears (measured: max ring 5 against a capacity of 16, 0 \ + evicting and 0 clearing beats), so a frame counted but no longer present is equally a \ + defect on THIS board" + ); let offer_beat = offer_beat.expect( "reach-guard: the bounded offer must FIRE on this real 4p drain, else arm (3) asserts \ about a certificate that was never published", @@ -11471,3 +11497,256 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact `impl PartialEq for GameState`)" ); } + +// ───── #7023 maintainer item: a beat's minted frames are a MEMBERSHIP set, not a length delta ───── + +/// `(minted, dropped)` for one beat, by `Arc` IDENTITY: frames the ring gained, and pre-beat +/// frames it lost. Generic over the sample type so the ring's private element type is never +/// named here. +/// +/// `before` must be a slice of `Arc` CLONES held across the beat, not of raw addresses. +/// `GameState::record_loop_detect_sample` calls `pop_front()` — which DROPS the evicted +/// allocation — before `Arc::new` claims a new one of identical layout, so an address-keyed +/// snapshot can be aliased by the allocator handing the freed block straight back, and a +/// genuinely new frame would then read as an old one. A retained strong reference makes every +/// snapshotted address un-reusable for the whole beat, so `ptr_eq` is exact by construction +/// rather than by luck. +fn ring_membership_delta( + before: &[std::sync::Arc], + after: &std::collections::VecDeque>, +) -> (usize, usize) { + let minted = after + .iter() + .filter(|f| !before.iter().any(|b| std::sync::Arc::ptr_eq(b, f))) + .count(); + let dropped = before + .iter() + .filter(|b| !after.iter().any(|f| std::sync::Arc::ptr_eq(b, f))) + .count(); + (minted, dropped) +} + +/// `dump_drive_one_beat`'s policy with its `Priority` arm taken directly. That policy is +/// unconditionally "pass" at a `Priority` window, so routing through the enumerator costs a full +/// per-viewer candidate scan — on the 152-entry `dellian` stack, the dominant cost of a long +/// drive — only to find the `PassPriority` the policy already chose. `apply` performs the real +/// legality check itself (`game::priority::pass_priority_legality`), so nothing is skipped but +/// the enumeration. Every other window still goes through the shared driver unchanged. +fn drive_one_beat_passing_fast(state: &mut GameState, pin: Option) -> Result<(), String> { + if let WaitingFor::Priority { player } = state.waiting_for { + return apply(state, player, GameAction::PassPriority) + .map(|_| ()) + .map_err(|e| format!("pass err: {e:?}")); + } + dump_drive_one_beat(state, pin).map(|_| ()) +} + +/// CR 732.2a. ROUTE ⓔ — EVICTION AT `LOOP_DETECT_RING_CAP`: a beat that MINTS WITHOUT GROWING. +/// +/// `answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact` reads a beat's +/// minted frames as the `loop_detect_ring`'s pre/post `Arc`-identity MEMBERSHIP DIFFERENCE. It +/// previously read them as `rev().take(after_len - before_len)` off a changed `back()`, and this +/// row plus its ⓒ sibling are the reachability half of the #7023 review that replaced that: the +/// scalar is wrong on two production routes, and the `dina_conqueror_4p` drive that row performs +/// reaches NEITHER — measured on it: max ring 5 against a capacity of 16, 0 evicting beats, 0 +/// clearing beats. Correcting a detector against routes no fixture reaches would re-open the +/// evidential hole the correction exists to close, so each route gets a board. +/// +/// THE MECHANISM. `GameState::record_loop_detect_sample` calls `pop_front()` and THEN +/// `push_back()` once the ring is at `LOOP_DETECT_RING_CAP`. One frame leaves, one arrives: the +/// back changes, the LENGTH DOES NOT. Net growth is 0 on a beat that minted 1, so +/// `rev().take(net)` inspects nothing and the deleted `assert!(grew >= 1, ..)` — labelled "fail +/// closed" in-tree — failed the test on an ordinary drain beat rather than on an anomaly. That +/// label was wrong about which side of the line the beat is on, and it went with the code. +/// +/// FIXTURE: the tracked `dellian_emblem_conqueror_4p` dump, driven through production `apply()`, +/// so the ring that reaches capacity is this drive's own accumulation through +/// `record_loop_detect_sample`. MEASURED: first evicting beat at 72 (ring 16 -> 16, minted 1, +/// dropped 1), three more inside the first 90. +/// +/// THE SEARCH PREDICATE IS STRUCTURAL AND THE ASSERTION IS THE CONSEQUENCE, never the reverse: +/// the witness is the first beat that LOST a pre-beat frame without the ring shrinking — pure +/// membership plus an ordering, saying nothing about minting — and the assertion is then what +/// the beat minted and what its length did. A witness selected on "minted while the length stood +/// still" would have carried its own conclusion into the arm that claims to test it. +/// +/// This row does NOT re-assert the answer-beat row's frame invariants (the synced +/// `waiting_for`/`priority_player` pair). Its subject is the DETECTOR, not the frames. +#[test] +fn an_evicting_beat_mints_without_growing_the_ring() { + let mut state = restore_dump(&gunzip_dump(include_bytes!( + "../fixtures/dellian_emblem_conqueror_4p.json.gz" + ))); + assert_eq!( + state.loop_detect_ring.len(), + 0, + "reach-guard: the dump ships with an EMPTY ring, so the capacity reached below is THIS \ + drive's accumulation through the production sampler and not a restored ring" + ); + + let pin = engine_live_opponents(&state, P0).first().copied(); + let mut max_ring = 0usize; + let mut beats_run = 0usize; + let mut evicting = None; + for beat in 0..120usize { + if matches!(state.waiting_for, WaitingFor::LoopShortcut { .. }) { + break; + } + let before: Vec<_> = state.loop_detect_ring.iter().cloned().collect(); + if drive_one_beat_passing_fast(&mut state, pin).is_err() { + break; + } + beats_run = beat + 1; + max_ring = max_ring.max(state.loop_detect_ring.len()); + let (minted, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); + if dropped == 0 || state.loop_detect_ring.len() < before.len() { + continue; + } + evicting = Some(( + beat, + before.len(), + state.loop_detect_ring.len(), + minted, + dropped, + )); + break; + } + + let (beat, before_len, after_len, minted, dropped) = evicting.unwrap_or_else(|| { + panic!( + "reach-guard: no beat replaced a ring frame without shrinking the ring, so this \ + drive never reached `LOOP_DETECT_RING_CAP` and the eviction route is untested — the \ + state the `dina_conqueror_4p` drive is permanently in, and the reason this row \ + exists on a different board. mode {:?}, max ring {max_ring} over {beats_run} beats", + state.loop_detection + ) + }); + assert_eq!( + (minted, after_len), + (1, before_len), + "beat {beat}: an evicting push is 1-for-1 — `record_loop_detect_sample` pops the front \ + and THEN pushes at capacity — so this beat MINTED while its length stood still at \ + {before_len}, dropping {dropped}. THE DELETED SCALAR ON THIS BEAT: \ + `after_len - before_len` is 0, so `rev().take(grew)` inspects NOTHING while the beat \ + minted a frame, and `assert!(grew >= 1)` fails the whole test on an ordinary drain \ + beat. The membership difference reports the mint; no length delta can" + ); +} + +/// CR 732.2a. ROUTE ⓒ — CLEAR-AND-REBUILD INSIDE ONE BEAT: a beat that MINTS MORE THAN IT GROWS. +/// +/// Sibling of `an_evicting_beat_mints_without_growing_the_ring`; the shared argument for why both +/// routes need a board of their own is stated there. +/// +/// THE MECHANISM. `game::engine::apply_action` clears the ring at its top for any action that is +/// neither `PassPriority` nor `OrderTriggers` answering a non-forced window. +/// `GameAction::SetAutoPass` at a `Priority` window is exactly that, and its own arm then calls +/// `pass_priority_once_with_pipeline`, whose settle sampler mints — after which `apply`'s +/// auto-pass loop can call it again inside the SAME beat. Net growth is therefore +/// `minted - cleared`, strictly below `minted` whenever the ring was non-empty, so `take(net)` +/// skips frames the answer-beat row claims to validate. MEASURED here: ring 2 -> 1, 2 dropped, 1 +/// minted, i.e. net 0 against a real mint — the same blindness route ⓔ produces, reached the +/// other way, and the shallow-clear case (`0 < net < minted`) is the same defect with a smaller +/// margin. +/// +/// `SetAutoPass { UntilStackEmpty }` IS NOT A TEST HOOK. It is the exact payload the client's +/// Arena-style "Resolve All" control dispatches (`client/src/game/dispatch.ts`), so this beat is +/// a player pressing that button mid-cascade. It is absent from +/// `ai_support::legal_actions_for_viewer`'s enumeration — `classify_flat_priority_action` files +/// it with the preference-propagation actions — which is why the generic dump driver never picks +/// it and why the route is dispatched by name here instead of being found by the driver's "first +/// legal action" policy. Dispatching it by name changes nothing about the beat: it is one +/// ordinary `apply()`, validated by the production reducer like any other. +/// +/// FIXTURE: the tracked `dina_conqueror_4p` dump — the SAME board the answer-beat row drives, so +/// the contrast is exact. Left to itself that drive never clears (measured: 0 clearing beats); +/// one "Resolve All" press at the first `Priority` window carrying 2 accumulated frames puts it +/// on this route at beat 6. +/// +/// SEARCH PREDICATE STRUCTURAL, ASSERTION THE CONSEQUENCE: the witness is the first beat that +/// lost EVERY pre-beat frame — a statement about membership alone — and the assertion is what +/// the beat minted and what it left behind. The threshold of 2 accumulated frames is not +/// cosmetic: below it a wiped ring and a merely-evicted one are indistinguishable by membership. +#[test] +fn a_clearing_beat_rebuilds_the_ring_inside_the_same_beat() { + let mut state = restore_dump(&gunzip_dump(include_bytes!( + "../fixtures/dina_conqueror_4p.json.gz" + ))); + assert_eq!( + state.loop_detect_ring.len(), + 0, + "reach-guard: the dump ships with an EMPTY ring, so the frames the clear below discards \ + were accumulated by THIS drive through the production sampler" + ); + + let pin = engine_live_opponents(&state, P0).first().copied(); + let mut fired = false; + let mut max_ring = 0usize; + let mut beats_run = 0usize; + let mut clearing = None; + for beat in 0..120usize { + if matches!(state.waiting_for, WaitingFor::LoopShortcut { .. }) { + break; + } + let before: Vec<_> = state.loop_detect_ring.iter().cloned().collect(); + max_ring = max_ring.max(before.len()); + let outcome = if !fired + && before.len() >= 2 + && matches!(state.waiting_for, WaitingFor::Priority { .. }) + { + fired = true; + let who = state + .waiting_for + .acting_player() + .expect("a `Priority` window names its actor"); + apply( + &mut state, + who, + GameAction::SetAutoPass { + mode: AutoPassRequest::UntilStackEmpty, + }, + ) + .map(|_| ()) + .map_err(|e| format!("resolve-all err: {e:?}")) + } else { + drive_one_beat_passing_fast(&mut state, pin) + }; + if outcome.is_err() { + break; + } + beats_run = beat + 1; + let (minted, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); + if before.is_empty() || dropped != before.len() { + continue; + } + clearing = Some(( + beat, + before.len(), + state.loop_detect_ring.len(), + minted, + dropped, + )); + break; + } + + let (beat, before_len, after_len, minted, _dropped) = clearing.unwrap_or_else(|| { + panic!( + "reach-guard: no beat lost EVERY pre-beat ring frame, so `apply_action`'s \ + top-of-beat clear never ran on an accumulated ring and the clear-and-rebuild route \ + is untested. resolve-all dispatched={fired}, max ring {max_ring} over {beats_run} \ + beats" + ) + }); + assert_eq!( + (minted, after_len), + (1, 1), + "beat {beat}: `apply_action` cleared all {before_len} accumulated frames at the top of \ + this beat; a sampler must then have MINTED inside the SAME beat, leaving exactly one \ + frame — a left of `(0, 0)` means the beat only cleared, which is a different route and \ + proves nothing about the detector. THE DELETED SCALAR ON THE REBUILD BEAT: \ + `after_len - before_len` saturates to 0 against a real mint, so `take(grew)` validates \ + none of the frames the answer-beat row claims — and where the clear is shallower than \ + the rebuild the scalar is positive but still short. Net growth is `minted - cleared`; \ + it can never name `minted`" + ); +} From f2df77c5d78b4b1db8bf521fa66d727a496bb267 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Wed, 5 Aug 2026 05:07:47 -0700 Subject: [PATCH 5/5] fix(PR-7023): unify minted-frame witnesses Use the shared membership helper for the answer-beat validator and bind the clear-and-rebuild witness to the SetAutoPass dispatch beat. Co-authored-by: lgray --- .../engine/tests/integration/loop_shortcut.rs | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/crates/engine/tests/integration/loop_shortcut.rs b/crates/engine/tests/integration/loop_shortcut.rs index 515f7a42f9..1a357a5e4b 100644 --- a/crates/engine/tests/integration/loop_shortcut.rs +++ b/crates/engine/tests/integration/loop_shortcut.rs @@ -11309,11 +11309,7 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact // Frames minted and then evicted WITHIN one beat are absent here by construction — // they are gone from the ring. "Every frame this beat added that the ring still holds" // is exactly the set the arms below claim, and exactly the set any consumer can read. - let minted: Vec<_> = state - .loop_detect_ring - .iter() - .filter(|f| !before.iter().any(|b| std::sync::Arc::ptr_eq(b, f))) - .collect(); + let (minted, _) = ring_membership_delta(&before, &state.loop_detect_ring); if minted.is_empty() { continue; } @@ -11500,9 +11496,9 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact // ───── #7023 maintainer item: a beat's minted frames are a MEMBERSHIP set, not a length delta ───── -/// `(minted, dropped)` for one beat, by `Arc` IDENTITY: frames the ring gained, and pre-beat -/// frames it lost. Generic over the sample type so the ring's private element type is never -/// named here. +/// `(minted_frames, dropped)` for one beat, by `Arc` IDENTITY: frames the ring gained, and +/// pre-beat frames it lost. Generic over the sample type so the ring's private element type is +/// never named here. /// /// `before` must be a slice of `Arc` CLONES held across the beat, not of raw addresses. /// `GameState::record_loop_detect_sample` calls `pop_front()` — which DROPS the evicted @@ -11511,14 +11507,14 @@ fn answer_beat_frames_carry_the_synced_window_and_the_offer_certificate_is_exact /// genuinely new frame would then read as an old one. A retained strong reference makes every /// snapshotted address un-reusable for the whole beat, so `ptr_eq` is exact by construction /// rather than by luck. -fn ring_membership_delta( +fn ring_membership_delta<'a, T>( before: &[std::sync::Arc], - after: &std::collections::VecDeque>, -) -> (usize, usize) { + after: &'a std::collections::VecDeque>, +) -> (Vec<&'a std::sync::Arc>, usize) { let minted = after .iter() .filter(|f| !before.iter().any(|b| std::sync::Arc::ptr_eq(b, f))) - .count(); + .collect(); let dropped = before .iter() .filter(|b| !after.iter().any(|f| std::sync::Arc::ptr_eq(b, f))) @@ -11598,7 +11594,8 @@ fn an_evicting_beat_mints_without_growing_the_ring() { } beats_run = beat + 1; max_ring = max_ring.max(state.loop_detect_ring.len()); - let (minted, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); + let (minted_frames, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); + let minted = minted_frames.len(); if dropped == 0 || state.loop_detect_ring.len() < before.len() { continue; } @@ -11663,10 +11660,10 @@ fn an_evicting_beat_mints_without_growing_the_ring() { /// one "Resolve All" press at the first `Priority` window carrying 2 accumulated frames puts it /// on this route at beat 6. /// -/// SEARCH PREDICATE STRUCTURAL, ASSERTION THE CONSEQUENCE: the witness is the first beat that -/// lost EVERY pre-beat frame — a statement about membership alone — and the assertion is what -/// the beat minted and what it left behind. The threshold of 2 accumulated frames is not -/// cosmetic: below it a wiped ring and a merely-evicted one are indistinguishable by membership. +/// SEARCH PREDICATE STRUCTURAL, ASSERTION THE CONSEQUENCE: the witness is the dispatched +/// `SetAutoPass` beat that lost EVERY pre-beat frame, and the assertion is what the beat minted +/// and what it left behind. The threshold of 2 accumulated frames is not cosmetic: below it a +/// wiped ring and a merely-evicted one are indistinguishable by membership. #[test] fn a_clearing_beat_rebuilds_the_ring_inside_the_same_beat() { let mut state = restore_dump(&gunzip_dump(include_bytes!( @@ -11690,10 +11687,9 @@ fn a_clearing_beat_rebuilds_the_ring_inside_the_same_beat() { } let before: Vec<_> = state.loop_detect_ring.iter().cloned().collect(); max_ring = max_ring.max(before.len()); - let outcome = if !fired - && before.len() >= 2 - && matches!(state.waiting_for, WaitingFor::Priority { .. }) - { + let dispatched_here = + !fired && before.len() >= 2 && matches!(state.waiting_for, WaitingFor::Priority { .. }); + let outcome = if dispatched_here { fired = true; let who = state .waiting_for @@ -11715,8 +11711,9 @@ fn a_clearing_beat_rebuilds_the_ring_inside_the_same_beat() { break; } beats_run = beat + 1; - let (minted, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); - if before.is_empty() || dropped != before.len() { + let (minted_frames, dropped) = ring_membership_delta(&before, &state.loop_detect_ring); + let minted = minted_frames.len(); + if !dispatched_here || before.is_empty() || dropped != before.len() { continue; } clearing = Some((