* test(#4764): failing-first — dep_phases must extract only Phase-prefixed references, never dates/shas/ledger ids/self * fix(#4764): dep_phases anchors phase references to their 'Phase' prose context and never emits the row's own number * fix(#4764): review fold-ins — hoist the anchored dep-reference grammar to phase-id, cover Oxford lists and hyphen ranges, repair the property test Adversarial review found: Oxford-comma lists under-extracted ('Phases 1, 2, and 3' dropped the tail member — a silent real-blocker clear, the dangerous direction); hyphen ranges ('Phases 1-3') kept only the first endpoint; the property test called fc.hexaString (absent in fast-check 4.8, threw every run) and passed the junk arbitrary unspread (vacuous guard) with no completeness assertion; planning-inspect's extractDependencyTokens carried the same whole-field scrape (generative-fix divergence). The anchored grammar now lives beside PHASE_NUMBER_TOKEN_SOURCE in phase-id.cts and both readers interpolate it. * chore(#4764): backfill changeset PR number (4876) --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/kind-cats-leap.md
Normal file
5
.changeset/kind-cats-leap.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 4876
|
||||
---
|
||||
**init.manager stops reporting dates, shas and ledger ids as phase dependencies** — dep_phases scraped every digit run out of a phase's Depends-on prose, so phases whose prose declares no dependency read as blocked (dates split into year/month/day, git shas fragmenting, WINDOWS ledger ids, even the phase's own number); extraction now pulls only Phase-prefixed references — including "Phases 1, 2, and 3" lists and "Phase 1-3" ranges — and planning-inspect's dependencies field uses the same anchored grammar. (#4764)
|
||||
36
src/init.cts
36
src/init.cts
@@ -105,6 +105,7 @@ const {
|
||||
matchPhaseDirs,
|
||||
stripProjectCodePrefix,
|
||||
PHASE_NUMBER_TOKEN_SOURCE,
|
||||
PHASE_DEP_REF_SOURCE,
|
||||
isForeignPrefixedPhaseQuery,
|
||||
isSentinelPhaseId,
|
||||
extractPhaseToken,
|
||||
@@ -3105,6 +3106,23 @@ function cmdInitManager(cwd: string, raw: boolean): void {
|
||||
return reaches(numA, numB) || reaches(numB, numA);
|
||||
}
|
||||
|
||||
// #4764: a phase reference in depends_on prose is a PHASE-SHAPED token in
|
||||
// context — directly following "Phase"/"Phases" — never a bare digit run.
|
||||
// The previous whole-field scrape matched the token grammar against every
|
||||
// digit run, so calendar dates ("2026-09-14" → 2026, 09, 14), git shas
|
||||
// ("8bf403100d" → 8b, 403100d, …), bracketed ledger ids (WINDOWS #1843) and
|
||||
// the row's OWN number all became "dependencies", and deps_satisfied came
|
||||
// back false for phases whose prose declares none (50 of 92 phases in the
|
||||
// reporter's milestone). The anchored grammar (owned by phase-id.cts as
|
||||
// PHASE_DEP_REF_SOURCE, shared with planning-inspect's dependencies) keeps
|
||||
// lists fully extracted ("Phases 601 and 602", "Phase 601, 602, and 603",
|
||||
// "Phase 1-3") — silently dropping a REAL dependency would clear
|
||||
// deps_satisfied prematurely, the dangerous direction. Negation prose
|
||||
// ("dropped the dependency on Phase 654") is NOT detected: the issue's own
|
||||
// minimum keeps such tokens.
|
||||
const depPhaseRefRe = new RegExp(`${PHASE_DEP_REF_SOURCE}`, 'gi');
|
||||
const depTokenRe = new RegExp(`${PHASE_NUMBER_TOKEN_SOURCE}`, 'gi');
|
||||
|
||||
for (const phase of phases) {
|
||||
if (
|
||||
!phase['depends_on'] ||
|
||||
@@ -3112,7 +3130,23 @@ function cmdInitManager(cwd: string, raw: boolean): void {
|
||||
) {
|
||||
phase['deps_satisfied'] = true;
|
||||
} else {
|
||||
const depNums = (phase['depends_on'] as string).match(new RegExp(`${PHASE_NUMBER_TOKEN_SOURCE}`, 'gi')) || [];
|
||||
const prose = phase['depends_on'] as string;
|
||||
const ownNumber = normalizePhaseNumber(phase['number'] as string);
|
||||
const depNums: string[] = [];
|
||||
const seen = new Set<string>();
|
||||
let refMatch: RegExpExecArray | null;
|
||||
depPhaseRefRe.lastIndex = 0;
|
||||
while ((refMatch = depPhaseRefRe.exec(prose)) !== null) {
|
||||
let tok: RegExpExecArray | null;
|
||||
depTokenRe.lastIndex = 0;
|
||||
while ((tok = depTokenRe.exec(refMatch[1])) !== null) {
|
||||
const normalized = normalizePhaseNumber(tok[0]);
|
||||
if (normalized === ownNumber) continue; // #4764: never the row's own phase
|
||||
if (seen.has(normalized)) continue;
|
||||
seen.add(normalized);
|
||||
depNums.push(tok[0]);
|
||||
}
|
||||
}
|
||||
phase['deps_satisfied'] = depNums.every((n) => completedNums.has(normalizePhaseNumber(n)));
|
||||
phase['dep_phases'] = depNums;
|
||||
}
|
||||
|
||||
@@ -64,6 +64,22 @@ const OPTIONAL_PHASE_TAG_SOURCE = '(?:\\s*\\([^)\\n]{0,200}\\))?';
|
||||
// introduced outside this module without a `// phase-id-owner:` justification.
|
||||
const PHASE_NUMBER_TOKEN_SOURCE = '\\d+[A-Z]?(?:\\.\\d+)*';
|
||||
|
||||
// #4764: a phase REFERENCE in depends-on PROSE — the token in context, directly
|
||||
// following "Phase"/"Phases", with bare-token list continuation ("Phases 1 and 2",
|
||||
// "Phase 1, 2, and 3", "Phase 1-3"). Built HERE, beside the token grammar it is
|
||||
// anchored on, because two readers consume Depends-on prose (init.manager's
|
||||
// dep_phases and planning-inspect's dependencies) and re-deriving the anchored
|
||||
// form at each would drift exactly the way PHASE_NUMBER_TOKEN_SOURCE's own
|
||||
// anti-divergence rule exists to prevent. A bare digit run is the right shape
|
||||
// test for a phase id and the WRONG test for a reference: the whole-field
|
||||
// scrape this replaces pulled calendar dates ("2026-09-14" → 2026/09/14), git
|
||||
// shas ("8bf403100d" → 8b/403100d/…) and ledger ids (WINDOWS #1843) in as
|
||||
// dependencies. The `-` separator deliberately extracts range ENDPOINTS only
|
||||
// ("Phase 1-3" → 1, 3) — the pre-#4764 behavior; interior enumeration stays
|
||||
// out (a range's middle is not written as a reference).
|
||||
const PHASE_DEP_REF_SOURCE =
|
||||
`\\bphases?\\s+(${PHASE_NUMBER_TOKEN_SOURCE}(?:(?:\\s*,\\s*(?:and\\s+)?|\\s+and\\s+|\\s*&\\s*|\\s+(?:to|through)\\s+|\\s*-\\s*)${PHASE_NUMBER_TOKEN_SOURCE})*)`;
|
||||
|
||||
// #2528 review: the CASE-FLEXIBLE renderings of the two sources above, for call
|
||||
// sites that scan directory names (where a project code or a variant suffix may
|
||||
// legitimately be lowercase) and therefore cannot use a case-sensitive class.
|
||||
@@ -1632,6 +1648,7 @@ export = {
|
||||
OPTIONAL_PROJECT_CODE_PREFIX_SOURCE,
|
||||
OPTIONAL_PHASE_TAG_SOURCE,
|
||||
PHASE_NUMBER_TOKEN_SOURCE,
|
||||
PHASE_DEP_REF_SOURCE,
|
||||
CASE_FLEXIBLE_PROJECT_CODE_PREFIX_SOURCE,
|
||||
CASE_FLEXIBLE_PHASE_NUMBER_TOKEN_SOURCE,
|
||||
PHASE_CONTINUATION_SEGMENT_SOURCE,
|
||||
|
||||
@@ -1059,7 +1059,23 @@ function extractGoalProse(sectionBody: string): string | null {
|
||||
function extractDependencyTokens(sectionBody: string): string[] {
|
||||
const m = DEPENDS_ON_LINE_RE.exec(sectionBody);
|
||||
if (!m) return [];
|
||||
return sortedUnique([...m[1].matchAll(/\d+(?:\.\d+)*/g)].map((t) => t[0]));
|
||||
// #4764: phase REFERENCES, not digit runs — the same prose-anchored grammar
|
||||
// init.manager's dep_phases extraction uses (owner: phase-id.cts's
|
||||
// PHASE_DEP_REF_SOURCE). The whole-field token scrape this replaces pulled
|
||||
// calendar dates, git shas and ledger ids in as dependencies. The grammar's
|
||||
// capture group 1 already excludes the "Phase(s)" anchor word, so no
|
||||
// prefix-strip literal is needed here. Self-exclusion (init.manager drops
|
||||
// the row's own number) is deliberately NOT applied: this reader has no row
|
||||
// context at the extraction site and reports informationally, it does not
|
||||
// gate.
|
||||
const refRe = new RegExp(phaseIdMod.PHASE_DEP_REF_SOURCE, 'gi');
|
||||
const tokenRe = new RegExp(phaseIdMod.PHASE_NUMBER_TOKEN_SOURCE, 'g');
|
||||
const tokens: string[] = [];
|
||||
let refMatch: RegExpExecArray | null;
|
||||
while ((refMatch = refRe.exec(m[1])) !== null) {
|
||||
for (const t of refMatch[1].matchAll(tokenRe)) tokens.push(t[0]);
|
||||
}
|
||||
return sortedUnique(tokens);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1646,3 +1646,187 @@ describe('bug-3584: validate health uses formatter for codex runtime too', () =>
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ─── #4764: dep_phases extracts only in-context phase references ────────────
|
||||
|
||||
describe('#4764 dep_phases extracts only Phase-prefixed references', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
// The issue's measured phase-663 prose: dates, a round name, ledger ids and
|
||||
// the row's own number, plus one genuine "Phase 654" mention.
|
||||
const ISSUE_663_PROSE =
|
||||
'Nothing. Round 663-DISPOSITION Q7 (3/3, 2026-09-14, ' +
|
||||
'`.planning/decisions/663-CROSSAI-stream-consumer-failure-disposition.md`) moved the ' +
|
||||
'"for retry" javadoc correction into this phase and dropped the dependency on Phase 654; ' +
|
||||
'the defect is already recorded (WINDOWS #1843, #1977-#1985, #1992).';
|
||||
|
||||
test("#4764: dep_phases extracts only Phase-prefixed references from the issue's measured prose", () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '663', name: 'Disposition phase', depends_on: ISSUE_663_PROSE },
|
||||
{ number: '654', name: 'Prior phase' },
|
||||
]);
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '663');
|
||||
|
||||
assert.deepEqual(row.dep_phases, ['654']);
|
||||
for (const junk of ['663', '2026', '09', '14', '7', '3', '1843', '1977', '1985', '1992']) {
|
||||
assert.ok(!row.dep_phases.includes(junk), `must not scrape ${junk} as a dependency`);
|
||||
}
|
||||
assert.strictEqual(row.deps_satisfied, false, '654 is incomplete — the only real reference');
|
||||
});
|
||||
|
||||
test('#4764: a date-only depends_on prose yields no dependencies and a satisfied row', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '674', name: 'Ops current', depends_on: 'Nothing. Opened by the owner on 2026-09-15 after asking whether the operations app was current with the backend.' },
|
||||
]);
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '674');
|
||||
|
||||
assert.deepEqual(row.dep_phases, []);
|
||||
assert.strictEqual(row.deps_satisfied, true);
|
||||
assert.strictEqual(row.deps_display, '—');
|
||||
});
|
||||
|
||||
test('#4764: sha-bearing prose contributes no dependency tokens', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '607', name: 'Sha phase', depends_on: 'Landed in 8bf403100d and 9445745c08; follow-up 76aea5c36f tracked separately.' },
|
||||
]);
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '607');
|
||||
|
||||
assert.deepEqual(row.dep_phases, []);
|
||||
assert.strictEqual(row.deps_satisfied, true);
|
||||
});
|
||||
|
||||
test('#4764: Phase lists stay fully extracted (and/comma separators)', () => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '1', name: 'A', complete: true },
|
||||
{ number: '2', name: 'B', complete: true },
|
||||
{ number: '3', name: 'C', depends_on: 'Phases 1 and 2' },
|
||||
{ number: '4', name: 'D', depends_on: 'Phase 1, Phase 2' },
|
||||
{ number: '5', name: 'E', depends_on: 'Phase 1 & 2' },
|
||||
{ number: '6', name: 'F', depends_on: 'Phase 1 to 2' },
|
||||
]);
|
||||
// Completion is DISK-strict (ADR-3180): a ticked checkbox does not feed
|
||||
// completedNums — the dep targets need real dirs + passed verifications.
|
||||
for (const n of [1, 2]) {
|
||||
writePassedVerification(scaffoldPhase(tmpDir, n, { plans: 1, summaries: 1 }), `0${n}`);
|
||||
}
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
assert.deepEqual(output.phases.find((p) => p.number === '3').dep_phases, ['1', '2']);
|
||||
assert.deepEqual(output.phases.find((p) => p.number === '4').dep_phases, ['1', '2']);
|
||||
assert.deepEqual(output.phases.find((p) => p.number === '5').dep_phases, ['1', '2'], '& separator');
|
||||
assert.deepEqual(output.phases.find((p) => p.number === '6').dep_phases, ['1', '2'], 'to-range separator');
|
||||
assert.strictEqual(output.phases.find((p) => p.number === '3').deps_satisfied, true);
|
||||
assert.strictEqual(output.phases.find((p) => p.number === '5').deps_satisfied, true);
|
||||
assert.strictEqual(output.phases.find((p) => p.number === '6').deps_satisfied, true);
|
||||
});
|
||||
|
||||
test('#4764: Oxford-comma lists stay fully extracted, and a missing member blocks', () => {
|
||||
// Adversarial-review fold-in: "Phases 1, 2, and 3" is the most common
|
||||
// English enumeration; the first grammar cut dropped 603-style tail
|
||||
// members, silently clearing a real blocker (the dangerous direction).
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '1', name: 'A', complete: true },
|
||||
{ number: '2', name: 'B', complete: true },
|
||||
{ number: '3', name: 'C', depends_on: 'Phases 1, 2, and 3' },
|
||||
]);
|
||||
for (const n of [1, 2]) {
|
||||
writePassedVerification(scaffoldPhase(tmpDir, n, { plans: 1, summaries: 1 }), `0${n}`);
|
||||
}
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '3');
|
||||
assert.deepEqual(row.dep_phases, ['1', '2']);
|
||||
assert.strictEqual(row.deps_satisfied, true);
|
||||
});
|
||||
|
||||
test('#4764: hyphen ranges extract both endpoints and a missing tail blocks', () => {
|
||||
// House style writes ranges ("Phases 1-9"); endpoints are the written
|
||||
// references (the pre-#4764 scrape also kept only endpoints).
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '1', name: 'A', complete: true },
|
||||
{ number: '3', name: 'C' },
|
||||
{ number: '4', name: 'D', depends_on: 'Phases 1-3' },
|
||||
]);
|
||||
writePassedVerification(scaffoldPhase(tmpDir, 1, { plans: 1, summaries: 1 }), '01');
|
||||
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '4');
|
||||
assert.deepEqual(row.dep_phases, ['1', '3']);
|
||||
assert.strictEqual(row.deps_satisfied, false, '3 is incomplete — a real blocker must not be dropped');
|
||||
});
|
||||
|
||||
test('#4764 property: extraction pulls exactly the Phase-prefixed references out of arbitrary prose', () => {
|
||||
// House fast-check config (seed 42); per-call numRuns caps the cost — each
|
||||
// run spawns the real CLI (runGsdTools), unlike the pure-function properties.
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
// Junk fragments whose digit runs the old whole-field scrape pulled in as
|
||||
// "dependencies": calendar dates, git shas, ledger ids, counts, round names.
|
||||
// (fc v4 has no hexaString — stringMatching is the legal sha generator.)
|
||||
const junkFragments = [
|
||||
fc.nat({ max: 28 }).map((d) => `2026-09-${String(d).padStart(2, '0')}`),
|
||||
fc.stringMatching(/[0-9a-f]{8,10}/),
|
||||
fc.nat({ max: 99999 }).map((n) => `WINDOWS #${n}`),
|
||||
fc.nat({ max: 999 }).map((n) => `#${n}-#${n + 1}`),
|
||||
fc.nat({ max: 9 }).map((n) => `round ${n}-DISPOSITION (${n}/3`),
|
||||
];
|
||||
const refFragment = fc.nat({ max: 8 }).map((n) => `Phase ${n + 1}`);
|
||||
const listFragment = fc.nat({ max: 7 }).map((n) => `Phases ${n + 1}, ${n + 2}, and ${n + 3}`);
|
||||
const selfFragment = fc.constant('Phase 9');
|
||||
|
||||
// The phase under test is 9: any "Phase 9" mention in its own prose must be
|
||||
// dropped as a self-reference.
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.array(fc.oneof(...junkFragments, refFragment, listFragment, selfFragment), { maxLength: 12 }),
|
||||
(fragments) => {
|
||||
writeState(tmpDir);
|
||||
writeRoadmap(tmpDir, [
|
||||
{ number: '9', name: 'Property phase', depends_on: fragments.join('; ') + '.' },
|
||||
{ number: '1', name: 'Ref target', complete: true },
|
||||
]);
|
||||
const output = JSON.parse(runGsdTools('init manager', tmpDir).output);
|
||||
const row = output.phases.find((p) => p.number === '9');
|
||||
const extracted = row.dep_phases.map((d) => String(d));
|
||||
|
||||
// Independent spec mirror: the expected set is every phase token a
|
||||
// fragment references ("Phase N" or inside a "Phases …" list),
|
||||
// minus the row's own number — junk never contributes.
|
||||
const expected = new Set();
|
||||
for (const f of fragments) {
|
||||
const refMatch = /^phases?\s+(.*)$/i.exec(f);
|
||||
if (!refMatch) continue;
|
||||
for (const tok of refMatch[1].match(/\d+(?:\.\d+)*/g) || []) {
|
||||
if (tok !== '9') expected.add(tok);
|
||||
}
|
||||
}
|
||||
assert.deepEqual(
|
||||
[...extracted].sort(),
|
||||
[...expected].sort(),
|
||||
`extraction must pull exactly the Phase-prefixed references from: ${fragments.join('; ')}`,
|
||||
);
|
||||
},
|
||||
),
|
||||
{ numRuns: 20 },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user