fix(engine): correct CR citation and preserve as-long-as gate in damage-redirection Pattern 3 (#5518 review feedback)

[rykerwilliams]

Summary

Addresses two medium-priority gemini-code-assist review comments left on the already-merged #5518 (Veteran Bodyguard / Weathered Bodyguards tap-gate fix), after the PR had already gone through CI and merged via the merge queue before the bot's comments landed.

1. The "combat damage" qualifier comment in parse_damage_redirection_replacement cited CR 615 (Prevention Effects), which is wrong — combat damage itself is defined under CR 120.2a. Corrected the annotation (verified against docs/MagicCompRules.txt directly, not the bot's suggestion alone).
2. Pattern 3 in the same function (Pariah's Shield's "if a source would deal damage to you, prevent that damage" shape) built and returned its ReplacementDefinition without ever attaching prefix_condition — the leading "as long as <tap-state>" gate extracted at the top of the function. No shipped card currently matches Pattern 3 with that prefix (Pariah / Pariah's Shield never carry it), so this was latent rather than live-wrong, but it would have silently dropped the gate for any future card matching that shape. Pattern 3 now attaches prefix_condition via .condition(), mirroring Pattern 1/2's existing handling 20 lines above in the same function.

Files changed

• crates/engine/src/parser/oracle_replacement.rs

Gate A

$ ./scripts/check-parser-combinators.sh
(exit 0, no output — no violations)

Anchored on

• crates/engine/src/parser/oracle_replacement.rs:8146-8148 (Pattern 1/2's existing if let Some(cond) = prefix_condition { def = def.condition(cond); }) — the exact precedent Pattern 3 now mirrors verbatim, in the same function.

CR references

• CR 120.2a — combat damage definition (corrected citation)
• CR 604.2 — static ability's own "as long as" continuous-effect activation gate (Pattern 3's now-preserved prefix condition)

Track

Developer

LLM

Model: claude-sonnet-5
Thinking: high

Verification

• cargo fmt --all — clean
• ./scripts/check-parser-combinators.sh — clean, exit 0
• Change verified by direct code read: (1) is a comment-only edit, no behavior change; (2) mirrors Pattern 1/2's already-compiled, already-tested .condition() call verbatim one function away, using the same prefix_condition: Option<ReplacementCondition> binding already in scope (Pattern 1/2 returns early when matched, so prefix_condition is never moved before reaching Pattern 3) — no new type, no new control-flow shape, no new code path exercised.

Validation NOT performed

cargo clippy and cargo check could not be confirmed post-change — 6 consecutive attempts (5 clippy, 1 check) were killed mid-compile with empty output. This matches a background-build-kill issue observed repeatedly across this fork's concurrent-agent worktrees this session (root cause since identified: shared ~/.cargo cache lock contention across ~10 concurrently active worktrees — I claimed and held the fork's advisory WORKLIST.md cargo-lock for every attempt, and confirmed via direct process inspection that no competing cargo/rustc process was running during at least one of the kills, so a residual harness-level cause is also in play). I'm disclosing this rather than claiming a clean compile that didn't happen. Given the change's scope (a comment plus a single conditional mirroring already-tested code in the same function, verified by direct read), the regression risk is low, but a maintainer or CI should get the authoritative compile/test signal this PR could not obtain locally.

Scope Expansion

None.

Validation Failures

cargo clippy -p engine --all-targets -- -D warnings and cargo check -p engine --lib — 6 consecutive background-build attempts killed mid-compile with empty output (see Validation NOT performed above). CI on this PR will provide the authoritative signal.

CI Failures

None.

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

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

Reviewed the code on head a5af7edd — this is correct and I'll approve once CI settles. Holding only for the Card data (generate, validate, coverage) gate, which is still in progress (I don't approve on a running check).

Both changes verified:

• CR citation fix (615 → 120.2a): confirmed against the Comprehensive Rules — CR 120.2a genuinely defines combat damage ("Each attacking and blocking creature deals combat damage equal to its power…"), whereas the old CR 615 is Prevention Effects, wrong for a "combat damage qualifier" comment. Good catch.
• Pattern 3 gate preservation: this fixes a real asymmetry, not a hypothetical. strip_as_long_as_condition_prefix already computes prefix_condition for all patterns (the shared prefix strip), and Patterns 1/2 consume it — Pattern 3 was silently discarding an already-parsed value, which is the classic parsed-AST-not-consumed no-op. Attaching it via .condition(cond) makes Pattern 3 consistent with its siblings. The CR 604.2 annotation is correct (static-ability continuous effect gate). Your comment honestly notes no current card exercises Pattern 3 with this prefix — that's fine; this is a building-block consistency fix, and a fail-open asymmetry is worth closing.

Once the coverage gate goes green I'll approve + enqueue. Since this only touches the damage-redirection replacement parser (a comment + a condition attach for a no-card branch), I don't expect a coverage delta.

[matthewevans]

Approving on settled green. The Card data (generate, validate, coverage) gate that was in progress last pass is now COMPLETED/SUCCESS, and the full rollup is green on the same head a5af7edd I reviewed. Both changes were verified: the CR citation fix (615 Prevention → 120.2a combat damage, grep-verified against docs/MagicCompRules.txt) and Pattern 3 now attaching the already-computed prefix_condition that Patterns 1/2 consume but Pattern 3 silently dropped (parsed-AST-not-consumed fix, CR 604.2). Clean scope — single file, no card-parse changes. Enqueuing.