Fix Jailbreak — bind the CR 110.2a "under their control" clause (#6691) - #6814
Fix Jailbreak — bind the CR 110.2a "under their control" clause (#6691)#6814keloide wants to merge 4 commits into
Conversation
…s#6691) The Oracle parser recognised only "under your control" and the owner-stating forms. "under their control" / "under that player's control" fell through the empty-tag arm and were silently dropped, so permanents entered under the wrong controller while the cards reported supported: true, gap_count: 0 — a silent wrong-game-result class. Collapse four independently-implemented copies of the `under <possessor> control` grammar into one nom combinator module (oracle_nom/enters_under.rs) and bind the anaphor from typed parse products only: N1 — the moved object's own FilterProp::Owned { controller != You } (CR 400.1 + 400.3 + 404.1 + 108.3: a card in a graveyard is in its owner's graveyard) -> ControllerRef::ParentTargetOwner N2 — ctx.relative_player_scope, mapped through an exhaustive wildcard-free table that admits only ScopedPlayer, ChosenPlayer and TriggeringPlayer Everything else fails closed to Effect::unimplemented rather than silently defaulting, so a clause the parser cannot bind becomes an honest coverage gap instead of a wrong controller. EntersUnderSpec carries that third state through the AST to every lowering site. Fixes Jailbreak (ParentTargetOwner) and Gerrymandering (ScopedPlayer) and their templates; 933 "your"/owner-form cards keep byte-identical behaviour through the single grammar. Addresses phase-rs#6691; does not close it — see the PR body for the residue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nr9Cg6eBZfBod5Twb6Vyi
…h + scope correction Review found the integration test passed with the fix reverted. Two causes, both fixed here: 1. The surgically-added `jailbreak` fixture entry was copied from a PRE-FIX `card-data.json`, so it carried no `enters_under` key. Since the field is `skip_serializing_if = "Option::is_none"`, it deserialized as the unfixed AST and the test measured old behaviour while appearing to pass. The entry is regenerated from post-fix card data (+0 keys, only `jailbreak` changed) and a fixture-integrity guard now fails loudly if it ever goes stale again. 2. The test claimed to be revert-failing. It is not, and that was MEASURED, not assumed: with `enters_under` stripped it still passes, because a graveyard → battlefield move already resolves the new object's controller to its OWNER, which for Jailbreak is exactly the player the clause names. Staging a divergent controller does not separate them either — the move reassigns it. The docstring now states this plainly and points at the discriminating test. The discriminating test is the parser-level assertion `jailbreak_enters_under_their_control_binds_to_the_owner`, verified by temporarily binding `Default` instead of `ParentTargetOwner`: it FAILS on revert and passes when restored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nr9Cg6eBZfBod5Twb6Vyi
github-actions
Bot
added
the
needs-maintainer
📝 WalkthroughWalkthroughThe parser adds control-clause parsing and binding with explicit, default, and unbound states. Zone-change effects propagate these states, lowering unbound anaphors to unimplemented effects. Prevent-damage recipient resolution and Jailbreak regression coverage are also added. ChangesEnters-under control binding
Prevent-damage recipient resolution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CardText
participant ReturnDestinationParser
participant ControlClauseBinder
participant ZoneChangeLowering
participant GameEffect
CardText->>ReturnDestinationParser: parse battlefield destination and control clause
ReturnDestinationParser->>ControlClauseBinder: pass possessor and antecedent context
ControlClauseBinder->>ZoneChangeLowering: return EntersUnderSpec
ZoneChangeLowering->>GameEffect: create controller override or unimplemented anaphor effect
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)crates/engine/tests/fixtures/integration_cards.jsonCheckov skipped this file: it is too large to scan (10844856 bytes) 🔧 ast-grep (0.45.0)crates/engine/tests/fixtures/integration_cards.jsonast-grep skipped this file: it is too large to scan (10844856 bytes) crates/engine/src/parser/oracle_effect/mod.rsast-grep timed out on this file Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
crates/engine/src/parser/oracle_nom/enters_under.rs (1)
325-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe final
(p, _)arm defeats the exhaustiveness discipline this module relies on elsewhere.
map_relative_player_scopeis deliberately wildcard-free so "adding aControllerRefvariant breaks the build here" (Line 246).bind_control_clausegives up that property: the deferred possessor forms listed at Lines 150-160 ("under an opponent's control","under target player's control", …) will, the moment they are added asControlClausePossessorvariants, fall silently intoUnboundAnaphorrather than forcing a decision at this binding table. Fail-closed is the safe direction, but the card becomes an unexplained coverage gap instead of a compile error.Match the possessor axis explicitly so the compiler flags a new variant.
As per coding guidelines: "Use exhaustive
matchexpressions without wildcard fallbacks when matching known enums, allowing the compiler to detect missing variants."♻️ Explicit anaphor arms instead of the tuple wildcard
- // Fail closed: no nameable antecedent (and, for the demonstrative, an - // object-owner antecedent it may not legally use). - (p, _) => EntersUnderSpec::UnboundAnaphor(p), + // Fail closed: no nameable antecedent (and, for the demonstrative, an + // object-owner antecedent it may not legally use). Enumerated so a new + // possessor variant breaks the build here rather than becoming a silent + // coverage gap. + ( + p @ (ControlClausePossessor::TheirAnaphor + | ControlClausePossessor::ThatPlayerDemonstrative), + ControlAnaphorAntecedent::MovedObjectOwner | ControlAnaphorAntecedent::Unnameable, + ) => EntersUnderSpec::UnboundAnaphor(p),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_nom/enters_under.rs` around lines 325 - 354, Update bind_control_clause to remove the tuple wildcard `(p, _)` and enumerate each currently unsupported ControlClausePossessor variant explicitly, returning UnboundAnaphor for those cases. Preserve the existing You, Owner, TheirAnaphor, and ThatPlayerDemonstrative behavior while making the match exhaustive so adding a new possessor variant produces a compile-time error.Source: Coding guidelines
crates/engine/src/parser/oracle_effect/sequence.rs (1)
5308-5326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThe searched player's filter is available here and would license the CR 108.3 owner antecedent.
Passing
Noneasmoved_objectis accurate for this AST shape, but the information is not actually absent: the precedingEffect::SearchLibraryindefscarries the player filter whose library is searched, and for "search target opponent's library … put it onto the battlefield under their control" that player is the found card's owner (CR 400.3 — a card in a library is in its owner's library). As written, such a card can only reachContextPlayeror fail closed. Not a defect in this PR's scope, but worth a note here (or in the deferred list) so the seam's gap is attributable rather than looking like an inherent limitation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/sequence.rs` around lines 5308 - 5326, Document this deferred limitation at the bind_control_clause seam: although moved_object is correctly None for this AST shape, the preceding Effect::SearchLibrary in defs provides the searched player filter, which can identify the found card’s owner for CR 108.3. Add a concise note here or to the deferred-work list explaining that opponent-library searches currently cannot use this owner antecedent and may resolve only to ContextPlayer or fail closed.crates/engine/src/parser/oracle_ir/ast.rs (1)
220-271: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThree-state contract is only half-adopted across
enters_underfields.
SearchDestination,ReturnToBattlefield,ReturnAllToZone,ZoneChange, andZoneChangeAllnow carryEntersUnderSpec, but the sibling controller-override fields in this same file stayOption<ControllerRef>:
ContinuationAst::DigFromAmong.enters_under(Line 406)ContinuationAst::RevealUntilKept.enters_under(Line 469)PutImperativeAst::Manifest.enters_under(Line 1511)Those three are fed by literal
scan_contains(lower, "under your control")checks inoracle_effect/sequence.rs, so a printed"under their control"/"under that player's control"at those seams still degrades to the CR 110.2 default rather than failing closed — exactly the class this PR is closing. Today no printed card appears to reach them with a third-person clause, so this is a coverage/uniformity gap rather than an active misparse; worth either migrating them ontoEntersUnderSpecin a follow-up or recording the carve-out in this doc comment so the asymmetry is intentional and discoverable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_ir/ast.rs` around lines 220 - 271, Complete the three-state contract for the remaining entry-controller fields: change ContinuationAst::DigFromAmong.enters_under, ContinuationAst::RevealUntilKept.enters_under, and PutImperativeAst::Manifest.enters_under from Option<ControllerRef> to EntersUnderSpec, and update their sequence-lowering/parsing paths to preserve unbound third-person control clauses as UnboundAnaphor instead of defaulting. Reuse EntersUnderSpec’s existing helpers and ensure all constructors and consumers handle Default, Override, and UnboundAnaphor consistently.crates/engine/src/parser/oracle_effect/tests.rs (1)
29864-29992: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a destination-layer unit test for the third-person control-clause fallback.
jailbreak_enters_under_their_control_binds_to_the_ownerandunbindable_anaphor_fails_closed_instead_of_defaultingexercise the newTheirAnaphor/ThatPlayerDemonstrativefallback only through fullparse_effect_chain/parse_oracle_text, conflating the destination-parsing layer (strip_return_destination_ext_with_remainderinlower.rs) with the downstream binding layer. A direct test mirroringreturn_destination_owners_control_not_under_your_control— e.g. assertingstrip_return_destination_ext("it to the battlefield under their control").1.unwrap().control == Some(ControlClausePossessor::TheirAnaphor)— would isolate a regression in the newparse_leading_control_clausefallback from a regression inbind_control_clause.🧪 Suggested additional test
#[test] fn return_destination_under_their_control_recognized_as_anaphor() { let (_, dest) = strip_return_destination_ext("it to the battlefield under their control"); let d = dest.expect("should parse destination"); assert_eq!(d.control, Some(ControlClausePossessor::TheirAnaphor)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/tests.rs` around lines 29864 - 29992, Add a focused destination-layer unit test alongside return-destination tests, using strip_return_destination_ext (or its remainder variant) to parse “it to the battlefield under their control” and assert the destination control is Some(ControlClausePossessor::TheirAnaphor). Keep this test independent of parse_effect_chain and bind_control_clause so it specifically covers parse_leading_control_clause’s fallback recognition.crates/engine/src/parser/oracle_effect/imperative.rs (1)
2331-2336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the fail-closed guard into one authority on
EntersUnderSpec. The identicalunbound_possessor()→Effect::unimplemented("change_zone_enters_under_anaphor", p.printed_clause())block, including the gap-name string literal, is repeated at five lowering sites. A single accessor (e.g.EntersUnderSpec::unimplemented_if_unbound(&self) -> Option<Effect>) keeps the gap name and CR 110.2a rationale in one place and makes a future lowering site that forgets the guard obvious.
crates/engine/src/parser/oracle_effect/imperative.rs#L2331-L2336: replace with the shared accessor (if let Some(e) = enters_under.unimplemented_if_unbound() { return e; }).crates/engine/src/parser/oracle_effect/imperative.rs#L2396-L2401: same replacement in theReturnAllToZonearm.crates/engine/src/parser/oracle_effect/imperative.rs#L6762-L6767: same replacement in thePutImperativeAst::ZoneChangeAllarm.crates/engine/src/parser/oracle_effect/imperative.rs#L6796-L6801: same replacement in thePutImperativeAst::ZoneChangearm.crates/engine/src/parser/oracle_effect/imperative.rs#L11635-L11640: same, wrapping the returned effect inparsed_clause(..).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/imperative.rs` around lines 2331 - 2336, Centralize the unbound-possessor fail-closed behavior in an accessor on EntersUnderSpec, such as unimplemented_if_unbound, preserving the change_zone_enters_under_anaphor gap name and printed clause. Replace the duplicated guards at crates/engine/src/parser/oracle_effect/imperative.rs lines 2331-2336, 2396-2401, 6762-6767, and 6796-6801 with the accessor and return its Effect; at lines 11635-11640, use the accessor and wrap the returned effect in parsed_clause as currently required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/engine/src/parser/oracle_effect/sequence.rs`:
- Around line 3727-3741: Handle the unbound enters_under.possessor() case as a
failed assembly: at
crates/engine/src/parser/oracle_effect/sequence.rs:3727-3741, stop or neutralize
the existing library-to-battlefield ChangeZone in previous before
apply_search_destination_to_ability_chain so no executable wrong-controller move
remains; at crates/engine/src/parser/oracle_effect/sequence.rs:3770-3781, skip
forward_result and Effect::Attach wiring when put_effect is the
Effect::unimplemented fragment, or fold the attach behavior into that fragment.
In `@crates/engine/tests/integration/issue_6691_enters_under_their_control.rs`:
- Around line 59-143: Add a separate integration test using a production card
ability with a named-player controller binding, such as The Beamtown Bullies’
“that player’s control,” where the default controller differs from the
referenced player. Build and resolve the scenario through the normal card
database and game runner pipeline, then assert the permanent enters under the
named player’s control and that the relevant effect does not revert.
---
Nitpick comments:
In `@crates/engine/src/parser/oracle_effect/imperative.rs`:
- Around line 2331-2336: Centralize the unbound-possessor fail-closed behavior
in an accessor on EntersUnderSpec, such as unimplemented_if_unbound, preserving
the change_zone_enters_under_anaphor gap name and printed clause. Replace the
duplicated guards at crates/engine/src/parser/oracle_effect/imperative.rs lines
2331-2336, 2396-2401, 6762-6767, and 6796-6801 with the accessor and return its
Effect; at lines 11635-11640, use the accessor and wrap the returned effect in
parsed_clause as currently required.
In `@crates/engine/src/parser/oracle_effect/sequence.rs`:
- Around line 5308-5326: Document this deferred limitation at the
bind_control_clause seam: although moved_object is correctly None for this AST
shape, the preceding Effect::SearchLibrary in defs provides the searched player
filter, which can identify the found card’s owner for CR 108.3. Add a concise
note here or to the deferred-work list explaining that opponent-library searches
currently cannot use this owner antecedent and may resolve only to ContextPlayer
or fail closed.
In `@crates/engine/src/parser/oracle_effect/tests.rs`:
- Around line 29864-29992: Add a focused destination-layer unit test alongside
return-destination tests, using strip_return_destination_ext (or its remainder
variant) to parse “it to the battlefield under their control” and assert the
destination control is Some(ControlClausePossessor::TheirAnaphor). Keep this
test independent of parse_effect_chain and bind_control_clause so it
specifically covers parse_leading_control_clause’s fallback recognition.
In `@crates/engine/src/parser/oracle_ir/ast.rs`:
- Around line 220-271: Complete the three-state contract for the remaining
entry-controller fields: change ContinuationAst::DigFromAmong.enters_under,
ContinuationAst::RevealUntilKept.enters_under, and
PutImperativeAst::Manifest.enters_under from Option<ControllerRef> to
EntersUnderSpec, and update their sequence-lowering/parsing paths to preserve
unbound third-person control clauses as UnboundAnaphor instead of defaulting.
Reuse EntersUnderSpec’s existing helpers and ensure all constructors and
consumers handle Default, Override, and UnboundAnaphor consistently.
In `@crates/engine/src/parser/oracle_nom/enters_under.rs`:
- Around line 325-354: Update bind_control_clause to remove the tuple wildcard
`(p, _)` and enumerate each currently unsupported ControlClausePossessor variant
explicitly, returning UnboundAnaphor for those cases. Preserve the existing You,
Owner, TheirAnaphor, and ThatPlayerDemonstrative behavior while making the match
exhaustive so adding a new possessor variant produces a compile-time error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 416d9312-f59c-4081-bf6b-12ba184477f1
⛔ Files ignored due to path filters (2)
crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snapis excluded by!**/*.snap,!**/snapshots/**crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snapis excluded by!**/*.snap,!**/snapshots/**
📒 Files selected for processing (11)
crates/engine/src/parser/oracle_effect/imperative.rscrates/engine/src/parser/oracle_effect/lower.rscrates/engine/src/parser/oracle_effect/mod.rscrates/engine/src/parser/oracle_effect/sequence.rscrates/engine/src/parser/oracle_effect/tests.rscrates/engine/src/parser/oracle_ir/ast.rscrates/engine/src/parser/oracle_nom/enters_under.rscrates/engine/src/parser/oracle_nom/mod.rscrates/engine/tests/fixtures/integration_cards.jsoncrates/engine/tests/integration/issue_6691_enters_under_their_control.rscrates/engine/tests/integration/main.rs
| // CR 110.2a (docs/MagicCompRules.txt:618): ALWAYS call the | ||
| // chain patcher, with the COLLAPSED reference. The patcher | ||
| // rewrites the existing destination and tapped flag | ||
| // UNCONDITIONALLY and the controller only behind its own | ||
| // `is_some()` guard, so skipping the whole call on an unbound | ||
| // anaphor would silently drop the DESTINATION and TAPPED | ||
| // patches as well. Passing `None` reuses that existing guard to | ||
| // no-op exactly the controller patch. | ||
| apply_search_destination_to_ability_chain( | ||
| previous, | ||
| destination, | ||
| enter_tapped, | ||
| enters_under.clone(), | ||
| enters_under.as_controller_ref(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The SearchDestination fail-closed path only replaces the new effect, not the surrounding assembly. enters_under.unbound_possessor() swaps the constructed ChangeZone for an Effect::unimplemented, but the rest of the arm still runs as though a real move were emitted — so an unbound "under their control" can produce a gap node plus an executable wrong-controller move and an attach rider bound to a result that never exists.
crates/engine/src/parser/oracle_effect/sequence.rs#L3727-L3741: on the unbound path, stop patching the pre-existing library→battlefieldChangeZoneinprevious(or neutralize it), so no wrong-controller move survives alongside the gap.crates/engine/src/parser/oracle_effect/sequence.rs#L3770-L3781: skip theforward_result/Effect::Attachwiring whenput_effectis the unimplemented fragment, or fold the attach clause into that fragment.
📍 Affects 1 file
crates/engine/src/parser/oracle_effect/sequence.rs#L3727-L3741(this comment)crates/engine/src/parser/oracle_effect/sequence.rs#L3770-L3781
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/engine/src/parser/oracle_effect/sequence.rs` around lines 3727 - 3741,
Handle the unbound enters_under.possessor() case as a failed assembly: at
crates/engine/src/parser/oracle_effect/sequence.rs:3727-3741, stop or neutralize
the existing library-to-battlefield ChangeZone in previous before
apply_search_destination_to_ability_chain so no executable wrong-controller move
remains; at crates/engine/src/parser/oracle_effect/sequence.rs:3770-3781, skip
forward_result and Effect::Attach wiring when put_effect is the
Effect::unimplemented fragment, or fold the attach behavior into that fragment.
| #[test] | ||
| fn jailbreak_returns_the_permanent_under_its_owners_control() { | ||
| let Some(db) = load_db() else { | ||
| eprintln!("skipping: integration card fixture not available"); | ||
| return; | ||
| }; | ||
|
|
||
| let mut scenario = GameScenario::new_n_player(3, 7); | ||
| scenario.at_phase(Phase::PreCombatMain); | ||
| let p2 = PlayerId(2); | ||
|
|
||
| let jailbreak = scenario.add_real_card(P0, "Jailbreak", Zone::Hand, db); | ||
| // The victim: owned by P1, sitting in P1's graveyard. | ||
| let victim = scenario.add_real_card(P1, "Grizzly Bears", Zone::Graveyard, db); | ||
| // A decoy permanent card in a DIFFERENT opponent's graveyard. If the | ||
| // binding were "an opponent" (a class, CR 102.2 @ :252) rather than the | ||
| // moved card's owner, the seat this resolves to would be ambiguous. | ||
| let _decoy = scenario.add_real_card(p2, "Grizzly Bears", Zone::Graveyard, db); | ||
| // Jailbreak costs {1}{W}; seed the pool so the cast is about the effect, | ||
| // not about mana. | ||
| scenario.with_mana_pool( | ||
| P0, | ||
| vec![ | ||
| ManaUnit::new(ManaType::White, ObjectId(9_999), false, vec![]), | ||
| ManaUnit::new(ManaType::Colorless, ObjectId(9_999), false, vec![]), | ||
| ], | ||
| ); | ||
|
|
||
| let mut runner = scenario.build(); | ||
| rehydrate_game_from_card_db(runner.state_mut(), db); | ||
|
|
||
| // FIXTURE-INTEGRITY GUARD. `Effect::ChangeZone.enters_under` is | ||
| // `skip_serializing_if = "Option::is_none"`, so a fixture entry captured | ||
| // from a PRE-FIX `card-data.json` deserializes as `enters_under: None` — | ||
| // i.e. the unfixed AST — and every assertion below would then be measuring | ||
| // the old behaviour while appearing to pass. Fail loudly instead. | ||
| { | ||
| let face = db | ||
| .get_face_by_name("Jailbreak") | ||
| .expect("Jailbreak present in the integration fixture"); | ||
| let dump = format!("{face:?}"); | ||
| assert!( | ||
| dump.contains("ParentTargetOwner"), | ||
| "stale fixture: Jailbreak's parsed ability must carry \ | ||
| enters_under: Some(ParentTargetOwner); regenerate \ | ||
| crates/engine/tests/fixtures/integration_cards.json" | ||
| ); | ||
| } | ||
|
|
||
| let outcome = runner | ||
| .cast(jailbreak) | ||
| .target_object(victim) | ||
| // Jailbreak's delayed "up to one target" trigger is optional; P0's | ||
| // graveyard is empty so it has no legal target either way. | ||
| .decline_optional() | ||
| .resolve(); | ||
| let state = outcome.state(); | ||
|
|
||
| // Reach guard (foot-gun #6): the move must actually have HAPPENED. Without | ||
| // this, a fizzle or an illegal-target failure would make the controller | ||
| // assertion below pass vacuously. | ||
| assert!( | ||
| state.objects.contains_key(&victim), | ||
| "the returned card must still be a live object" | ||
| ); | ||
| assert_eq!( | ||
| zone_of(state, victim), | ||
| Some(Zone::Battlefield), | ||
| "Jailbreak must actually return the card to the battlefield" | ||
| ); | ||
| // Reach guard: ownership is what LICENSES the binding, so pin it. | ||
| assert_eq!( | ||
| state.objects[&victim].owner, P1, | ||
| "fixture invariant: the victim must be OWNED by P1" | ||
| ); | ||
|
|
||
| // CR 110.2a resolution guard (see the module docstring for why this is a | ||
| // reach guard and not a revert-failing discriminator). | ||
| assert_eq!( | ||
| state.objects[&victim].controller, P1, | ||
| "CR 110.2a: the bound ParentTargetOwner must RESOLVE (not raise \ | ||
| InvalidParam) and yield the card's owner P1 — not the caster P0, \ | ||
| not the third seat P2" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a revert-failing runtime scenario.
This test explicitly passes with the pre-fix AST, so it cannot catch a regression in controller binding. Add a production-pipeline scenario where the default controller differs from the named player—such as The Beamtown Bullies’ “that player’s control”—and assert the resulting controller.
As per path instructions, regression tests must exercise the failure path the fix prevents through the production pipeline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/engine/tests/integration/issue_6691_enters_under_their_control.rs`
around lines 59 - 143, Add a separate integration test using a production card
ability with a named-player controller binding, such as The Beamtown Bullies’
“that player’s control,” where the default controller differs from the
referenced player. Build and resolve the scenario through the normal card
database and game runner pipeline, then assert the permanent enters under the
named player’s control and that the relevant effect does not revert.
Source: Path instructions
Resolve maintainer-caused merge conflicts by retaining both main's Notion Thief fixture/registration and phase-rs#6691's Jailbreak fixture/registration. Co-authored-by: Lcola98 <75585494+keloide@users.noreply.github.com>
|
Maintainer conflict port for |
Parse changes introduced by this PR · 6 card(s), 7 signature(s) (baseline: main
|
|
Maintainer CI repair is pushed on current head |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/engine/src/parser/oracle_effect/lower.rs (2)
6754-6768: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConsume battlefield riders after the control clause.
After
parse_leading_control_clausesucceeds, this path immediately returns after consuming one space. Valid forms such asto the battlefield under their control face down and tappedtherefore leave the riders in the remainder and returnface_down: false/enter_tapped: false. The other destination path already scans riders after control; reuse that logic here.
As per path instructions, engine destination parsing must remain composable and rules-correct across rider orderings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/lower.rs` around lines 6754 - 6768, Update the battlefield return path following parse_leading_control_clause to consume and apply trailing battlefield riders such as face down and tapped, including valid rider orderings, before constructing ReturnDestination. Reuse the existing rider-scanning logic from the other destination path, preserving the remaining input and setting face_down and enter_tapped correctly.Source: Path instructions
6639-6662: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve control clauses after enter-with-counters suffixes.
Control parsing only runs before
parse_with_counters_suffix_spanned. For valid text such aswith two stun counters under their control, the counter parser consumes the prefix, while the trailing control clause is neither bound nor returned; the destination silently falls back tocontrol: None, breaking CR 110.2a behavior. Parse the counter-suffix remainder for a control clause, and add an assertion fordest.controlto the existing regression fixture.
As per path instructions, engine parsing must preserve rules-correct control binding across valid clause orderings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/lower.rs` around lines 6639 - 6662, Update the destination parsing flow around parse_with_counters_suffix_spanned to inspect its remaining text for a trailing control clause before finalizing the destination. Use parse_leading_control_clause to bind the parsed player to dest.control while preserving existing rider and suffix handling for orderings such as counters followed by control. Extend the existing regression fixture with an assertion that dest.control is populated.Source: Path instructions
crates/engine/src/parser/oracle_effect/imperative.rs (1)
2052-2057: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCR citation mismatch: "CR 110.1" doesn't describe controller ownership.
CR 110.1 defines what a permanent is ("A permanent is a card or token on the battlefield") and says nothing about controllers. The claim "only permanents have a controller" maps to CR 110.2 ("Every permanent has a controller") together with CR 108.4 ("A card doesn't have a controller unless that card represents a permanent or spell"), not CR 110.1.
📝 Suggested annotation fix
- // CR 110.1 (docs/MagicCompRules.txt:614): only - // permanents have a controller. + // CR 110.2 + CR 108.4 (docs/MagicCompRules.txt): only + // permanents have a controller. enters_under: EntersUnderSpec::Default,Same mis-citation recurs at Line 2080-2082 (
d.zonevariant) — apply the same fix there.As per path instructions: "rules-touching code with no verified
CR <number>: <description>annotation, or a CR citation whose rule body does not describe the code" is a finding, and "119 starting-life / 120 damage / 121 draw are adjacent and confused" flags exactly this class of numeric-citation risk.Also applies to: 2080-2085
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/imperative.rs` around lines 2052 - 2057, Correct the rules citations in both `EntersUnderSpec::Default` initializations around the `d.zone` and non-`d.zone` variants: replace the incorrect CR 110.1 reference with citations to CR 110.2 and CR 108.4, which establish controller ownership for permanents and the applicable card types. Keep the existing behavior and annotation wording otherwise unchanged.Source: Path instructions
🧹 Nitpick comments (1)
crates/engine/src/parser/oracle_effect/imperative.rs (1)
2325-2357: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail-closed
enters_under.unbound_possessor()guard duplicated 5×.The pattern
if let Some(p) = enters_under.unbound_possessor() { return Effect::unimplemented("change_zone_enters_under_anaphor", p.printed_clause()); }followed byenters_under.as_controller_ref()repeats verbatim at Line 2331-2343 (ReturnToBattlefield), Line 2396-2414 (ReturnAllToZone), Line 6802-6819/6843-6849 (lower_put_ast's two arms), and Line 11682-11688 (lower_imperative_family_ast's partition arm). All five call sites are correctly guarded (verified — none is missing the check), but the boilerplate could collapse into one helper, e.g.EntersUnderSpec::resolve_or_unimplemented() -> Result<Option<ControllerRef>, Effect>, callable with?/matchat each site.♻️ Sketch of the consolidation (illustrative, not exhaustive)
- if let Some(p) = enters_under.unbound_possessor() { - return Effect::unimplemented( - "change_zone_enters_under_anaphor", - p.printed_clause(), - ); - } - Effect::ChangeZone { - ... - enters_under: enters_under.as_controller_ref(), + let enters_under = match enters_under.resolve_or_unimplemented("change_zone_enters_under_anaphor") { + Ok(v) => v, + Err(effect) => return effect, + }; + Effect::ChangeZone { + ... + enters_under,Also applies to: 2393-2401, 6787-6849, 11668-11688
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/src/parser/oracle_effect/imperative.rs` around lines 2325 - 2357, Consolidate the repeated fail-closed handling of enters_under.unbound_possessor() into a single EntersUnderSpec helper that returns either the resolved controller reference or the existing Effect::unimplemented result. Update the ReturnToBattlefield, ReturnAllToZone, both lower_put_ast arms, and the lower_imperative_family_ast partition arm to use this helper while preserving the current failure key and printed clause.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/engine/src/parser/oracle_effect/mod.rs`:
- Around line 15812-15817: Update both comments associated with
EntersUnderSpec::Default to cite CR 110.2 instead of CR 110.1, while preserving
the existing wording and behavior.
---
Outside diff comments:
In `@crates/engine/src/parser/oracle_effect/imperative.rs`:
- Around line 2052-2057: Correct the rules citations in both
`EntersUnderSpec::Default` initializations around the `d.zone` and non-`d.zone`
variants: replace the incorrect CR 110.1 reference with citations to CR 110.2
and CR 108.4, which establish controller ownership for permanents and the
applicable card types. Keep the existing behavior and annotation wording
otherwise unchanged.
In `@crates/engine/src/parser/oracle_effect/lower.rs`:
- Around line 6754-6768: Update the battlefield return path following
parse_leading_control_clause to consume and apply trailing battlefield riders
such as face down and tapped, including valid rider orderings, before
constructing ReturnDestination. Reuse the existing rider-scanning logic from the
other destination path, preserving the remaining input and setting face_down and
enter_tapped correctly.
- Around line 6639-6662: Update the destination parsing flow around
parse_with_counters_suffix_spanned to inspect its remaining text for a trailing
control clause before finalizing the destination. Use
parse_leading_control_clause to bind the parsed player to dest.control while
preserving existing rider and suffix handling for orderings such as counters
followed by control. Extend the existing regression fixture with an assertion
that dest.control is populated.
---
Nitpick comments:
In `@crates/engine/src/parser/oracle_effect/imperative.rs`:
- Around line 2325-2357: Consolidate the repeated fail-closed handling of
enters_under.unbound_possessor() into a single EntersUnderSpec helper that
returns either the resolved controller reference or the existing
Effect::unimplemented result. Update the ReturnToBattlefield, ReturnAllToZone,
both lower_put_ast arms, and the lower_imperative_family_ast partition arm to
use this helper while preserving the current failure key and printed clause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ae892d00-44e6-4cc1-a1ca-88756500c54c
⛔ Files ignored due to path filters (5)
crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__aangs_journey_ir.snapis excluded by!**/*.snap,!**/snapshots/**crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__analyze_the_pollen_ir.snapis excluded by!**/*.snap,!**/snapshots/**crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snapis excluded by!**/*.snap,!**/snapshots/**crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snapis excluded by!**/*.snap,!**/snapshots/**crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__stoneforge_mystic_ir.snapis excluded by!**/*.snap,!**/snapshots/**
📒 Files selected for processing (6)
crates/engine/src/parser/oracle_effect/imperative.rscrates/engine/src/parser/oracle_effect/lower.rscrates/engine/src/parser/oracle_effect/mod.rscrates/engine/src/parser/oracle_effect/tests.rscrates/engine/tests/fixtures/integration_cards.jsoncrates/engine/tests/integration/main.rs
| target, | ||
| origin, | ||
| destination: Zone::Hand, | ||
| enters_under: None, | ||
| // CR 110.1 (docs/MagicCompRules.txt:614): only | ||
| // permanents have a controller. | ||
| enters_under: EntersUnderSpec::Default, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '605,625p' docs/MagicCompRules.txtRepository: phase-rs/phase
Length of output: 219
🏁 Script executed:
fd -a 'MagicCompRules.txt' .Repository: phase-rs/phase
Length of output: 152
🏁 Script executed:
#!/bin/bash
fd -a 'MagicCompRules.txt' .Repository: phase-rs/phase
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
find . -name 'MagicCompRules.txt' -o -name 'magiccomprules.txt'Repository: phase-rs/phase
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- source around first site ---'
sed -n '15800,15825p' crates/engine/src/parser/oracle_effect/mod.rs
echo
echo '--- source around second site ---'
sed -n '15840,15860p' crates/engine/src/parser/oracle_effect/mod.rs
echo
echo '--- search for citation text ---'
rg -n "only permanents have a controller|CR 110\.1|controller" crates docs . -g '!target'Repository: phase-rs/phase
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '15800,15825p' crates/engine/src/parser/oracle_effect/mod.rs
printf '\n---\n'
sed -n '15840,15860p' crates/engine/src/parser/oracle_effect/mod.rs
printf '\n---\n'
rg -n "only permanents have a controller|CR 110\.1|controller" crates . -g '!target' -g '!target/**'Repository: phase-rs/phase
Length of output: 50371
🌐 Web query:
Magic Comprehensive Rules 110.1 controller permanents only have a controller
💡 Result:
According to the Magic: The Gathering Comprehensive Rules, Rule 110.1 defines what a permanent is, while the specific rule stating that every permanent has a controller is Rule 110.2 [1][2]. Rule 110.1 states: "A permanent is a card or token on the battlefield. A permanent remains on the battlefield indefinitely. A card or token becomes a permanent as it enters the battlefield and it stops being a permanent as it’s moved to another zone by an effect or rule." [1][3] Rule 110.2 explicitly addresses controllers, stating: "A permanent’s owner is the same as the owner of the card that represents it (unless it’s a token; see rule 111.2). A permanent’s controller is, by default, the player under whose control it entered the battlefield. Every permanent has a controller." [1][4]
Citations:
- 1: https://media.wizards.com/2026/downloads/MagicCompRules%2020260417.txt
- 2: https://mtg-rules.vercel.app/chapters/110
- 3: https://magic-rulebook.vercel.app/rules/110
- 4: https://media.wizards.com/images/magic/tcg/resources/rules/MagicCompRules_21031101.pdf
Use CR 110.2 here
CR 110.1 covers permanents; Every permanent has a controller is in CR 110.2. Update both EntersUnderSpec::Default notes to cite the correct rule.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/engine/src/parser/oracle_effect/mod.rs` around lines 15812 - 15817,
Update both comments associated with EntersUnderSpec::Default to cite CR 110.2
instead of CR 110.1, while preserving the existing wording and behavior.
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Blocked — the new fail-closed representation is bypassed on the SearchDestination assembly path, and the current test suite does not exercise that path.
🔴 Blocker
apply_clause_continuation still patches a pre-existing library-to-battlefield ChangeZone with enters_under.as_controller_ref() before it creates the new Effect::unimplemented node for an unbound anaphor. Evidence: crates/engine/src/parser/oracle_effect/sequence.rs:3727-3740 calls apply_search_destination_to_ability_chain with None; its implementation at crates/engine/src/parser/oracle_effect/sequence.rs:5048-5073 leaves the existing controller untouched while still rewriting destination/tapped. The later unbound branch at crates/engine/src/parser/oracle_effect/sequence.rs:3747-3781 only substitutes the newly appended node and still adds forward_result/Attach when present. That leaves an executable default-controller move alongside the reported change_zone_enters_under_anaphor gap, so the parser reports an honest failure while the chain can still perform the wrong move.
The existing failure-closed test does not cover this production branch: crates/engine/src/parser/oracle_effect/tests.rs:30079-30091 tests a direct return (lower_put_ast), while the affected path is the SearchLibrary continuation. The integration test also explicitly documents that it passes with the pre-fix AST at crates/engine/tests/integration/issue_6691_enters_under_their_control.rs:18-37.
Suggested fix: make EntersUnderSpec::UnboundAnaphor abort or replace the entire search-destination assembly before apply_search_destination_to_ability_chain mutates its predecessor, including the attachment continuation. Add a parser/production-path regression that exercises an unbound search-destination clause and proves no executable ChangeZone or attachment survives.
🟡 Non-blocking
bind_control_clause ends with (p, _) at crates/engine/src/parser/oracle_nom/enters_under.rs:353. The module otherwise deliberately uses exhaustive controller mappings; enumerate the remaining anaphor/antecedent combinations so a future ControlClausePossessor cannot silently become an unexplained gap.
The current parse-diff sticky comment reports six affected cards, while the PR body claims seven and names Endless Whispers as an additional honest gap. Reconcile that claim with the current artifact before re-review; no changed signature in the sticky evidence identifies Endless Whispers. CI is green, but it does not resolve the assembly-path defect above.
Recommendation: request changes. Repair the whole SearchDestination fail-closed assembly and cover that exact path before re-review.
3 participants
Summary
Addresses #6691 for the
FilterProp::Ownedtemplate and theplayer_scopefan-out template; does not close it — residue enumerated below.The parser recognised only
"under your control"and the owner-stating forms."under their control"/"under that player's control"fell through the empty-tag arm of the alternation atoracle_effect/lower.rs:6705and were silently dropped, so permanents entered under the wrong controller while the cards reportedsupported: true, gap_count: 0— a silent wrong-game-result class.This collapses four independently-implemented copies of the
under <possessor> controlgrammar into one nom combinator module and binds the anaphor from typed parse products only, never a text scan or a lowered-tree walk:FilterProp::Owned { controller != You }→ControllerRef::ParentTargetOwner. Licensed by CR 400.1 + 400.3 + 404.1 (a card in a graveyard is in its owner's graveyard) + CR 108.3.ctx.relative_player_scope, mapped through an exhaustive, wildcard-free table admitting onlyScopedPlayer,ChosenPlayer,TriggeringPlayer.TargetPlayer/TargetOpponentare refused by name: two incompatible provenances, andTargetPlayerresolves to the first player target, which would put every land under one player's control on a multi-target cast (Turtle Tracks).Anything unbindable fails closed to
Effect::unimplemented, so a clause the parser cannot bind becomes an honest coverage gap instead of a wrong controller.EntersUnderSpeccarries that third state — whichOption<ControllerRef>cannot express, and whose absence is the bug — through the AST to every lowering site.Files changed
crates/engine/src/parser/oracle_nom/enters_under.rs(new)crates/engine/src/parser/oracle_nom/mod.rscrates/engine/src/parser/oracle_ir/ast.rscrates/engine/src/parser/oracle_effect/{lower,mod,imperative,sequence,tests}.rscrates/engine/src/parser/oracle_ir/snapshots/*.snap(2)crates/engine/tests/integration/issue_6691_enters_under_their_control.rs(new),tests/integration/main.rs,tests/fixtures/integration_cards.jsonCR references
CR 110.2a @618 (authorizing) · 110.2 @616 · 110.1 @614 · 608.2c @2793 · 608.2k @2814 · 608.2e @2798 · 400.1 @1933 · 400.3 @1937 · 404.1 @2030 · 108.3 @564 · 109.4 @594 · 109.5 @610 · 115.10 @886 · 102.2/102.3 @252/254 · 603.2 @2561 · 614.1c @3060 · 508.4 @2309 · 708.3 @5707
All grep-verified in
docs/MagicCompRules.txtbefore being written; the diff gate returns zero UNVERIFIED.Measured impact (full-pool diff, not predicted)
Regenerated
card-data.jsonbefore/after. Exactly 7 cards move; zero others change:ParentTargetOwnerScopedPlayerScopedPlayer933
your/owner-form cards (558+316+27+20+6+6) keep byte-identical behaviour while flowing through a single grammar. The 13 deferred possessor forms become a one-alt-arm extension instead of four parallel edits.Coverage moves down by 4 by design: those cards were reporting
gap_count: 0on apriority:p2-wrong-game-resultclause. An honest gap beats a wrong controller.Implementation method
Method: /engine-implementer — plan →
/review-engine-planx2 (both returned blockers; both resolved) → implement →/review-impl(returned a blocker; resolved, see Validation Failures).Track
Developer
LLM
Model: claude-opus-5
Thinking: high
Verification
Required checks ran clean.
Gate A output below is for the current committed head.
Both anchors cite existing analogous code at the same seam.
Final
review-implcould not be re-run after the last commit — see Validation Failures.cargo fmt --all— cleancargo check -p engine --lib— 0 errors, 0 warningscargo test -p engine --lib parser::— 8678 passed; 0 failedcargo test -p engine --test integration— 4164 passed; 0 failedcargo clippy -p engine --all-targets— no warnings./scripts/gen-card-data.sh— regenerated; full-pool diff aboveManual grep for the ungated blind spots (
.rfind/.split/.split_once/.contains() in added parser lines — one hit, a test assertion on a debug dump attests.rs:29926, not dispatch. Noallow-noncombinatoradded anywhere.Revert-sensitivity proven, not asserted. Temporarily binding
EntersUnderSpec::Defaultinstead ofOverride(ParentTargetOwner)makesjailbreak_enters_under_their_control_binds_to_the_ownerFAIL; restoring makes it pass.Gate A
Anchored on
crates/engine/src/parser/oracle_effect/lower.rs:6705— the existingenters_underalt()binding" under your control"->Some(ControllerRef::You)and returningNonefor the owner forms. The exact seam the new grammar replaces; it already carried its ownCR 110.2aannotation.crates/engine/src/parser/oracle_effect/lower.rs:6641— the sibling destination-table carrierenters_under: enters_under_you.then_some(ControllerRef::You), showing how the same controller override is threaded through the phrase-table path.Claimed parse impact
Residue — why this does not close #6691
supported: true, gap_count: 0withenters_under: "You"on its "and the rest onto the battlefield tapped under their control" leg — parser:under their controlbinding unparsed — permanents enter under the wrong controller (CR 110.2a) #6691's exact failure mode surviving this fix, because that leg is produced by a dig/reveal path this change does not rewire (sequence.rs:5468/5560/6868/6930).is_static_pattern()at priority 7 (its text contains "can't block") and never reaches these seams — a separate line-classifier defect.// FOLLOW-UPcomments: the four dig/reveal legs,imperative.rs:390-398,imperative.rs:6440, andoracle_trigger.rs:9597(the trigger-condition layer, CR 603.2 — deliberately out of scope, and the home of the 7 Trap/condition cards that must not be touched).under an opponent's controletc.) are explicit NPs, not anaphors, and are unchanged.Validation Failures
The final
review-implpass could not be re-run against the current head2b25293. An independentreview-impldid run against the preceding head and returned one BLOCKER plus five lower findings; the BLOCKER is fixed in this branch (commit 2 — the integration test passed with the fix reverted, because the fixture entry was stale and the runtime default coincides with the correct answer for Jailbreak). The environment then exhausted its model budget, so the confirming re-review did not run.Recorded but not fixed, for maintainer triage:
apply_search_destination_to_ability_chainis called withNoneon an unbound anaphor so the destination/tapped patches survive (deliberately — skipping the call would drop them), but a predecessorChangeZonecan still execute with the default controller while only the newly-pushed def becomesUnimplemented.fold_control_clauseswidens the pattern set over an unchanged span. The span is byte-identical to the literal it replaces, but the fold now also matches the anaphor anywhere in it. No printed card currently pairs a conditional"...enter the battlefield under their control this turn"with a battlefield put on the same normalized line, so the guard is incidental rather than structural.enters_under: Nonemeans "keep current controller".lower->textbyte offsets across a clause containing'and U+2019. Both are length-preserving underto_lowercase(so the flagged risk is not live), butTextPairwould make it structural.Also not run:
cargo coverage,cargo semantic-audit,ordering_parity_sweep,scripts/coverage-regression-check.sh. The full-poolcard-data.jsondiff above is the substantive evidence in their place; predicted bucketparser_regress(non-fatal), net supported delta -4, which CI will confirm.CI Failures
None observed locally.
Generated by Claude Code
Summary by CodeRabbit