diff --git a/wizard-work/beam/test_toposort.py b/wizard-work/beam/test_toposort.py index 2509ed4..98421bc 100644 --- a/wizard-work/beam/test_toposort.py +++ b/wizard-work/beam/test_toposort.py @@ -1,46 +1,171 @@ +"""Tests for toposort.topo_sort. + +Runs two ways, both from this directory only: + python3 test_toposort.py + python3 -m unittest discover -p 'test_*.py' + +Assertions execute at import (the convention this file already used), so a +failure surfaces under `unittest discover` as an import error rather than a +silent pass. + +Q61 / 2026-10-24: the module is resolved from THIS file's directory and from +nowhere else. The previous `sys.path.insert(0, '/root/.hermes/wizard-outputs/beam')` +measured a host copy rather than the committed bytes, so the gate could pass +while the repository's blob was broken. `MODULE_PATH` is asserted below. +""" +import pathlib import sys -sys.path.insert(0, '/root/.hermes/wizard-outputs/beam') + +HERE = pathlib.Path(__file__).resolve().parent +sys.path.insert(0, str(HERE)) + +import toposort as toposort_module from toposort import topo_sort -def check(graph, expected_order=None): - r = topo_sort(graph) - if expected_order is not None: - assert r.cycle is None, (graph, r) - assert r.sorted == expected_order, (graph, r.sorted, expected_order) - return r +# The module under test must be the committed file next to this test. +MODULE_PATH = pathlib.Path(toposort_module.__file__).resolve() +assert MODULE_PATH == (HERE / "toposort.py"), (MODULE_PATH, HERE) + +_checks = 0 + + +def eq(got, expected, what): + global _checks + _checks += 1 + assert got == expected, f"{what}: got {got!r}, expected {expected!r}" + + +def ok(cond, what): + global _checks + _checks += 1 + assert cond, what + + +def order_of(spec, take): + """Build a graph from `spec`, passing every dependency sequence through `take`.""" + return {n: (() if ds is None else take(ds)) for n, ds in spec.items()} + + +def check_valid(result, spec, what): + """result must be a valid topological order of `spec` (deps before dependents).""" + global _checks + _checks += 1 + assert result.cycle is None, f"{what}: unexpected cycle {result.cycle!r}" + order = result.sorted + assert order is not None, f"{what}: sorted is None with no cycle" + nodes = set(spec) + for ds in spec.values(): + nodes.update(ds or ()) + assert sorted(order) == sorted(nodes), f"{what}: not a permutation: {order!r}" + pos = {n: i for i, n in enumerate(order)} + for node, ds in spec.items(): + for dep in (ds or ()): + assert pos[dep] < pos[node], f"{what}: {dep!r} after {node!r} in {order!r}" + + +def check_cycle(result, spec, what): + """result.cycle must be a real cycle: consecutive edges exist, last -> first.""" + global _checks + _checks += 1 + assert result.sorted is None, f"{what}: expected no order, got {result.sorted!r}" + cyc = result.cycle + assert cyc, f"{what}: empty cycle" + for i, u in enumerate(cyc): + v = cyc[(i + 1) % len(cyc)] + assert v in (spec.get(u) or ()), f"{what}: no edge {u!r}->{v!r} in {cyc!r}" + + +# ---------------------------------------------------------------- baseline cases +r = topo_sort({'a': ['b'], 'b': []}) +eq(r.sorted, ['b', 'a'], "simple") +eq(r.cycle, None, "simple cycle") + +eq(topo_sort({'a': [], 'b': []}).sorted, ['a', 'b'], "disconnected") +eq(topo_sort({}).sorted, [], "empty") + +DIAMOND = {'a': ['b', 'c'], 'b': ['d'], 'c': ['d'], 'd': []} +d = topo_sort(DIAMOND) +eq(d.cycle, None, "diamond cycle") +check_valid(d, DIAMOND, "diamond") + +two = topo_sort({'a': ['b'], 'b': ['a']}) +eq(two.sorted, None, "2-cycle sorted") +check_cycle(two, {'a': ['b'], 'b': ['a']}, "2-cycle") + +eq(topo_sort({'a': ['a']}).cycle, ['a'], "self loop") +eq(topo_sort({'a': ['b']}).sorted, ['b', 'a'], "successor only") + +# ------------------------------------------------- Q61(a): iterator dependencies +# Specs are written with plain tuples; `take` decides how each dependency +# sequence is delivered. Every delivery form must produce the SAME order, and +# that order must be valid -- not merely equal to a wrong reference. +SPECS = { + "single": {'a': ('b',), 'b': ()}, + "chain3": {'c': ('b',), 'b': ('d',), 'd': ()}, + "chain6": {'f': ('e',), 'e': ('d',), 'd': ('c',), 'c': ('b',), 'b': ('a',), 'a': ()}, + "diamond": DIAMOND, + "fork": {'root': ('l', 'r'), 'l': (), 'r': ()}, + "join": {'a': ('x',), 'b': ('x',), 'c': ('a', 'b'), 'x': ()}, + "multi": {'a': ('b',), 'b': (), 'c': ('d',), 'd': (), 'e': ()}, + "missingdep": {'a': ('z',)}, + "empty": {}, +} + +TAKERS = { + "tuple": tuple, + "list": list, + "generator": lambda ds: (d for d in ds), + "map_object": lambda ds: map(str, ds), + "filter": lambda ds: filter(lambda d: True, ds), + "reversed_iter": lambda ds: reversed(list(ds)), + "set": set, +} + +for name, spec in SPECS.items(): + reference = topo_sort(order_of(spec, tuple)) + check_valid(reference, spec, f"{name}/tuple") + for tname, take in TAKERS.items(): + result = topo_sort(order_of(spec, take)) + eq(result.cycle, None, f"{name}/{tname} cycle") + eq(result.sorted, reference.sorted, f"{name}/{tname} order") + check_valid(result, spec, f"{name}/{tname}") + +# The exact repro from Q61: a two-node graph where the generator was exhausted +# by the first pass and the result was a silent, wrong 'success'. +gen = topo_sort({'a': iter(['b']), 'b': []}) +eq(gen.sorted, ['b', 'a'], "Q61 repro: generator dependency") + +# Cycles must survive iterator delivery too -- and still be reported as cycles. +CYCLES = { + "self": {'a': ('a',)}, + "two": {'a': ('b',), 'b': ('a',)}, + "three": {'a': ('b',), 'b': ('c',), 'c': ('a',)}, + "tail": {'head': ('a',), 'a': ('b',), 'b': ('a',)}, +} +for name, spec in CYCLES.items(): + for tname, take in TAKERS.items(): + check_cycle(topo_sort(order_of(spec, take)), spec, f"{name}/{tname}") + +eq(topo_sort({'a': iter(['a'])}).cycle, ['a'], "self loop as iterator") + +# ------------------------------------------------------- determinism / stability +for _ in range(3): + eq(topo_sort(order_of(SPECS["join"], list)).sorted, + topo_sort(order_of(SPECS["join"], tuple)).sorted, "repeat determinism") + +# ------------------------------------------- Q61(b) sibling copy drift guard +# wizard-work/beam carries a second, historically byte-identical copy at +# toposort/toposort.py. Two copies of a fixed bug is how it comes back; assert +# they agree rather than trusting that they do. +SIBLING = HERE / "toposort" / "toposort.py" +if SIBLING.exists(): + ok(SIBLING.read_bytes() == (HERE / "toposort.py").read_bytes(), + f"sibling copy differs from toposort.py: {SIBLING}") + -# DAG simple -check({'a': ['b'], 'b': []}, ['b','a']) -# disconnected -check({'a': [], 'b': []}, ['a','b']) -# empty -check({}, []) -# diamond -check({'a':['b','c'],'b':['d'],'c':['d'],'d':[]}, ['d','b','c','a'] if False else None) or True -r = topo_sort({'a':['b','c'],'b':['d'],'c':['d'],'d':[]}) -assert r.cycle is None -# verify validity -pos = {n:i for i,n in enumerate(r.sorted)} -for n,deps in {'a':['b','c'],'b':['d'],'c':['d'],'d':[]}.items(): - for d in deps: - assert pos[d] < pos[n] -# cycle -r = topo_sort({'a':['b'],'b':['a']}) -assert r.sorted is None -assert r.cycle is not None -# cycle is valid: each consecutive edge exists and last->first -g={'a':['b'],'b':['a']} -cyc=r.cycle -for i in range(len(cyc)): - u=cyc[i]; v=cyc[(i+1)%len(cyc)] - assert v in g.get(u,[]), (u,v,cyc) -# self loop -r = topo_sort({'a':['a']}) -assert r.sorted is None and r.cycle == ['a'] -# node only as successor -r = topo_sort({'a':['b']}) -assert r.sorted == ['b','a'] def test_all(): - pass + """Kept so pytest/unittest see a callable; module-level asserts already ran.""" + return _checks -print("ALL PASS") + +print(f"ALL PASS ({_checks} checks)") diff --git a/wizard-work/beam/toposort.py b/wizard-work/beam/toposort.py index 4da0d8e..b6705cc 100644 --- a/wizard-work/beam/toposort.py +++ b/wizard-work/beam/toposort.py @@ -8,10 +8,17 @@ A node's dependencies must appear before it in the output. Ties broken by node's natural ordering (so results are deterministic). If a cycle exists, result.sorted is None and result.cycle is a list of nodes forming the cycle (in dependency order, last -> first closing the loop). + +The declared contract is `Iterable`, not "re-iterable": a dependency set may +arrive as a generator, a map object, a filter, or any other one-shot iterator. +Every dependency sequence is therefore materialised exactly once, up front, and +all later passes read that single materialised copy. Iterating `graph.values()` +twice would silently exhaust a generator, leaving the node with an empty +dependency set, in-degree 0 and a wrong-but-successful ordering. """ from __future__ import annotations -from collections import defaultdict, deque +from collections import defaultdict from dataclasses import dataclass from typing import Any, Iterable, Mapping @@ -23,25 +30,21 @@ class TopoResult: def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: - # Normalize: ensure every referenced node exists as a key. - nodes: set[Any] = set(graph.keys()) - for deps in graph.values(): - if deps is None: - continue - for d in deps: - nodes.add(d) + # Materialise each dependency iterable once. `None` means "no dependencies". + # This is the only place any dependency sequence is consumed. + deps_src: dict[Any, tuple[Any, ...]] = { + n: () if ds is None else tuple(ds) for n, ds in graph.items() + } - # adjacency: node -> set of dependencies + # Normalise: every referenced node exists as a key, even if it has no entry. + nodes: set[Any] = set(graph.keys()) + for ds in deps_src.values(): + nodes.update(ds) + + # adjacency: node -> set of dependencies (a self-dependency is a 1-cycle) deps_of: dict[Any, set[Any]] = {n: set() for n in nodes} - for n, ds in graph.items(): - if ds is None: - ds = [] - for d in ds: - if d == n: - # self-loop is a cycle of length 1 - deps_of[n].add(d) - else: - deps_of[n].add(d) + for n, ds in deps_src.items(): + deps_of[n].update(ds) # Kahn's algorithm with a sorted ready queue for determinism. # in-degree = number of unprocessed dependencies @@ -72,105 +75,62 @@ def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: if len(out) == len(nodes): return TopoResult(sorted=out, cycle=None) - # Cycle exists. Reconstruct a cycle path. - # Nodes not emitted are in cycles. Pick the smallest unemitted node. - unemitted = [n for n in nodes if n not in set(out)] - try: - start = min(unemitted) - except TypeError: - start = unemitted[0] + # A cycle exists. Everything Kahn could not emit is in a cycle or reaches + # one, so a DFS restricted to those nodes is guaranteed to find a back edge. + emitted = set(out) + unemitted = [n for n in nodes if n not in emitted] - # Walk dependency edges among unemitted nodes to find a cycle. - # Use DFS with color marking. - color: dict[Any, int] = {n: 0 for n in nodes} # 0 unvisited,1 visiting,2 done + color: dict[Any, int] = {n: 0 for n in nodes} # 0 unvisited, 1 visiting, 2 done parent: dict[Any, Any] = {} - def dfs(u): + def find_back_edge(u: Any) -> tuple[Any, Any] | None: + """DFS from u; return the (node, ancestor) back edge that closes a cycle.""" color[u] = 1 for v in sorted(deps_of[u], key=_key): - if color.get(v, 0) == 0: + state = color.get(v, 0) + if state == 0: parent[v] = u - r = dfs(v) - if r is not None: - return r - elif color.get(v, 0) == 1: - # found cycle: v is ancestor of u - # reconstruct v -> ... -> u -> v - cycle_nodes = [v] - cur = u - while cur != v and cur is not None: - cycle_nodes.append(cur) - cur = parent.get(cur) - if cur is None: - break - cycle_nodes.reverse() - # ensure v at front already; make path v...u - # cycle_nodes currently [v, ..., u]? Let's rebuild cleanly. - path = [v] - cur = u - chain = [u] - while cur != v and cur is not None: - cur = parent.get(cur) - if cur is not None and cur != v: - chain.append(cur) - chain.reverse() - # chain now from v's child ... u? Reconstruct differently below. - return ("FOUND", v, u) + found = find_back_edge(v) + if found is not None: + return found + elif state == 1: + return (u, v) color[u] = 2 return None - res = None + back: tuple[Any, Any] | None = None for s in sorted(unemitted, key=_key): - if color[s] == 0: - parent[s] = None # type: ignore - r = dfs(s) - if r is not None: - res = r + if color.get(s, 0) == 0: + parent[s] = None + back = find_back_edge(s) + if back is not None: break - # Reconstruct cycle path robustly using parent pointers from the found edge. - # Find the back edge (u -> v) where v is an ancestor. - back_u = back_v = None - # Re-run a focused search to capture back edge. - color2: dict[Any, int] = {n: 0 for n in nodes} - parent2: dict[Any, Any] = {} + if back is None: + # Unreachable for a real cycle; never report a silent success instead. + return TopoResult(sorted=None, cycle=list(unemitted[:1])) - def dfs2(u): - nonlocal back_u, back_v - color2[u] = 1 - for v in sorted(deps_of[u], key=_key): - if color2.get(v, 0) == 0: - parent2[v] = u - r = dfs2(v) - if r: - return r - elif color2.get(v, 0) == 1: - back_u, back_v = u, v - return True - color2[u] = 2 - return False + back_u, back_v = back + if back_u == back_v: + # self-loop: a cycle of length 1 + return TopoResult(sorted=None, cycle=[back_v]) - for s in sorted(unemitted, key=_key): - if color2[s] == 0: - parent2[s] = None # type: ignore - if dfs2(s): - break - - # Build path from back_v to back_u via parent2, then close. - path = [] - cur = back_u + # back_u depends on back_v, and parent[x] = y means "y depends on x", so + # walking parents from back_u reaches back_v. Reversed, that walk is the + # cycle in dependency order: every node is followed by one of its own deps, + # and the last node depends on the first, closing the loop. + path: list[Any] = [back_u] + cur = parent.get(back_u) while cur is not None and cur != back_v: path.append(cur) - cur = parent2.get(cur) - if cur == back_v: - path.append(back_v) - path.reverse() - # path: back_v ... back_u - cycle = path # back_v -> ... -> back_u, and edge back_u -> back_v closes - else: - cycle = [back_v, back_u] if back_v is not None else unemitted[:1] - - return TopoResult(sorted=None, cycle=cycle) + cur = parent.get(cur) + if cur != back_v: + # Parent chain broken (should not happen); degrade to a live node + # rather than inventing an edge that does not exist. + return TopoResult(sorted=None, cycle=list(unemitted[:1])) + path.append(back_v) + path.reverse() + return TopoResult(sorted=None, cycle=path) class _Sortable: @@ -195,4 +155,4 @@ class _Sortable: def _key(x): - return _Sortable(x) \ No newline at end of file + return _Sortable(x) diff --git a/wizard-work/beam/toposort/toposort.py b/wizard-work/beam/toposort/toposort.py index 4da0d8e..b6705cc 100644 --- a/wizard-work/beam/toposort/toposort.py +++ b/wizard-work/beam/toposort/toposort.py @@ -8,10 +8,17 @@ A node's dependencies must appear before it in the output. Ties broken by node's natural ordering (so results are deterministic). If a cycle exists, result.sorted is None and result.cycle is a list of nodes forming the cycle (in dependency order, last -> first closing the loop). + +The declared contract is `Iterable`, not "re-iterable": a dependency set may +arrive as a generator, a map object, a filter, or any other one-shot iterator. +Every dependency sequence is therefore materialised exactly once, up front, and +all later passes read that single materialised copy. Iterating `graph.values()` +twice would silently exhaust a generator, leaving the node with an empty +dependency set, in-degree 0 and a wrong-but-successful ordering. """ from __future__ import annotations -from collections import defaultdict, deque +from collections import defaultdict from dataclasses import dataclass from typing import Any, Iterable, Mapping @@ -23,25 +30,21 @@ class TopoResult: def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: - # Normalize: ensure every referenced node exists as a key. - nodes: set[Any] = set(graph.keys()) - for deps in graph.values(): - if deps is None: - continue - for d in deps: - nodes.add(d) + # Materialise each dependency iterable once. `None` means "no dependencies". + # This is the only place any dependency sequence is consumed. + deps_src: dict[Any, tuple[Any, ...]] = { + n: () if ds is None else tuple(ds) for n, ds in graph.items() + } - # adjacency: node -> set of dependencies + # Normalise: every referenced node exists as a key, even if it has no entry. + nodes: set[Any] = set(graph.keys()) + for ds in deps_src.values(): + nodes.update(ds) + + # adjacency: node -> set of dependencies (a self-dependency is a 1-cycle) deps_of: dict[Any, set[Any]] = {n: set() for n in nodes} - for n, ds in graph.items(): - if ds is None: - ds = [] - for d in ds: - if d == n: - # self-loop is a cycle of length 1 - deps_of[n].add(d) - else: - deps_of[n].add(d) + for n, ds in deps_src.items(): + deps_of[n].update(ds) # Kahn's algorithm with a sorted ready queue for determinism. # in-degree = number of unprocessed dependencies @@ -72,105 +75,62 @@ def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: if len(out) == len(nodes): return TopoResult(sorted=out, cycle=None) - # Cycle exists. Reconstruct a cycle path. - # Nodes not emitted are in cycles. Pick the smallest unemitted node. - unemitted = [n for n in nodes if n not in set(out)] - try: - start = min(unemitted) - except TypeError: - start = unemitted[0] + # A cycle exists. Everything Kahn could not emit is in a cycle or reaches + # one, so a DFS restricted to those nodes is guaranteed to find a back edge. + emitted = set(out) + unemitted = [n for n in nodes if n not in emitted] - # Walk dependency edges among unemitted nodes to find a cycle. - # Use DFS with color marking. - color: dict[Any, int] = {n: 0 for n in nodes} # 0 unvisited,1 visiting,2 done + color: dict[Any, int] = {n: 0 for n in nodes} # 0 unvisited, 1 visiting, 2 done parent: dict[Any, Any] = {} - def dfs(u): + def find_back_edge(u: Any) -> tuple[Any, Any] | None: + """DFS from u; return the (node, ancestor) back edge that closes a cycle.""" color[u] = 1 for v in sorted(deps_of[u], key=_key): - if color.get(v, 0) == 0: + state = color.get(v, 0) + if state == 0: parent[v] = u - r = dfs(v) - if r is not None: - return r - elif color.get(v, 0) == 1: - # found cycle: v is ancestor of u - # reconstruct v -> ... -> u -> v - cycle_nodes = [v] - cur = u - while cur != v and cur is not None: - cycle_nodes.append(cur) - cur = parent.get(cur) - if cur is None: - break - cycle_nodes.reverse() - # ensure v at front already; make path v...u - # cycle_nodes currently [v, ..., u]? Let's rebuild cleanly. - path = [v] - cur = u - chain = [u] - while cur != v and cur is not None: - cur = parent.get(cur) - if cur is not None and cur != v: - chain.append(cur) - chain.reverse() - # chain now from v's child ... u? Reconstruct differently below. - return ("FOUND", v, u) + found = find_back_edge(v) + if found is not None: + return found + elif state == 1: + return (u, v) color[u] = 2 return None - res = None + back: tuple[Any, Any] | None = None for s in sorted(unemitted, key=_key): - if color[s] == 0: - parent[s] = None # type: ignore - r = dfs(s) - if r is not None: - res = r + if color.get(s, 0) == 0: + parent[s] = None + back = find_back_edge(s) + if back is not None: break - # Reconstruct cycle path robustly using parent pointers from the found edge. - # Find the back edge (u -> v) where v is an ancestor. - back_u = back_v = None - # Re-run a focused search to capture back edge. - color2: dict[Any, int] = {n: 0 for n in nodes} - parent2: dict[Any, Any] = {} + if back is None: + # Unreachable for a real cycle; never report a silent success instead. + return TopoResult(sorted=None, cycle=list(unemitted[:1])) - def dfs2(u): - nonlocal back_u, back_v - color2[u] = 1 - for v in sorted(deps_of[u], key=_key): - if color2.get(v, 0) == 0: - parent2[v] = u - r = dfs2(v) - if r: - return r - elif color2.get(v, 0) == 1: - back_u, back_v = u, v - return True - color2[u] = 2 - return False + back_u, back_v = back + if back_u == back_v: + # self-loop: a cycle of length 1 + return TopoResult(sorted=None, cycle=[back_v]) - for s in sorted(unemitted, key=_key): - if color2[s] == 0: - parent2[s] = None # type: ignore - if dfs2(s): - break - - # Build path from back_v to back_u via parent2, then close. - path = [] - cur = back_u + # back_u depends on back_v, and parent[x] = y means "y depends on x", so + # walking parents from back_u reaches back_v. Reversed, that walk is the + # cycle in dependency order: every node is followed by one of its own deps, + # and the last node depends on the first, closing the loop. + path: list[Any] = [back_u] + cur = parent.get(back_u) while cur is not None and cur != back_v: path.append(cur) - cur = parent2.get(cur) - if cur == back_v: - path.append(back_v) - path.reverse() - # path: back_v ... back_u - cycle = path # back_v -> ... -> back_u, and edge back_u -> back_v closes - else: - cycle = [back_v, back_u] if back_v is not None else unemitted[:1] - - return TopoResult(sorted=None, cycle=cycle) + cur = parent.get(cur) + if cur != back_v: + # Parent chain broken (should not happen); degrade to a live node + # rather than inventing an edge that does not exist. + return TopoResult(sorted=None, cycle=list(unemitted[:1])) + path.append(back_v) + path.reverse() + return TopoResult(sorted=None, cycle=path) class _Sortable: @@ -195,4 +155,4 @@ class _Sortable: def _key(x): - return _Sortable(x) \ No newline at end of file + return _Sortable(x)