voyage/tests/lib/plan-review-dedup.test.mjs
Kjell Tore Guttormsen b4edc12bec fix(voyage): S23 — make /trekplan Phase 9 dedup executable (defect #1)
Phase 9's dedup hand-off was broken on two layers, both surfaced by the S22
dogfood: (a) plan-critic + scope-guardian (Read/Glob/Grep, no Write) were told
to write /tmp/*-out.json the dedup helper reads — they cannot; (b) even with
Write, their Output format emits markdown, not the helper's
{agent,findings:[{file,line,rule_key,text}]} schema. readJsonOrNull then
swallowed the absent files -> a silent empty merge that discarded every finding.

Fix (operator-chosen A'): keep the reviewers read-only; make the hand-off run.
- plan-review-dedup.mjs gains a --stdin mode reading {plan_critic,scope_guardian};
  malformed stdin exits non-zero so a broken hand-off surfaces loudly instead of
  collapsing into a silent empty merge. File mode + its tests are untouched.
- plan-critic.md + scope-guardian.md now emit a trailing machine-readable `json`
  findings block (the inline hand-off; no Write tool needed).
- trekplan.md + planning-orchestrator.md Phase 9 rewritten in lockstep: extract
  both blocks, pipe via heredoc into --stdin. No temp files, portable, no
  pathguard dependency.

TDD: malformed-stdin test failed first (CLI ignored stdin -> exit 0 = the bug),
green after impl. New S23 doc-pin asserts both docs use --stdin (not the dead
/tmp paths) and both agents declare the json block. Suite 724 (722/2/0); live
HEAD baseline was 720, not the stale 705 STATE carried forward.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqBYc8Ltrk7LipyJmGxXiB
2026-06-19 21:12:36 +02:00

173 lines
7.6 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('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',
);
});