* fix(#1659): dedup By-Phase rows across padded/unpadded phase numbers phaseRowPattern matched the phase number literally (escapeRegex(String(phaseNum))), so a seeded zero-padded row '| 05 |' was not matched by 'phase complete 5' (pattern '| 5 |'), producing a duplicate row that double-counted the phase. Canonicalize a numeric phase to its integer form (Number('05')===Number('5')===5) and match with a 0* prefix so 5/05/005 all collapse to the same row in either direction. Regression folded into state.test.cjs: seeded '| 05 |' + 'phase complete 5' yields exactly one phase-5 row. Non-numeric phase IDs retain the literal escapeRegex match. * chore(#1659): backfill changeset pr ref to 1663 * fix(#1659): add verification fixture to padded-dedup test under #1522 gate
This commit is contained in:
5
.changeset/rapid-jays-wave.md
Normal file
5
.changeset/rapid-jays-wave.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1663
|
||||
---
|
||||
**`phase complete` no longer duplicates a By-Phase row when the phase number's padding differs** — completing a phase by its unpadded number (e.g. `phase complete 5`) against an existing zero-padded By-Phase row (`| 05 |`) appended a second `| 5 |` row instead of updating it, double-counting the phase in any column sum. The row matcher now canonicalizes a numeric phase to its integer form (matching `5`, `05`, `005` in either direction), so the existing row is upserted regardless of padding.
|
||||
@@ -2425,7 +2425,12 @@ function updatePerformanceMetricsSection(content: string, cwd: string, phaseNum:
|
||||
const byPhaseMatch = content.match(byPhaseTablePattern);
|
||||
if (byPhaseMatch) {
|
||||
let tableBody = byPhaseMatch[2].trim();
|
||||
const phaseRowPattern = new RegExp(`^\\|\\s*${escapeRegex(String(phaseNum))}\\s*\\|.*$`, 'm');
|
||||
// Match the existing row for this phase, tolerating leading-zero padding in either
|
||||
// direction (#1659): canonicalize a numeric phase to its integer form so a seeded
|
||||
// "| 05 |" row is upserted (not duplicated) by `phase complete 5`, and vice-versa.
|
||||
const phaseNumStr = String(phaseNum);
|
||||
const canonCell = /^\d+$/.test(phaseNumStr) ? `0*${Number(phaseNumStr)}` : escapeRegex(phaseNumStr);
|
||||
const phaseRowPattern = new RegExp(`^\\|\\s*${canonCell}\\s*\\|.*$`, 'm');
|
||||
const newRow = `| ${phaseNum} | ${summaryCount} | - | - |`;
|
||||
|
||||
if (phaseRowPattern.test(tableBody)) {
|
||||
|
||||
@@ -2267,6 +2267,40 @@ describe('updatePerformanceMetricsSection', () => {
|
||||
'velocity total must update from the CRLF STATE.md body (proves CRLF content is processed)',
|
||||
);
|
||||
});
|
||||
|
||||
test('#1659 — completing an unpadded phase number upserts an existing zero-padded By-Phase row (no duplicate)', () => {
|
||||
const content = [
|
||||
'# Project State', '',
|
||||
'**Current Phase:** 05', '**Status:** Executing Phase 5', '',
|
||||
'## Performance Metrics', '',
|
||||
'**Velocity:**', '- Total plans completed: 1', '- Average duration: N/A', '- Total execution time: 0 hours', '',
|
||||
'**By Phase:**', '',
|
||||
'| Phase | Plans | Total | Avg/Plan |',
|
||||
'|-------|-------|-------|----------|',
|
||||
'| 05 | 1 | - | - |', // seeded ZERO-PADDED row
|
||||
'',
|
||||
'## Accumulated Context', '',
|
||||
].join('\n');
|
||||
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
|
||||
fs.writeFileSync(statePath, content, 'utf8');
|
||||
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-final');
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phaseDir, '05-01-PLAN.md'), '# Plan\n');
|
||||
fs.writeFileSync(path.join(phaseDir, '05-01-SUMMARY.md'), '# Summary\n');
|
||||
writePassedVerification(tmpDir, '05-final', '05');
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n\n## Phase 5: Final\n\n- [ ] Phase 5\n');
|
||||
|
||||
// phase complete with the UNPADDED number "5" — must upsert the seeded "| 05 |" row,
|
||||
// not append a duplicate "| 5 |".
|
||||
const result = runGsdTools('phase complete 5', tmpDir);
|
||||
assert.ok(result.success, `phase complete failed: ${result.error}`);
|
||||
|
||||
const after = fs.readFileSync(statePath, 'utf8');
|
||||
const rows05 = (after.match(/^\|\s*05\s*\|/gm) || []).length;
|
||||
const rows5 = (after.match(/^\|\s*5\s*\|/gm) || []).length;
|
||||
assert.equal(rows05 + rows5, 1, `phase 5 must appear exactly once in By Phase (got |05|=${rows05} |5|=${rows5}) — padded/unpadded must dedup (#1659)`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user