feat(loop): wire ADR 0064 P2 board seam — execution-grounded coder.solve()#61
Merged
Conversation
…lve() On a fresh build, when the `coder` plugin is importable, the feature has acceptance criteria, and a runnable acceptance-test command is configured, the loop now dispatches through coder.solve()'s execution-grounded ladder (greedy → best-of-k → tree-search) instead of a single delegate_to(acp) shot — gated on the feature's acceptance tests actually passing in a real candidate worktree, never an LLM judge. - coder_seam.py: a thin adapter composing plugins.coder.solve (imported lazily/best-effort — no hard dependency on the coder plugin). generate() creates a fresh throwaway worktree per candidate and dispatches the ACP coder into it; verify() runs the configured acceptance-test command in that worktree and reports real pass/fail. The winner is promoted to the feature's canonical worktree/branch; losers are reaped. A spent budget with no passing candidate raises SolveExhausted (a WorktreeError subclass), which the existing capability-failure handling treats exactly like NoChangesError/CoderTimeout — so the coders-map tier ladder still climbs when search itself stalls. - Honest degrade: should_use_solve requires all three gates (coder importable + acceptance_criteria + a test command); missing any falls back to today's single shot, so no existing deployment can regress. - store.py: record_gens_spent accumulates coder.solve()'s generation cost onto a `gens:<total>` label; `_project` surfaces it as `gens_spent` so portfolio_rollup can read cost without raw reads. - Deferred: compiling EARS acceptance criteria into generated tests. The coder is already prompted to write tests satisfying the acceptance criteria as part of its definition of done; verify() runs whatever tests exist via the configured command rather than fabricating them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coder.solve() (plugins/coder/solve.py) has no try/except around its injected generate()/verify() callables — it assumes a candidate attempt only ever fails its tests, never errors. But a real ACP dispatch can raise for real (CoderTimeout on one best-of-k candidate, a worktree op erroring), and that propagated straight out of dispatch() uncaught, leaking every worktree already created for that run: untracked in the loop's `_inflight` map until dispatch() returns, and invisible to the health sweep (a `.gN` candidate id isn't a real board feature, so the sweep's own get_feature() lookup raises and it skips reaping). This was a much more common trigger than the "crash mid-ladder" case the PR already called out as a known limitation — a single slow candidate in best-of-k hitting coder_timeout_s would hit it on any normal run. Wrap the solve() call so any exception reaps every candidate seen so far, surfaces the attempted generation count as spent cost (a failed dispatch still spent the gen), and re-raises the original exception unchanged so the loop's existing capability-failure classification (retry/escalate/block) still applies. Also drop the `typing.Optional` usage in this file for the `X | None` style every other module here already uses under `from __future__ import annotations`, and hoist the (always-available, stdlib) `importlib` import to the top of the file with the rest of the module's imports. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…/coder_solve precedence Two adversarial-review blockers on the ADR 0064 P2 board seam: - coder_seam.dispatch()'s two record_gens(...) calls (mid-ladder-raise and post-solve()) were unguarded, even though store.record_gens_spent's own docstring promises fire-and-forget semantics. A transient BoardError from a `br` hiccup would propagate past _drive's capability-failure handling into the outer `except BoardError`, which never reaps a worktree — leaking an already-verified (or already-reaped) candidate and blocking the feature over a bookkeeping label write. Wrapped both call sites in a shared _record_gens_best_effort() helper that logs and swallows. - _use_coder_solve() let coder.solve() preempt Max-Mode purely because the separate `coder` plugin became importable, even on a board already configured with max_mode_n>1 + local_gate_cmd (the README's own documented execution-grounded Max-Mode recipe). Since coder_solve defaults on and its test-cmd falls back to local_gate_cmd, that board flips from "always ships a best-effort PR" to "blocks outright on an exhausted search" with zero config change of its own. coder.solve now only preempts Max-Mode when max_mode_n<=1; a board wanting the ladder over Max-Mode must opt in explicitly. Added tests pinning both fixes (record_gens raising on the promoted-winner, exhausted, and mid-ladder-raise paths; Max-Mode-wins-precedence).
mabry1985
marked this pull request as ready for review
July 3, 2026 19:22
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
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.
HIGH-RISK — needs human review before merge
This wires the ADR 0064 P2 board seam: the loop's per-feature dispatch composes
the
coderplugin's execution-groundedsolve()ladder instead of a bare singledelegate_to(acp)shot, when it's safe to do so. Land as DRAFT.ADR: https://github.com/protoLabsAI/protoAgent/blob/main/docs/adr/0064-coder-execution-grounded-code-solve.md
What landed
coder_seam.py(new) — a thin adapter, not a reimplementation of the ladder.It composes
plugins.coder.solve(imported lazily/best-effort — this repocarries no hard dependency on the
coderplugin, matching the "compose delegates,don't reimplement" discipline this repo already follows for the ACP/A2A spawn
primitive):
generate(task, feedback)creates a fresh throwaway worktree per candidate(
feat-<id>.g<n>) and dispatches the ACP coder into it — the "independent-parallel acp attempts" the coder plugin's own
generate.pydocstring alreadyflags as "the P2 path."
verify(candidate_wt)runs the configured acceptance-test command in thatworktree and reports real pass/fail (never an LLM judge).
worktree/branch (reusing
worktree.promote_worktree, the same primitiveMax-Mode already uses) so the rest of the drive — fixups, the pre-PR local gate,
open_pr, the CI bounce, tier escalation — is completely unchanged. Everyother candidate is reaped.
SolveExhausted(aWorktreeErrorsubclass) rather than opening a PR on an unverified partial.loop.py— one new branch in_drive's dispatch selection (alongside theexisting
keep_wt/ max-mode / plain-single-shot branches):SolveExhaustedwas added to the existingcapabilityfailure tuple (next toNoChangesError/CoderTimeout) — zero other changes to the escalation/retrystate machine. This is the composition point the ADR asks for:
coder.solve()searches within the current model tier; a search that never passes is a
capability failure for THAT tier, so it climbs the
coders-map ladder (or blocks)exactly like a no-diff dispatch always has. The tier ladder is untouched.
store.py—record_gens_spent(fid, n)accumulatescoder.solve()'sgeneration cost onto a single, replaced
gens:<total>label (called once persolve()run, win or lose — a failed search still spent gens);_projectnowsurfaces it as
gens_spenton every feature soportfolio_rollup(a separateplugin) can read cost without raw reads, per the ADR's cost-v1 ethos.
protoagent.plugin.yaml/ README — new config:coder_solve(opt-out valve,default on),
coder_solve_test_cmd(falls back tolocal_gate_cmdif blank),coder_solve_test_timeout_s,coder_solve_budget,coder_solve_k,coder_solve_tree_depth. Version bumped0.25.0→0.26.0(minor).The exact seam
Honest-degrade behavior (non-regressing by construction)
_use_coder_solve/coder_seam.should_use_solverequire all three:coderplugin is importable (host has it enabled — it ships disabled bydefault, so most repos never reach this at all),
acceptance_criteria(the Ready gate's existing oracle),coder_solve_test_cmdorlocal_gate_cmd) — without one there is genuinely nothing to execute, and perthe ADR's no-LLM-judge rule we do not fake grounding with a judge instead.
Missing any of the three → today's single
delegate_to(acp)shot runs,byte-for-byte the same as before this PR. A carried-forward re-dispatch (CI bounce /
goal-fix / gate-fix, signalled by
_ci_feedback) also skips the seam — same ruleMax-Mode already follows, since that re-dispatch fixes the existing diff with one
coder rather than fanning out fresh candidates.
Deferred (see ADR + notes)
called out in the ADR as the "genuinely new work" and is more than one clean PR.
The simplest-correct path used here instead: the coder is already prompted
(
_build_prompt) to write tests satisfying the acceptance criteria as part of itsdefinition of done;
verify()just runs whatever tests exist in the candidateworktree via the configured command and gates on the real exit code — honest
execution, no fabricated grounding, but coarser-grained than a proper EARS→test
compiler (the
Verdictit returns doesn't name individual failing test cases,just pass/fail + the raw output tail as feedback).
_inflightuntilcoder_seam.dispatch()returns (same pre-existing limitation Max-Mode's_dispatch_max_modealready has) — they self-heal on the next attempt viacreate_worktree's idempotent stale-worktree cleanup, but aren't reaped by thehealth sweep in between. A companion improvement, not blocking for this cut.
How to review / test
coder_seam.pytop-to-bottom — it's the whole seam in one file.loop.pydiff: the one addedelifbranch in_drive's dispatch selection,the one line added to the
capabilitytuple, and the new config fields in__init__. Everything else is untouched.pytest tests/test_coder_seam.py— the dispatch decision(
should_use_solve) anddispatch()'s orchestration (winner promoted, losersreaped, gens-spent surfaced,
SolveExhaustedon exhaustion) withplugins.coder.solveinjected as fakes (no need for the actual
coderplugin repo).pytest tests/test_loop.py -k coder_solve— the honest-degrade matrix at the_drivelevel (coder available+oracle → solve path; missing any gate → single shot;
SolveExhausted→ blocked like a capability failure; CI-bounce → skips the seam).pytest tests/test_store.py -k gens_spent— the cost-accounting label.delegates+coder+project_board, declarean
acpcoder, setproject_board.coder_solve_test_cmd(orlocal_gate_cmd) toyour repo's real test command, and give a feature
acceptance_criteria. Watchthe loop create
feat-<id>.g1,.g2, … candidate worktrees before promoting thewinner to
feat-<id>.Gate output
(ruff pinned to 0.15.10 to match
.github/workflows/ci.yml.)🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com