feat(engine): S07 condition-if standard cluster — 28 cards + token-chain and as-long-as class fixes - #4990
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
matthewevans
left a comment
There was a problem hiding this comment.
Picked up current head 7ce29301 in the sweep. This touches broad engine/parser surface, so I’m holding review until the coverage-parse-diff sticky is present for this exact head.
That card-level parse diff is required evidence for parser/engine PRs before approval or enqueue. No approval/enqueue from the sweep yet.
Parse changes introduced by this PR · 71 card(s), 91 signature(s) (baseline: main
|
matthewevans
left a comment
There was a problem hiding this comment.
[MED] Canceling after a behold type choice leaves a stale creature type on the spell object. Evidence: crates/engine/src/game/casting_costs.rs:1945, crates/engine/src/game/casting_costs.rs:4255, and crates/engine/src/game/casting.rs:14203. Why it matters: the choice is written onto the card object, later cast attempts skip CostTypeChoice once any creature-type attribute exists, and handle_cancel_cast does not rewind that temporary choice, so a canceled cast can force the next cast attempt to reuse the old type. Suggested fix: keep the pre-cost choice in PendingCast/cost state, or explicitly remove the temporary CreatureType chosen attribute when the cast is canceled.
[MED] CostTypeChoice.options leaks hidden hand-derived feasibility. Evidence: crates/engine/src/game/casting_costs.rs:4276, crates/engine/src/game/filter.rs:2225, and crates/engine/src/game/casting_costs.rs:449. Why it matters: the public waiting state serializes creature-type options computed from hand cards, so opponents can infer private hand type/count information before only the selected behold cards are revealed. Suggested fix: keep serialized options public/generic and validate feasibility on submit, or add per-viewer redaction for CostTypeChoice.options.
[MED] manabrew-compat cannot prompt for the new CostTypeChoice waiting state. Evidence: crates/engine/src/game/casting_costs.rs:4276 emits WaitingFor::CostTypeChoice, but crates/manabrew-compat/src/lib.rs:730 only maps NamedChoice to chooseType, and the match falls through to UnsupportedPrompt at crates/manabrew-compat/src/lib.rs:804. Why it matters: Manabrew agents will fail to cast cards that enter this new behold type-choice step. Suggested fix: add a WaitingFor::CostTypeChoice arm mirroring NamedChoice, and include it in source_card_id using pending_cast.object_id.
[LOW] The stack UI drops the pending-cast indicator during CostTypeChoice. Evidence: client/src/adapter/types.ts:1227 defines CostTypeChoice with pending_cast, but getPendingCastObjectId only reads adjacent pending-cast states at client/src/components/stack/StackDisplay.tsx:35; that value drives isPending at client/src/components/stack/StackDisplay.tsx:330 and the “Casting” badge at client/src/components/stack/StackEntry.tsx:221. Why it matters: during this new casting-cost prompt, the frontend no longer marks the in-progress spell on the stack, unlike adjacent cost-choice states. Suggested fix: add case "CostTypeChoice": return waitingFor.data.pending_cast.object_id; to getPendingCastObjectId.
…rds) Wire leading- and trailing-if conditions gated on "this way" tracked sets / zone-change refs onto existing typed variants, plus a tight swallow_check recognizer for conditional-counters-on-put-onto-battlefield. Cards (9): Oviya (Automech Artisan), Spelunking, Town Greeter, Cache Grab, Nashi (Searcher in the Dark), Arid Archway, Break the Spell, Portent of Calamity, Transcendent Archaic. - New nom combinators for you-put-into-hand / returned-to-hand / control-or-returned / you-draw / exiled-N-or-more "this way" conditions; hoist active-voice this-way gates to fire under both `if` and `when` prefixes. - Portent: trailing-if free-cast gate via QuantityRef::TrackedSetSize GE 4 (exile-resume publishes cause-less, so the unfiltered count is correct). - Oviya: swallow_check recognizes conditional_enter_with_counters on a Hand->Battlefield ChangeZone as covering the Condition_If clause (runtime gate on moved-object type verified); trample static confirmed wired. - Zero new engine variants. All 9 flip supported:true gap_count:0. - Discriminating runtime tests (positive + negative sibling) per card; Portent GE-4 both arms; Oviya artifact->2 / non-artifact->0. - coverage-regression-check vs main: REGRESSED(engine)=0; swallowed 947->937. Assisted-by: ClaudeCode:claude-opus-4.8
…ition_If (5 cards)
Wire conditions gated on additional-cost payment, cost-object properties,
and gift-given state onto existing typed variants; add is_suspected to the
last-known-information channel so "the sacrificed creature was suspected"
resolves.
Cards (5): Agency Coroner, Grab the Prize, Cinder Strike, Coiling Rebirth,
Longstalk Brawl. (Celestial Reunion is a separate in-scope increment —
needs a new interactive behold-choose-a-creature-type cost subsystem — and
stays honestly unsupported until that lands.)
- B3 Agency Coroner: thread is_suspected through LKISnapshot + the synthesized
ZoneChangeRecord (both #[serde(default)]), captured live from the object
before the sacrifice reset; read at zone_change_record_matches_property so
FilterProp::Suspected evaluates on the sacrifice-cost path. CR 701.60.
- CostPaidPredicate::Color -> Property(FilterProp) (both applied via
typed.properties); add the suspected branch -> FilterProp::Suspected.
- Grab the Prize / Cinder Strike: additional-cost object predicate
("discarded card wasn't a land") / optional blight cost -> higher effect.
- Coiling Rebirth / Longstalk Brawl: gift-given (AdditionalCostPaid) gate.
- Zero new engine variants. All 5 flip supported:true gap_count:0.
- Discriminating tests: Coroner suspected->2/unsuspected->1; Grab nonland->2dmg
/land->0; Cinder blight->4/decline->2; gift cards gift->bonus/no-gift->base.
- coverage-regression vs main: REGRESSED(engine)=0; all 5 in GAINED.
Assisted-by: ClaudeCode:claude-opus-4.8
Wire conditions gated on target identity, combat presence, and populated
anaphora onto existing typed variants; add a full-cast hardening test for
Batch-2's Coiling Rebirth.
Cards (5): Steer Clear, Fear of Immobility, Charging Hooligan,
Yenna Redtooth Regent, Eliminate the Impossible. (A Killer Among Us and
Malamet Battle Glyph are deferred to their own increments — A Killer needs
a new interactive secret-choice subsystem; Malamet is blocked on a
pre-existing multi-target propagation bug and stays honestly unsupported.)
- Steer Clear: strip_instead_clause gains a multi-sentence guard so a prior
sentence + "instead if" line defers to the chain parser (root-cause fix);
ConditionInstead{ControllerControlledMatchingAsCast}. Structural-only,
REGRESSED(engine)=0.
- Fear of Immobility: stun sub gated And[HasObjectTarget, TargetMatchesFilter
{Opponent}] on the tapped target's live controller (CR 109.4).
- Charging Hooligan: parse_a_type_is_in_combat -> IsPresent{Rat+Attacking}
gates the trample grant (CR 508.1/509.1).
- Yenna: QuantityCheck{ObjectCount{LastCreated & Aura} >= 1} gates untap+scry
(field-expressive: the created token is not targets[0]).
- Eliminate the Impossible: resolve_populated_unsuspect_anaphors lowers
PumpAll -> Unsuspect{population, All} + existential Suspected gate
(CR 701.60a); reclaims the previously-swallowed clause.
- Coiling Rebirth hardening: full cast->resolve pinning ObjectId survival
across graveyard->battlefield so Not(Legendary) reads live supertypes;
gift+nonlegendary->token / legendary->none / gift-declined->none.
- Zero new engine variants. All 5 flip supported:true gap_count:0.
- coverage-regression vs main: REGRESSED(engine)=0; swallowed 947->920.
Assisted-by: ClaudeCode:claude-opus-4.8
…ost + conditional search destination
Celestial Reunion: "As an additional cost to cast this spell, you may choose a
creature type and behold two creatures of that type. Search your library for a
creature card. If you beheld a creature this way and that card is the chosen
type, put it onto the battlefield; otherwise put it into your hand."
Full-gate S07 increment (Condition_If shape gated on an optional additional cost
whose result-object destination depends on a resolution choice):
- AbilityCost::Behold gains type_choice: Option<ChoiceType>; new
WaitingFor::CostTypeChoice round-trip lets the caster choose the creature type
during cost payment (CR 601.2b), writing ChosenAttribute::CreatureType onto the
spell object; single-authority feasibility helper drives both B1 payability and
B2 option enumeration.
- TargetFilter::without_prop + FilterProp::IsChosenCreatureType express "that card
is the chosen type".
- Conditional destination lowered as SearchLibrary → ChangeZone{Battlefield,
And[AdditionalCostPaid, TMF{IsChosenCreatureType}]} / else ChangeZone{Hand}; a
post-lowering fold (fold_search_choose_type_conditional_destination) repairs the
mangled chain, corpus-grep-scoped to this card only.
- Deferral disjunct in effects/mod.rs gated tightly on
matches!(WaitingFor::SearchChoice) (result-injecting only) so no supported
EffectZoneChoice/grave-choice card regresses (byte-identical for every
non-SearchChoice wait).
- phase-ai: CostTypeChoice routed to DecisionKind::ActivateAbility (cost-phase
bucket); N-1 AI candidate/cancel-cast seam confirmed live.
- Frontend: CostTypeChoice reuses NamedChoiceModal (no new i18n strings).
Card flips supported:true gap_count:0. 6 discriminating WaitingFor→GameAction
round-trip tests (per-leg AdditionalCostPaid / IsChosenCreatureType discriminators,
non-vacuous by revert-trace). REGRESSED(engine)=0.
CR 205.3m, 601.2b, 601.2h, 701.4a, 702.73a, 608.2c (grep-verified).
Assisted-by: ClaudeCode:claude-opus-4.8
…n + 3 delayed/filter sites
Increment A of the S07 target-model work: behavior-neutral infrastructure that
teaches four resolution sites to honor `TargetFilter::ParentTargetSlot { index }`
(an anaphoric reference to a specific earlier chain target slot), where they
previously either fell through to all-objects or dropped the index. Flips no
card; unblocks indexed-slot consumers (S25 B3 delayed-trigger snapshot machinery
+ the S07 Malamet/Longstalk counter-target fix landing separately).
- `game/targeting.rs`: extract the inline root-lookup/flatten logic into a single
authority — `parent_chain_targets_from_root` (byte-identical to the old inline
block) + `resolve_parent_slot_from_root(state, ability, index)` (adds
`.nth(index)`). Refactor the existing caller to use it.
- `game/effects/counters.rs`: `resolve_defined_or_targets` gains a
`ParentTargetSlot { index }` arm resolving via the helper (CR 608.2c + CR 122.1).
- `game/effects/delayed_trigger.rs` `concrete_parent_target_filter`,
`game/effects/mod.rs` `filter_refs_parent_target`,
`game/filter.rs` `normalize_contextual_filter`: each gains a
`ParentTargetSlot { index }` arm (incl. the `Not(ParentTargetSlot)` case),
strictly additive — existing `ParentTarget` (non-indexed) callers byte-untouched
(CR 603.7c + CR 608.2c).
4 discriminating tests, each empirically revert-probed (all fail when the arm is
neutered): indexed resolution picks the right slot / binds the right object /
detects the ref / excludes only that slot. REGRESSED(engine)=0; engine suite
14562 passed.
CR 608.2c, 122.1, 603.7c (grep-verified).
Assisted-by: ClaudeCode:claude-opus-4.8
… condition + reciprocal-fight class fix)
Flips Malamet Battle Glyph to supported:true gap_count:0 and fixes the latent
cast-time panic in the committed Longstalk Brawl / Duel for Dominance
"those creatures fight each other" class (their tests only drove
evaluate_condition, never a full cast).
Malamet: "Choose target creature you control and target creature you don't
control. If the creature you control entered this turn, put a +1/+1 counter on
it. Then those creatures fight each other."
- Condition detector (parser/oracle_effect/conditions.rs): new
parse_target_entered_this_turn_condition — "if [anaphor | filter] entered this
turn" → TargetMatchesFilter{creature + controller + FilterProp::EnteredThisTurn}
(per-object, CR 400.7). Building-block: also flips Samut, Vizier of Naktamun
("if that creature entered this turn, draw a card"). Rejects control-count /
existential / source forms (over-fire negatives).
- Chain rewrite (parser/oracle_effect/lower.rs): rewrite_two_target_counter_chain
— for a ≥2 Typed-TargetOnly chain, re-key PutCounter{ParentTarget} →
ParentTargetSlot{0} and set subject_slot:Some(0) on the node's condition so the
counter + its condition bind the FIRST (you-control) target, not the
most-recent slot (CR 608.2c).
- subject_slot: Option<usize> on AbilityCondition::TargetMatchesFilter (serde
default + skip_serializing_if); None = legacy node-local first object.
- Reciprocal-fight cast fix: shared parse_fight_target ("each other" →
ParentTarget) wired into both fight dispatchers; resolve_fight_fighters recovers
both fighters from the flattened chain root when local object targets < 2 and
both root slots resolve to distinct objects (CR 701.14a). Root cause of the
spurious all-players target slot: the reciprocal Fight target lowers to a
context-ref (TrackedSet), which the fight slot-gen/assign arms did not skip —
broadened both from SelfRef|ParentTarget to filter.is_context_ref()
(fight-scoped, load-bearing: reverting it re-panics all three casts).
7 full cast().resolve() tests (Malamet both condition branches + which-creature +
both-fight; Longstalk castable; Duel under coven) + synthetic fight-guard +
4 detector tests. Tail Swipe correctly stays RED (unrelated Unimplemented node).
REGRESSED(engine)=0; only Malamet + Samut flip; semantic-audit 305→305.
CR 109.4, 400.7, 603.2, 608.2c, 701.14a (grep-verified).
Assisted-by: ClaudeCode:claude-opus-4.8
…drop the middle token
split_create_token_sequence recognized only " and " as a token separator, never the
intra-list comma, so "create A, a B, and a C" split into left="A, a B," / right="C"
and parse_token_description kept only the first item — silently dropping every middle
token, with no diagnostic (a false supported:true).
Rewrite the binary splitter into an N-way split over the conjunctive-only separator
set {", and ", ", ", " and "}, each guarded by peek(parse_token_noun_start) and a
quote-swallowing item unit (both lifted from the sibling split_choice_list_items),
keeping the existing "and "-gate so disjunctive "A, B, or C" lists stay routed to the
modal choice parser. All N tokens chain via sub_ability in written order. When the
gate fires with >=2 items but any item is not an Effect::Token, emit an honest
Effect::unimplemented instead of the silent single-token fallback.
Fixes the middle-token drop for 9 conjunctive cards (Bestial Menace, Fae Offering,
Somberwald Beastmaster, Liberated Livestock, Mascot Exhibition, Triplicate Titan,
Trostani's Summoner, The Companion of the Wilds, A Killer Among Us) and corrects The
Companion's can't-block misattribution (now on the Rat, not the Food token).
Overencumbered's subject-scoped "enchanted opponent creates ..." path
(parse_search_creation_imperative) is a separate parser with the same symptom —
tracked debt, not fixed here.
CR 608.2c. REGRESSED(engine)=0.
Assisted-by: ClaudeCode:claude-opus-4.8
…ent_ref_slots
effect_parent_ref_slots had Attach/Token/CopyTokenOf context-ref arms but no
UnattachAll arm, so the `_ => {}` fallback silently dropped a context-ref
`attachment`: a delayed "when you lose control of this, unattach it" trigger
snapshotted nothing and resolved inert. Add the mirror arm guarded by
`attachment.is_context_ref()` (class fix — Stolen Uniform, Ogre Geargrabber).
Assisted-by: ClaudeCode:claude-opus-4.8
…arget-hoist gate
Complete "A Killer Among Us" end-to-end (S07 condition-if tranche):
- N1: parameterize ChoiceType::CreatureType { options } for a restricted candidate
set (byte-stable unit-when-empty serde); compute_options honors it.
- N2: parse "secretly choose Human, Merfolk, or Goblin" as a restricted
creature-type enumeration (creature-only subtype vocab, no hardcoded names).
- N3: recognize "reveal the creature type you chose" as a no-op cost so the ability
resolves to a single Sacrifice(SelfRef), not a phantom double-Sacrifice.
- N4: hoist the condition's "target attacking creature token" into the body's first
anaphor so the ability declares slot 0, and gate the buff on
TargetMatchesFilter{ IsChosenCreatureType, subject_slot: Some(0) } — the counters
and deathtouch land on the attacking token, and the chosen type is read
post-sacrifice from the graveyard object via LKI (CR 608.2h).
7 cast-level tests including the matching/non-matching gate pair and a
multi-authority source-scoped-read fixture. A Killer flips supported:true
gap_count:0. Depends on the token-splitter fix (f2648a0cb) for the 3-token ETB.
CR 205.3e, 205.3m, 607.2d, 608.2c, 608.2h, 702.73a.
Assisted-by: ClaudeCode:claude-opus-4.8
… Kuja + Rollercrusher
Two Standard damage-doubling replacements were flagged supported:false by a
spurious SwallowedClause(Condition_If): the ability-word "—" prefix injects a
false leading " if " marker into swallow_check's detector.
- Trance Kuja (Flare Star): unconditional Wizard-source damage doubler. Add a
Condition_If exemption (unconditional_valmod_leading_if_is_only_if_marker) that
strips the ability word, requires the body to start "if " with no while/
as-long-as/only-if residual and exactly one "if", and a value-modifier
replacement with no condition. CR 614.1a + CR 120.8.
- The Rollercrusher Ride: Delirium-gated noncombat doubler. Generalize
parse_while_antecedent to scan forward to the " while " gate (the recipient
clause sits between the "would deal " anchor and the gate), and wire the
captured gate into ReplacementCondition::OnlyIfQuantity{DistinctCardTypes,GE,4}.
Life-gain caller unchanged (flush empty-prefix case); fail-closed on unparseable
guard. CR 614.1a.
Both flip supported:true gap_count:0; coverage REGRESSED(engine)=0 (+2 exactly
these cards); semantic-audit 0 findings. 7 discriminating cast/parse tests
(revert->fail proven).
Assisted-by: ClaudeCode:claude-opus-4.8
parent_target_snapshot seeded from the tail clause's per-clause ability.targets,
so a multi-clause parent chain — whose tail carries only its own slot — left an
inner ParentTargetSlot{index} anaphor indexing out of range, degrading valid_card
to Any and resolving the delayed effect inert.
Reseed from the flattened root chain (parent_chain_targets_from_root, the same
root-chain flatten resolve_parent_slot_from_root uses); a non-empty root chain
wins, else the TriggeringSource event-context fallback is byte-unchanged.
Single-target delayed triggers (Flickerwisp, Grave Betrayal) have root chain ==
per-clause targets, so they are unaffected by construction. CR 603.7c + CR 608.2c.
Class fix for every multi-clause delayed trigger snapshotting a parent-slot ref.
New revert-proven test (multi-clause tail snapshots full root chain — panics
[Object(10)] vs [Object(10),Object(11)] on revert); existing single-clause and
TriggeringSource sibling tests preserved.
Assisted-by: ClaudeCode:claude-opus-4.8
… re-attach split as-long-as condition
Cloud, Midgar Mercenary parsed DoubleTriggers{cause:Any} but dropped both the
affected scope and the "as long as equipped" gate (two swallows: Condition_If +
Condition_AsLongAs). Three coordinated fixes:
- Parser scope (oracle_static/evasion.rs): SelfRef is restrictive; new
parse_doubler_disjunct maps "~" -> SelfRef and "an Equipment attached to it" ->
Typed(Equipment, AttachedToSource) ("it" = the doubler source, not the
enchanted recipient). affected = Or[SelfRef, Typed(Equipment, AttachedToSource)].
- Parser gate (oracle_static/dispatch.rs): the inverted-"as long as" split
re-dispatches the remainder but only re-wrapped it with .description(), dropping
the split condition. Re-attach parse_static_condition(split.condition_text) when
the recursed def has no condition. Fires only on condition.is_none() and only
attaches a recognized condition — strictly corrective; fixes the whole class,
not just DoubleTriggers. "~ is equipped" -> SourceIsEquipped. CR 611.3a.
- Runtime self-inclusion (game/triggers.rs): apply_trigger_doubling
unconditionally self-excluded the doubler. Gate it: when the affected filter
references self (filter_references_self), the doubler's OWN triggers double too.
Panharmonicon/Isshin (affected:None) and Wayta (non-self) stay self-excluded.
CR 603.2d.
Cloud flips supported:true gap_count:0 (both swallows cleared). Measured
REGRESSED(engine)=0 across 35367 faces; the class fix additionally rescues 4
sibling inverted-as-long-as statics (Ethrimik, Glimpse the Cosmos, Pact Weapon,
The Bird Champion); -7 swallowed clauses accounted per-handler, 0 new swallows.
5 revert-proven tests (4 behavioral + parse-shape); DoubleTriggers-family
regression (Panharmonicon/Isshin/Wayta/Harmonic Prodigy/Splinter/Delney/Drivnod/
Hama Pashar) all unchanged. CR 301.5a.
Assisted-by: ClaudeCode:claude-opus-4.8
…ion_If
Two Standard cards flagged supported:false by a spurious SwallowedClause(Condition_If):
- Sonic Shrieker (detector-only): "If a player is dealt damage this way, they
discard a card." Already parses + resolves correctly (ETB DealDamage -> GainLife ->
Discard{ParentTarget}: the damaged player chooses a discard; a creature target is a
no-op). Only the swallow warning was spurious. Add a text-gated exemption
(any_ability_has_parent_target_discard + "dealt damage this way") mirroring the
existing "lost life this way" branch — the ParentTarget discard is the CR 608.2c
back-reference, structurally represented, not a swallowed condition. The prevented-
damage edge (CR 615.5) matches Screaming Nemesis's shipped fidelity ceiling.
- Slumbering Trudge: "If X is 2 or less, it enters tapped." The SetTapState ETB
replacement dropped its condition and tapped unconditionally. New combinator
parse_enters_tapped_if_x_comparison captures the cast-X gate into
ReplacementCondition::OnlyIfQuantity{CostXPaid, LE, 2}, dispatched before the
unconditional enters-tapped guard. Reuses existing typed surface (no new variant).
CR 107.3 / 614.1c / 614.1d.
Both flip supported:true gap_count:0. Measured REGRESSED(engine)=0; GAINED = exactly
these two cards (no class collateral); semantic-audit 0 findings. 7 discriminating
tests (both revert-proven; the no-swallow test uses a Flying keyword to avoid the
Unimplemented detector-suppressor).
Assisted-by: ClaudeCode:claude-opus-4.8
…(QuantityRef::BendTypesThisTurn)
Avatar Aang parsed ElementalBend -> Draw -> Transform with no condition, so it
transformed on every bend (two swallows: Condition_If + Duration_ThisTurn). The
per-turn bend tracking already exists (Player::bending_types_this_turn); the only
gap was a QuantityRef leaf reading its distinct-type count.
- New bare QuantityRef::BendTypesThisTurn (Controller-scoped), reading
bending_types_this_turn.len() — the distinct-bend-type cardinality axis, mirroring
the bare CrimesCommittedThisTurn/DescendedThisTurn siblings. Wired into every
exhaustive QuantityRef match: the 3 quantity.rs classification groups, the
triggers.rs cost-paid-object walker, and the ability_scan.rs growing-cascade
scanner (=> Axes::NONE), plus coverage describe/kind.
- Parser: "done all four this turn" -> QuantityComparison{BendTypesThisTurn, GE, 4}
in parse_youve_this_turn; the existing "then if" intervening-if machinery attaches
it to the Transform sub-ability (CR 603.4), not the Draw. Duration_ThisTurn swallow
cleared by registering the new marker. BendingType has exactly 4 variants, so
GE 4 == all four; four firebends give len 1 (distinct-type semantics).
CR 603.4 / 608.2c / 701.65b / 701.66b / 701.67c / 702.189b.
Flips supported:true gap_count:0 (both swallows cleared). Measured REGRESSED(engine)=0;
GAINED = exactly [Avatar Aang]; semantic-audit 0 findings. 3 runtime tests (partial-
bend discriminator revert-proven) + 1 inline parser test.
Assisted-by: ClaudeCode:claude-opus-4.8
…redaction, manabrew + stack-badge wiring Address the four review findings on the behold "choose a creature type" pre-cost (`WaitingFor::CostTypeChoice`, Celestial Reunion): - [MED] Cancel now clears the temporary `ChosenAttribute::CreatureType` from the spell object (`handle_cancel_cast`), so backing out fully rewinds the choice and a re-cast re-prompts instead of silently reusing the stale type (CR 601.2 + CR 733.1 — the entire action is reversed). - [MED] `filter_state_for_viewer` redacts `CostTypeChoice.options` to empty for viewers who cannot see the caster's private zones — the option list is derived from beholdable hand cards and leaked private info (CR 400.2). - [MED] manabrew-compat maps `CostTypeChoice` to `chooseType` with `validTypes` and `source_card_id` from `pending_cast.object_id` (was falling through to `UnsupportedPrompt`), plus a `waiting_for_type` label. - [LOW] `StackDisplay.getPendingCastObjectId` includes `CostTypeChoice` so the stack keeps its "Casting" badge during the prompt. Three discriminating tests, each revert-fail verified: cancel_after_type_choice_rewinds_and_recast_reprompts, cost_type_choice_options_redacted_for_opponent_viewer, cost_type_choice_prompt_maps_to_choose_type_with_source. Assisted-by: ClaudeCode:claude-opus-4.8
7ce2930 to
d8dbffb
Compare
|
🤖 AI text below 🤖 Thanks for the thorough review — all four findings are addressed in 1. Cancel leaves stale creature type — 2. Hidden-info leak — 3. manabrew-compat gap — Added a 4. Frontend badge drop — On the sweep-review hold: the coverage-parse-diff sticky regenerates for this exact head on this push; the parser changes it lists are the intended target-model/behold work from the tranche. Local measurement (main One additional item surfaced during our internal review of the fix, out of scope here: a pre-existing latent bug where |
matthewevans
left a comment
There was a problem hiding this comment.
Current-head re-review of d8dbffbd8c: the prior requested-change items are addressed, including cancel rewind for behold type choice, viewer redaction, manabrew prompt/source wiring, and stack badge display. Parse-diff evidence is broad but explainable for the claimed class-level parser/engine work; final fix commit does not alter parser scope. Approving; CI/merge queue can gate readiness.
|
🤖 AI text below 🤖 Note on the red Decision-cost perf gate on Per the repo convention (baseline refreshes ship with the paired-seed report attached), we'll submit the baseline refresh as an immediate follow-up PR computed against post-merge main rather than adding a commit to this queued PR — so this branch stays exactly as you reviewed it. Merge past the perf gate at your discretion, or if you'd rather the refresh land first, say the word and we'll reorder. |
🤖 AI text below 🤖
What
Completes the S07 condition-if standard cluster: 28 cards, all
supported:true gap_count:0, via 14 commits of building-block work (no card-specific hacks). Highlights by mechanism family:ParentTargetSlotindexed resolution (resolve_parent_slot_from_root) + delayed/filter sitesQuantityRef::BendTypesThisTurn)Class fixes beyond the 28
fix(parser): "create A, a B, and a C token" 3+-item chains silently dropped the middle token — N-way conjunctive split with peek-guarded separators; fixes 9 more printed cards (Bestial Menace, Fae Offering, Triplicate Titan, Trostani's Summoner, …) and corrects The Companion of the Wilds' can't-block misattribution.fix(engine): delayed-triggerParentTargetSlotnow snapshots the full root chain (multi-clause delayed triggers; Flickerwisp-class single-clause cards unaffected by construction) + context-refUnattachAllsurfaced ineffect_parent_ref_slots.Verification
cast().resolve()runtime tests for cast-path work (not condition-only probes).main; full CI-equivalent (fmt, workspace clippy-D warningswith proptest, engine test suite) green locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01CwQE5oyMqZ9T4BPMsih3Kj