From 2525680b12421c8a0756545aeb435c44ac4b47e2 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Mon, 1 Jun 2026 21:22:59 +0200 Subject: [PATCH 1/7] Add Pi review runner adapter --- cmd/entire/cli/agent/pi/generate.go | 25 +++ cmd/entire/cli/agent/pi/lifecycle.go | 17 ++ cmd/entire/cli/agent/pi/reviewer.go | 216 +++++++++++++++++++++ cmd/entire/cli/agent/pi/reviewer_test.go | 140 +++++++++++++ cmd/entire/cli/agent/text_generator_cli.go | 1 + cmd/entire/cli/explain_summary_provider.go | 2 +- cmd/entire/cli/review/cmd.go | 2 +- cmd/entire/cli/review/manifest_test.go | 53 +++++ cmd/entire/cli/review/picker.go | 2 +- cmd/entire/cli/review/profile.go | 6 +- cmd/entire/cli/review/types/template.go | 2 +- cmd/entire/cli/review_bridge.go | 7 +- cmd/entire/cli/settings/settings.go | 2 +- cmd/entire/cli/setup.go | 2 +- docs/architecture/review-command.md | 11 +- 15 files changed, 472 insertions(+), 16 deletions(-) create mode 100644 cmd/entire/cli/agent/pi/generate.go create mode 100644 cmd/entire/cli/agent/pi/reviewer.go create mode 100644 cmd/entire/cli/agent/pi/reviewer_test.go diff --git a/cmd/entire/cli/agent/pi/generate.go b/cmd/entire/cli/agent/pi/generate.go new file mode 100644 index 0000000000..2ef941e248 --- /dev/null +++ b/cmd/entire/cli/agent/pi/generate.go @@ -0,0 +1,25 @@ +package pi + +import ( + "context" + "fmt" + + "github.com/entireio/cli/cmd/entire/cli/agent" +) + +// GenerateText sends a prompt to Pi in non-interactive text mode and returns +// the raw response. The prompt is passed as a positional message because Pi's +// CLI consumes prompts from argv in --print mode. +func (a *PiAgent) GenerateText(ctx context.Context, prompt string, model string) (string, error) { + args := []string{"--print", "--no-tools", "--no-session"} + if model != "" { + args = append(args, "--model", model) + } + args = append(args, prompt) + + result, err := agent.RunIsolatedTextGeneratorCLI(ctx, nil, "pi", "pi", args, "") + if err != nil { + return "", fmt.Errorf("pi text generation failed: %w", err) + } + return result, nil +} diff --git a/cmd/entire/cli/agent/pi/lifecycle.go b/cmd/entire/cli/agent/pi/lifecycle.go index 0417525431..e309bd31b8 100644 --- a/cmd/entire/cli/agent/pi/lifecycle.go +++ b/cmd/entire/cli/agent/pi/lifecycle.go @@ -183,6 +183,7 @@ func (a *PiAgent) ParseHookEvent(ctx context.Context, hookName string, stdin io. Type: agent.TurnEnd, SessionID: sessionID, SessionRef: sessionRef, + Model: extractModelFromPiSessionFile(sessionRef), Timestamp: now, }, nil @@ -268,6 +269,22 @@ func cacheSessionID(ctx context.Context, id string) { } } +func extractModelFromPiSessionFile(path string) string { + if path == "" { + return "" + } + //nolint:gosec // path comes from Pi's hook payload or our captured transcript path + data, err := os.ReadFile(path) + if err != nil { + return "" + } + model, err := (&PiAgent{}).ExtractModel(data) + if err != nil { + return "" + } + return model +} + func readCachedSessionID(ctx context.Context) string { dir := resolveSessionDir(ctx) //nolint:gosec // path constructed from validated repo root diff --git a/cmd/entire/cli/agent/pi/reviewer.go b/cmd/entire/cli/agent/pi/reviewer.go new file mode 100644 index 0000000000..edf0b0721f --- /dev/null +++ b/cmd/entire/cli/agent/pi/reviewer.go @@ -0,0 +1,216 @@ +package pi + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + + "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" +) + +// NewReviewer returns the AgentReviewer for Pi. +// +// Argv shape: pi --mode json --print [--model ] . +// The prompt is passed as a positional message because Pi's CLI accepts prompts +// as message arguments in non-interactive mode. Stdout is newline-delimited JSON +// session events; the parser maps Pi's AgentSessionEvent stream into Entire's +// review Event stream. +func NewReviewer() *reviewtypes.ReviewerTemplate { + return &reviewtypes.ReviewerTemplate{ + AgentName: string(agent.AgentNamePi), + BuildCmd: buildPiReviewCmd, + Parser: parsePiReviewOutput, + } +} + +func buildPiReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd { + prompt := review.ComposeReviewPrompt(cfg) + args := []string{"--mode", "json", "--print"} + if cfg.Model != "" { + args = append(args, "--model", cfg.Model) + } + args = append(args, prompt) + cmd := exec.CommandContext(ctx, "pi", args...) + cmd.Env = review.AppendReviewEnv(os.Environ(), string(agent.AgentNamePi), cfg, prompt) + return cmd +} + +func parsePiReviewOutput(r io.Reader) <-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, piReviewMaxScannerBuf)), piReviewMaxScannerBuf) + messageIDsWithTextDelta := map[string]struct{}{} + finished := false + success := true + + for scanner.Scan() { + line := scanner.Bytes() + if len(line) == 0 { + continue + } + var env piReviewEnvelope + if err := json.Unmarshal(line, &env); err != nil { + out <- reviewtypes.RunError{Err: fmt.Errorf("pi --mode json: %w", err)} + continue + } + + switch env.Type { + case "session", "agent_start", "turn_start", "queue_update", "compaction_start", "compaction_end", "auto_retry_start", "auto_retry_end": + // Session/control events do not map to user-visible review output. + case "message_update": + if text := env.AssistantMessageEvent.TextDelta(); text != "" { + messageIDsWithTextDelta[env.MessageID()] = struct{}{} + out <- reviewtypes.AssistantText{Text: text} + } + case "message_end": + if env.Message.Role == "assistant" { + if env.Message.StopReason == "error" || env.Message.StopReason == "aborted" { + success = false + } + if env.Message.Usage != nil { + out <- piReviewTokens(env.Message.Usage) + } + if _, sawDelta := messageIDsWithTextDelta[env.MessageID()]; !sawDelta { + if text := piReviewMessageText(env.Message.Content); text != "" { + out <- reviewtypes.AssistantText{Text: text} + } + } + } + case "tool_execution_start": + out <- reviewtypes.ToolCall{Name: env.ToolName, Args: piReviewJSONArg(env.Args)} + case "tool_execution_end": + // Tool errors are part of normal agent execution (for example grep + // finding no matches). The agent's stopReason determines review + // success/failure. + case "turn_end": + if env.Message.StopReason == "error" || env.Message.StopReason == "aborted" { + success = false + } + if env.Message.Usage != nil { + out <- piReviewTokens(env.Message.Usage) + } + case "agent_end": + finished = true + out <- reviewtypes.Finished{Success: success} + default: + // Unknown future events are ignored; Pi's event stream is additive. + } + } + + if err := scanner.Err(); err != nil { + out <- reviewtypes.RunError{Err: fmt.Errorf("read stdout: %w", err)} + out <- reviewtypes.Finished{Success: false} + return + } + if !finished { + out <- reviewtypes.Finished{Success: false} + } + }() + return out +} + +const piReviewMaxScannerBuf = 64 * 1024 * 1024 + +type piReviewEnvelope struct { + Type string `json:"type"` + ID string `json:"id"` + Message piReviewMessage `json:"message"` + AssistantMessageEvent piAssistantMessageEvent `json:"assistantMessageEvent"` + ToolName string `json:"toolName"` + Args json.RawMessage `json:"args"` +} + +func (e piReviewEnvelope) MessageID() string { + if e.Message.ID != "" { + return e.Message.ID + } + return e.ID +} + +type piReviewMessage struct { + ID string `json:"id"` + Role string `json:"role"` + Content json.RawMessage `json:"content"` + Usage *piReviewUsage `json:"usage"` + StopReason string `json:"stopReason"` +} + +type piAssistantMessageEvent struct { + Type string `json:"type"` + Delta string `json:"delta"` + Text string `json:"text"` +} + +func (e piAssistantMessageEvent) TextDelta() string { + switch e.Type { + case "text_delta": + if e.Delta != "" { + return e.Delta + } + return e.Text + default: + return "" + } +} + +type piReviewUsage struct { + Input int `json:"input"` + Output int `json:"output"` + CacheRead int `json:"cacheRead"` + CacheWrite int `json:"cacheWrite"` +} + +func piReviewTokens(usage *piReviewUsage) reviewtypes.Tokens { + if usage == nil { + return reviewtypes.Tokens{} + } + return reviewtypes.Tokens{ + In: usage.Input + usage.CacheRead + usage.CacheWrite, + Out: usage.Output, + } +} + +func piReviewJSONArg(raw json.RawMessage) string { + if len(raw) == 0 || string(raw) == "null" { + return "" + } + return string(raw) +} + +func piReviewMessageText(raw json.RawMessage) string { + if len(raw) == 0 { + return "" + } + var s string + if err := json.Unmarshal(raw, &s); err == nil { + return s + } + var items []struct { + Type string `json:"type"` + Text string `json:"text"` + } + if err := json.Unmarshal(raw, &items); err != nil { + return "" + } + text := "" + for _, item := range items { + if item.Type != "text" || item.Text == "" { + continue + } + if text != "" { + text += "\n" + } + text += item.Text + } + return text +} diff --git a/cmd/entire/cli/agent/pi/reviewer_test.go b/cmd/entire/cli/agent/pi/reviewer_test.go new file mode 100644 index 0000000000..9e1234c613 --- /dev/null +++ b/cmd/entire/cli/agent/pi/reviewer_test.go @@ -0,0 +1,140 @@ +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" +) + +var _ reviewtypes.AgentReviewer = (*reviewtypes.ReviewerTemplate)(nil) + +func TestPiReviewer_NameMatchesRegistryKey(t *testing.T) { + t.Parallel() + if got := NewReviewer().Name(); got != string(agent.AgentNamePi) { + t.Fatalf("Name() = %q, want %q", got, agent.AgentNamePi) + } +} + +func TestPiReviewer_BuildCmd(t *testing.T) { + t.Parallel() + cfg := reviewtypes.RunConfig{ + Model: "anthropic/claude-sonnet-4-5:high", + Task: "Review the change.", + AlwaysPrompt: "Focus on API regressions.", + StartingSHA: "abc123", + } + cmd := buildPiReviewCmd(context.Background(), cfg) + + if cmd.Args[0] != "pi" { + t.Fatalf("Args[0] = %q, want pi; args=%v", cmd.Args[0], cmd.Args) + } + wantPrefix := []string{"pi", "--mode", "json", "--print", "--model", "anthropic/claude-sonnet-4-5:high"} + if len(cmd.Args) != len(wantPrefix)+1 { + t.Fatalf("args len = %d, want %d: %v", len(cmd.Args), len(wantPrefix)+1, cmd.Args) + } + for i, want := range wantPrefix { + if cmd.Args[i] != want { + t.Fatalf("Args[%d] = %q, want %q; args=%v", i, cmd.Args[i], want, cmd.Args) + } + } + if prompt := cmd.Args[len(cmd.Args)-1]; !strings.Contains(prompt, "Review the change.") || !strings.Contains(prompt, "Focus on API regressions.") { + t.Fatalf("prompt arg missing composed review content: %q", prompt) + } + + env := envMap(cmd.Env) + if env[review.EnvSession] != "1" { + t.Errorf("%s = %q, want 1", review.EnvSession, env[review.EnvSession]) + } + if env[review.EnvAgent] != string(agent.AgentNamePi) { + t.Errorf("%s = %q, want %q", review.EnvAgent, env[review.EnvAgent], agent.AgentNamePi) + } + if env[review.EnvStartingSHA] != "abc123" { + t.Errorf("%s = %q, want abc123", review.EnvStartingSHA, env[review.EnvStartingSHA]) + } +} + +func TestPiReviewer_ParseJSONEventStream(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"type":"session","version":3,"id":"s1","cwd":"/repo"}`, + `{"type":"agent_start"}`, + `{"type":"turn_start"}`, + `{"type":"message_update","message":{"role":"assistant"},"assistantMessageEvent":{"type":"text_delta","delta":"Finding "}}`, + `{"type":"tool_execution_start","toolName":"bash","args":{"command":"git diff --stat"}}`, + `{"type":"message_update","message":{"role":"assistant"},"assistantMessageEvent":{"type":"text_delta","delta":"one"}}`, + `{"type":"message_end","message":{"role":"assistant","usage":{"input":10,"output":4,"cacheRead":2,"cacheWrite":3},"stopReason":"stop"}}`, + `{"type":"agent_end","messages":[]}`, + }, "\n") + + events := collectPiReviewEvents(input) + if len(events) != 6 { + t.Fatalf("events len = %d, want 6: %#v", len(events), events) + } + if _, ok := events[0].(reviewtypes.Started); !ok { + t.Fatalf("events[0] = %T, want Started", events[0]) + } + if got, ok := events[1].(reviewtypes.AssistantText); !ok || got.Text != "Finding " { + t.Fatalf("events[1] = %#v, want AssistantText{Finding }", events[1]) + } + tool, ok := events[2].(reviewtypes.ToolCall) + if !ok || tool.Name != "bash" || !strings.Contains(tool.Args, "git diff --stat") { + t.Fatalf("events[2] = %#v, want ToolCall(bash)", events[2]) + } + if got, ok := events[3].(reviewtypes.AssistantText); !ok || got.Text != "one" { + t.Fatalf("events[3] = %#v, want AssistantText{one}", events[3]) + } + tokens, ok := events[4].(reviewtypes.Tokens) + if !ok || tokens.In != 15 || tokens.Out != 4 { + t.Fatalf("events[4] = %#v, want Tokens{In:15 Out:4}", events[4]) + } + finished, ok := events[5].(reviewtypes.Finished) + if !ok || !finished.Success { + t.Fatalf("events[5] = %#v, want Finished{Success:true}", events[5]) + } +} + +func TestPiReviewer_ParseMessageEndTextWithoutDeltas(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"type":"agent_start"}`, + `{"type":"message_end","message":{"role":"assistant","content":[{"type":"text","text":"Final review text"}],"stopReason":"stop"}}`, + `{"type":"agent_end"}`, + }, "\n") + + events := collectPiReviewEvents(input) + var found bool + for _, ev := range events { + text, ok := ev.(reviewtypes.AssistantText) + if ok && text.Text == "Final review text" { + found = true + } + } + if !found { + t.Fatalf("expected AssistantText from message_end content, got %#v", events) + } +} + +func collectPiReviewEvents(input string) []reviewtypes.Event { + ch := parsePiReviewOutput(strings.NewReader(input)) + var events []reviewtypes.Event + for ev := range ch { + events = append(events, ev) + } + return events +} + +func envMap(env []string) map[string]string { + out := map[string]string{} + for _, kv := range env { + idx := strings.IndexByte(kv, '=') + if idx < 0 { + continue + } + out[kv[:idx]] = kv[idx+1:] + } + return out +} diff --git a/cmd/entire/cli/agent/text_generator_cli.go b/cmd/entire/cli/agent/text_generator_cli.go index 6a18eb3a20..47e743bed9 100644 --- a/cmd/entire/cli/agent/text_generator_cli.go +++ b/cmd/entire/cli/agent/text_generator_cli.go @@ -76,6 +76,7 @@ var summaryProviderBinaries = map[types.AgentName]string{ AgentNameCopilotCLI: "copilot", AgentNameCursor: "agent", AgentNameGemini: "gemini", + AgentNamePi: "pi", } // IsSummaryCLIAvailable reports whether the CLI binary for a summary-capable diff --git a/cmd/entire/cli/explain_summary_provider.go b/cmd/entire/cli/explain_summary_provider.go index 0f24caecac..573b93da8c 100644 --- a/cmd/entire/cli/explain_summary_provider.go +++ b/cmd/entire/cli/explain_summary_provider.go @@ -61,7 +61,7 @@ func resolveCheckpointSummaryProvider(ctx context.Context, w io.Writer) (*checkp switch len(candidates) { case 0: - return nil, errors.New("no summary-capable provider is available; install claude, codex, gemini, cursor, or copilot, install an external entire-agent-* plugin that declares text_generator, or set summary_generation.provider in settings") + return nil, errors.New("no summary-capable provider is available; install claude, codex, gemini, pi, cursor, or copilot, install an external entire-agent-* plugin that declares text_generator, or set summary_generation.provider in settings") case 1: return autoSelectSummaryProvider(ctx, w, candidates[0].Name, "non-interactive auto-select: single installed provider") default: diff --git a/cmd/entire/cli/review/cmd.go b/cmd/entire/cli/review/cmd.go index 288511e385..a2def5a06a 100644 --- a/cmd/entire/cli/review/cmd.go +++ b/cmd/entire/cli/review/cmd.go @@ -2,7 +2,7 @@ // // cmd.go provides NewCommand(), the cobra entry point for `entire review`. // It routes through the new AgentReviewer / Sink / Run architecture for -// agents with review-runner adapters (claude-code, codex, gemini) and falls +// agents with review-runner adapters (claude-code, codex, gemini, pi) and falls // back to RunMarkerFallback for agents that are not yet wired into that review // runner contract. package review diff --git a/cmd/entire/cli/review/manifest_test.go b/cmd/entire/cli/review/manifest_test.go index 7d0a234ec5..96e32fbba9 100644 --- a/cmd/entire/cli/review/manifest_test.go +++ b/cmd/entire/cli/review/manifest_test.go @@ -370,6 +370,59 @@ func TestBuildLocalReviewManifestFromSummary_GroupsAgentSessionsAndAggregate(t * } } +func TestBuildLocalReviewManifestFromSummary_DisambiguatesSameAgentByModel(t *testing.T) { + started := time.Date(2026, 6, 1, 12, 0, 0, 0, time.UTC) + summary := reviewtypes.RunSummary{ + StartedAt: started, + AgentRuns: []reviewtypes.AgentRun{ + { + Name: "pi-sonnet", + AgentName: "pi", + Model: "anthropic/claude-sonnet:high", + Status: reviewtypes.AgentStatusSucceeded, + Buffer: []reviewtypes.Event{reviewtypes.AssistantText{Text: "Sonnet finding"}}, + }, + { + Name: "pi-opus", + AgentName: "pi", + Model: "opus", + Status: reviewtypes.AgentStatusSucceeded, + Buffer: []reviewtypes.Event{reviewtypes.AssistantText{Text: "Opus finding"}}, + }, + }, + } + states := []*session.State{ + { + SessionID: "opus-session", + Kind: session.KindAgentReview, + WorktreePath: "/repo", + BaseCommit: "abc123", + StartedAt: started.Add(time.Second), + ModelName: "claude-opus-4-1", + }, + { + SessionID: "sonnet-session", + Kind: session.KindAgentReview, + WorktreePath: "/repo", + BaseCommit: "abc123", + StartedAt: started.Add(2 * time.Second), + ModelName: "claude-sonnet-4-5", + }, + } + + manifest := buildLocalReviewManifestFromSummary("/repo", "abc123", summary, states, "") + + if len(manifest.Sources) != 2 { + t.Fatalf("sources = %d, want 2: %#v", len(manifest.Sources), manifest.Sources) + } + if manifest.Sources[0].SessionID != "sonnet-session" || manifest.Sources[0].Label != "pi-sonnet" { + t.Fatalf("sonnet source mismatch: %#v", manifest.Sources[0]) + } + if manifest.Sources[1].SessionID != "opus-session" || manifest.Sources[1].Label != "pi-opus" { + t.Fatalf("opus source mismatch: %#v", manifest.Sources[1]) + } +} + func TestWarnManifestNotWritten_PrintsReasonAndDiagnosticHints(t *testing.T) { var b strings.Builder diff --git a/cmd/entire/cli/review/picker.go b/cmd/entire/cli/review/picker.go index f2dfef2d8c..9e9e7dae32 100644 --- a/cmd/entire/cli/review/picker.go +++ b/cmd/entire/cli/review/picker.go @@ -96,7 +96,7 @@ func RunReviewGuidedSetup( launchable := launchableInstalledAgentNames(installed, reviewerFor) if len(launchable) == 0 { - return "", settings.ReviewProfileConfig{}, errors.New("no agents with review runner adapters and hooks installed; run `entire configure --agent claude-code`, `entire configure --agent codex`, or `entire configure --agent gemini`") + return "", settings.ReviewProfileConfig{}, errors.New("no agents with review runner adapters and hooks installed; run `entire configure --agent claude-code`, `entire configure --agent codex`, `entire configure --agent gemini`, or `entire configure --agent pi`") } profileName = strings.TrimSpace(profileName) diff --git a/cmd/entire/cli/review/profile.go b/cmd/entire/cli/review/profile.go index e49093072d..2b94241ab6 100644 --- a/cmd/entire/cli/review/profile.go +++ b/cmd/entire/cli/review/profile.go @@ -285,7 +285,7 @@ func defaultReviewProfileForInstalledAgents( agents[name] = cfg } if len(agents) == 0 { - return settings.ReviewProfileConfig{}, errors.New("no agents with review runner adapters and hooks installed; run `entire configure --agent claude-code`, `entire configure --agent codex`, or `entire configure --agent gemini`") + return settings.ReviewProfileConfig{}, errors.New("no agents with review runner adapters and hooks installed; run `entire configure --agent claude-code`, `entire configure --agent codex`, `entire configure --agent gemini`, or `entire configure --agent pi`") } return settings.ReviewProfileConfig{ Task: profileTask(profileName, settings.ReviewProfileConfig{}), @@ -304,7 +304,7 @@ func defaultReviewAgentConfig(profileName, agentName string) settings.ReviewConf return settings.ReviewConfig{Skills: []string{"/review"}, Prompt: focus} case string(agent.AgentNameCodex): return settings.ReviewConfig{Skills: []string{"/review"}, Prompt: focus} - case string(agent.AgentNameGemini): + case string(agent.AgentNameGemini), string(agent.AgentNamePi): prompt := "Review the change according to the profile task." if focus != "" { prompt += " " + focus @@ -327,7 +327,7 @@ func defaultProfileFocus(profileName string) string { } func defaultReviewMaster(ctx context.Context, configured map[string]settings.ReviewConfig) string { - for _, preferred := range []string{string(agent.AgentNameClaudeCode), string(agent.AgentNameCodex), string(agent.AgentNameGemini)} { + for _, preferred := range []string{string(agent.AgentNameClaudeCode), string(agent.AgentNameCodex), string(agent.AgentNameGemini), string(agent.AgentNamePi)} { for _, workerName := range sortedReviewConfigKeys(configured) { cfg := configured[workerName] if reviewAgentName(workerName, cfg) == preferred && agentSupportsTextGeneration(ctx, preferred) { diff --git a/cmd/entire/cli/review/types/template.go b/cmd/entire/cli/review/types/template.go index f3fe95f625..0f683ed364 100644 --- a/cmd/entire/cli/review/types/template.go +++ b/cmd/entire/cli/review/types/template.go @@ -4,7 +4,7 @@ // AgentReviewer using two caller-supplied functions: BuildCmd (per-agent // argv/env construction) and Parser (per-agent stdout-to-Event stream). // -// All three currently-supported agents (claude-code, codex, gemini) +// Current adapter-backed review agents (claude-code, codex, gemini, pi) // share the Start/Process/Wait/Events scaffolding. Only the build-cmd // step and the stdout parser genuinely differ. The template owns the // shared lifecycle (spawn → pipe stdout → run parser → forward events diff --git a/cmd/entire/cli/review_bridge.go b/cmd/entire/cli/review_bridge.go index f450b7f951..fb44d5f8dd 100644 --- a/cmd/entire/cli/review_bridge.go +++ b/cmd/entire/cli/review_bridge.go @@ -5,13 +5,14 @@ package cli // access (headHasReviewCheckpoint) and per-agent reviewer constructors // (launchableReviewerFor) live here to avoid the import cycle: // review → checkpoint → codex → review -// review → claudecode/codex/geminicli → review +// review → claudecode/codex/geminicli/pi → review import ( "github.com/entireio/cli/cmd/entire/cli/agent" "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" ) @@ -33,7 +34,7 @@ func buildReviewDeps() cliReview.Deps { // adapter, or nil for agents that are known to Entire but not yet wired into // `entire review` fan-out. This lives in the cli package to avoid the import cycle: // -// review/cmd.go → claudecode/codex/geminicli → review +// review/cmd.go → claudecode/codex/geminicli/pi → review func launchableReviewerFor(agentName string) reviewtypes.AgentReviewer { switch agentName { case string(agent.AgentNameClaudeCode): @@ -42,6 +43,8 @@ func launchableReviewerFor(agentName string) reviewtypes.AgentReviewer { return codex.NewReviewer() case string(agent.AgentNameGemini): return geminicli.NewReviewer() + case string(agent.AgentNamePi): + return pi.NewReviewer() default: return nil } diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index f2f9b54f2c..068c54a1c7 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -163,7 +163,7 @@ type ClonePreferences struct { // checkpoint summaries generated by explain --generate. type SummaryGenerationSettings struct { // Provider is the selected summary provider agent name - // (for example "claude-code", "codex", or "gemini"). + // (for example "claude-code", "codex", "gemini", or "pi"). Provider string `json:"provider,omitempty"` // Model is an optional model hint passed to the selected provider. diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index 5d65344293..aa1c58f910 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -741,7 +741,7 @@ Examples: cmd.Flags().BoolVarP(&opts.ForceHooks, flagForce, "f", false, "Reinstall the Entire git hook") cmd.Flags().BoolVar(&opts.SkipPushSessions, flagSkipPushSessions, false, "Disable automatic pushing of session logs on git push") cmd.Flags().StringVar(&opts.CheckpointRemote, flagCheckpointRemote, "", "Checkpoint remote in provider:owner/repo format (e.g., github:org/checkpoints-repo)") - cmd.Flags().StringVar(&summarizeProvider, flagSummarizeAgent, "", "Set the provider used by explain --generate (e.g., claude-code, codex, gemini, cursor, copilot-cli)") + cmd.Flags().StringVar(&summarizeProvider, flagSummarizeAgent, "", "Set the provider used by explain --generate (e.g., claude-code, codex, gemini, pi, cursor, copilot-cli)") cmd.Flags().StringVar(&summarizeModel, flagSummarizeModel, "", "Set the model hint used by explain --generate") cmd.Flags().IntVar(&summarizeTimeoutSeconds, flagSummarizeTimeout, 0, "Set the hard deadline (seconds) for explain --generate summary generation. 0 clears (falls back to 5m default).") cmd.Flags().BoolVar(&opts.Telemetry, flagTelemetry, true, "Enable anonymous usage analytics") diff --git a/docs/architecture/review-command.md b/docs/architecture/review-command.md index e85d4b092b..26456b992d 100644 --- a/docs/architecture/review-command.md +++ b/docs/architecture/review-command.md @@ -45,7 +45,8 @@ Review profiles are configured in clone-local preferences (or settings) under `r "task": "Review this change for correctness, regressions, tests, and maintainability.", "agents": { "claude-code": {"skills": ["/review"]}, - "codex": {"skills": ["/review"]} + "codex": {"skills": ["/review"]}, + "pi": {"model": "anthropic/claude-sonnet", "prompt": "Review the change according to the profile task."} }, "master": "claude-code" }, @@ -62,14 +63,14 @@ Review profiles are configured in clone-local preferences (or settings) under `r } ``` -`entire review --models` lists the models each review-runner agent advertises via the optional `agent.ModelLister` capability (`cmd/entire/cli/agent/model_lister.go`). Agents whose CLI has no enumeration command (claude-code, codex, gemini) return a curated, non-exhaustive list of common models/aliases; the `--model` flag still forwards any value the agent CLI accepts. Agents whose CLI can enumerate live (e.g. Pi's `pi --list-models`) may implement `ListModels` by shelling out. +`entire review --models` lists the models each review-runner agent advertises via the optional `agent.ModelLister` capability (`cmd/entire/cli/agent/model_lister.go`). claude-code returns a curated list of real aliases (opus/sonnet/haiku); Pi enumerates live by shelling out to `pi --list-models`. Agents whose CLI has no enumeration command (codex, gemini) do not implement `ListModels`, so the picker offers only Default + Custom. The `--model` flag still forwards any value the agent CLI accepts. -The profile-level `task` is the shared work item. Each `agents` map entry is a worker id. For simple entries the worker id is also the agent name; to run the same agent more than once, use aliases and set `agent` plus `model`. Per-worker `skills`, `prompt`, and `model` adapt that task to agent-specific mechanics. Settings fields: `EntireSettings.ReviewProfiles` and `EntireSettings.ReviewDefaultProfile` in `cmd/entire/cli/settings/settings.go`. The old top-level `review` map is no longer used by `entire review`. +The profile-level `task` is the shared work item. Each `agents` map entry is a worker id. For simple entries the worker id is also the agent name; to run the same agent more than once, use aliases and set `agent` plus `model`. Per-worker `skills`, `prompt`, and `model` adapt that task to agent-specific mechanics. Pi is a prompt/model-driven worker (`pi --mode json --print [--model ...]`) rather than a slash-command worker. Settings fields: `EntireSettings.ReviewProfiles` and `EntireSettings.ReviewDefaultProfile` in `cmd/entire/cli/settings/settings.go`. The old top-level `review` map is no longer used by `entire review`. ## How It Works (env-var handshake) 1. `entire review` selects a profile (positional/`--profile` → `review_default_profile` → `general` → only configured profile). If no profiles exist, it runs simple guided setup in an interactive terminal and asks before starting agents, or writes an opinionated clone-local default profile in non-interactive mode. It then composes worker prompts via `review.ComposeReviewPrompt` and computes scope (mainline base ref via `review.ComputeScopeStats`, overridable with `--base`). -2. **For agents with review-runner adapters** (claude-code, codex, gemini-cli): the spawned agent process is given env vars `ENTIRE_REVIEW_{SESSION,AGENT,SKILLS,PROMPT,STARTING_SHA}` that the agent's `UserPromptSubmit` lifecycle hook reads to tag the session as `Kind = "agent_review"` with the configured skills/prompt. Each spawned process has its own env, so multiple worktrees and multi-agent runs are correct by construction (no shared marker file, no race). +2. **For agents with review-runner adapters** (claude-code, codex, gemini-cli, pi): the spawned agent process is given env vars `ENTIRE_REVIEW_{SESSION,AGENT,SKILLS,PROMPT,STARTING_SHA}` that the agent's `UserPromptSubmit` lifecycle hook reads to tag the session as `Kind = "agent_review"` with the configured skills/prompt. Each spawned process has its own env, so multiple worktrees and multi-agent runs are correct by construction (no shared marker file, no race). 3. **For agents without review-runner adapters yet**: `RunMarkerFallback` writes a `PendingReviewMarker` file and prints guidance — the user opens the agent themselves and runs the skills. This is an adapter backlog path, not a statement that the agent cannot be launched headlessly. 4. Worker agents run the selected profile's task; each session ends naturally. 5. In multi-worker profiles, the configured master agent receives all worker reports and produces one critical final report. The master prompt asks it to reject unsupported claims, resolve contradictions, merge duplicates, and prioritize evidence-backed findings. @@ -133,7 +134,7 @@ The redesign eliminated several constructs from the prior implementation. None s - `cmd/entire/cli/review/synthesis_sink.go` / `synthesis_prompt.go` — profile master adjudication (runs automatically for multi-worker profiles) plus the legacy opt-in synthesis path - `cmd/entire/cli/review/types/{reviewer,sink,template}.go` — interface contracts (CU2 + CU4 + CU5b) - `cmd/entire/cli/review/env.go` — `ENTIRE_REVIEW_*` constants + `EncodeSkills`/`DecodeSkills` + `AppendReviewEnv` -- `cmd/entire/cli/agent/{claudecode,codex,geminicli}/reviewer.go` — per-agent `AgentReviewer` implementations (claude-code, codex, gemini-cli) +- `cmd/entire/cli/agent/{claudecode,codex,geminicli,pi}/reviewer.go` — per-agent `AgentReviewer` implementations (claude-code, codex, gemini-cli, pi) - `cmd/entire/cli/agent/claudecode/discovery.go` — skill discovery + `pickLatestVersion` plugin-cache dedupe - `cmd/entire/cli/lifecycle.go` — `adoptReviewEnv` reads `ENTIRE_REVIEW_*` from process env; replaces marker-file adoption - `cmd/entire/cli/review_bridge.go` / `review_helpers.go` — bridge code in `cli` package for cycle-bound functions (`headHasReviewCheckpoint`, `launchableReviewerFor`, `newReviewAttachCmd`) From d91d9958ffa1185df8c7e36c8ad6bc5d014bfa17 Mon Sep 17 00:00:00 2001 From: Daniel Adams Date: Tue, 2 Jun 2026 18:18:57 +0200 Subject: [PATCH 2/7] Implement Pi live model listing for entire review --models Pi has a real model-enumeration command, so ListModels shells out to `pi --list-models` and parses the table into provider/model entries. parsePiModelList is split out and unit-tested (no pi binary needed). Entire-Checkpoint: ebcc924da0df --- cmd/entire/cli/agent/pi/models.go | 49 ++++++++++++++++++++++++++ cmd/entire/cli/agent/pi/models_test.go | 35 ++++++++++++++++++ cmd/entire/cli/review/cmd_test.go | 8 +++-- 3 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 cmd/entire/cli/agent/pi/models.go create mode 100644 cmd/entire/cli/agent/pi/models_test.go diff --git a/cmd/entire/cli/agent/pi/models.go b/cmd/entire/cli/agent/pi/models.go new file mode 100644 index 0000000000..4e11aad625 --- /dev/null +++ b/cmd/entire/cli/agent/pi/models.go @@ -0,0 +1,49 @@ +package pi + +import ( + "bufio" + "context" + "fmt" + "strings" + + "github.com/entireio/cli/cmd/entire/cli/agent" +) + +var _ agent.ModelLister = (*PiAgent)(nil) + +// ListModels returns Pi's live model catalog by shelling out to +// `pi --list-models`. Unlike the curated lists for claude-code/codex/gemini, +// Pi has a real enumeration command spanning every configured provider, so the +// result reflects what this machine/account can actually use. +func (a *PiAgent) ListModels(ctx context.Context) ([]agent.ModelInfo, error) { + out, err := agent.RunIsolatedTextGeneratorCLI(ctx, nil, "pi", "pi", []string{"--list-models"}, "") + if err != nil { + return nil, fmt.Errorf("pi --list-models: %w", err) + } + return parsePiModelList(out), nil +} + +// parsePiModelList parses the tabular `pi --list-models` output. Each non-header +// row is " "; the model +// ID is rendered as "provider/model" (the unambiguous form Pi's --model accepts) +// with the context window kept as a note. +func parsePiModelList(raw string) []agent.ModelInfo { + var models []agent.ModelInfo + scanner := bufio.NewScanner(strings.NewReader(raw)) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + if len(fields) < 2 { + continue + } + provider, model := fields[0], fields[1] + if provider == "provider" && model == "model" { + continue // header row + } + note := "" + if len(fields) >= 3 { + note = fields[2] + " ctx" + } + models = append(models, agent.ModelInfo{ID: provider + "/" + model, Note: note}) + } + return models +} diff --git a/cmd/entire/cli/agent/pi/models_test.go b/cmd/entire/cli/agent/pi/models_test.go new file mode 100644 index 0000000000..82700c4655 --- /dev/null +++ b/cmd/entire/cli/agent/pi/models_test.go @@ -0,0 +1,35 @@ +package pi + +import "testing" + +func TestParsePiModelList(t *testing.T) { + raw := "provider model context max-out thinking images\n" + + "anthropic claude-opus-4-0 200K 32K yes yes \n" + + "openai gpt-5 400K 128K yes no \n" + + "\n" + + "google gemini-2.5-pro 1M 64K yes yes \n" + + got := parsePiModelList(raw) + if len(got) != 3 { + t.Fatalf("parsed %d models, want 3: %#v", len(got), got) + } + want := []struct{ id, note string }{ + {"anthropic/claude-opus-4-0", "200K ctx"}, + {"openai/gpt-5", "400K ctx"}, + {"google/gemini-2.5-pro", "1M ctx"}, + } + for i, w := range want { + if got[i].ID != w.id { + t.Errorf("model[%d].ID = %q, want %q", i, got[i].ID, w.id) + } + if got[i].Note != w.note { + t.Errorf("model[%d].Note = %q, want %q", i, got[i].Note, w.note) + } + } +} + +func TestParsePiModelList_HeaderAndBlanksSkipped(t *testing.T) { + if got := parsePiModelList("provider model\n\n \n"); len(got) != 0 { + t.Fatalf("expected no models, got %#v", got) + } +} diff --git a/cmd/entire/cli/review/cmd_test.go b/cmd/entire/cli/review/cmd_test.go index 849094f940..f5afb528b5 100644 --- a/cmd/entire/cli/review/cmd_test.go +++ b/cmd/entire/cli/review/cmd_test.go @@ -143,8 +143,12 @@ func TestReviewCmd_ListModels(t *testing.T) { t.Errorf("--models output missing %q:\n%s", want, out) } } - if strings.Contains(out, "gpt-5-codex") { - t.Errorf("--models should not invent example codex models:\n%s", out) + // codex has no enumeration command, so its own section must show the + // no-advertised-models note rather than invented examples. (A substring + // check would false-positive on Pi's live list, which legitimately + // includes openai/gpt-5-codex.) + if !strings.Contains(out, "codex:\n (no advertised models") { + t.Errorf("codex section should show no advertised models:\n%s", out) } } From 14a45e8d11c0da30e0ced6949040989f543a4381 Mon Sep 17 00:00:00 2001 From: dipree Date: Mon, 29 Jun 2026 10:41:34 +0200 Subject: [PATCH 3/7] fix(pi review): report cumulative token usage Entire-Checkpoint: 3f7ff1353636 --- cmd/entire/cli/agent/pi/reviewer.go | 35 ++++++++++++---- cmd/entire/cli/agent/pi/reviewer_test.go | 51 ++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/agent/pi/reviewer.go b/cmd/entire/cli/agent/pi/reviewer.go index edf0b0721f..43026e7769 100644 --- a/cmd/entire/cli/agent/pi/reviewer.go +++ b/cmd/entire/cli/agent/pi/reviewer.go @@ -50,6 +50,8 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { scanner := bufio.NewScanner(r) scanner.Buffer(make([]byte, min(1024*1024, piReviewMaxScannerBuf)), piReviewMaxScannerBuf) messageIDsWithTextDelta := map[string]struct{}{} + messageIDsWithUsage := map[string]struct{}{} + tokens := reviewtypes.Tokens{} finished := false success := true @@ -78,7 +80,7 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { success = false } if env.Message.Usage != nil { - out <- piReviewTokens(env.Message.Usage) + emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage) } if _, sawDelta := messageIDsWithTextDelta[env.MessageID()]; !sawDelta { if text := piReviewMessageText(env.Message.Content); text != "" { @@ -97,7 +99,7 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { success = false } if env.Message.Usage != nil { - out <- piReviewTokens(env.Message.Usage) + emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage) } case "agent_end": finished = true @@ -170,14 +172,31 @@ type piReviewUsage struct { CacheWrite int `json:"cacheWrite"` } -func piReviewTokens(usage *piReviewUsage) reviewtypes.Tokens { - if usage == nil { - return reviewtypes.Tokens{} +func emitPiReviewTokens(out chan<- reviewtypes.Event, env piReviewEnvelope, total *reviewtypes.Tokens, seen map[string]struct{}) { + if env.Message.Usage == nil || total == nil { + return } - return reviewtypes.Tokens{ - In: usage.Input + usage.CacheRead + usage.CacheWrite, - Out: usage.Output, + if key := env.MessageID(); key != "" { + if _, ok := seen[key]; ok { + return + } + seen[key] = struct{}{} } + *total = addPiReviewTokens(*total, env.Message.Usage) + out <- *total +} + +func addPiReviewTokens(total reviewtypes.Tokens, usage *piReviewUsage) reviewtypes.Tokens { + if usage == nil { + return total + } + total.In += usage.Input + usage.CacheRead + usage.CacheWrite + total.Out += usage.Output + return total +} + +func piReviewTokens(usage *piReviewUsage) reviewtypes.Tokens { + return addPiReviewTokens(reviewtypes.Tokens{}, usage) } func piReviewJSONArg(raw json.RawMessage) string { diff --git a/cmd/entire/cli/agent/pi/reviewer_test.go b/cmd/entire/cli/agent/pi/reviewer_test.go index 9e1234c613..54bd8005de 100644 --- a/cmd/entire/cli/agent/pi/reviewer_test.go +++ b/cmd/entire/cli/agent/pi/reviewer_test.go @@ -118,6 +118,57 @@ func TestPiReviewer_ParseMessageEndTextWithoutDeltas(t *testing.T) { } } +func TestPiReviewer_ParseTokensAreCumulative(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"type":"agent_start"}`, + `{"type":"message_end","id":"m1","message":{"id":"m1","role":"assistant","usage":{"input":100,"output":50,"cacheRead":10,"cacheWrite":5},"stopReason":"toolUse"}}`, + `{"type":"message_end","id":"m2","message":{"id":"m2","role":"assistant","usage":{"input":200,"output":30,"cacheRead":0,"cacheWrite":0},"stopReason":"stop"}}`, + `{"type":"agent_end"}`, + }, "\n") + + events := collectPiReviewEvents(input) + var tokens []reviewtypes.Tokens + for _, ev := range events { + if tok, ok := ev.(reviewtypes.Tokens); ok { + tokens = append(tokens, tok) + } + } + if len(tokens) != 2 { + t.Fatalf("token events = %d, want 2: %#v", len(tokens), events) + } + if got := tokens[0]; got.In != 115 || got.Out != 50 { + t.Fatalf("first Tokens = %#v, want In=115 Out=50", got) + } + if got := tokens[1]; got.In != 315 || got.Out != 80 { + t.Fatalf("final Tokens = %#v, want In=315 Out=80", got) + } +} + +func TestPiReviewer_ParseTokensDedupesTurnEndForSameMessage(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"type":"agent_start"}`, + `{"type":"message_end","id":"m1","message":{"id":"m1","role":"assistant","usage":{"input":10,"output":5},"stopReason":"stop"}}`, + `{"type":"turn_end","id":"m1","message":{"id":"m1","role":"assistant","usage":{"input":10,"output":5},"stopReason":"stop"}}`, + `{"type":"agent_end"}`, + }, "\n") + + events := collectPiReviewEvents(input) + var tokens []reviewtypes.Tokens + for _, ev := range events { + if tok, ok := ev.(reviewtypes.Tokens); ok { + tokens = append(tokens, tok) + } + } + if len(tokens) != 1 { + t.Fatalf("token events = %d, want 1: %#v", len(tokens), events) + } + if got := tokens[0]; got.In != 10 || got.Out != 5 { + t.Fatalf("Tokens = %#v, want In=10 Out=5", got) + } +} + func collectPiReviewEvents(input string) []reviewtypes.Event { ch := parsePiReviewOutput(strings.NewReader(input)) var events []reviewtypes.Event From 6ce16666092bf9f209d92dc28f9a3c6e9074ca0c Mon Sep 17 00:00:00 2001 From: dipree Date: Mon, 29 Jun 2026 10:45:39 +0200 Subject: [PATCH 4/7] chore(pi review): remove unused token helper Entire-Checkpoint: 04c405a0e030 --- cmd/entire/cli/agent/pi/reviewer.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/entire/cli/agent/pi/reviewer.go b/cmd/entire/cli/agent/pi/reviewer.go index 43026e7769..6beb8e3aef 100644 --- a/cmd/entire/cli/agent/pi/reviewer.go +++ b/cmd/entire/cli/agent/pi/reviewer.go @@ -195,10 +195,6 @@ func addPiReviewTokens(total reviewtypes.Tokens, usage *piReviewUsage) reviewtyp return total } -func piReviewTokens(usage *piReviewUsage) reviewtypes.Tokens { - return addPiReviewTokens(reviewtypes.Tokens{}, usage) -} - func piReviewJSONArg(raw json.RawMessage) string { if len(raw) == 0 || string(raw) == "null" { return "" From e52b748b474d047a5a83767f4337a349f2e04c38 Mon Sep 17 00:00:00 2001 From: dipree Date: Wed, 1 Jul 2026 19:30:00 +0200 Subject: [PATCH 5/7] Mention trail findings in trail help Entire-Checkpoint: c04e8ddfd9cf --- cmd/entire/cli/trail_cmd.go | 6 +++++- cmd/entire/cli/trail_cmd_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/trail_cmd.go b/cmd/entire/cli/trail_cmd.go index 20055896db..d75d3d36f3 100644 --- a/cmd/entire/cli/trail_cmd.go +++ b/cmd/entire/cli/trail_cmd.go @@ -38,6 +38,10 @@ const ( trailFindMaxPages = 10 ) +func trailContextBlurb() string { + return "A trail ties together the context for a branch. Use `entire trail` to view, create, update, or watch it; use `entire trail finding` to manage agent findings." +} + func newTrailCmd() *cobra.Command { var insecureHTTPAuth bool var repoOverride string @@ -54,7 +58,7 @@ func newTrailCmd() *cobra.Command { agentHelpRequiresTrailsAnnotation: agentHelpAnnotationEnabled, }, Args: cobra.NoArgs, - Long: "A trail ties together the context for a branch. Use `entire trail` to view, create, update, or watch it.", + Long: trailContextBlurb(), RunE: func(cmd *cobra.Command, _ []string) error { return cmd.Help() }, diff --git a/cmd/entire/cli/trail_cmd_test.go b/cmd/entire/cli/trail_cmd_test.go index 179a9529e0..562f46d8f2 100644 --- a/cmd/entire/cli/trail_cmd_test.go +++ b/cmd/entire/cli/trail_cmd_test.go @@ -460,7 +460,7 @@ func TestTrailRootPrintsHelp(t *testing.T) { t.Fatalf("execute trail root: %v", err) } text := out.String() - for _, want := range []string{"A trail ties together the context for a branch", "show", "list", "create"} { + for _, want := range []string{"A trail ties together the context for a branch", "`entire trail finding`", "show", "list", "create", "finding"} { if !strings.Contains(text, want) { t.Fatalf("help output missing %q, got:\n%s", want, text) } From 6699ec40abdb7b02c379c9346526cc02d3d2ab30 Mon Sep 17 00:00:00 2001 From: dipree Date: Wed, 1 Jul 2026 19:32:18 +0200 Subject: [PATCH 6/7] Avoid double counting Pi review cache tokens Entire-Checkpoint: 1b1a62d70f07 --- cmd/entire/cli/agent/pi/reviewer.go | 8 +++++++- cmd/entire/cli/agent/pi/reviewer_test.go | 12 ++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/agent/pi/reviewer.go b/cmd/entire/cli/agent/pi/reviewer.go index 6beb8e3aef..05fa28525e 100644 --- a/cmd/entire/cli/agent/pi/reviewer.go +++ b/cmd/entire/cli/agent/pi/reviewer.go @@ -190,7 +190,13 @@ func addPiReviewTokens(total reviewtypes.Tokens, usage *piReviewUsage) reviewtyp if usage == nil { return total } - total.In += usage.Input + usage.CacheRead + usage.CacheWrite + // Pi's usage shape is normalized across providers. For OpenAI-shaped + // backends, cached input is reported as a subset of input tokens; summing + // cacheRead/cacheWrite into Tokens.In would therefore double-count. The + // review event contract has only aggregate input/output fields, so report + // the provider's top-level input total and leave cache detail to transcript + // token accounting, which stores cache fields separately. + total.In += usage.Input total.Out += usage.Output return total } diff --git a/cmd/entire/cli/agent/pi/reviewer_test.go b/cmd/entire/cli/agent/pi/reviewer_test.go index 54bd8005de..2d87d45c25 100644 --- a/cmd/entire/cli/agent/pi/reviewer_test.go +++ b/cmd/entire/cli/agent/pi/reviewer_test.go @@ -88,8 +88,8 @@ func TestPiReviewer_ParseJSONEventStream(t *testing.T) { t.Fatalf("events[3] = %#v, want AssistantText{one}", events[3]) } tokens, ok := events[4].(reviewtypes.Tokens) - if !ok || tokens.In != 15 || tokens.Out != 4 { - t.Fatalf("events[4] = %#v, want Tokens{In:15 Out:4}", events[4]) + if !ok || tokens.In != 10 || tokens.Out != 4 { + t.Fatalf("events[4] = %#v, want Tokens{In:10 Out:4}", events[4]) } finished, ok := events[5].(reviewtypes.Finished) if !ok || !finished.Success { @@ -137,11 +137,11 @@ func TestPiReviewer_ParseTokensAreCumulative(t *testing.T) { if len(tokens) != 2 { t.Fatalf("token events = %d, want 2: %#v", len(tokens), events) } - if got := tokens[0]; got.In != 115 || got.Out != 50 { - t.Fatalf("first Tokens = %#v, want In=115 Out=50", got) + if got := tokens[0]; got.In != 100 || got.Out != 50 { + t.Fatalf("first Tokens = %#v, want In=100 Out=50", got) } - if got := tokens[1]; got.In != 315 || got.Out != 80 { - t.Fatalf("final Tokens = %#v, want In=315 Out=80", got) + if got := tokens[1]; got.In != 300 || got.Out != 80 { + t.Fatalf("final Tokens = %#v, want In=300 Out=80", got) } } From 40cb4d68acfe9ed0ec3fc0344500e80f17b13f10 Mon Sep 17 00:00:00 2001 From: dipree Date: Wed, 1 Jul 2026 19:39:03 +0200 Subject: [PATCH 7/7] Deduplicate no-id Pi review token events Entire-Checkpoint: 90ce539be7b6 --- cmd/entire/cli/agent/pi/reviewer.go | 52 ++++++++++++++++++++---- cmd/entire/cli/agent/pi/reviewer_test.go | 25 ++++++++++++ 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/agent/pi/reviewer.go b/cmd/entire/cli/agent/pi/reviewer.go index 05fa28525e..3146ca78f5 100644 --- a/cmd/entire/cli/agent/pi/reviewer.go +++ b/cmd/entire/cli/agent/pi/reviewer.go @@ -51,6 +51,8 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { scanner.Buffer(make([]byte, min(1024*1024, piReviewMaxScannerBuf)), piReviewMaxScannerBuf) messageIDsWithTextDelta := map[string]struct{}{} messageIDsWithUsage := map[string]struct{}{} + messageUsageByTurn := map[int]map[piReviewUsageKey]struct{}{} + turnNumber := 0 tokens := reviewtypes.Tokens{} finished := false success := true @@ -67,7 +69,9 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { } switch env.Type { - case "session", "agent_start", "turn_start", "queue_update", "compaction_start", "compaction_end", "auto_retry_start", "auto_retry_end": + case "turn_start": + turnNumber++ + case "session", "agent_start", "queue_update", "compaction_start", "compaction_end", "auto_retry_start", "auto_retry_end": // Session/control events do not map to user-visible review output. case "message_update": if text := env.AssistantMessageEvent.TextDelta(); text != "" { @@ -80,7 +84,7 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { success = false } if env.Message.Usage != nil { - emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage) + emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage, messageUsageByTurn, turnNumber) } if _, sawDelta := messageIDsWithTextDelta[env.MessageID()]; !sawDelta { if text := piReviewMessageText(env.Message.Content); text != "" { @@ -99,7 +103,7 @@ func parsePiReviewOutput(r io.Reader) <-chan reviewtypes.Event { success = false } if env.Message.Usage != nil { - emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage) + emitPiReviewTokens(out, env, &tokens, messageIDsWithUsage, messageUsageByTurn, turnNumber) } case "agent_end": finished = true @@ -172,18 +176,52 @@ type piReviewUsage struct { CacheWrite int `json:"cacheWrite"` } -func emitPiReviewTokens(out chan<- reviewtypes.Event, env piReviewEnvelope, total *reviewtypes.Tokens, seen map[string]struct{}) { +type piReviewUsageKey struct { + Input int + Output int + CacheRead int + CacheWrite int +} + +func emitPiReviewTokens(out chan<- reviewtypes.Event, env piReviewEnvelope, total *reviewtypes.Tokens, seen map[string]struct{}, messageUsageByTurn map[int]map[piReviewUsageKey]struct{}, turnNumber int) { if env.Message.Usage == nil || total == nil { return } + if shouldSkipPiReviewUsage(env, seen, messageUsageByTurn, turnNumber) { + return + } + *total = addPiReviewTokens(*total, env.Message.Usage) + out <- *total +} + +func shouldSkipPiReviewUsage(env piReviewEnvelope, seen map[string]struct{}, messageUsageByTurn map[int]map[piReviewUsageKey]struct{}, turnNumber int) bool { + usage := env.Message.Usage + if usage == nil { + return true + } + sig := piReviewUsageKey{Input: usage.Input, Output: usage.Output, CacheRead: usage.CacheRead, CacheWrite: usage.CacheWrite} + if env.Type == "message_end" && messageUsageByTurn != nil { + if messageUsageByTurn[turnNumber] == nil { + messageUsageByTurn[turnNumber] = map[piReviewUsageKey]struct{}{} + } + messageUsageByTurn[turnNumber][sig] = struct{}{} + } if key := env.MessageID(); key != "" { if _, ok := seen[key]; ok { - return + return true } seen[key] = struct{}{} + return false } - *total = addPiReviewTokens(*total, env.Message.Usage) - out <- *total + // Pi streams can emit usage on both message_end and turn_end. Some realistic + // streams omit ids on both events, so fall back to the current turn's usage + // signature to avoid counting a no-id turn_end duplicate of the message_end. + if env.Type == "turn_end" && messageUsageByTurn != nil { + if _, ok := messageUsageByTurn[turnNumber][sig]; ok { + return true + } + } + return false } func addPiReviewTokens(total reviewtypes.Tokens, usage *piReviewUsage) reviewtypes.Tokens { diff --git a/cmd/entire/cli/agent/pi/reviewer_test.go b/cmd/entire/cli/agent/pi/reviewer_test.go index 2d87d45c25..1abbc8cf66 100644 --- a/cmd/entire/cli/agent/pi/reviewer_test.go +++ b/cmd/entire/cli/agent/pi/reviewer_test.go @@ -169,6 +169,31 @@ func TestPiReviewer_ParseTokensDedupesTurnEndForSameMessage(t *testing.T) { } } +func TestPiReviewer_ParseTokensDedupesNoIDTurnEndForSameUsage(t *testing.T) { + t.Parallel() + input := strings.Join([]string{ + `{"type":"agent_start"}`, + `{"type":"turn_start"}`, + `{"type":"message_end","message":{"role":"assistant","usage":{"input":10,"output":5,"cacheRead":2,"cacheWrite":1},"stopReason":"stop"}}`, + `{"type":"turn_end","message":{"role":"assistant","usage":{"input":10,"output":5,"cacheRead":2,"cacheWrite":1},"stopReason":"stop"}}`, + `{"type":"agent_end"}`, + }, "\n") + + events := collectPiReviewEvents(input) + var tokens []reviewtypes.Tokens + for _, ev := range events { + if tok, ok := ev.(reviewtypes.Tokens); ok { + tokens = append(tokens, tok) + } + } + if len(tokens) != 1 { + t.Fatalf("token events = %d, want 1: %#v", len(tokens), events) + } + if got := tokens[0]; got.In != 10 || got.Out != 5 { + t.Fatalf("Tokens = %#v, want In=10 Out=5", got) + } +} + func collectPiReviewEvents(input string) []reviewtypes.Event { ch := parsePiReviewOutput(strings.NewReader(input)) var events []reviewtypes.Event