parser: add 'is/are returned to [possessive] hand' trigger condition by Whovencroft · Pull Request #1266 · phase-rs/phase

Whovencroft · 2026-05-28T09:31:42Z

Summary

Add parser support for the "is/are returned to [possessive] hand" bounce-trigger zone-change pattern (CR 603.6c + CR 603.10a).

This unlocks Warped Devotion and the entire class of cards using this Oracle trigger template.

Cards Unlocked

Warped Devotion (PLS)
Azorius Aethermage (DIS)
Stormfront Riders (PLC)
Tameshi, Reality Architect (NEO)

Implementation

New function try_parse_returned_to_hand() in oracle_trigger.rs:

Detects "is returned to " / "are returned to " verb via alt + tag
Parses possessive hand forms: "your hand", "a player's hand", "its owner's hand", "their owner's hand", "their owners' hands", bare "hand"
Builds TriggerDefinition with mode: ChangesZone, origin: Battlefield, destination: Hand
Self-referential triggers extend trigger_zones to include Hand (source has already moved when the ability fires)

Call site wired after try_parse_put_into_hand_from in the zone-change trigger dispatch chain.

Rules References

CR 603.6c: Leaves-the-battlefield abilities trigger when a permanent moves from the battlefield to another zone — "returned to hand" is a zone change from Battlefield to Hand
CR 603.10a: Zone-change triggers that look back in time — includes abilities that trigger when an object is put into a hand
CR 400.3: Objects go to their owner's corresponding zone — governs which player's hand receives the returned permanent

Testing

Unit test: trigger_returned_to_a_players_hand — Warped Devotion pattern, verifies ChangesZone mode, origin: Battlefield, destination: Hand, no owner constraint
Unit test: trigger_returned_to_your_hand — Azorius Aethermage pattern, verifies controller constraint on valid_target
Local clippy ✅, fmt ✅, parser-combinator gate ✅

Diff Stats

Add try_parse_returned_to_hand() for bounce-trigger zone-change events (CR 603.6c + CR 603.10a). Handles possessive variants: your hand, a player's hand, its/their owner's hand, bare hand. Cards unlocked: Warped Devotion, Azorius Aethermage, Stormfront Riders, Tameshi Reality Architect.

gemini-code-assist

Code Review

This pull request implements parsing for zone-change triggers where a permanent is returned to a player's hand (bounce triggers), mapping them to a ChangesZone trigger mode from Battlefield to Hand. The reviewer provided two medium-severity findings based on the repository style guide: first, the local parse_returned_hand helper duplicates existing parser logic and uses fragile verbatim string matching instead of modular nom combinators; second, the CR annotation format in the doc comment deviates from the required standard.

gemini-code-assist · 2026-05-28T09:34:40Z

+    // Parse the possessive hand form.
+    fn parse_returned_hand(input: &str) -> OracleResult<'_, Option<TargetFilter>> {
+        alt((
+            // "your hand" → controller
+            value(
+                Some(TargetFilter::Typed(
+                    TypedFilter::default().controller(ControllerRef::You),
+                )),
+                tag("your hand"),
+            ),
+            // "its owner's hand" / "their owner's hand" → owner (no constraint)
+            value(None, tag("its owner's hand")),
+            value(None, tag("their owner's hand")),
+            value(None, tag("their owners' hands")),
+            // "a player's hand" → any player (no constraint)
+            value(None, tag("a player's hand")),
+            // bare "hand" (Tameshi)
+            value(None, tag("hand")),
+        ))
+        .parse(input)
+    }
+    let (_after_hand, valid_target) = parse_returned_hand.parse(after_verb).ok()?;


[MEDIUM] Duplication of hand possessive parsing and verbatim string matching. Evidence: crates/engine/src/parser/oracle_trigger.rs:9135-9156.\n\nWhy it matters: Defining a local parse_returned_hand helper duplicates the existing parse_hand_possessive helper and violates the codebase's building block reuse principle. Furthermore, matching verbatim strings bypasses the robust nom-based parser and creates fragile matches.\n\nSuggested fix: Extend the existing parse_hand_possessive helper. Instead of matching verbatim strings, decompose these compound phrases into modular, reusable parsers for constituent parts (e.g., owner/player subjects, possessive pronouns, and hand/hands nouns) and compose them using idiomatic nom combinators (like nested alt and tag sequences).

References

Building block reuse: check parser/oracle_nom/, parser/oracle_util.rs, game/filter.rs, game/quantity.rs, game/ability_utils.rs, game/keywords.rs, game/zones.rs, game/targeting.rs before any new helper. New helpers must justify their existence. ^(link)

Avoid verbatim string equality for parsing Oracle phrases as it bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts and compose them using idiomatic combinator aggregates.

gemini-code-assist · 2026-05-28T09:34:40Z

+/// Parse "[subject] is/are returned to [possessive] hand" — zone-change trigger
+/// for bounce effects (CR 603.6c + CR 603.10a). The verb "returned" implies the object moved


