refactor(parser): Plan 05b T10c — the die-result branch table as IR (U0-45)

[matthewevans]

Plan 05b T10c — converts U0-45, the CR 706 die-roll table producer, to real IR. Follows #6733.

This is the only die row that owns its whole definition. The other five mutate a definition someone else already lowered and are a strictly harder seam — they stay out. T10c ships the type; T10d spends it.

#	commit	producers
1	DieResultBranchIr + AbilityIr.die_results	zero
2	convert try_parse_die_roll_table	1
3	pin the new step into the documented lowering order	—

The branch bodies are R1 — the identity is the function body

Each row's effect was parse_effect_chain(effect_text, kind). That call is lower_ability_ir(&parse_ability_ir_standalone(effect_text, kind)) — the entire body of parse_effect_chain, not a claim about it. Splitting it moves where lowering happens, never what it produces.

Box<AbilityIr> rather than Box<EffectChainIr> is load-bearing, not style: lower_effect_chain_ir skips finalize_effect_chain and the owner-library anchor, both of which parse_effect_chain runs today.

CR 706.3a is the row shape; CR 706.3b decides where the table lives

706.3a … The possible results indicated could be a single number, a range of numbers with two endpoints in the form "N1–N2," or a range with a single endpoint in the form "N+." …

That is {min, max, effect} verbatim.

706.3b An instruction to roll one or more dice, any instructions to modify that roll printed in the same paragraph, any additional instructions based on the result of the roll, and the associated results table are all part of one ability.

So the table is not a sibling document item that happens to follow — carrying it inside the AbilityIr is what the rules describe, and the same sentence licenses the line-consumption (next_i) behaviour: those following lines are not independent abilities. Both rules were grep-verified and their text read, not existence-checked.

The risk that had to be measured rather than argued

Today U0-45's definition is built by AbilityDefinition::new and never passes through finalize_effect_chain or apply_owner_library_reveal_anchor_from_text. Routing it through lower_ability_ir subjects it to both for the first time, across ~113 die cards.

