Skip to content

refactor(review): cross-agent synthesis, plugin-cache dedupe, docs (2/2)#1112

Open
peyton-alt wants to merge 4 commits intofeat/entire-review-v2-b1from
feat/entire-review-v2-b2
Open

refactor(review): cross-agent synthesis, plugin-cache dedupe, docs (2/2)#1112
peyton-alt wants to merge 4 commits intofeat/entire-review-v2-b1from
feat/entire-review-v2-b2

Conversation

@peyton-alt
Copy link
Copy Markdown
Contributor

@peyton-alt peyton-alt commented May 4, 2026

Second of two stacked meta-PRs in the PR-B multi-agent redesign. Stacked on #1111 (B1). When B1 merges, this PR auto-retargets up the stack.

The full redesign behind #993/#1018 is split across 5 stacked PRs:

What's in this PR

  • CU10 — opt-in cross-agent verdict synthesis. After all agents finish in TTY mode, SynthesisSink prompts y/N (default N): "Synthesize a unified verdict across all agent reviews?". On "y", composes a synthesis prompt covering each agent's narrative + the optional per-run prompt, calls the configured summary provider, and prints the unified verdict. Provider failures degrade gracefully (synthesis unavailable: <err>) so the user can still commit. Provider resolution is deferred to first call via lazySynthesisProvider — avoids running auto-select on every entire review --help.
  • CU11 — plugin-cache skill dedupe. pickLatestVersion picks ONE version directory per plugin: highest valid semver wins; lexicographic max otherwise. Without this, multiple installed versions of the same plugin produced duplicate skill entries in the picker and prompt.
  • CU12 — multi-agent docs in CLAUDE.md (auto-syncs to AGENTS.md via symlink): "Multi-Agent UI" section, "Skill Discovery (Claude Code)" section, updated Architecture and Key Files lists.

Smoke tests

  • Multi-agent run completes — y/N synthesis prompt appears
  • Press y → synthesis runs and prints verdict
  • Press N or Enter (default) → no synthesis output
  • Force a synthesis provider failure (e.g., misconfigured API key) → synthesis unavailable: <err> appears, no panic, commit still possible
  • On a repo with two installed versions of pr-review-toolkit, run entire review --edit → review skills appear once each (not duplicated)
  • Verify CLAUDE.md reflects the multi-agent architecture (Multi-Agent UI + Skill Discovery sections)
  • Run mise run check — lint clean, tests green

🤖 Generated with Claude Code


Note

Medium Risk
Adds new interactive multi-agent post-processing that calls a configured LLM provider (with deferred provider selection/persistence) and changes terminal output rendering, which could affect UX and settings writes in some environments.

Overview
Adds an opt-in cross-agent synthesis step for multi-agent entire review runs: in TTY mode (and when stdin can prompt), a new SynthesisSink offers a y/N prompt after the per-agent dump and, on yes, composes a combined prompt from agent narratives (+ per-run prompt) and calls a pluggable SynthesisProvider to print a unified verdict; provider failures degrade to synthesis unavailable: <err>.

Introduces mdrender, a shared terminal markdown renderer (TTY- and NO_COLOR-aware), and switches review dumps to emit markdown rendered via mdrender (raw markdown when redirected) while entire dispatch now reuses the shared palette instead of its local glamour style config.

Improves Claude Code skill discovery by deduping plugin cache versions (picks a single “latest” version dir per plugin via semver/lexicographic fallback), updates docs in CLAUDE.md, and expands tests plus linter allowlists for the new synthesis provider interface.

Reviewed by Cursor Bugbot for commit 1f27390. Configure here.

Copilot AI review requested due to automatic review settings May 4, 2026 23:07
@peyton-alt peyton-alt requested a review from a team as a code owner May 4, 2026 23:07
Comment thread cmd/entire/cli/review/synthesis_sink.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the second phase of the entire review multi-agent redesign by introducing an opt-in cross-agent synthesis step, deduping Claude Code plugin-cache skills across multiple installed plugin versions, and updating internal architecture documentation to reflect the new flow.

