feat(engine): combo-detection PR-1 — analysis sim harness feeding ResourceVector

[lgray]

🤖 AI text below 🤖

Where this sits in the staged infinite-combo-detection effort

Goal of the overall effort: confirm a claimed deterministic loop and classify which resource goes unbounded (so the engine can shortcut a beneficial loop per CR 732.2a instead of only drawing a truly-repeating mandatory loop per CR 104.4b / CR 732.4). The work is staged PR-0..PR-8; the design docs live untracked in .planning/combo-detection/ (IMPLEMENTATION.md §5/§7/§8, FEASIBILITY-AND-PLAN.md).

• PR-0 (predecessor): feat(engine): combo-detection PR-0 — ResourceVector + modulo-resource loop equality (additive, no behavior change) #4092 — ResourceVector (the monotone resource axes a loop can pump) + loop_states_equal_modulo_resources (board/zones/tap identical, resources allowed to differ), reusing the existing loop_fingerprint / normalize_for_loop / loop_states_equal machinery.
• PR-1 (this PR): the analysis simulation harness around GameRunner::act that consumes ResourceVector and makes its measurement substrate real.
• PR-2..PR-8: net-progress detection at the loop_window site → LoopCertificate (PR-2), emit_resolution_halt classification → live shortcut (PR-3), optional static ability-graph (PR-4), cargo combo-verify CLI over the corpus (PR-5), ∞ unbounded-resource display (PR-6), loop shortcut + opponent response window (PR-7), AI coupling (PR-8).

What PR-1 added

PR-0's ResourceVector::snapshot(&GameState) deliberately fills only the state-readable axes (mana, life, library size, counters) and leaves the event-fed axes at Default — damage_dealt, tokens_created, cards_drawn, casts_this_step, landfall_triggers, combat_phases, extra_turns, death_triggers, etb_triggers, ltb_triggers, sac_triggers, and generic_triggers. Those are events, not totals a single GameState retains.

This PR adds crates/engine/src/analysis/sim.rs:

• accumulate_events(&mut ResourceVector, &[GameEvent]) — folds one action's game-event stream into the event-fed axes.
• LoopProbe<'r> — wraps a GameRunner, snapshots the state-readable axes at iteration boundaries, accumulates events across the actions of an iteration via act, and iteration_delta() returns the per-iteration ResourceVector (state-readable half = snapshot delta; event-fed half = the per-iteration event tally taken verbatim).

analysis/mod.rs registers the sim module and re-exports LoopProbe / accumulate_events. No pub visibility bumps were needed (analysis is in-crate). data/engine-inventory.json is unchanged (no Effect/ability variants added).

Offline / zero gameplay change. The harness only observes the runner — no reducer, SBA, or resolution edits. The detector hook itself is PR-2.

Event-feed wiring approach

The fold is event-driven, not card-driven, so each axis covers a class of cards rather than one card:

Event-fed axis	GameEvent source
damage_dealt[player]	DamageDealt { target: Player } + CombatDamageDealtToPlayer
tokens_created	TokenCreated
cards_drawn	CardDrawn (per-card) + CardsDrawn { count } (batch; disjoint paths)
casts_this_step	SpellCast (copies are not casts → not counted)
combat_phases	PhaseChanged { BeginCombat }
extra_turns	TurnStarted
etb_triggers / landfall_triggers	ZoneChanged { to: Battlefield } (landfall iff the ZoneChangeRecord core types contain Land)
ltb_triggers / death_triggers	ZoneChanged { from: Battlefield } (death iff to: Graveyard)
sac_triggers	PermanentSacrificed
generic_triggers[Proliferate]	PlayerPerformedAction { Proliferate }

State-readable axes are intentionally not routed through the event feed — they come from ResourceVector::snapshot so the two halves never double-count.

Discriminating-test map (+ revert-probe)

• accumulate_events_feeds_every_axis — feeds the real GameEvent variant for every event-fed axis and asserts each populates. Each match arm is its own revert probe: delete an arm and that axis stays 0.
• accumulate_events_no_events_is_noop — empty stream ⇒ every event-fed axis at Default (pins the floor for the end-to-end probes).
• accumulate_events_within_battlefield_is_not_etb_or_ltb / accumulate_events_non_proliferate_player_action_ignored — negative cases (no spurious ETB/LTB on a within-battlefield move; non-proliferate player actions ignored).
• loop_probe_feeds_damage_from_real_pipeline — end-to-end through the real apply() pipeline: casts Lightning Bolt at the opponent via LoopProbe::act (CastSpell → SelectTargets → resolve), then asserts iteration_delta feeds damage_dealt = 3 and casts_this_step = 1 from the real DamageDealt/SpellCast events (revert probe: those arms are the only feed for those axes).
• loop_probe_iteration_delta_isolates_each_iteration — two damage iterations; each iteration_delta reports only its own 3 (boundary roll-forward; without it the 2nd would read 6).
• loop_probe_state_readable_axis_independent_of_event_feed — a poison counter set between boundaries surfaces from the snapshot with no event-feed arm involved (pins the state-readable vs event-fed division of labor).

Gate results

cargo fmt --all clean · check-parser-combinators.sh (with and without upstream/main) exit 0 · check-engine-authorities.sh upstream/main exit 0 · cargo clippy --workspace --exclude phase-tauri --all-targets --features engine/proptest -- -D warnings clean · cargo test -p engine: 13101 passed; 2 failed — the two failures are the known parallel-flaky delve_payment_skips_state_clone_per_graveyard_candidate and target_selection_legal_actions_do_not_simulate_each_target, both confirmed green in isolation with --test-threads=1. data/engine-inventory.json unchanged. No new CR annotations are unverified against docs/MagicCompRules.txt.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[matthewevans]

