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
+289 -244

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
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)")
+62 -102
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,
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)
return _Sortable(x)
+62 -102
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,
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)
return _Sortable(x)