Skip to content

docs: add convergence loops to multi-agent-plan and self-review-relay - #1168

Open
bjagg wants to merge 3 commits into
LIF-Initiative:mainfrom
bjagg:skill-multi-agent-plan-converge
Open

docs: add convergence loops to multi-agent-plan and self-review-relay#1168
bjagg wants to merge 3 commits into
LIF-Initiative:mainfrom
bjagg:skill-multi-agent-plan-converge

Conversation

@bjagg

@bjagg bjagg commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
Description of Change

Problem. multi-agent-plan ran a fixed four passes (v1→v4) and stopped. Four is arbitrary — v4 may still have gaps, v2 may already be clean, and nothing checked either way. The plan ended by declaring itself "implementation-ready", asserted by the same agent that wrote it.

Solution. A convergence tail. A fresh reviewer checks the plan against all three lens checklists plus scope and returns structured findings; if any are blockers, a targeted revision closes exactly those, and the reviewer runs again. At most two rounds.

The v1→v4 pipeline is deliberately unchanged. Its value is lens diversity, not repetition — looping the same lens adds nothing. The loop belongs on the tail, where the question stops being "what else should this cover?" and becomes "is it done?" — and that second question needs an independent answer.

The design risk, and the guards. A loop that exits when a judge reports zero issues applies pressure in exactly one direction: toward the judge reporting nothing. That is not the same as the plan being good. Four guards, each documented in the skill with its reason:

Guard Failure it prevents
Reviewer is never the reviser, and is told it didn't write the plan "Zero findings" being self-issued by the agent that just made the fix
cap-reached is a reported outcome, not a silent stop Stopping at N and reporting success would state the opposite of the truth — blockers survived
A round must produce a delta Blockers standing + unchanged plan is a stall; re-running re-litigates the same disagreement
A first-round zero is flagged, not banked On a 250–400 line plan, zero findings more likely measures a weak reviewer than a perfect plan

Findings reuse the severity vocabulary from self-review-relay (blocker / should-fix / nit), with section in place of file/line, so the two skills stay comparable and the plan reviewer can be pointed at a diff later.

Reporting. The workflow returns outcome plus a per-round findings array, and the skill's "After the Workflow returns" section now has a table telling the caller what each outcome means for whether the plan is usable. cap-reached and stalled both mean not ready.

Cost. 1–3 extra Opus agents on a skill already spending 4. Pre-flight now states 5–8 agents and 4–6 files instead of the old "~4 plan files".

Related Issues

None — dev-tooling change, no tracking issue.

Type of Change
  • Documentation update
Project Area(s) Affected
  • Documentation (docs/, READMEs, ARCHITECTURE.md, CLAUDE.md)

Checklist
  • commit message follows commit guidelines (see commitlint.config.mjs)
  • documentation is changed or added
Testing
  • Manual testing performed

The embedded script was extracted and exercised with stubbed agents across all five paths — clean, converged-after-revision, cap-reached, stalled, first-round-zero:

converged (r1 clean of blockers, has nits)  -> outcome=converged                finalVersion=v4  plan=PLAN-V4
converged after one revision                -> outcome=converged                finalVersion=v5  plan=PLAN-V5
cap-reached (blockers survive both rounds)  -> outcome=cap-reached              finalVersion=v6  plan=PLAN-V6
stalled (revision identical)                -> outcome=stalled                  finalVersion=v4  plan=PLAN-V4
clean-first-pass-verify (r1 zero findings)  -> outcome=clean-first-pass-verify  finalVersion=v4  plan=PLAN-V4

That dry run caught two real bugs before commit, which is the argument for doing it rather than eyeballing the script:

  1. The loop variable plan shadowed the existing plan prompt-builder function — a hard SyntaxError the script would have hit at runtime.
  2. finalVersion counted rounds-with-blockers instead of accepted revisions, so a stalled run reported v5 while finalPlan was still v4 — we'd have written a -v5.md file containing v4's content.

Syntax verified with node --check against the body wrapped in an async function, which is how Workflow actually executes it (a bare node --check mis-reports the top-level return that every workflow script ends with).

Additional Notes

The reasoning behind which skills should and shouldn't get this — including the ones that explicitly should not (test, promote-to-demo, issue-sweep) — is written up outside this repo in the AI-training notes. The short version: prefer a deterministic gate (ruff/ty/pytest/poly check) to a judge wherever one exists. A plan has no such oracle, which is the only reason a judge loop is right here.

self-review-relay is the natural next candidate — it already half-loops informally ("offer to fix the blockers, then re-run the relevant lens") without a cap or a fresh-reviewer rule. Not in this PR.

SKILL.md grows 140 → 283 lines. Most of that is the embedded script; skills load on demand rather than sitting in context like CLAUDE.md, so the ~200-line guidance for agent maps doesn't apply — but flagging the growth rather than letting it pass unmentioned.

The skill ran a fixed four passes, v1 through v4, and stopped. Four is arbitrary:
v4 may still have gaps and v2 may already be clean, and nothing checked either
way. The plan was declared "implementation-ready" by the same agent that wrote
it.

Adds a convergence tail. A fresh reviewer checks the plan against all three lens
checklists plus scope and returns structured findings; if any are blockers, a
targeted revision closes exactly those and the reviewer runs again. At most two
rounds.

The pipeline itself is unchanged, on purpose. The value of v1 to v4 is lens
diversity, not repetition -- looping the same lens adds nothing. The loop belongs
on the tail, where the question stops being "what else should this cover?" and
becomes "is it done?", which needs an independent answer.

A loop that exits when a judge reports zero issues pushes in exactly one
direction: toward the judge reporting nothing. Four guards separate a real
convergence from a manufactured one, and the skill documents why each exists:

- the reviewer is never the reviser, and is told it did not write the plan
- the cap is a reported outcome (cap-reached means blockers survived), not a
  silent stop that would state the opposite of the truth
- a round must produce a delta; blockers standing plus an unchanged plan is a
  stall, and re-running only re-litigates the same disagreement
- a first-round zero on a 250-400 line plan is flagged for a human as suspicious
  rather than banked as success

Findings reuse the severity vocabulary from the self-review-relay skill, with
`section` in place of `file`/`line`, so the two are comparable.

