Skip to content

fix(parser): stop the damage scan swallowing the word "excess" (CR 120.10)#5733

Merged
matthewevans merged 1 commit into
mainfrom
ship/t85-excess-channel-scan
Jul 13, 2026
Merged

fix(parser): stop the damage scan swallowing the word "excess" (CR 120.10)#5733
matthewevans merged 1 commit into
mainfrom
ship/t85-excess-channel-scan

Conversation

@matthewevans

Copy link
Copy Markdown
Member

parse_damage_dealt_phrase recognized the " this way" family with a
single unanchored scan:

let (input, _) = opt(take_until("damage dealt")).parse(input)?;
let (input, _) = tag("damage dealt").parse(input)?;

take_until("damage dealt") DISCARDS everything before the verb phrase —
including the word "excess". So "the amount of excess damage dealt this way"
matched the generic damage arm, and the caller stamped
PreviousEffectAmount { channel: Total } on it. An EXCESS phrase silently read
the TOTAL tally.

That is not a coverage gap. It is a live, shipped, wrong number, and it rendered
as fully supported:

Razor Rings 4 damage into a 1/1 is 3 excess (CR 120.10). "You gain life
equal to the excess damage dealt this way" gained 4.
Nahiri's Warcrafting "Look at the top X cards ... where X is the excess damage
dealt this way" looked at the full damage, not the overkill.
Overwhelming Victory "+X/+0 ... the amount of excess damage dealt this way"
pumped by the full damage.
Cramped Vents // Access Maze, Windswift Slice, Ravenous Pursuit — same overcount.

And a TEST PINNED IT: parse_event_context_quantity_previous_effect_this_way_variants
asserted the excess phrase mapped to the Total channel. The pin protected the bug.

The excess qualifier is an independent AXIS over the same verb phrase, not part of
it. The scan now STOPS AT the qualifier instead of swallowing it: the excess
reading is tried first with a scan anchored on "excess damage dealt", and only if
that fails does the pre-existing permissive total scan run, unchanged. Every phrase
that matched before still matches — this is a channel correction, not a narrowing.
The combinator now REPORTS the channel and the caller uses it, rather than assuming
Total; assuming Total at the call site is what made the bug invisible.

The Excess channel it needs already exists and is live — QuantityRef::PreviousEffectAmount { channel } and its resolver landed in #5728. No new engine types.

DISCLOSURE: the explicit channel: Total at both the production site and the pin was
my own mechanical migration in #5728. Behaviour did not change there (the site was
already total-only), but the widening stamped an explicit Total onto a wrong reading
and made it look deliberate. A mechanical migration that faithfully preserves behaviour
also faithfully preserves BUGS — and upgrades them from implicit to intentional-looking.

FULL-POOL LEDGER (35,396 faces; base d555cef; WHOLE-FACE structural diff — a
red-count ledger CANNOT see this fix, because it changes a field inside an
already-bound quantity and clears no Unimplemented):
6 faces changed
0 gained binding
0 gained honest red
0 re-shaped unexplained
6 CHANNEL CORRECTED (Total -> Excess): Razor Rings, Nahiri's Warcrafting,
Overwhelming Victory, Cramped Vents // Access Maze, Windswift Slice,
Ravenous Pursuit
The 8 pool faces that legitimately want the TOTAL ("you gain life equal to the
damage dealt this way") are untouched — the negative control holds at pool scale.
Nahiri's Warcrafting and Overwhelming Victory were NOT in my predicted set; the
ledger found them. My oracle-grep prediction was 4/6 (I wrongly expected Unleash
the Inferno and Bottle-Cap Blast, which use a bare demonstrative and a condition
respectively and correctly did not move). The ledger is the authority, not the grep.

Tests: 3 runtime witnesses (2 positive + 1 negative control), WATCHED FAILING pre-fix
(Razor Rings read 4 where it must read 3; the "amount of" variant read 9 where it must
read 5). The negative control — the plain non-excess phrase must KEEP reading the total
— passes in both states, so a fix that simply routed every damage phrase to the excess
channel would not survive it. The wrong pin is corrected into a channel-split pair.

Co-authored-by: matthewevans matthewevans@users.noreply.github.com

…0.10)

`parse_damage_dealt_phrase` recognized the "<amount> this way" family with a
single unanchored scan:

    let (input, _) = opt(take_until("damage dealt")).parse(input)?;
    let (input, _) = tag("damage dealt").parse(input)?;

