From 4774a1df22924f56d66219f7d6a24b21d1b5da9e Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Mon, 16 Mar 2026 18:09:06 -0500 Subject: [PATCH] =?UTF-8?q?fix(roadmap):=20expand=20range=20syntax=20in=20?= =?UTF-8?q?depends=20(S01-S04=20=E2=86=92=20S01,S02,S03,S04)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LLMs frequently write depends:[S01-S04] as natural shorthand. The parser split only on commas, so this produced a single literal element "S01-S04" that never matched any real slice ID — permanently blocking the slice with "No slice eligible". Changes: roadmap-slices.ts: - Add expandDependencies() helper — after comma-split, detect dep tokens matching /^PrefixN(-|..)PrefixM$/ and expand to individual IDs. Handles S01-S04 (dash range) and S01..S04 (dot-range). Zero-padding preserved. Mismatched prefixes and reversed ranges pass through unchanged. - Wire into parseRoadmapSlices() after the comma-split step. - Export for direct testing. doctor.ts: - Add "unresolvable_dependency" warning code. - In the slice audit loop, check each dep against the set of known slice IDs in the roadmap. Fires a warning with the bad dep name and the correct format hint. Catches leftover range IDs on roadmaps that were written before this fix, and catches typos. plan-milestone.md prompt: - Add explicit rule: use comma-separated depends:[S01,S02,S03], never range syntax. Defense-in-depth so LLMs don't generate the problem. Tests: - roadmap-slices.test.ts: 10 new expandDependencies cases + 2 parseRoadmapSlices integration cases (range + comma round-trip). - doctor.test.ts: unresolvable_dependency fires for unknown dep S99, does not fire for valid S01 dep. 952/952 unit tests pass. Closes #737 --- src/resources/extensions/gsd/doctor.ts | 21 ++++++- .../extensions/gsd/prompts/plan-milestone.md | 1 + .../extensions/gsd/roadmap-slices.ts | 42 +++++++++++++- .../extensions/gsd/tests/doctor.test.ts | 58 +++++++++++++++++++ .../gsd/tests/roadmap-slices.test.ts | 44 +++++++++++++- 5 files changed, 163 insertions(+), 3 deletions(-) diff --git a/src/resources/extensions/gsd/doctor.ts b/src/resources/extensions/gsd/doctor.ts index d54991b78..ca28fbce0 100644 --- a/src/resources/extensions/gsd/doctor.ts +++ b/src/resources/extensions/gsd/doctor.ts @@ -41,7 +41,8 @@ export type DoctorIssueCode = | "activity_log_bloat" | "state_file_stale" | "state_file_missing" - | "gitignore_missing_patterns"; + | "gitignore_missing_patterns" + | "unresolvable_dependency"; export interface DoctorIssue { severity: DoctorSeverity; @@ -1041,6 +1042,24 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean; }); } + // Check for unresolvable dependency IDs — catches range syntax like "S01-S04" + // that the parser expanded but that don't match any actual slice in the roadmap. + // Also catches plain typos or IDs referencing slices not yet defined. + const knownSliceIds = new Set(roadmap.slices.map(s => s.id)); + for (const dep of slice.depends) { + if (!knownSliceIds.has(dep)) { + issues.push({ + severity: "warning", + code: "unresolvable_dependency", + scope: "slice", + unitId, + message: `Slice ${unitId} depends on "${dep}" which is not a slice ID in this roadmap. This permanently blocks the slice. Use comma-separated IDs: \`depends:[S01,S02]\``, + file: relMilestoneFile(basePath, milestoneId, "ROADMAP"), + fixable: false, + }); + } + } + const slicePath = resolveSlicePath(basePath, milestoneId, slice.id); if (!slicePath) continue; diff --git a/src/resources/extensions/gsd/prompts/plan-milestone.md b/src/resources/extensions/gsd/prompts/plan-milestone.md index ea70a0467..18521ae4c 100644 --- a/src/resources/extensions/gsd/prompts/plan-milestone.md +++ b/src/resources/extensions/gsd/prompts/plan-milestone.md @@ -51,6 +51,7 @@ Apply these when decomposing and ordering slices: - **Completion must imply capability.** If every slice in this roadmap were completed exactly as written, the milestone's promised outcome should actually work at the proof level claimed. Do not write slices that can all be checked off while the user-visible capability still does not exist. - **Don't invent risks.** If the project is straightforward, skip the proof strategy and just ship value in smart order. Not everything has major unknowns. - **Ship features, not proofs.** A completed slice should leave the product in a state where the new capability is actually usable through its real interface. A login flow slice ends with a working login page, not a middleware function. An API slice ends with endpoints that return real data from a real store, not hardcoded fixtures. A dashboard slice ends with a real dashboard rendering real data, not a component that renders mock props. If a slice can't ship the real thing yet because a dependency isn't built, it should ship with realistic stubs that are clearly marked for replacement — but the user-facing surface must be real. +- **Dependency format is comma-separated, never range syntax.** Write `depends:[S01,S02,S03]` — not `depends:[S01-S03]`. Range syntax is not a valid format and permanently blocks the slice. - **Ambition matches the milestone.** The number and depth of slices should match the milestone's ambition. A milestone promising "core platform with auth, data model, and primary user loop" should have enough slices to actually deliver all three as working features — not two proof-of-concept slices and a note that "the rest will come in the next milestone." If the milestone's context promises an outcome, the roadmap must deliver it. - **Right-size the decomposition.** Match slice count to actual complexity. If the work is small enough to build and verify in one pass, it's one slice — don't split it into three just because you can identify sub-steps. Multiple requirements can share a single slice. Conversely, don't cram genuinely independent capabilities into one slice just to keep the count low. Let the work dictate the structure. diff --git a/src/resources/extensions/gsd/roadmap-slices.ts b/src/resources/extensions/gsd/roadmap-slices.ts index 2da593198..9eed68e27 100644 --- a/src/resources/extensions/gsd/roadmap-slices.ts +++ b/src/resources/extensions/gsd/roadmap-slices.ts @@ -1,5 +1,45 @@ import type { RoadmapSliceEntry, RiskLevel } from "./types.js"; +/** + * Expand dependency shorthand into individual slice IDs. + * + * Handles two common LLM-generated patterns that the roadmap parser + * previously treated as single literal IDs (silently blocking slices): + * + * "S01-S04" → ["S01", "S02", "S03", "S04"] (range syntax) + * "S01..S04" → ["S01", "S02", "S03", "S04"] (dot-range syntax) + * + * Plain IDs ("S01", "S02") and empty strings pass through unchanged. + */ +export function expandDependencies(deps: string[]): string[] { + const result: string[] = []; + for (const dep of deps) { + const trimmed = dep.trim(); + if (!trimmed) continue; + + // Match range syntax: S01-S04 or S01..S04 (case-insensitive prefix) + const rangeMatch = trimmed.match(/^([A-Za-z]+)(\d+)(?:-|\.\.)+([A-Za-z]+)(\d+)$/); + if (rangeMatch) { + const prefixA = rangeMatch[1]!.toUpperCase(); + const startNum = parseInt(rangeMatch[2]!, 10); + const prefixB = rangeMatch[3]!.toUpperCase(); + const endNum = parseInt(rangeMatch[4]!, 10); + + // Only expand when both prefixes match and range is valid + if (prefixA === prefixB && startNum <= endNum) { + const width = rangeMatch[2]!.length; // preserve zero-padding (S01 not S1) + for (let i = startNum; i <= endNum; i++) { + result.push(`${prefixA}${String(i).padStart(width, "0")}`); + } + continue; + } + } + + result.push(trimmed); + } + return result; +} + function extractSlicesSection(content: string): string { const headingMatch = /^## Slices\s*$/m.exec(content); if (!headingMatch || headingMatch.index == null) return ""; @@ -33,7 +73,7 @@ export function parseRoadmapSlices(content: string): RoadmapSliceEntry[] { const depsMatch = rest.match(/`depends:\[([^\]]*)\]`/); const depends = depsMatch && depsMatch[1]!.trim() - ? depsMatch[1]!.split(",").map(s => s.trim()) + ? expandDependencies(depsMatch[1]!.split(",").map(s => s.trim())) : []; currentSlice = { id, title, risk, depends, done, demo: "" }; diff --git a/src/resources/extensions/gsd/tests/doctor.test.ts b/src/resources/extensions/gsd/tests/doctor.test.ts index 7a2f47a8c..12c19c042 100644 --- a/src/resources/extensions/gsd/tests/doctor.test.ts +++ b/src/resources/extensions/gsd/tests/doctor.test.ts @@ -585,6 +585,64 @@ Discovered an issue. rmSync(dtBase, { recursive: true, force: true }); } + // ─── unresolvable_dependency: range syntax dep warns ───────────────── + console.log("\n=== doctor: unresolvable_dependency warns for leftover range ID ==="); + { + // Simulate a roadmap where expandDependencies did NOT expand (pre-fix stored artifact) + // by writing a dep that looks like a range but doesn't match any real slice. + const base = mkdtempSync(join(tmpdir(), "gsd-doctor-udep-")); + const mDir2 = join(base, ".gsd", "milestones", "M001"); + const sDir2 = join(mDir2, "slices", "S01"); + const tDir2 = join(sDir2, "tasks"); + mkdirSync(tDir2, { recursive: true }); + writeFileSync(join(mDir2, "M001-ROADMAP.md"), [ + "# M001: Test", + "", + "## Slices", + "- [x] **S01: Done** `risk:low` `depends:[]`", + " > After this: done", + "- [ ] **S02: Blocked** `risk:low` `depends:[S99]`", + " > After this: also done", + ].join("\n") + "\n"); + writeFileSync(join(sDir2, "S01-PLAN.md"), "# S01\n\n**Goal:** g\n**Demo:** d\n\n## Tasks\n- [x] **T01: t** `est:5m`\n"); + writeFileSync(join(tDir2, "T01-SUMMARY.md"), "---\nid: T01\nparent: S01\nmilestone: M001\n---\n# T01\n## What Happened\nDone.\n"); + + const r = await runGSDDoctor(base, { fix: false }); + const udepIssues = r.issues.filter(i => i.code === "unresolvable_dependency"); + assertTrue(udepIssues.length > 0, "unresolvable_dependency fires for unknown dep S99"); + assertEq(udepIssues[0]?.severity, "warning", "severity is warning"); + assertTrue(udepIssues[0]?.message.includes("S99"), "message names the bad dep"); + + rmSync(base, { recursive: true, force: true }); + } + + // ─── unresolvable_dependency: valid deps do not warn ───────────────── + console.log("\n=== doctor: no unresolvable_dependency for valid deps ==="); + { + const base = mkdtempSync(join(tmpdir(), "gsd-doctor-udep-ok-")); + const mDir2 = join(base, ".gsd", "milestones", "M001"); + const sDir2 = join(mDir2, "slices", "S01"); + const tDir2 = join(sDir2, "tasks"); + mkdirSync(tDir2, { recursive: true }); + writeFileSync(join(mDir2, "M001-ROADMAP.md"), [ + "# M001: Test", + "", + "## Slices", + "- [x] **S01: Done** `risk:low` `depends:[]`", + " > After this: done", + "- [ ] **S02: Next** `risk:low` `depends:[S01]`", + " > After this: next done", + ].join("\n") + "\n"); + writeFileSync(join(sDir2, "S01-PLAN.md"), "# S01\n\n**Goal:** g\n**Demo:** d\n\n## Tasks\n- [x] **T01: t** `est:5m`\n"); + writeFileSync(join(tDir2, "T01-SUMMARY.md"), "---\nid: T01\nparent: S01\nmilestone: M001\n---\n# T01\n## What Happened\nDone.\n"); + + const r = await runGSDDoctor(base, { fix: false }); + const udepIssues = r.issues.filter(i => i.code === "unresolvable_dependency"); + assertEq(udepIssues.length, 0, "no unresolvable_dependency for valid S01 dep"); + + rmSync(base, { recursive: true, force: true }); + } + report(); } diff --git a/src/resources/extensions/gsd/tests/roadmap-slices.test.ts b/src/resources/extensions/gsd/tests/roadmap-slices.test.ts index 2b0ef045e..dd7546098 100644 --- a/src/resources/extensions/gsd/tests/roadmap-slices.test.ts +++ b/src/resources/extensions/gsd/tests/roadmap-slices.test.ts @@ -1,5 +1,5 @@ import { parseRoadmap } from "../files.ts"; -import { parseRoadmapSlices } from "../roadmap-slices.ts"; +import { parseRoadmapSlices, expandDependencies } from "../roadmap-slices.ts"; import { createTestContext } from './test-helpers.ts'; const { assertEq, assertTrue, report } = createTestContext(); @@ -38,4 +38,46 @@ assertEq(roadmap.title, "M003: Current", "roadmap title preserved"); assertEq(roadmap.vision, "Build the thing.", "roadmap vision preserved"); assertTrue(roadmap.boundaryMap.length === 1, "boundary map still parsed"); +// ─── expandDependencies unit tests ───────────────────────────────────── + +console.log("\n=== expandDependencies: plain IDs pass through ==="); +assertEq(expandDependencies([]), [], "empty list"); +assertEq(expandDependencies(["S01"]), ["S01"], "single plain ID"); +assertEq(expandDependencies(["S01", "S03"]), ["S01", "S03"], "multiple plain IDs"); + +console.log("\n=== expandDependencies: dash range expansion ==="); +assertEq(expandDependencies(["S01-S04"]), ["S01", "S02", "S03", "S04"], "S01-S04 expands correctly"); +assertEq(expandDependencies(["S01-S01"]), ["S01"], "single-element range"); +assertEq(expandDependencies(["S03-S05"]), ["S03", "S04", "S05"], "mid-range expansion"); + +console.log("\n=== expandDependencies: dot-range expansion ==="); +assertEq(expandDependencies(["S01..S03"]), ["S01", "S02", "S03"], "S01..S03 dot range"); + +console.log("\n=== expandDependencies: zero-padding preserved ==="); +assertEq(expandDependencies(["S01-S03"]), ["S01", "S02", "S03"], "zero-padded IDs preserved"); + +console.log("\n=== expandDependencies: mixed list ==="); +assertEq(expandDependencies(["S01-S03", "S05"]), ["S01", "S02", "S03", "S05"], "range + plain mixed"); + +console.log("\n=== expandDependencies: invalid range passes through unchanged ==="); +assertEq(expandDependencies(["S04-S01"]), ["S04-S01"], "reversed range not expanded (start > end)"); +assertEq(expandDependencies(["S01-T04"]), ["S01-T04"], "mismatched prefix not expanded"); + +// ─── parseRoadmapSlices: range syntax in depends ───────────────────── + +console.log("\n=== parseRoadmapSlices: range syntax in depends expanded ==="); +{ + const rangeContent = `# M016: Test\n\n## Slices\n- [x] **S01: A** \`risk:low\` \`depends:[]\`\n- [x] **S02: B** \`risk:low\` \`depends:[]\`\n- [x] **S03: C** \`risk:low\` \`depends:[]\`\n- [x] **S04: D** \`risk:low\` \`depends:[]\`\n- [ ] **S05: E** \`risk:low\` \`depends:[S01-S04]\`\n > After this: all done\n`; + const rangeSlices = parseRoadmapSlices(rangeContent); + assertEq(rangeSlices.length, 5, "5 slices parsed"); + assertEq(rangeSlices[4]?.depends, ["S01", "S02", "S03", "S04"], "S01-S04 range expanded to individual IDs"); +} + +console.log("\n=== parseRoadmapSlices: comma-separated depends still works ==="); +{ + const commaContent = `# M001: Test\n\n## Slices\n- [ ] **S05: E** \`risk:low\` \`depends:[S01,S02,S03,S04]\`\n > After this: done\n`; + const commaSlices = parseRoadmapSlices(commaContent); + assertEq(commaSlices[0]?.depends, ["S01", "S02", "S03", "S04"], "comma-separated depends unchanged"); +} + report();