Skip to content

fix(parser): honest coverage for 'double each kind of counter' class (CR 701.10e)#6068

Merged
matthewevans merged 3 commits into
phase-rs:mainfrom
lgray:feat/std-dynqty-b-double-counters
Jul 17, 2026
Merged

fix(parser): honest coverage for 'double each kind of counter' class (CR 701.10e)#6068
matthewevans merged 3 commits into
phase-rs:mainfrom
lgray:feat/std-dynqty-b-double-counters

Conversation

@lgray

@lgray lgray commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Summary

Two parser-layer fixes for the "double the number of each kind of counter" class (CR 701.10e). FIX 1 teaches the DynamicQty swallow detector that Effect::Double{ DoubleTarget::Counters } is the intrinsic dynamic carrier for this phrase (it previously recognized only Effect::MultiplyCounter), clearing a false-positive SwallowedClause that kept 7 correctly-parsed targeted cards red. §5 gates the non-targeted mass form of the phrase to an honest Effect::unimplemented, because Effect::Double is targeted-only (no battlefield-enumeration path) and would otherwise double nothing at runtime — keeping Ultimate Spider-Man honestly red rather than a silent false-green.

Implementation method (required)

Method: /engine-implementer

CR references

  • CR 701.10e — doubling a kind of counter gives as many more as already present (amount is intrinsic to the resolver, never a QuantityExpr carrier).
  • CR 115.1 — the "target" keyword distinguishes a chosen target (TargetKeyword) from a mass descriptor.
  • CR 608.2k — untargeted anaphor ("on it" / "that creature") binds to the cost/trigger object.

Verification

  • Required checks ran clean, or the exact CI-owned alternative is stated below.

  • Gate A output below is for the current committed head.

  • Final review-impl below is clean for the current committed head.

  • Both anchors cite existing analogous code at the same seam.

  • cargo fmt --all — clean

  • cargo clippy -p engine --lib -- -D warnings — exit 0, no warnings

  • cargo test -p engine --lib (counter + swallow_check modules) — 196 passed, 0 failed, 2 ignored; 6 new tests (5 original + 1 singular-descriptor hardening)

  • scripts/check-parser-combinators.sh — exit 0

  • Coverage isolation (warm oracle-gen --filter regen vs merge-base card-data.json; full-corpus coverage_parse_diff + semantic-audit run in CI):

    • 7 targeted cards flip False→True ([DynamicQty][], correct Double{Counters}): Arcade Cabinet, Deepglow Skate, Ferrafor Young Yew, Gilder Bairn, The First Tyrannic War, Vorel of the Hull Clade, Zimone Paradox Sculptor.
    • Ultimate Spider-Man[DynamicQty][] + Effect::Unimplemented("double_counters_nontargeted") → stays honestly red, does NOT flip (non-targeted mass; runtime deferred to the DoubleCountersAll epic).
    • 0 off-class drift — all 26 Remove/Move/Multiply-counter cards (the complete non-double resolve_counter_target caller set) byte-identical abilities+triggers vs base.
    • Revert-probe: all 6 discriminating tests flip to FAILED under their targeted revert, restored.

Gate A

Gate A PASS head=30585b9a7040dfa9ecb894a21335196e00471663 base=be22e078bb1b05661d1ce34a21f3fbc07a136cab

Anchored on

  • crates/engine/src/parser/swallow_check.rs (detect_dynamic_qty MultiplyCounter escape hatch) — analogous intrinsic-dynamic-carrier suppression extended to Double{Counters}.
  • crates/engine/src/parser/oracle_effect/counter.rs:976 (resolve_counter_target anaphor precedence) — analogous SelfRef/it-pronoun/TriggeringSource guard order reused by classify_counter_target.

Final review-impl

Final review-impl PASS head=30585b9a7040dfa9ecb894a21335196e00471663

