voyage/lib/review/plan-review-dedup.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

243 lines
8.3 KiB
JavaScript

// lib/review/plan-review-dedup.mjs
// Phase-9 dedup helper for /trekplan adversarial review:
// merges plan-critic + scope-guardian findings into a single deduplicated
// stream, preserving provenance (which agent originally raised each finding).
//
// Two dedup signals:
// 1. Exact match — identical computeFindingId(file:line:rule_key) → merge.
// 2. Jaccard ≥ 0.7 on text-token sets → merge (catches near-duplicates).
//
// Provenance is preserved on the surviving finding's `raised_by` array.
// Severity is kept (normalized to blocker | major | minor, else null) and a
// merge keeps the more severe of the two: the plan gate (K3,
// lib/validators/plan-gate.mjs) stops on a BLOCKER, so a merge must never
// downgrade one.
//
// CLI shim — two input modes:
// file mode: node lib/review/plan-review-dedup.mjs \
// --plan-critic /tmp/x.json --scope-guardian /tmp/y.json
// stdin mode: node lib/review/plan-review-dedup.mjs --stdin (reads fd 0:
// one object {plan_critic, scope_guardian} of agent payloads)
// → stdout: deduped JSON, exit 0 on success.
//
// File mode tolerates empty / missing inputs (single-agent review still works).
// stdin mode is the Phase-9 path: the read-only reviewers (plan-critic,
// scope-guardian) cannot write temp files, so /trekplan pipes their inline JSON
// blocks here. Malformed stdin exits NON-ZERO — a broken hand-off must surface
// loudly, never collapse into a silent empty merge.
import { readFileSync, realpathSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { jaccardSimilarity, meetsThreshold } from '../parsers/jaccard.mjs';
import { computeFindingId } from '../parsers/finding-id.mjs';
export const DEFAULT_THRESHOLD = 0.7;
export const SEVERITIES = Object.freeze(['blocker', 'major', 'minor']);
/** 'Blocker' / 'MAJOR' → canonical lowercase; anything else → null (unrated). */
export function normalizeSeverity(s) {
const v = typeof s === 'string' ? s.trim().toLowerCase() : '';
return SEVERITIES.includes(v) ? v : null;
}
// Lower rank = more severe; unrated ranks with blocker so a merge never hides it.
const rank = s => (s === null ? 0 : SEVERITIES.indexOf(s));
function mergeInto(survivor, other) {
for (const a of other.raised_by) {
if (!survivor.raised_by.includes(a)) survivor.raised_by.push(a);
}
if (rank(other.severity) < rank(survivor.severity)) survivor.severity = other.severity;
}
/**
* Tokenize a finding's text for Jaccard comparison: lowercase, split on
* non-word, drop empties. Stable + deterministic.
*/
export function tokenize(text) {
if (typeof text !== 'string' || text.length === 0) return [];
return text.toLowerCase().split(/\W+/).filter(t => t.length > 0);
}
/**
* Normalize a single agent payload into an array of {agent, finding} pairs.
* Tolerates missing payload (returns []).
*/
function normalizeAgentPayload(payload, fallbackAgent) {
if (!payload || typeof payload !== 'object') return [];
const agent = (typeof payload.agent === 'string' && payload.agent.length > 0)
? payload.agent
: fallbackAgent;
const findings = Array.isArray(payload.findings) ? payload.findings : [];
return findings.map(f => ({ agent, finding: f }));
}
function annotate(finding, agent) {
const id = computeFindingId(
String(finding.file ?? 'unknown'),
finding.line ?? 0,
String(finding.rule_key ?? 'unknown'),
);
return {
id,
file: finding.file ?? null,
line: finding.line ?? null,
rule_key: finding.rule_key ?? null,
severity: normalizeSeverity(finding.severity),
text: typeof finding.text === 'string' ? finding.text : '',
raised_by: [agent],
};
}
/**
* Dedup an arbitrary collection of agent payloads.
*
* @param {Array<{agent: string, payload: object | null | undefined}>} sources
* @param {{ threshold?: number }} [opts]
* @returns {{
* findings: Array<object>,
* sources: Array<{agent: string, present: boolean}>,
* dedup_stats: { total_in: number, total_out: number,
* exact_id_dups: number, jaccard_dups: number }
* }}
*/
export function dedupFindings(sources, opts = {}) {
const threshold = typeof opts.threshold === 'number' ? opts.threshold : DEFAULT_THRESHOLD;
const incoming = [];
for (const s of sources) {
for (const pair of normalizeAgentPayload(s.payload, s.agent)) {
incoming.push(annotate(pair.finding, pair.agent));
}
}
const total_in = incoming.length;
// Pass 1 — exact id dedup
const byId = new Map();
let exact_id_dups = 0;
for (const f of incoming) {
const existing = byId.get(f.id);
if (existing) {
mergeInto(existing, f);
exact_id_dups += 1;
} else {
byId.set(f.id, f);
}
}
// Pass 2 — jaccard on text tokens; merge near-duplicates
const survivors = [];
let jaccard_dups = 0;
for (const f of byId.values()) {
const tokens = tokenize(f.text);
let merged = false;
for (const s of survivors) {
const sim = jaccardSimilarity(tokens, tokenize(s.text));
if (meetsThreshold(sim, threshold)) {
mergeInto(s, f);
jaccard_dups += 1;
merged = true;
break;
}
}
if (!merged) survivors.push(f);
}
return {
findings: survivors,
// Which reviewers actually delivered a findings block. `findings: []` from
// a reviewer that was absent (null payload, misspelled key, no `findings`)
// is not a clean review; plan-gate --record refuses it.
sources: sources.map(s => ({
agent: s.agent,
present: Boolean(s.payload && typeof s.payload === 'object' && Array.isArray(s.payload.findings)),
})),
dedup_stats: {
total_in,
total_out: survivors.length,
exact_id_dups,
jaccard_dups,
},
};
}
// ---- CLI shim ----------------------------------------------------------------
function parseArgs(argv) {
const out = {};
for (let i = 0; i < argv.length; i++) {
const a = argv[i];
if (a === '--plan-critic') out.planCritic = argv[++i];
else if (a === '--scope-guardian') out.scopeGuardian = argv[++i];
else if (a === '--threshold') out.threshold = Number(argv[++i]);
else if (a === '--stdin') out.stdin = true;
}
return out;
}
function readJsonOrNull(path) {
if (!path) return null;
try {
return JSON.parse(readFileSync(path, 'utf-8'));
} catch {
return null;
}
}
// --stdin mode reads ONE object {plan_critic, scope_guardian} from fd 0. Unlike
// the file path mode (which tolerates absent files so single-agent review still
// works), malformed stdin is a hard error: --stdin means the caller intended to
// pipe both inline review blocks, so a parse failure is a broken hand-off that
// must surface loudly — not collapse into a silent empty merge (the original
// Phase-9 defect: read-only reviewers never wrote the temp files, the helper
// swallowed the absence, and the dedup ran on nothing).
function readStdinSourcesOrExit() {
let raw;
try {
raw = readFileSync(0, 'utf-8');
} catch (err) {
process.stderr.write(`plan-review-dedup --stdin: cannot read stdin: ${err.message}\n`);
process.exit(1);
}
let parsed;
try {
parsed = JSON.parse(raw);
} catch (err) {
process.stderr.write(`plan-review-dedup --stdin: malformed JSON on stdin: ${err.message}\n`);
process.exit(1);
}
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
process.stderr.write('plan-review-dedup --stdin: expected an object {plan_critic, scope_guardian}\n');
process.exit(1);
}
return [
{ agent: 'plan-critic', payload: parsed.plan_critic ?? null },
{ agent: 'scope-guardian', payload: parsed.scope_guardian ?? null },
];
}
// realpath on both sides: `file://${argv[1]}` silently no-ops under a symlink
// or a path with a space.
function isMain() {
try {
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
} catch {
return false;
}
}
if (isMain()) {
const args = parseArgs(process.argv.slice(2));
const sources = args.stdin
? readStdinSourcesOrExit()
: [
{ agent: 'plan-critic', payload: readJsonOrNull(args.planCritic) },
{ agent: 'scope-guardian', payload: readJsonOrNull(args.scopeGuardian) },
];
const opts = {};
if (Number.isFinite(args.threshold)) opts.threshold = args.threshold;
const result = dedupFindings(sources, opts);
process.stdout.write(JSON.stringify(result, null, 2) + '\n');
process.exit(0);
}