From 888fb922bc5f2280f5fa3ba458800c02acaef78a Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 11:49:34 +0200 Subject: [PATCH 1/7] Make swarm --fix/--loop design-finding-aware (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two UX bugs where kind:"design" findings interact badly with the --fix/--loop paths, which were written for defects: - --fix re-confirm was defect-shaped: a design suggestion has no line-local defect to re-find, so an agreed (✅) design fix got silently reported skipped-stale and never applied. Step 1 now branches on kind — for kind:"design" it re-confirms the suggestion still applies (reuse target/duplication/simpler form/waste still present), only skipping when that target is genuinely gone. - --loop never converged on design churn: the tally made no defect/design distinction, so once only design suggestions remained (F>0 ∧ A>0 ∧ C>0 every round, each simplification spawning a fresh one) it ran to the cap. loop-closeout.py step gains --defects D and a new design-only reason (fixed order: after no-change, before cap); --pending is now defect-scoped (design never holds the loop open). Omitting --defects disables the reason (legacy callers unaffected). test_loop_closeout.py extended for the design-only ordering, pending-suppression, and range-check. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- plugins/swarm/scripts/loop-closeout.py | 53 +++++++++++++++------ plugins/swarm/scripts/test_loop_closeout.py | 19 ++++++++ plugins/swarm/skills/review/SKILL.md | 48 ++++++++++++++----- 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/plugins/swarm/scripts/loop-closeout.py b/plugins/swarm/scripts/loop-closeout.py index 808d45a..f918f87 100755 --- a/plugins/swarm/scripts/loop-closeout.py +++ b/plugins/swarm/scripts/loop-closeout.py @@ -16,17 +16,22 @@ there is no working-directory footgun. Usage: - loop-closeout.py step --round R --cap N --findings F --agreed A --changed C [--pending P] - -> prints `continue` or `terminate=` (one of the 4 reasons). - --pending P = agreed findings still awaiting a user decision (default 0); + loop-closeout.py step --round R --cap N --findings F --agreed A --changed C [--defects D] [--pending P] + -> prints `continue` or `terminate=` (one of the 5 reasons). + --defects D = defect-kind findings this round (design suggestions excluded); + when given, the loop converges via `design-only` once no defects remain, so + subjective design churn (each applied simplification spawning a fresh one) + can't run the loop to the cap. Omit to disable that reason (legacy callers). + --pending P = *defect* findings still awaiting a user decision (default 0); P > 0 keeps the loop alive past every convergence reason except the cap. + Design needs-decision does NOT count here — design never holds the loop open. On bad input it writes to stderr and exits non-zero with NO stdout token — the caller must treat a non-zero exit as abort, not as `continue`. loop-closeout.py box "15 7 4 4 5 4 3 1 1 0" --reason cap -> prints the close-out box + the termination reason line. -Reasons: 0-findings | nothing-agreed | no-change | cap +Reasons: 0-findings | nothing-agreed | no-change | design-only | cap """ import argparse import sys @@ -37,6 +42,7 @@ "0-findings": "0 findings — converged clean", "nothing-agreed": "nothing agreed — only disagreements (❌) left open", "no-change": "no files changed — fixed point reached", + "design-only": "no defect findings remain — design suggestions are advisory", "cap": "cap reached", } @@ -48,31 +54,45 @@ def cmd_step(a: argparse.Namespace) -> int: 1. review returned zero findings -> 0-findings (clean) 2. nothing was agreed (no ✅/🟨 this round) -> nothing-agreed 3. no files changed AND nothing is pending -> no-change (fixed point) - 4. this was the last allowed round -> cap + 4. no DEFECT findings remain (design only) -> design-only (advisory tail) + 5. this was the last allowed round -> cap Otherwise: continue. - Any agreed finding still awaiting a user decision (`--pending` > 0) keeps the - loop alive: it suppresses ALL THREE convergence reasons (0-findings, - nothing-agreed, no-change), because none of them is a true fixed point while a - choice is still owed. Only `cap` — the safety stop — can still fire with a - decision pending. Inputs are range-checked so a mis-parsed flag (e.g. - `--cap 0`) fails loudly instead of silently collapsing the loop. + `design-only` (only when `--defects` is given) is what stops the loop from + running to the cap on subjective design churn: design suggestions are + advisory, and each applied simplification can spawn a fresh one, so once no + defect-kind finding remains the loop has converged on the part that matters — + the design tail doesn't hold it open. Omitting `--defects` disables this + reason (legacy behavior: only the other four fire). + + Any DEFECT finding still awaiting a user decision (`--pending` > 0) keeps the + loop alive: it suppresses ALL FOUR convergence reasons (0-findings, + nothing-agreed, no-change, design-only), because none of them is a true fixed + point while a defect choice is still owed. Only `cap` — the safety stop — can + still fire with a decision pending. A *design* needs-decision is NOT passed as + pending (design never holds the loop). Inputs are range-checked so a mis-parsed + flag (e.g. `--cap 0`) fails loudly instead of silently collapsing the loop. """ - for name, val, lo in ( + checks = [ ("--cap", a.cap, 1), ("--round", a.round, 0), ("--findings", a.findings, 0), ("--agreed", a.agreed, 0), ("--changed", a.changed, 0), ("--pending", a.pending, 0), - ): + ] + if a.defects is not None: + checks.append(("--defects", a.defects, 0)) + for name, val, lo in checks: if val < lo: print(f"loop-closeout: {name} must be >= {lo} (got {val})", file=sys.stderr) return 2 - converged = a.pending <= 0 # a pending decision is never a fixed point + converged = a.pending <= 0 # a pending defect decision is never a fixed point if converged and a.findings <= 0: print("terminate=0-findings") elif converged and a.agreed <= 0: print("terminate=nothing-agreed") elif converged and a.changed <= 0: print("terminate=no-change") + elif converged and a.defects is not None and a.defects <= 0: + print("terminate=design-only") elif a.round + 1 >= a.cap: print("terminate=cap") else: @@ -133,8 +153,11 @@ def main() -> int: s.add_argument("--findings", type=int, required=True, help="findings this round") s.add_argument("--agreed", type=int, required=True, help="✅+🟨 findings this round") s.add_argument("--changed", type=int, required=True, help="files changed this round") + s.add_argument("--defects", type=int, default=None, + help="defect-kind findings this round (design excluded); enables the " + "design-only reason. Omit to disable it (legacy).") s.add_argument("--pending", type=int, default=0, - help="agreed findings still awaiting a user decision (default 0)") + help="DEFECT findings still awaiting a user decision (default 0)") s.set_defaults(func=cmd_step) b = sub.add_parser("box", help="render the OPEN-findings close-out box") diff --git a/plugins/swarm/scripts/test_loop_closeout.py b/plugins/swarm/scripts/test_loop_closeout.py index f3ee29e..3119f3d 100755 --- a/plugins/swarm/scripts/test_loop_closeout.py +++ b/plugins/swarm/scripts/test_loop_closeout.py @@ -57,9 +57,28 @@ def step(**kw): expect("pending defaults to 0", ["step", *step(round=1, cap=10, findings=5, agreed=2, changed=0)], out="terminate=no-change") +# --- design-only: no defect findings remain -> converge on the advisory tail +# (only when --defects is given; it slots in AFTER no-change, BEFORE cap) --- +expect("design-only fires", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=2, defects=0)], + out="terminate=design-only") +expect("defects>0 continues", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=2, defects=2)], + out="continue") +expect("no-change wins over design-only", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=0, defects=0)], + out="terminate=no-change") +expect("0-findings wins over design-only", ["step", *step(round=1, cap=10, findings=0, agreed=0, changed=2, defects=0)], + out="terminate=0-findings") +expect("design-only omitted = disabled", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=2)], + out="continue") +expect("pending blocks design-only", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=2, defects=0, pending=1)], + out="continue") +expect("cap wins over design-only at last round", ["step", *step(round=9, cap=10, findings=5, agreed=3, changed=2, defects=2)], + out="terminate=cap") +expect("design-only box renders", ["box", "3 2 1", "--reason", "design-only"], code=0) + # --- input range checks: bad values fail loudly (exit 2), no stdout token --- expect("cap<1 rejected", ["step", *step(round=0, cap=0, findings=3, agreed=2, changed=1)], out="", code=2) expect("negative changed rejected", ["step", *step(round=0, cap=10, findings=3, agreed=2, changed=-1)], out="", code=2) +expect("negative defects rejected", ["step", *step(round=0, cap=10, findings=3, agreed=2, changed=1, defects=-1)], out="", code=2) # --- box renders, and rejects malformed / negative counts --- rc, so, se = run(["box", "15 7 4 4 5 4 3 1 1 0", "--reason", "cap"]) diff --git a/plugins/swarm/skills/review/SKILL.md b/plugins/swarm/skills/review/SKILL.md index c807a72..53a2784 100644 --- a/plugins/swarm/skills/review/SKILL.md +++ b/plugins/swarm/skills/review/SKILL.md @@ -416,9 +416,21 @@ Work the ✅/🟨 findings, most severe first. For each: would leak. Report such a finding as ⏭️ skipped-unsafe-path and move on; never open the path to "check." 1. **Re-confirm claim-vs-code** — re-read the cited `file:line` (already confirmed - repo-safe in step 0) and confirm the defect is still there. If it's gone - (already fixed, comment rot, line drift, refactored away) → **skip it, report - as skipped-stale; never fabricate an edit** to justify a stale finding. **Anchor every edit on surrounding + repo-safe in step 0). What you re-confirm is **kind-specific**: + - **`kind:"defect"`** → confirm the **defect is still there**. If it's gone + (already fixed, comment rot, line drift, refactored away) → skip it, report + as skipped-stale. + - **`kind:"design"`** (`[reuse]`/`[simplification]`/`[efficiency]`/`[altitude]`) + → a suggestion has no line-local defect to re-find, so confirm the + **suggestion still applies**: the reuse target still exists / the duplication + is still present / the simpler form is still available and behavior-identical / + the claimed waste is still real. Only when that target is genuinely gone (e.g. + the duplicated block was already removed) is it skipped-stale — do **not** + report an agreed design fix as stale just because there's no defect to see at + the line. + + Either way: **skip stale findings, report as skipped-stale; never fabricate an + edit** to justify one. **Anchor every edit on surrounding content, not the report's raw line number** — an earlier fix in the same file this pass shifts later line numbers; the `Edit` tool matches strings, so re-reading the anchor text before each edit is what keeps a same-file batch @@ -456,7 +468,11 @@ fire while a decision is still pending. Each round: 1. You already hold this round's report (round 0 = step 3; later rounds = the - re-review below). Let `F` = findings, `A` = ✅+🟨 count. + re-review below). Let `F` = findings, `A` = ✅+🟨 count, and `D` = the + **defect-kind** findings (`kind:"defect"`) — design suggestions excluded. `D` + is what drives convergence: design findings are advisory (each applied + simplification can spawn a fresh one), so the loop stops once no defect remains + rather than churning to the cap on subjective design targets. 2. **Fix** — snapshot the tree, run the `--fix` procedure above, then derive `C` (files this round's fixes changed) **deterministically from git, not by hand**: ```sh @@ -471,21 +487,28 @@ Each round: += fixes applied`. Append this round's OPEN count to `OPEN[]`. 3. **Termination decision** — **deterministic arithmetic over judged inputs**: the script's branch logic is fixed. `C` is now git-derived (step 2), but - `F`/`A`/`P` and `OPEN[]` are still your in-session tallies, so a miscount there - feeds a wrong reason in (garbage-in). Count them carefully — especially `P`. Pass - `--pending

` = agreed findings still awaiting a user decision, so a round - that changed no files but has an open decision does **not** false-terminate as - `no-change`: + `F`/`A`/`D`/`P` and `OPEN[]` are still your in-session tallies, so a miscount + there feeds a wrong reason in (garbage-in). Count them carefully — especially + `P`. Pass `--defects ` (defect-kind findings this round) so the loop + converges via `design-only` once no defect remains, and `--pending

` = + **defect** findings still awaiting a user decision, so a round that changed no + files but has an open defect decision does **not** false-terminate as + `no-change`. A **design** needs-decision is NOT counted in `P` — design never + holds the loop open: ```sh python3 "${CLAUDE_PLUGIN_ROOT}/scripts/loop-closeout.py" step \ - --round --cap --findings --agreed --changed --pending

+ --round --cap --findings --agreed --changed --defects --pending

``` Read stdout: `continue` → step 4; `terminate=` → Close-out. **A non-zero exit (with no stdout token) means bad input — abort and surface the stderr message; never treat a missing token as `continue`.** **On `terminate=cap` the last round's fixes were applied but not re-reviewed** (cap fires before step 4) — say so in the close-out summary; the cap is a safety - stop, not a clean bill of health for the final edits. + stop, not a clean bill of health for the final edits. **On + `terminate=design-only`** the round's defect count hit zero but design + suggestions may remain (they were applied in this round's fix step, not + re-reviewed) — say so: the loop converged on defects, the design tail is + advisory. 4. **Re-review** — re-run steps 1–3 (Prepare diff → Workflow → Present) on the **new** working tree. Two guards before spending another (possibly `--max`) ensemble pass: @@ -647,7 +670,8 @@ post. Do **not** re-implement the sanitize/gate/post logic inline. against the code first (stale findings are skipped, not fabricated), 🟨 applies the session's own variant, and a finding with more than one good fix asks the user which path. `--loop[=N]` re-reviews after each fix round until it - converges (0 findings · nothing agreed · no files changed · cap, default 10). + converges (0 findings · nothing agreed · no files changed · no defects left + (design tail is advisory) · cap, default 10). The deterministic loop bits (termination decision, close-out box) live in `scripts/loop-closeout.py`, not this prose. - **Reviewing a PR** (`--pr []`): the *same* pipeline runs against the From 9afcf72857a4be95510942c7acb2046be0a07b99 Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 11:54:26 +0200 Subject: [PATCH 2/7] Infer design kind for untagged cluster findings (P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In default per-cluster mode a design-cluster finder that dropped its [lens] prefix fell back to 'unspecified' → kind:"defect", so a real design suggestion rendered in the defect table, skipped applicability verify, and (since P1b) inflated the --loop defect tally that drives convergence — reintroducing the churn P1b closes. The old guard rejected inference because "guessing lenses[0] could stamp a design kind on an off-lens defect". That collapsed a real distinction: a Claude finder reviews through ONE kind-homogeneous cluster, so its kind is recoverable even when the exact lens isn't; externals carry the full 11-lens (kind-mixed) set or none, so they stay 'unspecified'/defect and keep NOT voting. - pool: untagged finding from a homogeneous Claude unit inherits that unit's kind and is marked kindTrusted; lens stays 'unspecified' (we recover the kind, not a lens we can't know). - cluster kind vote uses kindTrusted members; untaggedOnly stays lens-tagged-based, so the consensus/auto-accept invariant is untouched — only design/defect routing is corrected. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- plugins/swarm/workflows/swarm-review.js | 64 +++++++++++++++++-------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/plugins/swarm/workflows/swarm-review.js b/plugins/swarm/workflows/swarm-review.js index 4cee982..2fd65d8 100644 --- a/plugins/swarm/workflows/swarm-review.js +++ b/plugins/swarm/workflows/swarm-review.js @@ -374,17 +374,34 @@ for (const v of voices) { // set, not the finder's own subset: off-lens bleed is real (a design finder // may spot a genuine [security] bug while reading), and coercing a validly // tagged foreign lens would flip `kind` and route the finding through the - // wrong verifier + report section. An unknown/missing prefix falls back to - // the finder's lens ONLY for a single-lens unit (--max — the assignment is - // unambiguous); a multi-lens cluster finding falls back to 'unspecified' - // (kind defect, the safe bucket) — guessing lenses[0] could stamp a design - // kind on an untagged off-lens defect from the design cluster. + // wrong verifier + report section. + // + // When the prefix is unknown/missing, the exact lens can't be recovered — but + // the KIND often can, and that's what routes a finding (design table + + // applicability verify vs defect ranking + P1b's --loop defect tally). A + // Claude finder reviews through ONE cluster whose lenses are single-KIND + // (breakage/threat/consistency = all defect, design = all design), so an + // untagged finding from it inherits that kind reliably (`kindTrusted`) — a + // dropped [reuse] prefix must not sink a design suggestion into the defect + // table. Externals carry the full 11-lens set (or none), so their unit is + // kind-MIXED → never inferred: an untagged external stays 'unspecified'/defect + // (the safe bucket) and keeps NOT voting on cluster kind (the load-bearing + // invariant). The lens itself stays 'unspecified' unless it was a --max + // single-lens unit — we recover the kind, not a specific lens we can't know. const m = /^\s*\[([\w-]+)\]/.exec(f.summary || '') let lens = m ? m[1].toLowerCase() : '' - if (!CANDIDATE_LENSES.includes(lens)) { - lens = Array.isArray(v.lenses) && v.lenses.length === 1 ? v.lenses[0] : 'unspecified' + let kind, kindTrusted + if (CANDIDATE_LENSES.includes(lens)) { + kind = lensKind(lens); kindTrusted = true + } else if (Array.isArray(v.lenses) && v.lenses.length === 1) { + lens = v.lenses[0]; kind = lensKind(lens); kindTrusted = true // --max: unambiguous lens + } else { + lens = 'unspecified' + const unitKinds = new Set((Array.isArray(v.lenses) ? v.lenses : []).map(lensKind)) + kindTrusted = unitKinds.size === 1 // homogeneous Claude unit only + kind = kindTrusted ? [...unitKinds][0] : 'defect' } - pool.push({ ...f, backend: v.backend, family: FAMILY[v.backend] || v.backend, lens, kind: lensKind(lens) }) + pool.push({ ...f, backend: v.backend, family: FAMILY[v.backend] || v.backend, lens, kind, kindTrusted }) } } log(`Fan-out: ${pool.length} raw findings from ${voices.length} voices` + @@ -433,18 +450,25 @@ if (pool.length > 0) { const backends = Array.from(new Set(members.map((i) => pool[i].backend))).sort() const families = Array.from(new Set(members.map((i) => pool[i].family))).sort() // Cluster kind from the MEMBERS (not the merge agent's free-text `lens`): - // design only when every TAGGED member is design — 'defect' is the single - // structural fallback (a design suggestion merged with a real defect must - // not drop out of the defect ranking). 'unspecified' members (untagged - // externals) do NOT vote: their kind is only the safe default, and one - // untagged voice must not drag a properly tagged design cluster into the - // defect ranking. An ALL-untagged cluster is kind 'defect' but flagged — - // its "consensus" is backed by no tagged lens, so it must never be - // auto-accepted (see needsVerify below): two diff-scoped externals can - // agree on an unverifiable suggestion without ever tagging it. - const known = members.filter((i) => pool[i].lens !== 'unspecified') - const kind = known.length > 0 && known.every((i) => pool[i].kind === 'design') ? 'design' : 'defect' - const untaggedOnly = known.length === 0 + // design only when every KIND-TRUSTED member is design — 'defect' is the + // single structural fallback (a design suggestion merged with a real defect + // must not drop out of the defect ranking). A member is kind-trusted when its + // kind is provenance-known: explicitly tagged, OR inferred from a homogeneous + // Claude unit (see the pool loop). Kind-UNtrusted members (untagged externals, + // whose 11-lens unit is kind-mixed) do NOT vote — one untagged external must + // not drag a properly tagged design cluster into the defect ranking, nor a + // homogeneous Claude design finding out of it. + // + // `untaggedOnly` is separate and stays LENS-tagged-based: an ALL-untagged + // cluster (no member carries a lens tag) is flagged so it is never + // auto-accepted (see needsVerify) — two diff-scoped externals can agree on an + // unverifiable suggestion without ever tagging it. An inferred-design finding + // still carries no lens tag, so this gate is untouched: the consensus / + // auto-accept invariant is unchanged — only the design/defect routing is + // corrected. + const voters = members.filter((i) => pool[i].kindTrusted) + const kind = voters.length > 0 && voters.every((i) => pool[i].kind === 'design') ? 'design' : 'defect' + const untaggedOnly = members.every((i) => pool[i].lens === 'unspecified') // The merge agent's `lens` is free text (schema caps length, not values): // accept it ONLY when it is a known lens AND some cluster member actually // carries it — a globally-valid lens no member tagged (merge says `security` From 7e094e2cb20f6b5c3ad9d709a2e6d0f64b3dc1f9 Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 11:57:29 +0200 Subject: [PATCH 3/7] Guard pr-post against design-row [lens] double-prefix (P3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render_body unconditionally prepended "[lens] " to design rows. The workflow doesn't strip a finder's own "[lens] " self-tag from the summary, and a merge can leave the representative summary tagged with a different design lens than the cluster's resolved lens — so a row could post as "[reuse] [reuse] …" or "[reuse] [simplification] …". Add an idempotency guard keyed on a DESIGN_LENS_TAGS tuple (mirrored from LENS_CLUSTERS.design, sync-checked by test_lens_sync.py): if the cell already opens with a known design-lens tag, keep it and don't add a second. The tuple is NOT a kind fallback (the retired DESIGN_LENSES was) — it runs only inside the already-decided design branch and can never move a row between tables. A non-lens bracket ([TODO] …) is not mistaken for a self-tag. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- plugins/swarm/scripts/pr-post.py | 27 +++++++++++++++++++++++-- plugins/swarm/scripts/test_lens_sync.py | 12 +++++++++++ plugins/swarm/scripts/test_pr_post.py | 12 +++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/plugins/swarm/scripts/pr-post.py b/plugins/swarm/scripts/pr-post.py index 3ef24b6..6ef8348 100755 --- a/plugins/swarm/scripts/pr-post.py +++ b/plugins/swarm/scripts/pr-post.py @@ -196,6 +196,18 @@ def _safe_pr_num(pr_num): return sanitize_prose(pr_num) +# Design-cluster lens names, mirrored from swarm-review.js LENS_CLUSTERS.design +# (kept in sync by test_lens_sync.py). NOT a kind fallback (the retired +# DESIGN_LENSES was — bucketing keys on `kind` ALONE, see _row_kind): this runs +# only INSIDE the already-decided design branch, purely to recognize a "[lens] " +# self-tag the finder may already have baked into the summary, so we don't add a +# second prefix. It can never move a row between the defect/design tables. +DESIGN_LENS_TAGS = ("reuse", "simplification", "efficiency", "altitude") +_DESIGN_PREFIX_RE = re.compile( + r"\s*\[(" + "|".join(DESIGN_LENS_TAGS) + r")\]\s", re.IGNORECASE +) + + def _row_kind(r: dict) -> str: """Row kind. 'design' ONLY when explicitly tagged `kind: "design"`; anything else — including a missing/junk kind — is a defect, the safe bucket. The lens @@ -258,10 +270,21 @@ def render_body(data: dict) -> str: # sanitizing: the brackets come out entity-encoded like any other # cell content and render back as literal [lens] — an untrusted # lens value gets the full sanitizer, same as the finding text. + # Idempotency guard: the workflow doesn't strip a finder's own + # "[lens] " self-tag from the summary, and a merge can leave the + # representative summary tagged with a DIFFERENT design lens than the + # cluster's resolved `lens`. Prefixing unconditionally would post + # "[reuse] [reuse] …" (or "[reuse] [simplification] …"). If the cell + # already opens with a known design-lens tag, keep it and don't add a + # second one. befund = r.get("befund") if _row_kind(r) == "design": - lens = str(r.get("lens") or "").strip() or "design" - befund = f"[{lens}] {'' if befund is None else befund}".rstrip() + body = "" if befund is None else str(befund) + if _DESIGN_PREFIX_RE.match(body): + befund = body # already self-tagged — leave it + else: + lens = str(r.get("lens") or "").strip() or "design" + befund = f"[{lens}] {body}".rstrip() cells = [ sanitize_prose(r.get("num", "")), sanitize_prose(r.get("sev", "")), diff --git a/plugins/swarm/scripts/test_lens_sync.py b/plugins/swarm/scripts/test_lens_sync.py index 305b138..43cfaea 100644 --- a/plugins/swarm/scripts/test_lens_sync.py +++ b/plugins/swarm/scripts/test_lens_sync.py @@ -92,6 +92,18 @@ def check(name, cond): methodological == set(clusters.get("breakage", [])) - TOPICAL_BREAKAGE, ) +# pr-post.py DESIGN_LENS_TAGS mirror: the publish path prefixes design rows with +# "[lens] " and guards against doubling an already-present tag; that guard keys +# on this hand-mirrored tuple, so a design lens added to the cluster but not here +# would slip past the guard and could re-post as "[reuse] [reuse] …". +PR_POST = PLUGIN / "scripts" / "pr-post.py" +pp = PR_POST.read_text(encoding="utf-8") +dm = re.search(r"DESIGN_LENS_TAGS = \(([^)]*)\)", pp) +check("pr-post: DESIGN_LENS_TAGS found", dm) +pr_design = set(re.findall(r"'([a-z][a-z-]*)'|\"([a-z][a-z-]*)\"", dm.group(1) if dm else "")) +pr_design = {a or b for a, b in pr_design} +check("pr-post DESIGN_LENS_TAGS == design cluster", pr_design == set(clusters.get("design", []))) + if FAILS: print("lens-sync tests FAILED:", file=sys.stderr) for f in FAILS: diff --git a/plugins/swarm/scripts/test_pr_post.py b/plugins/swarm/scripts/test_pr_post.py index 315e278..0b89c88 100644 --- a/plugins/swarm/scripts/test_pr_post.py +++ b/plugins/swarm/scripts/test_pr_post.py @@ -163,6 +163,18 @@ def header_row(rendered): evil = pr_post.render_body({"rows": [{"num": "1", "befund": "z", "kind": "design", "lens": "x|@y"}]}) check("evil lens sanitized", "[x|@y] z" in evil and "@y" not in evil) +# idempotency guard: a befund that already carries its own "[lens] " self-tag is +# NOT prefixed again (would post "[reuse] [reuse] …"). Guard is case-insensitive +# and covers a self-tag that differs from the row's resolved lens (merge picked a +# different design lens as representative). +dup = pr_post.render_body({"rows": [{"num": "1", "befund": "[reuse] use the helper", "kind": "design", "lens": "reuse"}]}) +check("no double [reuse] prefix", "[reuse] use the helper" in dup and dup.count("[reuse]") == 1) +dup2 = pr_post.render_body({"rows": [{"num": "1", "befund": "[Simplification] flatten it", "kind": "design", "lens": "reuse"}]}) +check("self-tag kept, row lens not re-added", "[Simplification] flatten it" in dup2 and "[reuse]" not in dup2) +# a non-lens bracket prefix is NOT mistaken for a self-tag — the lens is still added +todo = pr_post.render_body({"rows": [{"num": "1", "befund": "[TODO] drop this", "kind": "design", "lens": "reuse"}]}) +check("non-lens bracket still gets lens", "[reuse] [TODO] drop this" in todo) + # --- build subcommand round-trip via subprocess ---------------------------- # with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f: json.dump(DATA, f) From 716c3548977ab0cea6995d0f13ba9080fa2a04e3 Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 12:55:12 +0200 Subject: [PATCH 4/7] Add in-session design lens prefix + bump swarm to 0.5.1 - P3: the in-session Design table gains a [lens] Befund prefix, matching the PR-comment path so lens attribution reads identically on both. - P3: document the cluster-default's loss of per-lens failure isolation as an accepted tradeoff (--max restores it; a crash is a visible backendError). - Update the swarm-review-pipeline knowledge entry for the 0.5.1 design-lens hardening (P1 --fix/--loop, P2 kind inference, P3 guard + attribution + isolation tradeoff). - CHANGELOG 0.5.1 entry; bump plugin.json + marketplace.json. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- .claude-plugin/marketplace.json | 2 +- .../features/swarm-review-pipeline.md | 59 +++++++++++++++---- CHANGELOG.md | 8 +++ plugins/swarm/.claude-plugin/plugin.json | 2 +- plugins/swarm/skills/review/SKILL.md | 8 ++- 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 98252a4..c7c33cd 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -30,7 +30,7 @@ "name": "swarm", "source": "./plugins/swarm", "description": "Local mixture-of-agents code review for Claude Code. Fans a diff across Claude lenses plus the codex and grok CLIs (grok-4.5), merges by mechanism with cross-family consensus, verifies solo findings and all design suggestions, and presents one ranked report. Optional --fix / --loop applies the findings you agreed with; --pr reviews a GitHub PR diff and posts the result. Skills: /swarm:review, /swarm:agents.", - "version": "0.5.0" + "version": "0.5.1" } ] } diff --git a/.claude/knowledge/features/swarm-review-pipeline.md b/.claude/knowledge/features/swarm-review-pipeline.md index 651aa9a..2c8edd6 100644 --- a/.claude/knowledge/features/swarm-review-pipeline.md +++ b/.claude/knowledge/features/swarm-review-pipeline.md @@ -1,9 +1,9 @@ --- title: "Swarm Review Pipeline (/swarm:review)" createdAt: 2026-07-08 -updatedAt: 2026-07-17 +updatedAt: 2026-07-18 createdFrom: "PR #24" -updatedFrom: "PR #35" +updatedFrom: "harden-swarm-design-lens-integration" pluginVersion: 1.8.2 prime: false reindexedAt: 2026-07-12 @@ -39,14 +39,26 @@ truth** (the per-cluster externals follow-up consumes it): one broad pass → default = per-cluster → `--max` = per-lens. The **gate stays per-lens** (a fully-pruned cluster spawns no agent); design lenses are first-class in the gate prompt, skipped only when the diff can't pay off. + **Accepted tradeoff of the cluster default:** per-lens failure isolation is + gone — one crashed cluster finder drops that whole cluster's Claude coverage + for the round (a visible `backendError`, never silent); `--max` restores + per-lens isolation. Documented, not retried per-lens (minimal — the default + trades isolation for fewer agents). - **`kind` is derived from the lens name** (`design` vs `defect`) — no finding-schema change, so the 3-place schema mirror is untouched. A merged - cluster's kind comes from its TAGGED members (design only when every tagged - member is design — a design suggestion merged with a real defect must not - leave the defect ranking); untagged (`unspecified`) members don't vote, and - an **all-untagged cluster is never auto-accepted** — its "consensus" is - backed by no lens, so it is verified like a solo (the `--max` dogfooding - round found exactly this hole). + cluster's kind comes from its KIND-TRUSTED members (design only when every + trusted member is design — a design suggestion merged with a real defect must + not leave the defect ranking). **Kind-trusted = provenance-known:** explicitly + tagged, OR (0.5.1) inferred from a homogeneous Claude finder unit — a Claude + finder reviews through ONE kind-single cluster, so an untagged finding from it + still has a reliable kind (a dropped `[reuse]` prefix must not sink a design + suggestion into the defect table + the `--loop` defect tally). Externals carry + the full 11-lens (kind-mixed) set, so they're never inferred — an untagged + external stays `unspecified`/defect and does NOT vote. `untaggedOnly` stays + lens-tag-based, so an **all-untagged cluster is never auto-accepted** — its + "consensus" is backed by no lens, verified like a solo (the `--max` dogfooding + round found exactly this hole); the 0.5.1 kind-inference corrects routing only, + never auto-accept. - **Verify path decision: kind-aware prompt, not bypass.** Design findings are suggestion-shaped, but each has a falsifiable applicability core (reuse target exists? simpler form behavior-identical? claimed waste real?) — the @@ -60,7 +72,10 @@ truth** (the per-cluster externals follow-up consumes it): - **Report keeps kinds apart**: defects table first, then a same-format `Design` table (shared numbering — the workflow sorts defects first); `balance.design` counts the subset. Lens prefixes are parsed with `[\w-]` - (hyphenated names like `removed-behavior` — plain `\w` misses them). + (hyphenated names like `removed-behavior` — plain `\w` misses them). The Design + table has no lens column; instead each design row's finding cell is prefixed + `[lens]` (0.5.1) — the SAME in the in-session table and the PR comment, so lens + attribution reads identically on both surfaces. - Effort: design lenses run at the **same effort** as defect lenses (`xhigh` under `--max` — user call: depth applies to design thinking too). @@ -133,16 +148,26 @@ the diff out of the script, above). Claude applies edits between rounds. ❌-disagree is never touched and stays visible in the report. - **Re-confirm claim-vs-code before every edit** — a stale finding (comment rot, already-fixed, line drift) is reported as skipped, never fabricated into an edit. + **Kind-aware (0.5.1):** a `kind:"design"` finding has no line-local defect to + re-find, so re-confirm the *suggestion still applies* (reuse target / duplication + / simpler form / waste still present), not "defect still present" — else an + agreed design fix is silently dropped as skipped-stale. - **Deterministic bits live in `scripts/loop-closeout.py`, not skill prose** (per the project's prose-drift memory — stateful skill logic drifts as - prose): `step` = the 4-part termination decision in **fixed order** (0-findings - / nothing-agreed / no-change / cap, default 10), `box` = the OPEN-findings + prose): `step` = the termination decision in **fixed order** (0-findings / + nothing-agreed / no-change / **design-only** / cap, default 10), `box` = the OPEN-findings close-out visualization that **shows a legitimate rise** (a fix surfaced new findings) instead of hiding it. Stateless — Claude passes the per-round counts in; no state file, so no cwd footgun. The determinism is **the arithmetic, not - the inputs**: `F/A/C/pending/OPEN[]` are Claude's in-session tallies, so a + the inputs**: `F/A/C/D/pending/OPEN[]` are Claude's in-session tallies, so a miscount still feeds a wrong reason in (garbage-in) — the script can't make a judged count reproducible, only the branch logic over it. + **`design-only` (0.5.1, via `--defects D`):** design suggestions are subjective + and self-spawning (each applied simplification surfaces a fresh one), so a + defect/design-blind tally ran the loop to the cap on them. The loop now + converges once no *defect* finding remains — design is advisory and never holds + it open; `--pending` is defect-scoped for the same reason. Omitting `--defects` + disables the reason (legacy callers see the original four). - Loop mechanics mirror pr-flow `/cycle` run locally (no push / no `@claude` poll); the `Status` column (🔧/⏭️/🔁) and stable `#` across rounds come from the report table contract this entry defines above (P2 reserved them). @@ -211,7 +236,15 @@ filled* — `gh pr diff ` (bare `--pr` resolves the current branch's PR via 0.5.0 moved one more rule from prose to code the same way: rows carry optional `kind`/`lens`, and the script orders defect rows before design rows and prefixes design finding cells with `[lens]` — the caller passes findings - through verbatim, never hand-orders or hand-prefixes. + through verbatim, never hand-orders or hand-prefixes. 0.5.1 added an + **idempotency guard**: the finder's summary may already carry its own + `[lens]` self-tag (the workflow doesn't strip it, and a merge can leave a + *different* design lens as the representative), so prefixing unconditionally + posted `[reuse] [reuse] …` / `[reuse] [simplification] …`. If the cell already + opens with a known design-lens tag (`DESIGN_LENS_TAGS`, sync-checked by + `test_lens_sync.py`) it's kept as-is. Note it is **not** a kind fallback (the + retired `DESIGN_LENSES` was): it runs only inside the already-`kind`-decided + design branch and never moves a row between tables. ## Future idea (P3+): per-cluster external prompts diff --git a/CHANGELOG.md b/CHANGELOG.md index ca7a096..28801af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -193,6 +193,14 @@ entries are grouped per plugin, newest first. ## swarm +### 0.5.1 — 2026-07-18 +- Make `--fix` re-confirm design-aware: a `kind: "design"` finding has no line-local defect to re-find, so an agreed (✅) design fix was silently reported skipped-stale and never applied. Step 1 now branches on kind — for design findings it re-confirms the suggestion still applies (reuse target / duplication / simpler form / waste still present), only skipping when that target is genuinely gone. +- Make `--loop` converge on design churn: the tally made no defect/design distinction, so once only design suggestions remained (each applied simplification spawning a fresh one) it ran to the cap. `loop-closeout.py step` gains `--defects D` and a new `design-only` reason (fixed order: after no-change, before cap); `--pending` is now defect-scoped (design never holds the loop open). Omitting `--defects` disables the reason (legacy callers unaffected). +- Fix untagged design-cluster findings mis-routing to defect: a design finder that dropped its `[lens]` prefix fell back to `unspecified` → `kind: "defect"`, so a real suggestion rendered in the defect table, skipped applicability verify, and inflated the new `--loop` defect tally. A Claude finder reviews through one kind-homogeneous cluster, so its kind is now inferred (`kindTrusted`) even when the exact lens isn't; externals (kind-mixed 11-lens unit) stay `unspecified`/defect and keep not voting — the consensus/auto-accept invariant is untouched, only design/defect routing is corrected. +- Guard `pr-post.py` against a design-row `[lens]` double-prefix (`[reuse] [reuse] …`): if the finding cell already opens with a known design-lens self-tag, keep it and don't add a second (`DESIGN_LENS_TAGS`, sync-checked by `test_lens_sync.py`; not a kind fallback). +- Show the lens in the in-session Design table too: design rows now carry a `[lens]` `Befund` prefix, matching the PR-comment path (both surfaces read identically). +- Accept the cluster-default's loss of per-lens failure isolation as a documented tradeoff (one crashed cluster finder drops that cluster's Claude coverage as a visible `backendError`; `--max` restores per-lens isolation). + ### 0.5.0 — 2026-07-17 - Grow the review lens set from 5 to 11 (all default-on): methodological `removed-behavior` + `cross-file-trace` (factual, normal verify) and design-quality `reuse` / `simplification` / `efficiency` / `altitude` (suggestion-shaped, `kind: "design"`). - Organize the lenses into 4 clusters (`LENS_CLUSTERS` — single source of truth): breakage / threat / design / consistency. Claude fan-out runs one finder per cluster by default (≤4 agents); `--max` splits to one finder per lens (≤11). The gate still prunes per-lens. diff --git a/plugins/swarm/.claude-plugin/plugin.json b/plugins/swarm/.claude-plugin/plugin.json index 3930a5a..3e90b61 100644 --- a/plugins/swarm/.claude-plugin/plugin.json +++ b/plugins/swarm/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "swarm", "description": "Local mixture-of-agents code review for Claude Code. Fans a diff across Claude lenses plus the codex and grok CLIs (grok-4.5), merges by mechanism with cross-family consensus, verifies solo findings and all design suggestions, and presents one ranked report. Optional --fix / --loop applies the findings you agreed with; --pr reviews a GitHub PR diff and posts the result. Skills: /swarm:review, /swarm:agents.", - "version": "0.5.0", + "version": "0.5.1", "author": { "name": "gering" }, diff --git a/plugins/swarm/skills/review/SKILL.md b/plugins/swarm/skills/review/SKILL.md index 53a2784..487759b 100644 --- a/plugins/swarm/skills/review/SKILL.md +++ b/plugins/swarm/skills/review/SKILL.md @@ -306,8 +306,12 @@ findings table holds only `kind: "defect"` rows; when `kind: "design"` findings exist, render them after it as a second table under a short `**Design**` heading, with the SAME columns and budgets as the defect table **in the current round** (seven normally; eight including `Status` in `--loop` re-review rounds — -design rows carry 🔧/⏭️/🔁/🆕 like any other). Severity there reads as -importance, not breakage. Numbering is ONE shared sequence across both tables — +design rows carry 🔧/⏭️/🔁/🆕 like any other). **Prefix each design row's `Befund` +with its `[lens]`** (`[reuse]`/`[simplification]`/`[efficiency]`/`[altitude]`) — +the design table has no lens column, so this is where the lens attribution shows, +mirroring the PR-comment path exactly (both surfaces read the same); skip the +prefix only if the finding text already opens with that tag. Severity there reads +as importance, not breakage. Numbering is ONE shared sequence across both tables — render each finding's workflow-assigned `num` verbatim in round 0 (across `--loop` rounds the `#` column follows the cross-round identity rule below, not the workflow's re-assigned per-round `num`); no design findings → no From 7c9334ab9aa9f8a3940df889ed14ae461a8ae881 Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 14:56:17 +0200 Subject: [PATCH 5/7] Address swarm self-review findings (#1/#2/#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External-only (codex+grok) review of this branch's own diff: - #1: pr-post's design double-prefix guard required trailing whitespace after "[lens]", so a valid but unspaced self-tag ([reuse]text, bare [reuse]) — which the workflow parser accepts — slipped past and got re-prefixed to "[reuse] [reuse]…". Align the regex to the workflow parser (/^\s*\[([\w-]+)\]/, no trailing-space requirement). - #2: the SKILL in-session Design-table skip rule suppressed the prefix only for the row's OWN lens, while pr-post suppresses any known design-lens tag — so a merged row opening with a different member's tag rendered differently on the two surfaces, contradicting the "read identically" claim. Align the wording to the pr-post guard. - #4: the "cap wins over design-only" test used defects=2, so design-only was never eligible and it never exercised the ordering. Replace with the real precedence (design-only wins over cap at the last round, defects=0) + a correctly-named cap-with-defects case. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- plugins/swarm/scripts/pr-post.py | 7 ++++++- plugins/swarm/scripts/test_loop_closeout.py | 8 +++++++- plugins/swarm/scripts/test_pr_post.py | 6 ++++++ plugins/swarm/skills/review/SKILL.md | 7 +++++-- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/plugins/swarm/scripts/pr-post.py b/plugins/swarm/scripts/pr-post.py index 6ef8348..18e94c4 100755 --- a/plugins/swarm/scripts/pr-post.py +++ b/plugins/swarm/scripts/pr-post.py @@ -203,8 +203,13 @@ def _safe_pr_num(pr_num): # self-tag the finder may already have baked into the summary, so we don't add a # second prefix. It can never move a row between the defect/design tables. DESIGN_LENS_TAGS = ("reuse", "simplification", "efficiency", "altitude") +# Match a leading "[lens]" tag the SAME way the workflow's own parser does +# (/^\s*\[([\w-]+)\]/) — no trailing-whitespace requirement. Requiring a space +# after "]" would miss a valid but unspaced self-tag ("[reuse]ExtractHelper" or a +# bare "[reuse]"), which the workflow accepts as tagged, and re-prefix it to +# "[reuse] [reuse]…" — the exact double-prefix this guard exists to prevent. _DESIGN_PREFIX_RE = re.compile( - r"\s*\[(" + "|".join(DESIGN_LENS_TAGS) + r")\]\s", re.IGNORECASE + r"\s*\[(" + "|".join(DESIGN_LENS_TAGS) + r")\]", re.IGNORECASE ) diff --git a/plugins/swarm/scripts/test_loop_closeout.py b/plugins/swarm/scripts/test_loop_closeout.py index 3119f3d..358be4a 100755 --- a/plugins/swarm/scripts/test_loop_closeout.py +++ b/plugins/swarm/scripts/test_loop_closeout.py @@ -71,7 +71,13 @@ def step(**kw): out="continue") expect("pending blocks design-only", ["step", *step(round=1, cap=10, findings=5, agreed=3, changed=2, defects=0, pending=1)], out="continue") -expect("cap wins over design-only at last round", ["step", *step(round=9, cap=10, findings=5, agreed=3, changed=2, defects=2)], +# design-only slots BEFORE cap in the fixed order: at the last allowed round with +# NO defects left, the loop converges clean (design-only), not on the safety stop. +expect("design-only wins over cap at last round", ["step", *step(round=9, cap=10, findings=5, agreed=3, changed=2, defects=0)], + out="terminate=design-only") +# with defects remaining at the last round nothing converged, so cap fires — this +# is the true "cap" case when --defects is passed (design-only never eligible). +expect("cap fires with defects remaining", ["step", *step(round=9, cap=10, findings=5, agreed=3, changed=2, defects=2)], out="terminate=cap") expect("design-only box renders", ["box", "3 2 1", "--reason", "design-only"], code=0) diff --git a/plugins/swarm/scripts/test_pr_post.py b/plugins/swarm/scripts/test_pr_post.py index 0b89c88..4b6db2b 100644 --- a/plugins/swarm/scripts/test_pr_post.py +++ b/plugins/swarm/scripts/test_pr_post.py @@ -174,6 +174,12 @@ def header_row(rendered): # a non-lens bracket prefix is NOT mistaken for a self-tag — the lens is still added todo = pr_post.render_body({"rows": [{"num": "1", "befund": "[TODO] drop this", "kind": "design", "lens": "reuse"}]}) check("non-lens bracket still gets lens", "[reuse] [TODO] drop this" in todo) +# an unspaced self-tag ("[reuse]text", bare "[reuse]") is still recognized — the +# workflow parser accepts it, so the guard must too (else it double-prefixes) +unspaced = pr_post.render_body({"rows": [{"num": "1", "befund": "[reuse]ExtractHelper", "kind": "design", "lens": "reuse"}]}) +check("unspaced self-tag not doubled", "[reuse]ExtractHelper" in unspaced and unspaced.count("[reuse]") == 1) +bare = pr_post.render_body({"rows": [{"num": "1", "befund": "[reuse]", "kind": "design", "lens": "reuse"}]}) +check("bare self-tag not doubled", bare.count("[reuse]") == 1) # --- build subcommand round-trip via subprocess ---------------------------- # with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f: diff --git a/plugins/swarm/skills/review/SKILL.md b/plugins/swarm/skills/review/SKILL.md index 487759b..1734aac 100644 --- a/plugins/swarm/skills/review/SKILL.md +++ b/plugins/swarm/skills/review/SKILL.md @@ -310,8 +310,11 @@ design rows carry 🔧/⏭️/🔁/🆕 like any other). **Prefix each design ro with its `[lens]`** (`[reuse]`/`[simplification]`/`[efficiency]`/`[altitude]`) — the design table has no lens column, so this is where the lens attribution shows, mirroring the PR-comment path exactly (both surfaces read the same); skip the -prefix only if the finding text already opens with that tag. Severity there reads -as importance, not breakage. Numbering is ONE shared sequence across both tables — +prefix only if the finding text already opens with **any known design-lens tag** +(not just the row's own lens) — same rule as the PR path's guard, so a merged row +whose text opens with a different member's tag (`[simplification]` on a `reuse` +row) reads the same on both surfaces. Severity there reads as importance, not +breakage. Numbering is ONE shared sequence across both tables — render each finding's workflow-assigned `num` verbatim in round 0 (across `--loop` rounds the `#` column follows the cross-round identity rule below, not the workflow's re-assigned per-round `num`); no design findings → no From 77b916792c8ffde6bc4134eabe34246d63495b9c Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 15:03:54 +0200 Subject: [PATCH 6/7] Revert P2 kind-inference: untagged findings stay defect (#3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch's own external-only self-review flagged that inferring a design kind from cluster homogeneity is unsafe: a design finder is invited to report defects too, so an untagged finding from a design cluster may be a real off-lens bug. Inferring 'design' routed that bug to applicability verify (wrong rubric → can silently REFUTE/drop a real defect) and excluded it from the --loop defect tally (loop could converge design-only with a real defect still open). Dropping a real bug is the cardinal sin for a review tool and outweighs the benefit (keeping a rarer-but-lower-severity untagged design suggestion out of the defect table). Revert to the pre-0.5.1 safe default: untagged multi-lens findings fall to 'unspecified' → defect. Restores the original pool-level fallback and lens-tagged cluster-kind vote; drops kindTrusted/voters. The P3 double-prefix guard, in-session lens prefix, and P1 --fix/--loop changes are unaffected. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- .../features/swarm-review-pipeline.md | 26 +++---- CHANGELOG.md | 3 +- plugins/swarm/workflows/swarm-review.js | 71 ++++++++----------- 3 files changed, 42 insertions(+), 58 deletions(-) diff --git a/.claude/knowledge/features/swarm-review-pipeline.md b/.claude/knowledge/features/swarm-review-pipeline.md index 2c8edd6..b375f4b 100644 --- a/.claude/knowledge/features/swarm-review-pipeline.md +++ b/.claude/knowledge/features/swarm-review-pipeline.md @@ -46,19 +46,19 @@ truth** (the per-cluster externals follow-up consumes it): trades isolation for fewer agents). - **`kind` is derived from the lens name** (`design` vs `defect`) — no finding-schema change, so the 3-place schema mirror is untouched. A merged - cluster's kind comes from its KIND-TRUSTED members (design only when every - trusted member is design — a design suggestion merged with a real defect must - not leave the defect ranking). **Kind-trusted = provenance-known:** explicitly - tagged, OR (0.5.1) inferred from a homogeneous Claude finder unit — a Claude - finder reviews through ONE kind-single cluster, so an untagged finding from it - still has a reliable kind (a dropped `[reuse]` prefix must not sink a design - suggestion into the defect table + the `--loop` defect tally). Externals carry - the full 11-lens (kind-mixed) set, so they're never inferred — an untagged - external stays `unspecified`/defect and does NOT vote. `untaggedOnly` stays - lens-tag-based, so an **all-untagged cluster is never auto-accepted** — its - "consensus" is backed by no lens, verified like a solo (the `--max` dogfooding - round found exactly this hole); the 0.5.1 kind-inference corrects routing only, - never auto-accept. + cluster's kind comes from its TAGGED members (design only when every tagged + member is design — a design suggestion merged with a real defect must not + leave the defect ranking); untagged (`unspecified`) members don't vote, and + an **all-untagged cluster is never auto-accepted** — its "consensus" is + backed by no lens, so it is verified like a solo (the `--max` dogfooding + round found exactly this hole). **Untagged findings stay `kind: "defect"`** + (the safe bucket), NOT inferred as design from cluster homogeneity: 0.5.1 + tried that inference (to keep a dropped-`[reuse]` design suggestion out of the + defect table) and reverted it — a design finder is invited to report defects + too, so an untagged finding may be a real off-lens BUG, and inferring `design` + routes that bug to applicability verify (wrong rubric → can drop a real defect) + and out of the `--loop` defect tally. Dropping a bug outweighs mis-filing a + suggestion (the branch's own external-only self-review caught this). - **Verify path decision: kind-aware prompt, not bypass.** Design findings are suggestion-shaped, but each has a falsifiable applicability core (reuse target exists? simpler form behavior-identical? claimed waste real?) — the diff --git a/CHANGELOG.md b/CHANGELOG.md index 28801af..e7f2646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,8 +196,7 @@ entries are grouped per plugin, newest first. ### 0.5.1 — 2026-07-18 - Make `--fix` re-confirm design-aware: a `kind: "design"` finding has no line-local defect to re-find, so an agreed (✅) design fix was silently reported skipped-stale and never applied. Step 1 now branches on kind — for design findings it re-confirms the suggestion still applies (reuse target / duplication / simpler form / waste still present), only skipping when that target is genuinely gone. - Make `--loop` converge on design churn: the tally made no defect/design distinction, so once only design suggestions remained (each applied simplification spawning a fresh one) it ran to the cap. `loop-closeout.py step` gains `--defects D` and a new `design-only` reason (fixed order: after no-change, before cap); `--pending` is now defect-scoped (design never holds the loop open). Omitting `--defects` disables the reason (legacy callers unaffected). -- Fix untagged design-cluster findings mis-routing to defect: a design finder that dropped its `[lens]` prefix fell back to `unspecified` → `kind: "defect"`, so a real suggestion rendered in the defect table, skipped applicability verify, and inflated the new `--loop` defect tally. A Claude finder reviews through one kind-homogeneous cluster, so its kind is now inferred (`kindTrusted`) even when the exact lens isn't; externals (kind-mixed 11-lens unit) stay `unspecified`/defect and keep not voting — the consensus/auto-accept invariant is untouched, only design/defect routing is corrected. -- Guard `pr-post.py` against a design-row `[lens]` double-prefix (`[reuse] [reuse] …`): if the finding cell already opens with a known design-lens self-tag, keep it and don't add a second (`DESIGN_LENS_TAGS`, sync-checked by `test_lens_sync.py`; not a kind fallback). +- Guard `pr-post.py` against a design-row `[lens]` double-prefix (`[reuse] [reuse] …`): if the finding cell already opens with a known design-lens self-tag (spaced or not, matching the workflow tag parser), keep it and don't add a second (`DESIGN_LENS_TAGS`, sync-checked by `test_lens_sync.py`; not a kind fallback). Untagged findings deliberately stay `kind: "defect"` (the safe bucket): a design finder may report a real off-lens bug, and inferring `design` from cluster homogeneity would route that bug to applicability verify (wrong rubric) and out of the `--loop` defect tally — dropping a bug is worse than mis-filing a suggestion. - Show the lens in the in-session Design table too: design rows now carry a `[lens]` `Befund` prefix, matching the PR-comment path (both surfaces read identically). - Accept the cluster-default's loss of per-lens failure isolation as a documented tradeoff (one crashed cluster finder drops that cluster's Claude coverage as a visible `backendError`; `--max` restores per-lens isolation). diff --git a/plugins/swarm/workflows/swarm-review.js b/plugins/swarm/workflows/swarm-review.js index 2fd65d8..299a92b 100644 --- a/plugins/swarm/workflows/swarm-review.js +++ b/plugins/swarm/workflows/swarm-review.js @@ -374,34 +374,25 @@ for (const v of voices) { // set, not the finder's own subset: off-lens bleed is real (a design finder // may spot a genuine [security] bug while reading), and coercing a validly // tagged foreign lens would flip `kind` and route the finding through the - // wrong verifier + report section. + // wrong verifier + report section. An unknown/missing prefix falls back to + // the finder's lens ONLY for a single-lens unit (--max — the assignment is + // unambiguous); a multi-lens cluster finding falls back to 'unspecified' + // (kind defect, the safe bucket) — guessing lenses[0] could stamp a design + // kind on an untagged off-lens defect from the design cluster. // - // When the prefix is unknown/missing, the exact lens can't be recovered — but - // the KIND often can, and that's what routes a finding (design table + - // applicability verify vs defect ranking + P1b's --loop defect tally). A - // Claude finder reviews through ONE cluster whose lenses are single-KIND - // (breakage/threat/consistency = all defect, design = all design), so an - // untagged finding from it inherits that kind reliably (`kindTrusted`) — a - // dropped [reuse] prefix must not sink a design suggestion into the defect - // table. Externals carry the full 11-lens set (or none), so their unit is - // kind-MIXED → never inferred: an untagged external stays 'unspecified'/defect - // (the safe bucket) and keeps NOT voting on cluster kind (the load-bearing - // invariant). The lens itself stays 'unspecified' unless it was a --max - // single-lens unit — we recover the kind, not a specific lens we can't know. + // 0.5.1 briefly inferred the KIND from a homogeneous design unit here (a + // dropped [reuse] prefix would otherwise mis-file a design suggestion to the + // defect table). Reverted: a design finder is invited to report defects too, + // so an untagged finding from it may be a real off-lens BUG — inferring + // 'design' routes that bug to applicability verify (wrong rubric → can drop a + // real defect) and out of the --loop defect tally. Dropping a bug is worse + // than mis-filing a suggestion, so untagged stays 'defect' (the safe bucket). const m = /^\s*\[([\w-]+)\]/.exec(f.summary || '') let lens = m ? m[1].toLowerCase() : '' - let kind, kindTrusted - if (CANDIDATE_LENSES.includes(lens)) { - kind = lensKind(lens); kindTrusted = true - } else if (Array.isArray(v.lenses) && v.lenses.length === 1) { - lens = v.lenses[0]; kind = lensKind(lens); kindTrusted = true // --max: unambiguous lens - } else { - lens = 'unspecified' - const unitKinds = new Set((Array.isArray(v.lenses) ? v.lenses : []).map(lensKind)) - kindTrusted = unitKinds.size === 1 // homogeneous Claude unit only - kind = kindTrusted ? [...unitKinds][0] : 'defect' + if (!CANDIDATE_LENSES.includes(lens)) { + lens = Array.isArray(v.lenses) && v.lenses.length === 1 ? v.lenses[0] : 'unspecified' } - pool.push({ ...f, backend: v.backend, family: FAMILY[v.backend] || v.backend, lens, kind, kindTrusted }) + pool.push({ ...f, backend: v.backend, family: FAMILY[v.backend] || v.backend, lens, kind: lensKind(lens) }) } } log(`Fan-out: ${pool.length} raw findings from ${voices.length} voices` + @@ -450,25 +441,19 @@ if (pool.length > 0) { const backends = Array.from(new Set(members.map((i) => pool[i].backend))).sort() const families = Array.from(new Set(members.map((i) => pool[i].family))).sort() // Cluster kind from the MEMBERS (not the merge agent's free-text `lens`): - // design only when every KIND-TRUSTED member is design — 'defect' is the - // single structural fallback (a design suggestion merged with a real defect - // must not drop out of the defect ranking). A member is kind-trusted when its - // kind is provenance-known: explicitly tagged, OR inferred from a homogeneous - // Claude unit (see the pool loop). Kind-UNtrusted members (untagged externals, - // whose 11-lens unit is kind-mixed) do NOT vote — one untagged external must - // not drag a properly tagged design cluster into the defect ranking, nor a - // homogeneous Claude design finding out of it. - // - // `untaggedOnly` is separate and stays LENS-tagged-based: an ALL-untagged - // cluster (no member carries a lens tag) is flagged so it is never - // auto-accepted (see needsVerify) — two diff-scoped externals can agree on an - // unverifiable suggestion without ever tagging it. An inferred-design finding - // still carries no lens tag, so this gate is untouched: the consensus / - // auto-accept invariant is unchanged — only the design/defect routing is - // corrected. - const voters = members.filter((i) => pool[i].kindTrusted) - const kind = voters.length > 0 && voters.every((i) => pool[i].kind === 'design') ? 'design' : 'defect' - const untaggedOnly = members.every((i) => pool[i].lens === 'unspecified') + // design only when every TAGGED member is design — 'defect' is the single + // structural fallback (a design suggestion merged with a real defect must + // not drop out of the defect ranking). 'unspecified' members (untagged + // externals, or an untagged Claude finding — 0.5.1's kind-inference was + // reverted, see the pool loop) do NOT vote: their kind is only the safe + // default, and one untagged voice must not drag a properly tagged design + // cluster into the defect ranking. An ALL-untagged cluster is kind 'defect' + // but flagged — its "consensus" is backed by no tagged lens, so it must never + // be auto-accepted (see needsVerify below): two diff-scoped externals can + // agree on an unverifiable suggestion without ever tagging it. + const known = members.filter((i) => pool[i].lens !== 'unspecified') + const kind = known.length > 0 && known.every((i) => pool[i].kind === 'design') ? 'design' : 'defect' + const untaggedOnly = known.length === 0 // The merge agent's `lens` is free text (schema caps length, not values): // accept it ONLY when it is a known lens AND some cluster member actually // carries it — a globally-valid lens no member tagged (merge says `security` From db28865cf155fb66a0f1ccfc8b30b3d6dea3f50d Mon Sep 17 00:00:00 2001 From: Robert Gering Date: Sat, 18 Jul 2026 18:48:14 +0200 Subject: [PATCH 7/7] Sharpen design-only caveat: last fixes not re-reviewed (round-2 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch's second external-only self-review (CONFIRMED codex+grok) flagged that `--loop` design-only convergence terminates right after applying that round's design fixes, before any re-review — so a simplification that introduces a real defect this round is never caught. Forcing a re-review would re-open the churn design-only exists to close (design findings diverge), so the fix is explicit disclosure, not a behavior change: the SKILL close-out, loop-closeout.py's REASONS label + docstring, and the knowledge entry now state the last design edits were applied but NOT re-reviewed and recommend a fresh /swarm:review to confirm — same honesty as the cap caveat. (Round-2 finding B — the double-prefix guard keeping a self-tag that differs from the resolved cluster lens — was declined: the shown tag matches the shown text, so rewriting to the resolved lens would mislabel it; it's the deliberate anti-double-prefix tradeoff.) Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Jo5yGBLmefV64HN26hxV8n --- .claude/knowledge/features/swarm-review-pipeline.md | 8 +++++++- plugins/swarm/scripts/loop-closeout.py | 10 ++++++++-- plugins/swarm/skills/review/SKILL.md | 11 +++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/.claude/knowledge/features/swarm-review-pipeline.md b/.claude/knowledge/features/swarm-review-pipeline.md index b375f4b..5a12281 100644 --- a/.claude/knowledge/features/swarm-review-pipeline.md +++ b/.claude/knowledge/features/swarm-review-pipeline.md @@ -167,7 +167,13 @@ the diff out of the script, above). Claude applies edits between rounds. defect/design-blind tally ran the loop to the cap on them. The loop now converges once no *defect* finding remains — design is advisory and never holds it open; `--pending` is defect-scoped for the same reason. Omitting `--defects` - disables the reason (legacy callers see the original four). + disables the reason (legacy callers see the original four). **Accepted residual + (the branch's own self-review flagged it):** like `cap`, design-only fires + BEFORE the round's re-review, so this round's design fixes are applied but not + re-reviewed — a simplification could introduce a defect the loop never catches. + Forcing a re-review would re-open the very churn design-only closes (design + findings diverge), so the close-out instead flags the residual and recommends a + fresh `/swarm:review` over the result. - Loop mechanics mirror pr-flow `/cycle` run locally (no push / no `@claude` poll); the `Status` column (🔧/⏭️/🔁) and stable `#` across rounds come from the report table contract this entry defines above (P2 reserved them). diff --git a/plugins/swarm/scripts/loop-closeout.py b/plugins/swarm/scripts/loop-closeout.py index f918f87..f5b236c 100755 --- a/plugins/swarm/scripts/loop-closeout.py +++ b/plugins/swarm/scripts/loop-closeout.py @@ -42,7 +42,8 @@ "0-findings": "0 findings — converged clean", "nothing-agreed": "nothing agreed — only disagreements (❌) left open", "no-change": "no files changed — fixed point reached", - "design-only": "no defect findings remain — design suggestions are advisory", + "design-only": "no defect findings remain — design tail is advisory; its fixes " + "were applied but NOT re-reviewed (re-run to confirm they're clean)", "cap": "cap reached", } @@ -63,7 +64,12 @@ def cmd_step(a: argparse.Namespace) -> int: advisory, and each applied simplification can spawn a fresh one, so once no defect-kind finding remains the loop has converged on the part that matters — the design tail doesn't hold it open. Omitting `--defects` disables this - reason (legacy behavior: only the other four fire). + reason (legacy behavior: only the other four fire). Like `cap`, it fires + BEFORE the round's re-review, so this round's design fixes were applied but + NOT re-reviewed — a simplification could have introduced a defect this round + never catches. Forcing a re-review instead would re-open the churn this reason + exists to close (design findings diverge), so the caller must flag the residual + and recommend a fresh review over the result (see the SKILL close-out). Any DEFECT finding still awaiting a user decision (`--pending` > 0) keeps the loop alive: it suppresses ALL FOUR convergence reasons (0-findings, diff --git a/plugins/swarm/skills/review/SKILL.md b/plugins/swarm/skills/review/SKILL.md index 1734aac..4da41df 100644 --- a/plugins/swarm/skills/review/SKILL.md +++ b/plugins/swarm/skills/review/SKILL.md @@ -512,10 +512,13 @@ Each round: `terminate=cap` the last round's fixes were applied but not re-reviewed** (cap fires before step 4) — say so in the close-out summary; the cap is a safety stop, not a clean bill of health for the final edits. **On - `terminate=design-only`** the round's defect count hit zero but design - suggestions may remain (they were applied in this round's fix step, not - re-reviewed) — say so: the loop converged on defects, the design tail is - advisory. + `terminate=design-only`** the round's defect count hit zero, but its design + fixes were **applied and NOT re-reviewed** (design-only fires before step 4, + like cap) — and a simplification/refactor *can* introduce a real defect that + this round therefore never catches. Say so explicitly and recommend one more + `/swarm:review` over the result to confirm the final design edits are clean; + the loop converged on the defects it had, the design tail is advisory, not + a guarantee the last edits are bug-free. 4. **Re-review** — re-run steps 1–3 (Prepare diff → Workflow → Present) on the **new** working tree. Two guards before spending another (possibly `--max`) ensemble pass: