feat(engine): support "damage isn't removed during cleanup" statics

[philluiz2323]

Problem

Cards with a "Damage isn't removed from [subject] during cleanup steps" static were unsupported — the clause wasn't modeled, so the static fell to Unrecognized: Ancient Adamantoise, Patient Zero, Uthgardt Fury, Case of the Market Melee.

CR 514.2: removing all marked damage is a turn-based action during the cleanup step. These statics suppress that removal for the permanents they match, so marked damage persists across turns. The engine had no mechanism — execute_cleanup cleared every permanent's damage_marked unconditionally.

Fix

• StaticMode::DamageNotRemovedDuringCleanup — a nullary marker whose subject rides the definition's affected filter (this creature → SelfRef; creatures your opponents control → a typed filter). Classified data-carrying in coverage (enforced by a bespoke turn-based hook, not the layer registry).
• Parser parse_damage_not_removed_during_cleanup composes the sanctioned scan_contains / split_once_on_lower / parse_type_phrase building blocks (parser gate clean).
• Runtime: execute_cleanup gathers the permanents matched by an active such static (battlefield_active_statics + matches_target_filter) and skips clearing their marked damage.

Verification

• Parser tests: self-subject → SelfRef; typed subject → opponent-creatures filter (not collapsed to SelfRef).
• Runtime scenario execute_cleanup_preserves_damage_under_damage_not_removed_static: a protected permanent keeps its marked damage through cleanup while a normal creature's is removed.
• Full engine lib suite green (12413 passed, 0 regressions); workspace check clean (no cross-crate exhaustive-match breaks); clippy -D warnings clean.
• Local regen: 4 cards flip to supported, 0 regressions.

Closes #3644

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

VERDICT: request-changes

[MED] Parser arm is ad hoc and can over-accept instead of requiring the cleanup sentence grammar. Evidence: crates/engine/src/parser/oracle_static/dispatch.rs adds scan_contains prechecks plus split_once_on_lower substring extraction, then ignores the exact tail after " during " instead of all-consuming "during cleanup steps". Why it matters: this violates the repo's nom-first parser seam and can mark unrelated/future text as supported while bypassing grammar coverage. Suggested fix: rewrite this as a composed nom parser using the existing self-reference/type-phrase building blocks plus an explicit cleanup-step suffix.

[MED] Cleanup protection matching drops phased-out affected permanents even though this code is implementing CR 514.2's cleanup population. Evidence: crates/engine/src/game/turns.rs builds the protected set with matches_target_filter, whose normal object matcher excludes phased-out objects; CR 514.2 says cleanup removes damage from permanents including phased-out permanents, and the repo already has matches_target_filter_including_phased_out. Why it matters: a Patient Zero/Uthgardt Fury-style static will not preserve damage on a matching phased-out creature during the same cleanup action that otherwise still clears phased-out damage. Suggested fix: evaluate the protected set with the phased-out-aware matcher and add a phased-out affected-permanent regression test, or document/test the intentional CR 702.26b interpretation if exclusion is deliberate.

[LOW] data/engine-inventory.json is stale after adding the StaticMode variant. Evidence: StaticMode::DamageNotRemovedDuringCleanup is added in crates/engine/src/types/statics.rs, but the tracked engine inventory does not include it. Why it matters: the repo treats this generated inventory as the canonical engine-surface index for variant review and sibling-cluster checks. Suggested fix: regenerate and commit data/engine-inventory.json.

[matthewevans]

VERDICT: request-changes

[MED] The parser still uses the same over-accepting scan/split shape instead of an all-consuming cleanup sentence grammar. Evidence: crates/engine/src/parser/oracle_static/dispatch.rs still gates on scan_contains(..., "damage isn't removed from") plus scan_contains(..., "cleanup"), then splits at the first "damage isn't removed from " and first " during " without proving the suffix is exactly the cleanup-step phrase. Why it matters: this can classify text that merely mentions cleanup somewhere later, rather than only the Oracle class Damage isn't removed from <subject> during cleanup steps. Suggested fix: parse the whole predicate as a structured grammar, ending on the cleanup-step suffix, and add a negative parser test for a non-cleanup during ... sentence with a later cleanup mention.

[MED] The cleanup hook still evaluates protected objects with the normal phased-out-excluding matcher. Evidence: crates/engine/src/game/turns.rs builds damage_persists by iterating battlefield ids and calling matches_target_filter; that choke point excludes phased-out objects, and the code does not use the phased-out-aware matcher. Why it matters: cleanup damage removal is a turn-based cleanup action, not targeting, so a static that says damage is not removed from a class should be evaluated against the cleanup population rather than through the ordinary target-legality visibility gate. Suggested fix: use the phased-out-aware filter path for this cleanup-only membership check, or add a rules-backed test showing phased-out damaged permanents intentionally remain outside both cleanup removal and this prevention hook.

