Skip to content

ship/t85b set extractor#5739

Merged
matthewevans merged 2 commits into
mainfrom
ship/t85b-set-extractor
Jul 13, 2026
Merged

ship/t85b set extractor#5739
matthewevans merged 2 commits into
mainfrom
ship/t85b-set-extractor

Conversation

@matthewevans

Copy link
Copy Markdown
Member
  • fix(engine): reduce a batched set anaphor over the WHOLE trigger batch (CR 508.1)
  • fix(parser): bind the batch anaphor only where the trigger event carries its subjects (CR 603.2c)

…h (CR 508.1)

"their total power" / "the greatest power among them" / "... among those
creatures" name a SET, and the set they name depends on the clause context —
which the leaf quantity combinator cannot see.

Engine: `extract_sources_from_event` is the set-valued widening of the singleton
`extract_source_from_event`. The singleton deliberately collapses a multi-attacker
`AttackersDeclared` to `None` (there is no single "the" attacker), so a
`TrackedSetAggregate { source: TriggeringBatch }` reduced through it saw an EMPTY
set — 0 — on every board with 2+ attackers. The widening returns the whole batch
and DELEGATES every other event arm back to the singleton, so the two cannot
drift and no existing singleton caller is touched. Batched dies triggers are
unaffected: they emit one ZoneChanged per creature and are still collected across
events.

Parser: t78 gated Max/Min ("the greatest ... AMONG ...") out of the batch anaphor
precisely because that fan-out resolved to 0. With the extractor in place the gate
is lifted, and the possessive form ("their total <prop>") is added as a composed
combinator.

Clause layer: the disambiguator is the SIBLING CLAUSE, one layer above the leaf.
When an EARLIER clause of the same chain publishes a tracked set, the anaphor
re-anchors to it (CR 608.2c — instructions resolve in order, so the pronoun refers
to what the preceding instruction just established). Kylox, Visionary Inventor's
"their" = the creatures its own sacrifice clause just sacrificed; Witch-king, Sky
Scourge's = the attacking Wraiths. The scan is STRICTLY PRIOR, so Witch-king's own
ExileTop (itself a publisher) does not capture its own anaphor.

Also:
- `effect_references_tracked_set` gains an `ExileTop` arm. Without it Kylox's
  sacrifice never published its victims (`next_sub_needs_tracked_set` was false)
  and the chain-set aggregate reduced an empty set to a confident 0.
