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..5a12281 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,6 +39,11 @@ 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 @@ -46,7 +51,14 @@ truth** (the per-cluster externals follow-up consumes it): 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). + 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 @@ -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,32 @@ 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). **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). @@ -211,7 +242,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..e7f2646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -193,6 +193,13 @@ 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). +- 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). + ### 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/scripts/loop-closeout.py b/plugins/swarm/scripts/loop-closeout.py index 808d45a..f5b236c 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,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 tail is advisory; its fixes " + "were applied but NOT re-reviewed (re-run to confirm they're clean)", "cap": "cap reached", } @@ -48,31 +55,50 @@ 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). 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, + 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 +159,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/pr-post.py b/plugins/swarm/scripts/pr-post.py index 3ef24b6..18e94c4 100755 --- a/plugins/swarm/scripts/pr-post.py +++ b/plugins/swarm/scripts/pr-post.py @@ -196,6 +196,23 @@ 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") +# 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")\]", 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 +275,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_loop_closeout.py b/plugins/swarm/scripts/test_loop_closeout.py index f3ee29e..358be4a 100755 --- a/plugins/swarm/scripts/test_loop_closeout.py +++ b/plugins/swarm/scripts/test_loop_closeout.py @@ -57,9 +57,34 @@ 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") +# 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) + # --- 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/scripts/test_pr_post.py b/plugins/swarm/scripts/test_pr_post.py index 315e278..4b6db2b 100644 --- a/plugins/swarm/scripts/test_pr_post.py +++ b/plugins/swarm/scripts/test_pr_post.py @@ -163,6 +163,24 @@ 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) +# 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: json.dump(DATA, f) diff --git a/plugins/swarm/skills/review/SKILL.md b/plugins/swarm/skills/review/SKILL.md index c807a72..4da41df 100644 --- a/plugins/swarm/skills/review/SKILL.md +++ b/plugins/swarm/skills/review/SKILL.md @@ -306,8 +306,15 @@ 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 **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 @@ -416,9 +423,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 +475,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 +494,31 @@ 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 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: @@ -647,7 +680,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 diff --git a/plugins/swarm/workflows/swarm-review.js b/plugins/swarm/workflows/swarm-review.js index 4cee982..299a92b 100644 --- a/plugins/swarm/workflows/swarm-review.js +++ b/plugins/swarm/workflows/swarm-review.js @@ -379,6 +379,14 @@ for (const v of voices) { // 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. + // + // 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() : '' if (!CANDIDATE_LENSES.includes(lens)) { @@ -436,11 +444,12 @@ if (pool.length > 0) { // 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 + // 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'