feat(engine): combo-detection PR-0 — ResourceVector + modulo-resource loop equality (additive, no behavior change)

[lgray]

🤖 AI text below 🤖

PR-0 of the infinite-combo detector — the measurement substrate. See the design concept in the combo-detection plan (.planning/combo-detection/IMPLEMENTATION.md §5/§7; FEASIBILITY-AND-PLAN.md §11 anchors). This PR is purely additive: a new engine::analysis module of types + a comparison function + tests. No game-behavior change — no reducer, SBA, or resolution-loop path is touched.

What was added

New module crates/engine/src/analysis/ (mod.rs + resource.rs), wired via pub mod analysis; in crates/engine/src/lib.rs.

• ResourceVector — the monotone resource axes a net-progress loop can pump:
  • mana: [i64; 6] (W/U/B/R/G/C), per-player life / damage_dealt / library_delta (BTreeMap<PlayerId, i64>), tokens_created, cards_drawn, casts_this_step, landfall_triggers, combat_phases, extra_turns, death_triggers / etb_triggers / ltb_triggers / sac_triggers, counters: BTreeMap<(CounterClass, ObjectClass), i64> (incl. +1/+1, loyalty, poison, energy), generic_triggers: BTreeMap<TriggerKind, i64> (proliferate / magecraft / …).
  • snapshot(&GameState) reads the state-readable levels directly (floating mana, life, library sizes, object/player counters). Event-fed fields (damage, tokens, draws, casts, all *_triggers, generic_triggers) are left at Default for the PR-1 harness to feed — they are events, not totals a single GameState retains.
  • delta(before, after) (component-wise subtraction), is_net_progress() (≥1 component strictly > 0 and no consumed axis — mana/life — net-negative), and unbounded_components() → the typed ResourceAxis tags for later WinKind classification.
• loop_states_equal_modulo_resources(a, b) — the complement of the existing strict CR 104.4b loop_states_equal: board/zones/tap-state identical, but the monotone resources allowed to differ (CR 732.2a net-progress shortcut vs CR 104.4b / CR 732.4 mandatory draw). Built directly on the existing normalize_for_loop (inheriting its volatile-field exclusions) plus an additional resource projection, then delegating to loop_states_equal. No new fingerprint; reuses loop_fingerprint/normalize_for_loop/loop_states_equal with no visibility changes to those pub(crate) helpers (reachable from analysis within the engine crate).

Helper enums (typed, not stringly)

• ObjectClass { Creature, Planeswalker, Battle, Player, Other } — what kind of thing a counter accumulates on.
• CounterClass { Plus1Plus1, Minus1Minus1, Loyalty, Defense, Poison, Energy, Other } — analysis-layer counter classification.
• TriggerKind { Proliferate, Magecraft, Constellation, Landfall, Other } — generic trigger families.
• ResourceAxis — the tagged identity of one unbounded resource (consumed by the PR-2 WinKind classifier).

§5 deferral note

The spec's counters key was (CounterType, ObjectClass). The engine's CounterType derives neither Ord (required for BTreeMap keys) nor a small closed set (it carries Generic(String), Keyword(KeywordKind), parameterized PowerToughness). Adding Ord to that crate-wide enum (and transitively KeywordKind) to satisfy one analysis map would be a far larger, non-additive change — so this module owns a small Ord CounterClass and maps CounterType → CounterClass at the snapshot boundary. All §5 axes are present; only this key-type substitution differs, documented in code.

add-engine-variant verdict: N/A

ResourceVector / ObjectClass / CounterClass / TriggerKind / ResourceAxis are analysis types, not ability/Effect enum variants. data/engine-inventory.json is unchanged (confirmed: no cargo engine-inventory churn).

CR annotations (grep-verified against docs/MagicCompRules.txt)

Existence + description verified for every CR number, e.g.: CR 104.4b (mandatory-loop draw), CR 732.2a / CR 732.4 (shortcut vs draw), CR 106.1 (mana), CR 119.1 (life), CR 120.1 (damage), CR 121 (draw), CR 122.1 / 122.1a (counters), CR 306.5b (loyalty), CR 310.4c (defense), CR 401 (library), CR 500.7 (extra turns), CR 500.8 + 506 (extra combat), CR 603.6a/603.6c (ETB/LTB), CR 700.4 (dies), CR 701.21 (sacrifice), CR 701.34 (proliferate), CR 207.2c + 603 (magecraft / constellation / landfall ability words — no individual CR entry).

Discriminating-test map