[MEDIUM] Incorrect CR annotation format. Evidence: crates/engine/src/parser/oracle_trigger.rs:9116-9117.\n\nWhy it matters: Rule R6 requires every rules-touching line of engine code to carry a comment of the form CR <number>: <description>. The current comment format (CR 603.6c + CR 603.10a) does not adhere to this standard.\n\nSuggested fix: Reformat the doc comment to use the standard CR <number>: <description> prefix.

Suggested change

/// Parse "[subject] is/are returned to [possessive] hand" — zone-change trigger

/// for bounce effects (CR 603.6c + CR 603.10a). The verb "returned" implies the object moved

/// CR 603.6c + CR 603.10a: "[subject] is/are returned to [possessive] hand" — zone-change trigger

/// for bounce effects. The verb "returned" implies the object moved

References

R6. CR annotations are mandatory and verified: Every rules-touching line of engine code must carry a comment of the form CR <number>: <description> (regex CR \d{3}(\.\d+[a-z]?)?). ^(link)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de4348e2cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-28T09:34:50Z

+    def.origin = Some(Zone::Battlefield);
+    def.destination = Some(Zone::Hand);
+    def.valid_card = Some(subject.clone());
+    def.valid_target = valid_target;


Apply the hand-owner constraint to zone-change matching

For Whenever a permanent is returned to your hand (e.g. Azorius Aethermage), this stores the your hand qualifier in valid_target, but TriggerMode::ChangesZone matching never reads valid_target—match_changes_zone only checks origin/destination and valid_card. As a result, the trigger also fires when an opponent's permanent is bounced to that opponent's hand; the qualifier needs to be represented in the zone-change object/owner filter (or the matcher needs an explicit destination-owner check) rather than in the ignored player-target field.

Useful? React with 👍 / 👎.

…x CR format Address review comments on PR phase-rs#1266: 1. Gemini #1: Extract local parse_returned_hand / parse_hand_possessive into a shared module-level parse_hand_possessive() that both try_parse_put_into_hand_from and try_parse_returned_to_hand call. Returns Option<ControllerRef> for cleaner semantics. 2. Gemini #2: Reformat doc comment to CR <number>: <description> style. 3. Codex #1: Move hand-owner constraint from valid_target (not read by match_changes_zone) to valid_card via add_controller(), so the zone-change matcher correctly filters by the bounced permanent's controller.

…um + drop ParseContext flag Addresses architecture review on phase-rs#1268: the original PR introduced two new bool fields (`Effect::Bounce.non_targeting`, `ParseContext.saw_target_keyword`) which violate the codebase's "no bool fields — parameterize with typed enums" principle (CLAUDE.md, feedback_no_bool_flags.md). This commit: 1. Adds `BounceSelection { Targeted, AtResolution }` in `types/ability.rs`. Replaces `non_targeting: bool` on `Effect::Bounce` with `selection`. Mirror change on the IR-layer `TargetedImperativeAst::Return` variant in `oracle_ir/ast.rs`. ~40 construction/destructure sites updated. 2. Drops `ParseContext::saw_target_keyword` and adds `parse_target_with_syntax` returning `TargetSyntax { TargetKeyword, Descriptor }` as a return-value discriminator. `parse_target_with_ctx` is kept as a 2-line wrapper so the ~95 unchanged call sites are unaffected. The two consumers (`oracle_effect/imperative.rs`, `oracle_effect/mod.rs`) compute `BounceSelection` directly from the returned syntax. 3. Adds `crates/phase-ai/tests/whitemane_lion_bounded.rs` — discriminating end-to-end integration test that loads a Whitemane-Lion-heavy deck mirror into the AI search engine and asserts the game terminates within 4000 actions (pre-fix would hit 10,000-action safety cap). `#[ignore]` per the existing `greasefang_bounded.rs` pattern (loads card-data.json). 4. Merges `origin/main` into the PR branch, picking up phase-rs#1261/phase-rs#1263/phase-rs#1266/phase-rs#1277 without conflict. Reverts an unrelated `known-tokens.toml` regeneration artifact (author's PR description explicitly excluded this file). Verification: - `cargo fmt --all` clean - `./scripts/check-parser-combinators.sh` clean - `cargo clippy --all-targets -- -D warnings` clean - `cargo test -p engine`: 9346 passed, 8 ignored - `./scripts/gen-card-data.sh` clean; Whitemane Lion exports `"selection": "at_resolution"` correctly - `cargo test -p phase-ai --test whitemane_lion_bounded -- --ignored` passes in ~61s (game completes naturally, well under 4000-action bound)

