Merge pull request #745 from jeremymcs/fix/737-dependency-range-expansion
fix(roadmap): expand range syntax in depends (S01-S04 → S01,S02,S03,S04)
This commit is contained in:
commit
8ca9725bb0
5 changed files with 163 additions and 3 deletions
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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: "" };
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue