fix(sdd): stop the controller's context growing without bound - #2358
akalongman wants to merge 2 commits into
Conversation
Reviewers write the full report to a review file and return a verdict block with numbered finding ids; the fix loop sends the file path and the open ids instead of re-typing findings. Rounds 1 to 3 resume the implementer only while its context is under 300,000 tokens. A context checkpoint after every third completed task, and before the final review, ledgers a Checkpoint line and prints the compaction line. Measured on three long real runs of 6.3.0 (an OpenSpec change of four sections, plans of 14 and 25 tasks): controller context reached 650k to 781k tokens and the controller was 38 to 40 percent of every run's input tokens; task reviews ran 9k to 14k characters each and were then re-typed into fix messages; fix rounds resumed at 300k tokens and above cost about twice a fresh dispatch.
obra
added
enhancement
1. The pre-final-review checkpoint has no resume path (major) — "Checkpoint first (above)" is mandatory and the partner is told to re-invoke the skill. But the only resume rules are Setup, :147-153: "resume at the first task without [a 2. :28-31 now says "Verdict exactly these ids, and only these" and "A finding the list does not name is not yours to judge", but :62 still reads "Your scope is the findings list and the fix diff. Verdict every finding." Pre-image the findings were a curated 3. The The template orders the reviewer to "list every behavior you considered and set aside as outside the plan or spec, one line each, with the reason. The executor rules on each line; nothing you set aside is dropped silently." The new block (:113-117) is |
After the pre-final-review checkpoint every task has a completion line, so Setup's resume rule never fired; it now resumes at the Final Review when every task is complete. The re-review prompt's scope sentence still said 'verdict every finding' against the new 'exactly these ids'. The code-reviewer verdict block gains a D line per declined behavior and the Output Format a heading for them, so the list the template promises the executor survives the move of the report into a file.
|
Verified all three against the branch and pushed the fixes in one commit:
|
3 participants
Who is submitting this PR? (required)
claude-fable-5-1) ran the session; Claude Opus 5, 1M context (claude-opus-5[1m]) subagents measured the transcripts, drafted this diff and ran the testsWhat problem are you trying to solve?
Five real runs of
subagent-driven-development(plugin 6.3.0, Claude Code,1M-token context, 2026-09-15 to 2026-09-21) were instrumented from the
session transcripts. Three of them were long: an OpenSpec change executed as
4 sections, a 14-task plan, and a 25-task plan. The two others were small (0
and 3 dispatches) and are only mentioned for completeness.
In all three long runs the controller, not the work, was the dominant cost.
Controller context reached 650k, 752k and 781k tokens, and the controller
accounted for 38 to 40 percent of every run's input tokens. Three mechanisms
in the skill put it there.
1. Reviews come back as messages, then get re-typed. Both reviewer
templates say "Your final message is the report itself", so every task
review and every re-review lands in the controller's context and is re-read
on every later turn. Measured in the loop windows: 78,342 characters of
review reports over 6 reviews plus 77,868 over 7 re-reviews (4-section run);
186,153 over 13 plus 116,103 over 10 (14-task run); 213,635 over 23 plus
86,962 over 13 (25-task run). One report runs 9k to 14k characters. The
controller needs a verdict and a finding list out of that; it reads the rest
for the rest of the session.
The same text is then paid for twice, because the fix loop says "Send it the
open findings verbatim". In the 14-task run the controller sent 19 fix-round
messages totalling 98,267 characters, the largest 9,542 characters, each one
a re-typing of findings that were already resident.
2. Fix rounds resume the implementer with no context threshold. "Rounds
1-3 resume the original implementer" has no size limit, and a resumed
subagent re-reads its whole context on every turn. Across the three runs, by
segment: rounds resumed under 200k tokens cost 0.87x what a fresh
implementer would have (20 rounds), 200k to 300k cost 1.53x (11 rounds), 300k
and above cost 2.07x (21 rounds). The single task that reached the five-round
cap was resumed eleven times, from 62k to 683k tokens, and that one task
took 19 percent of its run's whole subagent bill. Rounds 4 and 5 were
resumed too, although both the skill and the model table call for a fresh
escalated dispatch, because nothing in the fix loop makes the switch
mechanical.
3. Nothing sheds controller context between tasks. The ledger exists so
progress survives compaction, but no step ever compacts. The 14-task run hit
Claude Code's automatic compaction twice, at 777,428 and 780,600 tokens, with
17k to 19k character summaries written mid-loop; the 25-task run was
compacted by the user at 714,970 tokens. Replaying the deduplicated
per-request series with a checkpoint after every third task shows 12, 20 and
36 percent of controller input tokens saved on the three runs, and every
checkpoint lands at a task boundary where the ledger already holds
everything, instead of mid-fix-round where the auto-compactions landed.
What does this PR change?
Reviewers write their full report to a review file and return a short
verdict block with numbered finding ids, and the fix loop passes that file
path plus the open ids instead of re-typing findings. Fix rounds 1 to 3
resume the implementer only while its context is under 300,000 tokens and
dispatch a fresh one above that. A context checkpoint after every third
completed task, and before the final review, ledgers a
Checkpoint:line,prints a
/compactline that keeps the plan, workspace, ledger andprogress, and stops for the partner to re-invoke the skill.
Is this change appropriate for the core library?
Yes. All three changes are properties of the skill's own dispatch protocol,
not of any project, language or tool. Any plan long enough to need SDD pays
these costs. There is no new dependency, no new script and no third-party
integration: the review file lives in the workspace
sdd-workspacealreadycreates, and the checkpoint uses whatever compaction the harness has, with a
stated fallback for harnesses that have none.
The
code-reviewer.mdchange is deliberately conditional.[REVIEW_FILE]is optional there, because that template is also used standalone through
requesting-code-review, where a human reads the report directly. With noreview file named, its behaviour is unchanged.
What alternatives did you consider?
Shorten the reports instead of relocating them. The reports are not
padding: the passing-check citations are what force the reviewer to do the
checks, and the fix detail is what the implementer works from. Cutting them
buys tokens by losing review quality. Relocating them costs one file write
and keeps everything.
Let the controller summarize the reviews into the ledger. That is the
behaviour being paid for today, in controller output tokens, and it puts a
paraphrase between the reviewer and the fixer. Ids plus a path remove the
paraphrase.
A hard token cap on the implementer rather than a resume threshold.
Capping the implementer punishes long tasks. The threshold only decides
where the next fix round runs, which is the actual cost driver, and it
leaves a small implementer resumed, which is still the cheapest case.
Compact on a token threshold instead of a task count. A threshold needs
the controller to observe its own context size, which not every harness
reports, and it fires wherever the run happens to be. A task count is
mechanical, and a task boundary is exactly where the ledger is sufficient.
Leave the checkpoint to the user. That is the status quo, and it is what
produced 781k-token contexts and two mid-loop automatic compactions in one
run.
Does this PR contain multiple unrelated changes?
No. One problem: the controller's context is the scarce resource in a long
SDD run, and the skill has no mechanism that bounds it. The three changes
are the three mechanisms that grow it, and two of them are coupled: the
verdict block is what makes "resume with ids, not findings" possible, and
the checkpoint is only safe because reports and reviews are on disk rather
than in the message history. A maintainer who wants only part of it can drop
the checkpoint section on its own.
A separate defect found in the same investigation,
scripts/task-briefleaking trailing plan sections into the last task's brief, is deliberately
not in this PR. It is a script correctness bug with its own prior art and
belongs in its own pull request.
Existing PRs
(searched on 2026-09-21, open and closed, for: review file, verdict,
compaction, checkpoint, resume implementer, task-brief, context,
subagent-driven-development, and the same terms across issues)
Feat: reduce reviewer subagent verbosity to conserve tokens #1930 (issue), Codex: Implementation of relatively simple plan with subagent-driven development consumes full 5h token budget in a single run (PLUS plan) #1152 (issue)
#1966 (open) implements the review-file contract for
task-reviewer-prompt.mdfrom issue #1930, and its own body records why it stopped there: the
re-reviewer and
code-reviewer.mdwere deferred, the latter because it isalso used standalone. This PR is the same idea carried through the whole
loop, which is where the measured cost actually is: the re-reviews are 29 to
50 percent of the review characters in the three runs measured here, and the fix
loop still re-types findings unless the verdict block carries ids the fix
message can name. If the maintainers prefer #1966's smaller diff, the right
outcome is to land it first and reduce this PR to the re-review, the final
review and the fix-loop ids. I did not want to open a duplicate without
saying so explicitly.
#1669 (open) adds a context checkpoint to
executing-plans, the inlinesibling skill, and #2274 (open) adds a ledger Discoveries section so
cross-task findings survive compaction. Neither touches the SDD controller's
checkpoint, and the checkpoint here is written so it composes with both.
#1998 (merged) introduced the resume-based fix rounds this PR puts a
threshold on. #2297 and #2322 (open) both add in-flight dispatch state to the
ledger; the
Checkpoint:line here is an additional line in the sameappend-only ledger and does not conflict. Issue #1152 reports a Codex SDD run
consuming a full token budget, and issue #2113 reports findings from a heavy
multi-phase run; neither proposes these mechanisms.
Environment tested
New harness support (required if this PR adds a new harness)
Not applicable. No harness is added. The checkpoint names
/compactbecausethat is Claude Code's command, and the text tells a harness without one to
use its equivalent or skip the checkpoint.
Clean-session transcript for "Let's make a react todo list"
Evaluation
skill that dispatches implementation work through
subagent-driven-development, using the transcripts of the five runs thathad already used it. The three findings above are the sub-skill's share of
that review. They came out of measuring five sessions that had already
happened, not out of reading the skill.
all pre-change, from instrumented real runs. What is verified today is
stated in the next section; what is not verified is whether a reviewer
subagent holds the verdict-block contract as well as it holds the current
one, and whether the checkpoint changes task outcomes. That is an A/B the
maintainers are right to ask for before merging, and I would rather say so
than dress up arithmetic as an eval.
task removed by the review file, 12 to 36 percent of controller input
tokens saved by the checkpoint, and about half the cost of every fix round
that today runs on an implementer above 300k tokens.
Rigor
superpowers:writing-skillsandcompleted adversarial pressure testing (paste results below)
rationalizations, "human partner" language) without extensive evals
showing the change is an improvement
The first two boxes are unchecked on purpose. The contract wording was
derived from the measurements and from the shape the implementer template
already proves (write the report to a file, return under 15 lines), not from
a pressure-testing sweep, and no adversarial session has been run against the
new wording yet. The Rationalizations table, the Red Flags content
and the "human partner" language are untouched. One tuned line did have to
move: Continuous execution says the only reasons to stop are the four named
below or all tasks complete, so the checkpoint is named there as a third
reason, and the checkpoint text itself states that it is not the check-in or
progress summary that line forbids.
What was verified, and how:
survive: a reviewer that finds nothing still returns two verdict lines and
a path, and a reviewer that finds a cannot-verify item still surfaces it in
the block, where the controller must resolve it, rather than burying it in
the file.
fix rounds split by the resumed agent's context at the time (0.87x under
200k, 1.53x from 200k to 300k, 2.07x at 300k and above), so 300,000 is the
point where a fresh dispatch wins under both an optimistic and a
pessimistic allowance for what the fresh agent must re-read.
of the three long runs, with a post-compaction floor taken from the
compactions those runs actually performed.
bash tests/claude-code/test-sdd-workspace.sh,bash tests/claude-code/test-executing-plans-scripts.shandbash tests/claude-code/test-worktree-path-policy.shpass unchanged. Thefourth file in
run-skill-tests.sh,test-subagent-driven-development.sh, drives the installed plugin throughthe CLI and asserts on the skill's description, which this PR does not
change; it was not run here.
Human review