`take_until("damage dealt")` DISCARDS everything before the verb phrase —
including the word "excess". So "the amount of excess damage dealt this way"
matched the generic damage arm, and the caller stamped
`PreviousEffectAmount { channel: Total }` on it. An EXCESS phrase silently read
the TOTAL tally.

That is not a coverage gap. It is a live, shipped, wrong number, and it rendered
as fully supported:

  Razor Rings          4 damage into a 1/1 is 3 excess (CR 120.10). "You gain life
                       equal to the excess damage dealt this way" gained 4.
  Nahiri's Warcrafting "Look at the top X cards ... where X is the excess damage
                       dealt this way" looked at the full damage, not the overkill.
  Overwhelming Victory "+X/+0 ... the amount of excess damage dealt this way"
                       pumped by the full damage.
  Cramped Vents // Access Maze, Windswift Slice, Ravenous Pursuit — same overcount.

And a TEST PINNED IT: `parse_event_context_quantity_previous_effect_this_way_variants`
asserted the excess phrase mapped to the Total channel. The pin protected the bug.

The excess qualifier is an independent AXIS over the same verb phrase, not part of
it. The scan now STOPS AT the qualifier instead of swallowing it: the excess
reading is tried first with a scan anchored on "excess damage dealt", and only if
that fails does the pre-existing permissive total scan run, unchanged. Every phrase
that matched before still matches — this is a channel correction, not a narrowing.
The combinator now REPORTS the channel and the caller uses it, rather than assuming
`Total`; assuming Total at the call site is what made the bug invisible.

The Excess channel it needs already exists and is live — `QuantityRef::PreviousEffectAmount
{ channel }` and its resolver landed in #5728. No new engine types.

DISCLOSURE: the explicit `channel: Total` at both the production site and the pin was
my own mechanical migration in #5728. Behaviour did not change there (the site was
already total-only), but the widening stamped an explicit `Total` onto a wrong reading
and made it look deliberate. A mechanical migration that faithfully preserves behaviour
also faithfully preserves BUGS — and upgrades them from implicit to intentional-looking.

FULL-POOL LEDGER (35,396 faces; base d555cef; WHOLE-FACE structural diff — a
red-count ledger CANNOT see this fix, because it changes a field inside an
already-bound quantity and clears no Unimplemented):
   6  faces changed
   0  gained binding
   0  gained honest red
   0  re-shaped unexplained
   6  CHANNEL CORRECTED (Total -> Excess): Razor Rings, Nahiri's Warcrafting,
      Overwhelming Victory, Cramped Vents // Access Maze, Windswift Slice,
      Ravenous Pursuit
  The 8 pool faces that legitimately want the TOTAL ("you gain life equal to the
  damage dealt this way") are untouched — the negative control holds at pool scale.
  Nahiri's Warcrafting and Overwhelming Victory were NOT in my predicted set; the
  ledger found them. My oracle-grep prediction was 4/6 (I wrongly expected Unleash
  the Inferno and Bottle-Cap Blast, which use a bare demonstrative and a condition
  respectively and correctly did not move). The ledger is the authority, not the grep.

Tests: 3 runtime witnesses (2 positive + 1 negative control), WATCHED FAILING pre-fix
(Razor Rings read 4 where it must read 3; the "amount of" variant read 9 where it must
read 5). The negative control — the plain non-excess phrase must KEEP reading the total
— passes in both states, so a fix that simply routed every damage phrase to the excess
channel would not survive it. The wrong pin is corrected into a channel-split pair.

Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
@matthewevans
matthewevans enabled auto-merge July 13, 2026 11:33

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request fixes a bug where subject-less excess damage phrases (e.g., 'the excess damage dealt this way') were incorrectly parsed as reading the total damage channel instead of the excess damage channel. The parser in oracle_quantity.rs was updated to scan for 'excess damage dealt' before falling back to the generic 'damage dealt' scan, ensuring the correct DamageChannel is returned. Comprehensive integration and unit tests were added to verify this behavior and prevent future regressions. There are no review comments, so I have no feedback to provide.

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.

@matthewevans
matthewevans added this pull request to the merge queue Jul 13, 2026
@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main 823c29c2d8fb)

1 card(s) · ability/Dig · field count: PreviousEffectAmount { channel: Total }PreviousEffectAmount { channel: Excess }

Examples: Nahiri's Warcrafting

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

Merged via the queue into main with commit d29d19c Jul 13, 2026
13 checks passed
@matthewevans
matthewevans deleted the ship/t85-excess-channel-scan branch July 13, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant