voyage/lib/review/review-rounds.mjs
Kjell Tore Guttormsen 9edc998fac
refactor: simplify pass (trekexecute)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-04 11:29:46 +01:00

362 lines
16 KiB
JavaScript

// lib/review/review-rounds.mjs
// /trekreview rounds (review_version 1.2, fix round 4.5b2 part B): round 2
// and later see the diff since the previous round and the previous findings,
// carry what is not resolved, and stop at one cap.
//
// `MAX_REVIEW_ROUNDS` is the one cap: /trekreview refuses a fourth round, and
// /trekexecute's review loop reads the same constant (`--cap`).
//
// prepare({projectDir, since, repo, workflow, dryRun}): the round of a run.
// `<run>` is the first 12 hex of `since` (the run's `scope_sha_start`).
// - Same run (review.md exists and its `scope_sha_start` equals `since`): a
// later round. An archive `review-round-<run>-<n>.md` is review.md's own
// archive (written before a stop at this point) when its bytes equal
// review.md's, or when all three hold: <n> is review.md's round (the
// `round` field, or 1 when absent), its trailing JSON block hashes to the
// same value as review.md's, and its `scope_sha_end` equals review.md's
// (an answer, its R-step or any other text outside the block and
// `scope_sha_end` recorded after the stop does not make another round).
// The other archives are A. The previous review's round is P = 1 + |A|,
// derived, never read from the `round` field alone: a `round` field that
// disagrees is exit 2. The new round is P + 1. An archive that is a
// regular file but cannot be read is exit 2 (not measured).
// - Round MAX_REVIEW_ROUNDS + 1 is refused (exit 1); so is round 2 or later
// with `workflow` (exit 1): the --workflow path neither hands the reviewers
// the round input nor writes their outputs.
// - The round base is the previous `scope_sha_end`: a full sha that is an
// ancestor of HEAD, else exit 2 (not measured).
// - review.md is copied to `review-round-<run>-<P>.md`; an existing file
// there is kept when its bytes are equal, refreshed with review.md's bytes
// (`archived: true`) when it is the own archive by the rule above, else
// exit 2.
// - Otherwise (no review.md, or another run): round 1.
// - Clean slate, every round: the regular files `.review-engine-payload-*.json`
// and `.review-out-*.md` of the project folder are deleted, so any such
// file finalize finds was written this round.
// - `dryRun` computes the same result and writes, copies and deletes nothing.
// Nothing is written before every check has passed.
//
// carry(previous, current, {marks, ran, previousRan}): a current finding
// matches a previous one in the same file with the same rule_key, or with
// `contentOverlap` >= DEFAULT_THRESHOLD (lib/review/plan-review-dedup.mjs); ids
// are never compared, and each finding matches at most one other. A previous
// finding resolves only on positive evidence of this round (the isResolved
// rule of lib/review/critic-diff.mjs, with "every reviewer that ran then"
// standing in for the raisers):
// - a matched previous finding is carried;
// - an engine finding is resolved only when its engine is in `ran.engines`,
// it was not raised again, and no reviewer marks it `resolved: false`;
// - a reviewer-only finding is resolved only when `previousRan.reviewers` is
// non-empty, every reviewer in it is in `ran.reviewers`, each marked it
// `resolved: true`, and none marked it `resolved: false`;
// - `previousRan` null (the previous review is not finalized): nothing resolves.
// An unresolved previous finding stays in `findings` with `carried_from` (the
// previous id), at the higher of the two severities; a matching current
// finding folds into it and keeps its place (and its id until finalize
// recomputes ids), so it is not counted new. A new finding keeps its severity
// wherever it cites: rule A never reaches reviews (decision 95). Two
// occurrences of one class stay two findings (`rule_key#n` ids).
//
// CLI:
// node lib/review/review-rounds.mjs prepare --project <d> --since <sha>
// [--repo <d>] [--workflow] [--dry-run] --json
// prints one JSON line {round, round_base, previous, run} or {reason};
// exit 0 = the round, 1 = refused (the cap, or round 2+ on --workflow),
// 2 = not measured (2 is never green)
// node lib/review/review-rounds.mjs --cap prints MAX_REVIEW_ROUNDS
import { spawnSync } from 'node:child_process';
import { createHash } from 'node:crypto';
import { copyFileSync, lstatSync, readFileSync, readdirSync, unlinkSync } from 'node:fs';
import { join } from 'node:path';
import { fencedBlocks, isJsonBlock } from './findings-schema.mjs';
import { contentOverlap, DEFAULT_THRESHOLD } from './plan-review-dedup.mjs';
import { FULL_SHA_RE, reviewedSha } from './reviewed-commit.mjs';
import { scopeShas } from './pre-existing.mjs';
import { getRule } from './rule-catalogue.mjs';
import { isMain } from '../util/is-main.mjs';
export const MAX_REVIEW_ROUNDS = 3;
const SEVERITY_RANK = ['BLOCKER', 'MAJOR', 'MINOR', 'SUGGESTION'];
const CLEAN_SLATE_RE = /^(?:\.review-engine-payload-.+\.json|\.review-out-.+\.md)$/;
/** The run id of a round: the first 12 hex of the run's `scope_sha_start`. */
export const runId = (sha) => String(sha ?? '').toLowerCase().slice(0, 12);
/** The archive name of round `n` of run `run`. */
export const archiveName = (run, n) => `review-round-${run}-${n}.md`;
/** A frontmatter field as written (raw), or null when absent. */
export function frontmatterField(text, key) {
const fm = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(String(text ?? ''));
if (!fm) return null;
const m = new RegExp(`^${key}:[ \\t]*(.*?)[ \\t]*$`, 'm').exec(fm[1]);
return m ? m[1] : null;
}
/** A regular file's bytes, or null (missing, a symlink, a directory, unreadable). */
function readRegular(p) {
try {
if (!lstatSync(p).isFile()) return null;
return readFileSync(p);
} catch { return null; }
}
/** The regular files of `dir` whose names match `re`. */
function regularFiles(dir, re) {
let names;
try { names = readdirSync(dir); } catch { return []; }
return names.filter((n) => re.test(n)).filter((n) => {
try { return lstatSync(join(dir, n)).isFile(); } catch { return false; }
}).sort();
}
/**
* sha256 of the trailing fenced block's body when it is a closed `json`
* block, else null. A copy of findingsBlockSha (lib/review/review-answers.mjs,
* which imports this module, so importing it back would close a cycle).
*/
function blockSha(text) {
const last = fencedBlocks(String(text ?? '')).at(-1);
return last && last.closed && isJsonBlock(last) ? createHash('sha256').update(last.body).digest('hex') : null;
}
const notMeasured = (reason, detail = '') => ({ exit: 2, reason: `${reason} — not measured${detail ? ` (${detail})` : ''}` });
/**
* The round of a run, with its archive and clean slate (see the header).
* @returns {{exit: 0, round: number, round_base: string|null, previous: string|null, run: string, archived?: boolean, deleted: string[]} | {exit: 1|2, reason: string}}
*/
export function prepare({ projectDir, since, repo, workflow = false, dryRun = false } = {}) {
if (!projectDir) return { exit: 2, reason: '--project is required' };
if (!FULL_SHA_RE.test(String(since ?? ''))) return notMeasured('--since must be a full commit id');
try {
if (!lstatSync(projectDir).isDirectory()) return notMeasured(`--project ${projectDir} is not a directory`);
} catch (e) { return notMeasured(`--project ${projectDir}: ${e.message}`); }
const run = runId(since);
const slate = regularFiles(projectDir, CLEAN_SLATE_RE);
const clean = () => {
if (dryRun) return [];
for (const n of slate) unlinkSync(join(projectDir, n));
return slate;
};
const reviewPath = join(projectDir, 'review.md');
let reviewThere = true;
try { lstatSync(reviewPath); } catch { reviewThere = false; }
const review = readRegular(reviewPath);
if (reviewThere && review === null) return notMeasured('review.md is not a readable regular file');
const text = review === null ? null : review.toString('utf8');
const start = text === null ? '' : scopeShas(text).start;
const sameRun = start !== '' && start.toLowerCase() === String(since).toLowerCase();
if (!sameRun) {
// Round 1, on either path.
return { exit: 0, round: 1, round_base: null, previous: null, run, deleted: clean() };
}
const archiveRe = new RegExp(`^review-round-${run}-(\\d+)\\.md$`);
const archives = regularFiles(projectDir, archiveRe);
const roundField = frontmatterField(text, 'round');
// review.md's own archive (see the header): byte-equal first, then by content.
const ownName = archiveName(run, roundField === null ? 1 : roundField);
const ownBlock = blockSha(text);
const ownEnd = reviewedSha(text);
const isOwn = (name, bytes) => {
if (bytes.equals(review)) return true;
if (name !== ownName || ownBlock === null || ownEnd === '') return false;
const t = bytes.toString('utf8');
return blockSha(t) === ownBlock && reviewedSha(t) === ownEnd;
};
const others = [];
for (const n of archives) {
const bytes = readRegular(join(projectDir, n));
if (bytes === null) return notMeasured(`archive ${n} is not readable`);
if (!isOwn(n, bytes)) others.push(n);
}
const P = 1 + others.length;
if (roundField !== null && roundField !== String(P)) {
return notMeasured('round count does not match the archives', `round: ${roundField}, ${others.length} earlier archive(s) of run ${run}`);
}
const round = P + 1;
if (round > MAX_REVIEW_ROUNDS) {
return { exit: 1, reason: `review round cap reached (${MAX_REVIEW_ROUNDS}) — the operator re-plans` };
}
if (workflow) {
return { exit: 1, reason: 'from round 2 /trekreview runs on the standard path — re-run without --workflow' };
}
const base = ownEnd;
if (!base || !isAncestorOfHead(repo ?? projectDir, base)) {
return notMeasured('round base is not an ancestor of HEAD');
}
const previous = archiveName(run, P);
const target = join(projectDir, previous);
let present = true;
try { lstatSync(target); } catch { present = false; }
// A file there that is not a regular file and review.md's own archive
// "differs"; the own archive with other bytes is refreshed.
let refresh = false;
if (present) {
const bytes = readRegular(target);
if (bytes === null || !isOwn(previous, bytes)) return notMeasured(`archive ${previous} differs`);
refresh = !bytes.equals(review);
}
let archived = false;
if (!dryRun && (!present || refresh)) {
copyFileSync(reviewPath, target);
archived = true;
}
return { exit: 0, round, round_base: base, previous, run, archived, deleted: clean() };
}
/** `git merge-base --is-ancestor <sha> HEAD` in `repo` succeeded. */
function isAncestorOfHead(repo, sha) {
const r = spawnSync('git', ['-C', repo, 'merge-base', '--is-ancestor', sha, 'HEAD'], { encoding: 'utf8' });
return !r.error && r.status === 0;
}
// ---- carry ----------------------------------------------------------------------
const sevOf = (f) => String(f?.severity ?? '').toUpperCase();
/** The more severe of two severities; an unknown one ranks as BLOCKER (never lower). */
function higher(a, b) {
const ra = SEVERITY_RANK.indexOf(a);
const rb = SEVERITY_RANK.indexOf(b);
if (ra === -1) return a || 'BLOCKER';
if (rb === -1) return b || 'BLOCKER';
return ra <= rb ? a : b;
}
const textOf = (f) => `${f?.title ?? ''} ${f?.detail ?? ''}`;
/**
* The carry rule: same file, then the same rule_key or a content overlap at
* or above the dedup threshold. Ids are never compared.
*/
export function sameFinding(a, b) {
if (!a || !b || typeof a.file !== 'string' || a.file === '' || a.file !== b.file) return false;
if (typeof a.rule_key === 'string' && a.rule_key !== '' && a.rule_key === b.rule_key) return true;
return contentOverlap(textOf(a), textOf(b)) >= DEFAULT_THRESHOLD;
}
/** Lexicographic comparison of two equal-length number lists. */
function before(a, b) {
for (let j = 0; j < a.length; j++) if (a[j] !== b[j]) return a[j] < b[j];
return false;
}
/** The current finding `p` folds into: same rule_key first, then the nearest line, then the larger overlap, then order. */
function bestMatch(p, current, taken) {
let best = -1;
let key = null;
current.forEach((c, i) => {
if (taken.has(i) || !sameFinding(p, c)) return;
const k = [
c.rule_key === p.rule_key ? 0 : 1,
Number.isInteger(c.line) && Number.isInteger(p.line) ? Math.abs(c.line - p.line) : Number.MAX_SAFE_INTEGER,
-contentOverlap(textOf(p), textOf(c)),
];
if (key === null || before(k, key)) {
best = i;
key = k;
}
});
return best;
}
/**
* A security finding: its catalogue category is `security`, or it came from
* the `security-review` engine. Never accepted, never left out as pre-existing.
* Defined here and re-exported by review-answers.mjs, which imports this module.
*/
export function isSecurityFinding(f) {
return getRule(f?.rule_key)?.category === 'security' || f?.engine === 'security-review';
}
/** The folded finding: the current one's place and text, the higher severity, a security origin never lost. */
function fold(p, c) {
const out = { ...c, severity: higher(sevOf(p), sevOf(c)), carried_from: p.id };
if (isSecurityFinding(p) && !isSecurityFinding(c)) {
out.rule_key = p.rule_key;
if (p.engine !== undefined) out.engine = p.engine;
}
return out;
}
/** A previous finding carried as it was: no id (finalize computes it), no old marks. */
function carried(p) {
const { id: _id, pre_existing: _pre, carried_from: _cf, ...rest } = p;
return { id: null, ...rest, severity: sevOf(p) || 'BLOCKER', carried_from: p.id };
}
/** Was previous finding `p` resolved this round (positive evidence only)? */
function resolvedNow(p, { marks, ran, previousRan }) {
if (!previousRan || typeof previousRan !== 'object') return false;
const ms = (Array.isArray(marks) ? marks : []).filter((m) => m && m.id === p.id);
if (ms.some((m) => m.resolved === false)) return false;
const ranEngines = Array.isArray(ran?.engines) ? ran.engines : [];
const ranReviewers = Array.isArray(ran?.reviewers) ? ran.reviewers : [];
if (typeof p.engine === 'string' && p.engine !== '') return ranEngines.includes(p.engine);
const then = Array.isArray(previousRan.reviewers) ? previousRan.reviewers : [];
if (then.length === 0) return false;
return then.every((r) => ranReviewers.includes(r) && ms.some((m) => m.reviewer === r && m.resolved === true));
}
/**
* Carry the previous round's findings into this round's (see the header).
* @param {object[]} previous - the previous review's JSON findings
* @param {object[]} current - this round's JSON findings
* @param {{ marks?: Array<{id, resolved, reviewer}>, ran?: {engines, reviewers}, previousRan?: {engines, reviewers}|null }} ctx
* @returns {{ findings: object[], resolved: string[], folded: number }}
*/
export function carry(previous, current, { marks = [], ran = { engines: [], reviewers: [] }, previousRan = null } = {}) {
const prev = (Array.isArray(previous) ? previous : []).filter((p) => p && typeof p === 'object');
const cur = (Array.isArray(current) ? current : []).filter((c) => c && typeof c === 'object');
const findings = cur.map((c) => ({ ...c }));
const taken = new Set();
const appended = [];
const resolved = [];
for (const p of prev) {
const i = bestMatch(p, cur, taken);
if (i >= 0) {
taken.add(i);
findings[i] = fold(p, cur[i]);
continue;
}
if (resolvedNow(p, { marks, ran, previousRan })) {
if (typeof p.id === 'string') resolved.push(p.id);
continue;
}
appended.push(carried(p));
}
return { findings: [...findings, ...appended], resolved, folded: taken.size };
}
// ---- CLI ------------------------------------------------------------------------
function opt(args, name) {
const i = args.indexOf(name);
return i >= 0 ? args[i + 1] : undefined;
}
if (isMain(import.meta.url)) {
const args = process.argv.slice(2);
if (args[0] === '--cap') {
process.stdout.write(`${MAX_REVIEW_ROUNDS}\n`);
process.exit(0);
}
let result;
if (args[0] !== 'prepare') {
process.stderr.write('Usage: review-rounds.mjs prepare --project <d> --since <sha> [--repo <d>] [--workflow] [--dry-run] --json | --cap\n');
process.exit(2);
}
try {
result = prepare({
projectDir: opt(args, '--project'), since: opt(args, '--since'), repo: opt(args, '--repo'),
workflow: args.includes('--workflow'), dryRun: args.includes('--dry-run'),
});
} catch (e) {
result = { exit: 2, reason: `review-rounds crashed: ${e.message} — not measured` };
}
const { exit, ...rest } = result;
process.stdout.write(`${JSON.stringify(rest)}\n`);
process.exit(exit);
}