From e3998832271cce1571f6700b121184f880e5e21d Mon Sep 17 00:00:00 2001 From: Claude Code Date: Tue, 28 Jul 2026 21:46:04 +0000 Subject: [PATCH] feat(templater): set each agent's effort by how often it fires, not by role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spec-author` ran at high effort three times per feat and `quality-gate` at medium, and both were the wrong end of the same trade. spec-author authors a SCAFFOLDED artifact whose shape the template already fixes, three times per feat. The argument for high was that a shallow draft pushes the real authoring back onto the opus reviewer as fix-lists, defeating the cost split — but a measured feat put $3.15 of its $4.69 on sonnet, so the split was already lost at that end. Medium, with the fix-list count as the signal to move it back. quality-gate runs the declared commands and relays their output; its own prompt says it decides nothing about the code. Low. The two reviewers stay at high deliberately. They fire once or twice per feat, so lowering them saves almost nothing, and they are pure judgment: the same measured feat arrived "implemented" with 691 green tests and a drawer that never collapsed at any width — the review is what found it. The validate/fix loop is bounded at two rounds. "Re-validate until it returns 0" is an open loop, and the finding it cannot resolve on the second pass is the one it will not resolve on the fourth either, having re-read an 800-line spec each time to retry a fix it already tried. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ARBSaXgJ7ssW1hvpy6r64y --- internal/templater/templater_test.go | 73 +++++++++++++++++-- .../templates/agents/quality-gate.md.tmpl | 2 +- .../templates/agents/spec-author.md.tmpl | 19 +++-- 3 files changed, 80 insertions(+), 14 deletions(-) diff --git a/internal/templater/templater_test.go b/internal/templater/templater_test.go index fd0a00c..4711409 100644 --- a/internal/templater/templater_test.go +++ b/internal/templater/templater_test.go @@ -301,16 +301,21 @@ func TestSpecAuthorAgentShipped(t *testing.T) { } // Least-privilege tools: it writes spec artifacts, so Edit/Write/Bash are in, // but it must not approve or implement. - // effort is high (not medium): spec authoring is the SDD contract — EARS, - // the design boundary map, traceability — a judgment task, and the repo runs - // every judgment sub-agent at high (code-reviewer, implementer, - // security-reviewer). A shallow first draft pushes the real authoring back - // onto the opus reviewer via fix-lists, defeating the cost split. + // + // effort is medium, and it used to be high. The argument for high was that + // authoring is judgment — EARS, the boundary map, traceability — and a shallow + // draft pushes the real work back onto the opus reviewer as fix-lists, which + // defeats the cost split. What a measured run showed is that the split was + // already lost at the other end: of one feat's $4.69, $3.15 was sonnet, and + // this agent is dispatched three times per feat (requirements, design, tasks) + // against a SCAFFOLDED artifact whose shape the template already fixes. High + // effort was buying deliberation over a form that is largely filled in, three + // times over. If fix-lists grow after this, that is the signal to move it back. for _, want := range []string{ "name: spec-author", "tools: Read, Grep, Glob, Edit, Write, Bash", "model: sonnet", - "effort: high", + "effort: medium", } { if !strings.Contains(body, want) { t.Errorf("spec-author.md frontmatter missing %q", want) @@ -348,6 +353,19 @@ func TestSpecAuthorAgentEncodesDiscipline(t *testing.T) { t.Errorf("spec-author.md must not instruct approving its own phase: %q", bad) } } + // The fix loop is BOUNDED. "Re-validate until it returns 0" is an open loop, + // and the finding it cannot resolve — usually a tasks.md shape that does not + // match the spec's development_flow — is exactly the one it will not resolve on + // the fourth pass either. Each pass re-reads an 800-line spec to retry a fix it + // already tried, which costs more than the orchestrator spends deciding. + if strings.Contains(body, "re-validate until it returns 0") { + t.Error("spec-author.md must not instruct an unbounded validate/fix loop") + } + for _, want := range []string{"at most twice", "two rounds"} { + if !strings.Contains(body, want) { + t.Errorf("spec-author.md should bound the fix loop (%q)", want) + } + } } func TestImplementerAgentEncodesDiscipline(t *testing.T) { agents, err := AgentFiles(FS) @@ -576,3 +594,46 @@ func TestPlanSessionEntryCarriesTheLoopContract(t *testing.T) { } } } + +// TestAgentEffortLadder pins where each sub-agent's reasoning effort sits. +// +// The dial is not per-agent taste. It is dispatch frequency weighed against what +// being shallow costs, and the two pull in opposite directions: +// +// - `implementer` fires ~7 times per feat, but each dispatch is one small, +// already-bounded task with a cycle skill to follow. It stays high because it +// WRITES the code — a shallow implementation is the expensive kind. +// - `spec-author` fires 3 times per feat against a SCAFFOLDED artifact whose +// shape the template already fixes. Medium: deliberation over a form that is +// largely filled in, three times over, was not buying its cost. +// - `code-reviewer` / `security-reviewer` fire once or twice, and they are pure +// judgment. Lowering them saves almost nothing and risks the defect the gate +// exists to catch: one measured feat arrived "implemented" with 691 green +// tests and a drawer that never collapsed at any width — the ARIA and state +// were there, the layout was not. The review is what found it. +// - `quality-gate` runs commands and relays their real output; its own prompt +// says it decides nothing about the code. Low. +// +// Change a value here only with the reasoning that moved it, in this comment. +func TestAgentEffortLadder(t *testing.T) { + agents, err := AgentFiles(FS) + if err != nil { + t.Fatal(err) + } + for name, want := range map[string]string{ + "implementer.md": "effort: high", + "spec-author.md": "effort: medium", + "code-reviewer.md": "effort: high", + "security-reviewer.md": "effort: high", + "quality-gate.md": "effort: low", + } { + body, ok := agents[name] + if !ok { + t.Errorf("AgentFiles is missing %s", name) + continue + } + if !strings.Contains(body, want) { + t.Errorf("%s should carry %q", name, want) + } + } +} diff --git a/internal/templater/templates/agents/quality-gate.md.tmpl b/internal/templater/templates/agents/quality-gate.md.tmpl index 23165fc..2cf18c1 100644 --- a/internal/templater/templates/agents/quality-gate.md.tmpl +++ b/internal/templater/templates/agents/quality-gate.md.tmpl @@ -3,7 +3,7 @@ name: quality-gate description: Runs the feat-exit verification gate — the plan's Quality Gate commands plus the Tier-3 test-report with coverage — and reports pass/fail with the real output. Use at feat exit, once every implementer has finished, dispatched alongside code-reviewer and security-reviewer. Not for per-task verification: that is the implementer's Tier-2 gate. tools: Read, Grep, Glob, Bash model: sonnet -effort: medium +effort: low color: cyan --- diff --git a/internal/templater/templates/agents/spec-author.md.tmpl b/internal/templater/templates/agents/spec-author.md.tmpl index e824e3d..6250fd2 100644 --- a/internal/templater/templates/agents/spec-author.md.tmpl +++ b/internal/templater/templates/agents/spec-author.md.tmpl @@ -3,7 +3,7 @@ name: spec-author description: Authors ONE spec phase artifact (requirements, design, or tasks) for a feat — scaffolds it with `csdd spec generate`, consults the knowledge graph for the governing code/ADRs/stack, writes the body to EARS and traceability, validates, and reports. Sonnet writes; the orchestrator (opus) reviews, validates, and approves. Does not implement, does not approve, does not touch the approved plan. tools: Read, Grep, Glob, Edit, Write, Bash model: sonnet -effort: high +effort: medium skills: - graph color: purple @@ -75,10 +75,14 @@ so reading past what the phase needs costs real time and buys nothing. `_Depends:_` IDs exist; no two `(P)` tasks share a boundary. Shape follows `development_flow` — the scaffolded template already carries the right shape; keep it. Sub-tasks are 1–3 hours of focused work. -4. **Validate — and fix what it finds.** - `csdd spec validate ` returns 2 on findings. Fix every finding in the - artifact body and re-validate until it returns 0. Do not `--force` past a - real finding. +4. **Validate — and fix what it finds, at most twice.** + `csdd spec validate ` returns 2 on findings. Fix them in the artifact + body and re-validate — but give it **two rounds, not an open loop**. A finding + that survives two honest attempts is one you are reading wrong, and each + further round re-reads a spec that is commonly 800+ lines for a fix you have + already tried: it costs more than the orchestrator spends deciding. Report + what is left, quoting the finding verbatim and what you tried. Do not + `--force` past a real finding. 5. **Report.** State the phase, the artifact path, a one-line summary of what it covers, the validate result (0 / findings left), and any unresolved gap or open question for the orchestrator. Your report **is** the handoff the @@ -114,8 +118,9 @@ so reading past what the phase needs costs real time and buys nothing. - [ ] The named phase artifact is scaffolded and authored under `specs//`. - [ ] EARS (requirements) / traceability + boundary map + stack-ADR conformance (design) / annotations + flow-correct shape (tasks) hold. -- [ ] `csdd spec validate ` returns 0 (or the remaining findings are named - and explicitly left for the orchestrator). +- [ ] `csdd spec validate ` returns 0 (or, after two fix rounds, the + remaining findings are quoted verbatim and explicitly left for the + orchestrator, with what you tried). - [ ] No `spec approve` called; `spec.json` frontmatter, the plan, and `.csdd/` untouched. - [ ] Report states: phase, artifact path, summary, validate result, open gaps. \ No newline at end of file