mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-07 17:28:23 +01:00
fix(ship): Steps 7 and 8 audits default to the owner's agent_model (GST-1449) (#4042)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
641d62291c
commit
37d55328b0
9 files changed
+58
-25
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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 <n>`. /ship reads `cost.tokens` and `cost.credits` from `tools.thread.get({ thread_id: "<your own jam 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 <n> --credits <c>`, 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: ". '<launch file>' && 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: "<launch file>", cwd: "<repo>", base: "<base>", out: "<run dir>/p<n>", pass: "<run>-p<n>", laneModel: "<your model id>", planFlags: [], logStart: ["review"], jam: "<jam id>", record: "<rec>", 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: "<RANGE_BASE>"`. 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 <file>` 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: ". '<launch file>' && 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: "<launch file>", cwd: "<repo>", base: "<base>", out: "<run dir>/p<n>", pass: "<run>-p<n>", laneModel: "<your model id>", planFlags: [], logStart: ["review"], jam: "<jam id>", record: "<rec>", 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: "<RANGE_BASE>"`. 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 <file>` on your transcript since /ship began. A small waiting child saves nothing.
|
||||
|
||||
## Plan, approve, ship
|
||||
|
||||
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
+11
-5
@@ -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}`);
|
||||
|
||||
+2
-2
@@ -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.
|
||||
|
||||
+2
-2
@@ -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.
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user