refactor(parser): reify CopyChosenHost relation synthesis - #6832
Merged
matthewevans merged 2 commits intoJul 31, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe Oracle parser adds relation-synthesis IR for ChangesOracle relation synthesis
AI retry test determinism
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OracleDocBuilder
participant CopyChosenHost
participant ChoosePermanent
OracleDocBuilder->>CopyChosenHost: finalize chooser/static relation
CopyChosenHost->>ChoosePermanent: lower filter and description
ChoosePermanent->>OracleDocBuilder: add Moved replacement and printed slot
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
matthewevans
enabled auto-merge
July 31, 2026 04:24
Parse changes introduced by this PR✓ No card-parse changes detected. |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 31, 2026
matthewevans
enabled auto-merge
July 31, 2026 06:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_tests.rs`:
- Around line 269-273: Update the assertion message in the retained_slots test
to describe that the value 2 is the copy ability’s own source_ability_index,
rather than claiming the synthesized chooser occupies printed slot 0. Keep the
assertion and expected value unchanged.
In `@crates/engine/src/parser/oracle.rs`:
- Around line 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.
🪄 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: 652a119a-ca52-40e1-ae65-7e22fefff0cf
📒 Files selected for processing (7)
client/src/game/controllers/__tests__/aiController.test.tscrates/engine/src/parser/oracle.rscrates/engine/src/parser/oracle_ir/doc.rscrates/engine/src/parser/oracle_ir/feature.rscrates/engine/src/parser/oracle_ir/relation.rscrates/engine/src/parser/oracle_tests.rsscripts/prelowered-ratchet.txt
Comment on lines
+269
to
+273
| assert_eq!( | ||
| retained_slots, | ||
| vec![2], | ||
| "the synthesized chooser is absent from result.abilities but still occupies printed slot 0" | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the assertion message: it describes a different fact than the assertion checks.
retained_slots holds the copy ability's own source_ability_index, which the assertion pins at 2. The message states the synthesized chooser "still occupies printed slot 0". The assertion does not check slot 0 for the chooser. State what the value means, so a future reader does not treat 2 as proof of the chooser's slot.
💚 Proposed message fix
assert_eq!(
retained_slots,
vec![2],
- "the synthesized chooser is absent from result.abilities but still occupies printed slot 0"
+ "the copy ability's retained index counts source-order printed slots, so the synthesized chooser at slot 0 still shifts it to 2"
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| assert_eq!( | |
| retained_slots, | |
| vec![2], | |
| "the synthesized chooser is absent from result.abilities but still occupies printed slot 0" | |
| ); | |
| assert_eq!( | |
| retained_slots, | |
| vec![2], | |
| "the copy ability's retained index counts source-order printed slots, so the synthesized chooser at slot 0 still shifts it to 2" | |
| ); |
🤖 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_tests.rs` around lines 269 - 273, Update the
assertion message in the retained_slots test to describe that the value 2 is the
copy ability’s own source_ability_index, rather than claiming the synthesized
chooser occupies printed slot 0. Keep the assertion and expected value
unchanged.
Comment on lines
+3039
to
+3043
| // 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; |
Contributor
There was a problem hiding this comment.
🗄️ 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 functionRepository: 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.rsRepository: 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 320Repository: 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.rsRepository: 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.
matthewevans
deleted the
ship/refactor-parser-reify-copychosenhost-relation-synthesis
branch
July 31, 2026 07:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Tests