Skip to content

Review profiles with master adjudication#1312

Draft
dipree wants to merge 16 commits into
mainfrom
review-profiles
Draft

Review profiles with master adjudication#1312
dipree wants to merge 16 commits into
mainfrom
review-profiles

Conversation

@dipree
Copy link
Copy Markdown
Contributor

@dipree dipree commented Jun 1, 2026

Summary

Redesigns entire review around named review profiles with mandatory master adjudication, replacing the old global per-agent review config. Experimental feature — legacy behavior and migration are removed entirely.

What changed

  • Profiles: new review_profiles + review_default_profile settings. A profile has a canonical task, a set of worker agents, and a master that consolidates worker reports into one final report.
  • CLI shape:
    • entire review, entire review <profile>, entire review --profile <name>
    • entire review --prompt for one-off per-run instructions
    • entire review --agent <name> to run a single worker
  • Fan-out: multi-worker profiles fan out to all configured workers (no per-run multi-picker); multi-worker profiles require a master, which runs after workers and produces the canonical report.
  • Simple guided setup: first run (and entire review --configure) walks through review type + worker agents + optional models, then asks before starting agents. Non-interactive runs fall back to an opinionated default general profile.
  • Worker models: each profile entry is a worker id with optional agent + model, so the same agent can run multiple times with different models. --model is threaded into each launchable agent's argv.
  • Terminology: "non-launchable" reframed as "no review-runner adapter yet" in code/docs — it's an adapter backlog, not a launch-capability claim.
  • Removed legacy review migration code/tests and the optional synthesis-prompt flow; master adjudication is now first-class.

Testing

  • env -u PI_CODING_AGENT go test ./cmd/entire/cli/...
  • targeted review integration tests

Notes

Stacked: the Pi review-runner adapter is in a follow-up PR on top of this branch.


Note

Medium Risk
Large behavioral change to an experimental command (settings shape, fan-out rules, no legacy migration); master synthesis and multi-agent concurrency affect how reviews run and attach to checkpoints.

Overview
entire review is rebuilt around named review profiles instead of the legacy per-agent review map. Settings add review_profiles and review_default_profile; each profile has a shared task, worker agents (with optional agent alias + model), and a master that produces the final report after workers finish.

The CLI gains --profile, positional profile names, --configure (guided setup without running), --prompt for one-off instructions, and --edit scoped to profile skill/model pickers. Multi-worker runs fan out to all profile workers—the old per-run multi-agent picker is gone—and require a valid master. First-run / non-TTY paths use guided setup or an opinionated default general profile.

Prompts and orchestration inject profile name + canonical task into worker prompts; RunConfig / AgentRun carry profile, model, and display vs registry agent names for manifests and session matching. Master adjudication replaces optional lazy checkpoint synthesis: AgentSynthesisProvider runs automatically (SynthesisSink auto mode) with a stricter merge/dispute prompt. Claude, Codex, and Gemini reviewers pass --model when configured; Codex argv order is fixed for stdin.

Removed: project→clone review migration (migration.go and tests) and Deps.SynthesisProvider from the review bridge. Docs and integration tests move to profile-shaped settings.

Reviewed by Cursor Bugbot for commit bc13a81. Configure here.

Copilot AI review requested due to automatic review settings June 1, 2026 19:26
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 5 potential issues.

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 bc13a81. Configure here.

Comment thread cmd/entire/cli/review/picker.go Outdated
Comment thread cmd/entire/cli/review/cmd.go Outdated
Comment thread cmd/entire/cli/review/cmd.go Outdated
Comment thread cmd/entire/cli/review/picker.go
Comment thread cmd/entire/cli/review/manifest.go Outdated
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

This PR redesigns the experimental entire review command around named review profiles with fan-out to multiple worker agents and mandatory master adjudication (for multi-worker profiles), replacing the legacy per-agent review config and removing the old migration flow.

Changes:

  • Introduces review_profiles + review_default_profile settings, including worker-level agent aliases and model hints, plus a profile master used for final adjudication.
  • Updates entire review CLI surface (--profile, positional profile name, --configure, --prompt, and --agent targeting a single worker).
  • Implements profile-native master synthesis via a new AgentSynthesisProvider, and threads --model into Claude/Codex/Gemini review runner argv.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
docs/architecture/review-command.md Updates architecture documentation for profile-based review and master adjudication.
cmd/entire/cli/settings/settings.go Adds settings schema/types for review profiles and default profile; keeps legacy fields for parsing.
cmd/entire/cli/root.go Updates root command comment to reflect profiles.
cmd/entire/cli/review/types/sink.go Extends per-run metadata to include display name vs registry name and model.
cmd/entire/cli/review/types/reviewer.go Extends RunConfig with profile/task/model context for profile-based prompts and runs.
cmd/entire/cli/review/synthesis_sink.go Adds agent-backed master synthesis provider and auto/adjudication-mode wiring.
cmd/entire/cli/review/synthesis_prompt.go Updates synthesis prompt to an adjudication-style “final report” with rules and sections.
cmd/entire/cli/review/run.go Records worker display name vs registry agent name and model in RunSummary; forwards events under display name.
cmd/entire/cli/review/run_multi.go Propagates agent registry name and model through multi-run state and summaries.
cmd/entire/cli/review/prompt.go Composes worker prompts from skills + profile name + canonical task + per-agent/per-run instructions.
cmd/entire/cli/review/profile.go New profile resolution/default-task logic, worker/master selection, and preference persistence helpers.
cmd/entire/cli/review/picker.go Adds guided setup flow and profile-aware editing; introduces master selection and model picking.
cmd/entire/cli/review/migration.go Removes legacy migration flow from project settings to clone preferences.
cmd/entire/cli/review/migration_test.go Removes tests for the deleted migration flow.
cmd/entire/cli/review/marker_fallback.go Updates terminology/docs for “no adapter yet” manual fallback flow.
cmd/entire/cli/review/manifest.go Improves manifest/session matching for worker aliases and model hints.
cmd/entire/cli/review/fix.go Updates fix-agent choice derivation to pull from profiles (including master) and dedupe aliases.
cmd/entire/cli/review/export_test.go Updates test export wrapper for changed synthesis prompt signature.
cmd/entire/cli/review/cmd.go Rebuilds entire review command flow: profile resolution, guided setup, fan-out, and master synthesis.
cmd/entire/cli/review/cmd_test.go Updates command tests for profile fan-out behavior and --prompt flow.
cmd/entire/cli/review_context_test.go Updates review-context test settings to the new profile schema.
cmd/entire/cli/review_bridge.go Removes legacy synthesis provider wiring; keeps reviewer wiring to avoid import cycles.
cmd/entire/cli/labs.go Updates labs/experimental command summary text.
cmd/entire/cli/integration_test/review_test.go Updates integration tests to configure reviews via profiles.
cmd/entire/cli/agent/geminicli/reviewer.go Threads model hint into Gemini review runner command.
cmd/entire/cli/agent/codex/reviewer.go Threads model hint into Codex review runner argv while keeping stdin ordering.
cmd/entire/cli/agent/claudecode/reviewer.go Threads model hint into Claude review runner argv.

Comment thread cmd/entire/cli/settings/settings.go
Comment thread cmd/entire/cli/review/picker.go Outdated
Comment thread docs/architecture/review-command.md Outdated
Comment thread docs/architecture/review-command.md Outdated
Comment thread cmd/entire/cli/review/synthesis_sink.go Outdated
Comment thread cmd/entire/cli/settings/settings.go
Comment thread cmd/entire/cli/review/picker.go Outdated
Comment thread docs/architecture/review-command.md Outdated
Comment thread docs/architecture/review-command.md Outdated
dipree added 2 commits June 2, 2026 12:14
- Preserve profile task/master_model when saving from --edit
- Route empty/placeholder profiles through first-run setup
- Skip re-review confirm in non-interactive runs instead of erroring
- Treat 'start review now?' abort as a clean cancel
- Fix lint: drop dead helpers, lowercase error, perfsprint, un-deprecate
  the still-used review_fix_agent field