Test	Asserts	Reverting what breaks it
identical_states_equal_under_both_comparisons	byte-identical states are equal under strict AND modulo	sanity baseline
same_board_different_resources_is_modulo_equal_but_strictly_unequal	the key discriminator: same board, different life/mana/+1+1/damage → modulo true, strict false	the resource projection
revert_probe_projection_is_load_bearing	modulo accepts a beneficial-loop point that the un-projected loop_states_equal rejects	revert probe — fall back to loop_states_equal and the discriminator collapses
extra_permanent_is_not_modulo_equal	an extra permanent → modulo false (no false positive)	projection over-blurring
different_tap_state_is_not_modulo_equal	tapped vs untapped → modulo false	projection over-blurring
snapshot_and_delta_measure_known_changes	snapshot reads mana/life/counters; delta measures a known monotone change exactly	snapshot/delta math
net_progress_classification	is_net_progress true for +damage/−nothing, false for no-op and for net-negative life	is_net_progress consumed-axis rule
unbounded_components_names_growing_axes	names the growing axis; mill surfaces as negative library	unbounded_components

Gate results

• cargo fmt --all: clean
• ./scripts/check-parser-combinators.sh: exit 0 (no parser files touched)
• ./scripts/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: 13095 passed; the only failure is the known parallel-flaky delve_payment_skips_state_clone_per_graveyard_candidate (passes in isolation, unrelated to this additive module).
• data/engine-inventory.json: unchanged.

