Paused player_scope fan-out loses completed seats' per-player counts

[lgray]

🤖 AI text below 🤖

Summary

A player_scope fan-out that pauses mid-way for a player choice loses the per-player counts of every seat that already completed. Each resumed continuation leg replaces last_effect_counts_by_player instead of extending it, so after the fan-out finishes the table holds only the last resumed seat's entry.

This is pre-existing behaviour, surfaced during review of the Windfall draw-aggregate fix and disclosed there rather than repaired, because the repair is resume-machinery scope and affects every count-producing fan-out — not just the three cards that PR fixes.

Measured behaviour

Four seats, player_scope: All, Effect::Discard { count: 1, target: ScopedPlayer, selection: Chosen }, hands 1/2/1/1 so seat 1 must choose and the fan-out pauses there:

waiting_for                      = DiscardChoice { player: PlayerId(1), .. }
table at pause                   = [(0, 1)]
table after the continuation     = [(3, 1)]        <-- seat 0's real count is gone
last_effect_amount               = Some(1)         <-- an accumulating table gives 4

Seat 0 genuinely discarded a card. After the continuation resumes, nothing records it. Note the remaining seats chain into a SINGLE continuation leg, so seat 2's publication is replaced as well before the fan-out ends — the table is overwritten more than once, not merely truncated.

Mechanism

• install_previous_effect_counts_by_player's Some arm assigns the table outright:
  state.last_effect_counts_by_player = counts_by_player;
• split_player_scope_chain sets scoped.player_scope = None, so each resumed leg is a bare ability that publishes only its own single entry rather than re-entering the fan-out driver.

Both in crates/engine/src/game/effects/mod.rs.

Scope — this is not aggregate-specific

last_effect_amount is derived from the same table (counts_by_player.values().copied().sum()), so the scalar loses the same counts. The Sum class is affected identically, which is the measurement that establishes this as pre-existing rather than introduced by the aggregate work: the drain shape (LoseLife → GainLife { PreviousEffectAmount }, 38 of the 41 quantity-position corpus carriers) reads that same scalar.

Reachability

Reachable for the three cards the Windfall PR fixes (Windfall, Jace's Archivist, Whispering Madness). They take discard::resolve's forced whole-hand branch, which emits no DiscardChoice — but that branch still pauses when a discard replacement effect requires a choice, in crates/engine/src/game/effects/discard.rs, at a site that already documents a related gap:

// Known limitation: EffectResolved is not emitted when replacement choice interrupts forced-discard (same systemic gap as sacrifice).

Consequence for the Max class: the draw link's capture_clause_minimum_snapshot then freezes Max over a table containing only the last resumed discarder, so every player draws that one player's hand size instead of the greatest number discarded (CR 608.2h determines the count once, over the whole completed action).

Suggested repair

Make the per-clause table accumulate across continuation legs rather than be replaced — either by merging on the install path when resuming a leg of the same clause, or by deferring the install until the fan-out's final leg completes. Either way the fix must keep the CR 608.2c semantics of Some(empty) (a real zero-result producer that must replace a stale table) distinct from "a resumed leg contributing one more entry", which is the reason a naive merge is not obviously correct.

What is already fixed and NOT part of this issue

The adjacent off-by-one — the pausing seat being published as a zero contributor before it had answered its choice — was introduced and fixed in the Windfall PR (applied_domain_end = i, not i + 1), with a frozen discriminating test paused_fan_out_excludes_the_seat_that_has_not_answered_its_choice. Probe with i + 1 restored: left: [(0,1),(1,0)] / right: [(0,1)].

CR references

CR 608.2c (instructions followed in the order written; the carry-forward channel), CR 608.2h (an effect's value is determined only once), CR 608.2e (the fan-out is one action processed simultaneously).

[lgray]

🤖 AI text below 🤖

Surfaced during review of #7494 (the Windfall draw-aggregate fix), which discloses this in its Scope Expansion section rather than repairing it — the repair is resume-machinery scope and affects every count-producing fan-out, including the 38-card LoseLife → GainLife { PreviousEffectAmount } drain shape that reads the same scalar.

[lgray]

🤖 AI text below 🤖

Status: fix in flight on #7494.

This was originally filed for posterity while #7494 shipped the disclosure. That decision was reversed after maintainer review requested the repair rather than the disclosure, so the continuation-safe accumulation is being implemented on #7494 itself — carry the in-flight per-clause table across the pause, merge on resume instead of the outright assign in install_previous_effect_counts_by_player's Some arm, a final-leg marker on the abilities split_player_scope_chain produces, and Some(empty) keeping its CR 608.2c replace semantics.

The "Suggested repair" section above stands as written, including the reason a naive merge is not obviously correct — that is going into the implementation plan as a named hazard with its own pinning test.

This issue stays open until #7494 merges.

[lgray]

🤖 AI text below 🤖

Correction: I overstated which cards this defect reaches

Planning the repair turned up a measurement that contradicts something I asserted earlier, so I am correcting it before it propagates into the fix.

What I said: that last_effect_amount is derived from the truncated table, so the Sum class loses the same counts "including the drain shape (LoseLife → GainLife { PreviousEffectAmount }), 38 of the 41 quantity-position carriers in the corpus."

What is actually true. previous_effect_counts_by_player_from_events (crates/engine/src/game/effects/mod.rs) returns Some for exactly three effect kinds:

let kind = match effect {
    Effect::Discard { .. } | Effect::DiscardCard { .. } | Effect::ChangeZoneAll { .. } => {
        EffectKind::from(effect)
    }
    _ => return None,
};

Effect::LoseLife is not among them. So the drain shape never publishes a per-player table at all — it takes the None arm, and its scalar comes from the previous_effect_amount_from_events fallback rather than from .values().sum() over the table. Those 38 carriers are not affected by the truncation. My "38 of 41" figure was a corpus census of which cards carry the reference, silently reused as if it were a census of which cards the defect reaches. Those are different questions and I did not check that they had the same answer.

What survives the correction. The defect is still not aggregate-specific, and that was the load-bearing part: a Sum consumer whose producer is one of the three table-publishing kinds does read a table-derived scalar and does lose counts across a pause, because install_previous_effect_counts_by_player's Some arm stamps last_effect_amount from the table it is assigning. So the repair still belongs at the last_effect_counts_by_player authority rather than in the aggregate reduction, which is why the shape of the fix is unchanged.

The affected population is "consumers fed by a table-publishing producer", not "the drain class". Narrower than I claimed, and in a different place.