Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 67 additions & 6 deletions internal/templater/templater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Comment on lines +623 to +635

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the effort value inside frontmatter.

TestAgentEffortLadder searches the entire template body, so it can pass if the expected effort: string appears in prose while the actual frontmatter is wrong. Since this test is intended to pin metadata, parse or isolate the frontmatter before asserting.

Proposed fix
 	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) {
+		parts := strings.SplitN(body, "---", 3)
+		if len(parts) != 3 || !strings.Contains(parts[1], "\n"+want+"\n") {
 			t.Errorf("%s should carry %q", name, want)
 		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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) {
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
}
parts := strings.SplitN(body, "---", 3)
if len(parts) != 3 || !strings.Contains(parts[1], "\n"+want+"\n") {
t.Errorf("%s should carry %q", name, want)
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/templater/templater_test.go` around lines 623 - 635, Update
TestAgentEffortLadder to isolate or parse each agent template’s frontmatter
before checking the expected effort value, ensuring the assertion validates
frontmatter metadata rather than matching prose elsewhere in the body. Keep the
existing agent-name coverage and expected effort mappings unchanged.

t.Errorf("%s should carry %q", name, want)
}
}
}
2 changes: 1 addition & 1 deletion internal/templater/templates/agents/quality-gate.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
---

Expand Down
19 changes: 12 additions & 7 deletions internal/templater/templates/agents/spec-author.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <feat>` 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 <feat>` 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
Expand Down Expand Up @@ -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/<feat>/`.
- [ ] EARS (requirements) / traceability + boundary map + stack-ADR conformance
(design) / annotations + flow-correct shape (tasks) hold.
- [ ] `csdd spec validate <feat>` returns 0 (or the remaining findings are named
and explicitly left for the orchestrator).
- [ ] `csdd spec validate <feat>` 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.
Loading