open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

fix(review): plan dedup passes the reviewers' marks and rule-B fields through and keeps distinct file-level findings

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-16 00:14:02 +02:00
commit 31fa6f8e43
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 322 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

119
lib/review/plan-review-dedup.mjs
View file

@ -15,6 +15,21 @@
// 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) 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,
@ -55,7 +70,18 @@ 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;
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];
}
}
}
/**
@ -92,6 +118,11 @@ function overlap(a, b) {
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 &&
@ -116,33 +147,57 @@ function anchorDuplicate(f, s, threshold) {
}
/**
* Normalize a single agent payload into an array of {agent, finding} pairs.
* Tolerates missing payload (returns []).
* 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 [];
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 : [];
return findings.map(f => ({ agent, finding: f }));
const previous = Array.isArray(payload.previous) ? payload.previous : [];
return { agent, pairs: findings.map(f => ({ agent, finding: f })), previous };
}
const FINDING_ID_RE = /^[0-9a-f]{40}$/;
/** 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 (typeof m.id !== 'string' || !FINDING_ID_RE.test(m.id)) 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 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,
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;
}
if (finding.lowered_from !== undefined && finding.lowered_from !== null) {
out.lowered_from = normalizeSeverity(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;
}
/**
@ -152,6 +207,7 @@ function annotate(finding, agent) {
* @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 }
@ -161,24 +217,33 @@ export function dedupFindings(sources, opts = {}) {
const threshold = typeof opts.threshold === 'number' ? opts.threshold : DEFAULT_THRESHOLD;
const incoming = [];
const marks = [];
for (const s of sources) {
for (const pair of normalizeAgentPayload(s.payload, s.agent)) {
incoming.push(annotate(pair.finding, pair.agent));
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
// 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 existing = byId.get(f.id);
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 {
byId.set(f.id, f);
if (anchored) byId.set(f.id, f);
pass2.push(f);
}
}
@ -187,7 +252,7 @@ export function dedupFindings(sources, opts = {}) {
const survivors = [];
let jaccard_dups = 0;
let anchor_dups = 0;
for (const f of byId.values()) {
for (const f of pass2) {
const tokens = tokenize(f.text);
let merged = false;
for (const s of survivors) {
@ -208,8 +273,20 @@ export function dedupFindings(sources, opts = {}) {
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;
while (taken.has(idOf(f.file, f.line, `${f.rule_key ?? 'unknown'}#${n}`))) n += 1;
f.id = idOf(f.file, f.line, `${f.rule_key ?? 'unknown'}#${n}`);
}
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.