- A `TriggeringBatch` anaphor in a chain with NO trigger event has no antecedent
  at all, so it now fails honestly via `Effect::unimplemented` instead of
  resolving to 0 (Angrath, Minotaur Pirate's loyalty ability).

Faces: Kylox (ChainSet), Witch-king Sky Scourge / Aloy / Shriekwood Devourer
(TriggeringBatch), plus Vulpine Harvester's silently-dropped MV<=power condition.
The Skullspore Nexus (dies batch, no prior publisher) is byte-identical.

Evidence: mutual watched reds (Kylox RED under TriggeringBatch, Witch-king RED
under ChainSet, both permanently pinned); red-first 2+ attacker runtime witness
(powers 2+3 -> 5 cards exiled; 0 under the singleton path, watched).
…ies its subjects (CR 603.2c)

Caught by the whole-face full-pool ledger, which is what it is for.

`TrackedSetAggregate { source: TriggeringBatch }` reduces the objects of the
current trigger event, read back through `extract_sources_from_event`. Only two
event shapes actually carry their subjects: the declared-attackers batch
(CR 508.1, `AttackersDeclared`) and the per-object zone-change batch (CR 603.10a,
one `ZoneChanged` per object). Every other trigger event exposes NO object set,
so an anaphor bound to its "batch" reduces an EMPTY set to a confident 0 while the
card still renders as fully supported.

A Premonition of Your Demise is the live proof: "When you reveal one or more
nonland cards this way, this scheme deals damage equal to their total mana value"
is a `SetInMotion` scheme trigger whose event carries no revealed cards. Before
this guard the possessive-anaphor grammar took it RED -> GREEN as a silent 0 —
strictly worse than the honest gap it replaced, and exactly the defect class this
unit exists to remove.

`mode_exposes_subject_batch` is a PROVEN whitelist, not a blacklist: a blacklist
would silently admit every newly-added trigger mode as a fresh 0. Unbindable
anaphors now fail honestly through `Effect::unimplemented`.
@matthewevans
matthewevans enabled auto-merge July 13, 2026 14:56

@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 implements correct resolution for set-anaphor aggregates (such as "their total power" or "the greatest power among them") by distinguishing between the triggering batch and the chain-published set. It introduces a set-valued event extractor to handle multi-attacker batches, adds possessive batch aggregate parsing, and handles rebinding or demoting unbindable aggregates at the clause assembly layer. Feedback is provided regarding the newly added in_trigger field in EffectChainIr, which may lack proper initialization in the production parser construction path, potentially leading to compilation failures or incorrect default behavior.

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.

Comment on lines 36 to +48
pub(crate) chain_rounding: Option<RoundingMode>,
/// CR 701.21a: Actor context threaded from ParseContext (per D-07).
pub(crate) actor: Option<ControllerRef>,
/// CR 603.2c: Whether this chain is the body of a TRIGGERED ability,
/// threaded from `ParseContext::in_trigger` (mirrors `actor`).
///
/// Assembly needs it to reject an unbindable batch anaphor: a
/// `TrackedSetAggregate { source: TriggeringBatch }` names the objects of
/// the CURRENT TRIGGER EVENT, so in a chain with no trigger event (a spell,
/// or a loyalty/activated ability) the pronoun has no antecedent and would
/// silently reduce an empty set to 0. Such a chain must fail honestly
/// instead. See `assemble_effect_chain`.
pub(crate) in_trigger: bool,

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.

high

[HIGH] Omission of production initialization for in_trigger field. Evidence: crates/engine/src/parser/oracle_ir/effect_chain.rs:36-48.
Why it matters: Adding the in_trigger field to EffectChainIr without updating its production construction site in the parser means the code will either fail to compile or default to false for all parsed abilities, causing valid triggered abilities (like Witch-king or Aloy) to be incorrectly demoted as unbindable in production.
Suggested fix: Ensure that the parser copies in_trigger: ctx.in_trigger from ParseContext when constructing EffectChainIr in production code.

@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 · 8 card(s), 10 signature(s) (baseline: main 3b52c67a9025)

3 card(s) · ability/tracked_set_anaphor · added: tracked_set_anaphor

Examples: A Premonition of Your Demise, Angrath, Minotaur Pirate, Song of Inspiration

3 card(s) · ability/where_x_binding · removed: where_x_binding

Examples: Aloy, Savior of Meridian, Kylox, Visionary Inventor, Witch-king, Sky Scourge

2 card(s) · ability/ExileTop · added: ExileTop (count=total power of those cards, player=controller)

Examples: Kylox, Visionary Inventor, Witch-king, Sky Scourge

1 card(s) · ability/ChangeZone · field conditional: recipient's mana value ≤ total power of those cards

Examples: Vulpine Harvester

1 card(s) · ability/Discover · added: Discover (mv limit=Ref { qty: TrackedSetAggregate { function: Max, property: Power, source: TriggeringBatch } }, player=Controller)

Examples: Aloy, Savior of Meridian

1 card(s) · ability/Untap · added: Untap (target=land, targets=0-max power of those cards)

Examples: Shriekwood Devourer

1 card(s) · ability/deal · removed: deal

Examples: Angrath, Minotaur Pirate

1 card(s) · ability/gain · removed: gain

Examples: Song of Inspiration

1 card(s) · ability/when · removed: when

Examples: A Premonition of Your Demise

1 card(s) · ability/where_x_binding · removed: where_x_binding (targets=0-X)

Examples: Shriekwood Devourer

Merged via the queue into main with commit 8aee471 Jul 13, 2026
13 checks passed
@matthewevans
matthewevans deleted the ship/t85b-set-extractor branch July 13, 2026 15:19
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