Skip to content

Commit ad225ff

Browse files
peyton-altclaude
andcommitted
feat(review): role-driven UX cutover + TUI (staging, fix picker, muted 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
1 parent 242e9bb commit ad225ff

31 files changed

Lines changed: 2876 additions & 1255 deletions

cmd/entire/cli/mdrender/mdrender.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,19 @@ const DefaultTerminalWidth = 80
3636
// of which indicate a malformed StyleConfig (programmer error) rather
3737
// than a runtime condition. Renderer panics are recovered and returned as
3838
// errors so callers can fall back to raw markdown instead of crashing.
39-
func Render(markdown string, width int, darkBackground bool) (rendered string, err error) {
39+
func Render(markdown string, width int, darkBackground bool) (string, error) {
40+
return renderWithStyles(markdown, width, stylesForBackground(darkBackground))
41+
}
42+
43+
// RenderMuted is Render with a low-chroma palette: hierarchy is conveyed by
44+
// bold + indentation rather than colour, and inline-code/link highlighting is
45+
// dropped. Use it for dense, markdown-heavy output (e.g. the multi-agent
46+
// review dump) where the full palette reads as noisy and hard to scan.
47+
func RenderMuted(markdown string, width int, darkBackground bool) (string, error) {
48+
return renderWithStyles(markdown, width, mutedStyles(darkBackground))
49+
}
50+
51+
func renderWithStyles(markdown string, width int, styles ansi.StyleConfig) (rendered string, err error) {
4052
defer func() {
4153
if r := recover(); r != nil {
4254
rendered = ""
@@ -45,7 +57,7 @@ func Render(markdown string, width int, darkBackground bool) (rendered string, e
4557
}()
4658

4759
renderer, err := glamour.NewTermRenderer(
48-
glamour.WithStyles(stylesForBackground(darkBackground)),
60+
glamour.WithStyles(styles),
4961
glamour.WithWordWrap(width),
5062
glamour.WithPreservedNewLines(),
5163
)
@@ -73,6 +85,15 @@ func RenderForWriter(w io.Writer, markdown string) (string, error) {
7385
return Render(markdown, terminalWidth(w), termenv.HasDarkBackground())
7486
}
7587

88+
// RenderMutedForWriter is RenderForWriter using the low-chroma palette (see
89+
// RenderMuted). Non-terminal / NO_COLOR writers still get raw markdown.
90+
func RenderMutedForWriter(w io.Writer, markdown string) (string, error) {
91+
if !shouldRender(w) {
92+
return markdown, nil
93+
}
94+
return RenderMuted(markdown, terminalWidth(w), termenv.HasDarkBackground())
95+
}
96+
7697
// shouldRender returns true if w is a terminal writer and NO_COLOR is unset.
7798
func shouldRender(w io.Writer) bool {
7899
if os.Getenv("NO_COLOR") != "" {
@@ -168,6 +189,36 @@ func stylesForBackground(darkBackground bool) ansi.StyleConfig {
168189
return styles
169190
}
170191

192+
// mutedStyles returns a calmer variant of the CLI palette for dense,
193+
// markdown-heavy output (the multi-agent review dump). It KEEPS the coloured,
194+
// bold headings — they're sparse and give the same scannable structure as
195+
// dispatch — and only neutralises the HIGH-FREQUENCY inline elements that
196+
// multiply with dense findings and read as noise: the highlight block behind
197+
// inline code (file paths), coloured list bullets, and coloured links. Bold
198+
// emphasis (e.g. severity labels) is left intact.
199+
func mutedStyles(darkBackground bool) ansi.StyleConfig {
200+
styles := stylesForBackground(darkBackground)
201+
neutral := "252"
202+
if !darkBackground {
203+
neutral = "234"
204+
}
205+
// Inline code: drop the background highlight and the orange foreground —
206+
// file paths appear in nearly every finding, so the block + accent read as
207+
// a sea of colour. Keep it as plain (slightly emphasised by mono) text.
208+
styles.Code.Color = strPtr(neutral)
209+
styles.Code.BackgroundColor = nil
210+
// List bullets / enumeration markers: neutral, not orange/indigo.
211+
styles.Item.Color = strPtr(neutral)
212+
styles.Enumeration.Color = strPtr(neutral)
213+
// Links: keep the underline as the affordance, drop the colour + bold so a
214+
// finding full of [file](path) links isn't multi-coloured.
215+
styles.Link.Color = nil
216+
styles.Link.Underline = boolPtrV(true)
217+
styles.LinkText.Color = nil
218+
styles.LinkText.Bold = boolPtrV(false)
219+
return styles
220+
}
221+
171222
// chromaForBackground returns the syntax-highlighting palette for code
172223
// blocks. Dark and light backgrounds use distinct text colors but share
173224
// the same accent colors for keywords/functions/literals.

cmd/entire/cli/review/banner.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package review
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
// formatContextBanner returns the transparency block printed below the scope
9+
// banner. It itemises the prior checkpoint/session context `entire review` is
10+
// folding into the agent prompt so the user can see exactly what's being
11+
// reviewed — the value over running the underlying skill manually. The block
12+
// is never omitted; the empty variant reassures the user nothing went wrong,
13+
// there simply is no history.
14+
//
15+
// Example:
16+
//
17+
// Checkpoints in scope (2):
18+
// • a3b2c4d5 feat(review): emit honest live tokens
19+
// • b4c3d5e6 feat(review): flag-driven roles
20+
// In-progress sessions (1):
21+
// • ac3d5c6e Claude Code
22+
//
23+
// When counts are present but the itemised slices aren't populated (defensive),
24+
// it falls back to a one-line count summary.
25+
func formatContextBanner(r ContextResult) string {
26+
if r.Checkpoints == 0 && r.Sessions == 0 {
27+
return "No prior session or checkpoint context for this branch yet."
28+
}
29+
var b strings.Builder
30+
switch {
31+
case len(r.CheckpointItems) > 0:
32+
fmt.Fprintf(&b, "Checkpoints in scope (%d):\n", len(r.CheckpointItems))
33+
for _, c := range r.CheckpointItems {
34+
summary := c.Summary
35+
if summary == "" {
36+
summary = "(no summary)"
37+
}
38+
fmt.Fprintf(&b, " • %s %s\n", c.ID, summary)
39+
}
40+
case r.Checkpoints > 0:
41+
fmt.Fprintf(&b, "%s in scope.\n", pluralizeContextNoun(r.Checkpoints, "checkpoint", "checkpoints"))
42+
}
43+
switch {
44+
case len(r.SessionItems) > 0:
45+
fmt.Fprintf(&b, "In-progress sessions (%d):\n", len(r.SessionItems))
46+
for _, s := range r.SessionItems {
47+
fmt.Fprintf(&b, " • %s %s\n", s.ID, s.Agent)
48+
}
49+
case r.Sessions > 0:
50+
fmt.Fprintf(&b, "%s in progress.\n", pluralizeContextNoun(r.Sessions, "session", "sessions"))
51+
}
52+
return strings.TrimRight(b.String(), "\n")
53+
}
54+
55+
// pluralizeContextNoun returns "<n> <singular>" when n == 1 and
56+
// "<n> <plural>" otherwise. Kept private to banner.go; the review package
57+
// has no other plural cases that would justify a shared utility.
58+
func pluralizeContextNoun(n int, singular, plural string) string {
59+
if n == 1 {
60+
return fmt.Sprintf("%d %s", n, singular)
61+
}
62+
return fmt.Sprintf("%d %s", n, plural)
63+
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package review
2+
3+
import "testing"
4+
5+
// TestFormatContextBanner pins the itemised scope banner: an empty state, the
6+
// itemised checkpoints+sessions layout, and the count-only fallback used when
7+
// items aren't populated.
8+
func TestFormatContextBanner(t *testing.T) {
9+
t.Parallel()
10+
11+
tests := []struct {
12+
name string
13+
in ContextResult
14+
want string
15+
}{
16+
{
17+
name: "neither",
18+
in: ContextResult{},
19+
want: "No prior session or checkpoint context for this branch yet.",
20+
},
21+
{
22+
name: "itemised checkpoints and sessions",
23+
in: ContextResult{
24+
Checkpoints: 2, Sessions: 1,
25+
CheckpointItems: []CheckpointScopeItem{
26+
{ID: "a3b2c4d5", Summary: "feat(review): emit honest live tokens"},
27+
{ID: "b4c3d5e6", Summary: "feat(review): flag-driven roles"},
28+
},
29+
SessionItems: []SessionScopeItem{
30+
{ID: "ac3d5c6e", Agent: "Claude Code"},
31+
},
32+
},
33+
want: "Checkpoints in scope (2):\n" +
34+
" • a3b2c4d5 feat(review): emit honest live tokens\n" +
35+
" • b4c3d5e6 feat(review): flag-driven roles\n" +
36+
"In-progress sessions (1):\n" +
37+
" • ac3d5c6e Claude Code",
38+
},
39+
{
40+
name: "sessions listed by short id and agent",
41+
in: ContextResult{
42+
Sessions: 2,
43+
SessionItems: []SessionScopeItem{
44+
{ID: "ac3d5c6e", Agent: "Claude Code"},
45+
{ID: "3d4c9f88", Agent: "Codex"},
46+
},
47+
},
48+
want: "In-progress sessions (2):\n" +
49+
" • ac3d5c6e Claude Code\n" +
50+
" • 3d4c9f88 Codex",
51+
},
52+
{
53+
name: "count-only fallback when items absent",
54+
in: ContextResult{Checkpoints: 3, Sessions: 1},
55+
want: "3 checkpoints in scope.\n1 session in progress.",
56+
},
57+
{
58+
name: "empty summary renders placeholder",
59+
in: ContextResult{
60+
Checkpoints: 1,
61+
CheckpointItems: []CheckpointScopeItem{{ID: "a3b2c4d5"}},
62+
},
63+
want: "Checkpoints in scope (1):\n • a3b2c4d5 (no summary)",
64+
},
65+
}
66+
for _, tc := range tests {
67+
t.Run(tc.name, func(t *testing.T) {
68+
t.Parallel()
69+
if got := formatContextBanner(tc.in); got != tc.want {
70+
t.Errorf("formatContextBanner(%+v) =\n%q\nwant\n%q", tc.in, got, tc.want)
71+
}
72+
})
73+
}
74+
}
75+
76+
func TestPluralizeContextNoun(t *testing.T) {
77+
t.Parallel()
78+
79+
tests := []struct {
80+
n int
81+
want string
82+
}{
83+
{n: 1, want: "1 checkpoint"},
84+
{n: 2, want: "2 checkpoints"},
85+
{n: 0, want: "0 checkpoints"},
86+
}
87+
for _, tc := range tests {
88+
if got := pluralizeContextNoun(tc.n, "checkpoint", "checkpoints"); got != tc.want {
89+
t.Errorf("pluralizeContextNoun(%d) = %q, want %q", tc.n, got, tc.want)
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)