fix(parser+engine): parse "after blockers are declared" casting window (Aleatory, Chaotic Strike, Curtain of Light, Flash Foliage)

[glorydavid03023]

Summary

Fixes #4996. Spells that print Cast this spell only during combat after blockers are declared. silently lost the after blockers are declared qualifier, becoming castable during all of combat (begin-combat and declare-attackers included) — strictly more permissive than the printed text (CR 601.3 timing violation).

parse_timing_restriction in oracle_casting.rs dispatched during … / before … / on … but had only a lone after combat leaf, so after blockers are declared matched nothing and the scan loop skipped it. This mirrors the existing before handling (parse_before_phrase → BeforeBlockersDeclared), which was already complete — only the after side was missing.

The fix adds the after prefix dispatch as an exact complement of before within the combat phase.

Covers a class (4 real cards below), not a single card.

Affected cards: Aleatory, Chaotic Strike, Curtain of Light, Flash Foliage. (Activated-ability sibling on the ActivationRestriction axis: Trap Runner — out of scope here.)

Files changed

• crates/engine/src/types/ability.rs — new CastingRestriction::AfterBlockersDeclared variant, placed in the combat-window cluster.
• crates/engine/src/parser/oracle_casting.rs — new parse_after_phrase sub-combinator (mirror of parse_before_phrase); parse_timing_restriction now dispatches preceded(tag("after "), parse_after_phrase), folding the former lone after combat leaf into it. Building-block parser test added.
• crates/engine/src/game/restrictions.rs — enforcement arm gating on Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat. Runtime enforcement test added.

CR references added/touched

• CR 509.1 — blockers are declared during the declare-blockers step (turn-based action).
• CR 506.1 — the combat phase's five steps; AfterBlockersDeclared ∪ BeforeBlockersDeclared partitions them.
• CR 510 / CR 511 — combat damage and end-of-combat steps remain within the window.
• CR 601.3 — casting-restriction enforcement (the timing rule being violated pre-fix).

Track

Non-developer — no local Rust toolchain in this environment, so local cargo verification was not run. CI runs clippy, test-engine, card-data, and the combinator-purity gate on this PR.

LLM

• Model: Claude Opus 4.8
• Thinking level: high

Gate A — nom combinators

Not run locally (no toolchain). The change is pure-combinator by construction: parse_after_phrase uses only alt, value, tag and is a line-for-line structural mirror of the adjacent parse_before_phrase; parse_timing_restriction gains one preceded(tag("after "), …) arm. No find/split_once/contains/starts_with or verbatim Oracle-text match is introduced. CI's combinator-purity script is authoritative and will confirm.

Gate B — pattern anchoring

Two analogous in-tree implementations mirrored:

1. crates/engine/src/parser/oracle_casting.rs:697 — parse_before_phrase and its BeforeBlockersDeclared arm (tag("blockers are declared")). parse_after_phrase is the direct mirror.
2. crates/engine/src/game/restrictions.rs:974 — CastingRestriction::BeforeBlockersDeclared => matches!(state.phase, Phase::BeginCombat | Phase::DeclareAttackers). The new arm is its exact complement within combat.

Enum-cluster anchor: crates/engine/src/types/ability.rs:13803-13806 (BeforeAttackersDeclared/BeforeBlockersDeclared/BeforeCombatDamage/AfterCombat) — the new variant slots into the existing combat-window cluster; no parameterization refactor is triggered (it extends the pre-existing per-window modeling the engine already uses, and AfterBlockersDeclared/BeforeBlockersDeclared are true CR complements, not a scope/comparator axis).

Scope expansion

None.

Validation failures

None.

CI failures

None.

Tier: Standard

[gemini-code-assist]

Code Review

This pull request implements the AfterBlockersDeclared casting restriction, updating the game engine, parser, and associated tests to support spells castable only after blockers are declared. The review feedback highlights a critical bug where the timing window is incorrectly active if the declare blockers step is skipped (violating CR 506.6f), and notes a corresponding gap in the test coverage. Additionally, the feedback points out missing mandatory CR annotations in ability.rs and oracle_casting.rs as required by the repository style guide.

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]

[HIGH] Incorrect timing window when declare blockers step is skipped. Evidence: crates/engine/src/game/restrictions.rs:982.

Why it matters: Under CR 506.6f, if the declare blockers step is skipped (which occurs when no creatures are declared as attackers or put onto the battlefield attacking, per CR 508.8), spells that can only be cast "after blockers are declared" cannot be cast during that combat phase at all. However, the current implementation checks only matches!(state.phase, Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat). If the declare blockers step is skipped, the phase transitions directly to Phase::EndCombat, where this check would incorrectly evaluate to true.

Suggested fix: Ensure that AfterBlockersDeclared evaluates to false if the declare blockers step was skipped (e.g., by checking if there are any attacking creatures).

        CastingRestriction::AfterBlockersDeclared => {
            let blockers_declared = state.combat.as_ref().is_some_and(|c| !c.attackers.is_empty());
            blockers_declared && matches!(
                state.phase,
                Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat
            )
        }

[gemini-code-assist]

[MEDIUM] Missing mandatory CR annotation. Evidence: crates/engine/src/types/ability.rs:13806.

Why it matters: According to the Repository Style Guide (R6), every rules-touching line of engine code in types/ must carry a comment of the form CR <number>: <description>.

Suggested fix: Add a CR 506.6f annotation to the AfterBlockersDeclared variant.

Suggested change

	AfterBlockersDeclared,
	/// CR 506.6f: Casting restriction for spells castable only after blockers are declared.
	AfterBlockersDeclared,

References

1. Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[gemini-code-assist]