Head history: the review-impl loop ran clean on 03b6dce. A maintainer then merged current main (incl. the already-merged dq-a PR #6066) into this branch via merge commit 3023f44, preserving this PR's parser logic byte-for-byte; a formatting-only rustfmt reflow of the swallow_check import block (30585b9) was applied on top. No parser logic changed after the reviewed head — Gate A above is re-run on the current head.

Claimed parse impact

  • Arcade Cabinet, Deepglow Skate, Ferrafor Young Yew, Gilder Bairn, The First Tyrannic War, Vorel of the Hull Clade, Zimone Paradox Sculptor — false-positive SwallowedClause cleared (now honestly supported).
  • Ultimate Spider-Man — non-targeted mass form now honestly Unimplemented (was a silent false-green: parsed Double{Counters} that doubles nothing at runtime). Full support deferred (see below).

Deferred follow-up (documented; not yet filed)

Making Ultimate Spider-Man (and any non-targeted mass "double each kind of counter on each X") an honest green requires an engine mass-runtime that does not exist yet:

  • Effect::DoubleCountersAll { target, counter_type } — a mass sibling of the targeted Effect::Double, mirroring PutCounterAll/DoublePTAll: registered in mass_all_target_filter, resolver enumerates the battlefield via matches_target_filter with CR 701.10e per-permanent dedupe (a permanent that is both a Spider and legendary is doubled once), plus the ~17 exhaustive Effect match sites across the 5 crates. Full /add-engine-effect + /add-engine-variant lifecycle.
  • General "each A and B you control" compound-population grammar — the supertype-led disjunct parser (starts_with_type_word supertype branch) that makes "each Spider and legendary creature you control" assemble the correct Or, travelling with the runtime above so USM's grammar + runtime land as one tested unit.

This PR intentionally keeps that scope out and USM honestly red, per the honesty gate.

…(CR 701.10e)

Teach the DynamicQty swallow detector that Effect::Double{Counters} is the
intrinsic dynamic carrier for 'double the number of each kind of counter'
(it previously recognized only Effect::MultiplyCounter), clearing a
false-positive SwallowedClause on 7 correctly-parsed targeted cards
(Arcade Cabinet, Deepglow Skate, Ferrafor Young Yew, Gilder Bairn,
The First Tyrannic War, Vorel of the Hull Clade, Zimone Paradox Sculptor).

Gate the non-targeted MASS form to an honest Effect::unimplemented: Effect::Double
is targeted-only (no battlefield-enumeration path; absent from mass_all_target_filter),
so a mass population would double nothing at runtime. classify_counter_target keys
on TargetSyntax::Descriptor after the anaphor guards; resolve_counter_target is a
thin wrapper (byte-equivalent for its remove/move/multiply callers). Keeps
Ultimate Spider-Man honestly red instead of a silent false-green pending the
DoubleCountersAll mass-runtime epic.

Assisted-by: ClaudeCode:claude-opus-4.8
@lgray
lgray requested a review from matthewevans as a code owner July 17, 2026 03:29
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR · 1 card(s), 2 signature(s) (baseline: main 5f95a06f72b9)

🟢 Added (1 signature)

  • 1 card · ➕ ability/double_counters_nontargeted · added: double_counters_nontargeted
    • Affected (first 3): Ultimate Spider-Man

🔴 Removed (1 signature)

  • 1 card · ➖ ability/Double · removed: Double (doubles=Counters { counter_type: None }, target=Spider)
    • Affected (first 3): Ultimate Spider-Man

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

@matthewevans matthewevans self-assigned this Jul 17, 2026
# Conflicts:
#	crates/engine/src/parser/swallow_check.rs
@matthewevans matthewevans removed their assignment Jul 17, 2026
…resolution

Merge 3023f44 hand-resolved the DamageModification/DoubleTarget import
conflict without running cargo fmt; reflow the use-list for the CI fmt gate.
No logic change.

Assisted-by: ClaudeCode:claude-opus-4.8
@matthewevans matthewevans self-assigned this Jul 17, 2026
@matthewevans matthewevans added the bug Bug fix label Jul 17, 2026
@matthewevans matthewevans removed their assignment Jul 17, 2026
@lgray

lgray commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

@matthewevans#6068 is green + CLEAN and ready to enqueue whenever you have a moment. This is the next PR in the DynQty parser series after #6066 (which you merged earlier).

  • HEAD 30585b9a7 (includes your merge 3023f44d7 + the follow-up fmt fix)
  • mergeStateStatus: CLEAN, mergeable: MERGEABLE
  • All required checks SUCCESS on this head: Rust lint (fmt/clippy/parser-gate), Rust tests shard 1&2, coverage-gate, Card data (generate/validate/coverage), WASM, Tauri, Lobby, Frontend, Superagent Security.

No action needed from your side beyond enqueue — I don't have merge-queue permission on the repo, so I'm handing the trigger to you. Thanks!

@matthewevans matthewevans self-assigned this Jul 17, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after current-head review: targeted counter doubling remains on the existing resolver, while the untargeted mass form is explicitly Unimplemented so coverage stays honest.

@matthewevans
matthewevans added this pull request to the merge queue Jul 17, 2026
@matthewevans matthewevans removed their assignment Jul 17, 2026
Merged via the queue into phase-rs:main with commit 2f46080 Jul 17, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants