feat(plan): cut plan-run token cost with a spec-author sub-agent and a leaner brief - #87
Merged
Merged
Conversation
…a leaner brief A real `csdd plan run` burned ~$200 in record time. The cost was the methodology itself: every feat paid, on the opus orchestrator, for full spec authoring, graph consultation, and gates before any code. This moves the bulk to cheaper models and trims what each feat re-reads, while keeping EARS, traceability, and phase gates — and adds the mechanical stack/ADR conformance the inlined brief was the only proxy for. - spec-author sub-agent (sonnet) authors each spec phase; opus only reviews, validates, and approves, re-dispatching spec-author with a fix-list - FeatBrief keeps only the feat and its specs: stack/ADR/wiki are refs only, fetched via `csdd graph explain`/`graph query`; bodies no longer inlined - default development_flow -> unit (tdd required for money/auth/tenancy); `spec generate --artifact tasks` is flow-aware (unit-shaped template) - mechanical stack/ADR conformance: `plan validate` requires design.md to cite each governing adr:<slug>; the code-reviewer checks the implementation against the decided stack - plan-dev reinforces test timing: Tier-2 tests the part during the task, Tier-3 tests the whole at feat exit; never the full suite inline - root CLAUDE.md trimmed and reframed MCP-first (the csdd_* tools are the default; typed params carry the artifact/phase contract)
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (30)
📝 WalkthroughWalkthroughThe change adds plan cost and delivery verification, per-model session accounting, worktree entry injection, generated-artifact merge handling, refs-only governance briefs, ADR/design validation, and unit-flow defaults with flow-specific task generation. ChangesPlan execution and delivery evidence
Unit development flow
Governance references and design validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant PlanCLI as plan CLI
participant Runner as plan runner
participant Worktree as feat worktree
participant Ledger as session ledger
participant Verify as delivery verifier
Operator->>PlanCLI: run plan
PlanCLI->>Runner: start bounded execution
Runner->>Worktree: prepare entry document and execute feat
Runner->>Ledger: record session metrics and verdict
Operator->>PlanCLI: request cost or verification report
PlanCLI->>Ledger: load session evidence
PlanCLI->>Verify: inspect artifacts, merges, and worktrees
Verify->>PlanCLI: return report and findings
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 4
🧹 Nitpick comments (2)
internal/plan/metrics.go (1)
134-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo unmarshals of the same entry.
modelTokensandmodelCosteach decoderawindependently; one decode into a shared map would be simpler and cheaper.♻️ Suggested consolidation
-func modelTokens(raw json.RawMessage) SessionTokens { - var m map[string]any - if json.Unmarshal(raw, &m) != nil { - return SessionTokens{} - } - return SessionTokens{ +func modelUsage(raw json.RawMessage) (SessionTokens, float64) { + var m map[string]any + if json.Unmarshal(raw, &m) != nil { + return SessionTokens{}, 0 + } + t := SessionTokens{ Input: pickNum(m, modelUsageKeys["input"]), Output: pickNum(m, modelUsageKeys["output"]), CacheRead: pickNum(m, modelUsageKeys["cache_read"]), CacheCreation: pickNum(m, modelUsageKeys["cache_creation"]), } -} - -func modelCost(raw json.RawMessage) float64 { - var m map[string]any - if json.Unmarshal(raw, &m) != nil { - return 0 - } for _, k := range modelUsageKeys["cost"] { if f, ok := m[k].(float64); ok { - return f + return t, f } } - return 0 + return t, 0 }🤖 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/plan/metrics.go` around lines 134 - 158, Consolidate the duplicate JSON decoding performed by modelTokens and modelCost into a single decode of each raw entry, then reuse the resulting map for both token and cost extraction. Preserve the existing zero-value behavior when unmarshalling fails and retain the current key-selection logic in pickNum and modelUsageKeys.internal/cli/plan.go (1)
71-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHelp text and defaults disagree for
--effort.Line 72 sets the default to
"medium"but says "Empty inherits the ambient default" — the flag can never be empty unless explicitly passed--effort="", andRunOptionsdocs (runner.go Line 92) still claim the CLI defaults to "opus / high". Worth reconciling the runner comment with the newmediumdefault.🤖 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/cli/plan.go` around lines 71 - 77, The --effort default and its documented behavior are inconsistent. Update the `effort` flag definition and the related `RunOptions` documentation in `runner.go` so the stated defaults and empty-value inheritance behavior accurately match the implementation, including reconciling the documented orchestrator model/effort defaults with the current CLI values.
🤖 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/cli/spec.go`:
- Around line 492-494: Update effectiveFlow to trim surrounding whitespace from
development_flow before determining the effective flow, keeping template
selection in SpecGenerate consistent with validator.specDevelopmentFlow. Add a
regression test covering a whitespace-only development_flow and verify it
selects the unit tasks template rather than the TDD template.
In `@internal/plan/brief.go`:
- Around line 111-128: Update the governing-decisions text in the brief
generation block to reference “csdd plan validate” instead of “csdd spec
validate,” including the self-check guidance for ADR citations. Keep the
existing ADR resolution and design citation requirements unchanged.
In `@internal/plan/cost.go`:
- Around line 52-54: Remove the unused FeatCost.wasted method from the cost
reporting code; callers can use the exported FeatCost.Gated field directly. Do
not add replacement logic unless an existing report specifically requires the
helper.
In `@internal/templater/templates/spec/tasks-unit.md.tmpl`:
- Around line 16-18: Update the development-flow guidance in the spec template
to identify both “tdd” and “tdd-e2e” as supported RED/GREEN alternatives,
phrased as “tdd or tdd-e2e, as appropriate.” Preserve the instruction to set
development_flow and regenerate tasks rather than manually shaping RED/GREEN
tasks.
---
Nitpick comments:
In `@internal/cli/plan.go`:
- Around line 71-77: The --effort default and its documented behavior are
inconsistent. Update the `effort` flag definition and the related `RunOptions`
documentation in `runner.go` so the stated defaults and empty-value inheritance
behavior accurately match the implementation, including reconciling the
documented orchestrator model/effort defaults with the current CLI values.
In `@internal/plan/metrics.go`:
- Around line 134-158: Consolidate the duplicate JSON decoding performed by
modelTokens and modelCost into a single decode of each raw entry, then reuse the
resulting map for both token and cost extraction. Preserve the existing
zero-value behavior when unmarshalling fails and retain the current
key-selection logic in pickNum and modelUsageKeys.
🪄 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: 195cc288-42cb-4e18-89c0-cafbcc4d245b
📒 Files selected for processing (30)
internal/cli/cli_extra_test.gointernal/cli/plan.gointernal/cli/plan_adr_test.gointernal/cli/spec.gointernal/cli/spec_flow_test.gointernal/plan/adr_test.gointernal/plan/brief.gointernal/plan/brief_test.gointernal/plan/cost.gointernal/plan/cost_verify_test.gointernal/plan/debug_test.gointernal/plan/ledger.gointernal/plan/metrics.gointernal/plan/runner.gointernal/plan/tree.gointernal/plan/validate.gointernal/plan/validate_test.gointernal/plan/verify.gointernal/templater/templater.gointernal/templater/templater_test.gointernal/templater/templates/agents/code-reviewer.md.tmplinternal/templater/templates/agents/spec-author.md.tmplinternal/templater/templates/commands/csdd-plan-run.md.tmplinternal/templater/templates/plan/CLAUDE.md.tmplinternal/templater/templates/root/CLAUDE.md.tmplinternal/templater/templates/skills/plan-dev/SKILL.md.tmplinternal/templater/templates/spec/tasks-unit.md.tmplinternal/templater/templates/spec/tasks.md.tmplinternal/validator/validator.gointernal/validator/validator_test.go
Comment on lines
+492
to
+494
| if opts.Artifact == "tasks" && effectiveFlow(data.DevelopmentFlow) == "unit" { | ||
| pair[1] = "templates/spec/tasks-unit.md.tmpl" | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize the flow before selecting the tasks template.
validator.specDevelopmentFlow trims development_flow, so a value such as " " resolves to unit. SpecGenerate calls effectiveFlow without trimming, falls through to the TDD template, and produces RED/GREEN tasks that validation rejects. Normalize in effectiveFlow and add a whitespace-only regression test.
Proposed fix
func effectiveFlow(f string) string {
+ f = strings.TrimSpace(f)
if f == "" {
return defaultDevelopmentFlow
}
return f
}🤖 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/cli/spec.go` around lines 492 - 494, Update effectiveFlow to trim
surrounding whitespace from development_flow before determining the effective
flow, keeping template selection in SpecGenerate consistent with
validator.specDevelopmentFlow. Add a regression test covering a whitespace-only
development_flow and verify it selects the unit tasks template rather than the
TDD template.
Comment on lines
+111
to
128
| // 3b. Governing decisions — refs only. The brief used to inline each cited | ||
| // ADR's title + body so a design could not silently ignore the decisions it | ||
| // was bound to; with the bodies gone, `designConformance` requires the | ||
| // authored design.md to cite each governing adr:<slug>, and the session | ||
| // fetches the decision itself via `csdd graph explain adr:<slug>`. | ||
| if len(feat.ADRRefs) > 0 { | ||
| adrs := ScanADRs(root) | ||
| w("## Decisions (docs/adr — the why)\n\n") | ||
| w("## Governing decisions (docs/adr — fetch the why; cite each in design)\n\n") | ||
| w("The brief no longer inlines ADR bodies. For each governor run `csdd graph explain adr:<slug>` for the decision; `csdd spec validate` requires your design.md to cite it.\n\n") | ||
| for _, slug := range feat.ADRRefs { | ||
| adr, res := adrs.Resolve(slug) | ||
| if res != ADRResolved { | ||
| w("- **adr:%s** — WARNING: does not resolve to a docs/adr record (validate should have caught this).\n", slug) | ||
| if _, res := adrs.Resolve(slug); res != ADRResolved { | ||
| w("- adr:%s — WARNING: does not resolve to a docs/adr record (validate should have caught this).\n", slug) | ||
| continue | ||
| } | ||
| w("### %s (adr:%s)\n\n", orDash(adr.Title), adr.Slug) | ||
| if adr.Status == ADRStatusSuperseded { | ||
| w("_status: superseded") | ||
| if adr.SupersededBy != 0 { | ||
| w(" by %s", fourDigit(adr.SupersededBy)) | ||
| } | ||
| w("_\n\n") | ||
| } | ||
| if adr.Body != "" { | ||
| w("%s\n\n", adr.Body) | ||
| } | ||
| w("- adr:%s\n", slug) | ||
| } | ||
| w("\n") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: confirm which CLI command invokes ValidatePlan/designConformance,
# and whether spec validate / the autonomous loop ever calls plan validate.
rg -n "ValidatePlan\(" internal/cli/plan.go internal/cli/spec.go
rg -n -B3 -A3 '"validate"' internal/cli/plan.go internal/cli/spec.go
rg -n "designConformance|plan.*validate" internal/plan/runner.go internal/plan/verify.go 2>/dev/nullRepository: protonspy/csdd
Length of output: 956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant command wiring and the autonomous loop.
sed -n '1,240p' internal/cli/plan.go
printf '\n--- SPEC ---\n'
sed -n '1,240p' internal/cli/spec.go
printf '\n--- PLAN VALIDATION ---\n'
sed -n '1,260p' internal/plan/validate.go
printf '\n--- BRIEF ---\n'
sed -n '80,150p' internal/plan/brief.go
printf '\n--- LOOP SEARCH ---\n'
rg -n "spec validate|plan validate|ValidatePlan|designConformance|quality-gate|done|self-check|acceptance" internal/plan internal/cli -g '!**/*_test.go'Repository: protonspy/csdd
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact validation implementations and the brief's self-check / acceptance gates.
sed -n '280,390p' internal/cli/plan.go
printf '\n--- SPEC VALIDATE ---\n'
sed -n '720,840p' internal/cli/spec.go
printf '\n--- BRIEF SELF-CHECKS ---\n'
sed -n '160,205p' internal/plan/brief.go
printf '\n--- DESIGN CONFORMANCE USAGE ---\n'
rg -n "designConformance\(" internal/plan internal/cliRepository: protonspy/csdd
Length of output: 9150
Point this brief at csdd plan validate, not csdd spec validate.
designConformance is enforced by plan validate, so this text points readers to the wrong check; if ADR citations must block done, the self-checks should mention plan validate too.
🤖 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/plan/brief.go` around lines 111 - 128, Update the
governing-decisions text in the brief generation block to reference “csdd plan
validate” instead of “csdd spec validate,” including the self-check guidance for
ADR citations. Keep the existing ADR resolution and design citation requirements
unchanged.
This was referenced Jul 28, 2026
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.
What
A real
csdd plan runburned ~$200 in record time. The cost was the methodology itself: every feat paid, on the opus orchestrator, for full spec authoring, graph consultation, and gates before any code. This moves the bulk to cheaper models and trims what each feat re-reads, while keeping EARS, traceability, and phase gates — and adds the mechanical stack/ADR conformance the inlined brief was the only proxy for.Changes
spec-authorsub-agent (sonnet) authors each spec phase; opus only reviews, validates, and approves, re-dispatchingspec-authorwith a fix-list. Authoring is the bulk of the per-feat spend; it does not need the orchestrator's model.csdd graph explain adr:<slug>/csdd graph querywhen it needs them. The brief is re-read every turn, so this is the highest-leverage cut.development_flow→unit(tdd/tdd-e2e required for money, auth, tenancy, anything irreversible).spec generate --artifact tasksis now flow-aware: a unit-flow spec scaffolds fromtasks-unit.md.tmpl(implement→cover, no RED/GREEN titles), so it passescheckDevelopmentFlowinstead of fighting it.plan validatenow requiresdesign.mdto cite each governingadr:<slug>the feat's Refs declare (the brief no longer inlines the bodies, so the design must reference each governor); thecode-reviewersub-agent checks the implementation against thedocs/stack.mdDecided rows.plan-devreinforces test timing: Tier-2 (the implementer,--fast) tests the part during the task; Tier-3 (the quality-gate, coverage on) tests the whole at batch-exit and feat-exit. Never run the full suite inline in the orchestrator context.CLAUDE.mdtrimmed and reframed MCP-first: thecsdd_*MCP tools are the default path (a stdio server running thecsddbinary; typed params carry theartifact/phasecontract); the CLI block is the canonical reference/fallback. Removed the skill/agent/MCP authoring instructions in favor of a pointer tocsdd --help.unitis the documented default.Verification
go build ./...,go vet ./internal/...,gofmtclean.go test -race ./internal/...all green.csdd initscaffoldsspec-author.md; default flow =unit; explicittddworks; flow-aware tasks scaffold (unit: no RED/GREEN, tdd: RED/GREEN);plan validatecatches a design not citing a governingadr:and clears when cited.Notes
CLAUDE.md.tmplis a load-bearing static content file; review welcome on the wording/trim.--model opus(orchestrator),--session-budget 0(no cap),--max-iterations 30,--feat-attempts 4.Summary by CodeRabbit
plan costreporting (totals, per-feature, and optional per-model breakdown).plan verifyto validate delivery evidence and flag inconsistencies.CLAUDE.mdis injected.