Add Sigarda hexproof Human-scope regression tests (#5322)

[andriypolanski]

Closes #5322

Summary

Sigarda, Heron's Grace ("You and Humans you control have hexproof.") was reported as granting hexproof to itself and all creatures you control, despite Sigarda not being a Human. Investigation on current main shows the full path already works:

1. Parser (parse_compound_subject_keyword_static) decomposes the line into two defs: Continuous with Typed(Human) on creatures you control, plus StaticMode::Hexproof for the controller (player half) — not a single Or{empty-typed You, …} that layers treat as all permanents.
2. Layers apply creature hexproof only to objects matching the Human subtype filter.
3. Targeting enforces player hexproof separately from permanent hexproof.

No engine logic changes were required. This PR adds integration regression coverage so a future regression in compound-subject parsing or layer application is caught immediately.

Root cause of the original report (triage)

Layer	Finding
Parser AST	Object-half Typed { subtype: Human, controller: You }; player-half Hexproof with controller-You
Unit test	compound_subject_keyword_static_splits_sigarda_hexproof already green in oracle_static/tests.rs
Gap	No integration test drove parsed Sigarda through layer evaluation + targeting
Integration	Human has hexproof; Bear and Sigarda (Angel) do not; opponent cannot target P0 player

The issue is labeled status:fixed-unreleased / classifier:supported-aspect-defect on GitHub — the misparse class was fixed via parse_compound_subject_keyword_static; runtime lacked e2e lock-in.

Changes

Integration tests (crates/engine/tests/integration/issue_5322_sigarda_hexproof.rs)

• Parse assertion: two static defs with Human filter on object half.
• Runtime positive: Human creature gains hexproof after layer eval.
• Runtime negative: non-Human Bear and Angel Sigarda do not gain creature hexproof.
• Player scope: opponent cannot target hexproof controller.

Registration

• mod issue_5322_sigarda_hexproof; in crates/engine/tests/integration/main.rs

Verification

cargo fmt --all
cargo test -p engine sigarda
cargo test -p engine compound_subject_keyword_static_splits_sigarda

Test plan

• cargo test -p engine sigarda — 4 related tests green (3 integration + 1 parser unit)
• Manual: Sigarda on battlefield — Human ally has hexproof, non-Human ally does not
• Manual: opponent cannot target you (player hexproof) but can target their own permanents

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

Request changes — this is a premature regression test for an unfixed bug: the assertions fail on main because #5322 is not yet resolved.

🔴 Blocker

• Issue #5322 (Sigarda, Heron's Grace granting hexproof to all your creatures rather than only Humans) is still open — the bug is unfixed on main. This PR adds only tests asserting the correct Human-scoped behavior, so the Rust test shards go red (Rust tests (shard 1/2) and (shard 2/2) both FAILURE): the engine still exhibits the bug the tests forbid. A regression test has to land with, or after, its fix — never before it, or it can't be green.
• The Rust lint (fmt, clippy, parser gate) check is also red; run cargo fmt --all and clear clippy before re-pushing.

Recommendation: hold until the Sigarda hexproof-scope fix lands — a test asserting behavior the engine does not yet produce cannot go green. The higher-value contribution here is the fix itself: scope the hexproof grant to Humans (the engine's Human-filter path), with this test alongside it in the same PR. As a standalone test on an open bug, it can't merge.

[github-actions]

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

1 card(s) · trigger/ChangesZone · field condition: ∅ → sum combat damage dealt this turn (self -> player) [ungrouped] ≥ 1

Examples: Wave of Rats

2 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[matthewevans]

Re-review of the new head ea6a15414f (supersedes my earlier request-changes; the head advanced past the commit I first reviewed). This can't be approved: the build is red and the branch has picked up unrelated, stale content. Blocking with concrete fixes.

🔴 Blocker — clippy -D warnings abort cascades the whole build red

crates/engine/tests/integration/issue_5322_sigarda_hexproof.rs:188

let mut runner = scenario.build();

runner is only ever read (runner.state() borrows &self), so clippy fires variable does not need to be mutable and, under -D warnings, aborts compilation of the integration test binary. That single error is why Rust lint (parser gate), both test shards, and coverage-gate are all red — the coverage-gate 4s fail is just the downstream aggregator of the failed shard, not a coverage regression.

Fix: drop mut → let runner = scenario.build();.

🔴 Blocker — the branch is behind main and carries a divergent copy of oracle_static/evasion.rs

The diff shows crates/engine/src/parser/oracle_static/evasion.rs +83/-0 plus deltas in oracle.rs, oracle_static/mod.rs, and bin/oracle_gen.rs. None of that is Sigarda-hexproof work — evasion.rs is combat block-restriction parsing (parse_min_blockers_phrase, parse_source_power_block_restriction, classify_block_exception). It already exists on main (blob bcff7c1f10…), and this branch carries an older, divergent copy (blob a060b01b94…). The +83 added is a stale-base artifact: the branch predates that file landing on main, so GitHub diffs it against a merge-base that lacks it.

Fix: rebase onto current main. That drops evasion.rs / oracle.rs / mod.rs / oracle_gen.rs from the diff entirely and leaves only the Sigarda regression tests. A Sigarda-hexproof PR must not carry (a stale copy of) the combat-evasion parser.

🟡 Non-blocking — description contradicts the diff

The body states "No engine logic changes were required", but the current diff modifies four parser/engine source files. After the rebase this resolves itself; update the description to match whatever remains.

Recommendation

Request changes. Rebase onto main, remove the unneeded mut, and reduce the diff to the issue_5322_sigarda_hexproof.rs regression tests only (per your own writeup the parse path already works on main). Re-request review once the build is green on the rebased head — at that point this is pure regression coverage for a fixed-unreleased issue, which I'm glad to take if the assertions are discriminating (they look it: Human gains hexproof, non-Human Bear + Angel Sigarda do not, opponent can't target the player).

[matthewevans]

Verdict: request changes — one genuinely new runtime test buried in four that duplicate existing coverage; slim to the unique one. The rebase cleared the earlier scope contamination and the mut clippy slip, and CI is fully green (bug #5322 is already fixed on main — the runtime assertions pass). Thank you for that. But four of the five tests re-assert coverage that already exists.

🟡 Duplicative — please remove

• sigarda_hexproof_static_splits_human_object_and_player_halves (issue_5322_sigarda_hexproof.rs) is an exact duplicate of the existing compound_subject_keyword_static_splits_sigarda_hexproof at crates/engine/src/parser/oracle_static/tests.rs:193 — same parse_static_line_multi("You and Humans you control have hexproof."), same defs.len() == 2, same object-half Continuous + controller == You + get_subtype() == "Human" + "not the empty-typed You filter", same player-half StaticMode::Hexproof. That existing test is already annotated with #5322.
• sigarda_build_oracle_face_splits_hexproof_halves and sigarda_full_oracle_parse_matches_line_split assert the same 2-way split invariant one layer up (build_oracle_face / parse_oracle_text). The split is already guarded by the test above; these add no discriminating coverage.
• sigarda_player_hexproof_blocks_opponent_player_targeting duplicates find_legal_targets_excludes_player_hexproof_from_opponents at crates/engine/src/game/targeting.rs:4360 (CR 702.11c, opponent cannot target a hexproof player).

✅ Keep — this one is load-bearing

• sigarda_grants_hexproof_to_humans_not_all_creatures is the one test that closes a real gap: it asserts at layer-evaluation time that a non-Human (Grizzly Bear) and Sigarda herself (an Angel) do not receive the object-half creature hexproof, while a Human Soldier does. That is the actual #5322 defect surface (an over-broad object-half filter granting to every creature), and no existing runtime test exercises the Human-subtype-scoped object half — the existing runtime coverage only tests the player half. It is discriminating (would fail if the object-half were the empty-typed You filter) and currently green.

Recommendation

Slim this PR to just sigarda_grants_hexproof_to_humans_not_all_creatures (and its has_creature_hexproof helper + imports). A 207-line integration file that re-links the engine is not justified for one runtime test wrapped in four duplicates — but that one runtime test, standing alone, is worth landing. Once slimmed and green, I'll approve as bug.

[matthewevans]

Verdict: slimmed exactly as requested — approving on settled green. The PR is now the single load-bearing runtime test; the four duplicative parse/targeting tests are gone.

✅ Clean

• issue_5322_sigarda_hexproof.rs:32 sigarda_grants_hexproof_to_humans_not_all_creatures — the one test that covers the actual #5322 defect surface: runtime layer evaluation of the Human-subtype-scoped object half. It's discriminating — a Human gets Sigarda's creature hexproof, a Grizzly Bear does not, and Sigarda herself (an Angel, not a Human) does not. That last assertion is the exact over-broad-filter regression #5322 was about, and no existing runtime test exercises it (the parse-split and player-targeting layers are already covered by oracle_static/tests.rs:193 and targeting.rs:4360).
• tests/integration/main.rs:445 — mod issue_5322_sigarda_hexproof; is registered, so the test is live (not an inert false-green).
• Scope contamination and the mut clippy slip from the prior head are both cleared: clean 2-file diff, runner correctly &mut.

Recommendation: approve + merge as bug once both Rust shards + lint settle green (currently pending on head a196cba7).

[matthewevans]

Approve — the slim landed exactly as requested; this is now a single load-bearing regression test for the #5322 fix.

✅ Clean

• The PR is down to a clean +52/−0 diff carrying only sigarda_grants_hexproof_to_humans_not_all_creatures, the runtime layer-evaluation test that a Human gets Sigarda's granted hexproof while a Grizzly Bear and Sigarda-herself (both non-Human) do not. That is the exact #5322 defect surface (over-broad object-half filter), and it's discriminating — it fails if the object filter were the empty-typed You filter.
• The four duplicative tests from the prior head (parse-split trio already covered by oracle_static/tests.rs:193, player-targeting already covered by targeting.rs:4360) are gone.
• mod registered in tests/integration/main.rs — not an inert top-level binary. The prior mut clippy slip and stale-base scope contamination are both resolved; CI is fully green (both Rust shards, lint, coverage-gate).

Recommendation: approve and merge as bug. #5322 is fixed on main; this guards the fix at the one layer no existing test covered.