* fix(#1988): exclude stray non-plan *-SUMMARY.md from phase completion count Stray remediation/gap-closure summaries (30-FIX-CR02-SUMMARY.md, 30-GAPCLOSURE-SUMMARY.md, …) inflated summary_count, and once summary_count >= plan_count the phase silently flipped to Complete even though several plans had no summary. A summary now counts toward completion only if it pairs with a real plan file. - core-utils.cts: new countMatchedSummaries(planFiles, summaryFiles) — layout-agnostic pairing via the PLAN→SUMMARY marker swap (root/nested/bare) plus the <stem>-SUMMARY.md form (bare PLAN.md↔PLAN-SUMMARY.md); the swap is applied to the basename only so a 'plans/' dir prefix isn't corrupted. - plan-scan.cts: scanPhasePlans.summaryCount/.completed use the matched count (summaryFiles array still holds every summary on disk for listing/reading). Fixes roadmap listing, state sync, verification, workstream inventory. - roadmap.cts: cmdRoadmapUpdatePlanProgress uses the matched count. - tests/roadmap.test.cjs: countMatchedSummaries unit tests (root/nested/bare/ stray) + E2E reproducing the exact #1988 report (4 plans, 1 plan summary, 3 strays → 1/4 In Progress, NOT Complete). Closes #1988 * docs(#1988): backfill changeset pr 2016 * test(#1988): strengthen countMatchedSummaries unit tests for mutation coverage Add direct unit tests for the extended (N-PLAN-MM-slug↔N-MM-SUMMARY), bare (PLAN↔SUMMARY, PLAN↔PLAN-SUMMARY), legacy (N-PLAN-NN↔N-PLAN-NN-SUMMARY), and stray-exclusion pairings so every branch of countMatchedSummaries is exercised (Stryker mutation-score coverage). * test(#1988): move countMatchedSummaries unit tests into core-utils.test.cjs The Stryker core-utils shard runs ONLY tests/core-utils.test.cjs (per scripts/mutation-matrix.cjs), so the unit tests for countMatchedSummaries must live there to be mutation-covered (previously in roadmap.test.cjs, the shard never ran them → mutants survived → Stryker gate failed). The E2E #1988 reproduction stays in roadmap.test.cjs. Added an absolute-path case to guard the lastIndexOf('/') >= 0 boundary.
This commit is contained in:
5
.changeset/1988-roadmap-stray-summary-count.md
Normal file
5
.changeset/1988-roadmap-stray-summary-count.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 2016
|
||||
---
|
||||
**`roadmap update-plan-progress` no longer counts stray non-plan `*-SUMMARY.md` files against phase completion** — remediation/gap-closure summaries (e.g. `30-FIX-CR02-SUMMARY.md`, `30-GAPCLOSURE-SUMMARY.md`) inflated `summary_count`, and once `summary_count >= plan_count` the phase silently flipped to `Complete` (checkbox checked, date stamped) even though several plans had no summary. A new `countMatchedSummaries` helper (core-utils) pairs summaries to plans via the `PLAN→SUMMARY` marker swap + the `<stem>-SUMMARY.md` form (layout-agnostic across root, bare, and nested layouts), so only a summary that corresponds to a real plan counts. Wired into `scanPhasePlans` (fixing roadmap listing, state sync, verification, workstream inventory at once) and `cmdRoadmapUpdatePlanProgress`. (#1988)
|
||||
@@ -186,6 +186,45 @@ function extractCanonicalPlanId(filename: string): string {
|
||||
return base;
|
||||
}
|
||||
|
||||
/**
|
||||
* Count summaries that correspond to a real plan (#1988).
|
||||
*
|
||||
* A summary counts toward phase completion iff it pairs with an existing plan
|
||||
* file. This excludes stray non-plan summaries — e.g. `30-FIX-CR02-SUMMARY.md`,
|
||||
* `30-GAPCLOSURE-SUMMARY.md` — that inflate the raw `*-SUMMARY.md` count and
|
||||
* silently flip a phase to Complete when plans are actually missing summaries.
|
||||
*
|
||||
* Pairing is layout-agnostic. For each plan, up to three candidate summary
|
||||
* filenames are generated and any match suffices:
|
||||
* 1. marker swap `PLAN`→`SUMMARY` on the basename — root padded
|
||||
* (`30-01-PLAN.md`↔`30-01-SUMMARY.md`), nested (`PLAN-01.md`↔
|
||||
* `SUMMARY-01.md`, incl. a `plans/` prefix), and bare (`PLAN.md`↔
|
||||
* `SUMMARY.md`);
|
||||
* 2. `<stem>-SUMMARY.md` — bare (`PLAN.md`↔`PLAN-SUMMARY.md`) and legacy
|
||||
* (`14-PLAN-01.md`↔`14-PLAN-01-SUMMARY.md`);
|
||||
* 3. extended `<n>-PLAN-<m>…`→`<n>-<m>-SUMMARY.md`
|
||||
* (`3-PLAN-01-setup.md`↔`3-01-SUMMARY.md`).
|
||||
* The swap is applied to the basename only so a lowercase `plans/` dir prefix
|
||||
* isn't corrupted to `SUMMARYs/…`.
|
||||
*/
|
||||
function countMatchedSummaries(planFiles: string[], summaryFiles: string[]): number {
|
||||
const summarySet = new Set(summaryFiles);
|
||||
let matched = 0;
|
||||
for (const plan of planFiles) {
|
||||
const slashIdx = plan.lastIndexOf('/');
|
||||
const dir = slashIdx >= 0 ? plan.slice(0, slashIdx + 1) : '';
|
||||
const base = (dir ? plan.slice(dir.length) : plan).replace(/\.md$/i, '');
|
||||
const candidates: string[] = [
|
||||
dir + base.replace(/PLAN/i, 'SUMMARY') + '.md',
|
||||
dir + base + '-SUMMARY.md',
|
||||
];
|
||||
const extended = base.match(/^(\d+)-PLAN-(\d+)/i);
|
||||
if (extended) candidates.push(dir + extended[1] + '-' + extended[2] + '-SUMMARY.md');
|
||||
if (candidates.some((c) => summarySet.has(c))) matched++;
|
||||
}
|
||||
return matched;
|
||||
}
|
||||
|
||||
export = {
|
||||
toPosixPath,
|
||||
detectSubRepos,
|
||||
@@ -198,4 +237,5 @@ export = {
|
||||
readSubdirectories,
|
||||
timeAgo,
|
||||
extractCanonicalPlanId,
|
||||
countMatchedSummaries,
|
||||
};
|
||||
|
||||
@@ -9,6 +9,9 @@
|
||||
|
||||
import { existsSync, readdirSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import coreUtils = require('./core-utils.cjs');
|
||||
const { countMatchedSummaries } = coreUtils;
|
||||
|
||||
// Excluded derivative files
|
||||
const PLAN_OUTLINE_RE = /-OUTLINE\.md$/i;
|
||||
@@ -82,7 +85,12 @@ function scanPhasePlans(phaseDir: string): PhaseScanResult {
|
||||
const planFiles = rootPlanFiles.concat(nestedPlanFiles);
|
||||
const summaryFiles = rootSummaryFiles.concat(nestedSummaryFiles);
|
||||
const planCount = planFiles.length;
|
||||
const summaryCount = summaryFiles.length;
|
||||
// Count only summaries that are the PLAN→SUMMARY partner of an existing plan
|
||||
// (#1988): stray non-plan summaries (e.g. 30-FIX-CR02-SUMMARY.md,
|
||||
// 30-GAPCLOSURE-SUMMARY.md) must not inflate summary_count or flip a phase to
|
||||
// Complete when plans are still missing summaries. summaryFiles (the array)
|
||||
// still holds every summary on disk for callers that read/list them.
|
||||
const summaryCount = countMatchedSummaries(planFiles, summaryFiles);
|
||||
|
||||
return {
|
||||
planCount,
|
||||
|
||||
@@ -29,6 +29,9 @@ const { planningPaths, withPlanningLock, findContextMdIn } = planningWorkspace;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import scanPhasePlans = require('./plan-scan.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import coreUtils = require('./core-utils.cjs');
|
||||
const { countMatchedSummaries } = coreUtils;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import frontmatter = require('./frontmatter.cjs');
|
||||
const { extractFrontmatter, parseMustHavesBlock } = frontmatter;
|
||||
|
||||
@@ -491,7 +494,10 @@ function cmdRoadmapUpdatePlanProgress(cwd: string, phaseNum: string | null | und
|
||||
}
|
||||
|
||||
const planCount = phaseInfo!.plans.length;
|
||||
const summaryCount = phaseInfo!.summaries.length;
|
||||
// Count only summaries that pair with a real plan (#1988): stray non-plan
|
||||
// summaries (30-FIX-CR02-SUMMARY.md, 30-GAPCLOSURE-SUMMARY.md, …) must not
|
||||
// inflate summary_count and silently flip the phase to Complete.
|
||||
const summaryCount = countMatchedSummaries(phaseInfo!.plans, phaseInfo!.summaries);
|
||||
|
||||
if (planCount === 0) {
|
||||
output({ updated: false, reason: 'No plans found', plan_count: 0, summary_count: 0 }, raw, 'no plans');
|
||||
|
||||
@@ -529,8 +529,72 @@ describe('extractCanonicalPlanId', () => {
|
||||
// extractCanonicalPlanId operates on a filename string (not a real path).
|
||||
// The function does not sanitize slashes — it strips .md suffixes and
|
||||
// attempts to find phase tokens. The result is a string (no crash).
|
||||
const result = coreUtils.extractCanonicalPlanId('../../../etc/passwd');
|
||||
const result = coreUtils.extractCanonicalPlanId('../../../etc/passwd');
|
||||
assert.ok(typeof result === 'string');
|
||||
assert.ok(result.length > 0);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── countMatchedSummaries (#1988) ───────────────────────────────────────────
|
||||
// Lives in core-utils.test.cjs (not roadmap.test.cjs) so the Stryker core-utils
|
||||
// shard — which runs only this file — actually covers the helper's mutants.
|
||||
|
||||
describe('countMatchedSummaries — stray non-plan summaries excluded (#1988)', () => {
|
||||
const { countMatchedSummaries } = coreUtils;
|
||||
|
||||
test('counts only summaries that are the PLAN→SUMMARY partner of a plan', () => {
|
||||
const plans = ['30-01-PLAN.md', '30-02-PLAN.md', '30-10-PLAN.md'];
|
||||
const summaries = ['30-01-SUMMARY.md', '30-FIX-CR02-SUMMARY.md', '30-GAPCLOSURE-SUMMARY.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 1);
|
||||
});
|
||||
|
||||
test('all plans have a partner summary → counts every plan', () => {
|
||||
const plans = ['01-01-PLAN.md', '01-02-PLAN.md'];
|
||||
const summaries = ['01-01-SUMMARY.md', '01-02-SUMMARY.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 2);
|
||||
});
|
||||
|
||||
test('nested layout pairing preserved (plans/PLAN-NN ↔ plans/SUMMARY-NN)', () => {
|
||||
const plans = ['plans/PLAN-01.md', 'plans/PLAN-02.md'];
|
||||
const summaries = ['plans/SUMMARY-01.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 1);
|
||||
});
|
||||
|
||||
test('extended layout pairing (N-PLAN-MM-slug ↔ N-MM-SUMMARY)', () => {
|
||||
const plans = ['3-PLAN-01-setup.md', '5-PLAN-02-migrations.md'];
|
||||
const summaries = ['3-01-SUMMARY.md', '5-02-SUMMARY.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 2);
|
||||
});
|
||||
|
||||
test('extended layout: only the matching plan counts (no cross-pairing)', () => {
|
||||
const plans = ['3-PLAN-01-setup.md', '3-PLAN-02-seed.md'];
|
||||
const summaries = ['3-01-SUMMARY.md']; // only plan 01 has a summary
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 1);
|
||||
});
|
||||
|
||||
test('bare PLAN.md ↔ SUMMARY.md', () => {
|
||||
assert.strictEqual(countMatchedSummaries(['PLAN.md'], ['SUMMARY.md']), 1);
|
||||
assert.strictEqual(countMatchedSummaries(['PLAN.md'], []), 0);
|
||||
});
|
||||
|
||||
test('bare PLAN.md ↔ PLAN-SUMMARY.md (stem-suffix convention)', () => {
|
||||
assert.strictEqual(countMatchedSummaries(['PLAN.md'], ['PLAN-SUMMARY.md']), 1);
|
||||
});
|
||||
|
||||
test('legacy <N>-PLAN-<NN> ↔ <N>-PLAN-<NN>-SUMMARY', () => {
|
||||
const plans = ['14-PLAN-01.md', '14-PLAN-02.md'];
|
||||
const summaries = ['14-PLAN-01-SUMMARY.md', '14-PLAN-02-SUMMARY.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, summaries), 2);
|
||||
});
|
||||
|
||||
test('stray summaries never count when no plan partners exist', () => {
|
||||
const plans = ['30-01-PLAN.md'];
|
||||
const strays = ['30-FIX-CR02-SUMMARY.md', '30-GAPCLOSURE-SUMMARY.md', '30-01-EXTRA-SUMMARY.md'];
|
||||
assert.strictEqual(countMatchedSummaries(plans, strays), 0);
|
||||
});
|
||||
|
||||
test('absolute path (leading slash) still pairs via the dir split', () => {
|
||||
// Guards the lastIndexOf('/') >= 0 boundary (slash at index 0).
|
||||
assert.strictEqual(countMatchedSummaries(['/abs/PLAN-01.md'], ['/abs/SUMMARY-01.md']), 1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -765,6 +765,55 @@ describe('roadmap update-plan-progress command', () => {
|
||||
assert.strictEqual(output.phases[0].disk_status, 'partial');
|
||||
});
|
||||
|
||||
test('#1988 — stray non-plan *-SUMMARY.md files do not inflate summary_count or flip phase to Complete', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
`# Roadmap
|
||||
|
||||
- [ ] **Phase 30: Big** - description
|
||||
|
||||
### Phase 30: Big
|
||||
**Goal:** big goal
|
||||
**Plans:** TBD
|
||||
|
||||
## Progress
|
||||
|
||||
| Phase | Milestone | Plans Complete | Status | Completed |
|
||||
|-------|-----------|----------------|--------|-----------|
|
||||
| 30. Big | v3.0 | 0/4 | Planned | - |
|
||||
`
|
||||
);
|
||||
|
||||
// 4 plans, only 1 has a real summary; plus 3 stray non-plan summaries
|
||||
// (the exact names from the #1988 report: FIX-CR02, FIX-WR02-04, GAPCLOSURE).
|
||||
const p = path.join(tmpDir, '.planning', 'phases', '30-big');
|
||||
fs.mkdirSync(p, { recursive: true });
|
||||
fs.writeFileSync(path.join(p, '30-01-PLAN.md'), '# Plan 1');
|
||||
fs.writeFileSync(path.join(p, '30-02-PLAN.md'), '# Plan 2');
|
||||
fs.writeFileSync(path.join(p, '30-03-PLAN.md'), '# Plan 3');
|
||||
fs.writeFileSync(path.join(p, '30-04-PLAN.md'), '# Plan 4');
|
||||
fs.writeFileSync(path.join(p, '30-01-SUMMARY.md'), '# Summary 1');
|
||||
// Strays — these must NOT count:
|
||||
fs.writeFileSync(path.join(p, '30-FIX-CR02-SUMMARY.md'), '# fix summary');
|
||||
fs.writeFileSync(path.join(p, '30-FIX-WR02-04-SUMMARY.md'), '# fix summary');
|
||||
fs.writeFileSync(path.join(p, '30-GAPCLOSURE-SUMMARY.md'), '# gap closure summary');
|
||||
|
||||
const result = runGsdTools('roadmap update-plan-progress 30', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.strictEqual(output.plan_count, 4, 'plan_count is the 4 real plans');
|
||||
assert.strictEqual(output.summary_count, 1, 'stray summaries must not inflate the count');
|
||||
assert.strictEqual(output.status, 'In Progress', 'must not flip to Complete');
|
||||
assert.strictEqual(output.complete, false, 'must not be complete');
|
||||
|
||||
// The ROADMAP row must NOT show 4/4 Complete.
|
||||
const roadmapContent = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
|
||||
assert.ok(roadmapContent.includes('1/4'), 'roadmap row must show the real 1/4 progress');
|
||||
assert.ok(!/4\/4\s*\|\s*Complete/.test(roadmapContent), 'must not stamp 4/4 Complete');
|
||||
assert.ok(!/\[x\] \*\*Phase 30/.test(roadmapContent), 'phase checkbox must not be checked');
|
||||
});
|
||||
|
||||
test('updates progress and checks checkbox on completion', () => {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
|
||||
Reference in New Issue
Block a user