diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 8d0e00551..11495c06f 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -24,7 +24,7 @@ import * as path from 'node:path'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { BRIEF_ROLES, BriefError, briefFile, checkReviewedTree, checkoutRoot, checkoutTree, promptHash, renderBrief, sectionTexts, sliceDiff, sliceNote, withWorkspace, type BriefRole, type Slots, type TreeCheck } from '../lib/review-brief'; import { cleanMarkers, collect, fixableRaises, mayBeUnavailable, parseFindings, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger'; -import { auditModel, buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; +import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, withAuditModel, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; import type { Dispatch } from '../lib/review-depth'; import { runScriptSync } from '../lib/script-invocation'; import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedRead } from '../lib/review-targeted'; @@ -321,11 +321,9 @@ function pass(): number { if (args.includes('--audit-only')) { const extrasFile = flag('--extras'); if (!extrasFile) { process.stderr.write('gstack-review-dispatch pass --audit-only: needs --extras (the audit readers, e.g. Step 14.5\'s docs reader)\n'); return EXIT.usage; } - const given = JSON.parse(readFile(extrasFile, 'extras')!) as Extra[]; - const audit = given.some(e => !e.model) ? auditModel() : null; - const extras = given.map(e => (e.model || !audit?.model ? e : { ...e, model: audit.model })); + const { extras, lines } = withAuditModel(JSON.parse(readFile(extrasFile, 'extras')!) as Extra[]); const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase: mergeBaseWith(base) ?? 'HEAD', root: git(['rev-parse', '--show-toplevel'])?.trim() ?? process.cwd(), - checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)', ...(audit ? [audit.line] : [])], extras, voices: args.includes('--voices') }); + checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: [], next: 'audit', planLines: ['PASS: audit-only (no reviewer plan; its extras run as one batch)', ...lines], extras, voices: args.includes('--voices') }); fs.mkdirSync(m.out, { recursive: true, mode: 0o700 }); const t = sectionTexts(ROOT); renderStage(m, 'first', t, branchDiff); @@ -351,7 +349,8 @@ function pass(): number { if (!diffBase) throw new BriefError(`no merge base between ${base} and HEAD`); const root = git(['rev-parse', '--show-toplevel'])?.trim() ?? process.cwd(); const extrasFile = flag('--extras'); - const extras = extrasFile ? JSON.parse(readFile(extrasFile, 'extras')!) as Extra[] : []; + const { extras, lines } = withAuditModel(extrasFile ? JSON.parse(readFile(extrasFile, 'extras')!) as Extra[] : []); + planLines.push(...lines); const m = buildManifest({ pass: id, out, ledger: flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'), base, diffBase, root, checklistRoot: flag('--checklist-root') ?? ROOT, dispatches: step === 'fix-check' ? p.dispatches : p.dispatches.filter(d => d.role !== 'fix-check'), next: step, planLines, learnings: readFile(flag('--learnings-file'), 'learnings') ?? null, extras, targetedClasses: p.targeted?.classes, mergeBase, rangeBase: flag('--diff-base') ? undefined : rangeBase, diff --git a/capy/skill/gstack/SKILL.md b/capy/skill/gstack/SKILL.md index 43d896f0d..a96d43bc2 100644 --- a/capy/skill/gstack/SKILL.md +++ b/capy/skill/gstack/SKILL.md @@ -46,7 +46,7 @@ Capy checklist for `/autoplan`: drive `gstack-autoplan`, never a hand adaptation Token caps on /ship: pass a brief's cap as `--token-cap `. /ship reads `cost.tokens` and `cost.credits` from `tools.thread.get({ thread_id: "" })` (its own jam; a subagent reading root `"this"` starts `--subagent --reading-from this --meter-scope tree`) inside the same execute program as the `gstack-ship-budget` command that takes them as `--reading --credits `, never alone. `ship/DRIVER.md` starts `--mode observe`: each program ends with `note`, and only a note past the cap stops (`docs/ship-budget.md`). -/ship on Capy is about a dozen calls, each an execute program (`ship/DRIVER.md`). Each review pass and Step 11 is one call to the checked-in pass program (fill only `input`): `const t0 = await now(); const p = (await tools.capy.bash({ command: ". '' && gstack-review-dispatch program", description: "Pass" })).output.split("\n"); return p[0] + "\n" + (await new (Object.getPrototypeOf(async () => {}).constructor)("tools", "sleep", "now", "input", JSON.parse(p[1]))(tools, sleep, now, { startedAt: t0, launch: "", cwd: "", base: "", out: "/p", pass: "-p", laneModel: "", planFlags: [], logStart: ["review"], jam: "", record: "", step: "9.2" }));` It plans, logs `logStart` (`reservation` mode from `RESERVATION_ID:`), sends each brief on `machine: "shared"`, then polls: one `tools.subagent.list` per ~20 s tick, returning 200 s after the program's first line, where a reader is settled only at status `done` or `failed` or with a `summary` (a just-created `idle` reader has not run); it starts readers under `rejected` with `tools.subagent.start` as slots free and stops on one stuck 20 minutes. It ends on `NEXT:` (`poll`: rerun) or `STOP:`. `models` maps roles to models (`outside-voice`: another family, pinned in `voice-models.json`); Step 11 swaps `logStart` for `stage: "voices"`; a full repeat pass, `diffBase: ""`. Steps 7, 8 and 14.5 (`step: "14.5"`) use `extras`, JSON `{name, prompt_file, first, model}` (prompts verbatim from their sections, Step 7 `first`); they cannot block, so `model: { model_id: "anthropic/claude-sonnet-5-5", reasoning_mode: "low" }` (not 14.5). Waits: never let 5 minutes pass between calls. While readers run, never end the turn on `wait`; rerun the loader. Else re-issue `wait` with `duration_seconds: 200` and operation ids. At Step 22 report the `CALLS:` line of `gstack-ship-budget calls --events ` on your transcript since /ship began. A small waiting child saves nothing. +/ship on Capy is about a dozen calls, each an execute program (`ship/DRIVER.md`). Each review pass and Step 11 is one call to the checked-in pass program (fill only `input`): `const t0 = await now(); const p = (await tools.capy.bash({ command: ". '' && gstack-review-dispatch program", description: "Pass" })).output.split("\n"); return p[0] + "\n" + (await new (Object.getPrototypeOf(async () => {}).constructor)("tools", "sleep", "now", "input", JSON.parse(p[1]))(tools, sleep, now, { startedAt: t0, launch: "", cwd: "", base: "", out: "/p", pass: "-p", laneModel: "", planFlags: [], logStart: ["review"], jam: "", record: "", step: "9.2" }));` It plans, logs `logStart` (`reservation` mode from `RESERVATION_ID:`), sends each brief on `machine: "shared"`, then polls: one `tools.subagent.list` per ~20 s tick, returning 200 s after the program's first line, where a reader is settled only at status `done` or `failed` or with a `summary` (a just-created `idle` reader has not run); it starts readers under `rejected` with `tools.subagent.start` as slots free and stops on one stuck 20 minutes. It ends on `NEXT:` (`poll`: rerun) or `STOP:`. `models` maps roles to models (`outside-voice`: another family, pinned in `voice-models.json`); Step 11 swaps `logStart` for `stage: "voices"`; a full repeat pass, `diffBase: ""`. Steps 7, 8 and 14.5 (`step: "14.5"`) use `extras`, JSON `{name, prompt_file, first, model}` (prompts verbatim from their sections, Step 7 `first`); they cannot block, so omit `model`: the pass prints `AUDIT_MODEL:`. Waits: never let 5 minutes pass between calls. While readers run, never end the turn on `wait`; rerun the loader. Else re-issue `wait` with `duration_seconds: 200` and operation ids. At Step 22 report the `CALLS:` line of `gstack-ship-budget calls --events ` on your transcript since /ship began. A small waiting child saves nothing. ## Plan, approve, ship diff --git a/changelog.d/capy-gst-1449-audit-readers-agent-model.md b/changelog.d/capy-gst-1449-audit-readers-agent-model.md new file mode 100644 index 000000000..c83088471 --- /dev/null +++ b/changelog.d/capy-gst-1449-audit-readers-agent-model.md @@ -0,0 +1 @@ +- **/ship's coverage and plan audits run on your model preference too.** Steps 7 and 8 were still told to run on Sonnet 5.5 at low effort after the Step 14.5 docs audit moved to your `agent_model`. A reviewer pass now gives any audit reader with no model your `agent_model` at low effort and prints the pick as `AUDIT_MODEL:`, the same way `--audit-only` does. With no preference they stay on Sonnet 5.5 at low effort, a model named in the extras still wins, and the Step 11 outside voice keeps its other-family model. (GST-1449) diff --git a/docs/model-policy.md b/docs/model-policy.md index 8405d6c5f..fd967a474 100644 --- a/docs/model-policy.md +++ b/docs/model-policy.md @@ -148,10 +148,10 @@ Two cases ignore the preference on purpose: a cross-family outside voice (the `plan-review` role picks the other provider) and a model under test in an eval (`eval-arms` and `/eval-plan` arms). -/ship's Step 14.5 docs audit runs on the preference at low effort when its -extras name no model; unset or unusable, it runs on `anthropic/claude-sonnet-5-5` -at low effort, and `host` runs it on the lane's own model. The pass prints the -pick as `AUDIT_MODEL:`. +/ship's audit readers (Steps 7, 8 and the Step 14.5 docs audit) run on the +preference at low effort when their extras name no model; unset or unusable, +they run on `anthropic/claude-sonnet-5-5` at low effort, and `host` runs them on +the lane's own model. The pass prints the pick as `AUDIT_MODEL:`. A value gstack can't use stops only the agent role, naming the file and the fix. `plan-review`, `implementation` and `eval-arms` keep resolving, and diff --git a/lib/review-pass.ts b/lib/review-pass.ts index 8a9f19588..5c233d637 100644 --- a/lib/review-pass.ts +++ b/lib/review-pass.ts @@ -71,13 +71,13 @@ export interface Manifest { } export interface FixCheckFiles { fix_list: string; hunks: string; tests: string; callers?: string } export interface Extra { name: string; prompt_file: string; first?: boolean; model?: PassModel } -/** Step 14.5's docs audit model when no preference is set (its pre-GST-1439 hard-code). */ -export const DOCS_AUDIT_DEFAULT: PassModel = { model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }; +/** The audit readers' model when no preference is set (their pre-GST-1439 hard-code). */ +export const AUDIT_DEFAULT: PassModel = { model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }; -/** The model an audit-only extra runs on when it names none (GST-1439): the owner's agent_model at low effort, - * DOCS_AUDIT_DEFAULT with no preference or an unusable one; agent_model host leaves it to the lane's own model. */ +/** The model an audit extra (Steps 7, 8, 14.5) runs on when it names none (GST-1439, GST-1449): the owner's agent_model at low effort, + * AUDIT_DEFAULT with no preference or an unusable one; agent_model host leaves it to the lane's own model. */ export function auditModel(env: NodeJS.ProcessEnv = process.env): { model?: PassModel; line: string } { - const fallback = (why: string) => ({ model: DOCS_AUDIT_DEFAULT, line: `AUDIT_MODEL: ${DOCS_AUDIT_DEFAULT.model_id} reasoning=low (default; ${why})` }); + const fallback = (why: string) => ({ model: AUDIT_DEFAULT, line: `AUDIT_MODEL: ${AUDIT_DEFAULT.model_id} reasoning=low (default; ${why})` }); let s; try { s = resolveAgentModel({ env }); } catch (error) { if (!(error instanceof AgentModelError)) throw error; @@ -87,6 +87,12 @@ export function auditModel(env: NodeJS.ProcessEnv = process.env): { model?: Pass if (!s.id) return { line: `AUDIT_MODEL: host (agent_model host in ${s.source.path}: the lane's own model)` }; return { model: { model_id: s.id, reasoning_mode: 'low' }, line: `AUDIT_MODEL: ${s.id} reasoning=low (agent_model ${s.value} in ${s.source.path})` }; } + +/** Fills auditModel() into the extras that name no model, and the AUDIT_MODEL: line for the plan when any did. */ +export function withAuditModel(given: Extra[], env: NodeJS.ProcessEnv = process.env): { extras: Extra[]; lines: string[] } { + const audit = given.some(e => !e.model) ? auditModel(env) : null; + return { extras: given.map(e => (e.model || !audit?.model ? e : { ...e, model: audit.model })), lines: audit ? [audit.line] : [] }; +} /** Step 9.2.1's own first line: its probes are the parent's, so a QA subagent only refuses (C4-36, C5-36: "parent-only"). */ export const PARENT_ONLY_QA = 'Only the parent runs report-only discovery.'; const qaRefusal = (name: string) => new BriefError(`Step 9.2.1 is the parent's report-only discovery (ship/sections/review-army.md: "${PARENT_ONLY_QA}"), never an audit reader: ${name}`); diff --git a/ship/DRIVER.md b/ship/DRIVER.md index ed1001571..896a2f4c1 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -23,8 +23,8 @@ version, CHANGELOG, commits), **Documentation** (Step 14.5) and **Next steps** ( **Reviewers are the sections' own.** Every reviewer brief, its severity and confidence rules and gates come from its section verbatim; this file never shortens a reviewer's brief. A blocking reader's prompt is its `gstack-review-dispatch render` -file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, -14.5 once on `AUDIT_MODEL:`; findings go to the PR body. +file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort on `AUDIT_MODEL:`, +14.5 once; findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's section read or fix-check, plus a keep-alive per 200 s waited. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index c50dd0aa4..40d0edc72 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -21,8 +21,8 @@ version, CHANGELOG, commits), **Documentation** (Step 14.5) and **Next steps** ( **Reviewers are the sections' own.** Every reviewer brief, its severity and confidence rules and gates come from its section verbatim; this file never shortens a reviewer's brief. A blocking reader's prompt is its `gstack-review-dispatch render` -file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, -14.5 once on `AUDIT_MODEL:`; findings go to the PR body. +file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` refuses a drop or downgrade. Audit readers (Steps 7, 8, 14.5) cannot block: low effort on `AUDIT_MODEL:`, +14.5 once; findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's section read or fix-check, plus a keep-alive per 200 s waited. diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 218d1ffcd..bc6250369 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -741,6 +741,33 @@ describe('Capy pass program (fake host, real bins)', () => { expect(bad.stdout).toContain('ignored)'); }, 240_000); + // Value: protects=Steps 7 and 8's audit readers ride the reviewer pass on the owner's agent_model at low effort, Sonnet 5.5 low with no preference, and a given model wins; fails_when=the reviewer pass leaves an audit extra with no model, so it inherits the lane's model and effort, or the Capy skill's Sonnet hard-code comes back (GST-1449); + // why_new=only the audit-only pass (Step 14.5) resolved AUDIT_MODEL; seam=gstack-review-dispatch pass with --extras; no_claim=reviewers and Step 11's voices, which keep their own models + test('Steps 7 and 8 audit extras on a reviewer pass default to the owner agent_model at low effort; a given model wins', () => { + const { dir, env } = repo(); + const prompt = path.join(dir, '.state/audit.prompt.md'); + fs.mkdirSync(path.dirname(prompt), { recursive: true }); + fs.writeFileSync(prompt, 'Run the audit as written.\n'); + const prefs = path.join(dir, '.state/preferences.yaml'); + fs.writeFileSync(prefs, 'agent_model: opus\n'); + const given = { model_id: 'anthropic/claude-haiku-5-5', reasoning_mode: 'low' }; + const run = (id: string, e: NodeJS.ProcessEnv) => { + const extras = path.join(dir, `.state/${id}.json`); + fs.writeFileSync(extras, JSON.stringify([{ name: 'coverage', prompt_file: prompt, first: true }, { name: 'plan', prompt_file: prompt }, { name: 'given', prompt_file: prompt, model: given }])); + const out = path.join(dir, '.state/run', id); + const r = bin(dir, e, ['pass', '--base', 'main', '--out', out, '--pass', id, '--extras', extras, '--', '--depth-floor', 'full']); + expect(r.status, r.stdout + r.stderr).toBe(0); + const readers = JSON.parse(fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8')).readers as { reader: string; model?: object }[]; + return { stdout: r.stdout, model: (n: string) => readers.find(x => x.reader === `audit/${n}`)!.model }; + }; + const opus = run('a-opus', { ...env, GSTACK_PREFERENCES_FILE: prefs }); + expect([opus.model('coverage'), opus.model('plan')]).toEqual([{ model_id: 'anthropic/claude-opus-5-5', reasoning_mode: 'low' }, { model_id: 'anthropic/claude-opus-5-5', reasoning_mode: 'low' }]); + expect(opus.model('given')).toEqual(given); + expect(opus.stdout).toContain('AUDIT_MODEL: anthropic/claude-opus-5-5 reasoning=low (agent_model'); + const { GSTACK_PREFERENCES_FILE: _, ...noFile } = env; + expect(run('a-none', noFile).model('plan')).toEqual({ model_id: 'anthropic/claude-sonnet-5-5', reasoning_mode: 'low' }); + }, 240_000); + // Value: protects=a pass id typed in capitals is read as its lowercase id, so the manifest, its titles and a resume agree, and an id with other characters is still refused naming the rule; fails_when=--pass P1 is refused as a missing flag (gstack-evals#191 /ship), a capitalized resume of p1 reads as another pass, or p_1 is accepted; // why_new=the id was matched case-sensitively and its refusal read as a missing flag; seam=gstack-review-dispatch pass; no_claim=the reviewer plan, which --audit-only skips test('pass reads its id as lowercase; an id with other characters is still refused with the rule', () => { diff --git a/test/ship-driver.test.ts b/test/ship-driver.test.ts index 2f74da5f6..3b7540271 100644 --- a/test/ship-driver.test.ts +++ b/test/ship-driver.test.ts @@ -219,14 +219,14 @@ describe('variant G: the call plan, observe budget, waits and audit readers (GST expect(flat(read('ship/sections/budget-handoff.md.tmpl'))).toContain('the wait until the last settles (`done`, `failed` or a final summary; a fresh `idle` has not run)'); }); - // Value: protects=audits that cannot block (Steps 7, 8, 14.5) run at low effort (7 and 8 on a small model, 14.5 on AUDIT_MODEL, GST-1439) with their sections' own briefs, and skip their reader when --audit decides them; fails_when=an audit reader inherits the lane's model and effort (#36: the coverage audit alone was 2.45M at Opus), or an audit brief is paraphrased; + // Value: protects=audits that cannot block (Steps 7, 8, 14.5) run at low effort on AUDIT_MODEL (GST-1439, GST-1449) with their sections' own briefs, and skip their reader when --audit decides them; fails_when=an audit reader inherits the lane's model and effort (#36: the coverage audit alone was 2.45M at Opus), the Capy skill hard-codes a model for Steps 7 and 8 again, or an audit brief is paraphrased; // why_new=audit model routing is new; seam=none; no_claim=the audits' recall - test('audit readers run on a small model at low effort, named for Capy, and Phase 3 dispatches only what READER names', () => { - expect(flat(doc)).toContain('Audit readers (Steps 7, 8, 14.5) cannot block: low effort, 7 and 8 on a small model, 14.5 once on `AUDIT_MODEL:`; findings go to the PR body'); + test('audit readers run on AUDIT_MODEL at low effort, named for Capy, and Phase 3 dispatches only what READER names', () => { + expect(flat(doc)).toContain('Audit readers (Steps 7, 8, 14.5) cannot block: low effort on `AUDIT_MODEL:`, 14.5 once; findings go to the PR body'); expect(flat(doc)).toContain('`Coverage:` and Step 8\'s line come from `--audit` when no reader ran'); const capy = read('capy/skill/gstack/SKILL.md'); - expect(capy).toContain('model: { model_id: "anthropic/claude-sonnet-5-5", reasoning_mode: "low" }'); - expect(capy).toContain('reasoning_mode: "low" }` (not 14.5)'); + expect(capy).toContain('they cannot block, so omit `model`: the pass prints `AUDIT_MODEL:`.'); + expect(capy).not.toContain('claude-sonnet-5-5'); }); // Value: protects=Step 9.2.1's report-only probes run in the parent's own Phase 4 program, as the section says, with its Reads in Phase 3, and