perf(#314): index roadmap plan lookups by id (O(lines×plans) → O(lines+plans)) (#396)

Hot path in cmdRoadmapAnnotateDependencies called planData.find() on every
checklist line. Replaced with a first-wins Map built once before the loop so
each line resolves in O(1); first-wins preserves exact .find() semantics and
null-on-miss → wave-1 default is unchanged.

Fixes #314

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-27 20:21:46 -04:00
committed by GitHub
parent 8c8887f00e
commit 4361d83279
3 changed files with 54 additions and 1 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 314
---
`roadmap annotate-dependencies` now indexes plan data by ID in a Map before the checklist loop, reducing plan lookup from O(lines×plans) to O(lines+plans).

View File

@@ -572,6 +572,14 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) {
if (listLines.length === 0) return;
// #314 perf: build a first-wins Map so per-line lookup is O(1) instead of O(plans).
// First-wins mirrors .find() semantics: if the same planId appears more than once
// in planData, the earlier entry wins — identical to what .find() returned before.
const planById = new Map();
for (const p of planData) {
if (!planById.has(p.planId)) planById.set(p.planId, p);
}
// Build wave-annotated plan list
const linesByWave = new Map();
for (const line of listLines) {
@@ -586,7 +594,7 @@ function cmdRoadmapAnnotateDependencies(cwd, phaseNum, raw) {
// (e.g. `.invalid-PLAN.md`) would silently default to wave 1 — defensively
// skip the line instead so corrupted ROADMAP entries don't corrupt wave layout.
if (planId && !/^\w[\w.-]*$/.test(planId)) continue;
const planEntry = planId ? planData.find(p => p.planId === planId) : null;
const planEntry = planId ? (planById.get(planId) || null) : null;
const wave = planEntry ? planEntry.wave : 1;
if (!linesByWave.has(wave)) linesByWave.set(wave, []);
linesByWave.get(wave).push(line);

View File

@@ -217,6 +217,46 @@ Plans:
assert.ok(typeof out.updated === 'boolean', 'should return a valid result object');
});
test('#314 map-lookup: found-path uses plan wave, miss-path defaults to wave 1', () => {
// Behavior lock for the O(1) Map swap: asserts BOTH branches of the lookup.
// - 01-01-PLAN.md is in planData (wave 2) → checklist line must land under Wave 2.
// - 01-99-PLAN.md is NOT in planData → null-on-miss → defaults to wave 1.
tmpDir = makePlanProject({
'.planning/ROADMAP.md': `# Roadmap
### Phase 1: Foundation
**Goal:** Set up project
**Plans:** 2 plans
Plans:
- [ ] 01-01-PLAN.md — Known plan
- [ ] 01-99-PLAN.md — Unknown plan (no PLAN.md)
`,
'.planning/phases/01-foundation/01-01-PLAN.md': PLAN_TEMPLATE(2),
// 01-99-PLAN.md intentionally absent — simulates a checklist entry with no backing plan file
});
const result = runGsdTools('roadmap annotate-dependencies 1', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
// Both waves must be present (wave 1 from the miss, wave 2 from the found entry)
assert.ok(roadmap.includes('**Wave 1**'), 'Wave 1 header present (miss-path default)');
assert.ok(roadmap.includes('**Wave 2**'), 'Wave 2 header present (found-path)');
// Known plan (wave 2) must appear AFTER Wave 2 header
const wave2Idx = roadmap.indexOf('**Wave 2**');
const knownLineIdx = roadmap.indexOf('01-01-PLAN.md');
assert.ok(knownLineIdx > wave2Idx, 'known plan line grouped under Wave 2');
// Unknown plan (wave 1 default) must appear AFTER Wave 1 header and BEFORE Wave 2 header
const wave1Idx = roadmap.indexOf('**Wave 1**');
const unknownLineIdx = roadmap.indexOf('01-99-PLAN.md');
assert.ok(unknownLineIdx > wave1Idx, 'unknown plan line grouped under Wave 1');
assert.ok(unknownLineIdx < wave2Idx, 'unknown plan line appears before Wave 2 section');
});
test('plan-phase.md documents annotate-dependencies step', () => {
const planPhase = fs.readFileSync(
path.join(__dirname, '../get-shit-done/workflows/plan-phase.md'), 'utf-8'