fix(engine): batch delayed phase triggers with normal triggers - #5048
Merged
matthewevans merged 1 commit intoJul 3, 2026
Merged
Conversation
matthewevans
enabled auto-merge
July 3, 2026 22:58
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
matthewevans
deleted the
ship/fixengine-batch-delayed-phase-triggers-with-nor
branch
July 3, 2026 23:25
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 4, 2026
…rigger_ordering (CR 603.3b) Close the HIGH-2 finding (PR phase-rs#5072 review): check_delayed_triggers (CR 603.7) APNAP-sorted its firing batch then DIRECTLY dispatched each trigger, bypassing begin_trigger_ordering. So two+ simultaneous same-controller order-dependent non-phase delayed triggered abilities (e.g. two "when a creature dies" deals-damage / put-counter triggers off one death) reached the stack in fixed order with no CR 603.3b ordering choice. This dispatch tail was byte-identical base->HEAD (pre-existing); the PR made begin_trigger_ordering the sound ordering authority but never wired this path. The phase-delayed path (process_collected_triggers_with_delayed_phase_events, phase-rs#5048) is the template. Fix (maintainer's shape, minimal): - check_delayed_triggers: convert the to_fire batch into PendingTriggerContexts (reusing delayed_trigger_to_context), APNAP-sort, route through begin_trigger_ordering -- PromptForChoice sets waiting_for, NoChoiceNeeded dispatches via dispatch_deferred_triggers_in_order. The bespoke Paused / DroppedTargetUnresolved / DroppedNoLegalMode / ResolvedInline arms are deleted: the shared dispatcher applies every Delayed-origin disposition unchanged (Good King Mog DroppedTargetUnresolved->Pushed, Breeches reflexive pause, inline mana). - engine_priority: surface an OrderTriggers prompt set by check_delayed_triggers before check_state_triggers / the Priority fallthrough clobbers it (scoped to OrderTriggers, so the Breeches pending_trigger pause is untouched). Pure CR 603.3b tightening: singletons and provably-commuting identical no-input groups still auto-order; only genuinely order-dependent or distinct-input same-controller batches now prompt. One integration test (daretti_emblem_simultaneous_death) asserted the old direct-dispatch contract for two distinct-target simultaneous returns; it is updated to a positive OrderTriggers pin (its outcome assertions are unchanged and still pass). The impl-plan's caller census under-counted that test: it grepped crates/engine/src (inline #[cfg(test)] modules) but not crates/engine/tests/ (integration dir). A full source re-census plus the exactly-one-failure signal confirmed Daretti is the only newly-prompting caller (encore's 2x Sacrifice{SelfRef} is order-independent and still auto-orders; every other caller is a singleton). Follow-up (latent, no card/test exercises it): the check_delayed_triggers call in engine_resolution_choices (ChooseBranch deferred-ETB replay) could surface an OrderTriggers prompt that drain_pending_continuation might overwrite; strictly no worse than pre-HIGH-2 (which never ordered anywhere in check_delayed_triggers). phase-rs#4809 (Braids / Spinal Embrace) is a distinct phase-path seam, not this bug: "at the beginning of the next end step" is phase-classified and already merged + ordered by the phase path; this fix neither closes nor regresses it. Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 4, 2026
…rigger_ordering (CR 603.3b) Close the HIGH-2 finding (PR phase-rs#5072 review): check_delayed_triggers (CR 603.7) APNAP-sorted its firing batch then DIRECTLY dispatched each trigger, bypassing begin_trigger_ordering. So two+ simultaneous same-controller order-dependent non-phase delayed triggered abilities (e.g. two "when a creature dies" deals-damage / put-counter triggers off one death) reached the stack in fixed order with no CR 603.3b ordering choice. This dispatch tail was byte-identical base->HEAD (pre-existing); the PR made begin_trigger_ordering the sound ordering authority but never wired this path. The phase-delayed path (process_collected_triggers_with_delayed_phase_events, phase-rs#5048) is the template. Fix (maintainer's shape, minimal): - check_delayed_triggers: convert the to_fire batch into PendingTriggerContexts (reusing delayed_trigger_to_context), APNAP-sort, route through begin_trigger_ordering -- PromptForChoice sets waiting_for, NoChoiceNeeded dispatches via dispatch_deferred_triggers_in_order. The bespoke Paused / DroppedTargetUnresolved / DroppedNoLegalMode / ResolvedInline arms are deleted: the shared dispatcher applies every Delayed-origin disposition unchanged (Good King Mog DroppedTargetUnresolved->Pushed, Breeches reflexive pause, inline mana). - engine_priority: surface an OrderTriggers prompt set by check_delayed_triggers before check_state_triggers / the Priority fallthrough clobbers it (scoped to OrderTriggers, so the Breeches pending_trigger pause is untouched). Pure CR 603.3b tightening: singletons and provably-commuting identical no-input groups still auto-order; only genuinely order-dependent or distinct-input same-controller batches now prompt. One integration test (daretti_emblem_simultaneous_death) asserted the old direct-dispatch contract for two distinct-target simultaneous returns; it is updated to a positive OrderTriggers pin (its outcome assertions are unchanged and still pass). The impl-plan's caller census under-counted that test: it grepped crates/engine/src (inline #[cfg(test)] modules) but not crates/engine/tests/ (integration dir). A full source re-census plus the exactly-one-failure signal confirmed Daretti is the only newly-prompting caller (encore's 2x Sacrifice{SelfRef} is order-independent and still auto-orders; every other caller is a singleton). Follow-up (latent, no card/test exercises it): the check_delayed_triggers call in engine_resolution_choices (ChooseBranch deferred-ETB replay) could surface an OrderTriggers prompt that drain_pending_continuation might overwrite; strictly no worse than pre-HIGH-2 (which never ordered anywhere in check_delayed_triggers). phase-rs#4809 (Braids / Spinal Embrace) is a distinct phase-path seam, not this bug: "at the beginning of the next end step" is phase-classified and already merged + ordered by the phase path; this fix neither closes nor regresses it. Assisted-by: ClaudeCode:claude-opus-4.8
lgray
added a commit
to lgray/phase
that referenced
this pull request
Jul 4, 2026
…rigger_ordering (CR 603.3b) Close the HIGH-2 finding (PR phase-rs#5072 review): check_delayed_triggers (CR 603.7) APNAP-sorted its firing batch then DIRECTLY dispatched each trigger, bypassing begin_trigger_ordering. So two+ simultaneous same-controller order-dependent non-phase delayed triggered abilities (e.g. two "when a creature dies" deals-damage / put-counter triggers off one death) reached the stack in fixed order with no CR 603.3b ordering choice. This dispatch tail was byte-identical base->HEAD (pre-existing); the PR made begin_trigger_ordering the sound ordering authority but never wired this path. The phase-delayed path (process_collected_triggers_with_delayed_phase_events, phase-rs#5048) is the template. Fix (maintainer's shape, minimal): - check_delayed_triggers: convert the to_fire batch into PendingTriggerContexts (reusing delayed_trigger_to_context), APNAP-sort, route through begin_trigger_ordering -- PromptForChoice sets waiting_for, NoChoiceNeeded dispatches via dispatch_deferred_triggers_in_order. The bespoke Paused / DroppedTargetUnresolved / DroppedNoLegalMode / ResolvedInline arms are deleted: the shared dispatcher applies every Delayed-origin disposition unchanged (Good King Mog DroppedTargetUnresolved->Pushed, Breeches reflexive pause, inline mana). - engine_priority: surface an OrderTriggers prompt set by check_delayed_triggers before check_state_triggers / the Priority fallthrough clobbers it (scoped to OrderTriggers, so the Breeches pending_trigger pause is untouched). Pure CR 603.3b tightening: singletons and provably-commuting identical no-input groups still auto-order; only genuinely order-dependent or distinct-input same-controller batches now prompt. One integration test (daretti_emblem_simultaneous_death) asserted the old direct-dispatch contract for two distinct-target simultaneous returns; it is updated to a positive OrderTriggers pin (its outcome assertions are unchanged and still pass). The impl-plan's caller census under-counted that test: it grepped crates/engine/src (inline #[cfg(test)] modules) but not crates/engine/tests/ (integration dir). A full source re-census plus the exactly-one-failure signal confirmed Daretti is the only newly-prompting caller (encore's 2x Sacrifice{SelfRef} is order-independent and still auto-orders; every other caller is a singleton). Follow-up (latent, no card/test exercises it): the check_delayed_triggers call in engine_resolution_choices (ChooseBranch deferred-ETB replay) could surface an OrderTriggers prompt that drain_pending_continuation might overwrite; strictly no worse than pre-HIGH-2 (which never ordered anywhere in check_delayed_triggers). phase-rs#4809 (Braids / Spinal Embrace) is a distinct phase-path seam, not this bug: "at the beginning of the next end step" is phase-classified and already merged + ordered by the phase path; this fix neither closes nor regresses it. Assisted-by: ClaudeCode:claude-opus-4.8
matthewevans
pushed a commit
that referenced
this pull request
Jul 4, 2026
…d CR 603.3b same-event trigger ordering (#5072) * feat(engine): PR-6.75 commit 1 — ability_rw read/write conflict profiler (CR 603.3b) The D-profile classifier for sound trigger-ordering (CR 603.3b): a kind- and scope-aware read/write conflict profile over one ability, for the legacy UNGATED ordering paths (C0-full + C1). No consumer yet — the triggers.rs rewiring lands in commit 2, so this commit is zero behavior change and loop_detection is unaffected. NEW crates/engine/src/game/ability_rw.rs: - StateKind/KindSet/Census/SourceCensus/RwProfile (+ source_independent) / GroupStructure / feeds / profiles_conflict / ability_rw_profile / trigger_condition_rw_profile. Exhaustive wildcard-free walk over Effect/QuantityRef/QuantityExpr/*Condition/TargetFilter/ObjectScope/… mirroring ability_scan's traversal-closure discipline (M3 binding mandate: precise arms bind all payload fields; { .. } only on maximal-conservative RHS). - The §1.2 commutation formula: CR 603.10a + CR 400.7 LKI freeze, CR 603.4 intervening-if, CR 603.5 resolution-time-choice exclusions, CR 603.7 deferred bodies; the census-overlap membership feed row; the ParentTarget chain-root + event-object disjointness anaphoric rules (resolver-pinned); D5 legacy_batch_prompt retained-prompt classification. Sound modulo the documented source-actor residual (CR 702.15 lifelink / CR 702.2 deathtouch / CR 800.4a player-loss cascade), inherited unchanged from the shipped short-circuit. - 16 N-E unit pairings (§5.4) — each conflict/clean pair revert-fail-proven. Analysis-internal types only; no gated-enum variant added (engine-inventory clean). triggers.rs and ability_scan.rs byte-untouched (axis-3 immutability invariant, D2). Assisted-by: ClaudeCode:claude-opus-4.8 * feat(engine): PR-6.75 commit 2 — C0-full + C1 sound trigger-ordering gate (CR 603.3b) Retire the fail-open 12-string serde-walk allowlist (C0-full) and install a sound read/write conflict gate at the same-event short-circuit (C1) — the latent CR 603.3b bug where identical order-dependent siblings were auto-ordered instead of prompting the controller. Both live on the legacy UNGATED paths (the same-event/batch short-circuit that always ran and always auto-ordered); C1 is monotone-safe (OLD always auto-ordered, so C1 can only add mandatory ordering prompts, never remove one). The game-changing distinct-event term (C2) stays gated on loop_detection. - triggers.rs: rewire trigger_events_match_for_ordering / group_is_order_independent onto the ability_rw conflict profiler. C1 same-event + C0-full ZoneChanged departure batch inherited unconditionally; C2 distinct-event auto-order stays loop_detection-gated (default OFF byte-preserves distinct-event gameplay). - ability_rw.rs: Class-C classifier refinements on the profiler — zone-aware SetMembership census (CR 400.1, Tombstone Stairwell battlefield-write vs graveyard-read), external counter object-census (CR 122.1, Earthbender Ascension), and delver/abattoir/deadeye read-kind corrections; ParentTargetSlot classified as a non-D5 event ref (restores the exactly-12-tag legacy_batch_prompt property). - triggers_ordering_parity_tests.rs: whole-corpus §5.2 allowlist-parity sweep (frozen 12-string oracle vs the new gate — 0 unexplained modulo the 7 proven category-(1) rows) plus the N-A..N-F soundness discriminators and the class-A reachability guards (legendary CR 704.5j, per-source DamageDone via valid_source, CR 603.4 condition self-exclusion). - ability_scan.rs: dual-walk maintenance doc paragraph only (D2 — zero match-arm / Axes literal change). Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 commit 3 — position-agnostic D5 legacy-ref visitor (CR 603.10a) The batch-path retained-prompt flag (legacy_batch_prompt, D5) was set only at scattered typed leaf sites reached by the read/write classification walk, so a legacy event-context ref sitting in an effect target/count position — or a deeper subtree the walk doesn't descend (TypedFilter.controller, FilterProp interior, static-ability affected filter, granted-trigger body) — left the flag false and auto-ordered a departure batch the shipped engine prompted (CR 603.3b fail-open; 50 cards in the full-DB corpus). Replace the leaf-hooking with a dedicated typed recursive visitor `contains_legacy_event_ref(&ResolvedAbility)` that visits every structural position a legacy ref can occupy — all TargetFilters (incl. nested / chain-root), QuantityExpr/QuantityRef counts, ObjectScope/PlayerFilter/ControllerRef, across all Effect variants, sub-ability/modal chains, and the trigger condition. Every match is exhaustive with no wildcard, so a future variant fails to compile until classified. legacy_batch_prompt is set authoritatively from the visitor, decoupled from walk descent. Typed, not a serde walk — this runs on the trigger-ordering hot path (the regression #4912 exists to prevent). Proof (typed == frozen serde oracle, zero disagreement): - Mechanical tag x position matrix test: 12 frozen tags across 5 target / 3 quantity / scope / player / controller positions, driven through the production profiling entry points, with negative controls. - Full-DB parity sweep: BATCH prompt->auto diffs 50 -> 0, no over-flagging (total unexplained is exactly the out-of-scope 15 batch auto->prompt + 49 same-event), retained_prompt +50. The frozen serde oracle stays test-side as the reference. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 commit 4 — cheap read/write conflict levers (CR 603.3b) Ten pure read/write-classification refinements to the trigger-ordering conflict profiler, each turning a provably-commuting same-event / departure- batch group from a needless PROMPT into an AUTO-order (fail-closed: a missed refinement over-prompts, never under-prompts). - L1 ZoneCardCount{zone,card_types} → extract census + read-zone (was Any/Any) - L2 zone-change/sacrifice/counter journals keyed to destination zone / frozen look-back; self token-copy records its battlefield creation zone - L3 TargetMatchesFilter{use_lki:true} → frozen LKI read (CR 603.10a) - L6 AttachedTo valid_card provably excludes every source (CR 701.3d) - L7 ObjectsShareQuality{LastRevealed} operand is a per-resolution local - L9 CopySpell{TriggeringSource|ParentTarget} reads the original by id; ChangeTargets is a StackShape write, not the conservative fallback (CR 115.7) - L12 Suspect/Unsuspect is an idempotent designation with no observable write - L13 RemoveFromCombat/SkipNextStep/SkipNextTurn → new TurnStructure kind - L14 ExtraTurn/AdditionalPhase → TurnStructure (was the Other catch-all) - voltstorm: reflexive-modal mode_abilities descend as a union, not conservative New StateKind::TurnStructure (CR 500/505.6/614.10) — a sequencing kind that only self-conflicts, replacing over-broad Other for turn/combat-structure writes. Full-DB sweep (bb03392 corpus): 64 → 27 unexplained over-prompts cleared. 12 discriminating unit tests (POS commute + adjacent NEG), each with revert-fail evidence. paroxysm / flamewake phoenix left as documented conservative-safe residual (context-free unclassifiable). L10/L11 (controller/player scope) split out to the same_controller commit; L4/L5/skyfisher to commit 5. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 same_controller trigger-ordering group property (CR 603.3b) Adds a controller-uniform AND owner-aligned group fact (GroupStructure.same_controller, live-computed at the group_is_order_independent chokepoint) plus gate-scoped PlayerSpan axes on RwProfile, so same-controller sibling triggers whose reads/writes are player/controller-disjoint auto-order instead of over-prompting. CR 603.3b + CR 109.5/102.2 (you vs opponents disjoint at N players) + CR 400.3/110.2/108.3 (owner-keyed self-write destinations are resolvable only under owner alignment). Clears 3 over-prompts (full-DB sweep 27 -> 24): defense of the heart, rekindled flame, brink of madness. Osseous sticktwister stays a documented-conservative prompt -- its your-graveyard read against an opponents-sacrifice write is order-observable when you own an opponent-controlled permanent (CR 701.21a: sacrifice moves to the owner's graveyard), so clearing it would be an under-prompt. Fail-closed by construction: same_controller=false makes profiles_conflict byte-identical to before. 11 discriminating tests (S1-S7) drive the production authority group_is_order_independent, including a 3-player multiplayer proof and revert-fail evidence for the gate and the owner-alignment conjunct. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 batch-hard Batch-T1 uniformity theorem (CR 603.3b) Refactors the same_controller group fact into ControllerUniformity {Mixed, Uniform, UniformAligned} and adds a Batch-T1 clause: in a controller-uniform co-departure batch of normalized-identical members, a resolution that consults neither its source binding, per-member bound storage (new reads_member_bound fact), its firing event, nor event-object writes is one function f(state, controller) shared by every member, so any resolution order composes f with f -- CR 603.3b ordering is unobservable. Uniformity, not controller-independence, is the sound bar: production partitions ordering groups by controller (begin_trigger_ordering), so every non-team group is uniform by construction; only CR 805.7 team pools mix, and those compute uniformity live and fail closed to Mixed. A mixed-controller batch is genuinely order-observable -- a discard's cause-source controller (EventSourceControlledBy) flips with resolution order. Clears 5 batch over-prompts (full-DB sweep 24 -> 19): emrakul the world anew, mindslicer, ruin grinder, slithermuse, yukora the prisoner. Day of the Dragons stays prompting -- its per-source TrackedSet return feeds the other member's re-sacrifice, so it is genuinely order-dependent (a correct prompt, not an over-prompt). Skyfisher Spider stays prompting -- owner-misaligned self-exile makes its graveyard read order-observable. Fail-closed by construction: ControllerUniformity::Mixed consults no refinement; UniformAligned reproduces the landed same_controller==true decisions byte-for-byte (27->24 preserved). CR 603.10a per-member-bound referents are flagged fail-closed at every TargetFilter, quantity, and scope/anchor position. 12 discriminating tests (B-1..B-7 plus an executable mixed-controller Dodecapod negative) drive the production authority group_is_order_independent, with revert-fail evidence for every T1 conjunct. Assisted-by: ClaudeCode:claude-opus-4.8 * test(engine): PR-6.75 sweep-green — document 18 conservative over-prompts + DotD genuine (CR 603.3b) Makes the full-DB trigger-ordering parity sweep GREEN (was panicking at 19 unexplained). Adds a self-documenting DOCUMENTED_OVER_PROMPT allowlist (18 conservative over-prompts, each with its class + reason) consulted behind a direction gate (decision_old && !decision_new — an under-prompt is never suppressible), a separate BATCH_GENUINE_ROWS set for Day of the Dragons (the one genuinely order-dependent batch, CR 603.3b), and full-DB completeness asserts (a stale or renamed entry fails). 19 = 18 documented-conservative + 1 genuine (Day of the Dragons). The conservative 18: 12 L8-held monotone/self-limiting (CR 603.4); osseous sticktwister + skyfisher spider (owner-misalignment / osseous-class: auto-safe when owner-aligned, correctly prompted when misaligned via CR 701.21a); paroxysm + flamewake phoenix (context-free-unclassifiable); deep-sea kraken + ichorplate golem (parse-blocked commutes). Floor re-tune: batch_self_srcread REMOVED — its cell is empty in-corpus; the old >=40 population was an artifact of the D5 fail-open hole closed in commit N-G, now counted by retained_prompt, so a >=0 assert would be decorative. The four surviving floors are re-tuned to full-DB measured value minus 5% (batch_obs >=469, retained_prompt >=271, t1_source_indep >=2727, hadcounters_batch_self >=1) and each is mutation-proven non-vacuous (probe matrix M1-M9). Test-only: zero engine-logic change. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 same-event member-bound trigger-ordering discriminator (CR 603.3b) Close a hole in the same-event ordering-soundness gate (PR #5072 review, HIGH-1): profiles_conflict's same-event fast path auto-ordered any source_independent group, but a group of DISTINCT sources whose identical resolution reads per-source bound storage (CR 603.10a look-back -- TrackedSet / ExiledBySource / ChosenCard, via member_bound_target_filter) is NOT one shared f(state): member A reads A's storage, member B reads B's, so the identical-function commutation proof breaks and the order can be observable. Same-event order-independence is decided solely by !same_event_conflict (no c2 backstop), so this auto-ordered genuinely order-dependent groups -- e.g. two Mimic Vats racing to imprint one shared dying creature. Fix (mirrors the batch path's !reads_member_bound conjunct at same-event depth): - exclude member-bound from the source_independent fast-path disjunct; - add the fail-closed discriminator `if s.same_event && p.reads_member_bound { true }`. all_same_source stays auto-ordered (one shared source => one shared storage => f_A=f_B). The batch path is byte-inert (both edits are s.same_event-guarded). Not latent: the full-DB sweep flips 48 same-event cards auto->prompt (all over-prompt direction -- the base engine auto-ordered every same-event group unconditionally, so these can never be under-prompts). 6 GENUINE order-dependent (mimic vat, mirror of life trapping, moonring mirror, duplicity, world queller, blood tyrant) + 42 conservative safe over-prompts. The sweep documents them as a predicate-keyed CLASS (reads_member_bound, corpus-churn-robust) with the 6 genuine enumerated (SAME_EVENT_MEMBER_BOUND_GENUINE) and the 42 conservative derived + emitted as evidence -- no 48-name allowlist. Sweep stays green (unexplained=0); t1_source_indep measured 2871->2830 (the source-independent slice now prompts), floor unchanged. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 route non-phase delayed triggers through begin_trigger_ordering (CR 603.3b) Close the HIGH-2 finding (PR #5072 review): check_delayed_triggers (CR 603.7) APNAP-sorted its firing batch then DIRECTLY dispatched each trigger, bypassing begin_trigger_ordering. So two+ simultaneous same-controller order-dependent non-phase delayed triggered abilities (e.g. two "when a creature dies" deals-damage / put-counter triggers off one death) reached the stack in fixed order with no CR 603.3b ordering choice. This dispatch tail was byte-identical base->HEAD (pre-existing); the PR made begin_trigger_ordering the sound ordering authority but never wired this path. The phase-delayed path (process_collected_triggers_with_delayed_phase_events, #5048) is the template. Fix (maintainer's shape, minimal): - check_delayed_triggers: convert the to_fire batch into PendingTriggerContexts (reusing delayed_trigger_to_context), APNAP-sort, route through begin_trigger_ordering -- PromptForChoice sets waiting_for, NoChoiceNeeded dispatches via dispatch_deferred_triggers_in_order. The bespoke Paused / DroppedTargetUnresolved / DroppedNoLegalMode / ResolvedInline arms are deleted: the shared dispatcher applies every Delayed-origin disposition unchanged (Good King Mog DroppedTargetUnresolved->Pushed, Breeches reflexive pause, inline mana). - engine_priority: surface an OrderTriggers prompt set by check_delayed_triggers before check_state_triggers / the Priority fallthrough clobbers it (scoped to OrderTriggers, so the Breeches pending_trigger pause is untouched). Pure CR 603.3b tightening: singletons and provably-commuting identical no-input groups still auto-order; only genuinely order-dependent or distinct-input same-controller batches now prompt. One integration test (daretti_emblem_simultaneous_death) asserted the old direct-dispatch contract for two distinct-target simultaneous returns; it is updated to a positive OrderTriggers pin (its outcome assertions are unchanged and still pass). The impl-plan's caller census under-counted that test: it grepped crates/engine/src (inline #[cfg(test)] modules) but not crates/engine/tests/ (integration dir). A full source re-census plus the exactly-one-failure signal confirmed Daretti is the only newly-prompting caller (encore's 2x Sacrifice{SelfRef} is order-independent and still auto-orders; every other caller is a singleton). Follow-up (latent, no card/test exercises it): the check_delayed_triggers call in engine_resolution_choices (ChooseBranch deferred-ETB replay) could surface an OrderTriggers prompt that drain_pending_continuation might overwrite; strictly no worse than pre-HIGH-2 (which never ordered anywhere in check_delayed_triggers). #4809 (Braids / Spinal Embrace) is a distinct phase-path seam, not this bug: "at the beginning of the next end step" is phase-classified and already merged + ordered by the phase path; this fix neither closes nor regresses it. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 classify v0.15.0's new enum variants in the ability_rw dual-walk (CR 603.3b) Rebasing the PR-6.75 series onto v0.15.0 (upstream/main 1badc19) surfaced 6 new upstream enum variants that the wildcard-free ability_rw read/write dual-walk must classify (E0004 non-exhaustive match). Each is classified per its rw-axis, with grep-verified CR annotations: FilterProp::InTrackedSet member-bound (chain tracked-set membership, property form of TargetFilter::TrackedSet) CR 603.10a Effect::EachSourceDealsDamage board-membership read + damage/life write CR 120.1 / 120.3a / 608.2c Effect::ChooseCounterKind ObjectCounters read + member-bound (persists per-source chosen counter kind) CR 122 / 603.10a Effect::PutChosenCounter ObjectCounters write + member-bound (consumes per-source chosen counter kind) CR 122.1 / 122.6 / 603.10a Effect::CreatePlaneswalkReplacement deferred body (descend reads, drop writes) CR 614.1a / 611.2c / 603.7 Effect::ChaosEnsues external write (planar chaos trigger) CR 311.7 / 901.9b WHY a standalone commit rather than folded into the owning profiler commits (c1 rw_effect / c3 legacy_* / c6 member_bound): three arms depend on engine features introduced LATER in this very series. CreatePlaneswalkReplacement uses `pscope` / `chain_move_owner` / the 4-arg `rw_effect` signature (born in the same_controller commit); ChooseCounterKind / PutChosenCounter set `p.reads_member_bound` (the field is born in the batch-hard commit). Since rw_effect's match must be exhaustive from the profiler commit onward, a per-commit-compiles fold would require deliberately- wrong evolving stub arms across three commits -- worse than an honest single adaptation. Per-commit-compiles for c1-c8 was in any case already forfeited by the rebase itself (those commits are non-exhaustive against the new base's variants by construction); the sound bisect build points are the base (1badc19) and this HEAD. (The pre-PR rebase folded its 15 new-variant arms into owning commits; this rebase could not, for the dependency reason above -- measured necessity, not a convention lapse.) Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): PR-6.75 same-event event-object trigger-ordering discriminator (CR 603.3b) Maintainer review (#5072): the same-event CR 603.3b gate was not fail-closed for the event-object read/write feed. `reads_event_live` was consulted only on the batch path (profiles_conflict all_same_source fast path + freeze-invalidation row, both !same_event-guarded), never in same-event feed analysis — so a same-event group that WRITES the shared triggering object and READS its live characteristic ("put a +1/+1 counter on it, then transform ~ if that creature's power >= 6" x2) auto-ordered, contradicting the group_is_order_independent contract ("can never auto-order order-sensitive triggers"). Root cause: `read_object_scope` maps EventSource/EventTarget reads to `reads_event_live`, which sets only a bool and records no KindSet, so the feeds() kind x kind matrix is structurally blind to a `writes_event_object` mutation feeding an event-object live read (CR 608.2h: the read uses the object's current information). On the same-event path all members share ONE live event object, so a member's write is observed by a sibling's live read => order-observable. Fix (close the feed in the classifier), mirroring the R3 HIGH-1 member-bound discriminator, DERIVED (not copied) from the batch-T1 event-object conjunct: - New `reads_and_writes_event_object() = reads_event_live && writes_event_object .any()`. The batch guard is a disjunction-of-negations (refusing the batch fast path only DEFERS to the feed rows); the same-event discriminator returns a PROMPT, so it fires only on a real feed = BOTH endpoints, a CONJUNCTION. - profiles_conflict: the same-event T1 fast-path source_independent branch gains `&& !(s.event_object_present && reads_and_writes_event_object())`, and a new discriminator `if s.same_event && s.event_object_present && reads_and_writes_event_object() { return true }` after the member-bound one. - Gated on `event_object_present` (mirrors effective_external): a write to a non-present event object no-ops (targeting.rs:951), so a Phase-mode trigger stays auto (no live object => no feed). A genuine feed always has a live event object, so the conjunct can only drop vacuous prompts (zero under-prompt risk) and converges with the batch path's net behavior. all_same_source stays auto (identical f over one shared event object, deterministic accumulation). Both edits are s.same_event-guarded => the batch path is byte-inert. The same-event arm's decision_old is always auto, so every flip is auto->prompt (over-prompt, never under-prompt). CR 603.3b + CR 608.2h (grep-verified). Full-DB parity sweep: +13 same-event flips, ALL conservative (0 genuine), predicate-keyed as `se_event_object_class` (membership DERIVED + emitted as the handoff evidence artifact, corpus-churn-robust). GOVERNING THEOREM (why R4 has no genuine member, unlike R3): the discriminator groups IDENTICAL siblings, and every R4 write targets the SHARED event object (identical f o f is relabel- invariant) or each copy's disjoint self => order-invariant final state. R3's genuine members wrote to PER-SOURCE storage (imprint pile A != pile B), an asymmetric destination that IS order-divergent. The class is a pure fail-closed safety over-prompt: sound but never necessary. `SAME_EVENT_EVENT_OBJECT_GENUINE` ships EMPTY (0 genuine, exact-set asserted). Class disjoint from `se_member_bound_class` by construction (asserted). Full per-card both-orders evidence in the review artifact. Module doc rewritten: the direct event-object feed is now CLOSED at both depths (same-event discriminator; batch freeze/T1). Two residuals REMAIN, both SYMMETRIC across depths: the source-actor granted-state channel and a board-wide external-write x event-live-read channel (feeds() blind, batch T1 keys on writes_event_object too). The gate is sound modulo these two profile-invisible symmetric residuals; the group_is_order_independent contract now records exactly that. Discriminating unit test `r4_same_event_event_object_feed_conjunct` (NEG prompts; read-only / write-only / event_object_present / all_same_source guards auto). Revert-to-red measured: disabling the discriminator turns the NEG red. Assisted-by: ClaudeCode:claude-opus-4.8 * test(engine): PR-6.75 caller-level regression for the same-event event-object discriminator (CR 603.3b) Maintainer review (#5072): the R4 event-object discriminator had only a predicate-level test (`r4_same_event_event_object_feed_conjunct`, ability_rw) that calls `profiles_conflict` on a hand-built profile + GroupStructure. That does not exercise the caller `group_is_order_independent`, which DERIVES `event_object_present` from the pending trigger's firing event (`extract_source_from_event`, triggers.rs) and threads it into the structure. A future caller that mis-threaded `event_object_present` (or dropped the same-event structure) could auto-order the R4 shape while the predicate test stayed green — the hot-path seam a prior loop regressed on. The existing caller-level regression (`high1_same_event_member_bound_prompts`) covers member-bound only. Adds `r4_same_event_event_object_prompts_at_caller`, mirroring the member-bound caller-level test. Two DISTINCT sources fire on ONE shared ETB event (object 99); each identical resolution READS that creature's LIVE power (`Power{EventSource}` => `reads_event_live`, CR 608.2h) and WRITES it (`PutCounter{TriggeringSource}` => `writes_event_object`) => a sibling's write feeds the other's live read => order-observable => the caller PROMPTS. The feed is `source_independent`, so it exercises BOTH profiler edits (the fast-path exclusion and the discriminator). Four guards prove the discriminator is not a blanket same-event prompt: - read-only (a live power read that gains life, no event-object write) => auto; - write-only (a fixed-count event-object write, no live read) => auto; - no-event-object (the SAME feed on a `Phase` event, `extract_source_from_event` => None => `event_object_present` threads FALSE) => auto — directly pinning the caller threading: the identical ability prompts WITH an event object and autos WITHOUT; - all_same_source (the feed on one shared source) => auto. Revert-fail (caller-level, measured): disabling the discriminator turns the NEG case red at the `group_is_order_independent` level. Test-only; no production change. Assisted-by: ClaudeCode:claude-opus-4.8 * fix(engine): adopt #5084's C2 ungating + close the surfaced Dodecapod under-prompt (CR 603.3b) Rebasing the PR-6.75 series onto `cf7cd7cb9` replayed over upstream #5084 (`perf(engine): batch identical self-counter triggers`), which — besides its resolution-time self-counter batching (orthogonal to this series' ordering gate) — removed the `is_on`/`loop_detection` parameter from `group_is_order_independent` and `trigger_events_match_for_ordering` and UNGATED the C2 distinct-event term (`loop_detection_on && c2_order_independent` -> `c2_order_independent`). Its rationale: loop detection governs optional infinite-combo shortcutting, not the finite CR 603.3b ordering UX. Adopting that ungating verbatim SURFACED a latent bug in this series: the coarse `ability_scan` C2 walker is blind to the source-actor residual (the CR 805.7 cause-source-controller channel, lifelink/deathtouch CR 702.15/702.2) that the precise `ability_rw` profiler catches only through the controller-uniformity gate. With C2 always-on, a C2-"clean" verdict could OVERRIDE that gate and auto-order a genuinely order-dependent Mixed-controller co-departure batch — an unsound under-prompt (the `condition1_dodecapod` witness). This was reachable in the series all along under `loop_detection = On`; the loop-OFF-only negatives never exercised it, and #5084's ungating made it always-on. Fix (precision dominates coarseness): the C2 term becomes `c2_order_independent && !batch_conflict` — C2 may auto-order a distinct-event group only when the precise batch profiler ALSO agrees it is conflict-clean, so C2 can never override the uniformity/batch gate. `batch_conflict` is already a parameter here; it defaults false for clean non-departure groups (so #5084's distinct-event fan-out still auto-orders) and is true for the Mixed-controller Dodecapod (so it prompts). This is NOT a semantic revert of #5084 — loop detection stays ungated; the `!batch_conflict` conjunct is a series-specific soundness gate keyed on machinery that did not exist upstream when #5084 was written. Also re-applies the mechanical ungating across the series' larger call surface (the C1 rebase conflict was resolved to this series' sound-gate version, which temporarily reverted #5084's ungating): - `trigger_events_match_for_ordering`: drop the `loop_detection_on` param. - `group_is_order_independent`: drop the `is_on` param (keep `state`). - `begin_trigger_ordering`: drop the `loop_detection_on` local and the arg. - all callers (production + the ~42 unit/parity-sweep test calls that passed `false`) drop the `is_on` argument. - `pr625_c2_distinct_event_gate_off_prompts_on_auto_resolves` is renamed to `pr625_c2_distinct_event_auto_orders_even_when_loop_detection_off` with its OFF arm now asserting auto-order (adopting #5084's flipped assertion). - doc/comments updated to the ungated framing. Gates: `condition1_dodecapod` and `b1_mindslicer` go from red (bare ungating) to green (with the conjunct) — the revert-to-red for the fix. `pr625_c2` stays green (#5084's behavior preserved). The full-DB ordering-parity sweep is unaffected — it drives `profiles_conflict`'s same-event and departure-batch arms, not the distinct-event C2 term — measured zero-movement at the rebased corpus. Assisted-by: ClaudeCode:claude-opus-4.8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.