Blocking this one for a PR-1-specific false-positive in the event-fed axes, plus the inherited PR-0 ResourceVector blocker.

HIGH: ordinary turn/combat progression is counted as extra turns and extra combats

The new event feed maps every GameEvent::PhaseChanged { phase: BeginCombat } to combat_phases += 1 and every GameEvent::TurnStarted to extra_turns += 1 (crates/engine/src/analysis/sim.rs:90-99). LoopProbe::iteration_delta then splices those event-fed counts verbatim into the returned delta (sim.rs:211-220, sim.rs:246-254), and ResourceVector::is_net_progress treats both combat_phases and extra_turns as gained axes (crates/engine/src/analysis/resource.rs:310-321, resource.rs:349-360).

Those events are not evidence that an extra phase/turn was created. Natural phase entry also emits PhaseChanged { BeginCombat }, and the turn code emits the same TurnStarted event after both natural and extra turns: start_next_turn computes an is_extra_turn flag for replacement handling, but the emitted GameEvent::TurnStarted carries only player_id and turn_number (crates/engine/src/game/turns.rs:463-489, turns.rs:686-689; crates/engine/src/types/events.rs:115-123). Extra turns are created by pushing state.extra_turns in the ExtraTurn resolver (crates/engine/src/game/effects/extra_turn.rs:35-41), and extra combats are created as state.extra_phases entries (crates/engine/src/game/effects/additional_phase.rs:194-202, additional_phase.rs:222-237), not by the later ordinary-looking phase/turn events.

As written, a probe window that naturally crosses begin combat or the next turn reports net progress on CombatPhases/ExtraTurns even if no effect created an additional phase or turn. That is exactly the kind of false-positive loop certificate the combo detector cannot build on. Please feed these axes from the creation/queueing event or add an event field that distinguishes natural vs extra, and add negative tests for a natural BeginCombat and a natural next turn producing zero extra-combat/extra-turn progress.

This head also still contains the PR-0 ResourceVector commit that already has a Changes Requested review for erasing consumable/state-gating counters in loop_states_equal_modulo_resources, so PR-1 should not be approved until that foundation is corrected as well.

[lgray]

🤖 AI text below 🤖

Addressed the blocking review and rebased the stack.

Rebased onto the latest upstream/main. Predecessor PR-0 (#4092) has merged, so this branch now sits directly on main (dd52bdf99) and the PR diff shows only PR-1's own changes (analysis/sim.rs new file + the analysis/resource.rs/mod.rs deltas) — no duplicated PR-0 content. New head: 065aca47e.

Fix for the HIGH finding (ordinary turn/combat progression counted as extra turns/combats). Both axes are now fed from creation signals instead of natural-progression events, per-axis because the two are not symmetric:

• extra_turns stays event-fed but switches its source from the natural GameEvent::TurnStarted to the EffectResolved{ExtraTurn} creation event (CR 500.7). Effect::ExtraTurn resolution is a clean 1:1 — one push to state.extra_turns, one event, no no-op early-return path, no multi-push — so this is a correct creation count.
• combat_phases moves out of the event feed entirely and becomes state-readable in ResourceVector::snapshot: extra combats already entered (combat_phases_started_this_turn minus the one natural combat per CR 506.1) plus extra BeginCombat phases still queued in state.extra_phases (CR 500.8). The two terms are disjoint (advance_phase removes an extra phase before entering it), so a consumed extra combat is retained by the entered term and a self-sustaining extra-combat loop does not net to zero. This deliberately sidesteps the EffectResolved{AdditionalPhase} event's three defects that a naive event-count would inherit: one event for N pushes (Obeka under-count), an event emitted on no-op early returns (CR 500.10a wrong-player / count==0 false positive), and no phase-kind to combat-filter.

Net effect: a probe window crossing a natural begin-combat or next turn now reports zero extra-combat/extra-turn progress.

Negative tests (the reviewer's explicit ask), driven through the real pipeline:

• natural_begin_combat_is_not_extra_combat — drives PassPriority into a natural BeginCombat, asserts the natural PhaseChanged{BeginCombat} actually reached the probe (non-vacuity), then asserts delta.combat_phases == 0.
• natural_next_turn_is_not_extra_turn — drives a natural turn rollover, asserts the natural TurnStarted reached the probe, then delta.extra_turns == 0.

Both were verified to fail when the old arms are restored. Also added positive creation feeds, an Obeka-class multi-push (N=3), and hostile fixtures for the no-op AdditionalPhase (CR 500.10a), a non-combat extra phase, and create-then-consume retention.

Offline / zero gameplay change: snapshot only reads existing GameState fields; no reducer/SBA/event-emission edit; data/engine-inventory.json unchanged. One honest coverage gap is documented in the extra_turns field doc: the scheduled grant_extra_turn_after turn-control path pushes onto state.extra_turns without emitting EffectResolved{ExtraTurn}, so that less-common class is not counted on this axis (not a regression; out-of-scope to change here).

The PR-0 ResourceVector consumable-counter blocker referenced in the review was against the stale embedded PR-0 commit; it is resolved in merged PR-0 and is now part of the base.

Gates (Tilt down, foreground): cargo fmt --all clean; cargo clippy -p engine --lib -- -D warnings exit 0; cargo test -p engine --lib analysis:: 40 passed / 0 failed.

[matthewevans]

Prior blocker is addressed on the current head: natural PhaseChanged { BeginCombat } and natural TurnStarted no longer feed the extra-combat/extra-turn axes, and the new runner-level regressions exercise those natural events non-vacuously. The extra-turn axis is now tied to EffectResolved { ExtraTurn }, while extra combat is read from queued/entered ExtraPhase { BeginCombat } state with no-op/non-combat controls.

CI is green on the reviewed head and the repush guard held. VERDICT: approve