Skip to content

refactor(parser): reify CopyChosenHost relation synthesis #6832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matthewevans merged 2 commits into from
Jul 31, 2026
Merged

refactor(parser): reify CopyChosenHost relation synthesis #6832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7177b97
refactor(parser): reify CopyChosenHost relation synthesis
matthewevans Jul 31, 2026
4cf3ccd
test(client): stabilize AI controller retry timing
matthewevans Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions client/src/game/controllers/__tests__/aiController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ let storeState: {
isResolvingAll: boolean;
};
let storeSubscriber: (() => void) | null = null;
let randomSpy: ReturnType<typeof vi.spyOn>;

vi.mock("../../../stores/gameStore", () => ({
useGameStore: {
Expand Down Expand Up @@ -82,6 +83,10 @@ function deferred<T>() {

beforeEach(() => {
vi.useFakeTimers();
// Keep retry timing deterministic: `runOnce` advances a fixed 1s window,
// while production delay variance would otherwise allow a variable number
// of nested retries to fit inside that same window.
randomSpy = vi.spyOn(Math, "random").mockReturnValue(1);
dispatchAiActionProposal.mockReset();
notifyEngineLost.mockReset();
attemptStateRehydrate.mockReset();
Expand All @@ -101,6 +106,7 @@ beforeEach(() => {

afterEach(() => {
storeSubscriber = null;
randomSpy.mockRestore();
vi.useRealTimers();
});

Expand Down
172 changes: 94 additions & 78 deletions crates/engine/src/parser/oracle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ use super::oracle_ir::diagnostic::OracleDiagnostic;
use super::oracle_ir::doc::{
stamp_printed_ability_slot, stamp_printed_trigger_slot, OracleDocBuilder, OracleDocIr,
OracleItemId, OracleItemIr, OracleNodeIr, OracleSourceSpan, OracleUnitSource,
PrintedAbilityIndex, PrintedTriggerIndex, SpellPayloadIr, UnsupportedAbilityIr,
PrintedAbilityIndex, PrintedTriggerIndex, RelationSynthesisIr, SpellPayloadIr,
UnsupportedAbilityIr,
};
use super::oracle_ir::effect_chain::{
AbilityIr, AbilityRootTransform, AbilityShellIr, EffectChainIr, ResidualConditionPolicy,
Expand Down Expand Up @@ -1273,11 +1274,44 @@ fn item_static(item: &OracleItemIr) -> Option<&StaticDefinition> {
/// list, pairing producer/consumer items by `OracleItemId`. Runs at parse time;
/// both the main and Class document-construction paths converge here.
fn finalize_document_relations(mut doc: OracleDocIr, types: &[String]) -> OracleDocIr {
doc.relations
.extend(detect_document_relations(&doc.items, types));
let relations = detect_document_relations(&doc.items, types);
finalize_relation_syntheses(&mut doc, &relations);
doc.relations.extend(relations);
doc
}

/// Install relation-derived nodes onto their already-emitted source item. This
/// preserves identity, source provenance, source order, and the builder's
/// historical printed-slot accounting; the builder deliberately cannot emit a
/// relation synthesis as a fresh item.
fn finalize_relation_syntheses(doc: &mut OracleDocIr, relations: &[DocumentRelationIr]) {
for relation in relations {
let DocumentRelationIr::LinkedChoice(LinkedChoiceKind::CopyChosenHost {
chooser,
copy_static,
filter,
description,
}) = relation
else {
continue;
};
let Some(item) = doc.items.iter_mut().find(|item| item.id == *chooser) else {
continue;
};
// Fail closed if a relation producer no longer names the unsupported
// chooser form it proved during discovery. Never overwrite another IR
// kind just because its id happens to match.
if !matches!(&item.node, OracleNodeIr::Unsupported { .. }) {
continue;
}
item.node = OracleNodeIr::RelationSynthesis(RelationSynthesisIr {
filter: filter.clone(),
description: description.clone(),
copy_static: *copy_static,
});
}
}

fn detect_document_relations(items: &[OracleItemIr], types: &[String]) -> Vec<DocumentRelationIr> {
let mut relations = Vec::new();
detect_linked_choice_etb_counter(items, &mut relations);
Expand Down Expand Up @@ -1838,80 +1872,39 @@ fn detect_linked_choice_copy_chosen_host(
items: &[OracleItemIr],
relations: &mut Vec<DocumentRelationIr>,
) {
let chooser = items.iter().find(|item| {
item_ability(item).is_some_and(|def| ability_is_as_enters_choose_permanent_gap(&def))
});
let chooser = items.iter().find_map(as_enters_choose_permanent_gap_item);
let copy_static = items.iter().find(|item| {
item_static(item).is_some_and(|s| {
s.modifications
.contains(&ContinuousModification::CopyChosen)
})
});
if let (Some(chooser), Some(copy_static)) = (chooser, copy_static) {
if chooser.id != copy_static.id {
if let (Some((chooser, filter, description)), Some(copy_static)) = (chooser, copy_static) {
if chooser != copy_static.id {
relations.push(DocumentRelationIr::LinkedChoice(
LinkedChoiceKind::CopyChosenHost {
chooser: chooser.id,
chooser,
copy_static: copy_static.id,
filter,
description,
},
));
}
}
}

/// CR 607.2d + CR 707.2c + CR 614.12a: Replace the proven chooser gap ability
/// with a Moved `ChoosePermanent` replacement. Filter is re-derived from the
/// Unimplemented description so line-local parse never assigns copy-host
/// semantics without this relation.
fn apply_linked_choice_copy_chosen_host(
result: &mut ParsedAbilities,
relations: &[DocumentRelationIr],
ability_ids: &mut Vec<OracleItemId>,
replacement_ids: &mut Vec<OracleItemId>,
) {
for relation in relations {
let DocumentRelationIr::LinkedChoice(LinkedChoiceKind::CopyChosenHost { chooser, .. }) =
relation
else {
continue;
};
let Some(ability_pos) = position_of(ability_ids, *chooser) else {
continue;
};
let Some(description) = result.abilities[ability_pos]
.effect
.unimplemented_description()
.map(str::to_owned)
else {
continue;
};
let Some(filter) = filter_from_as_enters_choose_permanent_text(&description) else {
continue;
};
result.abilities.remove(ability_pos);
ability_ids.remove(ability_pos);
let execute =
AbilityDefinition::new(AbilityKind::Spell, Effect::ChoosePermanent { filter });
result.replacements.push(
ReplacementDefinition::new(ReplacementEvent::Moved)
.execute(execute)
.valid_card(TargetFilter::SelfRef)
// CR 614.1c: battlefield-entry-scoped.
.destination_zone(Zone::Battlefield)
.description(description),
);
replacement_ids.push(*chooser);
}
}

/// An Unimplemented ability whose fragment is an as-enters permanent-object
/// choice ("As … enters, choose a creature/permanent…"). Framing + Typed filter
/// must both match — same grammar as `as_enters_choose_permanent_filter`.
fn ability_is_as_enters_choose_permanent_gap(def: &AbilityDefinition) -> bool {
let Some(description) = def.effect.unimplemented_description() else {
return false;
/// Typed facts from a proven unsupported chooser source. The legacy post-fold
/// path read `Effect::Unimplemented`'s description, which
/// `lower_unsupported_node` derives from this residual's fragment (not its
/// display description), so relation synthesis preserves that exact contract.
fn as_enters_choose_permanent_gap_item(
item: &OracleItemIr,
) -> Option<(OracleItemId, TargetFilter, String)> {
let OracleNodeIr::Unsupported { unsupported, .. } = &item.node else {
return None;
};
filter_from_as_enters_choose_permanent_text(description).is_some()
let filter = filter_from_as_enters_choose_permanent_text(&unsupported.fragment)?;
Some((item.id, filter, unsupported.fragment.clone()))
}

fn filter_from_as_enters_choose_permanent_text(description: &str) -> Option<TargetFilter> {
Expand Down Expand Up @@ -3043,6 +3036,11 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
let mut trigger_ids: Vec<OracleItemId> = Vec::new();
let mut static_ids: Vec<OracleItemId> = Vec::new();
let mut replacement_ids: Vec<OracleItemId> = Vec::new();
// An already-emitted unsupported chooser can become a relation-synthesized
// replacement without entering `result.abilities`.
// Its historical printed slot still exists, so this source-order counter is
// deliberately independent of the published ability vector length.
let mut printed_ability_slot = 0usize;
Comment on lines +3039 to +3043

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate the runtime consumer of RetainPrintedAbilityFromSource and see which list source_ability_index addresses.
rg -nP -C10 'RetainPrintedAbilityFromSource' crates/engine/src --glob '!**/parser/**'
rg -nP -C6 '\bsource_ability_index\b' crates/engine/src
ast-grep outline crates/engine/src/game --items all --match 'Retain|copy|Copy' --type function

Repository: phase-rs/phase

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- oracle parser target ---'
sed -n '2980,3135p' crates/engine/src/parser/oracle.rs
printf '%s\n' '--- IR slot stamping and lowering ---'
sed -n '1230,1335p' crates/engine/src/parser/oracle_ir/doc.rs
rg -n -C8 'printed_ability_slot|RelationSynthesis|source_ability_index' crates/engine/src/parser/oracle.rs crates/engine/src/parser/oracle_ir/doc.rs crates/engine/src/parser/oracle_ir
printf '%s\n' '--- runtime ability list definitions and copy application ---'
sed -n '6200,6250p' crates/engine/src/game/layers.rs
sed -n '6975,7005p' crates/engine/src/game/layers.rs
sed -n '7218,7242p' crates/engine/src/game/layers.rs

Repository: phase-rs/phase

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- parsed abilities to object base_abilities ---'
rg -n -C8 'base_abilities\s*[:=]|ParsedAbilities|\.abilities\.clone|parsed\.abilities' crates/engine/src --glob '*.rs' | head -n 240
printf '%s\n' '--- relation-synthesis fixture and expected vectors ---'
sed -n '1,290p' crates/engine/src/parser/oracle_tests.rs
printf '%s\n' '--- lower result consumers ---'
rg -n -C10 'lower_oracle_ir|parse_warnings|base_abilities' crates/engine/src/parser crates/engine/src/game crates/engine/src/types --glob '*.rs' | head -n 320

Repository: phase-rs/phase

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- card abilities and object initialization ---'
rg -n -C5 'base_abilities|abilities:\s*parsed\.abilities|abilities:\s*self\.abilities|abilities:\s*card\.abilities' crates/engine/src --glob '*.rs' | head -n 260
printf '%s\n' '--- relation replacement lowering completion ---'
sed -n '3150,3275p' crates/engine/src/parser/oracle.rs
printf '%s\n' '--- exact copy-layer reads ---'
sed -n '6228,6245p' crates/engine/src/game/layers.rs
sed -n '7220,7240p' crates/engine/src/game/layers.rs

Repository: phase-rs/phase

Length of output: 30010

Keep source_ability_index aligned with source.base_abilities.

The copy layer reads source.base_abilities[source_ability_index]. RelationSynthesis publishes only a replacement, so it does not add an entry to base_abilities. The later copy ability therefore receives an out-of-range index. Do not advance printed_ability_slot for RelationSynthesis, or rewrite the index after lowering.

🤖 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.rs` around lines 3039 - 3043, Keep
source_ability_index aligned with source.base_abilities while processing printed
abilities: do not advance printed_ability_slot for RelationSynthesis
replacements, since they do not create base_abilities entries. Preserve the
existing slot progression for actual source abilities so later copy abilities
receive valid indices.

// CR 707.9a printed slots are resolved in this loop, not in
// `OracleDocBuilder::finish` where they used to be. The stamp rewrites the
// `placeholder()` (= 0) the dispatch loop baked into each
Expand All @@ -3054,9 +3052,9 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
// source-ordered `BTreeMap` and count each category separately, so the k-th
// spell item is at ability slot k either way.
//
// The slot IS `result.<category>.len()` at the moment of the push, which is
// what makes this the correct seam rather than merely a possible one. Stamped
// BEFORE the relation passes below, matching the pre-relation state the
// The slot counter advances for every source spell item, including a
// `RelationSynthesis` that publishes only a replacement. Stamped BEFORE the
// relation passes below, matching the pre-relation state the
// `finish()` walk saw — several of those passes insert into, remove from, and
// move ids between the category tracks.
//
Expand All @@ -3067,9 +3065,10 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
match &item.node {
OracleNodeIr::Spell(ability_ir) => {
let mut def = lower_ability_ir(ability_ir);
stamp_printed_ability_slot(&mut def, result.abilities.len());
stamp_printed_ability_slot(&mut def, printed_ability_slot);
result.abilities.push(def);
ability_ids.push(item.id);
printed_ability_slot += 1;
}
// Same three steps as the two arms around it: lower, stamp the
// CR 707.9a printed ability slot, push. The residual is stamped like
Expand All @@ -3080,9 +3079,28 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
min_x_value,
} => {
let mut def = lower_unsupported_node(unsupported, *min_x_value);
stamp_printed_ability_slot(&mut def, result.abilities.len());
stamp_printed_ability_slot(&mut def, printed_ability_slot);
result.abilities.push(def);
ability_ids.push(item.id);
printed_ability_slot += 1;
}
OracleNodeIr::RelationSynthesis(synthesis) => {
let execute = AbilityDefinition::new(
AbilityKind::Spell,
Effect::ChoosePermanent {
filter: synthesis.filter.clone(),
},
);
result.replacements.push(
ReplacementDefinition::new(ReplacementEvent::Moved)
.execute(execute)
.valid_card(TargetFilter::SelfRef)
// CR 614.1c: battlefield-entry-scoped.
.destination_zone(Zone::Battlefield)
.description(synthesis.description.clone()),
);
replacement_ids.push(item.id);
printed_ability_slot += 1;
}
OracleNodeIr::Trigger(trigger_node) => {
let mut def = lower_trigger_node_ir(trigger_node);
Expand Down Expand Up @@ -3129,9 +3147,10 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
}
OracleNodeIr::PreLoweredSpell(def) => {
let mut def = def.clone();
stamp_printed_ability_slot(&mut def, result.abilities.len());
stamp_printed_ability_slot(&mut def, printed_ability_slot);
result.abilities.push(def);
ability_ids.push(item.id);
printed_ability_slot += 1;
}
}
}
Expand Down Expand Up @@ -3166,12 +3185,6 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
);
reconcile_host_bound_phase_outs(&mut result);
apply_linked_choice_persisted_player(&mut result, &ir.relations, &ability_ids, &trigger_ids);
apply_linked_choice_copy_chosen_host(
&mut result,
&ir.relations,
&mut ability_ids,
&mut replacement_ids,
);

// Architectural rule: the parser must never silently discard Oracle text. Run
// the swallow audit against the parsed result so any unrepresented clause
Expand All @@ -3189,11 +3202,9 @@ pub(crate) fn lower_oracle_ir(ir: &mut OracleDocIr) -> ParsedAbilities {
//
// The tracks are sound to zip here: of the relation passes above,
// `apply_linked_choice_etb_counter` removes from `result.replacements` and
// `replacement_ids` at the same index, and `apply_linked_choice_copy_chosen_host`
// moves an ability id onto the replacement track. This is also exactly why
// the audit stays HERE, post-relation: a pre-lowering audit is blind to
// relation-synthesized semantics (that pass *synthesizes a replacement*), so
// the false-positive wave U1 bounded to 31 faces would be caused, not avoided.
// `replacement_ids` at the same index. Relation synthesis already populated
// the replacement track during the source-order fold, which is why the audit
// stays HERE: a pre-lowering audit is blind to that semantic output.
//
// Emitted into a local vec and appended, rather than passing `&mut
// ir.diagnostics` directly: the audit reads `ir.items` and writes the
Expand Down Expand Up @@ -3897,6 +3908,11 @@ impl<'a> DocEmitter<'a> {
match node {
OracleNodeIr::Static(ir) => self.static_ir_at(item_line, ir),
OracleNodeIr::Trigger(ir) => self.trigger_ir_at(item_line, ir),
OracleNodeIr::RelationSynthesis(_) => {
panic!(
"relation synthesis is finalization-only and cannot be forwarded by DocEmitter"
);
}
other => {
self.emit_at(item_line, other);
}
Expand Down
Loading
Loading