fix(916): the cross-family review step follows the lenses instead of running beside them, and fix commits are a sha range
One .NET slot's review round ran two worktree-isolated reviewers at once — the correctness lens and, on a rubric change, the Codex fallback — and took swap from 6.8 GB to 10.8 GB in three minutes on the 16 GB host; three slots reached load 82. review() now awaits the lenses, then the Codex runner, then its fallback. The finisher's fix attribution is the sha range the fixer's report head advances (head_sha is required on every report), replacing a line-set difference over free text that listed all eleven #563 commits as fixes. The harness gains a case that records how many lenses were still in flight when the cross-family agents started (must be zero); moving the fallback back into the parallel batch reddens it in both scripts. The mechanics page and the standing prompt state the measured cap: one .NET-building slot at a time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
This commit is contained in:
@@ -38,16 +38,30 @@ const finding = (severity) => ({ severity, file: 'f', summary: `${severity} find
|
||||
// Every stub lens returns the same findings, so a finding counts once per lens (two lenses → 2).
|
||||
// reviewRounds: array indexed by round-1 of {findings, lensesNull?}; landOverrides: extra fields on the finisher report;
|
||||
// postRebase: findings returned by the review after a patch-changing rebase, or 'lensesNull'; fixerNull: the fixer agent dies.
|
||||
function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerCommits = null }) {
|
||||
function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null, fixerNull = false, fixerDone = true, fixerNoCommit = false }) {
|
||||
const calls = [];
|
||||
let round = 0;
|
||||
let fixCount = 0;
|
||||
let lensesInFlight = 0;
|
||||
const xfamilyStartedWithLensesInFlight = [];
|
||||
const agent = async (prompt, opts) => {
|
||||
const label = (opts && opts.label) || '';
|
||||
calls.push({ label, prompt, opts });
|
||||
if (label.startsWith('impl:') || label.startsWith('fix:#')) return { done: true, summary: '', verified: '', left: '', commits: IMPL_COMMITS };
|
||||
if (label.startsWith('impl:') || label.startsWith('fix:#')) return { done: true, summary: '', verified: '', left: '', commits: IMPL_COMMITS, head_sha: 'aaaaaaa' };
|
||||
if (label.startsWith('review:codex:')) {
|
||||
xfamilyStartedWithLensesInFlight.push(lensesInFlight);
|
||||
return { verdict: 'merge', findings: [], ran: false };
|
||||
}
|
||||
if (label.startsWith('review:fallback:')) {
|
||||
xfamilyStartedWithLensesInFlight.push(lensesInFlight);
|
||||
const spec = reviewRounds[round - 1] || { findings: [] };
|
||||
return { verdict: 'merge', findings: spec.findings };
|
||||
}
|
||||
if (label.startsWith('review:')) {
|
||||
const spec = reviewRounds[round - 1] || { findings: [] };
|
||||
lensesInFlight += 1;
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
lensesInFlight -= 1;
|
||||
if (spec.lensesNull) return null;
|
||||
return { verdict: 'merge', findings: spec.findings };
|
||||
}
|
||||
@@ -55,7 +69,7 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null
|
||||
fixCount += 1;
|
||||
if (fixerNull) return null;
|
||||
if (!fixerDone) return { done: false, summary: 'stopped', verified: '', left: 'blocked', commits: IMPL_COMMITS };
|
||||
return { done: true, summary: '', verified: '', left: '', commits: fixerCommits !== null ? fixerCommits : `f${fixCount}xxxxx fix round ${fixCount}\n${IMPL_COMMITS}` };
|
||||
return { done: true, summary: '', verified: '', left: '', commits: fixerNoCommit ? IMPL_COMMITS : `f${fixCount}xxxxx fix round ${fixCount}\n${IMPL_COMMITS}`, head_sha: fixerNoCommit ? 'aaaaaaa' : `f${fixCount}xxxxx` };
|
||||
}
|
||||
if (label.startsWith('land:')) return { done: true, summary: '', verified: '', left: '', commits: '', pr_url: 'http://pr/1', head_sha: 'deadbeef', patch_changed: false, ...landOverrides };
|
||||
throw new Error(`unexpected agent label ${label}`);
|
||||
@@ -67,7 +81,7 @@ function run(script, args, { reviewRounds, landOverrides = {}, postRebase = null
|
||||
}
|
||||
return Promise.all(thunks.map((t) => t().catch(() => null)));
|
||||
};
|
||||
return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount }));
|
||||
return script(args, agent, parallel, () => {}, () => {}).then((result) => ({ result, calls, fixCount, xfamilyStartedWithLensesInFlight }));
|
||||
}
|
||||
|
||||
const cases = [
|
||||
@@ -94,10 +108,10 @@ describe.each(cases)('%s review loop', (name, args) => {
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(fixCount).toBe(1);
|
||||
const hist = finisherPrompt(calls);
|
||||
expect(hist).toContain('round 1: 2 lens(es); 0 blocking, 2 should-fix, 0 nit; answered by fix commit(s): f1xxxxx fix round 1');
|
||||
expect(hist).toContain('round 1: 2 lens(es); 0 blocking, 2 should-fix, 0 nit; answered by the fix commit(s) in git log --oneline aaaaaaa..f1xxxxx');
|
||||
expect(hist).toContain('round 2: 2 lens(es); 0 blocking, 0 should-fix, 0 nit; clean — loop ended');
|
||||
expect(hist).not.toContain(IMPL_COMMITS);
|
||||
expect(hist).toContain('what each of these fix commits changed');
|
||||
expect(hist).toContain('for exactly the commits git log --oneline lists in these ranges: aaaaaaa..f1xxxxx');
|
||||
});
|
||||
|
||||
it('a blocking finding that survives two fix rounds stops before the push', async () => {
|
||||
@@ -147,7 +161,7 @@ describe.each(cases)('%s review loop', (name, args) => {
|
||||
});
|
||||
|
||||
it('findings answered without a new commit are described as such, never as a clean round one', async () => {
|
||||
const { calls } = await run(script, args, { reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [] }], fixerCommits: IMPL_COMMITS });
|
||||
const { calls } = await run(script, args, { reviewRounds: [{ findings: [finding('should-fix')] }, { findings: [] }], fixerNoCommit: true });
|
||||
const p = finisherPrompt(calls);
|
||||
expect(p).toContain('answered without a new commit');
|
||||
expect(p).toContain('every finding was answered without a new commit');
|
||||
@@ -159,6 +173,16 @@ describe.each(cases)('%s review loop', (name, args) => {
|
||||
expect(result.error).toMatch(/^the post-rebase review round produced no reviews/);
|
||||
});
|
||||
|
||||
it('on a rubric-class change the cross-family step starts only after both lenses have returned', async () => {
|
||||
const rubricArgs = { ...args, risk: 'rubric' };
|
||||
const { result, calls, xfamilyStartedWithLensesInFlight } = await run(script, rubricArgs, { reviewRounds: [{ findings: [] }] });
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(calls.filter((c) => c.label.startsWith('review:codex:')).length).toBe(1);
|
||||
expect(calls.filter((c) => c.label.startsWith('review:fallback:')).length).toBe(1);
|
||||
expect(xfamilyStartedWithLensesInFlight).toEqual([0, 0]);
|
||||
expect(finisherPrompt(calls)).toContain('substituted a cold same-family review-only agent');
|
||||
});
|
||||
|
||||
it('a should-fix on the pushed head after a patch-changing rebase is an error', async () => {
|
||||
const { result } = await run(script, args, {
|
||||
reviewRounds: [{ findings: [] }],
|
||||
|
||||
Reference in New Issue
Block a user