Changes:

  • Introduces SynthesisSink + prompt composer to optionally generate a unified verdict after multi-agent runs (TTY + prompt-capable stdin).
  • Dedupes Claude Code plugin-cache skill discovery by selecting a single “latest” version directory per plugin (semver-first, lexicographic fallback).
  • Wires synthesis lazily via lazySynthesisProvider, expands test coverage, and updates docs/lint configuration.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
cmd/entire/cli/review/synthesis_sink.go New sink that prompts (y/N) and, on “yes”, calls a synthesis provider and prints a unified verdict.
cmd/entire/cli/review/synthesis_sink_test.go Unit tests covering skip conditions, prompt behavior, provider errors, and per-run prompt threading.
cmd/entire/cli/review/synthesis_prompt.go Pure prompt composer for cross-agent synthesis; filters to “usable” agent narratives.
cmd/entire/cli/review/synthesis_prompt_test.go Tests ensuring narratives/sections are included, empty narratives excluded, determinism, and per-run prompt placement.
cmd/entire/cli/review/export_test.go Test-only exports to allow review_test package to drive internal helpers.
cmd/entire/cli/review/cmd.go Multi-agent sink composition updated to optionally append SynthesisSink; adds helpers for sink composition + TUI discovery.
cmd/entire/cli/review/cmd_test.go Tests for sink composition and dispatch behavior when synthesis is nil / single-agent path.
cmd/entire/cli/review_bridge.go CLI wiring: adds lazy synthesis provider resolution on first use to avoid startup side effects.
cmd/entire/cli/agent/claudecode/discovery.go Plugin-cache scan now selects one version per plugin via semver-aware pickLatestVersion.
cmd/entire/cli/agent/claudecode/discovery_test.go Tests ensuring plugin-cache version dedupe works (semver wins; non-semver fallback).
CLAUDE.md Documentation updates for multi-agent UI, synthesis, and skill discovery behavior.
.golangci.yaml Updates iface allowlist to include the new review.SynthesisProvider interface.

Comment thread cmd/entire/cli/review/synthesis_prompt.go Outdated
Comment thread cmd/entire/cli/review/synthesis_prompt.go Outdated
Comment thread cmd/entire/cli/review/synthesis_sink.go
Comment thread cmd/entire/cli/review_bridge.go Outdated
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b2 branch from 3d353cc to 42cfe7c Compare May 4, 2026 23:49
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b1 branch from b423db9 to 97a72a1 Compare May 5, 2026 00:30
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b2 branch from 42cfe7c to 5a86122 Compare May 5, 2026 00:30
@peyton-alt
Copy link
Copy Markdown
Contributor Author

@BugBot review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1f27390. Configure here.

Comment thread cmd/entire/cli/review/synthesis_sink.go Outdated
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b2 branch from 1f27390 to d016f2b Compare May 5, 2026 03:32
peyton-alt and others added 4 commits May 4, 2026 20:42
CU10 introduces SynthesisSink, an opt-in Sink that asks a configured
summary provider to produce a unified verdict across the per-agent
narratives after a multi-agent review. Composed AFTER DumpSink in TTY
mode so the y/N prompt appears below the per-agent narrative dump.

  - review/synthesis_sink.go: SynthesisSink with SynthesisProvider
    interface for test injection. RunFinished skips silently on non-TTY,
    cancelled run, or fewer than 2 agents with usable output. On user y:
    compose prompt, call provider, print response. On provider failure:
    print "synthesis unavailable: <err>" — user can still commit.
  - review/synthesis_prompt.go: composeSynthesisPrompt builds the LLM
    prompt with per-agent narrative sections + a verdict template
    (common findings / unique findings / disagreements / priority order).
    Honors per-run prompt if user supplied one in the multi-picker.
  - cmd.go: SynthesisSink appended to TTY-mode sink slice when
    deps.SynthesisProvider is non-nil. Threads picked.PerRun through.
  - review_bridge.go: production wiring uses lazySynthesisProvider that
    resolves the summary provider on-demand (not at CLI startup) reusing
    the same provider config as `entire explain`. Lazy resolution avoids
    startup side-effects (persistence writes, auto-select notices) that
    caused integration test regressions with eager resolution.
  - .golangci.yaml: add review.SynthesisProvider to ireturn allow list.
  - tui_model.go: move nolint directives to function declaration lines
    (gofmt strips lone //nolint comments in doc-comment continuation
    blocks under Go 1.26).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
