feat: robustness in Module 5 (nesting guard, Death Phase limit, well-formed Assemble, multi-summon order)

A trigger is never queued while that same (minion, trigger) is resolving;
more than DEATH_PHASE_LIMIT = 64 Death Phases is an error ('Death Phase
limit'); Assemble refuses and never offers ill-formed text (wellFormed /
well_formed); summon count > 1 places tokens in summon order. Modules 05-10,
both tracks. Four new Module 5 tests, a tools test that every shipped card
is well-formed, lessons 2, 3, 5, 13 and harness.md updated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

course/07-rule-resolution/reference/rust/src/engine/assemble.rs

@@ -10,7 +10,7 @@
 
 use std::fmt;
 
-use super::card::{CardDef, PoolFilter};
+use super::card::{well_formed, CardDef, PoolFilter};
 use super::world::{EntityId, PlayerId, World, Zone};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -30,19 +30,33 @@ impl Half {
     }
 }
 
-/// Assembling `base` with `donor`'s `half` would leave the base's text unchanged.
+/// Why an Assemble was refused.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum AssembleFailure {
+    /// The new text would equal the base's.
+    Unchanged,
+    /// The new text would not be well-formed (see `well_formed`).
+    IllFormed,
+}
+
+/// Assembling `base` with `donor`'s `half` would leave the base's text unchanged, or ill-formed.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct AssembleError {
     pub base: String,
     pub donor: String,
     pub half: Half,
+    pub reason: AssembleFailure,
 }
 
 impl fmt::Display for AssembleError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let result = match self.reason {
+            AssembleFailure::Unchanged => "leaves its text unchanged",
+            AssembleFailure::IllFormed => "gives ill-formed text",
+        };
         write!(
             f,
-            "assembling {:?} with the {} half of {:?} leaves its text unchanged",
+            "assembling {:?} with the {} half of {:?} {result}",
             self.base,
             self.half.label(),
             self.donor
@@ -62,12 +76,17 @@ pub fn assemble(base: &CardDef, donor: &CardDef, half: Half) -> Result<CardDef,
         Half::Bottom => (base_top, donor_bottom),
     };
     let text = [top, bottom].concat();
+    let refuse = |reason| AssembleError {
+        base: base.name.clone(),
+        donor: donor.name.clone(),
+        half,
+        reason,
+    };
     if text == base.text {
-        return Err(AssembleError {
-            base: base.name.clone(),
-            donor: donor.name.clone(),
-            half,
-        });
+        return Err(refuse(AssembleFailure::Unchanged));
+    }
+    if !well_formed(&text, base.card_type) {
+        return Err(refuse(AssembleFailure::IllFormed));
     }
     Ok(CardDef {
         name: format!("{} (Assembled)", base.name),
@@ -92,9 +111,11 @@ pub fn register_assembled(world: &mut World, mut def: CardDef) -> String {
 }
 
 /// The `assemble` effect: the owner's chooser picks a base minion from the pool
-/// (filtered), a donor that changes its text, and the half to replace. The
-/// Assembled minion goes to the owner's hand. Real Hearthstone picks the base at
-/// random and offers two donors; here every choice is the chooser's.
+/// (filtered), a donor, and the half to replace. Only combinations that change the
+/// base's text and leave it well-formed are offered: a donor is offered if one of its
+/// halves gives such a text, and a base if it has such a donor. The Assembled minion
+/// goes to the owner's hand. Real Hearthstone picks the base at random and offers two
+/// donors; here every choice is the chooser's.
 pub fn assemble_from_pool(
     world: &mut World,
     player: PlayerId,
@@ -111,9 +132,18 @@ pub fn assemble_from_pool(
             )
         })
         .collect();
+    // Does assembling `base` with `donor`'s `half` give a new, well-formed text?
+    let changes = |base: &CardDef, donor: &CardDef, half: Half| assemble(base, donor, half).is_ok();
+    let donors_for = |base: &CardDef| -> Vec<String> {
+        pool.iter()
+            .map(|(d, _)| d)
+            .filter(|d| d.name != base.name && Half::BOTH.iter().any(|&h| changes(base, d, h)))
+            .map(|d| d.name.clone())
+            .collect()
+    };
     let bases: Vec<String> = pool
         .iter()
-        .filter(|(_, allowed)| *allowed)
+        .filter(|(d, allowed)| *allowed && !donors_for(d).is_empty())
         .map(|(d, _)| d.name.clone())
         .collect();
     if bases.is_empty() {
@@ -122,22 +152,12 @@ pub fn assemble_from_pool(
     let base_name = world.choose("base", &bases);
     let base = world.card(&base_name).def.clone();
 
-    let changes = |donor: &CardDef, half: Half| assemble(&base, donor, half).is_ok();
-    let donors: Vec<String> = pool
-        .iter()
-        .map(|(d, _)| d)
-        .filter(|d| d.name != base.name && Half::BOTH.iter().any(|&h| changes(d, h)))
-        .map(|d| d.name.clone())
-        .collect();
-    if donors.is_empty() {
-        return None;
-    }
-    let donor_name = world.choose("donor", &donors);
+    let donor_name = world.choose("donor", &donors_for(&base));
     let donor = world.card(&donor_name).def.clone();
 
     let halves: Vec<String> = Half::BOTH
         .iter()
-        .filter(|&&h| changes(&donor, h))
+        .filter(|&&h| changes(&base, &donor, h))
         .map(|h| h.label().to_string())
         .collect();
     let half = world.choose("half", &halves);
@@ -147,7 +167,8 @@ pub fn assemble_from_pool(
         Half::Bottom
     };
 
-    let def = assemble(&base, &donor, half).expect("only halves that change the text are offered");
+    let def = assemble(&base, &donor, half)
+        .expect("only halves that give a new, well-formed text are offered");
     let name = register_assembled(world, def);
     Some(world.create(&name, player, Zone::Hand))
 }

course/07-rule-resolution/reference/rust/src/engine/card.rs

@@ -258,6 +258,34 @@ impl Card {
     }
 }
 
+/// Does `text` mean what it says? For a minion, every effect clause belongs to a
+/// trigger (none comes before the first trigger, or after an aura or keyword), and
+/// every trigger has at least one effect before the next trigger, aura, keyword or
+/// the end. The rule is for minions (Assemble makes only minions): a spell passes.
+pub fn well_formed(text: &[Clause], card_type: CardType) -> bool {
+    if card_type != CardType::Minion {
+        return true;
+    }
+    // The trigger whose effects are being listed: none, one with no effect yet, or one with some.
+    #[derive(PartialEq)]
+    enum Open {
+        None,
+        Empty,
+        Filled,
+    }
+    let mut open = Open::None;
+    for clause in text {
+        match clause {
+            Clause::Effect { .. } if open == Open::None => return false,
+            Clause::Effect { .. } => open = Open::Filled,
+            _ if open == Open::Empty => return false,
+            Clause::Trigger { .. } => open = Open::Empty,
+            Clause::Aura { .. } | Clause::Keyword { .. } => open = Open::None,
+        }
+    }
+    open != Open::Empty
+}
+
 /// Every card the game can create, looked up by name.
 #[derive(Debug, Default)]
 pub struct CardLibrary {

course/07-rule-resolution/reference/rust/src/engine/effect.rs

@@ -20,13 +20,17 @@ pub fn actions(effect: &Effect, ctx: &Ctx) -> Vec<Action> {
         Effect::Draw { count } => (0..*count)
             .map(|_| Action::Draw { player: owner })
             .collect(),
-        Effect::Summon { card, count } => (0..*count)
-            .map(|_| Action::Summon {
-                card: card.clone(),
-                player: owner,
-                at: summon_position(ctx),
-            })
-            .collect(),
+        // Several tokens stand left to right in summon order: each goes right of the one before.
+        Effect::Summon { card, count } => {
+            let at = summon_position(ctx);
+            (0..*count)
+                .map(|i| Action::Summon {
+                    card: card.clone(),
+                    player: owner,
+                    at: at.map(|at| at + i as usize),
+                })
+                .collect()
+        }
         Effect::GainArmor { amount } => vec![Action::GainArmor {
             player: owner,
             amount: *amount,

course/07-rule-resolution/reference/rust/src/engine/mod.rs

@@ -18,7 +18,9 @@
 //!
 //! Module 5: behaviour as data. Cards are definitions in the course's card format,
 //! loaded from JSON; one interpreter turns their effects into actions. Assemble
-//! builds a new card by swapping half of one card's text for half of another's.
+//! builds a new card by swapping half of one card's text for half of another's, and
+//! offers only results that are well-formed. A Sequence is bounded: a trigger never
+//! nests inside itself, and more than `DEATH_PHASE_LIMIT` Death Phases is an error.
 //!
 //! Part II lives in `puzzle`: a second engine, for the rule-writing puzzle game,
 //! beside the first.
@@ -35,11 +37,11 @@ pub mod puzzle;
 mod queue;
 mod world;
 
-pub use assemble::{assemble, register_assembled, AssembleError, Half};
+pub use assemble::{assemble, register_assembled, AssembleError, AssembleFailure, Half};
 pub use card::{
-    Aura, Card, CardDef, CardLibrary, CardType, Clause, Effect, PoolFilter, RedirectFrom,
-    RedirectTo, Stacking, Target, Trigger, TriggerOn,
+    well_formed, Aura, Card, CardDef, CardLibrary, CardType, Clause, Effect, PoolFilter,
+    RedirectFrom, RedirectTo, Stacking, Target, Trigger, TriggerOn,
 };
 pub use event::Event;
-pub use queue::{run_sequence, summon, Action, Ctx, DamageStep, DAMAGE_STEPS};
+pub use queue::{run_sequence, summon, Action, Ctx, DamageStep, DAMAGE_STEPS, DEATH_PHASE_LIMIT};
 pub use world::{Chooser, EntityId, PlayerId, World, Zone, HERO_HEALTH};

course/07-rule-resolution/reference/rust/src/engine/queue.rs

@@ -20,6 +20,11 @@
 //! Module 5: a card's effects are data. Each one is interpreted (`effect.rs`) and
 //! its actions performed before the next effect of the same ability is read, so
 //! "Assemble a minion. Give all minions in your hand +2/+2." buffs the new minion.
+//!
+//! Module 5, robustness: once a test can write any card as data, one card can make a
+//! Sequence endless. A trigger is never queued while that same trigger of that same
+//! minion is resolving (the nesting guard in `react`), and a Sequence that needs more
+//! than `DEATH_PHASE_LIMIT` Death Phases is an error.
 
 use super::assemble::assemble_from_pool;
 use super::aura::{activations, aura_update_other, redirect_for};
@@ -121,6 +126,10 @@ impl Ctx<'_> {
     }
 }
 
+/// The most Death Phases one Sequence may have. One more is an error, not a cut-off:
+/// a Sequence that stopped early would resolve differently without saying so.
+pub const DEATH_PHASE_LIMIT: u32 = 64;
+
 /// Run one Sequence: a player action and everything it causes.
 pub fn run_sequence(world: &mut World, action: Action) {
     perform(world, action);
@@ -130,6 +139,11 @@ pub fn run_sequence(world: &mut World, action: Action) {
     let mut round = 0;
     while !deaths.is_empty() {
         round += 1;
+        // Between Death Phases no trigger is resolving, so the panic leaves no block open.
+        assert!(
+            round <= DEATH_PHASE_LIMIT,
+            "Death Phase limit: more than {DEATH_PHASE_LIMIT} Death Phases in one Sequence"
+        );
         world.record(Event::DeathPhase { round });
         react(world, &deaths);
         deaths = end_of_outermost_phase(world);
@@ -268,18 +282,27 @@ fn apply_damage(world: &mut World, target: EntityId, amount: i32) -> Event {
 /// A Phase: queue every trigger that responds to `events`, then resolve them one by one.
 fn react(world: &mut World, events: &[Event]) {
     // Fill the Queue: for each event (already in order of play), each listener in order of play.
+    // The nesting guard: a trigger that is resolving right now is not queued again, so
+    // a trigger is never nested inside itself.
     let mut queue = Vec::new();
     for event in events {
         for listener in listeners(world, event) {
-            for trigger in &world.card(world.card_name(listener)).triggers {
-                if responds(world, trigger.on, listener, event) {
-                    queue.push((listener, trigger.clone(), event));
+            for (index, trigger) in world
+                .card(world.card_name(listener))
+                .triggers
+                .iter()
+                .enumerate()
+            {
+                if responds(world, trigger.on, listener, event)
+                    && !world.is_resolving(listener, index)
+                {
+                    queue.push((listener, index, trigger.clone(), event));
                 }
             }
         }
     }
     // The Queue is now locked: nothing that happens below adds to it.
-    for (source, trigger, event) in queue {
+    for (source, index, trigger, event) in queue {
         // The interceptor: the entry was queued once; the auras in force when it
         // starts decide how many times it activates, back to back.
         let times = activations(world, world.owner_of(source), trigger.on);
@@ -295,7 +318,7 @@ fn react(world: &mut World, events: &[Event]) {
                 _ => world.board_index(source),
             };
             // The trigger's consequences are one level deeper in the log tree.
-            world.open_block();
+            world.open_block(source, index);
             let owner = world.owner_of(source);
             run_effects(world, &trigger.effects, owner, Some(source), None, position);
             world.close_block();

course/07-rule-resolution/reference/rust/src/engine/world.rs

@@ -87,8 +87,10 @@ pub struct World {
     log: Vec<Event>,
     /// The nesting depth at which each event in `log` was recorded, index for index.
     depths: Vec<u32>,
-    /// How many triggers are resolving right now, one inside another.
-    depth: u32,
+    /// The triggers resolving right now, one inside another, outermost first: each is a
+    /// minion and the index of the trigger in its card's `triggers`. How many there are
+    /// is the nesting depth of the log tree.
+    resolving: Vec<(EntityId, usize)>,
     /// The auras in force as of the last Aura Update. Changes only there (see `aura.rs`).
     auras: Vec<ActiveAura>,
 }
@@ -105,7 +107,7 @@ impl World {
             next_order_of_play: 0,
             log: Vec::new(),
             depths: Vec::new(),
-            depth: 0,
+            resolving: Vec::new(),
             auras: Vec::new(),
         };
         for owner in 0..2 {
@@ -304,17 +306,23 @@ impl World {
 
     pub fn record(&mut self, event: Event) {
         self.log.push(event);
-        self.depths.push(self.depth);
+        self.depths.push(self.resolving.len() as u32);
     }
 
-    /// A trigger starts resolving: the events it causes are recorded one level deeper.
-    pub fn open_block(&mut self) {
-        self.depth += 1;
+    /// Trigger number `trigger` of `source` starts resolving: the events it causes are
+    /// recorded one level deeper.
+    pub fn open_block(&mut self, source: EntityId, trigger: usize) {
+        self.resolving.push((source, trigger));
     }
 
     /// The trigger that last opened a block has finished resolving.
     pub fn close_block(&mut self) {
-        self.depth -= 1;
+        self.resolving.pop();
+    }
+
+    /// Is trigger number `trigger` of `source` resolving right now (at any depth)?
+    pub fn is_resolving(&self, source: EntityId, trigger: usize) -> bool {
+        self.resolving.contains(&(source, trigger))
     }
 
     /// Add a card to the library, or replace the card with the same name.