beam: Q61 -- materialise each dependency iterable once, resolve the module locally, drop the dead first pass

topo_sort read graph.values() twice, so a dependency delivered as a generator,
map, filter or reverse iterator was exhausted by the first walk and the node
kept an empty dependency set. The result was a successful call with a wrong
order -- and, measured, a 3-cycle delivered as generators reported as a
successful order with cycle=None.

Every dependency sequence is now materialised exactly once, up front; every
later pass reads that copy, so double-consumption is structurally impossible.
Removed the unreachable first DFS pass in the cycle path (ruff F841 start/path/
res) leaving one find_back_edge; the self-loop case is explicit.

test_toposort.py resolved the module from /root/.hermes/wizard-outputs/beam,
a host path ahead of the test's own directory: a module that refuses to import
beside it left the suite green. It now resolves from Path(__file__).parent and
asserts the imported file's identity. 242 checks: 9 graph shapes x 7 delivery
forms, cycles in all 7, plus a guard that the duplicate toposort/ copy has not
drifted.

Falsified both ways: the suite fails on blob 4da0d8e and on the one-token
mutation tuple(ds) -> ds. CHECK rc 0 (85 unittests OK), verify.py 8/8.
This commit is contained in:
spark committed 2026-10-24 16:14:24 +00:00
1 parent 8555902b43
commit cdf6828fe0
3 files changed
+281 -236

No files matched your search

