feat(templater): set each agent's effort by how often it fires, not by role - #94
Merged
Conversation
…y role `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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARBSaXgJ7ssW1hvpy6r64y
📝 WalkthroughWalkthroughThe agent templates update effort tiers, bound the spec-author validation/fix loop to two rounds, and expand templater tests to verify both the workflow wording and expected effort metadata. ChangesAgent template updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@internal/templater/templater_test.go`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 361c8208-3cdb-44d8-a7cd-00dc501345fd
📒 Files selected for processing (3)
internal/templater/templater_test.gointernal/templater/templates/agents/quality-gate.md.tmplinternal/templater/templates/agents/spec-author.md.tmpl
Comment on lines
+623
to
+635
| 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) { |
There was a problem hiding this comment.
🎯 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The dial
Reasoning effort was set per agent by what the agent is. It should be set by how often it fires, weighed against what being shallow costs there. Those pull in opposite directions, and the ladder below is where they balance.
implementerspec-authorcode-reviewersecurity-reviewerquality-gateEvidence
From a measured run's ledger (one feat,
panel-shell):spec-authorat high was that a shallow draft pushes real authoring back onto the opus reviewer as fix-lists, defeating the cost split — but the split was already lost at the sonnet end, three dispatches per feat over a largely pre-filled form. That reasoning is not deleted; it is rewritten in the test comment with the measurement that moved it, and with the signal to reverse it (fix-lists growing).data-drawer="open"read by no selector. The feat-exit review is what found it. That is what high effort buys on a reviewer, and why the two reviewers do not move.quality-gateneeds no measurement: its own prompt says "You decide nothing about the code — you execute the declared checks and relay their real output."Also: the fix loop is bounded
spec-authorstep 4 said "re-validate until it returns 0" — an open loop. The finding it cannot resolve on the second pass (usually atasks.mdshape that does not match the spec'sdevelopment_flow) is not one it resolves on the fourth either, and every pass re-reads a spec that is commonly 800+ lines to retry a fix it already tried. Two rounds, then report what is left quoting the finding verbatim and what was tried — the orchestrator decides in one turn what the loop was spending many on.Verification
make checkgreen.TestAgentEffortLadderpins all five values in one place, with the reasoning per tier in its comment and an explicit instruction: change a value here only together with the reasoning that moved it.re-validate until it returns 0, so the open loop cannot come back by edit.TestSpecAuthorAgentShipped/…EncodesDisciplinestill hold.Installed workspaces pick this up via
csdd update(a hand-edited agent is preserved as a numbered.oldbackup, as always).🤖 Generated with Claude Code
https://claude.ai/code/session_01ARBSaXgJ7ssW1hvpy6r64y
Summary by CodeRabbit
New Features
Improvements