…e ETB) (#1268) * Fix #563: AI infinite loop casting Whitemane Lion (non-targeted bounce ETB) The Whitemane Lion ETB ("return a creature you control to its owner's hand") is non-targeted per CR 115.1 and the Whitemane Lion ruling, but the parser was wiring it as a targeted bounce and surfacing a TriggerTargetSelection slot. The AI filled that slot with the Lion itself, returning it to hand on resolution and re-casting it forever. Fix spans three layers: - Parser (root cause): add `saw_target_keyword: bool` to ParseContext and `non_targeting: bool` to Effect::Bounce so "return a creature you control" produces non_targeting=true, distinguished from "return target creature". - Resolver: extract_target_filter_from_effect carves out Bounce { non_targeting: true } so no target slot is opened. effects/bounce.rs adds a non-targeted branch that surfaces WaitingFor::EffectZoneChoice for the controller, mirroring the existing Sacrifice pattern (CR 608.2c/d). - AI (defence-in-depth): cast_facts.rs no longer requires targets for non-targeting bounces; search.rs caps same-card casts per turn at MAX_CASTS_OF_SAME_CARD_PER_TURN = 3 to prevent any remaining loop-prone pathology. Covers the class of non-targeted controller-scoped bounce ETBs: Whitemane Lion, Stonecloaker, Aether Channeler, Dream Stalker, Emancipation Angel, Esperzoa, Cache Raiders, Ambrosia Whiteheart, etc. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(PR-1268): replace non_targeting bool with BounceSelection enum + drop ParseContext flag Addresses architecture review on #1268: the original PR introduced two new bool fields (`Effect::Bounce.non_targeting`, `ParseContext.saw_target_keyword`) which violate the codebase's "no bool fields — parameterize with typed enums" principle (CLAUDE.md, feedback_no_bool_flags.md). This commit: 1. Adds `BounceSelection { Targeted, AtResolution }` in `types/ability.rs`. Replaces `non_targeting: bool` on `Effect::Bounce` with `selection`. Mirror change on the IR-layer `TargetedImperativeAst::Return` variant in `oracle_ir/ast.rs`. ~40 construction/destructure sites updated. 2. Drops `ParseContext::saw_target_keyword` and adds `parse_target_with_syntax` returning `TargetSyntax { TargetKeyword, Descriptor }` as a return-value discriminator. `parse_target_with_ctx` is kept as a 2-line wrapper so the ~95 unchanged call sites are unaffected. The two consumers (`oracle_effect/imperative.rs`, `oracle_effect/mod.rs`) compute `BounceSelection` directly from the returned syntax. 3. Adds `crates/phase-ai/tests/whitemane_lion_bounded.rs` — discriminating end-to-end integration test that loads a Whitemane-Lion-heavy deck mirror into the AI search engine and asserts the game terminates within 4000 actions (pre-fix would hit 10,000-action safety cap). `#[ignore]` per the existing `greasefang_bounded.rs` pattern (loads card-data.json). 4. Merges `origin/main` into the PR branch, picking up #1261/#1263/#1266/#1277 without conflict. Reverts an unrelated `known-tokens.toml` regeneration artifact (author's PR description explicitly excluded this file). Verification: - `cargo fmt --all` clean - `./scripts/check-parser-combinators.sh` clean - `cargo clippy --all-targets -- -D warnings` clean - `cargo test -p engine`: 9346 passed, 8 ignored - `./scripts/gen-card-data.sh` clean; Whitemane Lion exports `"selection": "at_resolution"` correctly - `cargo test -p phase-ai --test whitemane_lion_bounded -- --ignored` passes in ~61s (game completes naturally, well under 4000-action bound) --------- Co-authored-by: Michael Briningstool <mbriningstool@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Matt Evans <1388610+matthewevans@users.noreply.github.com>

Whovencroft requested a review from matthewevans as a code owner May 28, 2026 09:31

github-actions Bot added the ai-contribution PR opened via docs/AI-CONTRIBUTOR.md flow label May 28, 2026

gemini-code-assist Bot reviewed May 28, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 28, 2026

View reviewed changes

matthewevans added the status:ready-to-merge Maintainer-reviewed and ready to merge label May 28, 2026

matthewevans added this pull request to the merge queue May 28, 2026

Merged via the queue into phase-rs:main with commit a12c7ca May 28, 2026
9 checks passed

matthewevans mentioned this pull request May 28, 2026

Fix #563: AI infinite loop casting Whitemane Lion (non-targeted bounce ETB) #1268

Merged

Whovencroft deleted the card/warped-devotion branch July 9, 2026 02:27

Navigation Menu

Uh oh!

Uh oh!

parser: add 'is/are returned to [possessive] hand' trigger condition - #1266

parser: add 'is/are returned to [possessive] hand' trigger condition#1266
matthewevans merged 2 commits into
phase-rs:mainfrom
Whovencroft:card/warped-devotion

Whovencroft commented May 28, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

gemini-code-assist Bot May 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		/// Parse "[subject] is/are returned to [possessive] hand" — zone-change trigger
		/// for bounce effects (CR 603.6c + CR 603.10a). The verb "returned" implies the object moved

Uh oh!

Uh oh!

Conversation

Whovencroft commented May 28, 2026

Summary

Cards Unlocked

Implementation

Rules References

Testing

Diff Stats

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants