From 4dc0880e81435d0a53eb792005dc20fb37bcd757 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 15:39:30 +0000 Subject: [PATCH] fix: Step 11 review fixes (resume on dispatched voice models, case-insensitive family, every outside voice checked) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/capy-review-pass.js | 54 ++++++++++++++++++++++++++-------------- test/review-pass.test.ts | 29 ++++++++++++++++++++- 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/lib/capy-review-pass.js b/lib/capy-review-pass.js index b5b886b2f..c18769604 100644 --- a/lib/capy-review-pass.js +++ b/lib/capy-review-pass.js @@ -74,36 +74,45 @@ const briefText = async (r) => { }; const roleArgs = (r) => ' --role ' + q(r.role) + (r.name ? ' --name ' + q(r.name) : ''); const REASONING = { medium: 'medium', light: 'low' }; -const family = (id) => (typeof id === 'string' && id.includes('/') ? id.slice(0, id.indexOf('/')) : ''); +const family = (id) => (typeof id === 'string' && id.includes('/') ? id.slice(0, id.indexOf('/')).toLowerCase() : ''); const CERTIFIED_OUTSIDE = { model_id: 'openai/gpt-6.1-sol', reasoning_mode: 'medium' }; -const laneFamily = family(input.laneModel); -const modelOf = (r) => r.model || (input.models || {})[r.role === 'specialist-batched' ? 'specialist' : r.role] +const voiceFile = input.out.replace(/\/+$/, '') + '/voice-models.json'; +let voices = { laneModel: null, readers: {} }; +let laneFamily = family(input.laneModel); +const modelOf = (r) => r.model || (r.reader in voices.readers ? voices.readers[r.reader] || undefined + : (input.models || {})[r.role === 'specialist-batched' ? 'specialist' : r.role] || (r.role === 'outside-voice' && laneFamily === 'anthropic' ? CERTIFIED_OUTSIDE : undefined) - || (REASONING[r.effort] && input.laneModel ? { model_id: input.laneModel, reasoning_mode: REASONING[r.effort] } : undefined); -const outsideNote = (r) => { + || (REASONING[r.effort] && input.laneModel ? { model_id: input.laneModel, reasoning_mode: REASONING[r.effort] } : undefined)); +const outsideNote = (r, source) => { const ov = modelOf(r); const certified = laneFamily === 'anthropic' && ov.model_id === CERTIFIED_OUTSIDE.model_id && ov.reasoning_mode === CERTIFIED_OUTSIDE.reasoning_mode; - return 'OUTSIDE_VOICE: ' + ov.model_id + ' reasoning=' + (ov.reasoning_mode || 'default') + ((input.models || {})['outside-voice'] ? ' given' : ' default for an anthropic lane') + return 'OUTSIDE_VOICE: ' + ov.model_id + ' reasoning=' + (ov.reasoning_mode || 'default') + ' ' + source + (certified ? ' (certified configuration)' : '; outside the certified configuration (' + CERTIFIED_OUTSIDE.model_id + ' at ' + CERTIFIED_OUTSIDE.reasoning_mode + ' beside an anthropic lane)'); }; const familyStop = (readers) => { - const ov = readers.find((x) => x.role === 'outside-voice' && !['created', 'collected', 'failed'].includes(x.state)); - if (!ov) return null; - const om = modelOf(ov); - if (!om) return "the outside voice needs input.models['outside-voice'] (a model of another family); none was given"; const native = readers.find((x) => x.role === 'adversarial-native'); const nativeFamily = family(native && modelOf(native) ? modelOf(native).model_id : undefined); - if (!laneFamily && !nativeFamily) return 'the outside voice ' + om.model_id + ' cannot be checked against another family: neither input.laneModel nor a native voice model was given'; - const f = family(om.model_id); - if (!f) return 'the outside voice ' + om.model_id + ' has no provider prefix, so its family cannot be checked'; - if (f !== laneFamily && f !== nativeFamily) return null; - return 'the outside voice ' + om.model_id + ' is the ' + f + ' family, the same as the ' + (f === nativeFamily ? 'native voice' : 'lane model') + "; Step 11 needs input.models['outside-voice'] from another family"; + for (const ov of readers.filter((x) => x.role === 'outside-voice' && !['created', 'collected', 'failed'].includes(x.state))) { + const om = modelOf(ov); + if (!om) return "the outside voice needs input.models['outside-voice'] (a model of another family); none was given"; + const lane = voices.laneModel || input.laneModel; + if (!laneFamily && !nativeFamily) return 'the outside voice ' + om.model_id + ' cannot be checked against another family: ' + (lane ? 'the lane model ' + lane + ' has no provider prefix and no native voice model was given' : 'neither input.laneModel nor a native voice model was given'); + const f = family(om.model_id); + if (!f) return 'the outside voice ' + om.model_id + ' has no provider prefix, so its family cannot be checked'; + if (f === laneFamily || f === nativeFamily) return 'the outside voice ' + om.model_id + ' is the ' + f + ' family, the same as the ' + (f === nativeFamily ? 'native voice' : 'lane model') + "; Step 11 needs input.models['outside-voice'] from another family"; + } + return null; }; const safe = (s) => s.replace(/[^a-zA-Z0-9._-]/g, '_'); const active = input.stage === 'voices' ? ['voices'] : ['first', 'specialists', 'red-team']; try { let m = await load(); + if (input.stage === 'voices') { + const v = await sh('cat ' + q(voiceFile) + ' 2>/dev/null || true'); + if (v.out.trim()) voices = JSON.parse(v.out); + if (voices.laneModel) laneFamily = family(voices.laneModel); + } if (input.stage === 'voices' && !m.readers.some((r) => r.stage === 'voices')) return finish(['VOICES: none (the pass plan lists no Step 11 voice)', 'NEXT: judge']); for (const s of input.logStart || []) { const key = (input.stage === 'voices' ? 'voices:' : 'pass:') + s; @@ -116,8 +125,6 @@ try { lines.push('REVIEW_LOG_START: ' + s + ' token=' + token); } for (;;) { - const sameFamily = input.stage === 'voices' && familyStop(m.readers); - if (sameFamily) return stop(sameFamily); const failed = m.readers.find((x) => active.includes(x.stage) && x.state === 'failed'); if (failed) return stop(failed.reader + ' (subagent ' + failed.path + ') ' + (failed.error || 'failed') + '; its review is incomplete, so the pass cannot certify coverage; restore it with a fresh pass under a new --pass and --out'); const list = await tools.subagent.list({}); @@ -129,12 +136,17 @@ try { await mark(r.reader, found[0].status === 'draft' ? 'queued' : 'created', { path: found[0].path }); } m = await load(); + const sameFamily = input.stage === 'voices' && familyStop(m.readers); + if (sameFamily) return stop(sameFamily); for (const r of m.readers.filter((x) => active.includes(x.stage) && x.state === 'queued')) { const t = byPath.get(r.path); if (t && t.status !== 'draft') { await mark(r.reader, 'created', {}); continue; } const model = modelOf(r); const s = await tools.subagent.start({ indices: [Number(r.path)], machine: 'shared', ...(model ? { model } : {}) }); - if ((s.started || []).length) await mark(r.reader, 'created', {}); + if ((s.started || []).length) { + if (r.role === 'outside-voice') lines.push(outsideNote(r, 'as dispatched, started from the queue')); + await mark(r.reader, 'created', {}); + } else if (r.state_at && (await now()) - Date.parse(r.state_at) > stuckMs) return stop(r.reader + ' (subagent ' + r.path + ') still queued after ' + Math.round(stuckMs / 60000) + ' minutes; free a subagent slot, then rerun'); } for (const r of m.readers.filter((x) => active.includes(x.stage) && x.state === 'created')) { @@ -186,7 +198,11 @@ try { const b = await reading(); if (b) { await sh(bin + ' mark --manifest ' + q(mf) + ' --before ' + b.tokens + ',' + b.credits); m.before = b; } } - if (r.role === 'outside-voice') lines.push(outsideNote(r)); + if (r.role === 'outside-voice') lines.push(outsideNote(r, r.model || (input.models || {})['outside-voice'] ? 'given' : 'default for an anthropic lane')); + if (r.stage === 'voices') { + voices = { laneModel: voices.laneModel || input.laneModel || null, readers: { ...voices.readers, [r.reader]: model || null } }; + await tools.capy.write({ path: voiceFile, content: JSON.stringify(voices) }); + } await mark(r.reader, 'intent', {}); const res = await tools.subagent.create({ tasks: [{ title: r.title, prompt: text }], start: { machine: 'shared', ...(model ? { model } : {}) } }); const idx = String(res.tasks[0].index); diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 3d10ef490..6f243db80 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -166,7 +166,7 @@ function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) if (mode === 'after') throw new Error('create result lost'); return { tasks: [{ index, title: t.title }], ...(rejected ? { rejected: [{ index, error: 'cap' }] } : {}) }; }, - start: async ({ indices }: { indices: number[] }) => { starts.push(indices); for (const i of indices) subs.get(String(i))!.status = 'working'; return { started: indices.map(index => ({ index })), rejected: [] }; }, + start: async ({ indices, model }: { indices: number[]; model?: unknown }) => { starts.push(indices); for (const i of indices) { const s = subs.get(String(i))!; s.status = 'working'; if (model) s.model = model; } return { started: indices.map(index => ({ index })), rejected: [] }; }, list: async () => { advance(); return { tasks: [...subs].map(([p, s]) => ({ path: p, title: s.title, status: s.status })), total: subs.size }; }, get: async ({ path: p }: { path: string }) => ({ path: p, status: subs.get(p)!.status, summary: subs.get(p)!.summary }), }, @@ -344,8 +344,31 @@ describe('Capy pass program (fake host, real bins)', () => { expect(await runPass(h2, base2)).toContain('NEXT: judge (Phase 4)'); expect(await runPass(h2, { ...base2, stage: 'voices', budgetMs: 1 })).toMatch(/DISPATCHED: outside-voice path=\d+ queued/); const starts = h2.starts.length; + const recorded = path.join(base2.out, 'voice-models.json'); + const kept = fs.readFileSync(recorded, 'utf8'); + fs.rmSync(recorded); expect(await runPass(h2, { ...base2, stage: 'voices', models: same })).toMatch(refused); expect(h2.starts.length).toBe(starts); + fs.writeFileSync(recorded, kept); + const resumed = await runPass(h2, { ...base2, stage: 'voices', models: same }); + expect(resumed).toContain('OUTSIDE_VOICE: openai/gpt-6.1-sol reasoning=medium as dispatched, started from the queue (certified configuration)'); + expect(h2.creates.find(c => c.title === 'ship x18p1 outside-voice')!.model).toEqual({ model_id: 'openai/gpt-6.1-sol', reasoning_mode: 'medium' }); + }, 300_000); + + // Value: protects=a queued outside voice resumes on the models recorded when the voices were dispatched, so changed rerun inputs cannot pair it with a native voice of its own family; fails_when=the rerun's input decides the queued voice's model and the native family (outside-voice finding on #3698: two OpenAI voices started); + // why_new=/ship Step 11 review of #3698; seam=none; no_claim=inputs that change between reruns on purpose + test('a queued outside voice resumes on its dispatched model, checked against the dispatched native family', async () => { + const { dir, env } = repo(); + const h = host(dir, env, { reject: t => t.endsWith('outside-voice') }); + const base = { base: 'main', out: path.join(dir, '.state/run/p1'), pass: 'x19p1', planFlags: ['--depth-floor', 'full'] }; + expect(await runPass(h, base)).toContain('NEXT: judge (Phase 4)'); + const first = { 'adversarial-native': { model_id: 'openai/gpt-6-astra', reasoning_mode: 'high' }, 'outside-voice': { model_id: 'google/gemini-4-argon', reasoning_mode: 'medium' } }; + expect(await runPass(h, { ...base, stage: 'voices', models: first, budgetMs: 1 })).toMatch(/DISPATCHED: outside-voice path=\d+ queued/); + const res = await runPass(h, { ...base, stage: 'voices' }); + expect(res).not.toContain('STOP:'); + expect(res).toContain('OUTSIDE_VOICE: google/gemini-4-argon reasoning=medium as dispatched, started from the queue; outside the certified configuration'); + expect(h.creates.find(c => c.title === 'ship x19p1 outside-voice')!.model).toEqual(first['outside-voice']); + expect(h.creates.find(c => c.title === 'ship x19p1 adversarial-native')!.model).toEqual(first['adversarial-native']); }, 300_000); // Value: protects=a lane whose family is unknown or not Anthropic gets no default outside voice, and an explicit one cannot skip the family check; fails_when=the pin applies to a lane it was not certified for, or an outside voice runs with nothing to compare its family to; @@ -362,6 +385,10 @@ describe('Capy pass program (fake host, real bins)', () => { .toContain('STOP: the outside voice openai/gpt-6.1-sol cannot be checked against another family: neither input.laneModel nor a native voice model was given'); expect(await runPass(h, { ...base, stage: 'voices', models: { 'outside-voice': { model_id: 'claude-sonnet-5-5' } } })) .toContain('STOP: the outside voice claude-sonnet-5-5 has no provider prefix, so its family cannot be checked'); + expect(await runPass(h, { ...base, stage: 'voices', models: { 'outside-voice': { model_id: 'Anthropic/claude-sonnet-5-5' } } })) + .toContain('STOP: the outside voice Anthropic/claude-sonnet-5-5 is the anthropic family, the same as the lane model'); + expect(await runPass(h, { ...base, stage: 'voices', laneModel: 'claude-opus-5-5', models: { 'outside-voice': { model_id: 'openai/gpt-6.1-sol' } } })) + .toContain('STOP: the outside voice openai/gpt-6.1-sol cannot be checked against another family: the lane model claude-opus-5-5 has no provider prefix and no native voice model was given'); expect(h.creates.some(c => /^ship x16p1 (adversarial-native|outside-voice)$/.test(c.title))).toBe(false); }, 240_000);