[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!

[lgray]

🤖 AI text below 🤖

Design context: where PR-0 sits in the infinite-combo-detection plan

This PR is the first slice of a larger, staged effort. The full design lives in two planning docs that are not in the source tree (they're local/untracked), so this comment summarizes their scope and the PR roadmap so the change has context for review. Happy to land the docs under docs/ or a .planning/ directory if you'd prefer them tracked.

Goal

Ship an infinite-combo confirmation tool: given a board (or card set), verify a claimed deterministic loop and report what resource goes unbounded (damage, mana, tokens, proliferate triggers, mill, …). This enables three downstream wins — auto-shortcutting loops in live play per CR 732, an AI that recognizes/executes its own winning lines, and a UI that renders ∞ instead of forcing manual iteration.

Why this is a narrow extension, not greenfield

The engine already detects and draws repeating mandatory loops (CR 104.4b / 732.4) via GameState::loop_fingerprint() + the loop_window deep-equality buffer in the resolution loop (game/engine.rs). The genuine gap is net-progress (beneficial) loops — the kind that make progress on a resource each cycle so the board returns identical while a counter climbs. Today emit_resolution_halt() merely halts those (its own comment: a net-progress loop is "a CR 732.2 shortcut the engine cannot infer an iteration count for"). The detector fills exactly that gap, reusing the existing fingerprint/window machinery rather than building parallel infra.

The design is deliberately sound-but-incomplete and resource-bounded: Magic is Turing-complete (arXiv:1904.09828), so no detector can be complete. It never falsely confirms a loop; it confirms the deterministic resource-loop class that dominates real play. Theory basis: combo = directed-graph cycle detection (Tarjan SCC); "what goes infinite" = Petri-net / VASS coverability; confirmation = bounded model checking via a state fixpoint modulo monotone resources — which is precisely what this PR's loop_states_equal_modulo_resources provides.

Acceptance suite (the spec's definition of done)

3 driving combos + a card-disjoint 50-combo corpus (113 distinct cards, all verified present in card-data.json, every combo a real Commander Spellbook listing). 49/53 are end-to-end testable on today's engine; the other 4 are gated only on a handful of cards still carrying Effect::Unimplemented parts (Doc Aurlock, Professor Onyx, Animate Dead, Grindstone+Painter's Servant) and flip green automatically as those cards complete — no detector change. The detector is decoupled from card coverage.

PR roadmap

PR	Scope	Risk	Behavior change?
0 (this PR)	ResourceVector (the monotone resource axes) + loop_states_equal_modulo_resources (complement of the strict CR 104.4b equality). Reuses loop_fingerprint / normalize_for_loop.	low	none (additive)
1	Offline analysis sim harness around GameRunner::act that feeds ResourceVector's event-fed fields (damage/tokens/draws/casts/triggers) from the run.	low	none (offline)
2	Net-progress detection at the existing loop_window site → emit LoopCertificate { unbounded, win_kind, mandatory }. Unlocks all 49 testable corpus combos.	low	none (offline)
3	Classify emit_resolution_halt net-progress in live play → loop shortcut (CR 732.2a) or draw if all-mandatory (CR 732.4).	med	yes (resolution)
4	(optional) static ability-graph (Engine B) for card-list scanning — not on the corpus critical path.	low	none
5	cargo combo-verify CLI/API over the corpus.	low	none
6	∞ unbounded-resource display (generalizes the existing debug_infinite_mana).	med	UI
7	Loop shortcut + opponent response window (CR 732.2a / 732.5).	high	yes (interactivity)
8	AI coupling — LoopCertificate drives the existing phase-ai/src/combo layer (which today asserts loops rather than verifying them).	med	AI

The corpus-passing milestone is PR-0 + PR-1 + PR-2 (~500–900 net-new LOC, offline, zero gameplay change, validated by 49 concrete tests). PR-3 adds the live shortcut; PR-4 (the only high-churn item) is explicitly not required to pass the corpus. Each cluster is independently reviewable, and clusters 0–2 add zero game-behavior change.

What PR-0 specifically contributes to that arc

ResourceVector is the typed catalogue of every unbounded-resource family the corpus exercises (mana, life, damage, mill, tokens, draws, casts, landfall/combat/turn counts, death/ETB/LTB/sac triggers, counters by (CounterClass, ObjectClass), and generic triggers like proliferate). loop_states_equal_modulo_resources is the "state fixpoint modulo monotone resources" primitive the confirmation stage (PR-2) is built on — it answers "same board, resources may differ?" as the exact complement of the engine's existing "same everything including resources?" mandatory-loop check. Nothing here is wired into a behavior path yet; PR-1/PR-2 do that.

[lgray]

@matthewevans I'm going to do this one with humans in the loop, I'll wait for PR-0 to get merged, then PR-1, PR-1, PR-2, are all fairly benign so we can talk about them together, and then we can go lock-step for the test, etc. So we can have any proper design discussion that needs to happen.

[matthewevans]

Thanks for splitting this out as PR-0. I agree with the direction of having a separate resource-projected equality for beneficial-loop analysis, but I don't think this base predicate is safe yet.

The blocker is that loop_states_equal_modulo_resources currently projects out every object counter (object.counters.clear()), while CounterClass::Other deliberately folds the long-tail counter kinds together. In this engine, several of those long-tail counters are not merely monotone resources: stun counters prevent untapping and are consumed during untap (CR 122.1d), shield counters are consumed by destruction/damage replacement effects and can gate continuous control effects, lore counters drive Saga chapter/SBA behavior, and time counters drive suspend/impending/vanishing-style duration behavior. Because all of those are erased by the projection, a loop cycle that consumes one of those finite counters while producing damage/mana/etc. can compare as the “same board + net resource,” even though the cycle is not repeatable after the counter is gone. That is a false positive in the equality primitive that PR-1/PR-2 would trust.

Concrete code evidence:

• crates/engine/src/analysis/resource.rs:88-100 classifies all long-tail counters as Other.
• crates/engine/src/analysis/resource.rs:542-548 clears every object's counters before equality.
• Existing engine semantics show these counters affect future legality/state: CounterType::Stun documents/removes on untap in crates/engine/src/types/counter.rs:26-35 and crates/engine/src/game/turns.rs:903-914; shield counters are consumed by destruction in crates/engine/src/game/effects/destroy.rs:643-659 and can gate control duration in crates/engine/src/game/effects/gain_control.rs:441-465.

Please narrow the projection to counters that are actually safe monotone resource axes, or make the counter classification preserve enough semantics that consumable/duration/state-gating counters remain part of loop-state equality. Add a regression where state B has a consumed stun/shield/time/lore-style counter plus a positive resource delta and prove loop_states_equal_modulo_resources rejects it.

There's also a smaller correctness gap in is_net_progress: the module docs explicitly list “mill 1 more card” as a beneficial loop resource, and unbounded_components reports negative library_delta, but is_net_progress only sets progress when some component is > 0 (resource.rs:333-360). A pure mill loop with only library_delta = -N is therefore not net progress. If the intended contract is that mill can be the unbounded resource, this should be fixed and covered with a direct test; if PR-1 is supposed to add a separate positive event axis for mill, the contract here should not claim negative library delta itself can satisfy progress.

[lgray]

🤖 AI text below 🤖

Rebased onto current upstream/main (inventory untouched — these are additive analysis types, not Effect/ability variants).

Addressed the review via the full /engine-implementer pipeline: the monotone/consumable counter partition now lives on CounterType::is_monotone_loop_resource (exhaustive match, so a future counter variant is a compile error), project_out_resources retains non-monotone counters instead of clearing all, and is_net_progress recognizes pure-mill (negative library_delta). Also corrected a pre-existing wrong Lore CR annotation (714.1 → 714.3 + 714.4). Purely additive, zero game-behavior change.

New head: 27ef23ddf.

[matthewevans]

VERDICT: approve

Re-reviewed current head 27ef23d after the follow-up. The two prior blockers are addressed: project_out_resources now only projects counters classified by CounterType::is_monotone_loop_resource() and preserves shield/stun/time/lore/generic-style counters, with a shield-consumption negative test plus a +1/+1 positive control; ResourceVector::is_net_progress() now recognizes pure negative library_delta as mill progress, with empty-delta and consumed-life controls.

Repush guard held immediately before approval, the required enhancement label is present, merge state is CLEAN, and the current-head GitHub checks are green.