Assert parent selection where the cost scenarios actually specify it

Four scenarios exist to test cost-based parent selection and each named
its expected outcome in a comment that nothing read. Add a tree_parents
assertion mapping a node to the parent it must have in the final tree
snapshot, and encode it for the two scenarios whose stated outcome the
implementation meets: cost-avoidance (n04 takes the fiber n03) and
bottleneck-parent (n06 takes the fiber n03, n09 keeps its only parent
n05). Both hold in all six provably-completed archived runs.

The other two are left unasserted on purpose, and each for a different
reason worth keeping distinct.

mixed-technology says n06 and n08 should both pick the fiber parent n03.
They do not. Across the five completed archived runs n06 picks n10 three
times and n08 picks n04 four times. Encoding the criterion as written
would red the scenario most runs, and encoding the observed behaviour
would bless something nobody specified, so the disagreement is recorded
at the site as an open question. n06 preferring n10 may well be correct
and the comment stale, since n10 reaches it by fiber too.

depth-vs-cost is different: its validation line does not name an outcome
at all, saying only that the choice "reflects the actual cost tradeoff",
which either answer satisfies. The corpus shows both occurring under one
seed, three runs to two. That scenario needs a protocol decision about
which parent is correct before it can have an assertion.

Compare parents by address resolved from the snapshot's own
my_node_addr, and fail rather than skip when a node is absent from the
snapshot or still claims to be its own root. Those two states are the
common ones in the older corpus and both produce the same "no match" a
wrong parent does, so only separating them keeps a harness problem from
reading as a routing verdict.

testing/chaos/scenarios/bottleneck-parent.yaml

@@ -87,6 +87,21 @@ traffic:
   duration_secs: {min: 5, max: 10}
   parallel_streams: 2
 
+# The two test subjects named at the top of this file, encoded. Both
+# existed only as comments and were checked by nothing.
+#
+# n06 has a Bluetooth parent (n02) and a fiber one (n03) at the same depth
+# and should take the fiber. n09 has only Bluetooth, so asserting it keeps
+# n05 is asserting that the absence of an alternative is handled without
+# thrashing, not that Bluetooth was preferred.
+#
+# From the archived corpus: across the six provably-completed runs, n06's
+# parent is n03 in all six and n09's is n05 in all six.
+assertions:
+  tree_parents:
+    n06: n03
+    n09: n05
+
 logging:
   rust_log: "info"
   output_dir: "./sim-results"

testing/chaos/scenarios/cost-avoidance.yaml

@@ -61,6 +61,21 @@ traffic:
   duration_secs: {min: 5, max: 10}
   parallel_streams: 2
 
+# The validation line at the top of this file, encoded. It existed only as
+# that comment and was checked by nothing, so the scenario could not fail
+# on the one thing it was built to test.
+#
+# From the archived corpus: of the six runs that carry a status.txt and are
+# therefore provably completed, n04's parent is n03 in all six. Among the
+# 176 older runs the picture is quite different -- n04 is absent from the
+# snapshot in 107 of them and is its own root in 49 -- but those are runs
+# whose tree had not converged when the snapshot was taken rather than runs
+# that chose the wrong parent, which is why the assertion fails an absent
+# or self-parented node explicitly instead of skipping it.
+assertions:
+  tree_parents:
+    n04: n03
+
 logging:
   rust_log: "info"
   output_dir: "./sim-results"

testing/chaos/scenarios/depth-vs-cost.yaml

@@ -23,6 +23,23 @@
 #
 # Validation: tree snapshot shows n04's parent selection reflects the
 # actual cost tradeoff between depth and link quality.
