From 9328b49ec442bf3c8111bdf2a236166bf2a64110 Mon Sep 17 00:00:00 2001 From: Rainier Potgieter Date: Wed, 27 May 2026 10:53:23 +0200 Subject: [PATCH] perf(core): tighten resolve() sorting to drop the quadratic hot paths resolve() re-sorted the Kahn queue on every pop (O(V^2 log V)) and rebuilt each depth level with a filter-and-sort over the full node array (O(V*D), quadratic on a linear chain). This is Candidate 3 of issue #46. The output (levels and order) is derived only from the depth map and a per-level alphabetical sort: the depth map is the longest path from a root and is order-invariant (a node is dequeued only after every predecessor is processed, so its depth is final when read), and each level is sorted alphabetically. The dequeue order therefore has no effect on output. So: walk the alphabetically-seeded queue with a FIFO index pointer instead of re-sorting on every pop, drop the per-node dependents sort, and build the levels in a single O(V) pass over the pre-sorted names array (which drops each node into its depth bucket already in alphabetical order) instead of filter-and-sort per level. Determinism is preserved exactly. A golden-master baseline captures the pre-optimisation resolve() output across linear chain, diamond, wide fan-out, multiple roots, disconnected components, cross-level depth, reverse-name ordering, and the cycle / self-dependency / missing-dependency error shapes; the optimised resolve() reproduces every snapshot byte-for-byte. The baseline was verified to fail under a perturbed level ordering, so it genuinely guards determinism rather than passing vacuously. Proof (npm run bench, resolve on a 1000-step linear chain, this machine): best 21.2ms before, 1.19ms after (~18x), widening the margin against the 800ms assertion from ~38x to ~670x. Thresholds left unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/core/dag.test.ts | 130 ++++++++++++++++++++++++++++++++++++++ packages/core/dag.ts | 28 +++++--- 2 files changed, 150 insertions(+), 8 deletions(-) diff --git a/packages/core/dag.test.ts b/packages/core/dag.test.ts index 67ad664..b04fcd2 100644 --- a/packages/core/dag.test.ts +++ b/packages/core/dag.test.ts @@ -309,3 +309,133 @@ describe("deterministic output", () => { expect(r1).toEqual(r2); }); }); + +// ============================================================================= +// Determinism baseline (issue #46, Candidate 3) +// +// resolve() output (levels + order, and error shapes) must be byte-identical +// before and after the sort-tightening optimisation. These are golden-master +// snapshots captured from the pre-optimisation implementation on main across +// varied DAG shapes; the optimised resolve() must reproduce them exactly. +// ============================================================================= + +describe("determinism baseline: resolve output is byte-identical across shapes", () => { + const cases: Array<{ name: string; input: Record; expected: unknown }> = [ + { name: "empty", input: {}, expected: { ok: true, levels: [], order: [] } }, + { name: "single", input: { a: [] }, expected: { ok: true, levels: [["a"]], order: ["a"] } }, + { + name: "linear5", + input: { a: [], b: ["a"], c: ["b"], d: ["c"], e: ["d"] }, + expected: { + ok: true, + levels: [["a"], ["b"], ["c"], ["d"], ["e"]], + order: ["a", "b", "c", "d", "e"], + }, + }, + { + name: "diamond", + input: { a: [], b: ["a"], c: ["a"], d: ["b", "c"] }, + expected: { ok: true, levels: [["a"], ["b", "c"], ["d"]], order: ["a", "b", "c", "d"] }, + }, + { + name: "wideFanout", + input: { r: [], c1: ["r"], c2: ["r"], c3: ["r"], c4: ["r"], c5: ["r"] }, + expected: { + ok: true, + levels: [["r"], ["c1", "c2", "c3", "c4", "c5"]], + order: ["r", "c1", "c2", "c3", "c4", "c5"], + }, + }, + { + name: "multiRoot", + input: { r2: [], r1: [], j: ["r1", "r2"], solo: [] }, + expected: { + ok: true, + levels: [["r1", "r2", "solo"], ["j"]], + order: ["r1", "r2", "solo", "j"], + }, + }, + { + name: "disconnected", + input: { a: [], b: ["a"], x: [], y: ["x"] }, + expected: { + ok: true, + levels: [ + ["a", "x"], + ["b", "y"], + ], + order: ["a", "x", "b", "y"], + }, + }, + { + name: "crossLevel", + input: { a: [], b: ["a"], c: ["a", "b"] }, + expected: { ok: true, levels: [["a"], ["b"], ["c"]], order: ["a", "b", "c"] }, + }, + { + name: "reverseNames", + input: { z: [], y: ["z"], x: ["y"] }, + expected: { ok: true, levels: [["z"], ["y"], ["x"]], order: ["z", "y", "x"] }, + }, + { + // Alphabetical order (a, b, c) deliberately conflicts with topological + // order: b is the root, and c depends on both a and b so it sits two + // levels deep. Locks that ordering follows depth, not global sort. + name: "alphaTopoConflict", + input: { b: [], a: ["b"], c: ["a", "b"] }, + expected: { ok: true, levels: [["b"], ["a"], ["c"]], order: ["b", "a", "c"] }, + }, + { + name: "cycle", + input: { a: ["b"], b: ["a"] }, + expected: { + ok: false, + errors: [ + { + type: "cycle", + message: "Circular dependency detected: a -> b -> a", + nodes: ["a", "b"], + }, + ], + }, + }, + { + name: "selfDep", + input: { a: ["a"] }, + expected: { + ok: false, + errors: [{ type: "cycle", message: 'Step "a" depends on itself', nodes: ["a"] }], + }, + }, + { + name: "missingDep", + input: { a: ["ghost"] }, + expected: { + ok: false, + errors: [ + { + type: "missing_dependency", + message: 'Step "a" depends on "ghost" which does not exist', + nodes: ["a", "ghost"], + }, + ], + }, + }, + ]; + + it.each(cases)("reproduces the baseline for $name", ({ input, expected }) => { + expect(resolve(steps(input))).toEqual(expected); + }); + + it("preserves alphabetical ordering within a level on a wide chain", () => { + // 26 siblings under one root: the level must be alphabetically sorted, + // the property the per-level sort guaranteed before optimisation. + const input: Record = { root: [] }; + const letters = "abcdefghijklmnopqrstuvwxyz".split(""); + for (const ch of letters) input[`n_${ch}`] = ["root"]; + const result = resolve(steps(input)); + if (!result.ok) throw new Error("expected ok"); + expect(result.levels[0]).toEqual(["root"]); + expect(result.levels[1]).toEqual(letters.map((c) => `n_${c}`)); + }); +}); diff --git a/packages/core/dag.ts b/packages/core/dag.ts index d7b0c53..43d4bfe 100644 --- a/packages/core/dag.ts +++ b/packages/core/dag.ts @@ -116,15 +116,22 @@ export function resolve(steps: Record): DagResult { } } + // The initial queue is seeded from `names` (already sorted) in step above, so + // it starts in alphabetical order. Walk it with a FIFO index pointer instead + // of re-sorting on every pop. The output (levels and order) is derived from + // the depth map and the per-level alphabetical sort below, both independent + // of dequeue order, so this yields byte-identical output (issue #46, + // Candidate 3). depth is the longest path from a root and is order-invariant: + // a node is dequeued only after every predecessor has been processed, so its + // depth is final when read. const sorted: string[] = []; - - while (queue.length > 0) { - queue.sort(); - const current = queue.shift()!; + let head = 0; + while (head < queue.length) { + const current = queue[head++]!; sorted.push(current); const d = depth.get(current)!; - for (const dep of dependents.get(current)!.slice().sort()) { + for (const dep of dependents.get(current)!) { const newDeg = inDegree.get(dep)! - 1; inDegree.set(dep, newDeg); depth.set(dep, Math.max(depth.get(dep) ?? 0, d + 1)); @@ -168,11 +175,16 @@ export function resolve(steps: Record): DagResult { if (errors.length > 0) return { ok: false, errors }; - // 5. Group by depth level - const maxDepth = Math.max(...sorted.map((n) => depth.get(n)!), -1); + // 5. Group by depth level. Iterating the pre-sorted `names` drops each node + // into its depth bucket in alphabetical order, reproducing the per-level sort + // in a single O(V) pass instead of an O(V*D) filter-and-sort per level. + const maxDepth = Math.max(...names.map((n) => depth.get(n)!), -1); const levels: string[][] = []; for (let d = 0; d <= maxDepth; d++) { - levels.push(sorted.filter((n) => depth.get(n) === d).sort()); + levels.push([]); + } + for (const name of names) { + levels[depth.get(name)!]!.push(name); } return { ok: true, levels, order: levels.flat() };