Remove Pi-specifics from review-profiles (belong on PR #1313) · Entire
Remove Pi-specifics from review-profiles (belong on PR #1313)
34a4b83→main·
dipree·1mo ago·6 files·+4 added/-496 removed
The Pi reviewer adapter and Pi model lister were mistakenly added here; they live on the stacked review-pi-reviewer branch (PR #1313), which is based on review-profiles and already carries a fuller Pi integration (reviewer + live models + generate). Strip them so review-profiles is the clean general scout feature with no Pi:
- delete pi/reviewer.go, pi/reviewer_test.go, pi/models.go, pi/models_test.go
- drop the pi case + import from launchableReviewerFor
- de-Pi the ModelLister doc comment
#1313 will be rebased onto this to re-add Pi on top.
Sessions
1c4621c47750View transcript
[?
List me all the potential command combinations for review.Pi·Opus 4.8·2 steps](/content/gh/entireio/cli/session/019e9fb5-004d-7145-8f48-db6a240d0ab3#timeline-1c4621c47750/index.html)
Changes
6
cmd/entire/cli
agent
Mmodel_lister.go+4/-5
pi
Dmodels.go-54
Dmodels_test.go-48
Dreviewer.go-180
Dreviewer_test.go-206
Mreview_bridge.go-3
14 unmodified lines
15
16
17
18
19
20
21
22
18
19
20
21
22
23
24
14 unmodified lines
// ModelLister is an optional capability for agents that can advertise the
// models usable with `entire scout --model`.
//
// Pi enumerates models live by shelling out to `pi --list-models`. claude-code
// advertises a small curated list of real, valid aliases (opus/sonnet/haiku).
// Agents whose CLI has no enumeration command (codex, gemini) do not implement
// this interface at all; the picker then offers only Default + Custom, since
// `--model` ultimately accepts anything the agent CLI does.
// claude-code advertises a small curated list of real, valid aliases
// (opus/sonnet/haiku). Agents whose CLI has no enumeration command do not
// implement this interface at all; the picker then offers only Default +
// Custom, since `--model` ultimately accepts anything the agent CLI does.
type ModelLister interface {
Agent
}
Mcmd/entire/cli/agent/model_lister.go+4/-5
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
package pi
import (
"context"
"fmt"
"os/exec"
"strings"
"github.com/entireio/cli/cmd/entire/cli/agent"
)
var _ agent.ModelLister = (*PiAgent)(nil)
// ListModels returns the models Pi can run, fetched live from `pi
// --list-models`. Pi is the only supported agent whose CLI can enumerate
// models, so this is a real list rather than a curated/example one. The model
// picker always offers Default and Custom on top of this, and falls back to
// Default + Custom when the call fails (returned error).
func (a *PiAgent) ListModels(ctx context.Context) ([]agent.ModelInfo, error) {
bin, err := exec.LookPath("pi")
if err != nil {
return nil, fmt.Errorf("pi not found on PATH: %w", err)
}
// `pi --list-models` prints the table to stderr, so capture combined output.
out, err := exec.CommandContext(ctx, bin, "--list-models").CombinedOutput()
if err != nil {
return nil, fmt.Errorf("pi --list-models: %w", err)
}
return parsePiModelList(string(out)), nil
}
// parsePiModelList parses the tabular `pi --list-models` output into ModelInfo
// values. The output is a whitespace-aligned table:
//
// provider model context max-out thinking images
// anthropic claude-opus-4-5 200K 64K yes yes
//
// The first two columns (provider, model) become the "provider/model" id Pi
// accepts via --model; the header row and malformed lines are skipped.
func parsePiModelList(output string) []agent.ModelInfo {
var models []agent.ModelInfo
for _, line := range strings.Split(output, "\n") {
fields := strings.Fields(line)
if len(fields) < 2 {
continue
}
provider, model := fields[0], fields[1]
if provider == "provider" && model == "model" {
continue // header row
}
models = append(models, agent.ModelInfo{ID: provider + "/" + model})
}
return models
}
Dcmd/entire/cli/agent/pi/models.go-54
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
package pi
import (
"testing"
"github.com/entireio/cli/cmd/entire/cli/agent"
)
// TestPiAgent_IsModelLister locks Pi in as the one agent with live enumeration.
func TestPiAgent_IsModelLister(t *testing.T) {
t.Parallel()
if _, ok := agent.AsModelLister(NewPiAgent()); !ok {
t.Fatal("PiAgent should implement agent.ModelLister")
}
}
func TestParsePiModelList(t *testing.T) {
t.Parallel()
output := `provider model context max-out thinking images\
anthropic claude-opus-4-5 200K 64K yes yes\
anthropic claude-sonnet-4-5 200K 64K yes yes\
openai gpt-5 400K 128K yes yes\
`
models := parsePiModelList(output)
want := []string{
"anthropic/claude-opus-4-5",
"anthropic/claude-sonnet-4-5",
"openai/gpt-5",
}
if len(models) != len(want) {
t.Fatalf("got %d models, want %d: %+v", len(models), len(want), models)
}
for i, w := range want {
if models[i].ID != w {
t.Errorf("models[%d].ID = %q, want %q", i, models[i].ID, w)
}
}
}
func TestParsePiModelList_SkipsHeaderAndBlanks(t *testing.T) {
t.Parallel()
if got := parsePiModelList("provider model context\n\n \n"); len(got) != 0 {
t.Fatalf("expected no models from header/blank-only output, got %+v", got)
}
if got := parsePiModelList(""); got != nil {
t.Fatalf("expected nil for empty output, got %+v", got)
}
}
}
Dcmd/entire/cli/agent/pi/models_test.go-48
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
package pi
import (
"bufio"
"context"
"encoding/json"
"fmt"
"io"
"os"
"os/exec"
"github.com/entireio/cli/cmd/entire/cli/review"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
)
// piReviewMaxScannerBuf is the bufio.Scanner cap for the Pi review parser.
// 64MB matches the claude-code and codex parsers so all three tolerate the
// same worst-case line length (a tool result packed into one JSON envelope).
const piReviewMaxScannerBuf = 64 * 1024 * 1024
// NewReviewer returns the AgentReviewer for the Pi coding agent.
//
// Argv shape: pi --mode json <prompt> [--model <pattern>]. The prompt is a
// positional argument; stdin is unused. Stdout is newline-delimited JSON
// event envelopes (one event per line, see Pi's docs/json.md), which the
// parser decodes into the review Event stream.
//
// Pi spawned this way is a child process of `entire review`, so it inherits
// the `ENTIRE_REVIEW_*` env vars set by AppendReviewEnv. Pi's lifecycle hook
// then self-tags the session as a review via the shared adoptReviewEnv path —
// no marker file and no `entire attach` are needed.
func NewReviewer() *reviewtypes.ReviewerTemplate {
return &reviewtypes.ReviewerTemplate{
AgentName: "pi",
BuildCmd: buildReviewCmd,
Parser: parsePiOutput,
}
}
// buildReviewCmd builds the exec.Cmd for a Pi review run.
// Exposed at package level for test inspection of argv and env.
func buildReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd {
prompt := review.ComposeReviewPrompt(cfg)
args := []string{"--mode", "json", prompt}
if cfg.Model != "" {
args = append(args, "--model", cfg.Model)
}
cmd := exec.CommandContext(ctx, "pi", args...)
cmd.Env = review.AppendReviewEnv(os.Environ(), "pi", cfg, prompt)
return cmd
}
// piStreamEnvelope is one line of `pi --mode json` stdout. Only the fields the
// review parser consumes are declared; unknown fields and event types are
// ignored so new Pi event types don't break parsing.
type piStreamEnvelope struct {
Type string `json:"type"`
ToolName string `json:"toolName"`
Args json.RawMessage `json:"args"`
Message *piStreamMessage `json:"message"`
}
type piStreamMessage struct {
Role string `json:"role"`
Content json.RawMessage `json:"content"`
Usage *piStreamUsage `json:"usage"`
}
// piStreamUsage mirrors pi-ai's Usage shape (token counts only).
type piStreamUsage struct {
Input int `json:"input"`
Output int `json:"output"`
CacheRead int `json:"cacheRead"`
CacheWrite int `json:"cacheWrite"`
}
type piStreamContentBlock struct {
Type string `json:"type"`
Text string `json:"text"`
}
// parsePiOutput converts `pi --mode json` stdout into a stream of review
// Events. Each stdout line is one JSON envelope:
// - {"type":"session",...} session header; swallowed
// - {"type":"agent_start"} covered by the leading Started event
// - {"type":"message_end","message":{role:"assistant",content:[...],usage}}
// assistant text → AssistantText; usage tallied for the final Tokens event
// - {"type":"tool_execution_start","toolName":..,"args":..} → ToolCall
// - {"type":"agent_end",...} marks a clean completion
//
// Emits Started first, Finished{Success:...} last. Success is true only when
// an agent_end envelope was observed. On a scanner error (torn stream) it
// emits RunError then Finished{Success:false}.
//
// Token math: Pi reports per-message usage on each assistant message. Output
// tokens are summed across messages; input is taken from the last assistant
// message (its context already includes prior turns, so summing input would
// double-count). These are advisory live totals — the authoritative figures
// are hydrated from the persisted transcript after the run.
func parsePiOutput(r io.Reader) <-chan reviewtypes.Event {
return parsePiOutputBuf(r, piReviewMaxScannerBuf)
}
// parsePiOutputBuf is the parameterized variant of parsePiOutput, used by
// tests to shrink the scanner cap so the "token too long" branch can be
// exercised without writing 64MB of fixture data.
func parsePiOutputBuf(r io.Reader, maxBuf int) <-chan reviewtypes.Event {
out := make(chan reviewtypes.Event, 32)
go func() {
defer close(out)
out <- reviewtypes.Started{}
scanner := bufio.NewScanner(r)
scanner.Buffer(make([]byte, min(1024*1024, maxBuf)), maxBuf)
var sawEnd bool
var sumOut, lastIn int
for scanner.Scan() {
line := scanner.Bytes()
if len(line) == 0 {
continue
}
var env piStreamEnvelope
if err := json.Unmarshal(line, &env); err != nil {
out <- reviewtypes.RunError{Err: fmt.Errorf("pi json stream: %w", err)}
continue
}
switch env.Type {
case "message_end":
if env.Message == nil || env.Message.Role != "assistant" {
continue
}
for _, text := range assistantTextBlocks(env.Message.Content) {
if text != "" {
out <- reviewtypes.AssistantText{Text: text}
}
}
if u := env.Message.Usage; u != nil {
sumOut += u.Output
lastIn = u.Input + u.CacheRead
}
case "tool_execution_start":
out <- reviewtypes.ToolCall{Name: env.ToolName, Args: string(env.Args)}
case "agent_end":
sawEnd = true
}
}
if err := scanner.Err(); err != nil {
out <- reviewtypes.RunError{Err: fmt.Errorf("read stdout: %w", err)}
out <- reviewtypes.Finished{Success: false}
return
}
out <- reviewtypes.Tokens{In: lastIn, Out: sumOut}
out <- reviewtypes.Finished{Success: sawEnd}
}
}()
return out
}
// assistantTextBlocks extracts the text from a Pi assistant message's content,
// which is normally an array of content blocks but may be a plain JSON string.
// Non-text blocks (tool calls, etc.) are ignored — tool activity is surfaced
// via the dedicated tool_execution_start envelope instead.
func assistantTextBlocks(content json.RawMessage) []string {
if len(content) == 0 {
return nil
}
var blocks []piStreamContentBlock
if err := json.Unmarshal(content, &blocks); err == nil {
var texts []string
for _, b := range blocks {
if b.Type == "text" && b.Text != "" {
texts = append(texts, b.Text)
}
}
return texts
}
var s string
if err := json.Unmarshal(content, &s); err == nil && s != "" {
return []string{s}
}
return nil
}
Dcmd/entire/cli/agent/pi/reviewer.go-180
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
package pi
import (
"context"
"strings"
"testing"
"github.com/entireio/cli/cmd/entire/cli/agent"
"github.com/entireio/cli/cmd/entire/cli/review"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
)
// Compile-time interface check: ReviewerTemplate implements AgentReviewer.
var _ reviewtypes.AgentReviewer = (*reviewtypes.ReviewerTemplate)(nil)
const wantAgentName = "pi"
// TestReviewer_NameMatchesRegistryKey locks the reviewer's name to the agent
// registry's stable key. adoptReviewEnv compares ENTIRE_REVIEW_AGENT against
// string(ag.Name()); drift here silently breaks review-session self-tagging.
func TestReviewer_NameMatchesRegistryKey(t *testing.T) {
t.Parallel()
if wantAgentName != string(agent.AgentNamePi) {
t.Fatalf("wantAgentName = %q, agent.AgentNamePi = %q — keep these aligned",
wantAgentName, string(agent.AgentNamePi))
}
if got := NewReviewer().Name(); got != wantAgentName {
t.Errorf("Name() = %q, want %q", got, wantAgentName)
}
}
func TestReviewer_EnvVarsSet(t *testing.T) {
t.Parallel()
cfg := reviewtypes.RunConfig{
Skills: []string{"/pr-review-toolkit:review-pr"},
PerRunPrompt: "Focus on the auth module.",
StartingSHA: "abc123def456",
}
cmd := buildReviewCmd(context.Background(), cfg)
envMap := make(map[string]string)
for _, e := range cmd.Env {
if idx := strings.IndexByte(e, '='); idx >= 0 {
envMap[e[:idx]] = e[idx+1:]
}
}
for _, key := range []string{review.EnvSession, review.EnvAgent, review.EnvSkills, review.EnvPrompt, review.EnvStartingSHA} {
if _, ok := envMap[key]; !ok {
t.Errorf("env var %s not set on cmd", key)
}
}
if envMap[review.EnvSession] != "1" {
t.Errorf("%s = %q, want \"1\"", review.EnvSession, envMap[review.EnvSession])
}
if envMap[review.EnvAgent] != wantAgentName {
t.Errorf("%s = %q, want %q", review.EnvAgent, envMap[review.EnvAgent], wantAgentName)
}
if envMap[review.EnvStartingSHA] != "abc123def456" {
t.Errorf("%s = %q, want %q", review.EnvStartingSHA, envMap[review.EnvStartingSHA], "abc123def456")
}
}
func TestReviewer_ArgvShape(t *testing.T) {
t.Parallel()
// Without a model: pi --mode json <prompt>
cmd := buildReviewCmd(context.Background(), reviewtypes.RunConfig{PerRunPrompt: "extra"})
if len(cmd.Args) != 4 {
t.Fatalf("expected 4 args, got %d: %v", len(cmd.Args), cmd.Args)
}
if cmd.Args[0] != "pi" || cmd.Args[1] != "--mode" || cmd.Args[2] != "json" {
t.Errorf("argv prefix = %v, want [pi --mode json ...]", cmd.Args[:3])
}
if cmd.Args[3] == "" {
t.Error("Args[3] (prompt) is empty")
}
if cmd.Stdin != nil {
t.Errorf("cmd.Stdin = %v, want nil (pi receives prompt via argv)", cmd.Stdin)
}
// With a model: --model <pattern> is appended.
cmd = buildReviewCmd(context.Background(), reviewtypes.RunConfig{Model: "sonnet:high"})
if len(cmd.Args) != 6 || cmd.Args[4] != "--model" || cmd.Args[5] != "sonnet:high" {
t.Errorf("argv = %v, want [... --model sonnet:high]", cmd.Args)
}
}
// collectEvents drains the parser channel into a slice for assertions.
func collectEvents(ch <-chan reviewtypes.Event) []reviewtypes.Event {
var evs []reviewtypes.Event
for e := range ch {
ev = append(evs, e)
}
return evs
}
func TestParsePiOutput_FullStream(t *testing.T) {
t.Parallel()
stream := strings.Join([]string{
`{"type":"session","version":3,"id":"uuid","cwd":"/repo"}`,
`{"type":"agent_start"}`,
`{"type":"turn_start"}`,
`{"type":"tool_execution_start","toolCallId":"t1","toolName":"read","args":{"path":"main.go"}}`,
`{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Found an issue."}],"usage":{"input":100,"output":20,"cacheRead":10}}}`,
`{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Here is the fix."}],"usage":{"input":150,"output":30,"cacheRead":40}}}`,
`{"type":"agent_end","messages":[]}`,
}, "\n")
ev := collectEvents(parsePiOutput(strings.NewReader(stream)))
if len(evs) == 0 {
t.Fatal("no events produced")
}
if _, ok := evs[0].(reviewtypes.Started); !ok {
t.Errorf("first event = %T, want Started", evs[0])
}
var texts []string
var sawToolRead bool
var tokens reviewtypes.Tokens
var finished reviewtypes.Finished
var sawTokens, sawFinished bool
for _, e := range evs {
switch ev := e.(type) {
case reviewtypes.AssistantText:
texts = append(texts, ev.Text)
case reviewtypes.ToolCall:
if ev.Name == "read" {
sawToolRead = true
}
case reviewtypes.Tokens:
tokens, sawTokens = ev, true
case reviewtypes.Finished:
finished, sawFinished = ev, true
}
}
if got := strings.Join(texts, "|"); got != "Found an issue.|Here is the fix." {
t.Errorf("assistant text = %q, want both messages in order", got)
}
if !sawToolRead {
t.Error("expected ToolCall for read")
}
if !sawTokens {
t.Fatal("expected a Tokens event")
}
// Output summed across messages: 20+30 = 50; input from last message: 150+40 = 190.
if tokens.Out != 50 {
t.Errorf("tokens.Out = %d, want 50", tokens.Out)
}
if tokens.In != 190 {
t.Errorf("tokens.In = %d, want 190", tokens.In)
}
if !sawFinished || !finished.Success {
t.Errorf("Finished = %+v (saw=%v), want Success=true", finished, sawFinished)
}
}
// TestParsePiOutput_NoAgentEndIsFailure: a torn stream that never reaches
// agent_end must report Finished{Success:false}.
func TestParsePiOutput_NoAgentEndIsFailure(t *testing.T) {
t.Parallel()
stream := `{"type":"agent_start"}
{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"partial"}]}}
`
ev := collectEvents(parsePiOutput(strings.NewReader(stream)))
last := evs[len(evs)-1]
fin, ok := last.(reviewtypes.Finished)
if !ok {
t.Fatalf("last event = %T, want Finished", last)
}
if fin.Success {
t.Error("Finished.Success = true, want false (no agent_end)")
}
}
// TestParsePiOutput_StringContent: assistant content may be a plain string
// rather than an array of blocks.
func TestParsePiOutput_StringContent(t *testing.T) {
t.Parallel()
stream := `{"type":"message_end","message":{"role":"assistant","content":"plain string body"}}
{"type":"agent_end"}`
var got string
for _, e := range collectEvents(parsePiOutput(strings.NewReader(stream))) {
if at, ok := e.(reviewtypes.AssistantText); ok {
got = at.Text
}
}
if got != "plain string body" {
t.Errorf("assistant text = %q, want %q", got, "plain string body")
}
}
// TestParsePiOutput_IgnoresUserMessages: tool-result echoes (role != assistant)
// must not surface as assistant text.
func TestParsePiOutput_IgnoresUserMessages(t *testing.T) {
t.Parallel()
stream := `{"type":"message_end","message":{"role":"toolResult","content":[{"type":"text","text":"file contents"}]}}
{"type":"agent_end"}`
for _, e := range collectEvents(parsePiOutput(strings.NewReader(stream))) {
if _, ok := e.(reviewtypes.AssistantText); ok {
t.Errorf("non-assistant message leaked as AssistantText: %+v", e)
}
}
}
Dcmd/entire/cli/agent/pi/reviewer_test.go-206
11 unmodified lines
12
13
14
15
15
16
17
24 unmodified lines
42
43
44
46
47
45
46
47
11 unmodified lines
"github.com/entireio/cli/cmd/entire/cli/agent/claudecode"
"github.com/entireio/cli/cmd/entire/cli/agent/codex"
"github.com/entireio/cli/cmd/entire/cli/agent/geminicli"
"github.com/entireio/cli/cmd/entire/cli/agent/pi"
cliReview "github.com/entireio/cli/cmd/entire/cli/review"
reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
)
}