Both were expected inert — a one-clause chain has no continuations to finalize, and the anchor is text-driven off source_text, which here is a "Roll a dN…" header with no owner-library reveal phrasing. But that is an R2 argument about today's corpus, not an identity. This campaign's one landed defect (#6123) passed review precisely because its byte-identity claim was true while its reach argument was untested. So it was measured.

FULL-POOL BYTE-IDENTICAL
2737d0783ece3443cb0041323114ee06eaa06f8e30431f817784e4dd5312f289   6cedb28c44  base
2737d0783ece3443cb0041323114ee06eaa06f8e30431f817784e4dd5312f289   commit 1
2737d0783ece3443cb0041323114ee06eaa06f8e30431f817784e4dd5312f289   commit 2

Neither mechanism fired. Three fresh generator builds, distinct target dirs, MTGJSON_SKIP_REFRESH=1, real AtomicCards.json (158,014,511 bytes — resolved and size-checked, since the fixture population would be void evidence rather than a green).

Stated rather than buried: the measured revisions differ from the shipped ones by the ten accepted .snap files and commit 3's doc comment. That was verified mechanically, not asserted — every differing line is either a .snap fixture or a /// line, so no generator input or code path differs and the hashes carry to the tip.

Commit 1 is inert by construction

die_results defaults to empty and the applier is guarded on non-empty, so every pre-existing producer lowers exactly as before — the same defer-on-default property that made the A0 shell widening byte-identical. The existing find_terminal_roll_die walk is reused, not duplicated; only its visibility changed.

Ten *_ir.snap fixtures moved, all the identical schema addition ("die_results": []) and nothing else. The _ir/_lowered masking trap was handled correctly: the ten _ir assertions were accepted, then the snapshot module was re-run to confirm the _lowered assertions that follow them still pass. Zero *.snap.new pending.

Commit 3 — the ordering doc had gone stale, and it is the load-bearing kind

lower_ability_ir's doc declares itself the single authority on a sequence that is "pinned and behavior-load-bearing", ending "Do not reorder for elegance." Commit 1 inserted a step into that sequence without it appearing there.

Attaching die results before finalize_effect_chain is a real decision: finalize and the anchor then see the complete effect, exactly as they would for an ability whose table was printed inline. Attaching later would hand those two a RollDie with an empty results vector and then mutate the result behind them. That reasoning now lives in the doc that claims to own the order.

Gates

cargo fmt --all                                 clean
clippy -p engine --lib -- -D warnings           zero warnings
cargo nextest run -p engine --lib               17909 passed, 6 skipped
cargo nextest run -p engine --test integration   4159 passed, 2 skipped
Gate P (PreLowered ratchet)                     PASS — no movement expected; this row's
                                                producer was never a PreLowered token
check-parser-combinators.sh                     Gate G PASS / Gate A PASS
check-skill-doc.sh                              PASS
*.snap.new pending                              0

Scope held

The five attach_die_result_branches_to_chain call sites are untouched — they mutate an already-lowered definition and return an advanced line index, which is a different and harder problem. They are U0-28 (T10b) and U0-30/32/49 (T10d), and T10d inherits DieResultBranchIr from here.

One behaviour worth naming for a reviewer: the applier silently no-ops if the walk finds no terminal RollDie. That mirrors the existing attach_die_result_branches_to_chain, which returns early on the same condition — house convention rather than a new failure mode, and unreachable from the single producer, which always builds a RollDie root.

Summary by CodeRabbit

• New Features

  • Added support for parsing and displaying die-roll result tables with range-based outcomes.
  • Die-roll outcomes can now include nested effects and abilities.

• Bug Fixes

  • Improved handling of activated abilities and die-roll results to ensure outcomes are applied consistently.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 04f0c4fa-93e0-4305-bddb-40b11a83efd2

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0ecd1 and f29849d.

⛔ Files ignored due to path filters (10)

• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__arni_brokenbrow_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__blunt_the_assault_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__boseiju_who_endures_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__case_of_the_stashed_skeleton_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__fog_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__ghost_lit_stalker_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__govern_the_guildless_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__guul_draz_assassin_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__joraga_treespeaker_ir.snap is excluded by !**/*.snap, !**/snapshots/**
• crates/engine/src/parser/oracle_ir/snapshots/engine__parser__oracle_ir__snapshot_tests__liliana_the_repentant_ir.snap is excluded by !**/*.snap, !**/snapshots/**

📒 Files selected for processing (4)

• crates/engine/src/parser/oracle_class.rs
• crates/engine/src/parser/oracle_effect/mod.rs
• crates/engine/src/parser/oracle_ir/effect_chain.rs
• crates/engine/src/parser/oracle_special.rs

---

📝 Walkthrough

Walkthrough

Die-roll table parsing now stores result branches in AbilityIr. lower_ability_ir recursively lowers those branches and attaches them to the terminal RollDie. Other AbilityIr constructors explicitly initialize empty die-result metadata.

Changes

Die-roll result IR integration

Layer / File(s)	Summary
Die-result IR contract crates/engine/src/parser/oracle_ir/effect_chain.rs	Adds DieResultBranchIr and the AbilityIr.die_results field for representing die-roll result ranges and their effects.
Die-roll table IR construction crates/engine/src/parser/oracle_special.rs	Refactors die-roll table parsing to create AbilityIr branches and a containing RollDie ability, then lowers the result.
Die-result lowering and initialization crates/engine/src/parser/oracle_effect/mod.rs, crates/engine/src/parser/oracle_class.rs	Updates lower_ability_ir to attach recursively lowered branches to the terminal RollDie and initializes empty die-result vectors in other IR constructors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant OracleSpecialParser
  participant AbilityIr
  participant LowerAbilityIr
  participant RollDieEffect
  OracleSpecialParser->>AbilityIr: Build roll instruction and result branches
  AbilityIr->>LowerAbilityIr: Pass parsed ability IR
  LowerAbilityIr->>RollDieEffect: Attach lowered result branches

Loading

Possibly related PRs

• phase-rs/phase#6714: Shares the oracle effect lowering and effect-chain IR pipeline.
• phase-rs/phase#6720: Uses the same AbilityIr lowering path in oracle_class.rs.
• phase-rs/phase#6724: Extends the refactored AbilityIr pipeline used by this die-result lowering.

Suggested labels: quality

Suggested reviewers: jsdevninja

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: converting the die-result branch table to IR, even though it includes extra plan/task labels.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch wt/p05b-t10c

---

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

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.