+#
+# NO PARENT IS ASSERTED HERE, and unlike the other cost scenarios that is
+# not because the implementation disagrees with the comment. It is because
+# the validation line above does not name an outcome: "reflects the actual
+# cost tradeoff" is satisfied by either answer, so there is nothing to
+# encode. The comment above even says n04 "may still pick n01".
+#
+# The corpus agrees that both answers occur. Across the five provably
+# completed archived runs, n04's parent is n01 in three and n03 in two,
+# with the same seed each time, so the choice is not deterministic under
+# this configuration.
+#
+# Pinning whichever answer is more common would convert an unspecified
+# behaviour into a regression guard for something nobody decided, and
+# would go red roughly two runs in five besides. What this scenario needs
+# first is a decision about which parent is correct given the stated
+# effective depths, and that is a protocol question, not a harness one.
 
 scenario:
   name: "depth-vs-cost"

testing/chaos/scenarios/mixed-technology.yaml

@@ -30,6 +30,25 @@
 #   - n08 has fiber (n03) and Bluetooth (n04) parents — should pick n03
 #
 # Netem mutation shifts fiber-only links between normal and degraded.
+#
+# NEITHER TEST SUBJECT IS ASSERTED, because the implementation does not do
+# what the two lines above say. Across the five archived runs that carry a
+# status.txt and are therefore provably completed, n06's parent is n10 in
+# three and n03 in two, and n08's is n04 in four and n03 in one. Encoding
+# either as written would red the scenario most runs; encoding what it
+# actually does would pin behaviour nobody specified and would silently
+# bless it.
+#
+# So this is an open question rather than a missing assertion: either
+# cost-based selection is not preferring fiber here, or the criteria were
+# written for a topology this file no longer has. n06 taking n10 is the
+# more interesting half — n10 reaches n06 by fiber, so the choice may be
+# defensible and the comment simply stale. It needs someone to work out
+# which, and until then this scenario proves nothing about parent choice.
+#
+# That leaves this scenario asserting nothing about its own subject. The
+# baseline convergence assertion is the right floor for it and is tracked
+# separately; this note is here so the gap is visible until that lands.
 
 scenario:
   name: "mixed-technology"

testing/chaos/sim/assertions.py

@@ -24,6 +24,7 @@ from .scenario import (
     MaxErrorsAssertion,
     MaxParentSwitchesAssertion,
     MinParentSwitchesAssertion,
+    TreeParentsAssertion,
 )
 from .topology import SimTopology
 
@@ -172,6 +173,81 @@ def evaluate_max_parent_switches(
     )
 
 