[matthewevans]

Formalizing my blocking review comment: this still needs the cleanup parser grammar and cleanup-time phased-out matcher issue addressed before merge.

[philluiz2323]

Thanks — all three addressed:

• Parser grammar (MED): rewritten as a composed nom grammar — nom_tag_lower("damage isn't removed from ") → self-reference (~/this creature/this permanent) or parse_type_phrase subject → an anchored " during cleanup steps" suffix that must be end-of-sentence (only an optional period may follow, else it bails). No more scan_contains/substring split. Added a negative test (..._rejects_non_cleanup_during) for a during combat … sentence that merely mentions cleanup later.
• Phased-out matcher (MED): the cleanup protected set is now built with matches_target_filter_including_phased_out, mirroring the unconditional removal it guards (CR 514.2 cleanup is a turn-based action over the whole battlefield, incl. phased-out permanents — not targeting). Added a regression test (execute_cleanup_preserves_phased_out_creature_damage_under_static) with a phased-out opponent creature whose damage is preserved.
• Inventory (LOW): regenerated and committed data/engine-inventory.json with the new StaticMode variant.

Full engine lib suite green (12439 passed), clippy -D warnings + parser gate clean.

[matthewevans]

VERDICT: request-changes

Prior findings status: the parser is now anchored to the Damage isn't removed from <subject> during cleanup steps. sentence shape, and the direct execute_cleanup path now uses the phased-out-aware matcher with a regression test. Those two blockers are addressed.

[HIGH] The deferred cleanup-discard path still ignores DamageNotRemovedDuringCleanup. Evidence: crates/engine/src/game/turns.rs:1350 returns WaitingFor::DiscardToHandSize before the new protected-set logic runs, and crates/engine/src/game/turns.rs:1524 then clears every damaged battlefield permanent in finish_cleanup_discard without consulting StaticMode::DamageNotRemovedDuringCleanup. Why it matters: in any cleanup where the active player has to discard, Patient Zero / Ancient Adamantoise / Uthgardt Fury-style statics stop working and protected damage is removed anyway. Suggested fix: share the cleanup damage-clearing helper between execute_cleanup and finish_cleanup_discard so both paths compute the same protected set, and add a regression where cleanup first prompts for discard and then preserves protected marked damage after finish_cleanup_discard.

[philluiz2323]

Both [MED] items are already resolved in the current HEAD (8412bb090) — the cited evidence is from the original revision, not the pushed rework:

• Parser: parse_damage_not_removed_during_cleanup no longer uses scan_contains/split. It is a composed nom grammar (dispatch.rs:243-261): nom_tag_lower("damage isn't removed from ") → self-reference or parse_type_phrase subject → nom_tag_lower(" during cleanup steps") anchored at end-of-sentence (the tail must be empty/., else it bails). Negative test damage_not_removed_during_cleanup_rejects_non_cleanup_during covers a during combat … cleanup later sentence.
• Phased-out: the cleanup membership now uses matches_target_filter_including_phased_out (turns.rs:1367,1388), with regression test execute_cleanup_preserves_phased_out_creature_damage_under_static.
• Inventory regenerated.

Could you re-run the review against 8412bb090? Full lib suite green (12439 passed), clippy -D warnings + parser gate clean.

[matthewevans]

Thanks for the follow-up. I did review the current 8412bb090 head in the later formal review at 2026-06-18T02:23:53Z. That review explicitly marked the two earlier MED findings as addressed:

• parser is now anchored to the Damage isn't removed from <subject> during cleanup steps. sentence shape
• the direct execute_cleanup path now uses matches_target_filter_including_phased_out

The remaining blocker is a different path on the same head: execute_cleanup returns WaitingFor::DiscardToHandSize before the new protected-set logic runs, and then finish_cleanup_discard clears every damaged battlefield permanent without consulting StaticMode::DamageNotRemovedDuringCleanup. On 8412bb090, that is still visible in crates/engine/src/game/turns.rs: the early return is around lines 1350-1358, while the deferred damage clear is around lines 1524-1544.

So the ask is not to revisit the parser/phased-out fixes; those are fine. Please share the cleanup damage-clearing helper between execute_cleanup and finish_cleanup_discard (or otherwise apply the same protected-set computation in both paths), and add a regression where cleanup first prompts for discard and protected marked damage remains after finish_cleanup_discard completes.