feat(review): role-driven UX cutover + TUI (PR 3 of 3)#1352
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ 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 ba16605. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR completes the entire review redesign by improving codex review correctness (skill invocation/discovery, live token tracking, manifest inclusion) and upgrading the review/fix UX (role-driven setup, staging view, unified fix picker, muted rendering), while keeping review configuration clone-local for privacy.
Changes:
- Add per-agent review knobs (
model,reasoning_effort) and thread them through reviewer run config (notably codex-m/-c model_reasoning_effort=). - Replace spawn-time picker flows with role-driven configuration (
entire review setup), pre-launch staging/inline prompts, post-review fix prompt, and a unified interactive fix picker. - Improve codex integration: discover
$skill-form skills, tail rollout JSONL for live token counts, and ensure codex output is included in fix manifests even without hook-tagged sessions.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/settings/settings.go | Add model / reasoning_effort to persisted ReviewConfig and update zero-check logic. |
| cmd/entire/cli/review/types/reviewer.go | Thread per-spawn model/effort overrides into reviewer RunConfig. |
| cmd/entire/cli/review/tui_model.go | Calm dashboard header styling; improve token column rendering for input-only vs final totals. |
| cmd/entire/cli/review/tui_model_test.go | Add coverage for token display branches in the dashboard. |
| cmd/entire/cli/review/synthesis_sink.go | Switch synthesis rendering to muted markdown palette. |
| cmd/entire/cli/review/setup.go | Implement role-first review setup, invoker detection, opt-in reviewer defaults, banner improvements. |
| cmd/entire/cli/review/setup_test.go | Update setup tests for clone-local persistence + opt-in role defaults + banner annotations. |
| cmd/entire/cli/review/setup_invoker_test.go | Add tests for invoker env-sentinel detection. |
| cmd/entire/cli/review/post_review.go | Add staging view + inline per-run prompt + post-review fix prompt/launcher + clone-local persistence for “Always”. |
| cmd/entire/cli/review/post_review_test.go | Add tests covering findings counting and post-review fix prompt behavior. |
| cmd/entire/cli/review/picker.go | Remove legacy picker/setup flows; codex skill verification becomes advisory; keep selection helpers. |
| cmd/entire/cli/review/picker_test.go | Pin codex advisory skill verification behavior. |
| cmd/entire/cli/review/multipicker.go | Remove spawn-time multi-agent picker (roles drive reviewer membership). |
| cmd/entire/cli/review/multipicker_test.go | Remove tests for deleted multi-picker. |
| cmd/entire/cli/review/manifest.go | Include session-less sources with output (codex exec) in fix manifests; improve filename handle selection. |
| cmd/entire/cli/review/manifest_test.go | Update command strings and add coverage for “output but no session” manifest inclusion. |
| cmd/entire/cli/review/inline_prompt.go | Add single-key reader helper for post-review fix prompt. |
| cmd/entire/cli/review/inline_prompt_test.go | Add tests for ReadSingleKey behavior. |
| cmd/entire/cli/review/flags.go | Add --reviewers / --fixer one-off override resolution + merging with saved skills/prompt. |
| cmd/entire/cli/review/flags_test.go | Add tests for flag override resolution + merge behavior. |
| cmd/entire/cli/review/fix.go | Add entire review fix subcommand, recursion guard, unified interactive fix picker, severity parsing improvements, and FixerOf-based resolution. |
| cmd/entire/cli/review/fix_findings_test.go | Add parsing tests for codex bolded severity bullets and severity title matching. |
| cmd/entire/cli/review/export_test.go | Export run-config mapping helper for external tests; remove multipicker export. |
| cmd/entire/cli/review/dump.go | Switch per-agent dump rendering to muted markdown palette. |
| cmd/entire/cli/review/cmd_test.go | Update dispatch tests for role-driven flow, recursion guard, flags behavior, and model/effort mapping. |
| cmd/entire/cli/review/banner.go | Add context transparency banner formatting helper. |
| cmd/entire/cli/review/banner_test.go | Add tests for context banner output and pluralization. |
| cmd/entire/cli/review_context.go | Return structured context result (prompt + counts + items) for staging/banner and prompt composition. |
| cmd/entire/cli/review_context_test.go | Update tests for new context result return type and counts. |
| cmd/entire/cli/review_bridge.go | Remove unused deps field wiring related to legacy picker behavior. |
| cmd/entire/cli/mdrender/mdrender.go | Add muted rendering mode and wire through writer helpers; refactor renderer to accept styles. |
| cmd/entire/cli/lifecycle.go | Treat any non-empty ENTIRE_REVIEW_SESSION as a review session marker for env adoption. |
| cmd/entire/cli/agent/skilldiscovery/scan.go | Introduce shared skill discovery scanners + version selection + frontmatter parsing. |
| cmd/entire/cli/agent/skilldiscovery/registry.go | Remove codex hardcoded /review; adjust install-hint strings for huh rendering. |
| cmd/entire/cli/agent/skilldiscovery/registry_test.go | Update assertions for codex curated built-ins now being empty. |
| cmd/entire/cli/agent/codex/reviewer.go | Pass skills verbatim, add model/effort flags, add rollout token tailer + per-turn token emission. |
| cmd/entire/cli/agent/codex/reviewer_test.go | Update tests for verbatim skill pass-through, flag mapping, and per-turn token emission. |
| cmd/entire/cli/agent/codex/review_tokens.go | Add rollout JSONL token tailer to emit live cumulative token totals. |
| cmd/entire/cli/agent/codex/review_tokens_test.go | Add tests for rollout token parsing and tailing appended lines. |
| cmd/entire/cli/agent/codex/discovery.go | Implement codex $name/$plugin:name skill discovery using shared scanners. |
| cmd/entire/cli/agent/codex/discovery_test.go | Add discovery tests for user, plugin cache, and superpowers layouts. |
| cmd/entire/cli/agent/claudecode/testdata/stream_with_deltas.jsonl | Add fixture for Claude stream-json usage snapshot vs final result usage. |
| cmd/entire/cli/agent/claudecode/reviewer.go | Emit input-only live Tokens on assistant envelopes and final totals on result. |
| cmd/entire/cli/agent/claudecode/reviewer_test.go | Update tests for new live-token behavior and fixture expectations. |
| cmd/entire/cli/agent/claudecode/discovery.go | Delegate discovery logic to shared skilldiscovery package scanners. |
| CLAUDE.md | Update docs for role-driven review flow, new commands, and removed legacy constructs. |
- Codex skill seeding: add seedDefaultSkills, used by the invoker-only fallback and the --reviewers/--fixer override. When an agent ships no curated built-ins (codex's review skills are discovered on disk, not bundled), fall back to on-disk discovered skills whose name signals a review skill, so codex invokes e.g. $code-reviewer instead of running a generic scope-only prompt. Thread ctx through mergeFlagOverrideWithSavedSkills. - Completion footer: print for a session-less codex-only manifest (sources with output but no SessionID). Skip only when the manifest is truly empty. The fix command omits the handle when there is none — `entire review fix --all` resolves to the most-recent run. - inline_prompt: correct KeyChoice doc (ReadSingleKey returns Default on EOF/Enter; it does not detect TTY — callers gate on CanPromptInteractively) and rename the misleading "NonTTY" test to "EmptyReader". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: e4ee64104fd8
811dc0e to
c9e19a2
Compare
c9e19a2 to
9f28084
Compare
ad225ff to
a4af9de
Compare
…d output) The command-layer redesign of `entire review`, stacked on the codex correctness PR. Drops the legacy in-flow picker; reviewer membership is decided up front by roles (`entire review setup`) or one-off --reviewers/--fixer flags. - Role resolution + invoker-aware non-interactive fallback; recursion guard on ENTIRE_REVIEW_SESSION; `entire review fix` promoted to a real subcommand (legacy --fix hidden). Deletes the spawn-time multi-agent picker. - Pre-launch staging view: scope banner + itemised checkpoints/sessions + optional per-run prompt in one huh form before fan-out. - Inline post-review fix prompt [Y]es/[s]elect/[n]o/[A]lways; unified navigable source⇄findings fix picker (aggregate selectable); FixerOf is the single source of truth for the fix agent. - Live-token display (TUI input-only during streaming) and muted markdown palette for dense review/synthesis output. - Opt-in roles (new agents default Skip) with a ≥1-reviewer guard. - Addresses PR review feedback: codex skill seeding via seedDefaultSkills (name-matched on-disk discovery when curated builtins are empty), codex-only completion footer, inline_prompt doc/test naming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 91311075f208
a4af9de to
2dbd78a
Compare

Stacked on #1370 (PR 2: codex review correctness), which is stacked on #1241 (PR 1: role + setup foundation). Diffed against
review-codex.The command-layer redesign of
entire review. Originally bundled with codex correctness; that agent-layer work was peeled into #1370 so this PR is purely the cobra/huh/TUI surface (noagent/*churn here).What this adds
entire review setup) or one-off--reviewers/--fixerflags. Invoker-aware non-interactive fallback; recursion guard onENTIRE_REVIEW_SESSION. Deletes the spawn-time multi-agent picker.entire review fixpromoted to a real subcommand (legacy--fixhidden).FixerOf(s)is the single source of truth for the fix agent.[Y]es · [s]elect · [n]o · [A]lways; unified navigable source⇄findings fix picker (aggregate selectable).seedDefaultSkills, codex-only completion footer,inline_promptdoc/test naming. Removed a stray.antigravitycliconfig committed in error.Verified: build + vet clean,
mise run lint0 issues, full unit suite green (underTZ=UTC; the one local failure is a pre-existing timezone-sensitiveauthtest, green on CI).🤖 Generated with Claude Code