+def evaluate_tree_parents(
+    cfg: TreeParentsAssertion,
+    snapshot: dict | None,
+) -> AssertionOutcome:
+    """Check each node's parent in the final tree snapshot.
+
+    Parents are compared by node address, resolved from the snapshot's own
+    ``my_node_addr`` fields, so the check does not depend on the display
+    name a node happened to publish.
+
+    Every way of not knowing the answer is a failure: no snapshot, the
+    node absent from it, the expected parent absent from it, or the node
+    still claiming to be its own root. Each of those produces the same
+    "no match" that a genuinely wrong parent does, and only saying so
+    separately keeps a harness problem from reading as a routing verdict.
+    """
+    if not snapshot:
+        return AssertionOutcome(
+            name="tree_parents",
+            passed=False,
+            detail=(
+                "FAIL tree_parents: no final tree snapshot was taken, so no "
+                "node's parent was observed. This is a harness failure, not "
+                "a statement about the tree."
+            ),
+        )
+
+    addr_of = {
+        nid: data.get("my_node_addr")
+        for nid, data in snapshot.items()
+        if data.get("my_node_addr")
+    }
+    id_of = {addr: nid for nid, addr in addr_of.items()}
+
+    good, bad = [], []
+    for child, want_parent in sorted(cfg.expected.items()):
+        entry = snapshot.get(child)
+        if entry is None:
+            bad.append(
+                f"{child} is absent from the snapshot ({len(snapshot)} node(s) "
+                f"present: {', '.join(sorted(snapshot))})"
+            )
+            continue
+        want_addr = addr_of.get(want_parent)
+        if want_addr is None:
+            bad.append(
+                f"{child}: expected parent {want_parent} is absent from the "
+                f"snapshot, so its address cannot be resolved"
+            )
+            continue
+        got_addr = entry.get("parent")
+        if got_addr == entry.get("my_node_addr"):
+            bad.append(
+                f"{child} is its own parent — it still believes it is root, "
+                f"so the tree never converged around it (wanted {want_parent})"
+            )
+            continue
+        if got_addr == want_addr:
+            good.append(f"{child}->{want_parent}")
+            continue
+        got_id = id_of.get(got_addr) or entry.get("parent_display_name") or got_addr
+        bad.append(f"{child} chose {got_id}, wanted {want_parent}")
+
+    if bad:
+        detail = f"FAIL tree_parents: {'; '.join(bad)}"
+        if good:
+            detail += f". Correct: {', '.join(good)}"
+        return AssertionOutcome(name="tree_parents", passed=False, detail=detail)
+    return AssertionOutcome(
+        name="tree_parents",
+        passed=True,
+        detail=f"PASS tree_parents: {', '.join(good)}",
+    )
+
+
 _CONGESTION_FLOORS = (
     ("min_nodes_detected", "congestion_detected"),
     ("min_nodes_ce_forwarded", "ce_forwarded"),

testing/chaos/sim/runner.py

@@ -17,6 +17,7 @@ from .assertions import (
     BloomSendRateMonitor,
     evaluate_congestion_signals,
     evaluate_max_errors,
+    evaluate_tree_parents,
     evaluate_max_parent_switches,
     evaluate_min_parent_switches,
 )
@@ -80,6 +81,7 @@ class SimRunner:
         # which the congestion assertion must treat as a failure rather
         # than as an absence of congestion.
         self.final_congestion: dict | None = None
+        self.final_tree: dict | None = None
 
     def _evaluate_max_parent_switches(
         self, cfg, parent_switches: list[tuple[str, str]]
@@ -622,6 +624,15 @@ class SimRunner:
                 else:
                     log.error("%s", outcome.detail)
 
+            tp_cfg = self.scenario.assertions.tree_parents
+            if tp_cfg is not None:
+                outcome = evaluate_tree_parents(tp_cfg, self.final_tree)
+                self.assertion_outcomes.append(outcome)
+                if outcome.passed:
+                    log.info("%s", outcome.detail)
+                else:
+                    log.error("%s", outcome.detail)
+
             cong_cfg = self.scenario.assertions.congestion_signals
             if cong_cfg is not None:
                 outcome = evaluate_congestion_signals(
@@ -698,6 +709,7 @@ class SimRunner:
         # failure present as an assertion that saw nothing.
         if label == "final":
             self.final_congestion = congestion_snap
+            self.final_tree = tree_snap
         os.makedirs(self.output_dir, exist_ok=True)
         with open(tree_path, "w") as f:
             json.dump(tree_snap, f, indent=2)

testing/chaos/sim/scenario.py

@@ -3,10 +3,16 @@
 from __future__ import annotations
 
 import os
+import re
 from dataclasses import dataclass, field
 
 import yaml
 
+# Node ids are rendered nNN by the topology generator, zero-padded to two
+# digits. Matching the shape here keeps a typo such as "no4" or "n4x" from
+# reaching an assertion as a node that simply never appears.
+_NODE_ID_RE = re.compile(r"n\d+")
+
 
 @dataclass
 class Range:
@@ -218,6 +224,24 @@ class MaxParentSwitchesAssertion:
     node: str | None = None
 
 
+@dataclass
+class TreeParentsAssertion:
+    """Expected parent per node in the final tree snapshot.
+
+    ``expected`` maps a node id to the node id its parent must be, e.g.
+    ``{"n04": "n03"}``. Both sides are node ids rather than node
+    addresses: an address is a per-run key derived from the generated
+    identity, so a scenario could not name one ahead of time.
+
+    A node absent from the snapshot fails rather than being skipped. That
+    case is not hypothetical — more than half the archived runs of these
+    scenarios have no entry for the node under test, and a skip would
+    have reported those as satisfied.
+    """
+
+    expected: dict[str, str] = field(default_factory=dict)
+
+
 @dataclass
 class CongestionSignalsAssertion:
     """Floors on how many nodes observed each congestion signal.
@@ -269,6 +293,7 @@ class AssertionsConfig:
     max_parent_switches: MaxParentSwitchesAssertion | None = None
     max_errors: MaxErrorsAssertion | None = None
     congestion_signals: CongestionSignalsAssertion | None = None
+    tree_parents: TreeParentsAssertion | None = None
 
 
 @dataclass
@@ -304,10 +329,13 @@ class Scenario:
 # YAML still looks correct — a mistyped "assertion:" disarms a scenario's only
 # assertions, a mistyped "link_flap:" turns off the chaos it was meant to inject.
 #
-# Three mappings are deliberately NOT listed, because their keys are names the
+# Four mappings are deliberately NOT listed, because their keys are names the
 # scenario author chooses rather than a schema: netem.mutation.policies,
-# link_swap.policies and topology.transport_mix. Two sub-trees are passed through
+# link_swap.policies, topology.transport_mix and assertions.tree_parents (whose
-# whole and are likewise not checked: fips_overrides and topology.params.
+# keys are node ids). Two sub-trees are passed through whole and are likewise
+# not checked: fips_overrides and topology.params. tree_parents is not thereby
+# unchecked -- both sides of every entry are validated as node ids that exist in
+# this scenario's topology, which is the check that matters for it.
 #
 # NOTE: adding a new assertion type means registering it in TWO places below —
 # _SECTION_KEYS["assertions"] (so the block accepts its name) and _ASSERTION_KEYS
@@ -345,7 +373,7 @@ _SECTION_KEYS = {
     "link_swap.edges[]": {"edge", "policy"},
     "assertions": {
         "bloom_send_rate", "min_parent_switches", "max_parent_switches",
-        "max_errors", "congestion_signals",
+        "max_errors", "congestion_signals", "tree_parents",
     },
     "logging": {"rust_log", "output_dir"},
 }
@@ -697,6 +725,39 @@ def load_scenario(path: str) -> Scenario:
                 "min_nodes_ce_received); a block with none asserts nothing"
             )
         s.assertions.congestion_signals = CongestionSignalsAssertion(**floors)
+    if "tree_parents" in asrt:
+        tp = asrt["tree_parents"]
+        if not isinstance(tp, dict) or not tp:
+            raise ValueError(
+                "assertions.tree_parents: give it at least one "
+                "'<node>: <expected parent>' entry; an empty block asserts "
+                "nothing"
+            )
+        expected = {}
+        for child, parent in tp.items():
+            for role, val in (("node", child), ("parent", parent)):
+                if not isinstance(val, str) or not _NODE_ID_RE.fullmatch(val):
+                    raise ValueError(
+                        f"assertions.tree_parents: {role} {val!r} is not a node "
+                        f"id of the form 'n04'"
+                    )
+                idx = int(val[1:])
+                if idx < 1 or idx > s.topology.num_nodes:
+                    raise ValueError(
+                        f"assertions.tree_parents: {role} '{val}' is outside "
+                        f"this scenario's {s.topology.num_nodes} nodes. An "
+                        f"assertion about a node that cannot exist would fail "
+                        f"for the wrong reason every run."
+                    )
+            if child == parent:
+                raise ValueError(
+                    f"assertions.tree_parents: '{child}' is given itself as "
+                    f"its parent. A node is its own parent only when it "
+                    f"believes it is root, which is what an unconverged tree "
+                    f"looks like; assert that some other way."
+                )
+            expected[child] = parent
+        s.assertions.tree_parents = TreeParentsAssertion(expected=expected)
 
     # Logging section
     lg = raw.get("logging", {})