+165 -40
View File
@@ -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 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 from toposort import topo_sort
def check(graph, expected_order=None): # The module under test must be the committed file next to this test.
r = topo_sort(graph) MODULE_PATH = pathlib.Path(toposort_module.__file__).resolve()
if expected_order is not None: assert MODULE_PATH == (HERE / "toposort.py"), (MODULE_PATH, HERE)
assert r.cycle is None, (graph, r)
assert r.sorted == expected_order, (graph, r.sorted, expected_order) _checks = 0
return r
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(): 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)")
+58 -98
View File
@@ -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, 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 result.sorted is None and result.cycle is a list of nodes forming the cycle
(in dependency order, last -> first closing the loop). (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 __future__ import annotations
from collections import defaultdict, deque from collections import defaultdict
from dataclasses import dataclass from dataclasses import dataclass
from typing import Any, Iterable, Mapping from typing import Any, Iterable, Mapping
@@ -23,25 +30,21 @@ class TopoResult:
def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult:
# Normalize: ensure every referenced node exists as a key. # Materialise each dependency iterable once. `None` means "no dependencies".
nodes: set[Any] = set(graph.keys()) # This is the only place any dependency sequence is consumed.
for deps in graph.values(): deps_src: dict[Any, tuple[Any, ...]] = {
if deps is None: n: () if ds is None else tuple(ds) for n, ds in graph.items()
continue }
for d in deps:
nodes.add(d)
# 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} deps_of: dict[Any, set[Any]] = {n: set() for n in nodes}
for n, ds in graph.items(): for n, ds in deps_src.items():
if ds is None: deps_of[n].update(ds)
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)
# Kahn's algorithm with a sorted ready queue for determinism. # Kahn's algorithm with a sorted ready queue for determinism.
# in-degree = number of unprocessed dependencies # in-degree = number of unprocessed dependencies
@@ -72,105 +75,62 @@ def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult:
if len(out) == len(nodes): if len(out) == len(nodes):
return TopoResult(sorted=out, cycle=None) return TopoResult(sorted=out, cycle=None)
# Cycle exists. Reconstruct a cycle path. # A cycle exists. Everything Kahn could not emit is in a cycle or reaches
# Nodes not emitted are in cycles. Pick the smallest unemitted node. # one, so a DFS restricted to those nodes is guaranteed to find a back edge.
unemitted = [n for n in nodes if n not in set(out)] emitted = set(out)
try: unemitted = [n for n in nodes if n not in emitted]
start = min(unemitted)
except TypeError:
start = unemitted[0]
# 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] = {} 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 color[u] = 1
for v in sorted(deps_of[u], key=_key): 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 parent[v] = u
r = dfs(v) found = find_back_edge(v)
if r is not None: if found is not None:
return r return found
elif color.get(v, 0) == 1: elif state == 1:
# found cycle: v is ancestor of u return (u, v)
# 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)
color[u] = 2 color[u] = 2
return None return None
res = None back: tuple[Any, Any] | None = None
for s in sorted(unemitted, key=_key): for s in sorted(unemitted, key=_key):
if color[s] == 0: if color.get(s, 0) == 0:
parent[s] = None # type: ignore parent[s] = None
r = dfs(s) back = find_back_edge(s)
if r is not None: if back is not None:
res = r
break break
# Reconstruct cycle path robustly using parent pointers from the found edge. if back is None:
# Find the back edge (u -> v) where v is an ancestor. # Unreachable for a real cycle; never report a silent success instead.
back_u = back_v = None return TopoResult(sorted=None, cycle=list(unemitted[:1]))
# Re-run a focused search to capture back edge.
color2: dict[Any, int] = {n: 0 for n in nodes}
parent2: dict[Any, Any] = {}
def dfs2(u): back_u, back_v = back
nonlocal back_u, back_v if back_u == back_v:
color2[u] = 1 # self-loop: a cycle of length 1
for v in sorted(deps_of[u], key=_key): return TopoResult(sorted=None, cycle=[back_v])
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
for s in sorted(unemitted, key=_key): # back_u depends on back_v, and parent[x] = y means "y depends on x", so
if color2[s] == 0: # walking parents from back_u reaches back_v. Reversed, that walk is the
parent2[s] = None # type: ignore # cycle in dependency order: every node is followed by one of its own deps,
if dfs2(s): # and the last node depends on the first, closing the loop.
break path: list[Any] = [back_u]
cur = parent.get(back_u)
# Build path from back_v to back_u via parent2, then close.
path = []
cur = back_u
while cur is not None and cur != back_v: while cur is not None and cur != back_v:
path.append(cur) path.append(cur)
cur = parent2.get(cur) cur = parent.get(cur)
if cur == back_v: 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.append(back_v)
path.reverse() path.reverse()
# path: back_v ... back_u return TopoResult(sorted=None, cycle=path)
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)
class _Sortable: class _Sortable:
+58 -98
View File
@@ -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, 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 result.sorted is None and result.cycle is a list of nodes forming the cycle
(in dependency order, last -> first closing the loop). (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 __future__ import annotations
from collections import defaultdict, deque from collections import defaultdict
from dataclasses import dataclass from dataclasses import dataclass
from typing import Any, Iterable, Mapping from typing import Any, Iterable, Mapping
@@ -23,25 +30,21 @@ class TopoResult:
def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult: def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult:
# Normalize: ensure every referenced node exists as a key. # Materialise each dependency iterable once. `None` means "no dependencies".
nodes: set[Any] = set(graph.keys()) # This is the only place any dependency sequence is consumed.
for deps in graph.values(): deps_src: dict[Any, tuple[Any, ...]] = {
if deps is None: n: () if ds is None else tuple(ds) for n, ds in graph.items()
continue }
for d in deps:
nodes.add(d)
# 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} deps_of: dict[Any, set[Any]] = {n: set() for n in nodes}
for n, ds in graph.items(): for n, ds in deps_src.items():
if ds is None: deps_of[n].update(ds)
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)
# Kahn's algorithm with a sorted ready queue for determinism. # Kahn's algorithm with a sorted ready queue for determinism.
# in-degree = number of unprocessed dependencies # in-degree = number of unprocessed dependencies
@@ -72,105 +75,62 @@ def topo_sort(graph: Mapping[Any, Iterable[Any]]) -> TopoResult:
if len(out) == len(nodes): if len(out) == len(nodes):
return TopoResult(sorted=out, cycle=None) return TopoResult(sorted=out, cycle=None)
# Cycle exists. Reconstruct a cycle path. # A cycle exists. Everything Kahn could not emit is in a cycle or reaches
# Nodes not emitted are in cycles. Pick the smallest unemitted node. # one, so a DFS restricted to those nodes is guaranteed to find a back edge.
unemitted = [n for n in nodes if n not in set(out)] emitted = set(out)
try: unemitted = [n for n in nodes if n not in emitted]
start = min(unemitted)
except TypeError:
start = unemitted[0]
# 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] = {} 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 color[u] = 1
for v in sorted(deps_of[u], key=_key): 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 parent[v] = u
r = dfs(v) found = find_back_edge(v)
if r is not None: if found is not None:
return r return found
elif color.get(v, 0) == 1: elif state == 1:
# found cycle: v is ancestor of u return (u, v)
# 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)
color[u] = 2 color[u] = 2
return None return None
res = None back: tuple[Any, Any] | None = None
for s in sorted(unemitted, key=_key): for s in sorted(unemitted, key=_key):
if color[s] == 0: if color.get(s, 0) == 0:
parent[s] = None # type: ignore parent[s] = None
r = dfs(s) back = find_back_edge(s)
if r is not None: if back is not None:
res = r
break break
# Reconstruct cycle path robustly using parent pointers from the found edge. if back is None:
# Find the back edge (u -> v) where v is an ancestor. # Unreachable for a real cycle; never report a silent success instead.
back_u = back_v = None return TopoResult(sorted=None, cycle=list(unemitted[:1]))
# Re-run a focused search to capture back edge.
color2: dict[Any, int] = {n: 0 for n in nodes}
parent2: dict[Any, Any] = {}
def dfs2(u): back_u, back_v = back
nonlocal back_u, back_v if back_u == back_v:
color2[u] = 1 # self-loop: a cycle of length 1
for v in sorted(deps_of[u], key=_key): return TopoResult(sorted=None, cycle=[back_v])
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
for s in sorted(unemitted, key=_key): # back_u depends on back_v, and parent[x] = y means "y depends on x", so
if color2[s] == 0: # walking parents from back_u reaches back_v. Reversed, that walk is the
parent2[s] = None # type: ignore # cycle in dependency order: every node is followed by one of its own deps,
if dfs2(s): # and the last node depends on the first, closing the loop.
break path: list[Any] = [back_u]
cur = parent.get(back_u)
# Build path from back_v to back_u via parent2, then close.
path = []
cur = back_u
while cur is not None and cur != back_v: while cur is not None and cur != back_v:
path.append(cur) path.append(cur)
cur = parent2.get(cur) cur = parent.get(cur)
if cur == back_v: 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.append(back_v)
path.reverse() path.reverse()
# path: back_v ... back_u return TopoResult(sorted=None, cycle=path)
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)
class _Sortable: class _Sortable: