refactor: simplify pass (trekexecute)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
f6bdc2de59
commit
af90e82794
12 changed files with 303 additions and 186 deletions
|
|
@ -291,11 +291,7 @@ export function dedupByTriplet(findings) {
|
|||
if (!tripletGroups.has(key)) tripletGroups.set(key, []);
|
||||
tripletGroups.get(key).push({ f, i });
|
||||
});
|
||||
const groups = [...tripletGroups.values()].flatMap((entries) => groupTriplet(
|
||||
entries,
|
||||
(a, b) => a.reviewer !== b.reviewer || sameTitle(a, b),
|
||||
(f) => f.reviewer,
|
||||
));
|
||||
const groups = [...tripletGroups.values()].flatMap((entries) => groupTriplet(entries));
|
||||
const out = [];
|
||||
for (const group of groups) {
|
||||
// Ranked as the JSDoc above says. A brief-conflict mark is not carried over
|
||||
|
|
@ -353,9 +349,14 @@ export function dedupByTriplet(findings) {
|
|||
return out.map((f, i) => ({ ...f, id: ids[i] }));
|
||||
}
|
||||
|
||||
/** A finding's title as the grouping compares it: trimmed, lower case. */
|
||||
function normTitle(f) {
|
||||
return String(f.title).trim().toLowerCase();
|
||||
}
|
||||
|
||||
function sameTitle(a, b) {
|
||||
if (a.title === undefined || b.title === undefined) return true;
|
||||
return String(a.title).trim().toLowerCase() === String(b.title).trim().toLowerCase();
|
||||
return normTitle(a) === normTitle(b);
|
||||
}
|
||||
|
||||
/** Code-unit order of two strings (never the locale's). */
|
||||
|
|
@ -364,20 +365,15 @@ function cmpStr(a, b) {
|
|||
}
|
||||
|
||||
/**
|
||||
* The groups of one triplet's findings, in the two passes `dedupByTriplet`
|
||||
* describes. `entries` are `{ f, i }` (the finding and its input index);
|
||||
* `fits(m, f)` says member `m` may share a group with `f`; `whoOf(f)` names the
|
||||
* reviewer for the canonical order. Returns arrays of findings, each in the
|
||||
* order the passes place them (classes alphabetically, each class in canonical
|
||||
* order, untitled findings last in canonical order), the groups in the order
|
||||
* of their earliest input index.
|
||||
* The groups of one triplet's findings, in the two passes and the order
|
||||
* `dedupByTriplet` describes. `entries` are `{ f, i }` (the finding and its
|
||||
* input index). Returns arrays of findings.
|
||||
* @param {Array<{f: object, i: number}>} entries
|
||||
* @param {(m: object, f: object) => boolean} fits
|
||||
* @param {(f: object) => unknown} whoOf
|
||||
* @returns {object[][]}
|
||||
*/
|
||||
function groupTriplet(entries, fits, whoOf) {
|
||||
const canon = (x, y) => cmpStr(String(whoOf(x.f) ?? ''), String(whoOf(y.f) ?? ''))
|
||||
function groupTriplet(entries) {
|
||||
const fits = (m, f) => m.reviewer !== f.reviewer || sameTitle(m, f);
|
||||
const canon = (x, y) => cmpStr(String(x.f.reviewer ?? ''), String(y.f.reviewer ?? ''))
|
||||
|| cmpStr(String(x.f.detail ?? ''), String(y.f.detail ?? ''))
|
||||
|| cmpStr(JSON.stringify(x.f), JSON.stringify(y.f));
|
||||
const classes = new Map();
|
||||
|
|
@ -387,7 +383,7 @@ function groupTriplet(entries, fits, whoOf) {
|
|||
untitled.push(e);
|
||||
continue;
|
||||
}
|
||||
const t = String(e.f.title).trim().toLowerCase();
|
||||
const t = normTitle(e.f);
|
||||
if (!classes.has(t)) classes.set(t, []);
|
||||
classes.get(t).push(e);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -25,10 +25,10 @@
|
|||
// shell; a git call that fails is a stale review (fails closed), never ok.
|
||||
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { MARK_START, MARK_END, withPointer } from './recurrence.mjs';
|
||||
import { LESSONS_FILE_NAME, MARK_START, MARK_END, withPointer } from './recurrence.mjs';
|
||||
|
||||
const SHA_RE = /^[0-9a-f]{7,40}$/i;
|
||||
const LESSONS_REL = '.claude/rules/voyage-review-lessons.md';
|
||||
const LESSONS_REL = `.claude/rules/${LESSONS_FILE_NAME}`; // posix, as `git diff` prints it
|
||||
const CLAUDE_REL = 'CLAUDE.md';
|
||||
const SYMLINK_MODE = '120000';
|
||||
|
||||
|
|
@ -42,7 +42,7 @@ export function reviewedSha(reviewText) {
|
|||
const m = fm && /^scope_sha_end:[ \t]*(["']?)([^"'\s#]*)\1[ \t]*(?:#.*)?$/m.exec(fm[1]);
|
||||
// A commit sha only: a ref name (HEAD, main, a tag) resolves to whatever it
|
||||
// points at now, and coverage would pass whatever was reviewed (round 4).
|
||||
return m && /^[0-9a-f]{7,40}$/i.test(m[2]) ? m[2] : '';
|
||||
return m && SHA_RE.test(m[2]) ? m[2] : '';
|
||||
}
|
||||
|
||||
function git(repo, args) {
|
||||
|
|
@ -53,10 +53,14 @@ function stale(reason) {
|
|||
return { ok: false, reason: `review-stale: ${reason}` };
|
||||
}
|
||||
|
||||
/** What a git call said: its spawn error, else the first line of its stderr. */
|
||||
function gitNote(r) {
|
||||
return r.error?.message ?? String(r.stderr ?? '').trim().split('\n')[0];
|
||||
}
|
||||
|
||||
/** A failed git call as a stale reason: fails closed. */
|
||||
function gitFailed(what, r) {
|
||||
const note = (r.error?.message ?? String(r.stderr ?? '').trim().split('\n')[0]) || `exit ${r.status}`;
|
||||
return stale(`git ${what} failed (${note})`);
|
||||
return stale(`git ${what} failed (${gitNote(r) || `exit ${r.status}`})`);
|
||||
}
|
||||
|
||||
const short = (c) => c.slice(0, 12);
|
||||
|
|
@ -74,18 +78,18 @@ function outsideBlock(text) {
|
|||
}
|
||||
|
||||
/**
|
||||
* One file in one tree: `{ ok, absent, mode, text }`; absent reads as ''.
|
||||
* One file in one tree: `{ ok, mode, text }`; absent reads as mode null, text ''.
|
||||
* `--full-tree`: the path is from the repo root, as `git diff` prints it.
|
||||
*/
|
||||
function blobAt(repo, rev, path) {
|
||||
const ls = git(repo, ['ls-tree', '--full-tree', '-z', rev, '--', path]);
|
||||
if (ls.status !== 0) return { ok: false, fail: gitFailed(`ls-tree ${rev}`, ls) };
|
||||
const entry = ls.stdout.split('\0').find(Boolean);
|
||||
if (!entry) return { ok: true, absent: true, mode: null, text: '' };
|
||||
if (!entry) return { ok: true, mode: null, text: '' };
|
||||
const mode = entry.split(' ')[0];
|
||||
const blob = git(repo, ['cat-file', 'blob', `${rev}:${path}`]);
|
||||
if (blob.status !== 0) return { ok: false, fail: gitFailed(`cat-file ${rev}:${path}`, blob) };
|
||||
return { ok: true, absent: false, mode, text: blob.stdout };
|
||||
return { ok: true, mode, text: blob.stdout };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -101,7 +105,7 @@ export function checkReviewedCommit({ repo, sha } = {}) {
|
|||
if (!SHA_RE.test(given)) return stale(notCommit);
|
||||
const resolved = git(repo, ['rev-parse', '--verify', '--quiet', `${given}^{commit}`]);
|
||||
if (resolved.status !== 0) {
|
||||
const note = (resolved.error?.message ?? String(resolved.stderr ?? '').trim().split('\n')[0]);
|
||||
const note = gitNote(resolved);
|
||||
return stale(note ? `${notCommit} (${note})` : notCommit);
|
||||
}
|
||||
const full = resolved.stdout.trim();
|
||||
|
|
@ -123,14 +127,14 @@ export function checkReviewedCommit({ repo, sha } = {}) {
|
|||
const lessons = `${prefix}${LESSONS_REL}`;
|
||||
const claude = `${prefix}${CLAUDE_REL}`;
|
||||
|
||||
const roots = git(repo, ['rev-list', '--max-parents=0', `${full}..HEAD`]);
|
||||
if (roots.status !== 0) return gitFailed('rev-list --max-parents=0', roots);
|
||||
const root = roots.stdout.split('\n').find(Boolean);
|
||||
if (root) return stale(`commit ${short(root)} has no parent`);
|
||||
// One line per commit, `<commit> <parent>...`; a line with one field is a root.
|
||||
const list = git(repo, ['rev-list', '--parents', `${full}..HEAD`]);
|
||||
if (list.status !== 0) return gitFailed('rev-list --parents', list);
|
||||
const lines = list.stdout.split('\n').filter(Boolean).map((l) => l.split(' '));
|
||||
const root = lines.find((f) => f.length === 1);
|
||||
if (root) return stale(`commit ${short(root[0])} has no parent`);
|
||||
|
||||
const list = git(repo, ['rev-list', `${full}..HEAD`]);
|
||||
if (list.status !== 0) return gitFailed('rev-list', list);
|
||||
for (const c of list.stdout.split('\n').filter(Boolean)) {
|
||||
for (const [c] of lines) {
|
||||
// `--no-relative` and `--ignore-submodules=none`: neither `diff.relative`
|
||||
// nor an `ignore =` setting (config or .gitmodules) may hide a changed path.
|
||||
const names = git(repo, ['diff', '--no-relative', '--ignore-submodules=none', '--name-only', '-z', '--no-renames', `${c}^1`, c]);
|
||||
|
|
|
|||
|
|
@ -116,15 +116,18 @@ const GATES_USAGE = 'usage: --gates takes true, false, closed or open';
|
|||
|
||||
function parseArgs(argv) {
|
||||
const out = {};
|
||||
const setGates = (v) => {
|
||||
const g = gatesValue(v);
|
||||
if (g === undefined) out.error = GATES_USAGE;
|
||||
else out.gates = g;
|
||||
};
|
||||
for (let i = 0; i < argv.length; i++) {
|
||||
const a = argv[i];
|
||||
if (a === '--state') out.state = argv[++i];
|
||||
else if (a === '--event') out.event = argv[++i];
|
||||
else if (a.startsWith('--gates=')) {
|
||||
// The one-token form, read as `--gates <value>`; an empty value is no value.
|
||||
const g = gatesValue(a.slice('--gates='.length));
|
||||
if (g === undefined) out.error = GATES_USAGE;
|
||||
else out.gates = g;
|
||||
setGates(a.slice('--gates='.length));
|
||||
} else if (a === '--gates') {
|
||||
// Bare `--gates` (last, or followed by another flag) means on and
|
||||
// consumes nothing. A value is case-sensitive: true|closed = on,
|
||||
|
|
@ -133,9 +136,7 @@ function parseArgs(argv) {
|
|||
if (v === undefined || v.startsWith('--')) {
|
||||
out.gates = true;
|
||||
} else {
|
||||
const g = gatesValue(v);
|
||||
if (g === undefined) out.error = GATES_USAGE;
|
||||
else out.gates = g;
|
||||
setGates(v);
|
||||
i++;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -47,11 +47,11 @@ export function newFilesInDiff(diffText) {
|
|||
const paths = [];
|
||||
const unread = [];
|
||||
let header = null;
|
||||
let headers = 0;
|
||||
let sawHeader = false;
|
||||
const lines = String(diffText).split('\n').map((l) => l.replace(ANSI_RE, ''));
|
||||
for (const line of lines) {
|
||||
if (line.startsWith('diff --git ')) {
|
||||
headers++;
|
||||
sawHeader = true;
|
||||
header = line;
|
||||
continue;
|
||||
}
|
||||
|
|
@ -64,7 +64,7 @@ export function newFilesInDiff(diffText) {
|
|||
}
|
||||
}
|
||||
const first = lines.find((l) => l.trim() !== '');
|
||||
if (headers === 0 && first !== undefined) unread.push(`no diff --git header in a non-empty diff (first line: ${first.slice(0, 200)})`);
|
||||
if (!sawHeader && first !== undefined) unread.push(`no diff --git header in a non-empty diff (first line: ${first.slice(0, 200)})`);
|
||||
return { paths, unread };
|
||||
}
|
||||
|
||||
|
|
@ -118,14 +118,9 @@ export function parkAttemptFiles({ diffText, step, attempt, cwd = process.cwd()
|
|||
continue;
|
||||
}
|
||||
const posix = rel.split('\\').join('/');
|
||||
let intentToAdd = false;
|
||||
if (trackedPaths.has(posix)) {
|
||||
if (inHead(root, posix)) {
|
||||
kept.push({ path, reason: 'tracked since the attempt: not the executor\'s to move', blocking: false });
|
||||
continue;
|
||||
}
|
||||
// In the index but not in HEAD: registered --intent-to-add by the attempt.
|
||||
intentToAdd = true;
|
||||
if (trackedPaths.has(posix) && inHead(root, posix)) {
|
||||
kept.push({ path, reason: 'tracked since the attempt: not the executor\'s to move', blocking: false });
|
||||
continue;
|
||||
}
|
||||
const target = join(to, rel);
|
||||
if (lexists(target)) {
|
||||
|
|
@ -133,7 +128,8 @@ export function parkAttemptFiles({ diffText, step, attempt, cwd = process.cwd()
|
|||
continue;
|
||||
}
|
||||
moves.push({ path, abs, rel, target });
|
||||
if (intentToAdd) unindex.push(posix);
|
||||
// In the index but not in HEAD: registered --intent-to-add by the attempt.
|
||||
if (trackedPaths.has(posix)) unindex.push(posix);
|
||||
}
|
||||
// Out of the index first, all in ONE call before anything moves: git
|
||||
// refuses the whole call when one path cannot be removed, so a failure
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue