refactor(parser): source-addressed OracleItemIr and OracleDocBuilder (unit 3a)#5504
Conversation
…(unit 3a)
Introduce a source-addressed document IR as the safe midpoint of the Oracle
parser provenance refactor. Zero lowered-output change: `data/card-data.json`
is byte-identical under a same-base A/B regen, and no snapshot outside
`oracle_ir/snapshots` moves.
- `OracleItemIr { id, source, node }` + `OracleNodeIr`, retaining the four
`PreLowered*` variants as unit-4 debt.
- `OracleDocBuilder` emits items through a single guarded path. `emit` fails
closed on fragment/precision mismatch, duplicate item position, and
overlapping sibling spans, in that order, before mutating any state.
- `PrintedAbilityIndex` / `PrintedTriggerIndex` newtypes for CR 707.9a printed
slots. Private field, no `From<usize>`, no `Default`; the sole escape hatch
`from_category_vector_len` is the exact, compiler-enforced cutover list for
unit 3b. Adding it surfaced two absolute index derivations in
`oracle_class.rs` that no grep or runtime assertion had found, because the
Class path early-returns before the main dispatch loop.
- `ability_index()` is `spells_emitted.len()` rather than a parallel counter:
two values that must agree can drift, one value cannot. `take_last_spell()`
pops that emission-ordered stack, mirroring the parser's only category
`pop()`, so an earlier spell's printed slot cannot be freed.
- `SpanPrecision { Exact, WholeDocument }` so a consumer can refuse to print a
line number rather than print a coarse one. `fragment` is `Option<String>`,
`None` unless the span is exact.
- Child spans are validated by `UnitAllocator::allocate_with_span`, the only
way to mint an `OracleUnitSource`, which nested parsers actually receive.
Three review rounds found five latent CR 707.9a mis-indexing hazards, none of
them reachable by any current card and none detectable by the test suite, the
snapshots, or the 35k-card corpus.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a structured document builder (OracleDocBuilder) and source-span tracking types (OracleSourceSpan, OracleUnitSource) to transition the MTG Oracle IR parser from category-grouped ordering to a source-positioned, document-ordered model. It also replaces raw usize counters with type-safe newtypes (PrintedAbilityIndex, PrintedTriggerIndex) to ensure strict compliance with CR 707.9a for copy effects. The feedback highlights an opportunity to clean up dead code by removing the unused issued field from the UnitAllocator struct.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub(crate) struct UnitAllocator { | ||
| item: OracleItemId, | ||
| /// The owning item's span. Held here, not on `ItemSlot`, because the | ||
| /// allocator is what nested parsers actually receive (through `ParseContext`) | ||
| /// — see `allocate_with_span`. | ||
| parent_span: OracleSourceSpan, | ||
| next_ordinal: u32, | ||
| issued: Vec<u32>, | ||
| } |
There was a problem hiding this comment.
The issued field in UnitAllocator is completely unused (it is initialized and appended to, but never read). Ironically, because UnitAllocator itself is marked with #[allow(dead_code)] on line 478, this dead field was hidden from the compiler's unused-field analysis—exactly the kind of silent defect warned about in the module's header comment (line 19).
Please remove issued from the struct definition, and also clean up its initialization in UnitAllocator::new (line 497) and UnitAllocator::allocate (line 540).
pub(crate) struct UnitAllocator {
item: OracleItemId,
/// The owning item's span. Held here, not on `ItemSlot`, because the
/// allocator is what nested parsers actually receive (through `ParseContext`)
/// — see `allocate_with_span`.
parent_span: OracleSourceSpan,
next_ordinal: u32,
}
Parse changes introduced by this PR✓ No card-parse changes detected. |
1 participant
Introduce a source-addressed document IR as the safe midpoint of the Oracle
parser provenance refactor. Zero lowered-output change:
data/card-data.jsonis byte-identical under a same-base A/B regen, and no snapshot outside
oracle_ir/snapshotsmoves.OracleItemIr { id, source, node }+OracleNodeIr, retaining the fourPreLowered*variants as unit-4 debt.OracleDocBuilderemits items through a single guarded path.emitfailsclosed on fragment/precision mismatch, duplicate item position, and
overlapping sibling spans, in that order, before mutating any state.
PrintedAbilityIndex/PrintedTriggerIndexnewtypes for CR 707.9a printedslots. Private field, no
From<usize>, noDefault; the sole escape hatchfrom_category_vector_lenis the exact, compiler-enforced cutover list forunit 3b. Adding it surfaced two absolute index derivations in
oracle_class.rsthat no grep or runtime assertion had found, because theClass path early-returns before the main dispatch loop.
ability_index()isspells_emitted.len()rather than a parallel counter:two values that must agree can drift, one value cannot.
take_last_spell()pops that emission-ordered stack, mirroring the parser's only category
pop(), so an earlier spell's printed slot cannot be freed.SpanPrecision { Exact, WholeDocument }so a consumer can refuse to print aline number rather than print a coarse one.
fragmentisOption<String>,Noneunless the span is exact.UnitAllocator::allocate_with_span, the onlyway to mint an
OracleUnitSource, which nested parsers actually receive.Three review rounds found five latent CR 707.9a mis-indexing hazards, none of
them reachable by any current card and none detectable by the test suite, the
snapshots, or the 35k-card corpus.