[MEDIUM] Missing mandatory CR annotation. Evidence: crates/engine/src/parser/oracle_casting.rs:719.

Why it matters: According to the Repository Style Guide (R6), every rules-touching line of engine code in parser/ must carry a comment of the form CR <number>: <description>.

Suggested fix: Add a CR 506.6f annotation to the parse_after_phrase parser function.

/// CR 506.6f: Parses the "after blockers are declared" casting restriction.
fn parse_after_phrase(input: &str) -> nom::IResult<&str, CastingRestriction, OracleError<'_>> {

References

1. Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[gemini-code-assist]

[MEDIUM] Test does not cover skipped declare blockers step. Evidence: crates/engine/src/game/restrictions.rs:2705.

Why it matters: The test should verify the edge case where the declare blockers step is skipped (CR 506.6f). In this scenario, even during Phase::EndCombat, the casting restriction should apply (i.e., return false for casting permission) because blockers were never declared.

Suggested fix: Add an assertion verifying that AfterBlockersDeclared evaluates to false during Phase::EndCombat if no attackers were declared.

[matthewevans]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

Requesting changes on current head 625ea621.

The new AfterBlockersDeclared gate is still phase-only:

CastingRestriction::AfterBlockersDeclared => matches!(
    state.phase,
    Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat
),

That admits any DeclareBlockers state, including the pre-declaration waiting state before the CR 509.1 blocker-declaration turn-based action has run. The codebase already tracks the relevant event in CombatState::blockers_declared_by and fills it in handle_declare_blockers, so using only state.phase is too coarse.

CR 506.7f is stricter for “during combat after blockers are declared”: if the declare-blockers step is skipped, the spell may not be cast during that combat. CR 509.1/509.2 also make the blocker declaration action the opening point, not the phase label. The current test sets only state.phase = Phase::DeclareBlockers, so it proves the bug instead of guarding the edge.

Please gate AfterBlockersDeclared on blocker-declaration history or an equivalent engine signal that the declaration action has actually completed, and add regressions for skipped declare-blockers/no attackers plus declare-blockers before vs. after the empty-blockers action.

[matthewevans]

I still need changes on current head 055d0dc86d06fffec6e7406ac7c5d9870d92008d.

The runtime gate is still phase-only:

CastingRestriction::AfterBlockersDeclared => matches!(
    state.phase,
    Phase::DeclareBlockers | Phase::CombatDamage | Phase::EndCombat
),

That does not prove blockers have actually been declared. It admits the pre-declaration DeclareBlockers waiting state, and it also admits combats where the declare-blockers step was skipped. CR 506.7f explicitly says spells with “only during combat after blockers are declared” cannot be cast in a combat where the declare-blockers step is skipped, and CR 509.1 makes the declaration a turn-based action rather than the phase label itself.

The test added in this revision encodes the same bug: it sets only state.phase = Phase::DeclareBlockers and expects the restriction to apply. Please gate this on blocker-declaration history or an equivalent engine signal that the declare-blockers turn-based action has completed, and add the skipped-declare-blockers / pre-vs-post empty-blockers regressions from the prior review.

[glorydavid03023]

Thanks — you're right, the phase-only gate was a rules approximation. Fixed in bbbb228.

The gate now keys on the actual CR 509.1 turn-based action rather than the phase label. CombatState::blockers_declared_by records each defending player as they declare blockers (in declare_blockers_for_player, combat.rs — the push is unconditional, so a declaration of zero blockers still counts) and is never cleared mid-combat:

fn is_after_blockers_declared(state: &GameState) -> bool {
    state.phase.is_combat()
        && state
            .combat
            .as_ref()
            .is_some_and(|combat| !combat.blockers_declared_by.is_empty())
}

This closes both holes you flagged:

• Pre-declaration DeclareBlockers window — blockers_declared_by is still empty until the turn-based action runs, so the priority window before declaration is excluded.
• Skipped declare-blockers step (CR 506.7f) — a combat that never runs the declaration never populates blockers_declared_by, so combat damage / end of combat stay excluded.
• Pre-vs-post empty-blockers — distinguished, because a zero-blocker declaration still records the defending player (empty blocker_assignments ≠ empty blockers_declared_by).

The runtime test (after_blockers_declared_requires_the_declare_blockers_turn_based_action) now exercises: pre-declaration exclusion, post-declaration inclusion persisting through combat damage and end of combat, the skipped-declare-blockers combat (excluded in both damage and end steps), the before-blockers phases, and outside-combat.

[matthewevans]

Requesting changes on current head bbbb228130c9a7e274dddeb0e53a262d996acc2d.

The phase-only issue is fixed for the direct CombatState fixture, but the new gate is still wrong on the production end-of-combat path. is_after_blockers_declared now requires state.combat.as_ref().is_some_and(|combat| !combat.blockers_declared_by.is_empty()), while turns::auto_advance clears state.combat = None as soon as it enters Phase::EndCombat and before it returns end-combat priority for triggers:

// turns.rs, Phase::EndCombat
state.combat = None;
...
if triggers_fired {
    return WaitingFor::Priority { player: state.active_player };
}

CR 506.1 still makes EndCombat part of the combat phase, and CR 506.7f only excludes a combat where the declare-blockers step was skipped. So after blockers were declared, the spell should still be legal in the end-of-combat priority window, but production state has already lost the only latch this restriction reads.

The added test misses this because it constructs Phase::EndCombat with a synthetic non-empty CombatState; the real turn path clears that state first. Please preserve the blocker-declared latch until the combat phase actually leaves, or otherwise store the fact separately from state.combat, and add a production-path regression that reaches end-combat priority after blockers were declared and proves AfterBlockersDeclared still applies there.