voyage/lib/review/plan-review-dedup.mjs
2026-10-16 16:36:38 +02:00

377 lines
15 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).
//
// Three dedup signals:
// 1. Exact match — identical computeFindingId(file:line:rule_key) → merge.
// 2. Jaccard ≥ 0.7 on text-token sets → merge (catches near-duplicates).
// 3. Same anchor — the same file and the same line (≥ 1), and the content
// words of the shorter text mostly in the longer one (overlap ≥ the same
// 0.7) → merge. Two reviewers naming one defect at one place word it
// differently and pick their own rule_key: trial 2's pair on plan.md:248
// measured Jaccard 0.44 and stayed two (voyage-lekerepo, 2026-10-08).
// Only an anchored pair from two different reviewers, the shorter text
// holding ≥ 6 content words, is measured this way; the threshold for
// every other pair is unchanged.
//
// Pass-through for the converging critic loop (2026-10-15). A reviewer's
// top-level `previous` marks ({id: 40-hex, resolved: boolean}) come out as
// top-level `marks: [{agent, id, resolved}]` in input order; a malformed mark
// is dropped, never thrown on, and `sources` keeps exactly {agent, present}.
// A finding keeps an integer `line_end` ≥ `line`, rule B's `lowered_from`
// (normalized when it is a severity, absent when blank, and kept as given
// otherwise, so rule B refuses it and counts it as blocker) and a non-empty
// `caught_by`, and carries `rule_keys`: the union
// of the keys merged into it, so a security key survives a merge into a
// differently-keyed survivor. Two defects are fixed. A `line` that is not a
// finite integer becomes null and an empty `file` or `rule_key` becomes null,
// with 'unknown' / 0 given to computeFindingId, which threw on them and took
// the CLI down. And pass 1 merges equal ids only on a line ≥ 1: file-level
// findings (line 0 or none) of one rule_key no longer collapse into one; they
// go through the text passes, and a second survivor with a taken id gets
// `<rule_key>#n` (n = 2, 3, …) in its id.
//
// 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 } from 'node:fs';
import { jaccardSimilarity, meetsThreshold } from '../parsers/jaccard.mjs';
import { computeFindingId, parseFindingId } from '../parsers/finding-id.mjs';
import { isMain } from '../util/is-main.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);
}
const keys = [...(survivor.rule_keys ?? [])];
for (const k of other.rule_keys ?? []) if (!keys.includes(k)) keys.push(k);
if (keys.length > 0) survivor.rule_keys = keys;
// The more severe partner brings its rule-B record with it (or none); on
// equal severity the survivor keeps its own.
if (rank(other.severity) < rank(survivor.severity)) {
survivor.severity = other.severity;
for (const field of ['lowered_from', 'caught_by']) {
if (field in other) survivor[field] = other[field];
else delete survivor[field];
}
}
}
/**
* 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);
}
// Words that carry no content of a finding.
const STOP_WORDS = new Set([
'a', 'an', 'the', 'and', 'or', 'but', 'of', 'in', 'on', 'at', 'to', 'for', 'with', 'by', 'from', 'as',
'is', 'are', 'was', 'were', 'be', 'been', 'it', 'its', 'this', 'that', 'these', 'those', 'only', 'both',
'where', 'which', 'while', 'when', 'than', 'then', 'so', 'if', 'also', 'each', 'every', 'all', 'any',
'unlike', 'own',
]);
/** Content words of a finding: tokens minus stop words and single letters, a plural -s folded. */
export function contentTokens(text) {
return tokenize(text)
.filter(t => t.length > 1 && !STOP_WORDS.has(t))
.map(t => (t.length > 3 && t.endsWith('s') && !t.endsWith('ss') ? t.slice(0, -1) : t));
}
/** |A ∩ B| / min(|A|, |B|): how much of the shorter text the longer one holds. */
function overlap(a, b) {
const A = new Set(a);
const B = new Set(b);
if (A.size === 0 || B.size === 0) return 0;
let n = 0;
for (const x of A) if (B.has(x)) n += 1;
return n / Math.min(A.size, B.size);
}
/** Content-word overlap of two texts: how much of the shorter one the longer holds (0..1). */
export function contentOverlap(a, b) {
return overlap(contentTokens(a), contentTokens(b));
}
/** Same file and same line ≥ 1: line 0 (file-scoped) and a missing line anchor nothing. */
function sameAnchor(a, b) {
return typeof a.file === 'string' && a.file !== '' && a.file === b.file &&
Number.isInteger(a.line) && a.line > 0 && a.line === b.line;
}
// Overlap measures the shorter text against the longer, so a short text is
// held by almost any longer one on its line ("The token is checked." inside
// "The token is not checked before the write." scores 1.0). The anchor rule
// therefore merges only a pair from two different reviewers — one reviewer's
// two findings on one line are two findings by its own count — and only when
// the shorter text names enough to be compared (independent review, 2026-10-08).
export const ANCHOR_MIN_CONTENT_WORDS = 6;
function anchorDuplicate(f, s, threshold) {
if (!sameAnchor(f, s)) return false;
if (f.raised_by.some(a => s.raised_by.includes(a))) return false;
const a = new Set(contentTokens(f.text));
const b = new Set(contentTokens(s.text));
if (Math.min(a.size, b.size) < ANCHOR_MIN_CONTENT_WORDS) return false;
return meetsThreshold(overlap([...a], [...b]), threshold);
}
/**
* Normalize a single agent payload into its {agent, finding} pairs and its
* `previous` array. Tolerates missing payload (no pairs, no previous).
*/
function normalizeAgentPayload(payload, fallbackAgent) {
if (!payload || typeof payload !== 'object') return { agent: fallbackAgent, pairs: [], previous: [] };
const agent = (typeof payload.agent === 'string' && payload.agent.length > 0)
? payload.agent
: fallbackAgent;
const findings = Array.isArray(payload.findings) ? payload.findings : [];
const previous = Array.isArray(payload.previous) ? payload.previous : [];
return { agent, pairs: findings.map(f => ({ agent, finding: f })), previous };
}
/** A reviewer's mark on a previous finding, or null when it is not {id: 40-hex, resolved: boolean}. */
function readMark(m, agent) {
if (!m || typeof m !== 'object') return null;
if (!parseFindingId(m.id).valid) return null;
if (typeof m.resolved !== 'boolean') return null;
return { agent, id: m.id, resolved: m.resolved };
}
// The substitutes computeFindingId gets for a null file, line or rule_key.
const idOf = (file, line, ruleKey) =>
computeFindingId(String(file ?? 'unknown'), line ?? 0, String(ruleKey ?? 'unknown'));
function annotate(finding, agent) {
const file = finding.file === '' ? null : (finding.file ?? null);
const line = Number.isInteger(finding.line) ? finding.line : null;
const ruleKey = finding.rule_key === '' ? null : (finding.rule_key ?? null);
const out = {
id: idOf(file, line, ruleKey),
file,
line,
rule_key: ruleKey,
severity: normalizeSeverity(finding.severity),
text: typeof finding.text === 'string' ? finding.text : '',
raised_by: [agent],
};
if (line !== null && Number.isInteger(finding.line_end) && finding.line_end >= line) {
out.line_end = finding.line_end;
}
const lf = finding.lowered_from;
if (lf !== undefined && lf !== null && !(typeof lf === 'string' && lf.trim() === '')) {
out.lowered_from = normalizeSeverity(finding.lowered_from) ?? finding.lowered_from;
}
if (typeof finding.caught_by === 'string' && finding.caught_by.trim() !== '') {
out.caught_by = finding.caught_by;
}
if (typeof ruleKey === 'string') out.rule_keys = [ruleKey];
return out;
}
/**
* Dedup an arbitrary collection of agent payloads.
*
* @param {Array<{agent: string, payload: object | null | undefined}>} sources
* @param {{ threshold?: number }} [opts]
* @returns {{
* findings: Array<object>,
* marks: Array<{agent: string, id: string, resolved: boolean}>,
* sources: Array<{agent: string, present: boolean}>,
* dedup_stats: { total_in: number, total_out: number,
* exact_id_dups: number, jaccard_dups: number, anchor_dups: number }
* }}
*/
export function dedupFindings(sources, opts = {}) {
const threshold = typeof opts.threshold === 'number' ? opts.threshold : DEFAULT_THRESHOLD;
const incoming = [];
const marks = [];
for (const s of sources) {
const { agent, pairs, previous } = normalizeAgentPayload(s.payload, s.agent);
for (const pair of pairs) incoming.push(annotate(pair.finding, pair.agent));
for (const m of previous) {
const mark = readMark(m, agent);
if (mark) marks.push(mark);
}
}
const total_in = incoming.length;
// Pass 1 — exact id dedup, only on a line ≥ 1. A file-level finding (line 0
// or none) shares its id with every other one of its rule_key, so equal ids
// there say nothing: those go on to the text passes.
const byId = new Map();
const pass2 = [];
let exact_id_dups = 0;
for (const f of incoming) {
const anchored = Number.isInteger(f.line) && f.line > 0;
const existing = anchored ? byId.get(f.id) : undefined;
if (existing) {
mergeInto(existing, f);
exact_id_dups += 1;
} else {
if (anchored) byId.set(f.id, f);
pass2.push(f);
}
}
// Pass 2 — jaccard on text tokens; merge near-duplicates. Pass 3 rides in
// the same loop: an anchored pair that Jaccard misses is measured by overlap.
const survivors = [];
let jaccard_dups = 0;
let anchor_dups = 0;
for (const f of pass2) {
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 (anchorDuplicate(f, s, threshold)) {
mergeInto(s, f);
anchor_dups += 1;
merged = true;
break;
}
}
if (!merged) survivors.push(f);
}
// A second survivor with an id already taken gets `<rule_key>#n` in its id.
const taken = new Set();
for (const f of survivors) {
if (taken.has(f.id)) {
let n = 2;
let id;
while (taken.has(id = idOf(f.file, f.line, `${f.rule_key ?? 'unknown'}#${n}`))) n += 1;
f.id = id;
}
taken.add(f.id);
}
return {
findings: survivors,
marks,
// 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,
anchor_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 },
];
}
if (isMain(import.meta.url)) {
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);
}