fix(parser): swallow audit — typed, KIND-SPECIFIC duration evidence (CR 611.2a)#5685
Conversation
…CR 611.2a)
`Duration` is a BOUNDED fact — its parse-time carriers are a small closed set —
so it is hand-typed over an enumerated carrier list rather than probed. Three
vacuities died with the old code:
1. `static_has_duration(s) { let _ = s; true }` — returned `true` for ANY static
ability whatsoever (a keyword grant, an anthem, a cost modifier).
`StaticDefinition` has no `Duration` field at all; a static's duration lives on
the `Effect::GenericEffect` that wraps it. The stub was not a conservative
approximation of a duration — it was unrelated to one.
2. `any_ability_has_duration` (its only caller, and the sole evidence gate on
`detect_duration_until_eot`) accepted a duration of ANY KIND, so a `Permanent`
grant discharged an "until end of turn" expectation.
3. `detect_duration_next_turn`'s markers were `["YourNextTurn", "NextTurn",
"UntilYourNextTurn"]`. `NextTurn` is a MEASURED SUBSTRING COLLISION. Counting
`[A-Za-z]*NextTurn[A-Za-z]*` over the full 35,396-face export:
UntilNextTurnOf 216 | UntilEndOfNextTurnOf 182 <- real durations
SkipNextTurn 10 | ControlNextTurn 8 <- NOT durations
So an "until your next turn" expectation was dischargeable by
`Effect::SkipNextTurn` / `Effect::ControlNextTurn` — facts about extra/skipped
turns, not about how long a continuous effect lasts. Same defect as APNAP's
`player_scope`, arriving by substring instead of by wrong field.
Evidence is now the kind the expectation asked about: `unit_has_duration(parsed,
&duration_is_end_of_turn)` / `&duration_is_next_turn`. The walk reaches every
container (Token / GenericEffect statics, GrantTrigger, CreateEmblem, FlipCoin,
ChooseOneOf, CreateDelayedTrigger, Vote, SeparateIntoPiles, RevealFromHand,
AddTargetReplacement), which is what the `json_has_any` fallback existed for —
the old structured walk could not descend into `Effect::Token`.
THE CARRIER LIST IS THE CORRECTNESS ARGUMENT, so the population is pinned:
rg ':\s*[^,/]*\bDuration\b' crates/engine/src/types/{ability,statics,triggers,replacements,mana}.rs
10 parse-time carriers: AbilityDefinition.duration; Effect::{BecomeCopy,
GainActivatedAbilitiesOfTarget, GenericEffect, CastFromZone, ForceAttack}.duration;
Effect::PreventDamage.prevention_duration; CastingPermission::{ExileWithAltCost,
PlayFromExile}.duration; ManaSpellGrant::AddKeywordUntilEndOfTurn.duration. Plus
the duration-EQUIVALENT `Effect::Mana.expiry` (`ManaExpiry`, CR 106.4), and the
structural CR 614.6 / CR 514.2 lifetime of a one-shot `ReplacementEvent::DamageDone`
shield (which has no duration field to carry it).
THAT GREP IS THE SECOND ONE, and the first one's failure is the lesson. It read
`:\s*(Option<)?(Box<)?Duration\b` over ability/statics/triggers/replacements and
MISSED `ManaSpellGrant` — boxed, fully qualified, and in mana.rs. The full-pool
delta caught it as a false-positive cluster, and deleting `any_ability_has_duration`
wholesale also dropped its `DamageDone` leg, caught the same way. A carrier list is
a CLAIM ABOUT A POPULATION; state the population, then measure the claim.
MEASURED, full pool (35,396 faces; pool sha 2d168167…), vs the previous commit:
Duration_UntilEndOfTurn 9 -> 25 (+16, 15 distinct faces)
Duration_NextTurn 1 -> 2 (+1 — cone of cold)
every other detector +0
LEDGER B (losses) 0 — no warning disappeared
export minus parse_warnings 0 faces changed (byte-identical)
Iteration, stated rather than hidden: +23 -> +19 (ManaSpellGrant carrier restored)
-> +16 (DamageDone lifetime restored). Both regressions were MINE, and both were
found by the full-pool delta, not by review.
16,237 engine lib tests pass. Refs: Plan 02 / U2 commit 3 (Duration leg; the
DynamicQty / Condition_* detectors still ride `ast_json`).
There was a problem hiding this comment.
Code Review
This pull request refactors the duration-swallowing detectors (detect_duration_until_eot and detect_duration_next_turn) in swallow_check.rs to use precise, typed AST traversal instead of fragile JSON substring matching or overly broad blanket checks. It replaces the old any_ability_has_duration and static_has_duration helpers with structured checks (unit_has_duration) parameterized by specific duration predicates (duration_is_end_of_turn, duration_is_next_turn) and duration-equivalents like mana expiry. No review comments were provided, and the changes are highly idiomatic, aligning perfectly with the repository's architectural rules for strict rule fidelity and robust type-safe validation; therefore, 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.
Parse changes introduced by this PR✓ No card-parse changes detected. |
Durationis a BOUNDED fact — its parse-time carriers are a small closed set —so it is hand-typed over an enumerated carrier list rather than probed. Three
vacuities died with the old code:
static_has_duration(s) { let _ = s; true }— returnedtruefor ANY staticability whatsoever (a keyword grant, an anthem, a cost modifier).
StaticDefinitionhas noDurationfield at all; a static's duration lives onthe
Effect::GenericEffectthat wraps it. The stub was not a conservativeapproximation of a duration — it was unrelated to one.
any_ability_has_duration(its only caller, and the sole evidence gate ondetect_duration_until_eot) accepted a duration of ANY KIND, so aPermanentgrant discharged an "until end of turn" expectation.
detect_duration_next_turn's markers were["YourNextTurn", "NextTurn", "UntilYourNextTurn"].NextTurnis a MEASURED SUBSTRING COLLISION. Counting[A-Za-z]*NextTurn[A-Za-z]*over the full 35,396-face export:So an "until your next turn" expectation was dischargeable by
Effect::SkipNextTurn/Effect::ControlNextTurn— facts about extra/skippedturns, not about how long a continuous effect lasts. Same defect as APNAP's
player_scope, arriving by substring instead of by wrong field.Evidence is now the kind the expectation asked about:
unit_has_duration(parsed, &duration_is_end_of_turn)/&duration_is_next_turn. The walk reaches everycontainer (Token / GenericEffect statics, GrantTrigger, CreateEmblem, FlipCoin,
ChooseOneOf, CreateDelayedTrigger, Vote, SeparateIntoPiles, RevealFromHand,
AddTargetReplacement), which is what the
json_has_anyfallback existed for —the old structured walk could not descend into
Effect::Token.THE CARRIER LIST IS THE CORRECTNESS ARGUMENT, so the population is pinned:
10 parse-time carriers: AbilityDefinition.duration; Effect::{BecomeCopy,
GainActivatedAbilitiesOfTarget, GenericEffect, CastFromZone, ForceAttack}.duration;
Effect::PreventDamage.prevention_duration; CastingPermission::{ExileWithAltCost,
PlayFromExile}.duration; ManaSpellGrant::AddKeywordUntilEndOfTurn.duration. Plus
the duration-EQUIVALENT
Effect::Mana.expiry(ManaExpiry, CR 106.4), and thestructural CR 614.6 / CR 514.2 lifetime of a one-shot
ReplacementEvent::DamageDoneshield (which has no duration field to carry it).
THAT GREP IS THE SECOND ONE, and the first one's failure is the lesson. It read
:\s*(Option<)?(Box<)?Duration\bover ability/statics/triggers/replacements andMISSED
ManaSpellGrant— boxed, fully qualified, and in mana.rs. The full-pooldelta caught it as a false-positive cluster, and deleting
any_ability_has_durationwholesale also dropped its
DamageDoneleg, caught the same way. A carrier list isa CLAIM ABOUT A POPULATION; state the population, then measure the claim.
MEASURED, full pool (35,396 faces; pool sha 2d168167…), vs the previous commit:
Duration_UntilEndOfTurn 9 -> 25 (+16, 15 distinct faces)
Duration_NextTurn 1 -> 2 (+1 — cone of cold)
every other detector +0
LEDGER B (losses) 0 — no warning disappeared
export minus parse_warnings 0 faces changed (byte-identical)
Iteration, stated rather than hidden: +23 -> +19 (ManaSpellGrant carrier restored)
-> +16 (DamageDone lifetime restored). Both regressions were MINE, and both were
found by the full-pool delta, not by review.
16,237 engine lib tests pass. Refs: Plan 02 / U2 commit 3 (Duration leg; the
DynamicQty / Condition_* detectors still ride
ast_json).