Verified by extracting the script and exercising the loop with stubbed agents
across all five paths (clean, converged-after-revision, cap-reached, stalled,
first-round-zero). That caught two real bugs before commit: the loop variable
shadowed the existing `plan` prompt-builder, and finalVersion counted rounds
rather than accepted revisions, so a stalled run would have written a -v5.md
file containing v4's content.
bjagg added 2 commits August 25, 2026 14:05
The convergence tail exited on the FIRST review that found no blockers. That
declares a plan done on one opinion, and spec-forge already had the rule that
says why it's not enough (orchestration/process.md, "Loop control", June 2026):

    the open-gap set is empty for two consecutive audit passes
    (loop-until-dry -- a single clean pass isn't enough)

Ported. A clean pass now increments a streak and the same unchanged plan goes
back to a fresh reviewer; convergence needs two in a row. That is requiring two
reviewers to agree, not shopping until one says yes -- the plan is never revised
between the two passes, so the second reviewer is a second sample, not a retry.

The dry run shows why it matters: in the clean -> blocker -> clean -> clean case
the old code exited after the first clean pass and shipped a plan the very next
reviewer found a blocker in.

Replaces the old first-round-zero guard with a stronger one. A single zero-finding
round is no longer special-cased, because two clean passes already cover it; what
is now flagged is a run where NO reviewer found anything at any severity, which
measures agreement between two weak reviewers rather than plan quality. That
reports as clean-but-unverified.

Caps move from 2 rounds to MAX_REVIEWS = 4, since two clean passes plus a
revision cycle no longer fits in two. cap-reached now also covers the subtle case
where a lone clean pass was followed by a reviewer that found more.

Verified by re-running the stubbed dry run over six paths: two-clean, streak-reset,
one-clean-then-cap, blockers-throughout, stalled, and all-zero.
The relay was one shot. It told you what one round of reviewers saw in one
version of the diff, then stopped -- and the only nod to iteration was an
informal "offer to fix the blockers, then re-run the relevant lens", with no cap,
no fresh-reviewer rule, and no definition of done.

Adds an explicit convergence loop, driven by the main loop since the relay
reviews and never edits. Exit requires BOTH: zero blockers on two consecutive
relay passes, and ruff + ty + pytest green. Capped at 3 relay runs; hitting the
cap means not ready, and the skill says to report it that way.

The two-condition exit is the point, and it is deliberately NOT a copy of
multi-agent-plan. A plan has no oracle, so a judge loop is all there is. A diff
has one, and the two catch different things: the gate catches a failing test or a
type error the judge cannot verify; the judge catches scope creep, a missing
tenant-isolation argument, an undocumented env var, a silent fallback that passes
every test. A green suite is not a clean review and a clean review is not a green
suite. Also states the cheap ordering rule -- if the gate is red, fix it before
spending judges on a diff nobody will ship.

Two consecutive clean passes comes from spec-forge (orchestration/process.md,
"Loop control"): a single clean pass is one opinion, not evidence. When the diff
did not change between passes the second relay is an independent second sample,
which is requiring two panels to agree rather than re-rolling until one says yes.

The synthesis agent now returns a structured verdict (blockers / shouldFix / nits)
instead of prose, because the loop branches on blockers.length and a verdict
buried in a paragraph cannot be branched on reliably. Reuses the lens FINDINGS
item schema so severities stay consistent end to end.
@bjagg bjagg changed the title docs: converge multi-agent-plan on a fresh-reviewer pass docs: add convergence loops to multi-agent-plan and self-review-relay Aug 25, 2026
@bjagg

bjagg commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Update — scope now covers both skills, plus a correction to my own first commit

Two commits were added after the initial review request. Both are worth reading separately.

1. multi-agent-plan exited too early — fixed (c9ae435)

My first commit had the loop exit on the first review that found no blockers. That declares a plan done on a single opinion, and spec-forge/orchestration/process.md:70 has had the rule that says why since June 2026:

the open-gap set is empty for two consecutive audit passes (loop-until-dry — a single clean pass isn't enough)

Now ported properly: a clean pass increments a streak, the unchanged plan goes back to a fresh reviewer, and convergence needs two in a row. The plan is never revised between those two passes, so the second reviewer is a second sample, not a retry — requiring two reviewers to agree, not shopping until one says yes.

The dry run shows the cost of the original. In the clean → blocker → clean → clean case the old code exited after the first clean pass and shipped a plan the very next reviewer found a blocker in.

Knock-ons: cap moves from 2 rounds to MAX_REVIEWS = 4 (two clean passes plus a revision cycle no longer fits in two); the old first-round-zero guard is replaced by a stronger clean-but-unverified outcome for a run where no reviewer ever found anything at any severity — that measures agreement between two weak reviewers, not plan quality.

Re-verified across six paths: two-clean, streak-reset, one-clean-then-cap, blockers-throughout, stalled, all-zero.

2. self-review-relay gets the same loop — with one deliberate difference (6a2a4fc)

It was one-shot, with an informal "offer to fix the blockers, then re-run the relevant lens" and no cap, no fresh-reviewer rule, and no definition of done.

Now: exit requires both zero blockers on two consecutive relay passes and ruff + ty + pytest green. Capped at 3 relay runs.

Both conditions, deliberately not a copy of multi-agent-plan. A plan has no oracle, so a judge loop is all there is. A diff has one, and the two catch different things — the gate catches a failing test or type error the judge can't verify; the judge catches scope creep, a missing tenant-isolation argument, an undocumented env var, a silent fallback that passes every test. A green suite is not a clean review, and a clean review is not a green suite.

Also adds the cheap ordering rule: if the gate is red, don't spend judges on a diff nobody will ship.

The synthesis agent now returns a structured verdict (blockers / shouldFix / nits) instead of prose, because the loop branches on blockers.length and a verdict buried in a paragraph can't be branched on reliably. It reuses the lens FINDINGS item schema so severities stay consistent end to end.

Reviewer note

The thing most worth checking is whether two consecutive clean passes is the right cost/benefit for self-review-relay specifically. It's 3 relay runs × 4 agents worst case, and unlike a plan, the diff already has an independent oracle — so a reasonable person could argue one clean pass plus a green gate is enough there. I chose consistency with spec-forge's rule; I'd take the argument the other way.

@bjagg

bjagg commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Dereck — you now have two from me. Take #1167 first; it's the one blocking a live demo outage. This one has no deadline at all.

What to skip here: the four lens prompts (v1–v4) and the three relay lens prompts are unchanged from what's already on main. Don't re-read them.

What's actually new is the loop control in multi-agent-plan (roughly lines 190–250 of the script block) and the "Convergence loop" section in self-review-relay. Everything else is prose explaining why.

The one question I'd most like your read on is in the addendum above: is two consecutive clean passes worth it for self-review-relay? Worst case that's 3 relay runs × 4 agents on an artifact that already has an oracle (pytest/ruff/ty). I went with consistency against spec-forge's rule, but "one clean pass plus a green gate" is a defensible answer for a diff and I'd rather you push on it than rubber-stamp it.

Context you might find useful more than the diff: the second commit exists because my first one got this wrong — I exited the loop on the first clean review, when a skill already in ~/.claude/skills/spec-forge/ had documented since June that one clean pass isn't enough. Same failure the change is meant to prevent, made while writing it. If the guards read as over-explained, that's why — they're there so the next person doesn't repeat it.

No rush, and a "this isn't worth the agents" verdict is a perfectly good outcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants