voyage/tests/lib/plan-review-dedup.test.mjs
Kjell Tore Guttormsen f5ca220bc0
feat(plan): plan 1.8, the plan gate, the design part and the plan-side checks
Wave 1, order 3 (P3 plan mode as the start, P2 design as its own part, P9
plan side). Contracts K3-K6 for the later merges.

- plan_version 1.8 (1.7 plans stay valid): Test first on every step, one
  End-to-end ("run the thing") step, optional Interface in / Interface out /
  Review focus fields, a ceremony class in the header (absent =
  architectural), and for architectural plans a ## Design part: module map
  without cycles, architecture rules that name their enforcer and the step
  that proves them, a requirements check asked as keep / change.
- K3 lib/validators/plan-gate.mjs: the critic verdict is recorded INSIDE the
  plan (Claude Code's plan approval hands a hook only the plan file's text),
  bound to the hash taken before the reviewers read it; 0 go / 1 stop / 2
  could not run; at most 3 critic rounds, then stop and ask.
- K5 lib/verification/arch-rules.mjs: runs the target repo's own
  dependency-cruiser or import-linter (installs nothing; 2 = not measured,
  never green); --prove --rule shows pass -> fail on a planted violation of
  that rule -> pass. Verified against dependency-cruiser 18.5.0 and
  import-linter 2.15; madge 8.0.0 left out (missed a planted .mjs cycle).
- K6 lib/verification/plan-scope-sweep.mjs: changed files no step names.
- /trekplan Phases 8-10: design part, bounded critic loop, the gate before
  "execute", a plan-mode path through ExitPlanMode (not yet run end to end
  inside plan mode). plan-critic 10 -> 12 dimensions; scope-guardian checks
  the ceremony.
- Fixed on the way: plan-validator and plan-review-dedup used the
  file://argv[1] main guard (no-op under a symlink or a space); CRLF plans
  failed every manifest; the dedup dropped finding severity.

Borrowed, credited in place: obra/superpowers, Kiro (docs), addyosmani/
agent-skills, mattpocock/skills, GitHub Spec Kit, bashebr/ai-native-sdlc,
Boris Cherny's public threads.

Suite 1497 (1495 / 0 / 2), 86 new tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-06 20:10:57 +02:00

198 lines
9.3 KiB
JavaScript

// tests/lib/plan-review-dedup.test.mjs
// Cover lib/review/plan-review-dedup.mjs:
// - identical findings dedupe to 1 (exact-id path)
// - distinct findings stay separate
// - jaccard threshold 0.7 catches near-duplicates
// - empty / missing payloads tolerated
// - CLI shim emits parseable JSON on stdout
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { execFileSync } from 'node:child_process';
import { writeFileSync, mkdtempSync, rmSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { tmpdir } from 'node:os';
import { fileURLToPath } from 'node:url';
import { dedupFindings, tokenize, DEFAULT_THRESHOLD } from '../../lib/review/plan-review-dedup.mjs';
const HERE = dirname(fileURLToPath(import.meta.url));
const SHIM = join(HERE, '..', '..', 'lib', 'review', 'plan-review-dedup.mjs');
function tmp(prefix = 'plan-review-dedup-') {
return mkdtempSync(join(tmpdir(), prefix));
}
test('tokenize splits on non-word and lowercases', () => {
assert.deepEqual(
tokenize('Step 4 LACKS verifiable acceptance!'),
['step', '4', 'lacks', 'verifiable', 'acceptance'],
);
assert.deepEqual(tokenize(''), []);
assert.deepEqual(tokenize(undefined), []);
});
test('DEFAULT_THRESHOLD is 0.7 per plan-v2 spec', () => {
assert.equal(DEFAULT_THRESHOLD, 0.7);
});
test('identical findings (same file/line/rule_key) dedupe to 1, raised_by merged', () => {
const sources = [
{ agent: 'plan-critic', payload: { agent: 'plan-critic', findings: [{ file: 'plan.md', line: 42, rule_key: 'PC1', text: 'Step 4 lacks verifiable acceptance criteria' }] } },
{ agent: 'scope-guardian', payload: { agent: 'scope-guardian', findings: [{ file: 'plan.md', line: 42, rule_key: 'PC1', text: 'Step 4 lacks verifiable acceptance criteria' }] } },
];
const r = dedupFindings(sources);
assert.equal(r.findings.length, 1);
assert.deepEqual(r.findings[0].raised_by.sort(), ['plan-critic', 'scope-guardian']);
assert.equal(r.dedup_stats.total_in, 2);
assert.equal(r.dedup_stats.total_out, 1);
assert.equal(r.dedup_stats.exact_id_dups, 1);
});
test('distinct findings (different file/line/rule_key) stay separate', () => {
const sources = [
{ agent: 'plan-critic', payload: { findings: [
{ file: 'plan.md', line: 10, rule_key: 'PC1', text: 'thing one' },
{ file: 'plan.md', line: 20, rule_key: 'PC2', text: 'thing two unrelated entirely' },
] } },
];
const r = dedupFindings(sources);
assert.equal(r.findings.length, 2);
assert.equal(r.dedup_stats.exact_id_dups, 0);
assert.equal(r.dedup_stats.jaccard_dups, 0);
});
test('jaccard ≥ 0.7 on near-duplicate text merges (different file/line so id differs)', () => {
const sources = [
{ agent: 'plan-critic', payload: { findings: [{ file: 'plan.md', line: 10, rule_key: 'PC1', text: 'step lacks verifiable acceptance criteria for path A' }] } },
{ agent: 'scope-guardian', payload: { findings: [{ file: 'plan.md', line: 11, rule_key: 'SG1', text: 'step lacks verifiable acceptance criteria for path A' }] } },
];
const r = dedupFindings(sources);
assert.equal(r.findings.length, 1, 'jaccard merge should collapse near-duplicates');
assert.deepEqual(r.findings[0].raised_by.sort(), ['plan-critic', 'scope-guardian']);
assert.equal(r.dedup_stats.jaccard_dups, 1);
});
test('jaccard below threshold keeps both findings separate', () => {
const sources = [
{ agent: 'plan-critic', payload: { findings: [{ file: 'a.md', line: 1, rule_key: 'PC1', text: 'database migration risk' }] } },
{ agent: 'scope-guardian', payload: { findings: [{ file: 'b.md', line: 2, rule_key: 'SG1', text: 'unrelated frontend hover state polish' }] } },
];
const r = dedupFindings(sources);
assert.equal(r.findings.length, 2);
assert.equal(r.dedup_stats.jaccard_dups, 0);
});
test('empty / missing payloads tolerated (single-agent input)', () => {
const r = dedupFindings([
{ agent: 'plan-critic', payload: { findings: [{ file: 'a.md', line: 1, rule_key: 'PC1', text: 'one' }] } },
{ agent: 'scope-guardian', payload: null },
]);
assert.equal(r.findings.length, 1);
assert.deepEqual(r.findings[0].raised_by, ['plan-critic']);
});
test('all sources empty → empty result, dedup_stats zeros', () => {
const r = dedupFindings([
{ agent: 'plan-critic', payload: null },
{ agent: 'scope-guardian', payload: { findings: [] } },
]);
assert.equal(r.findings.length, 0);
assert.equal(r.dedup_stats.total_in, 0);
assert.equal(r.dedup_stats.total_out, 0);
});
test('CLI shim parses input files and emits valid deduped JSON', () => {
const dir = tmp();
try {
const planCritic = join(dir, 'pc.json');
const scopeGuardian = join(dir, 'sg.json');
writeFileSync(planCritic, JSON.stringify({
agent: 'plan-critic',
findings: [{ file: 'plan.md', line: 5, rule_key: 'PC1', text: 'duplicate finding shared by both' }],
}));
writeFileSync(scopeGuardian, JSON.stringify({
agent: 'scope-guardian',
findings: [{ file: 'plan.md', line: 5, rule_key: 'PC1', text: 'duplicate finding shared by both' }],
}));
const out = execFileSync(process.execPath, [
SHIM, '--plan-critic', planCritic, '--scope-guardian', scopeGuardian,
], { encoding: 'utf-8' });
const parsed = JSON.parse(out);
assert.equal(parsed.findings.length, 1);
assert.deepEqual(parsed.findings[0].raised_by.sort(), ['plan-critic', 'scope-guardian']);
assert.equal(parsed.dedup_stats.total_out, 1);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
test('CLI shim tolerates missing input files (returns empty deduped JSON)', () => {
const out = execFileSync(process.execPath, [SHIM], { encoding: 'utf-8' });
const parsed = JSON.parse(out);
assert.equal(parsed.findings.length, 0);
assert.equal(parsed.dedup_stats.total_in, 0);
});
// --- S23 — --stdin mode (Phase-9 dedup fix: read-only reviewers can't write
// temp files, so the orchestrator pipes both inline JSON blocks via stdin) ---
test('CLI --stdin merges {plan_critic, scope_guardian} payloads, provenance preserved', () => {
const stdin = JSON.stringify({
plan_critic: {
agent: 'plan-critic',
findings: [{ file: 'plan.md', line: 5, rule_key: 'PC1', text: 'duplicate finding shared by both' }],
},
scope_guardian: {
agent: 'scope-guardian',
findings: [{ file: 'plan.md', line: 5, rule_key: 'PC1', text: 'duplicate finding shared by both' }],
},
});
const out = execFileSync(process.execPath, [SHIM, '--stdin'], { input: stdin, encoding: 'utf-8' });
const parsed = JSON.parse(out);
assert.equal(parsed.findings.length, 1);
assert.deepEqual(parsed.findings[0].raised_by.sort(), ['plan-critic', 'scope-guardian']);
assert.equal(parsed.dedup_stats.total_out, 1);
});
test('CLI --stdin tolerates a single present payload (other key absent)', () => {
const stdin = JSON.stringify({
plan_critic: { agent: 'plan-critic', findings: [{ file: 'a.md', line: 1, rule_key: 'PC1', text: 'lonely finding' }] },
});
const out = execFileSync(process.execPath, [SHIM, '--stdin'], { input: stdin, encoding: 'utf-8' });
const parsed = JSON.parse(out);
assert.equal(parsed.findings.length, 1);
assert.deepEqual(parsed.findings[0].raised_by, ['plan-critic']);
});
test('severity survives the dedup, and a merge keeps the more severe of the two (the plan gate reads it)', () => {
const sources = [
{ agent: 'plan-critic', payload: { findings: [{ file: 'plan.md', line: 42, rule_key: 'PC1', severity: 'Minor', text: 'Step 4 lacks verifiable acceptance criteria' }] } },
{ agent: 'scope-guardian', payload: { findings: [{ file: 'plan.md', line: 42, rule_key: 'PC1', severity: 'blocker', text: 'Step 4 lacks verifiable acceptance criteria' }] } },
];
const r = dedupFindings(sources);
assert.equal(r.findings.length, 1);
assert.equal(r.findings[0].severity, 'blocker');
const lone = dedupFindings([{ agent: 'plan-critic', payload: { findings: [{ file: 'p', line: 1, rule_key: 'k', severity: 'MAJOR', text: 't' }] } }]);
assert.equal(lone.findings[0].severity, 'major');
const unrated = dedupFindings([{ agent: 'plan-critic', payload: { findings: [{ file: 'p', line: 1, rule_key: 'k', text: 't' }] } }]);
assert.equal(unrated.findings[0].severity, null, 'an unrated finding stays unrated here; the gate decides what that means');
});
test('sources says which reviewers delivered a findings block (null, misnamed or findings-less = absent)', () => {
const stdin = JSON.stringify({ plan_critic: { agent: 'plan-critic', findings: [] }, scopeGuardian: { findings: [] } });
const parsed = JSON.parse(execFileSync(process.execPath, [SHIM, '--stdin'], { input: stdin, encoding: 'utf-8' }));
assert.deepEqual(parsed.sources, [
{ agent: 'plan-critic', present: true },
{ agent: 'scope-guardian', present: false },
]);
const noFindings = dedupFindings([{ agent: 'plan-critic', payload: { agent: 'plan-critic' } }]);
assert.deepEqual(noFindings.sources, [{ agent: 'plan-critic', present: false }]);
});
test('CLI --stdin exits non-zero on malformed input (loud, not a silent empty merge)', () => {
assert.throws(
() => execFileSync(process.execPath, [SHIM, '--stdin'], { input: 'not json{', encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }),
/Command failed|status 1/,
'malformed --stdin must fail loudly so a broken hand-off cannot hide behind an empty merge',
);
});