// 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 // `#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). A merge // keeps the copy rule B counts highest on the merged keys, then the higher // rated severity; on a full tie the survivor keeps its own. The plan gate (K3, // lib/validators/plan-gate.mjs) stops on a BLOCKER, so a merge must never // downgrade one, in either reviewer order. // // 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'; import { getRule } from './rule-catalogue.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)); // ---- rule B (lib/review/critic-diff.mjs imports these back) -------------------- const higher = (a, b) => (SEVERITIES.indexOf(a) <= SEVERITIES.indexOf(b) ? a : b); /** A rule_key whose catalogue entry is in the security category (case and spaces ignored). */ export function isSecurityKey(key) { return getRule(String(key).trim().toUpperCase())?.category === 'security'; } /** The rule keys of a finding: `rule_keys` when it is a list, else its one `rule_key`. */ export function keysOf(f) { const keys = Array.isArray(f.rule_keys) ? f.rule_keys : [f.rule_key]; return keys.filter(k => typeof k === 'string' && k !== ''); } export const isSecurity = f => keysOf(f).some(isSecurityKey); /** * The rule-B record of one finding at its rated severity. * A null or absent `lowered_from` is no record at all. * @returns {{severity: string, record: object}} */ export function ruleB(f, rated) { if (f.lowered_from === undefined || f.lowered_from === null) return { severity: rated, record: {} }; const from = normalizeSeverity(f.lowered_from); const caught = typeof f.caught_by === 'string' && f.caught_by.trim() !== '' ? f.caught_by : null; const security = isSecurity(f); if (from === 'major' && rated === 'minor' && caught && !security) { return { severity: 'minor', record: { original_severity: 'major', lowered_by: `rule B: ${caught}`, caught_by: caught } }; } let why; if (from === null) why = 'rule B: lowered_from is not a severity; counted as blocker'; else if (from === 'blocker') why = 'rule B never lowers a blocker'; else if (from !== 'major') why = `rule B lowers only major to minor, not ${from}`; else if (rated !== 'minor') why = `rule B lowers major to minor only; the finding is rated ${rated}`; else if (security) why = 'rule B never lowers a security finding'; else why = 'rule B needs caught_by: the named test, Regression: command or gate that will fail on it'; const severity = higher(from ?? 'blocker', rated); const record = { lowering_refused: why }; if (caught) record.caught_by = caught; if (severity !== rated) record.original_severity = rated; return { severity, record }; } // ---- merge --------------------------------------------------------------------- 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 partner wins when rule B counts it higher on the merged keys, or, on an // equal count, when it is rated higher; the winner brings its severity and // its rule-B record (or none). On a full tie the survivor keeps its own. const counted = x => ruleB({ ...x, rule_keys: keys }, normalizeSeverity(x.severity) ?? 'blocker').severity; const co = rank(counted(other)); const cs = rank(counted(survivor)); if (co < cs || (co === cs && 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, * 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 `#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); }