perf(#307): use head-index queue for phase dependency BFS (O(V^2) -> O(V+E)) (#383)

The Pass-2 topological level assignment in cmdPhasePlanIndex dequeued its
Kahn's-algorithm queue with Array.shift(), which is O(n) per call in V8, so the
BFS was O(V^2) and slowed superlinearly on deep queues (wide fan-in plan
graphs). Extract the traversal into a pure, exported computeDependencyLevels
(rawPlans, planMap, canonicalToId) and dequeue via a head index (queue[head++])
-> O(V+E). Behavior is identical: same FIFO order, same longest-path levels,
same visited-count cycle detection. A complexity-contract comment above the loop
documents why shift() must not be reintroduced.

Adds tests/phase-dependency-levels.test.cjs with deterministic behavior and
edge-case coverage (linear chain, diamond longest-path, independent set, cycle,
canonical-prefix resolution, empty, self-loop, duplicate edge, external dep). A
timing-based complexity guard was intentionally omitted: the O(V+E) Map-build
constant dilutes the O(V^2) signal until impractical N (~1e6), so an empirical
guard is inherently flaky on contended CI — the contract is enforced by the
inline comment and correctness tests instead.

Fixes #307

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.changeset/jolly-moles-climb.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 383
+---
+Phase dependency level assignment (gsd-tools phase-plan-index) now dequeues its Kahns-algorithm BFS via a head index instead of Array.shift(), fixing O(V^2) behavior that slowed superlinearly on wide fan-in plan graphs (Array.shift() is O(n) per call in V8). Now O(V+E); behavior is unchanged (same topological levels, same cycle detection). Fixes #307.

get-shit-done/bin/lib/phase.cjs

@@ -353,6 +353,64 @@ function extractObjective(content) {
   return m ? m[1].trim() : null;
 }
 
+// O(V + E). Assigns each in-phase plan its longest-path topological level over the
+// in-phase dependsOn DAG (Kahn's algorithm). Returns { level: Map<id,number>, visited: number }.
+// visited < rawPlans.length signals a dependency cycle.
+function computeDependencyLevels(rawPlans, planMap, canonicalToId) {
+  // Kahn's algorithm — compute in-degree and adjacency for in-phase deps only.
+  const level = new Map();
+  const inDeg = new Map();
+  const adj = new Map();
+
+  for (const p of rawPlans) {
+    if (!inDeg.has(p.id)) inDeg.set(p.id, 0);
+    if (!adj.has(p.id)) adj.set(p.id, []);
+    for (const dep of p.dependsOn) {
+      // Accept both full-stem ('03-01-auth-hardening') and canonical-prefix ('03-01') forms.
+      // All lookups are lowercased so mixed-case depends_on refs resolve correctly (#3785).
+      const depLower = dep.toLowerCase();
+      const resolvedDep = planMap.has(depLower) ? planMap.get(depLower).id : canonicalToId.get(depLower);
+      if (!resolvedDep) continue; // external dep — ignore
+      if (!adj.has(resolvedDep)) adj.set(resolvedDep, []);
+      adj.get(resolvedDep).push(p.id);
+      inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1);
+    }
+  }
+
+  // Start with nodes that have no in-phase dependencies.
+  const queue = [];
+  for (const p of rawPlans) {
+    if ((inDeg.get(p.id) ?? 0) === 0) {
+      queue.push(p.id);
+      level.set(p.id, 0);
+    }
+  }
+
+  // Dequeue by head index (queue[head++]), NOT Array.shift(): shift() is O(n) per
+  // call in V8 (it re-indexes the backing store), which would make this Kahn's BFS
+  // O(V^2) on deep queues (e.g. wide fan-in graphs). Head-index dequeue is O(1)
+  // amortized -> O(V+E) overall. Do not "simplify" this back to queue.shift(). (#307)
+  let head = 0;
+  let visited = 0;
+  while (head < queue.length) {
+    const cur = queue[head++];
+    visited++;
+    const curLevel = level.get(cur);
+    for (const dep of (adj.get(cur) ?? [])) {
+      const newLevel = curLevel + 1;
+      if (newLevel > (level.get(dep) ?? -1)) {
+        level.set(dep, newLevel);
+      }
+      inDeg.set(dep, inDeg.get(dep) - 1);
+      if (inDeg.get(dep) === 0) {
+        queue.push(dep);
+      }
+    }
+  }
+
+  return { level, visited };
+}
+
 function cmdPhasePlanIndex(cwd, phase, raw) {
   if (!phase) {
     error('phase required for phase-plan-index');
@@ -496,51 +554,7 @@ function cmdPhasePlanIndex(cwd, phase, raw) {
   // like '01' or '01A'. Adding the third tier here is tracked as a parity
   // gap and is out of scope for #3785 / PR #3798.
 
-  // Kahn's algorithm — compute in-degree and adjacency for in-phase deps only.
-  const level = new Map();
-  const inDeg = new Map();
-  const adj = new Map();
-
-  for (const p of rawPlans) {
-    if (!inDeg.has(p.id)) inDeg.set(p.id, 0);
-    if (!adj.has(p.id)) adj.set(p.id, []);
-    for (const dep of p.dependsOn) {
-      // Accept both full-stem ('03-01-auth-hardening') and canonical-prefix ('03-01') forms.
-      // All lookups are lowercased so mixed-case depends_on refs resolve correctly (#3785).
-      const depLower = dep.toLowerCase();
-      const resolvedDep = planMap.has(depLower) ? planMap.get(depLower).id : canonicalToId.get(depLower);
-      if (!resolvedDep) continue; // external dep — ignore
-      if (!adj.has(resolvedDep)) adj.set(resolvedDep, []);
-      adj.get(resolvedDep).push(p.id);
-      inDeg.set(p.id, (inDeg.get(p.id) ?? 0) + 1);
-    }
-  }
-
-  // Start with nodes that have no in-phase dependencies.
-  const queue = [];
-  for (const p of rawPlans) {
-    if ((inDeg.get(p.id) ?? 0) === 0) {
-      queue.push(p.id);
-      level.set(p.id, 0);
-    }
-  }
-
-  let visited = 0;
-  while (queue.length > 0) {
-    const cur = queue.shift();
-    visited++;
-    const curLevel = level.get(cur);
-    for (const dep of (adj.get(cur) ?? [])) {
-      const newLevel = curLevel + 1;
-      if (newLevel > (level.get(dep) ?? -1)) {
-        level.set(dep, newLevel);
-      }
-      inDeg.set(dep, inDeg.get(dep) - 1);
-      if (inDeg.get(dep) === 0) {
-        queue.push(dep);
-      }
-    }
-  }
+  const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
 
   // Cycle detection — any node not visited has a cycle.
   if (visited < rawPlans.length) {
@@ -1555,4 +1569,5 @@ module.exports = {
   cmdPhaseInsert,
   cmdPhaseRemove,
   cmdPhaseComplete,
+  computeDependencyLevels,
 };

tests/phase-dependency-levels.test.cjs

@@ -0,0 +1,154 @@
+// allow-test-rule: source-text-is-the-product
+// Tests for the extracted computeDependencyLevels pure function in phase.cjs.
+// Covers correctness (behavior) and edge cases. The O(V+E) complexity contract
+// is documented inline in computeDependencyLevels (phase.cjs) above the head-index
+// queue loop; timing-based guards were removed (#307) because the O(V+E)
+// Map-build constant dilutes the O(V^2) signal until N is ~1e6, making
+// empirical ratio tests inherently flaky on contended CI runners.
+
+'use strict';
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const { computeDependencyLevels } = require('../get-shit-done/bin/lib/phase.cjs');
+
+// Helper: build rawPlans + planMap + canonicalToId from a simple spec.
+// spec is an array of { id, dependsOn } objects.
+function buildInputs(spec) {
+  const rawPlans = spec.map(s => ({ id: s.id, dependsOn: s.dependsOn ?? [] }));
+  const planMap = new Map(rawPlans.map(p => [p.id.toLowerCase(), p]));
+  const canonicalToId = new Map(rawPlans.map(p => [p.id.toLowerCase(), p.id]));
+  return { rawPlans, planMap, canonicalToId };
+}
+
+describe('computeDependencyLevels — behavior tests', () => {
+
+  // (a) Linear chain: 0 ← 1 ← 2  (1 depends on 0, 2 depends on 1)
+  test('(a) linear chain assigns levels 0,1,2 and visits all nodes', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'p0', dependsOn: [] },
+      { id: 'p1', dependsOn: ['p0'] },
+      { id: 'p2', dependsOn: ['p1'] },
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 3);
+    assert.equal(level.get('p0'), 0);
+    assert.equal(level.get('p1'), 1);
+    assert.equal(level.get('p2'), 2);
+  });
+
+  // (b) Diamond: A; B,C depend on A; D depends on B and C → longest-path levels
+  // A=0, B=1, C=1, D=2
+  test('(b) diamond uses longest-path (D=2, not 1)', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'A', dependsOn: [] },
+      { id: 'B', dependsOn: ['A'] },
+      { id: 'C', dependsOn: ['A'] },
+      { id: 'D', dependsOn: ['B', 'C'] },
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 4);
+    assert.equal(level.get('A'), 0);
+    assert.equal(level.get('B'), 1);
+    assert.equal(level.get('C'), 1);
+    assert.equal(level.get('D'), 2);
+  });
+
+  // (c) Independent set: no deps → all level 0
+  test('(c) independent set: all nodes at level 0, all visited', () => {
+    const N = 10;
+    const spec = Array.from({ length: N }, (_, i) => ({ id: `node-${i}`, dependsOn: [] }));
+    const { rawPlans, planMap, canonicalToId } = buildInputs(spec);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, N);
+    for (let i = 0; i < N; i++) {
+      assert.equal(level.get(`node-${i}`), 0, `node-${i} should be level 0`);
+    }
+  });
+
+  // (d) Cycle: A depends on B and B depends on A → visited < N
+  test('(d) cycle: visited < N signals cycle (not all nodes reachable)', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'A', dependsOn: ['B'] },
+      { id: 'B', dependsOn: ['A'] },
+    ]);
+    const { visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.ok(visited < rawPlans.length, `expected visited < 2, got ${visited}`);
+  });
+
+  // (e) Prefix resolution via canonicalToId: dep given as canonical prefix resolves
+  // to the full plan ID. Mirrors the #3785 behavior.
+  test('(e) canonical prefix resolution (depends_on short form resolves via canonicalToId)', () => {
+    // Plan with full stem ID; dep references canonical prefix only
+    const rawPlans = [
+      { id: '03-01-auth-hardening', dependsOn: [] },
+      { id: '03-02-token-rotation', dependsOn: ['03-01'] }, // short prefix dep
+    ];
+    // planMap uses full-stem lowercase keys (planMap.get('03-01') would miss)
+    const planMap = new Map(rawPlans.map(p => [p.id.toLowerCase(), p]));
+    // canonicalToId maps prefix → full ID
+    const canonicalToId = new Map([
+      ['03-01', '03-01-auth-hardening'],
+      ['03-02', '03-02-token-rotation'],
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 2, 'both plans should be visited (no cycle)');
+    assert.equal(level.get('03-01-auth-hardening'), 0);
+    assert.equal(level.get('03-02-token-rotation'), 1, 'prefix dep resolved → level 1');
+  });
+
+  // (f) EMPTY: no plans → level.size === 0 and visited === 0, no throw.
+  test('(f) empty rawPlans: returns empty level map and visited === 0', () => {
+    const { level, visited } = computeDependencyLevels([], new Map(), new Map());
+    assert.equal(level.size, 0, 'level map should be empty');
+    assert.equal(visited, 0, 'visited should be 0 with no plans');
+  });
+
+  // (g) SELF-LOOP: a plan whose dependsOn includes its own id → in-degree is never
+  // decremented to 0 → the plan is never enqueued → visited === 0 (cycle signalled).
+  // Self-dep: inDeg starts at 0, then gets +1 for the self-edge → inDeg = 1 forever.
+  test('(g) self-loop: plan is never enqueued, visited === 0 (cycle signalled)', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'solo', dependsOn: ['solo'] },
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 0, 'self-loop plan should never be visited');
+    assert.ok(visited < rawPlans.length, 'visited < rawPlans.length signals cycle');
+    assert.ok(!level.has('solo'), 'self-loop plan should not appear in level map');
+  });
+
+  // (h) DUPLICATE EDGE: plan B lists the same dep A twice.
+  // Effect: inDeg(B) = 2 (double-counted), adj(A) = ['B', 'B'] (double-pushed).
+  // When A is processed (curLevel=0):
+  //   First 'B': inDeg(B) → 1, level(B) set to 1 (not pushed yet).
+  //   Second 'B': inDeg(B) → 0, level(B) stays 1 (already set), B pushed.
+  // Result: visited === 2, level(A) === 0, level(B) === 1.
+  // This documents the existing behavior: duplicate deps double-count in-degree and
+  // double-push the adjacency list, but the final level/visited result is still correct
+  // because each decrement pairs with a push.
+  test('(h) duplicate edge: B lists dep A twice → visited === 2, levels A=0 B=1', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'A', dependsOn: [] },
+      { id: 'B', dependsOn: ['A', 'A'] }, // duplicate dep
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 2, 'both A and B should be visited');
+    assert.equal(level.get('A'), 0, 'A has no deps → level 0');
+    assert.equal(level.get('B'), 1, 'B depends on A → level 1');
+  });
+
+  // (i) EXTERNAL/UNRESOLVED DEP: plan B lists a dep that is neither in planMap nor in
+  // canonicalToId → the dep is ignored (continue), B retains in-degree 0 → B is level 0.
+  // Both A and B have in-degree 0 and are visited → visited === 2.
+  test('(i) unresolved/external dep is ignored: B still enqueued at level 0', () => {
+    const { rawPlans, planMap, canonicalToId } = buildInputs([
+      { id: 'A', dependsOn: [] },
+      { id: 'B', dependsOn: ['nonexistent-external-plan'] },
+    ]);
+    const { level, visited } = computeDependencyLevels(rawPlans, planMap, canonicalToId);
+    assert.equal(visited, 2, 'both plans should be visited (external dep ignored)');
+    assert.equal(level.get('A'), 0, 'A has no deps → level 0');
+    assert.equal(level.get('B'), 0, 'B external dep ignored → in-degree 0 → level 0');
+  });
+
+});