#2821 reworked parsePhaseFromProse's NAME precedence (dash-vs-paren choice, status-keyword tails, `Milestone:` tails, paren-stripped separator search, the lone-ALL-CAPS-tail rule) but shipped it pinned only by hand-picked examples. The two pre-existing property blocks cover phase-token ANCHORING (#2111) and "N of M" phase extraction; neither touches name precedence. This adds nine properties over the canonical parser. P1 and P9 are the delta guards: both fail against the pre-#2821 paren-first parser (proved with a standalone mutation harness), because each requires a genuine em-dash name to win over a co-present parenthetical. P9 additionally exercises the paren-stripped separator search, since the losing parenthetical itself contains an em-dash. P2-P4 are characterization tests for precedence contracts both parser versions satisfy; P5-P8 pin totality and phase-token extraction, which #2821 left alone. The section comment states which is which so a future reader does not mistake the characterization tests for delta guards. The generator's status-word exclusion filter is a test-local mirror of the private, unexported STATUSY_TAIL_RE. A divergence-guard test pins that mirror to observable parser behavior (not source text, which the no-source-grep rule forbids), so an implementation vocabulary change fails loudly instead of silently weakening every property that depends on the filter. Also clears six pre-existing no-unused-vars lint warnings surfaced by the lint run for this change (dead bindings in four unrelated test files, deleted rather than underscore-renamed); removing the never-called openCodeBlock helper cascaded to its now-unused fs/path/reviewPath consts in both fold scopes. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -388,11 +388,6 @@ describe('#2481 — ADR-443 mechanism callers, as they actually exist', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('#2481 review workflow resolves effort per reviewer', () => {
|
describe('#2481 review workflow resolves effort per reviewer', () => {
|
||||||
const reviewMd = fs.readFileSync(
|
|
||||||
path.join(REPO_ROOT, 'gsd-core', 'workflows', 'review.md'),
|
|
||||||
'utf8',
|
|
||||||
);
|
|
||||||
|
|
||||||
test('shipped orchestration invokes resolve-execution — the grep ADR-443 said returned zero hits', () => {
|
test('shipped orchestration invokes resolve-execution — the grep ADR-443 said returned zero hits', () => {
|
||||||
// Phase 5b (#2799) moved the call out of review.md's per-lane bash and into the review-lane
|
// Phase 5b (#2799) moved the call out of review.md's per-lane bash and into the review-lane
|
||||||
// route, which resolves effort once per selected lane through the SAME surface. ADR-443's
|
// route, which resolves effort once per selected lane through the SAME surface. ADR-443's
|
||||||
|
|||||||
@@ -31,8 +31,6 @@ const path = require('node:path');
|
|||||||
const { cleanup } = require('./helpers.cjs');
|
const { cleanup } = require('./helpers.cjs');
|
||||||
|
|
||||||
const REVIEW_MD = path.join(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
const REVIEW_MD = path.join(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
||||||
const SHIP_MD = path.join(__dirname, '..', 'gsd-core', 'workflows', 'ship.md');
|
|
||||||
const REVIEWER_INSTANCES_MD = path.join(__dirname, '..', 'gsd-core', 'references', 'reviewer-instances.md');
|
|
||||||
|
|
||||||
describe('#2358 review.md temp paths are run-scoped, not phase-only', () => {
|
describe('#2358 review.md temp paths are run-scoped, not phase-only', () => {
|
||||||
const content = fs.readFileSync(REVIEW_MD, 'utf-8');
|
const content = fs.readFileSync(REVIEW_MD, 'utf-8');
|
||||||
|
|||||||
@@ -22,7 +22,6 @@
|
|||||||
'use strict';
|
'use strict';
|
||||||
|
|
||||||
const { describe, test } = require('node:test');
|
const { describe, test } = require('node:test');
|
||||||
const assert = require('node:assert/strict');
|
|
||||||
const fc = require('fast-check');
|
const fc = require('fast-check');
|
||||||
|
|
||||||
const { handleOpencodeOutput } = require('../gsd-core/bin/lib/review-lane-runner.cjs');
|
const { handleOpencodeOutput } = require('../gsd-core/bin/lib/review-lane-runner.cjs');
|
||||||
|
|||||||
@@ -781,3 +781,197 @@ describe('#2232 continuation cap — properties', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ─── #2736 prose name-precedence property tests (fast-check) ─────────────────
|
||||||
|
|
||||||
|
// #2821's only behavioral delta in parsePhaseFromProse is that a GENUINE
|
||||||
|
// (non-status) em-dash name now takes precedence over a parenthetical name;
|
||||||
|
// phase-token extraction and totality were unchanged by that commit.
|
||||||
|
//
|
||||||
|
// P1 and P9 are the delta guards: both fail against the pre-#2821 paren-first
|
||||||
|
// parser (verified by the standalone mutation check against
|
||||||
|
// parsePhaseFromProseOLD), because they each require the dash name to win
|
||||||
|
// over a co-present parenthetical — P9 additionally exercises the
|
||||||
|
// paren-stripped separator search, since the losing parenthetical itself
|
||||||
|
// contains an em-dash.
|
||||||
|
//
|
||||||
|
// P2, P3, P4 are characterization tests: they pin currently-true precedence
|
||||||
|
// contracts (status tails and em-dash-inside-parens both lose to a
|
||||||
|
// parenthetical name) that the pre-#2821 parser ALSO satisfied, so they guard
|
||||||
|
// against future regressions rather than proving the #2821 delta.
|
||||||
|
//
|
||||||
|
// P5-P8 pin totality and phase-token extraction, neither of which #2821
|
||||||
|
// changed.
|
||||||
|
|
||||||
|
const phaseToken = fc
|
||||||
|
.tuple(
|
||||||
|
digitRun(1, 3),
|
||||||
|
fc.option(fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'), { nil: '' }),
|
||||||
|
fc.array(digitRun(1, 2), { maxLength: 2 }),
|
||||||
|
)
|
||||||
|
.map(([lead, letter, decimals]) => `${lead}${letter}${decimals.map((d) => `.${d}`).join('')}`);
|
||||||
|
|
||||||
|
const STATUSY =
|
||||||
|
/^(?:completed?|executing|not started|planning|planned|ready(?:\s+to\s+\S.{0,50})?|done|in progress|blocked|paused|verifying)$/i;
|
||||||
|
|
||||||
|
const genuineName = fc
|
||||||
|
.string({
|
||||||
|
unit: fc.constantFrom(
|
||||||
|
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
|
||||||
|
'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z',
|
||||||
|
' ', 'A', 'B', 'C',
|
||||||
|
),
|
||||||
|
minLength: 1,
|
||||||
|
maxLength: 40,
|
||||||
|
})
|
||||||
|
.map((s) => s.trim())
|
||||||
|
.filter((s) => s.length > 0 && !STATUSY.test(s) && !/^milestone\s*:/i.test(s) && !/^[A-Z][A-Z0-9_-]*$/.test(s));
|
||||||
|
|
||||||
|
const asideText = fc
|
||||||
|
.string({
|
||||||
|
unit: fc.constantFrom(...'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 -_'),
|
||||||
|
minLength: 1,
|
||||||
|
maxLength: 30,
|
||||||
|
})
|
||||||
|
.map((s) => s.trim())
|
||||||
|
.filter((s) => s.length > 0);
|
||||||
|
|
||||||
|
const statusTail = fc.constantFrom(
|
||||||
|
'COMPLETE', 'COMPLETED', 'EXECUTING', 'READY', 'DONE', 'IN PROGRESS',
|
||||||
|
'BLOCKED', 'PAUSED', 'VERIFYING', 'PLANNING', 'PLANNED', 'NOT STARTED',
|
||||||
|
);
|
||||||
|
|
||||||
|
const capsToken = fc.string({
|
||||||
|
unit: fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'),
|
||||||
|
minLength: 2,
|
||||||
|
maxLength: 12,
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('#2736 prose name precedence — properties', () => {
|
||||||
|
test('P1 dash name beats a trailing parenthetical aside', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, asideText, (tok, name, aside) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`${tok} — ${name} (${aside})`);
|
||||||
|
return p.phase === tok && p.name === name;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P2 a status-keyword tail never displaces a parenthetical name', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, statusTail, (tok, name, status) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`${tok} (${name}) — ${status}`);
|
||||||
|
return p.phase === tok && p.name === name;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P3 an em-dash inside parens is not mistaken for the separator', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, genuineName, statusTail, (tok, a, b, status) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`${tok} (${a} — ${b}) — ${status}`);
|
||||||
|
return p.phase === tok && p.name === `${a} — ${b}`;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P4 a lone ALL-CAPS tail loses to a parenthetical name', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, capsToken, (tok, name, caps) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`${tok} (${name}) — ${caps}`);
|
||||||
|
return p.phase === tok && p.name === name;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P5 parsePhaseFromProse is total over arbitrary input', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(fc.string({ maxLength: 300 }), (s) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(s);
|
||||||
|
return (
|
||||||
|
p !== null &&
|
||||||
|
typeof p === 'object' &&
|
||||||
|
(p.phase === null || typeof p.phase === 'string') &&
|
||||||
|
(p.name === null || typeof p.name === 'string')
|
||||||
|
);
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P6 pathological paren/em-dash runs stay total', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(fc.integer({ min: 1, max: 400 }), (n) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`3 ${'('.repeat(n)}${'—'.repeat(n)}`);
|
||||||
|
return p.phase === '3' && (p.name === null || typeof p.name === 'string');
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P7 the phase token round-trips out of first-party prose shapes', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, (tok, name) =>
|
||||||
|
phaseId.parsePhaseFromProse(`${tok} (${name})`).phase === tok &&
|
||||||
|
phaseId.parsePhaseFromProse(`Phase ${tok} — ${name}`).phase === tok &&
|
||||||
|
phaseId.parsePhaseFromProse(`${tok}`).phase === tok,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P8 a milestone-prefixed token still yields the bare phase', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(
|
||||||
|
fc.string({ unit: fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'), minLength: 1, maxLength: 3 }),
|
||||||
|
phaseToken,
|
||||||
|
genuineName,
|
||||||
|
(ms, tok, name) => phaseId.parsePhaseFromProse(`${ms}1-${tok} (${name})`).phase === tok,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('P9 a genuine dash name wins over a paren containing an em-dash', () => {
|
||||||
|
fc.assert(
|
||||||
|
fc.property(phaseToken, genuineName, genuineName, genuineName, (tok, a, b, name) => {
|
||||||
|
const p = phaseId.parsePhaseFromProse(`${tok} (${a} — ${b}) — ${name}`);
|
||||||
|
return p.phase === tok && p.name === name;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The STATUSY regex above is a test-local mirror of the private, unexported
|
||||||
|
// STATUSY_TAIL_RE in src/phase-id.cts — it is not imported, only
|
||||||
|
// reimplemented. If a future edit to the implementation's status
|
||||||
|
// vocabulary drifts from this mirror, the properties above that rely on
|
||||||
|
// STATUSY (P2, genuineName's exclusion filter, etc.) would silently weaken
|
||||||
|
// rather than fail. This test pins the mirror to OBSERVABLE parser
|
||||||
|
// behavior instead of source text, so a divergence fails loudly here.
|
||||||
|
test('the test-local STATUSY mirror still agrees with the parser (divergence guard)', () => {
|
||||||
|
const statusVocab = [
|
||||||
|
'complete', 'completed', 'executing', 'not started', 'planning',
|
||||||
|
'planned', 'ready', 'done', 'in progress', 'blocked', 'paused',
|
||||||
|
'verifying',
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const w of statusVocab) {
|
||||||
|
assert.equal(
|
||||||
|
phaseId.parsePhaseFromProse(`3 (Real Name) — ${w}`).name,
|
||||||
|
'Real Name',
|
||||||
|
`expected status word "${w}" to lose to the parenthetical name`,
|
||||||
|
);
|
||||||
|
const upper = w.toUpperCase();
|
||||||
|
assert.equal(
|
||||||
|
phaseId.parsePhaseFromProse(`3 (Real Name) — ${upper}`).name,
|
||||||
|
'Real Name',
|
||||||
|
`expected status word "${upper}" to lose to the parenthetical name`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const nonStatusNames = ['Foundation', 'Native Hotkey', 'setup work'];
|
||||||
|
for (const n of nonStatusNames) {
|
||||||
|
assert.equal(
|
||||||
|
phaseId.parsePhaseFromProse(`3 — ${n} (aside)`).name,
|
||||||
|
n,
|
||||||
|
`expected non-status name "${n}" to win as the dash name over the parenthetical aside`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -165,11 +165,6 @@ describe('review workflow source-grounding requirement in build_prompt (#1318)',
|
|||||||
|
|
||||||
const { describe, test } = require('node:test');
|
const { describe, test } = require('node:test');
|
||||||
const assert = require('node:assert/strict');
|
const assert = require('node:assert/strict');
|
||||||
const fs = require('node:fs');
|
|
||||||
const path = require('node:path');
|
|
||||||
|
|
||||||
const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
|
||||||
const read = () => fs.readFileSync(reviewPath, 'utf-8');
|
|
||||||
|
|
||||||
describe('bug #687 → #2073: agy print mode is bounded, and its fallback chain fires', () => {
|
describe('bug #687 → #2073: agy print mode is bounded, and its fallback chain fires', () => {
|
||||||
// Phase 5b (#2799) moved the agy invocation out of review.md's bash into the declared lane plus
|
// Phase 5b (#2799) moved the agy invocation out of review.md's bash into the declared lane plus
|
||||||
@@ -345,23 +340,6 @@ describe('#1698 regression: codex review is captured via --output-last-message,
|
|||||||
|
|
||||||
const { describe, test } = require('node:test');
|
const { describe, test } = require('node:test');
|
||||||
const assert = require('node:assert/strict');
|
const assert = require('node:assert/strict');
|
||||||
const fs = require('node:fs');
|
|
||||||
const path = require('node:path');
|
|
||||||
|
|
||||||
const reviewPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'review.md');
|
|
||||||
const read = () => fs.readFileSync(reviewPath, 'utf-8');
|
|
||||||
|
|
||||||
// Isolate the base OpenCode reviewer block (heading -> next reviewer heading) so
|
|
||||||
// assertions about its stderr handling don't accidentally match sibling reviewers
|
|
||||||
// (gemini/claude/coderabbit/qwen legitimately use /dev/null).
|
|
||||||
function openCodeBlock() {
|
|
||||||
const c = read();
|
|
||||||
const start = c.indexOf('**OpenCode (via GitHub Copilot):**');
|
|
||||||
assert.notStrictEqual(start, -1, 'review.md must contain the base OpenCode reviewer block');
|
|
||||||
const rest = c.slice(start + 1);
|
|
||||||
const nextHeading = rest.search(/\n\*\*[A-Z][^\n]*:\*\*/);
|
|
||||||
return nextHeading === -1 ? c.slice(start) : c.slice(start, start + 1 + nextHeading);
|
|
||||||
}
|
|
||||||
|
|
||||||
describe('bug #1936: OpenCode reviewer must not silently yield an empty review', () => {
|
describe('bug #1936: OpenCode reviewer must not silently yield an empty review', () => {
|
||||||
// The agent can end its turn with ZERO output tokens, and `--format default` then drops the
|
// The agent can end its turn with ZERO output tokens, and `--format default` then drops the
|
||||||
|
|||||||
Reference in New Issue
Block a user