fix(parser): parse enters-with-counters unless-conditions (Hotheaded …

[keloide]

Summary

Fixes the silently-misparsed ETB replacement of Hotheaded Giant — "This creature enters with two -1/-1 counters on it unless you've cast another red spell this turn." The " unless …" tail was dropped (condition: null), so the counters were ALWAYS applied. Built for the class "enters with [counter payload] on it unless [game-state condition]": Steel Exemplar ("… unless two or more colors of mana were spent to cast it") is fixed by the same seam, and the own-cast-exclusion resolver semantics also make the pre-existing "other spells you've cast this turn" effect-quantity class (Thunder Salvo, Lock and Load) rules-correct instead of accidentally-fail-closed-to-0.

Key decisions: unless-tails route through parse_inner_condition (single condition authority) into the existing ReplacementCondition::UnlessQuantity — zero new enum variants; "another red spell" is resolved object-relatively (SpellCastRecord.spell_object_id provenance + positional last-same-id exclusion, matching the official Gatherer ruling for reanimated entries) instead of the naive count ≥ 2 encoding; unparseable unless-tails and co-occurring condition suffixes fail closed to Effect::unimplemented (coverage stays honest).

Implementation method (required)

• Produced via the /engine-implementer pipeline (plan → review-plan → implement → review-impl → commit)
• Not /engine-implementer — explain why below

Plan-review ran 3 rounds (2 gap rounds + clean); implementation-review ran 2 rounds — round 1 returned 1 HIGH + 2 LOW findings (all fixed with code in a fresh executor round), round 2 verified the fixes and returned 1 LOW comment-only finding (a CR citation, applied verbatim per the reviewer's grep-verified prescription) and explicitly "no other findings".

Files changed

• crates/engine/src/parser/oracle_replacement.rs
• crates/engine/src/parser/oracle_nom/condition.rs
• crates/engine/src/types/ability.rs
• crates/engine/src/types/game_state.rs
• crates/engine/src/game/quantity.rs
• crates/engine/src/game/restrictions.rs
• crates/engine/src/game/filter.rs
• crates/engine/src/game/casting.rs
• crates/engine/src/game/replacement.rs
• crates/engine/src/game/triggers.rs
• crates/engine/src/game/casting_tests.rs
• crates/engine/src/game/enters_with_unless_runtime_tests.rs (new)
• crates/engine/tests/integration/narset_jeskai_waymaster_draw_spells_cast.rs

CR references

CR 106.3, CR 109.1, CR 207.2c, CR 400.7, CR 400.7d, CR 601.2h, CR 601.2i, CR 603.4, CR 608.2n, CR 614.1c, CR 614.1d, CR 614.12, CR 614.12a, CR 700.4, CR 702.33d, CR 702.138c, CR 702.188a — every number grep-verified against docs/MagicCompRules.txt before annotation.

Track

Developer

LLM

Model: Claude Fable 5
Thinking: high

Tier: Frontier

Gate A

./scripts/check-parser-combinators.sh → exit 0. The script prints no output on success; no violations. (No .contains/.split_once/.starts_with/match-arm string-literal dispatch introduced; the only grep hits are Vec<FilterProp>::contains structural AST assertions inside #[cfg(test)] blocks.)

Anchored on

• crates/engine/src/parser/oracle_replacement.rs:3421 — extract_enters_with_only_if_suffix: the existing " if " suffix extractor on enters-with replacements; the new extract_enters_with_unless_suffix mirrors its split/trim/full-consumption shape for the " unless " polarity.
• crates/engine/src/parser/oracle_replacement.rs:8656 — parse_turn_of_game_condition: the existing "enters tapped unless …" parser emitting ReplacementCondition::UnlessQuantity (Starting Town) — the same suppressed-when-true carrier the new code reuses.
• crates/engine/src/parser/oracle_replacement.rs:3967 — replacement_condition_from_static: the existing positive StaticCondition→ReplacementCondition mapper; the new replacement_condition_from_static_unless is its unless-polarity sibling.
• crates/engine/src/parser/oracle_nom/condition.rs:5222 — parse_mana_spent_threshold: the existing "N or more mana was spent to cast" condition family; the new parse_colors_of_mana_spent_threshold is a sibling arm composed from the same factored parse_amount_threshold building block.

Verification

Developer track — Tilt is down in this environment; direct cargo used (sanctioned fallback). WASM tooling unavailable (engine-only change; no frontend/WASM surface touched).

• cargo fmt --all — clean
• ./scripts/check-parser-combinators.sh — clean (exit 0)
• cargo clippy -p engine --lib -- -D warnings — clean (exit 0)
• cargo test -p engine --lib — 15,174 pass / 0 fail (includes all new parser-shape, condition-arm, resolver building-block, and cast-pipeline runtime discriminators; the ETB suppression tests flip if the fix is reverted)
• cargo test -p engine full integration binaries were NOT built locally (disk-limited container; linking 145 binaries exceeds the allowance) — the one integration file this diff touches compiles and is covered by CI
• ./scripts/gen-card-data.sh (full regen) — Hotheaded Giant & Steel Exemplar: 0 Unimplemented entries, condition: UnlessQuantity serialized; both cards no longer appear in parser-warning patterns; global coverage 31357 → 31359 (+2, exactly these cards); Thunder Salvo / Lock and Load parse output unchanged
• cargo coverage — Hotheaded Giant: supported: true, gap_count: 0; Steel Exemplar: supported: true, gap_count: 0
• cargo semantic-audit (full DB) — 0 findings for Hotheaded Giant, Steel Exemplar, Thunder Salvo, Lock and Load (all 317 global findings pre-date this change)

Runtime discriminators (real cast/replacement pipeline): red spell → Giant enters with 0 counters; Giant first → two −1/−1; reanimated after a red cast → 0 counters (Gatherer ruling); die-and-recast → 0 counters (positional own-cast exclusion); Thunder Salvo alone deals 2 (was 3 mid-fix, 2-by-accident before); Steel Exemplar two colors → 0 counters, mono → 2, un-cast entry → 2; unless+kicker co-occurrence fails closed.

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R3a9PpLzMRJYFuQY4Z38U5

---

Generated by Claude Code

[gemini-code-assist]

Code Review

This pull request implements support for the "enters with [counters] on it unless [game-state condition]" replacement class (CR 614.1c), such as for Hotheaded Giant and Steel Exemplar. It introduces tracking of the stable storage ID (spell_object_id) in SpellCastRecord to correctly handle own-cast exclusions (CR 109.1 / CR 601.2i) when counting spells cast this turn. Additionally, it adds parsing for colors of mana spent thresholds and includes comprehensive runtime tests. Feedback is provided to simplify the peel_own_cast_exclusion helper in TargetFilter by comparing against TypedFilter::card() directly rather than manually checking individual fields.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

[MEDIUM] Simplify and improve robustness of peel_own_cast_exclusion by comparing against TypedFilter::card() directly.

Why it matters: Checking individual fields of TypedFilter manually is error-prone and fragile. If new fields (like subtypes, supertypes, or colors) are added to TypedFilter in the future, or if any existing fields are non-empty, this manual check could incorrectly classify a filter with other constraints as a bare marker, leading to silent bugs where constraints are discarded. Comparing the peeled filter directly to TypedFilter::card() is simpler, safer, and future-proof.

        let is_bare_marker = |f: &TargetFilter| {
            if let TargetFilter::Typed(t) = f {
                let mut peeled = t.clone();
                peeled.properties.retain(|p| p != &FilterProp::Another);
                peeled == TypedFilter::card()
            } else {
                false
            }
        };
        match self {
            TargetFilter::Typed(typed) if typed.properties.contains(&FilterProp::Another) => {
                let mut peeled = typed.clone();
                peeled.properties.retain(|p| p != &FilterProp::Another);
                if peeled == TypedFilter::card() {
                    Some(None)
                } else {
                    Some(Some(TargetFilter::Typed(peeled)))
                }
            }

[matthewevans]

Maintainer update: resolved the origin/main conflict and pushed 7254d2e8d3. The merge preserves main's shared fuse-aware SpellCastRecord projection and adds this PR's cast-object provenance at that single authority; I also applied the outstanding filter-marker review feedback.

Evidence: targeted diff review plus an independent adversarial pass; the parser-combinator gate passed against origin/main. Tilt explicitly reports it watches the primary checkout rather than this isolated PR worktree, so I did not run direct cargo builds/tests; required CI and parser-diff evidence must complete before approval or enqueue.

[matthewevans]

Maintainer follow-up: CI identified an unused compatibility wrapper left by the conflict-resolution merge. I removed that obsolete wrapper in ad501c254b; the canonical fuse-aware record projection remains unchanged. Fresh CI and parse-diff evidence on this new head are required before approval or enqueue.

[github-actions]

Parse changes introduced by this PR · 2 card(s), 2 signature(s) (baseline: main bd770b5670a7)

🟡 Modified fields (2 signatures)

• 1 card · 🔄 replacement/Moved · changed field condition: ∅ → UnlessQuantity { lhs: Ref { qty: ManaSpentToCast { scope: SelfObject, metric: DistinctColors } }, comparator: GE, rhs: …
  • Affected (first 3): Steel Exemplar
• 1 card · 🔄 replacement/Moved · changed field condition: ∅ → UnlessQuantity { lhs: Ref { qty: SpellsCastThisTurn { scope: Controller, filter: Some(Typed(TypedFilter { type_filters:…
  • Affected (first 3): Hotheaded Giant

[matthewevans]

Approved after maintainer conflict resolution and CI revalidation: the replacement-condition parser and cast-history provenance changes are at their shared authorities, with discriminating runtime coverage for Hotheaded Giant, Steel Exemplar, and own-cast exclusion.

[matthewevans]

Follow-up review on 463442b: the post-queue change is a safe structural equality check against TypedFilter::card(), preserving the intended fail-closed behavior if that canonical filter gains non-default fields. The queued approval remains valid; GitHub is awaiting checks for this updated head.

[matthewevans]

Fresh approval for 463442b after its post-queue review. The structural TypedFilter::card() comparison is correct and preserves the fail-closed residual-filter behavior; fresh CI remains required by the merge queue.

[matthewevans]

Fresh approval for 463442b after its post-queue review. The structural TypedFilter::card() comparison is correct and preserves the fail-closed residual-filter behavior; fresh CI remains required by the merge queue.

[matthewevans]

Fresh approval was submitted for 463442b after its post-queue review. I deliberately removed the stale merge-queue entry first; GitHub accepted auto-merge but currently reports this PR as BEHIND and has not re-added it to the queue. It is therefore not being represented as enqueued in this sweep.