scanPluginCache walked every <plugin>/<version>/ directory, which produces
duplicate skill entries when a plugin has multiple versions on disk
(common after upgrades — old versions aren't always cleaned up). The
picker would show the same skill twice with no way to tell the entries
apart, and the agent prompt would list the same invocation multiple times.

Pick a single version per plugin via pickLatestVersion: prefer the highest
valid semver among the available version dirs, falling back to the
lexicographic max when none parse (handles the "unknown" sentinel some
plugins ship). Adds tests covering: dedupe across two semver versions,
non-semver "unknown" fallback, and semver winning when both shapes coexist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates CLAUDE.md (and AGENTS.md via symlink) to cover the multi-agent
shape introduced in CU8–CU11:

  - Command surface: --agent now skips the multi-picker; multi-select
    flow is described for the 2+-launchable-agents-no-override case
  - Architecture: split single-agent Run vs N-agent RunMulti, sink
    catalog updated with TUISink and SynthesisSink, perAgentConfigured-
    Reviewer adapter explained
  - New "Multi-Agent UI" section: TUISink (live dashboard, drill-in,
    Ctrl+C wiring through tea.WithoutSignalHandler), SynthesisSink
    (opt-in cross-agent verdict, graceful degradation), and
    composeMultiAgentSinks/findTUISink composition helpers
  - New "Skill Discovery (Claude Code)" section: pickLatestVersion
    plugin-cache dedupe (semver-first, lexicographic fallback)
  - Key Files: tui_sink/model/detail, synthesis_sink/prompt, multi-
    picker, run_multi, claudecode/discovery, lazySynthesisProvider in
    review_bridge

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Agents emit markdown — claude-code, codex, and gemini-cli all return
narrative with headings, lists, code blocks, and inline formatting. Until
now DumpSink printed it as raw text, so the user saw bare ─────── name
review ─────── separators followed by unstyled markdown like `# foo` and
`- bar`. Same for the cross-agent synthesis verdict.

This commit:

  - Extracts the glamour palette from dispatch_tui.go into a shared
    cmd/entire/cli/mdrender/ package. Render(markdown, width, dark)
    is the pure primitive; RenderForWriter(w, markdown) is TTY-aware
    and returns raw markdown for non-TTY writers (so redirected output
    stays grep-friendly and pipeline-safe).
  - Updates DumpSink to compose each agent's section as markdown:
    H1 heading for agent name; cancelled runs render as "_cancelled_";
    failed runs use **Failed: `err`** plus blockquoted RunError details;
    succeeded runs render the AssistantText narrative directly. The
    counts line at the end stays plain (it's a status summary, not
    narrative content — keeping it grep-friendly is more useful than
    styling it).
  - Updates SynthesisSink to render the cross-agent verdict through the
    same palette.
  - Updates dispatch_tui.go's defaultRenderTerminalMarkdown to delegate
    to mdrender.Render — preserving its existing always-render behavior
    (dispatch emits ANSI even when redirected, by long-standing intent).

The TUI's drill-in screen and dashboard rows are unchanged; lipgloss
styling there already does its job and doesn't need glamour.

Tests: TTY/non-TTY paths in mdrender_test.go, NO_COLOR honored, dark vs
light palette both produce styled output. DumpSink tests rewritten to
assert markdown body (since bytes.Buffer is non-TTY). All existing tests
continue to pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b1 branch from 97a72a1 to ac94870 Compare May 5, 2026 03:43
@peyton-alt peyton-alt force-pushed the feat/entire-review-v2-b2 branch from d016f2b to ed9ff97 Compare May 5, 2026 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants