Validate all groups before mutating any in grouped append_move/append_event

doc/tutorials/advanced_tutorials/h_selector_prototype.ipynb

@@ -132,7 +132,7 @@
     }
    },
    "outputs": [],
-   "source": "CARD_VALUE = {\"J\": 0, \"Q\": 1, \"K\": 2}\ncards = list(CARD_VALUE)\n\ng = gbt.ExtensiveGame(players=[\"Alice\", \"Bob\"], title=\"Kuhn poker\")\ng.append_event(H.path(), dict.fromkeys(cards, gbt.Rational(1, 3)))\nfor c in cards:\n    remaining = [x for x in cards if x != c]\n    g.append_event(H.path(c), dict.fromkeys(remaining, gbt.Rational(1, 2)))\n\nalice_partition = H.path(...).by(lambda h: h[0].action).by_last_action(\"Alice\")\ng.append_move(alice_partition.plays, \"Alice\", [\"Check\", \"Bet\"])\n\nbob_partition = H.path(..., ...).by(lambda h: h[1].action)\ng.append_move(bob_partition.plays.after(\"Check\"), \"Bob\", [\"Check\", \"Bet\"])\n\ng.append_move(alice_partition.plays.after(\"Check\", \"Bet\"), \"Alice\", [\"Fold\", \"Call\"])\ng.append_move(bob_partition.plays.after(\"Bet\"), \"Bob\", [\"Fold\", \"Call\"])\n\ndraw(g)\nprint(\"is_perfect_recall:\", g.is_perfect_recall)"
+   "source": "CARD_VALUE = {\"J\": 0, \"Q\": 1, \"K\": 2}\ncards = list(CARD_VALUE)\n\ng = gbt.ExtensiveGame(players=[\"Alice\", \"Bob\"], title=\"Kuhn poker\")\ng.append_event(H.path(), dict.fromkeys(cards, gbt.Rational(1, 3)))\nfor c in cards:\n    remaining = [x for x in cards if x != c]\n    g.append_event(H.path(c), dict.fromkeys(remaining, gbt.Rational(1, 2)))\n\nalice_partition = H.path(...).by(lambda h: h[0].action).by_last_action(\"Alice\")\ng.append_move(alice_partition.plays, \"Alice\", [\"Check\", \"Bet\"])\n\nbob_partition = H.path(..., ...).by(lambda h: h[1].action)\ng.append_move(bob_partition.plays.after(\"Check\"), \"Bob\", [\"Check\", \"Bet\"])\n\n# Alice's second decision only exists on the Check, Bet branch -- the group\n# keyed by her having Bet directly is empty by construction (Bob decides next\n# there instead), and `append_move` now raises on any empty group rather than\n# skipping it, so we materialize the groups ourselves and skip the empty one.\nfor _key, histories in g._get_groups(alice_partition.plays.after(\"Check\", \"Bet\")).items():\n    if histories:\n        g.append_move(selector_for_histories(histories), \"Alice\", [\"Fold\", \"Call\"])\ng.append_move(bob_partition.plays.after(\"Bet\"), \"Bob\", [\"Fold\", \"Call\"])\n\ndraw(g)\nprint(\"is_perfect_recall:\", g.is_perfect_recall)"
   },
   {
    "cell_type": "code",

src/pygambit/gametree.pxi

@@ -797,6 +797,19 @@ class ExtensiveGame(Game):
             raise ValueError(f"{funcname}(): Each node must be referenced only once")
         return resolved_nodes
 
+    @cython.cfunc
+    def _resolve_move_nodes(self, nodes: typing.Any, funcname: str) -> list[Node]:
+        """Internal: resolve `nodes` to a nonempty set of distinct terminal nodes,
+        without mutating the game -- shared by `append_move` and `append_event` to
+        validate every group of a `GroupedSelector` before any of them is mutated,
+        so a later group's failure can never leave an earlier group's mutation
+        applied.  An empty group therefore raises, the same as an empty selector.
+        """
+        resolved_nodes = self._resolve_nodes(nodes, funcname, "nodes")
+        if any(not cython.cast(Node, n)._is_terminal() for n in resolved_nodes):
+            raise UndefinedOperationError(f"{funcname}(): `nodes` must be terminal nodes")
+        return resolved_nodes
+
     @cython.cfunc
     def _resolve_infoset(self,
                          infoset: Selector, funcname: str, argname: str = "infoset") -> Node:
@@ -962,10 +975,12 @@ class ExtensiveGame(Game):
             an empty or a duplicated label.
         """
         if isinstance(nodes, GroupedSelector):
-            for group in self._group_nodes(nodes).values():
-                if not group:
-                    continue
-                self._append_move_at(group, player, actions)
+            resolved_groups = [
+                self._resolve_move_nodes(group, "append_move")
+                for group in self._group_nodes(nodes).values()
+            ]
+            for resolved_nodes in resolved_groups:
+                self._append_move_at(resolved_nodes, player, actions)
             return
         if not isinstance(nodes, Selector):
             raise TypeError(
@@ -1096,10 +1111,12 @@ class ExtensiveGame(Game):
             summing to exactly one.
         """
         if isinstance(nodes, GroupedSelector):
-            for group in self._group_nodes(nodes).values():
-                if not group:
-                    continue
-                self._append_event_at(group, actions)
+            resolved_groups = [
+                self._resolve_move_nodes(group, "append_event")
+                for group in self._group_nodes(nodes).values()
+            ]
+            for resolved_nodes in resolved_groups:
+                self._append_event_at(resolved_nodes, actions)
             return
         if not isinstance(nodes, Selector):
             raise TypeError(

tests/test_hsel.py

@@ -208,3 +208,31 @@ def test_selector_deduplicates_plays():
     plays = game.get_histories(gbt.H.after("go").plays)
     assert {h.actions for h in plays} == {("go", "go"), ("go", "stay")}
     assert len(plays) == 2, f"expected 2 unique plays, got {len(plays)}: {plays}"
+
+
+def test_grouped_mutation_raises_on_empty_group():
+    # A post-op (`.after`) can filter one group's members down to none while
+    # leaving others nonempty. That must raise, the same as an ungrouped
+    # selector matching no nodes, rather than being silently skipped.
+    game = gbt.ExtensiveGame(players=["A", "B"])
+    game.append_move(gbt.H.path(), "A", ["U", "D"])
+    game.append_move(gbt.H.path("U"), "B", ["x", "y"])
+    game.append_move(gbt.H.path("D"), "B", ["z", "w"])
+
+    grouped = gbt.H.plays.by(lambda h: h[:].actions[0]).after("x", "z")
+    with pytest.raises(ValueError):
+        game.append_move(grouped, "A", ["p", "q"])
+
+
+def test_grouped_mutation_validates_all_groups_before_mutating_any():
+    # If a later group fails validation, an earlier group must not have been
+    # mutated -- grouped mutation calls validate every group before applying
+    # any of them.
+    game = gbt.ExtensiveGame(players=["A"])
+    game.append_move(gbt.H.path(), "A", ["U", "D"])
+    game.append_move(gbt.H.path("U"), "A", ["x", "y"])  # U-group node no longer terminal
+
+    grouped = gbt.H.path(...).by(lambda h: h[:].actions[0])
+    with pytest.raises(gbt.UndefinedOperationError):
+        game.append_move(grouped, "A", ["m", "n"])
+    assert game.get_actions(gbt.H.path("D")) == []