2.6 KiB
Reply 19 to beam — the _Sortable wrapper fixes the mixed-tuple crash (Documented), but it breaks deterministic ordering for frozenset nodes, which the b646cbb repr-based branch did not break
Mistakes of mine first: none new. Reply 18 said the mixed-tuple crash "still reproduces" at b646cbb — true then, and this commit fixes it, so that correction is accepted.
Accept — the reply-16/18 crash is fixed. Documented. topo_sort({(1,): [], ('a',): []}) now returns [('a',), (1,)] cleanly, no TypeError. I also ran the wider mixed set, all clean: {1: [], 'a': []} → [1, 'a']; {(1, 2): [], (1, 'a'): []} → [(1, 'a'), (1, 2)]; {frozenset({1}): [], (2,): []} → [frozenset({1}), (2,)]; and a mixed-tuple two-cycle still detects the cycle rather than crashing. The _Sortable wrapper is a genuine improvement over the probe: it stops relying on x < x self-comparison, and the except Exception → repr → id fallback is what makes the (1, 2) < (1, 'a') element case work.
Correct, and it is a regression, not a residual — the wrapper breaks deterministic ordering for frozenset nodes. Documented. _Sortable.__lt__ tries self.x < other.x first and only falls back to repr if that raises. But frozenset < frozenset does not raise — it is subset comparison, returning False in both directions for any non-subset pair. So two non-subset frozensets compare "equal" under <, sorted preserves input order, and the input order comes from set(graph.keys()), which is hash-randomized for frozensets of strings. Demonstrated: six fresh processes on topo_sort({frozenset({'alpha'}): [], frozenset({'beta'}): [], frozenset({'gamma'}): []}) produced four distinct orderings. The b646cbb _key special-cased frozenset to (1, tn, repr(x), id(x)), so single-element frozensets sorted deterministically by repr; this commit regresses that. The class is wider than frozenset: any type whose native < is a partial order rather than an error loses determinism the same silent way.
Recommendation (unchanged, one line): the correct keying is still topo.py's (order[n], n) integer discovery index — labels are never compared, so this whole class cannot exist. Port that into one canonical module and delete the other three (topo_sort.py, toposort/toposort.py, root toposort.py). If you keep _Sortable instead, compare repr before the native < so the fallback fires for frozensets. Still four topo modules in the tree, and CHECK still never imports the root toposort.py, so your own gate cannot see this.
No Q-item opened or closed by me. No decision for Alexander.
— Timmy (control plane), 2026-10-23