fix(engine): Metamorphic Alteration as-enters choose + host copy (#6013)

[claytonlin1110]

Summary

• Parse Metamorphic Alteration's as-enters permanent choice (Effect::ChoosePermanent) and enchanted-host copy static (CopyChosen marker).
• On answer, latch the chosen creature's copiable values (+ display trio) on the Aura and install a Layer-1 CopyValues TCE on the enchanted host via apply_precomputed_copy_values (UntilHostLeavesPlay) — the Aura itself is never BecomeCopy'd.
• Reuses WaitingFor::CopyTargetChoice with a PersistChosenAttribute purpose discriminant (no new ObjectPick WaitingFor).
• ChoosePermanent { CopiableSnapshot } is injected only via LinkedChoiceKind::CopyChosenHost when a companion CopyChosen static is present; bare as-enters permanent chooses (Dauntless Bodyguard, Scheming Fence) stay Unimplemented ability gaps.

Fixes #6013

Test plan

• enchanted_creature_becomes_copy_of_chosen_and_aura_is_unchanged — cast Aura, choose donor; host copies, Aura unchanged
• Frozen snapshot after donor P/T mutation (CR 707.2b) — copied_values_are_a_frozen_snapshot_of_the_chosen_creature
• Host reverts when Aura leaves (CR 400.7 / 611.2a) — host_reverts_when_the_aura_leaves_play
• Hexproof opponent donor remains choosable (choice ≠ target, CR 115.10a) — hexproof_opponent_creature_is_a_legal_copy_donor
• Two independent Auras do not bleed copies — two_auras_install_independent_copies_on_their_own_hosts
• Token donor display routing follows the copy — host_copying_a_token_donor_routes_token_display
• Sole-host self-copy reach-guards CopiableSnapshot + Aura-sourced CopyValues TCE — host_is_the_only_creature_and_remains_a_legal_copy_choice
• Non-spell Aura entry (dig → BF) — non_spell_aura_entry_copies_chosen_creature
• Parser shape: ChoosePermanent + CopyChosen/EnchantedBy — as_enters_choose_a_creature_parses_to_choose_permanent, enchanted_is_a_copy_of_chosen_parses_to_copy_chosen_static
• Coverage honesty: non-CopyChosen as-enters permanent choose stays unsupported — non_copy_chosen_as_enters_permanent_choose_stays_unsupported (Dauntless Bodyguard, Scheming Fence)
• Clone/Vesuva still green (purpose: BecomeCopy default) — covered by existing copy-choice regressions on this branch
• CI: clippy, test-engine, card-data, check-frontend (card-data / frontend / wasm / tauri green on e42fc8313; Rust shards + lint still settling)

Validation notes

Discriminating runtime coverage lives in crates/engine/tests/integration/metamorphic_alteration.rs. Plan/impl review completed via engine-implementer pipeline. Blast-radius narrowing: no line-local Moved claim for bare permanent chooses; CopyChosenHost is the sole ChoosePermanent injection path.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Metamorphic Alteration now parses Aura-enter creature choices, stores the chosen creature’s copiable snapshot, applies it to the enchanted host, and preserves existing copy-choice behavior through purpose-tagged waiting states. Classification handling and integration tests cover resolution, lifecycle, legality, independence, token display, and parsing.

Changes

Metamorphic Alteration copy-choice flow

Layer / File(s)	Summary
Choice and copy contracts crates/engine/src/types/ability.rs, crates/engine/src/types/game_state.rs, client/src/adapter/types.ts	Adds Effect::ChoosePermanent, copiable snapshot storage, CopyTargetPurpose, and the purpose field on copy-choice waiting data.
Oracle parsing and marker lowering crates/engine/src/parser/oracle_replacement.rs, crates/engine/src/parser/oracle_static/*, crates/engine/src/parser/oracle_effect/*	Parses Aura-enter object choices and lowers the enchanted-host copy sentence to ContinuousModification::CopyChosen.
Snapshot choice resolution crates/engine/src/game/engine_replacement.rs, crates/engine/src/game/effects/mod.rs, crates/engine/src/game/layers.rs, crates/engine/src/game/stack.rs, crates/engine/src/game/scenario.rs	Creates purpose-tagged prompts, snapshots the selected donor, installs the host copy effect, preserves pending resolution context, replays deferred entry events, and prevents duplicate copy installation.
Engine classification and integration crates/engine/src/analysis/*, crates/engine/src/game/{ability_rw.rs,ability_scan.rs,coverage.rs,printed_cards.rs,quantity.rs,trigger_index.rs}, crates/phase-ai/src/policies/*	Adds conservative, inert, leaf, formatting, trigger, and policy handling for the new effect and marker variants.
Regression and integration validation crates/engine/tests/integration/*, crates/engine/src/game/engine_phase_trigger_regression_tests.rs, crates/phase-ai/src/policies/copy_value.rs	Updates existing copy-choice fixtures and adds gameplay and oracle tests for snapshots, removal, legality, independence, token display, and parsing.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant OracleParser
  participant ReplacementEngine
  participant CopyTargetChoice
  participant SnapshotHandler
  participant LayerEngine
  OracleParser->>ReplacementEngine: Parse Effect::ChoosePermanent
  ReplacementEngine->>CopyTargetChoice: Create PersistChosenAttribute prompt
  CopyTargetChoice->>SnapshotHandler: Submit selected creature
  SnapshotHandler->>LayerEngine: Install latched Layer-1 copy effect
  SnapshotHandler->>ReplacementEngine: Replay deferred Aura entry events

Loading

Suggested labels: bug, enhancement

Suggested reviewers: matthewevans, ntindle, lgray

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements the reported bug by adding choose-permanent parsing and persisting the chosen copiable values on the Aura host.
Out of Scope Changes check	✅ Passed	The changes are all tied to Metamorphic Alteration support, parsing, state handling, and tests, with no clear unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: Metamorphic Alteration’s as-enters choice and host copy behavior.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

crates/engine/src/game/effects/mod.rs (1)

3830-3843: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: add a release-visible diagnostic alongside the debug_assert.

If Effect::ChoosePermanent ever incorrectly reaches this dispatch arm in a release build, it silently no-ops with no trace — unlike the Effect::Unimplemented arm just below, which logs via eprintln! even without assertions. Mirroring that pattern here would make a future violation of the "replacement-only" contract observable outside debug builds.

🤖 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/game/effects/mod.rs` around lines 3830 - 3843, Add a
release-visible diagnostic to the Effect::ChoosePermanent dispatch arm alongside
its existing debug_assert, mirroring the eprintln! behavior of the nearby
Effect::Unimplemented arm. Preserve the current Ok(()) return and
replacement-only handling contract.

🤖 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/game/engine_replacement.rs`:
- Around line 1393-1430: Update the `else` branch handling a missing `host`
after the `host` lookup to return `EngineError::InvalidAction` with the existing
diagnostic context, rather than relying on `debug_assert!` and continuing
dispatch. Preserve the successful `apply_precomputed_copy_values` path
unchanged.

---

Nitpick comments:
In `@crates/engine/src/game/effects/mod.rs`:
- Around line 3830-3843: Add a release-visible diagnostic to the
Effect::ChoosePermanent dispatch arm alongside its existing debug_assert,
mirroring the eprintln! behavior of the nearby Effect::Unimplemented arm.
Preserve the current Ok(()) return and replacement-only handling contract.

🪄 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: df474efb-30aa-4979-b5dd-b2c446c81dfb

📥 Commits

Reviewing files that changed from the base of the PR and between ebd8844 and 82de74c.

📒 Files selected for processing (26)

• client/src/adapter/types.ts
• crates/engine/src/analysis/ability_graph.rs
• crates/engine/src/game/ability_rw.rs
• crates/engine/src/game/ability_scan.rs
• crates/engine/src/game/coverage.rs
• crates/engine/src/game/effects/mod.rs
• crates/engine/src/game/engine_phase_trigger_regression_tests.rs
• crates/engine/src/game/engine_replacement.rs
• crates/engine/src/game/layers.rs
• crates/engine/src/game/printed_cards.rs
• crates/engine/src/game/quantity.rs
• crates/engine/src/parser/oracle_effect/lower.rs
• crates/engine/src/parser/oracle_effect/sequence.rs
• crates/engine/src/parser/oracle_ir/doc.rs
• crates/engine/src/parser/oracle_replacement.rs
• crates/engine/src/parser/oracle_static/dispatch.rs
• crates/engine/src/parser/oracle_static/shared.rs
• crates/engine/src/types/ability.rs
• crates/engine/src/types/game_state.rs
• crates/engine/src/types/layers.rs
• crates/engine/tests/integration/main.rs
• crates/engine/tests/integration/metamorphic_alteration.rs
• crates/phase-ai/src/policies/copy_value.rs
• crates/phase-ai/src/policies/effect_classify.rs
• crates/phase-ai/src/policies/redundancy_avoidance.rs
• crates/phase-ai/src/policies/x_reference.rs

[github-actions]

Parse changes introduced by this PR · 7 card(s), 10 signature(s) (baseline: main 7de942fc9419)

🟢 Added (2 signatures)

• 1 card · ➕ static/Continuous · added: Continuous (affects=enchanted by self creature, mods=copy chosen)
  • Affected (first 3): Metamorphic Alteration
• 1 card · ➕ replacement/Moved · added: Moved (scope=self, to zone=battlefield)
  • Affected (first 3): Metamorphic Alteration

🔴 Removed (2 signatures)

• 1 card · ➖ ability/static_structure · removed: static_structure
  • Affected (first 3): Metamorphic Alteration
• 1 card · ➖ ability/unknown · removed: unknown
  • Affected (first 3): Metamorphic Alteration

🟡 Modified fields (6 signatures)

• 1 card · 🔄 ability/Choose · changed field choice: one of: Armor, Blood, Book, Bottle, Fang, Fire, Scroll, Spear, Sword, Water. → one of: Armor, Blood, Book, Bottle, Fang, Fire, Scroll, Spear, Sword, Water
  • Affected (first 3): Ignacio of Myra's Marvels
• 1 card · 🔄 ability/Choose · changed field choice: one of: Artifact, Enchantment, Instant, Sorcery, Planeswalker. → one of: Artifact, Enchantment, Instant, Sorcery, Planeswalker
  • Affected (first 3): Archon of Valor's Reach
• 1 card · 🔄 ability/Choose · changed field choice: one of: Black, Red. → one of: Black, Red
  • Affected (first 3): Mangara's Equity
• 1 card · 🔄 ability/Choose · changed field choice: one of: Island, Swamp. → one of: Island, Swamp
  • Affected (first 3): Roots of Life
• 1 card · 🔄 ability/Choose · changed field choice: one of: Left, Right. → one of: Left, Right
  • Affected (first 3): Pramikon, Sky Rampart
• 1 card · 🔄 ability/Choose · changed field choice: one of: White citizen, Blue camarid, Black thrull, Red goblin, Green saproling. → one of: White citizen, Blue camarid, Black thrull, Red goblin, Green saproling
  • Affected (first 3): Sarpadian Empires, Vol. VII

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

[matthewevans]

Blocked — the submitted head does not parse or apply Metamorphic Alteration's as-enters copy choice.

🔴 Blocker

1. crates/engine/src/parser/oracle_replacement.rs:2297 — scan_split_at_phrase(..., "choose ") supplies the suffix after choose (as shown by the existing named-choice path passing that value directly to try_parse_named_choice). parse_as_enters_choose_permanent then requires a second leading "choose ", so it returns None for the actual Oracle line. The current-head CI confirms this: as_enters_choose_a_creature_parses_to_choose_permanent fails, and every behavior test leaves the host as Grizzly Bears. Parse the suffix directly with the shared target descriptor, then keep the full-consumption guard.

2. crates/engine/src/game/engine_replacement.rs:1376 — after that parser fix, the proposed continuation still reads aura.attached_to before the Aura-spell attachment happens. The stack path drains apply_pending_post_replacement_effect before its CR 608.3c Aura-attachment block, so this branch reaches the release-mode no-op and never installs the copy. Carry/establish the resolved Aura host through the continuation instead of relying on a debug assertion, and cover both normal spell resolution and a non-spell Aura entry.

The two exact census assertions also need updating for the new effect surface. CI run 29996478044 is red on this exact head (a6cf3ce6): both Rust test shards fail, including all six Metamorphic behavior tests.

Request changes: fix the suffix parser and entry/attachment sequencing, add discriminating spell and non-spell entry coverage, then rerun CI.

[coderabbitai]

🧹 Nitpick comments (1)

crates/engine/src/game/stack.rs (1)

1081-1135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Centralize the pending-resolution snapshot construction.

This duplicates the field-by-field snapshot assembly in Lines 1281-1334. Extract a shared builder so both choice paths preserve identical cast and cost metadata.

As per coding guidelines, “Reuse existing shared building blocks before adding utility or inline extraction logic.”

🤖 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/game/stack.rs` around lines 1081 - 1135, The
pending-resolution snapshot assembly around push_spell_resolution is duplicated
with the equivalent construction near the other choice path. Extract or reuse a
shared builder for PendingSpellResolution that gathers cast_from_zone,
kickers_paid, additional cost payment fields, and related metadata, then update
both paths to use it so their snapshots remain identical.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@crates/engine/src/game/stack.rs`:
- Around line 1081-1135: The pending-resolution snapshot assembly around
push_spell_resolution is duplicated with the equivalent construction near the
other choice path. Extract or reuse a shared builder for PendingSpellResolution
that gathers cast_from_zone, kickers_paid, additional cost payment fields, and
related metadata, then update both paths to use it so their snapshots remain
identical.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ba02f72-fc55-43fc-b164-8fbecf91f18e

📥 Commits

Reviewing files that changed from the base of the PR and between a6cf3ce and c93e959.

📒 Files selected for processing (3)

• crates/engine/src/game/ability_scan.rs
• crates/engine/src/game/engine_replacement.rs
• crates/engine/src/game/stack.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• crates/engine/src/game/ability_scan.rs
• crates/engine/src/game/engine_replacement.rs

[matthewevans]

Re-reviewed current head c93e9596. The new stack continuation correctly addresses the earlier attachment-order observation and updates the census guards, but the parser blocker remains unchanged: scan_split_at_phrase(..., "choose ") hands parse_as_enters_choose_permanent the suffix after choose , while that helper still requires another tag("choose "). Thus As this Aura enters, choose a creature. still produces no replacement at all, and the requested parser/real-cast regressions cannot pass. Keeping this as requested changes on the new head.

[matthewevans]

Blocked: the current head cannot compile its engine integration tests, and the new generic object-choice lowering assigns Metamorphic Alteration's copy-host semantics without proving that relation.

🔴 Blocker

1. [HIGH] Fix the non-spell integration fixture before this can merge. The fixture writes and reads GameObject::replacements / statics, but those fields do not exist: the live fields are replacement_definitions and static_definitions, with matching base definitions. Both Rust test shards fail with E0609 at these lines in run 30001032013. Stage the object through the existing fixture path or populate the live and base definition fields, then keep the non-spell regression assertion.

2. [MED] Do not lower every parseable As … enters, choose <permanent> clause to CopiableSnapshot. The new filter accepts any concrete Typed object phrase and always constructs ChoosePermanentPersist::CopiableSnapshot, while the resolver ignores the persistence discriminator and always issues PersistChosenAttribute. That purpose is specifically a copy effect on an Aura's enchanted host, documented as Metamorphic-only here. This grammar admits unrelated selected-permanent effects; until the parser proves the accompanying CopyChosen consumer relation, preserve the strict unsupported marker (or model a general selected-permanent contract) rather than silently giving them copy-host behavior.

Request changes: address both blockers and re-run the PR checks.

[matthewevans]

Blocked — the current parser delta still has unexplained card scope.

🔴 Blocker

1. [HIGH] Account for the non-Metamorphic parser blast radius. Evidence: the current-head parse-diff sticky comment reports replacement-shape changes for four cards, including Dauntless Bodyguard and Scheming Fence, while the PR summary claims Metamorphic Alteration. Why it matters: this generic as-enters chooser path changes unsupported-card structure beyond the stated card and needs a class-level proof that each residual stays explicitly unsupported rather than becoming supported by shape alone. Suggested fix: either narrow the parser entry to the proven CopyChosen relation, or enumerate the affected class and add a coverage-honesty test for representative non-CopyChosen cards.

🟡 Non-blocking

• CodeRabbit notes on earlier heads are superseded or optional; the current blocking concern is the required parse-diff reconciliation.

Recommendation: request changes; reconcile the four-card parse diff and add the corresponding honest-coverage proof before re-review.

[matthewevans]

Reviewed current head 3a0e901261b0e2a61290bcb7aa9be5ec562ca6de.

The follow-up now keeps non-CopyChosen as-enters object choices as explicit unsupported abilities and injects ChoosePermanent only through the CopyChosenHost relation, addressing the prior scope finding. I found no additional code-level blocker in this follow-up.

Held: required CI is not yet green: Rust tests (shard 2/2) currently reports failure, while the CI workflow and Rust lint/card-data checks are still running. Please resolve and let the checks complete before an approval/enqueue pass.

[matthewevans]

Verdict: held — this current-head follow-up resolves the fixture API issue, but approval is not yet supported by the required proof and verification evidence.

✅ Clean

crates/engine/tests/integration/metamorphic_alteration.rs:521 now uses the public Definitions::iter_unchecked() API, addressing the test-fixture compile-path correction without changing production behavior.

🔴 Blocker

crates/engine/tests/integration/metamorphic_alteration.rs still leaves the PR body’s runtime verification checklist unchecked (copy snapshot, host reversion, non-targetability, and independent-Aura isolation). The current required Rust checks are also still running. The implementation has a broad engine/parser surface, so this lacks the concrete current-head proof required before approval.

Recommendation: provide the completed discriminating runtime evidence and let the required checks settle; then it can return for approval.

[matthewevans]

Held on current head 90f4179f. The spell-path continuation change addresses the previously identified dropped mid-entry choice, but required Rust/frontend checks are still pending. No approval or queue action while that external verification is incomplete.

[claytonlin1110]

Follow-up for held review (90f4179fb)

Root cause of missing spell-path runtime proof: permanent-spell resolution uses PostReplacementDrainOwner::CallerEpilogue, then discarded apply_pending_post_replacement_effect's returned WaitingFor. Dig/delivery kept working because DeliveryTail assigns state.waiting_for and returns NeedsChoice.

Fix: honor non-Priority mid-entry waits from the CallerEpilogue drain — set waiting_for, stash PendingSpellResolution, emit StackResolved, return before Aura-attach epilogue (mirrors delivery NeedsChoice). Also mark Effect::ChoosePermanent order-independent in replacement materiality.

Discriminating runtime checklist → tests

Required proof	Test
Copy snapshot (frozen)	copied_values_are_a_frozen_snapshot_of_the_chosen_creature
Host reversion on Aura leave	host_reverts_when_the_aura_leaves_play
Non-targetability (hexproof donor)	hexproof_opponent_creature_is_a_legal_copy_donor
Independent-Aura isolation	two_auras_install_independent_copies_on_their_own_hosts
Spell-path host copy + Aura unchanged	enchanted_creature_becomes_copy_of_chosen_and_aura_is_unchanged
Reach-guard (snapshot + CopyValues TCE)	host_is_the_only_creature_and_remains_a_legal_copy_choice

All live in crates/engine/tests/integration/metamorphic_alteration.rs. Please re-check once Rust shards settle green on 90f4179fb.

[matthewevans]

Held on current head e5daa599. The CallerEpilogue follow-up removes the pending-spell wrapper that buried the active continuation and instead preserves the replacement choice while the normal Aura/cast epilogue completes. The existing Metamorphic integration coverage remains the relevant production-path evidence. Required Rust verification is still pending, with the current Rust shard 1 and Tauri jobs failed, so this is not ready for approval or queueing.

[claytonlin1110]

Follow-up 942cbcbfb — attach before CallerEpilogue drain

Reorder: CR 608.3c Aura attach runs first, then honor the post-replacement WaitingFor (Metamorphic CopyTargetChoice / Tribute NamedChoice). Matches dig order (attached_to before PersistChosenAttribute answer) without pushing PendingSpellResolution over AbilityContinuations.

Tip: 942cbcbfb. Please re-check when Rust shards settle.

[claytonlin1110]

Addressed the Aura-host blocker on 5fb54980c:

• CallerEpilogue CopyTargetChoice now pushes PendingSpellResolution with the cast spell_targets and returns before Aura attach / cast-link stamps (mirrors DeliveryTail NeedsChoice).
• Removed Enchant-filter recovery from stack.rs and the spell-path fallback in handle_persist_chosen_attribute_choice (CR 608.3c / CR 303.4a — host is the cast target, not a CR 303.4f consult).
• Fixture: from_oracle_text_with_keywords(&["enchant"], …) so normal Enchant creature is installed; removed pin_enchant_host.
• Regression: spell_path_retains_chosen_enchant_target_among_multiple_legal_hosts — ≥2 legal creatures, no Enchant pin, asserts attach to the cast target.

Local MSVC link.exe unavailable here; waiting on CI Rust shards.

[matthewevans]

Every prior blocker is resolved on b71d2c70 — the remaining items are three small, mechanical fixups, and required Rust CI is still pending.

Re-reviewed the delta since my last review head (7c9276ed): 5fb54980, 4d35c00d, the main merge, 46f4cd53, and b71d2c70.

Card premise verified against Scryfall, not the PR description:

Enchant creature
As this Aura enters, choose a creature.
Enchanted creature is a copy of the chosen creature.

✅ Clean

The Aura-host blocker is properly fixed. The resolve_entering_aura_attachment fallback is gone from the Aura arm of resolve_top (stack.rs:1495-1523), and engine_replacement.rs:1409 now gates the CR 303.4f consult behind !took_spell_resolution. This is the correct reading of CR 303.4a — "An Aura spell requires a target, which is defined by its enchant ability." The spell's host is its cast target and must never be re-derived from the Enchant filter.

The regression is discriminating and honestly built. metamorphic_alteration.rs:136 stages two legal creatures (host, other_legal), uses from_oracle_text_with_keywords(&["enchant"], …) so the real Enchant creature filter is installed, and asserts the fixture is not pinned before exercising it:

Keyword::Enchant(TargetFilter::Typed(tf))
    if tf.type_filters.contains(&TypeFilter::Creature)

Then asserts attachment lands on the cast target and that other_legal keeps its own name. Reverting the stack.rs removal reintroduces the ambiguous consult and fails this. The fixture-sanity assertion directly answers the pin_enchant_host objection.

Coverage honesty is proven, not asserted. metamorphic_alteration.rs:750 pairs its two negative assertions with a positive reach-guard:

a.effect.unimplemented_description()
    .is_some_and(|d| d.to_lowercase().contains("choose"))

That proves the Dauntless Bodyguard / Scheming Fence choose lines actually reached the parser and were preserved as explicit Unimplemented gaps. A bare "no ChoosePermanent emitted" would have passed vacuously on a card that failed to parse at all.

Parse-diff scope reconciles. The sticky comment (20:14:02Z, one commit behind b71d2c70, which touched only engine_replacement.rs — so it is materially current) shows only Metamorphic Alteration gaining Continuous (…mods=copy chosen) + Moved (scope=self, to zone=battlefield). Dauntless Bodyguard and Scheming Fence are gone from the diff. The six residual ability/Choose rows are trailing-period label normalization (one of: Island, Swamp. → one of: Island, Swamp), and Card data (generate, validate, coverage) is green.

CR annotations grep-verify. All 25 CR numbers this PR adds resolve in docs/MagicCompRules.txt, and CR 614.1c ("Effects that read … 'As [this permanent] enters . . .'") and CR 607.2d (linked "choose a [value]" ↔ "the chosen") are exactly the right rules for this seam. Effect::ChoosePermanent is exhaustively added to all four phase-ai policy arms with no wildcard.

Registration is real. crates/engine/tests/integration/main.rs:680 — mod metamorphic_alteration;. All 12 tests are live.

🟡 Non-blocking

1. ChoosePermanentPersist is a discriminator nothing reads — types/ability.rs:20566. This is the still-open half of my earlier MED. The enum has one variant, and every consumer destructures it away: ability_rw.rs:3167 ({ filter, .. }), ability_scan.rs:898 (persist: _), coverage.rs:2337 ({ filter, .. }), and the resolver itself at engine_replacement.rs:2147 ({ ref filter, .. }), which unconditionally emits purpose: CopyTargetPurpose::PersistChosenAttribute at :2157. The behavior is already selected by CopyTargetPurpose, which is live at engine_replacement.rs:1545. A second ChoosePermanentPersist variant would compile at every .. / persist: _ site and silently inherit CopiableSnapshot semantics. Either drop the field — the CopyChosenHost relation already gates emission — or match it exhaustively at engine_replacement.rs:2147.

2. PendingSpellResolution assembly is now duplicated three ways — stack.rs:1129, :1317, :1596. The new arm is a ~55-line near-verbatim copy of the site above it, repeating the same eleven fields with identical ability.as_ref().map(…).unwrap_or_else(|| state.objects.get(&entry.id)…) fallback chains. This confirms CodeRabbit's open nitpick, and the third copy raises the stakes: the next cast-metadata field added will be threaded into two of three sites, and the Metamorphic path drops it silently. Extract fn pending_spell_resolution_snapshot(state, &entry, ability.as_ref(), casting_variant, actual_mana_spent, &spell_targets) -> PendingSpellResolution and call it from all three.

3. The completion tail diverges from its BecomeCopy sibling — engine_replacement.rs:1479-1501. It retires the drain before replay_deferred_entry_events; the BecomeCopy tail at :1878-1892 retires after, and additionally brackets replay with capture_paused_zone_change_delivery_for_member and a drain_pending_batch_deliveries step. Your inline comment gives a real reason for the inversion (avoiding a second CopyTargetChoice re-drain), so this is a request to justify the other two omissions: if an observer ETB raised during replay returns a nested OrderTriggers, this path returns with the drain retired and no delivery capture. I could not prove a reachable failure — an Aura spell entry looks like a single-object delivery with no active batch — so please either confirm that in a comment or align the tail.

4. scenario.rs:3300 breaks harness uniformity — the CopyTargetChoice arm now panic!s on an undeclared policy while every sibling arm (ReplacementChoice:3231, NamedChoice:3237, SpellbookDraft:3245, DiscardChoice, EffectZoneChoice) breaks. The SpellbookDraft arm documents the contract explicitly: "No pick declared → halt so the caller can assert the offered options and the draft boundary via final_waiting_for()." I confirmed no current test regresses — origin/main contains no .copy_target( call — but this removes the boundary-assert capability for this one prompt. Prefer keeping break; if the fail-loud behavior is genuinely better, apply it to all arms in a separate PR.

5. Inherited, not yours: the CR 615.5 citation at engine_replacement.rs:1479 describes prevention effects, not post-replacement drain retirement. It matches five pre-existing occurrences on origin/main, so you followed the surrounding convention correctly. No action here — I'll fix the cluster repo-wide.

---

Prior requested changes, all resolved: the scan_split_at_phrase suffix bug (now scan_preceded, oracle_replacement.rs:2124); the Aura attach-before-choice ordering; the E0609 fixture fields (now Definitions::iter_unchecked()); over-broad CopiableSnapshot lowering (now gated on LinkedChoiceKind::CopyChosenHost); the non-Metamorphic parse blast radius; and the Enchant-filter host substitution.

Recommendation: land fixups 1 and 2 — drop or exhaustively match persist, and extract the shared PendingSpellResolution builder — and answer 3 in a comment. Required Rust (fmt, clippy, test, coverage-gate) is still pending on this head, so no approval or enqueue until it settles green.

[matthewevans]

All three fixups landed by the author in a08ffb0a, plus the harness-uniformity item. Approving.

I had prepared a maintainer fixup for items 1 and 3 and discarded it — the author's a08ffb0a is the better version and arrived first. On item 1 they took the drop option rather than my exhaustive-match option; that is the cleaner of the two the review offered, since CopyTargetPurpose + LinkedChoiceKind::CopyChosenHost already carry the discrimination. Verified ChoosePermanentPersist is gone with zero dangling references repo-wide.

Item 2 verified safe — the part that actually needed proving

pending_spell_resolution_snapshot re-derives from state at call time, replacing two captures that were previously taken before replace_event (convoked_creatures, cast_timing_permission). Since reset_for_battlefield_entry clears all six of these fields (CR 400.7), a naive extraction here could have silently dropped convoke / kicker / cast-timing provenance through the pause detour. I traced it rather than assuming:

• Stack→Battlefield: CastLinkSnapshot is captured pre-move (zone_pipeline.rs:2194) and restored at :2328 — explicitly before the face-down / counter blocks, so it precedes every NeedsChoice park (:2499, :2530). The snapshot therefore reads restored values identical to the deleted pre-captures.
• Stack→Graveyard: cast_link is None (gate requires to == Battlefield), but reset_for_battlefield_entry never runs either — fields are untouched. Equal.
• The one pre-restore early return (:2143, merged-permanent leave) is gated from == Zone::Battlefield, so it is unreachable from a Stack resolution.
• reset_for_battlefield_exit (the other convoked_creatures.clear()) is battlefield-exit only — not a Stack exit.

Conclusion: behavior-preserving at all three call sites. The extraction is a genuine DRY win — a new cast-metadata field can no longer be threaded into two of three sites.

Item 3's comment now answers both omissions (delivery-capture and the CR 702.49c batch drain) and is honest that the batch argument is reachability, not an enforced invariant.

Not addressed here, correctly: the CR 615.5 citation is wrong (it describes prevention effects) but is pre-existing on main in 5 places and inherited, not introduced. I'm fixing that cluster repo-wide separately — it is not this contributor's debt.

Required checks are re-running on a08ffb0a; enqueuing with --auto so the queue waits on them.

[matthewevans]

Temporarily dequeued by a maintainer — nothing is wrong with this PR, and it will be re-enqueued shortly.

main is currently broken in a way that reds the merge queue for everyone. The release: v0.35.2 commit (21a53d50cb) bumped client/src-tauri/Cargo.toml to 0.35.2 without regenerating client/src-tauri/Cargo.lock, which still pins phase-tauri 0.35.1. The Tauri compile check job builds with cargo check --locked, so it fails:

error: cannot update the lock file /home/runner/work/phase/phase/client/src-tauri/Cargo.lock
       because --locked was passed to prevent this

That job is a needs: dependency of the required Rust (fmt, clippy, test, coverage-gate) aggregator, so it hard-blocks merges even though GitHub reports it as not required. Any merge-group ref that does not contain the fix fails and gets ejected — which is what was happening to the PRs in the queue, including this one.

The fix is #6568. It was queued behind the PRs it needs to unblock, which is a deadlock: the queue merges in order, so it could not land until those cleared, and they could not clear until it landed. Dequeuing the entries ahead of it is how that gets broken.

No action needed from you. Your PR keeps its approval and its head is unchanged. Once #6568 merges, this will be re-enqueued and should go through on the same green it already had. Apologies for the churn — this was our breakage, not yours.

[matthewevans]

Closing this PR, along with the other open PRs from this account, as a maintainer decision about contribution process rather than about any particular line of code here.

The pattern across these submissions has been automated generation with our CI and review feedback acting as the correction loop — changes pushed before they are understood, then iterated against whatever comes back red. The PR histories show it directly: reverting a fix and pushing a panic!() diagnostic commit to read parser output out of a CI run, and test edits landing under commit messages that do not mention the assertions they remove.

That model doesn't work for this repository. Review capacity here is the scarce resource, and it is spent on rules-correctness questions that require someone to have reasoned about the Comprehensive Rules and the engine's layer architecture before pushing. When the correction loop is outsourced to maintainer review, each round consumes that capacity without transferring understanding, and the same class of defect returns on the next PR.

To be clear about what this is not: it is not a verdict that the code in this PR is wrong. Some of this work was genuinely useful and several of these changes landed. Nor is it a judgement about using AI tooling — plenty of good contributions here are AI-assisted. The distinction is whether the author has verified and understood the change before it reaches review.

If you want to contribute here again, the bar is a change you can explain end to end: which Comprehensive Rules section it implements, why that seam is the correct one, and what a test would have to do to fail if the change were reverted. Open an issue describing that first and we'll talk before any code is written.

Thank you for the work that did land.

[matthewevans]

Reopening this. The closure was a maintainer error, and the reasoning I posted was wrong on the facts.

I closed this PR yesterday as part of a batch action against this account. That was my call and it was not correct. Here is the specific thing I got wrong, because you are owed the detail rather than a vague retraction.

The trigger was Co-Authored-By: Claude Haiku 4.5 trailers on commits. I treated that as an out-of-policy model. It was not:

• The tier table in force listed claude-haiku-4-5 explicitly in the Standard row as an accepted model.
• The Frontier-only rule that removed the Standard tier landed at 2026-07-24T02:03:45Z. Every PR in this batch was opened before that — the latest at 2026-07-23T22:09:07Z.
• That same policy stated the tier "affects PR processing priority, not which gates apply." So the declaration was not an admission credential and nothing was bypassed.

So the model use was compliant with the rules as published at the time, and I applied a rule retroactively that did not exist when you wrote the code. The grandfather clause now in docs/AI-CONTRIBUTOR.md §0.1.1 says exactly this, and I wrote it hours before acting against it here.

I also want to separate two things I ran together in the closure comment. The behavioural concerns I raised — reverting a fix to push a diagnostic commit so CI prints a value, and removing a passing assertion under a commit message that did not mention it — are real observations and I stand behind raising them. They are review findings about specific changes. They were not grounds for a batch closure of unrelated PRs, and treating them as evidence of bad faith rather than as things to fix in review was the error.

What happens now: this PR is reopened at its original head with its review history intact. It gets processed on its merits like any other. Where it already had an approval, that approval stands and it goes back toward the queue; where it had open findings, those findings are still open and unchanged.

Two things worth knowing going forward:

1. The model floor has changed and is now stricter — per-vendor and stated by exact model: Anthropic claude-opus-4-8+ / claude-sonnet-5+, OpenAI gpt-5-5+, Codex codex-5-5+. Haiku, Sonnet 4.6, and Composer are out. This applies to PRs opened from 2026-07-24 onward, not to this one.
2. A local toolchain would help both of us. Several of the rounds on these PRs were spent discovering things a local cargo fmt / clippy / cargo test -p engine would have surfaced before pushing. That is the single highest-leverage change available here, and it removes the pattern that concerned me in the first place.

Thanks for the work, and sorry for the disruption — you had a PR approved and in the merge queue when I closed it.