From 1a186013a4489ba318d2528f7b53f488f4a8d54c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 14 Jun 2026 10:46:22 -0400 Subject: [PATCH] fix(#1205): roadmapper applies phase_id_convention to generated phase IDs (#1215) * fix(#1205): roadmapper applies phase_id_convention to generated phase IDs - Add Phase ID Convention section to block: documents sequential (default) vs milestone-prefixed forms, and instructs the agent to read phase_id_convention from config.json - Update to show both header and checklist forms for sequential and milestone-prefixed conventions with examples (e.g. ### Phase 1-01: Name, - [ ] **Phase 1-01: Name**) - Add TDD regression test tests/bug-1205-roadmapper-convention.test.cjs (5 assertions, confirmed fail-first then pass after fix) - Update tests/agent-size-baseline.json to reflect legitimate growth - Add .changeset/brave-otters-leap.md (Fixed, pr:0 placeholder) Co-Authored-By: Claude Opus 4.8 (1M context) * chore: backfill changeset pr: 1215 for fix/1205 Co-Authored-By: Claude Opus 4.8 (1M context) * test(#1205): move phase_id_convention regression into roadmapper-granularity.test.cjs lint-regression-test-names rejects new standalone bug-NNNN-*.test.cjs files; regression cases must live in the owning module's test file. Move the 5 phase_id_convention assertions (#1205 regression) from the removed tests/bug-1205-roadmapper-convention.test.cjs into tests/roadmapper-granularity.test.cjs as a new describe block, alongside the existing granularity calibration tests. Also update the allow-test-rule comment to cover both #163 and #1205 surface contracts. Co-Authored-By: Claude Opus 4.8 (1M context) * test(#1205): fix lint-allow-test-rule-refs for roadmapper-granularity - Add issue ref (see #1205) to allow-test-rule comment in tests/roadmapper-granularity.test.cjs so lint-allow-test-rule-refs passes (new exemptions require #NNN per ADR-456) - Prune stale 'source-text-is-the-product' entry from scripts/lint-allow-test-rule-refs.allowlist.json (ratchet-down; comment now compliant and no longer needs grandfathering) Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .changeset/brave-otters-leap.md | 5 ++ agents/gsd-roadmapper.md | 55 ++++++++++++++- .../lint-allow-test-rule-refs.allowlist.json | 1 - tests/agent-size-baseline.json | 2 +- tests/roadmapper-granularity.test.cjs | 67 +++++++++++++++++-- 5 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 .changeset/brave-otters-leap.md diff --git a/.changeset/brave-otters-leap.md b/.changeset/brave-otters-leap.md new file mode 100644 index 000000000..236d268eb --- /dev/null +++ b/.changeset/brave-otters-leap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1215 +--- +**Roadmapper honors phase_id_convention** — new-project roadmaps now use milestone-prefixed phase IDs when phase_id_convention is set, instead of ignoring the default. (#1205) diff --git a/agents/gsd-roadmapper.md b/agents/gsd-roadmapper.md index 0984a06a1..00ad0d6c4 100644 --- a/agents/gsd-roadmapper.md +++ b/agents/gsd-roadmapper.md @@ -209,6 +209,23 @@ Track coverage as you go. - New milestone: Start at 1 - Continuing milestone: Check existing phases, start at last + 1 +## Phase ID Convention + +Read `phase_id_convention` from config.json. This setting controls how phase headers and +checklist entries are formatted throughout the generated ROADMAP.md. + +| Convention | Summary checklist form | Detail header form | +|---|---|---| +| `sequential` (default) | `- [ ] **Phase 1: Name**` | `### Phase 1: Name` | +| `milestone-prefixed` | `- [ ] **Phase 1-01: Name**` | `### Phase 1-01: Name` | + +When `phase_id_convention` is absent or set to `"sequential"`, use plain sequential phase IDs +(e.g. `Phase 1`, `Phase 2`). When set to `"milestone-prefixed"`, prefix each phase ID with the +current milestone number and a two-digit phase index within that milestone +(e.g. `Phase 1-01`, `Phase 1-02`, `Phase 2-01`). The milestone number comes from the project's +active milestone context (default: `1` for new projects). This ensures downstream tools that +parse `### Phase N-NN:` headers for milestone-scoped workflows receive correctly prefixed IDs. + ## Granularity Calibration Read granularity from config.json. Granularity controls compression tolerance. @@ -310,14 +327,30 @@ After roadmap creation, REQUIREMENTS.md gets updated with phase mappings: ### 1. Summary Checklist (under `## Phases`) +Use the form matching `phase_id_convention` from config. + +**Sequential (default — when absent or `"sequential"`):** + ```markdown - [ ] **Phase 1: Name** - One-line description - [ ] **Phase 2: Name** - One-line description - [ ] **Phase 3: Name** - One-line description ``` +**Milestone-prefixed (when `phase_id_convention: "milestone-prefixed"`):** + +```markdown +- [ ] **Phase 1-01: Name** - One-line description +- [ ] **Phase 1-02: Name** - One-line description +- [ ] **Phase 1-03: Name** - One-line description +``` + ### 2. Detail Sections (under `## Phase Details`) +Use the header form matching `phase_id_convention` from config. + +**Sequential (default):** + ```markdown ### Phase 1: Name **Goal**: What this phase delivers @@ -334,7 +367,25 @@ After roadmap creation, REQUIREMENTS.md gets updated with phase mappings: ... ``` -**The `### Phase X:` headers are parsed by downstream tools.** If you only write the summary checklist, phase lookups will fail. +**Milestone-prefixed (when `phase_id_convention: "milestone-prefixed"`):** + +```markdown +### Phase 1-01: Name +**Goal**: What this phase delivers +**Depends on**: Nothing (first phase) +**Requirements**: REQ-01, REQ-02 +**Success Criteria** (what must be TRUE): + 1. Observable behavior from user perspective + 2. Observable behavior from user perspective +**Plans**: TBD + +### Phase 1-02: Name +**Goal**: What this phase delivers +**Depends on**: Phase 1-01 +... +``` + +**The `### Phase X:` headers are parsed by downstream tools.** If you only write the summary checklist, phase lookups will fail. Use the correct form for the configured convention so downstream parsing succeeds. ### UI Phase Detection @@ -476,6 +527,8 @@ Apply phase identification methodology: 2. Identify dependencies between groups 3. Create phases that complete coherent capabilities 4. Check granularity setting for compression guidance +5. Read `phase_id_convention` from config (`sequential` or `milestone-prefixed`); apply the + matching header and checklist form throughout all output sections ## Step 5: Derive Success Criteria diff --git a/scripts/lint-allow-test-rule-refs.allowlist.json b/scripts/lint-allow-test-rule-refs.allowlist.json index 4c09d7fd7..1d623b611 100644 --- a/scripts/lint-allow-test-rule-refs.allowlist.json +++ b/scripts/lint-allow-test-rule-refs.allowlist.json @@ -291,7 +291,6 @@ "tests/research-agent-profiles.test.cjs :: research agent .md content is the governed surface", "tests/review-default-reviewers-workflow.test.cjs :: source-text-is-the-product", "tests/roadmap.test.cjs :: source-text-is-the-product", - "tests/roadmapper-granularity.test.cjs :: source-text-is-the-product", "tests/run-tests-harness.test.cjs :: run-tests.cjs is a CLI test harness whose only IR is its", "tests/runtime-launcher-parity.test.cjs :: structural parity/drift guard — asserts literal presence/absence of the canonical gsd_run launcher and the retired $GSD_SDK / `/gsd-tools` tokens across workflow markdown; there is no typed IR for \"this source file does not contain substring X\".", "tests/runtime-name-policy.test.cjs :: runtime-contract-is-the-product — FALLBACK_ALIASES source text IS the", diff --git a/tests/agent-size-baseline.json b/tests/agent-size-baseline.json index 55ab48258..aaaa6f70a 100644 --- a/tests/agent-size-baseline.json +++ b/tests/agent-size-baseline.json @@ -26,7 +26,7 @@ "gsd-planner.md": 48885, "gsd-project-researcher.md": 21987, "gsd-research-synthesizer.md": 13646, - "gsd-roadmapper.md": 19652, + "gsd-roadmapper.md": 21774, "gsd-security-auditor.md": 6216, "gsd-ui-auditor.md": 17152, "gsd-ui-checker.md": 11081, diff --git a/tests/roadmapper-granularity.test.cjs b/tests/roadmapper-granularity.test.cjs index 1b8380040..fd47548f8 100644 --- a/tests/roadmapper-granularity.test.cjs +++ b/tests/roadmapper-granularity.test.cjs @@ -1,7 +1,7 @@ -// allow-test-rule: source-text-is-the-product -// agents/gsd-roadmapper.md is the installed agent — the Granularity Calibration -// table IS the deployed instruction. Asserting on its text asserts what runs in -// production. Locks the tightened phase-count buckets from #163. +// allow-test-rule: runtime-contract-is-the-product agent .md instruction surface see #1205 +// agents/gsd-roadmapper.md is the deployed agent — the Granularity Calibration table +// AND the phase_id_convention instructions ARE the deployed behavior. Asserting on +// their prose asserts what runs in production (#163, #1205). 'use strict'; const { describe, test } = require('node:test'); @@ -26,6 +26,17 @@ function granularitySection(content) { return nextHeading === -1 ? rest : rest.slice(0, nextHeading); } +// Extract a named XML-tag block (e.g. …) +function extractBlock(content, tag) { + const open = `<${tag}>`; + const close = ``; + const start = content.indexOf(open); + const end = content.indexOf(close); + assert.ok(start !== -1, `<${tag}> block must exist in agent`); + assert.ok(end !== -1, ` must close the block`); + return content.slice(start + open.length, end); +} + describe('gsd-roadmapper granularity calibration (#163)', () => { const section = granularitySection(readAgent('gsd-roadmapper')); @@ -56,3 +67,51 @@ describe('gsd-roadmapper granularity calibration (#163)', () => { ); }); }); + +describe('gsd-roadmapper phase_id_convention support (#1205)', () => { + const content = readAgent('gsd-roadmapper'); + + test('phase_identification section reads phase_id_convention from config', () => { + const section = extractBlock(content, 'phase_identification'); + assert.ok( + section.includes('phase_id_convention'), + 'phase_identification block must reference phase_id_convention config key' + ); + }); + + test('output_formats documents milestone-prefixed header format', () => { + const section = extractBlock(content, 'output_formats'); + assert.ok( + section.includes('milestone-prefixed'), + 'output_formats block must document the milestone-prefixed convention' + ); + }); + + test('output_formats shows milestone-prefixed phase header example (e.g. ### Phase 1-01:)', () => { + const section = extractBlock(content, 'output_formats'); + assert.ok( + /###\s+Phase\s+\d+-\d{2}:/.test(section), + 'output_formats must show a milestone-prefixed header example like "### Phase 1-01: Name"' + ); + }); + + test('output_formats shows both sequential and milestone-prefixed summary checklist forms', () => { + const section = extractBlock(content, 'output_formats'); + assert.ok( + /- \[ \] \*\*Phase \d+:/.test(section), + 'output_formats must still show sequential summary checklist form "- [ ] **Phase N:"' + ); + assert.ok( + /- \[ \] \*\*Phase \d+-\d{2}:/.test(section), + 'output_formats must show milestone-prefixed checklist form "- [ ] **Phase N-NN:"' + ); + }); + + test('phase_identification section falls back to sequential when convention absent or "sequential"', () => { + const section = extractBlock(content, 'phase_identification'); + assert.ok( + section.includes('sequential'), + 'phase_identification block must document that sequential is the default/fallback' + ); + }); +});