fix(engine): add active-voice "that dealt damage this turn" creature filter (#5615)

[philluiz2323]

Summary

Fixes #5615. Red Guardian, Super-Soldier — "When Red Guardian enters, destroy target creature an opponent controls that dealt damage this turn" — could destroy a creature that had dealt no damage. The active-voice damage-source clause had no filter and was silently dropped, so the target collapsed to "creature an opponent controls" (any).

Implementation method (required)

How was the engine/parser logic in this PR produced? Check exactly one:

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

Small, self-contained parser + filter fix, routed through the /add-engine-variant gate before implementation and covered by parser + filter-evaluator unit tests plus the full --lib regression. One new FilterProp variant plus its evaluator and classification arms; no effect/resolver/targeting-flow changes.

Root cause

The parser recognized only the passive "that was dealt damage this turn" (FilterProp::WasDealtDamageThisTurn — the creature is the damage recipient). Red Guardian's clause is the active voice — the creature is the damage source — for which no FilterProp existed. The relative clause was dropped, leaving:

Typed { type_filters: [Creature], controller: Opponent }   // "that dealt damage this turn" lost

so any opponent creature was a legal target.

Fix

Add FilterProp::DealtDamageThisTurn (CR 120.1: "an object that deals damage is the source of that damage"). It reads the same per-turn ledger state.damage_dealt_this_turn the passive filter reads, matching on source_id instead of target — so it reuses existing infrastructure rather than adding new bookkeeping. Wired through:

• the parser verb-phrase table ("dealt damage this turn", distinct first word from the passive "was dealt damage this turn" — no ordering conflict);
• the live and LKI filter evaluators;
• every exhaustive classification site the compiler enforces: read/write profile (ability_rw), projected-resource axis (ability_scan → CONSERVATIVE, same as the passive arm), AI memoization-poison set, swallow-check, event-support, and the coverage doc-emitter.

The /add-engine-variant gate returned EXTEND_OK / WITHIN_SECTION: the pair shares the CR-120 damage-role axis but sits below the 3-sibling refactor threshold; a doc-comment records that a third damage-role filter should fold the pair into DamageThisTurn { role, combat_only }.

CR references

• CR 120.1 — an object that deals damage is the source of that damage (the filtered creature is that source).

Verification

• cargo test -p engine --lib → 16675 passed, 0 failed (full suite; a target-filter change can silently break unrelated cards).
• New oracle_target test: "that dealt damage this turn" parses to DealtDamageThisTurn (active) and NOT WasDealtDamageThisTurn (passive).
• New filter test: the damage source matches DealtDamageThisTurn while the recipient does not — and the exact mirror for the passive filter — proving the two are not conflated.
• cargo fmt --all clean.

[gemini-code-assist]

Code Review

This pull request introduces the FilterProp::DealtDamageThisTurn variant to represent the active-voice counterpart of WasDealtDamageThisTurn (CR 120.1), matching objects that were the source of a damage event this turn. The change integrates this new property across the engine's filter, parser, AI support, coverage, and event snapshot modules, and includes corresponding unit tests to verify both active-voice parsing and correct source-matching behavior. I have no feedback to provide.

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.

[github-actions]

Parse changes introduced by this PR · 4 card(s), 5 signature(s) (baseline: main 553b97bd5c9f)

2 card(s) · ability/Destroy · field duration: until end of turn → ∅

Examples: Avenging Arrow, Red Guardian, Super-Soldier

1 card(s) · ability/BounceAll · field filter: creature → dealt damage this turn creature

Examples: Restore the Peace

1 card(s) · ability/Destroy · field target: creature → dealt damage this turn creature

Examples: Avenging Arrow

1 card(s) · ability/Destroy · field target: opponent controls creature → dealt damage this turn opponent controls creature

Examples: Red Guardian, Super-Soldier

1 card(s) · ability/Pump · field target: creature → dealt damage this turn creature

Examples: Executioner's Swing

[matthewevans]

Verdict: changes requested — the active-voice filter is plausible, but the measured parser impact and production behavior are not yet adequately accounted for.

🔴 Blocker

The current parse-diff reports 32 changed cards, while the PR claims only Red Guardian. crates/engine/src/game/coverage.rs:946-947 explains 30 of those as a passive-voice coverage-label correction (dealt damage → was dealt damage), but the same artifact reports two cards losing until end of turn and two active-voice target additions (Red Guardian and Avenging Arrow). Please explicitly reconcile every category in the sticky diff and verify the two duration removals are correct parser behavior rather than an unintended suffix-consumption change.

The tests exercise parse_target and matches_target_filter directly, but neither drives the affected card through target generation/resolution. Add a production-pipeline regression for Red Guardian (or a shared card-level path) showing that a creature that dealt damage this turn is targetable and an otherwise identical creature that did not is not. That test must fail if the new FilterProp or its parser wiring is reverted.

🟡 Non-blocking

The active and passive damage roles belong to the same CR 120.1 axis, and reusing damage_dealt_this_turn is the right direction. The coverage-label change may be valid; the blocker is that its 32-card blast radius is currently unexplained and untested.

✅ Clean

The live and LKI evaluators both use the source ID for the active voice, while the passive filter remains target-based; the direct filter test correctly distinguishes those roles.

Recommendation: reconcile the full parse-diff and add a discriminating card-level runtime test, then request re-review.

[philluiz2323]

Thanks — both points addressed in the latest push.

Parse-diff reconciled (all 32 categories):

• 30 cards (dealt damage this turn → was dealt damage this turn): these were purely a coverage-label change I made to WasDealtDamageThisTurn's description string — the parsed AST was identical (still WasDealtDamageThisTurn). I've reverted that label change; both damage-role arms now share the "dealt damage this turn" label, so these 30 cosmetic entries drop out of the diff entirely.
• 2 cards losing until end of turn → ∅ (Red Guardian, Avenging Arrow): correct, and intended. Neither card's Oracle text contains "until end of turn". The old parser failed to consume "that dealt damage this turn" as a filter and mis-read the trailing bare "this turn" as a spurious duration: until end of turn. Consuming it as DealtDamageThisTurn removes that phantom duration. The new card-level test locks in the correct target set for these cards.
• Active-voice target additions (creature → dealt damage this turn creature, etc.): the genuine fix — Red Guardian and Avenging Arrow (Destroy), plus a BounceAll and a Pump card, all of which read "that dealt damage this turn" and previously dropped the filter.

Card-level runtime test added: red_guardian_targets_only_a_creature_that_dealt_damage_this_turn parses the card's real Oracle text and drives the resulting filter through the production find_legal_targets authority — an opponent creature that dealt damage this turn is targetable; an otherwise-identical one that did not is not. It fails if the FilterProp or its parser wiring is reverted.

cargo test -p engine --lib → 16675+ passed, 0 failed. Re-requesting review.