- Doc/comment fixes (ReviewConfig prompt/skills, master adjudication)
Strip the provider prefix and thinking-level suffix when matching a
configured profile model against a session's recorded model, so hints like
'anthropic/claude-sonnet:high' link to session model 'claude-sonnet-4-5'.
Same-model/different-thinking workers fall back to start-time + used-session
disambiguation.
@dipree
Copy link
Copy Markdown
Contributor Author

dipree commented Jun 2, 2026

Addressed the review feedback (Cursor Bugbot + Copilot) in b29ad76 and a1297da:

  • Edit save drops profile task / master_model (picker.go): saveReviewProfileConfig now merges into the existing profile, rewriting only agents + master. Custom task text and master_model are preserved.
  • Empty profiles skip first-run setup (cmd.go): the first-run gate now counts non-zero profiles, so a placeholder/empty general entry routes through guided setup / the non-interactive default instead of dead-ending in selectReviewProfile.
  • Multi re-review blocks non-interactive runs (cmd.go): the "already reviewed" guard is factored into confirmReReviewOrProceed, which proceeds (with a note) in non-interactive contexts instead of erroring on a confirm form.
  • Setup cancel prints command error (picker.go): aborting "Start review now?" now returns ErrPickerCancelled, mapped to a clean silent exit.
  • Model prefix skews session match (manifest.go): model matching strips the provider prefix and thinking-level suffix; documented that same-model/different-thinking workers fall back to start-time + used-session disambiguation.
  • Docs/comments (settings.go, synthesis_sink.go, review-command.md): clarified ReviewConfig.Skills/Prompt semantics, that the master adjudication runs automatically, and the key-files list now points at ReviewProfiles/ReviewDefaultProfile.

Also fixed all lint findings (dead helpers removed, lowercased error string, perfsprint, and un-deprecated the still-used review_fix_agent field). Local golangci-lint run ./cmd/entire/cli/review/... is clean and the affected tests pass.

dipree added 8 commits June 2, 2026 12:56
- Drop the --fix and --all flags and the fix-apply pathway (findings remain
  browsable via --findings).
- Add --model to override the model for a single --agent worker run
  (--model requires --agent).
- Remove now-dead fix-agent selection helpers; keep the shared default/saved
  agent-pick helpers used by the master picker.
- Update post-run footer/findings output and docs to drop --fix references.
- entire review --configure now prints available review agents (adapter-backed,
  with installed status) and current profiles every time, so it's the
  discovery entry point.
- Accept --set-agents/--set-master/--set-task/--set-model to write a profile
  non-interactively (no TUI); --set-* writes preserve untouched profile-level
  fields (task, master_model). With no --set-* flags it falls back to the
  guided wizard (interactive) or a discovery view (non-interactive).
- --agent errors now list the profile's configured workers.
- Available agents derive from the registry + ReviewerFor, so the catalog
  never drifts from what review can actually launch.
The catalog now appears only in the non-interactive discovery view; the
interactive wizard already lists selectable agents, so showing the catalog
first was redundant. Scripted --set-* writes just confirm.
- New optional agent.ModelLister capability (agent.ModelInfo + AsModelLister).
- Curated, clearly-labeled model lists for claude-code, codex, gemini (their
  CLIs have no --list-models). --model still forwards any value the CLI accepts.
- entire review --models lists advertised models per review agent (optionally
  filtered by --agent); needs no repo/profile.
- Designed so agents with a live model command (e.g. Pi's pi --list-models)
  can implement ListModels by shelling out later.
Symmetric with --models: --agents lists the worker agents valid for --agent
in the resolved profile (with hook-install status and the master marked),
falling back to the available review-agent catalog when no profile exists.
Replace the free-text 'Model for <agent>' input with a select of the agent's
advertised models (via agent.ModelLister), plus 'Default' and 'Custom…' (free
text) options. Falls back to free text when an agent advertises no models.
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