fix(engine): keep Portent exile picks out of the graveyard (#6498)

PR_6498.md

@@ -0,0 +1,24 @@
+Closes #6498
+
+## Summary
+
+Discord report: **Portent of Calamity** did not let you keep selected revealed cards — they went to the graveyard.
+
+Oracle: `Reveal the top X cards … For each card type, you may exile a card of that type from among them. Put the rest into your graveyard. …`
+
+## Root cause
+
+1. **"Put the rest into your graveyard"** was parsed as `ChangeZoneAll { Exile → Graveyard, TrackedSet }`. After `ForEachCategory` exile, the tracked set is the *exiled* picks — so selecting a card immediately dumped it to the graveyard.
+2. Dig→`RevealTop` demotion collapsed non-fixed counts (X) to `1`.
+
+## Changes
+
+- Keep dynamic-count reveal Digs as reveal-only peeks (`keep_count: 0`) instead of collapsing X to `RevealTop { count: 1 }`.
+- After `ForEachCategory` exile, "put the rest" emits `ChangeZoneAll { Library → Graveyard, LastRevealed }` so only non-exiled revealed cards go to the graveyard.
+- Publish `CardsRevealed` from reveal-only Digs into the chain tracked-set path; `ForEachCategory` pool falls back to `last_revealed_ids`.
+
+## Test plan
+
+- [x] `cargo test -p engine --lib -- portent`
+- [x] `cargo test -p engine --test integration -- portent`
+- [ ] CI green

crates/engine/src/game/effects/choose_from_zone.rs

@@ -556,6 +556,12 @@ fn resolve_category_pool(state: &GameState, ability: &ResolvedAbility) -> Vec<Ob
             crate::game::targeting::latest_tracked_set_id(state)
                 .and_then(|id| state.tracked_object_sets.get(&id).cloned())
         })
+        // CR 701.20b + CR 608.2c: Reveal-only Dig / RevealTop may leave the
+        // revealed pile only in `last_revealed_ids` when no TrackedSet consumer
+        // forced publication (Portent of Calamity: Dig → ForEachCategory →
+        // LastRevealed rest-move). Prefer the live reveal window over an empty
+        // ability-target fallback.
+        .or_else(|| (!state.last_revealed_ids.is_empty()).then(|| state.last_revealed_ids.clone()))
         .unwrap_or_else(|| {
             ability
                 .targets

crates/engine/src/game/effects/dig.rs

@@ -141,21 +141,38 @@ pub fn resolve(
         .collect::<Vec<_>>();
     let raw_keep_count = raw_keep_num.min(cards.len());
 
-    // CR 701.20e: Pure-peek pattern (keep_count = 0): "look at the top card" with no
-    // player selection — the sub_ability condition decides whether to take it. Set
-    // last_revealed_ids so RevealedHasCardType can evaluate, then return without
-    // creating a DigChoice interaction.
-    if raw_keep_count == 0 && !is_reveal {
+    // CR 701.20e / CR 701.20a: Pure-peek pattern (keep_count = 0): "look at" /
+    // "reveal the top N" with no player selection on this step — a following
+    // ForEachCategory / LastRevealed move decides disposition (Portent of
+    // Calamity). Set last_revealed_ids (and emit CardsRevealed for public
+    // reveals) then return without creating a DigChoice interaction.
+    if raw_keep_count == 0 {
         state.last_revealed_ids = cards.clone();
-        // CR 701.20e: "look at" privately reveals the cards to the looking
-        // player. The looker is the ability controller (e.g. Delver of Secrets'
-        // "look at the top card of your library"). Record the looker-scoped peek
-        // window so `filter_state_for_viewer` keeps these cards visible to the
-        // looker — and only the looker — through any subsequent "you may reveal
-        // that card" optional decision, instead of leaving the looking player to
-        // choose blind.
-        state.private_look_ids = cards.clone();
-        state.private_look_player = Some(ability.controller);
+        if is_reveal {
+            // CR 701.20a: public reveal — show to all players.
+            for &card_id in &cards {
+                state.revealed_cards.insert(card_id);
+            }
+            let card_names: Vec<String> = cards
+                .iter()
+                .filter_map(|id| state.objects.get(id).map(|o| o.name.clone()))
+                .collect();
+            events.push(GameEvent::CardsRevealed {
+                player: ability.controller,
+                card_ids: cards.clone(),
+                card_names,
+            });
+        } else {
+            // CR 701.20e: "look at" privately reveals the cards to the looking
+            // player. The looker is the ability controller (e.g. Delver of Secrets'
+            // "look at the top card of your library"). Record the looker-scoped peek
+            // window so `filter_state_for_viewer` keeps these cards visible to the
+            // looker — and only the looker — through any subsequent "you may reveal
+            // that card" optional decision, instead of leaving the looking player to
+            // choose blind.
+            state.private_look_ids = cards.clone();
+            state.private_look_player = Some(ability.controller);
+        }
         events.push(GameEvent::EffectResolved {
             kind: EffectKind::from(&ability.effect),
             source_id: ability.source_id,

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

@@ -4742,8 +4742,17 @@ fn affected_objects_from_events(
         // CR 701.20b + CR 608.2c: Reveal instructions do not move cards, so they
         // emit `CardsRevealed` rather than `ZoneChanged`. Publish the revealed
         // card ids for downstream "from among the revealed cards"
-        // `ChooseFromZone` continuations (Atraxa, Grand Unifier class).
-        Effect::RevealTop { .. } | Effect::RevealHand { .. } | Effect::Clash => events
+        // `ChooseFromZone` / `ForEachCategory` continuations (Atraxa, Portent of
+        // Calamity). Reveal-only Digs with `keep_count: 0` take the same path —
+        // they never emit ZoneChanged either.
+        Effect::RevealTop { .. }
+        | Effect::RevealHand { .. }
+        | Effect::Clash
+        | Effect::Dig {
+            reveal: true,
+            keep_count: Some(0),
+            ..
+        } => events
             .iter()
             .filter_map(|event| match event {
                 GameEvent::CardsRevealed { card_ids, .. } => Some(card_ids.as_slice()),

crates/engine/src/parser/oracle_effect/assembly.rs

@@ -2302,6 +2302,11 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition {
     // continuation patched it. An unpatched Dig { reveal: true, keep_count: None, filter: Any }
     // is a simple "reveal the top N" with no player selection — it must resolve synchronously
     // (via RevealTop) so that sub_ability chains like RevealedHasCardType evaluate inline.
+    //
+    // CR 107.3 + CR 701.20a: Dynamic counts (Portent of Calamity's "top X cards") cannot
+    // round-trip through `RevealTop { count: u32 }` without collapsing to 1. Keep those
+    // Digs as reveal-only peeks (`keep_count: 0`) so X resolves at runtime; a later
+    // ForEachCategory / LastRevealed rest-move consumes the revealed pool.
     for def in &mut defs {
         if let Effect::Dig {
             count,
@@ -2317,14 +2322,27 @@ pub(crate) fn assemble_effect_chain(ir: &EffectChainIr) -> AbilityDefinition {
             if destination == &Some(Zone::Library) && rest_destination == &Some(Zone::Library) {
                 continue;
             }
-            let count_val = match count {
-                QuantityExpr::Fixed { value } => *value as u32,
-                _ => 1,
-            };
-            *def.effect = Effect::RevealTop {
-                player: player.clone(),
-                count: count_val,
-            };
+            match count {
+                QuantityExpr::Fixed { value } => {
+                    *def.effect = Effect::RevealTop {
+                        player: player.clone(),
+                        count: *value as u32,
+                    };
+                }
+                _ => {
+                    if let Effect::Dig {
+                        keep_count,
+                        destination,
+                        rest_destination,
+                        ..
+                    } = &mut *def.effect
+                    {
+                        *keep_count = Some(0);
+                        *destination = None;
+                        *rest_destination = None;
+                    }
+                }
+            }
         }
     }
 

crates/engine/src/parser/oracle_effect/sequence.rs

@@ -3975,6 +3975,40 @@ pub(super) fn apply_clause_continuation(
             destination,
             reorder_all,
         } => {
+            // CR 608.2c + CR 701.20b (Portent of Calamity): After a per-category
+            // exile from among revealed cards, "put the rest into <zone>" moves
+            // the revealed cards still in the library — `LastRevealed` ∩ origin
+            // Library — NOT Dig.rest_destination (a keep_count-0 reveal Dig
+            // returns before applying rest) and NOT the chain tracked set of
+            // cards just exiled (which would dump the player's picks into the
+            // graveyard). Prefer this over Dig patching when both antecedents
+            // exist in the clause list.
+            let for_each_bound = defs.iter().rposition(|def| {
+                matches!(
+                    &*def.effect,
+                    Effect::ForEachCategory {
+                        action: ForEachCategoryAction::ExileFromPool { .. },
+                        ..
+                    }
+                )
+            });
+            if for_each_bound.is_some() {
+                defs.push(AbilityDefinition::new(
+                    kind,
+                    Effect::ChangeZoneAll {
+                        origin: Some(Zone::Library),
+                        destination,
+                        target: TargetFilter::LastRevealed,
+                        enters_under: None,
+                        enter_tapped: crate::types::zones::EtbTapState::Unspecified,
+                        enter_with_counters: vec![],
+                        face_down_profile: None,
+                        library_position: None,
+                        random_order: false,
+                    },
+                ));
+                return;
+            }
             // Absorbed into preceding Dig or RevealUntil — sets rest_destination
             // for unchosen/non-matching cards. CR 608.2c: When the preceding def is
             // a conditional "instead" alternative (new def with `else_ability =
@@ -3995,10 +4029,13 @@ pub(super) fn apply_clause_continuation(
                 None,
                 super::assembly::OnMiss::Ignore,
             );
-            let Some(bound_index) = bound else {
-                return;
-            };
-            patch_rest_destination_recursively(&mut defs[bound_index], destination, reorder_all);
+            if let Some(bound_index) = bound {
+                patch_rest_destination_recursively(
+                    &mut defs[bound_index],
+                    destination,
+                    reorder_all,
+                );
+            }
         }
         ContinuationAst::DigFromAmong {
             quantity,
@@ -6615,6 +6652,35 @@ pub(super) fn parse_followup_continuation_ast(
                 reorder_all: false,
             })
         }
+        // CR 608.2c + CR 701.20b (Portent of Calamity / Sanar class): "Put the
+        // rest into your graveyard" after a per-category exile from among the
+        // revealed cards. The rest are the revealed cards still in the library
+        // (not the cards just exiled into the chain tracked set).
+        Effect::ForEachCategory {
+            action: ForEachCategoryAction::ExileFromPool { .. },
+            ..
+        } if nom_primitives::scan_contains(&lower, "put the rest") =>
+        {
+            let destination = if nom_primitives::scan_contains(&lower, "into your graveyard")
+                || nom_primitives::scan_contains(&lower, "into their graveyard")
+            {
+                Zone::Graveyard
+            } else if nom_primitives::scan_contains(&lower, "into your hand")
+                || nom_primitives::scan_contains(&lower, "into their hand")
+            {
+                Zone::Hand
+            } else if nom_primitives::scan_contains(&lower, "on the bottom")
+                || nom_primitives::scan_contains(&lower, "on top of")
+            {
+                Zone::Library
+            } else {
+                Zone::Graveyard
+            };
+            Some(ContinuationAst::PutRest {
+                destination,
+                reorder_all: false,
+            })
+        }
         // CR 701.20a + CR 608.2c: A reveal-until rest-pile clause may be
         // separated from the RevealUntil by a transparent intervening effect
         // ("~ deals damage equal to that card's mana value. Put that card into