Skip to content

Add Pi review-runner adapter#1313

Draft
dipree wants to merge 10 commits into
review-profilesfrom
review-pi-reviewer
Draft

Add Pi review-runner adapter#1313
dipree wants to merge 10 commits into
review-profilesfrom
review-pi-reviewer

Conversation

@dipree
Copy link
Copy Markdown
Contributor

@dipree dipree commented Jun 1, 2026

Summary

Adds Pi as a first-class entire review worker by implementing its AgentReviewer adapter. This is the concrete proof that the previous "only claude-code/codex/gemini" limit was just missing adapters, not a launch-capability wall.

Stacked on #1312 (review profiles). Review/merge that first.

What changed

  • cmd/entire/cli/agent/pi/reviewer.goNewReviewer() returning a ReviewerTemplate:
    • BuildCmd: pi --mode json --print [--model <model>] <prompt> with ENTIRE_REVIEW_* env via AppendReviewEnv
    • Parser: maps Pi's --mode json AgentSessionEvent stream (message_update text deltas, tool_execution_start, message_end/turn_end usage, agent_end) into Entire's review Event stream. Falls back to message_end content text when no deltas streamed; uses stopReason for success.
  • cmd/entire/cli/agent/pi/generate.goGenerateText so Pi can also serve as a master / summary provider.
  • Wiring: launchableReviewerFor returns pi.NewReviewer(); Pi added to summary provider binaries, default-profile agents, master-preference order, and config/help text.
  • Session matching: Pi runs are matched to their lifecycle sessions by model too (same-agent/different-model workers resolve correctly); agent_end backfills the model via ExtractModel.

Testing

  • reviewer_test.go: name, argv/env shape with --model, JSON event-stream parsing, message-end fallback text
  • manifest test for same-agent/different-model disambiguation
  • go vet ./cmd/entire/cli/agent/pi/...
  • env -u PI_CODING_AGENT go test ./cmd/entire/cli/...
  • targeted review integration tests

Note

Medium Risk
Touches review orchestration, manifest session matching, and summary provider selection; behavior is well covered by unit tests but depends on Pi CLI JSON event stability.

Overview
Pi is wired into entire review as a first-class worker alongside Claude Code, Codex, and Gemini, using the shared ReviewerTemplate pattern instead of marker fallback.

A new pi review adapter runs pi --mode json --print [--model …] <prompt> with ENTIRE_REVIEW_* env, and maps Pi’s NDJSON session stream (text deltas, tools, usage, stopReason, agent_end) into Entire review events, including fallback text from message_end when no deltas streamed. GenerateText (pi --print --no-tools --no-session) lets Pi act as a review master and checkpoint summary provider.

Integration: launchableReviewerFor returns pi.NewReviewer(); Pi is added to summary-provider binaries, default profile agents (prompt-driven like Gemini), master preference order, and user-facing configure/help text. On agent_end, TurnEnd now sets Model from the captured session transcript via ExtractModel.

Manifest matching uses tighter model ID normalization (strip provider prefix / tier suffix) so multiple Pi workers with different models map to the correct lifecycle sessions; tests cover argv/env, JSON parsing, and same-agent/model disambiguation.

Reviewed by Cursor Bugbot for commit 27f6781. Configure here.

Copilot AI review requested due to automatic review settings June 1, 2026 19:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Pi as a first-class entire review worker by wiring a new AgentReviewer adapter that runs pi --mode json --print and maps Pi's NDJSON AgentSessionEvent stream into Entire review events. Pi is also added as a summary/text-generation provider and to the default profile / master preference order, and review-run model matching is tightened so multiple Pi workers using different models map to the correct lifecycle sessions.

Changes:

  • New agent/pi/reviewer.go (+ tests) implementing ReviewerTemplate for Pi, and new agent/pi/generate.go so Pi can serve as a summary/master provider.
  • review_bridge.launchableReviewerFor returns pi.NewReviewer(); Pi added to summary provider binaries, default profile workers, master preference order, and user-facing help/error text.
  • review/manifest.go normalizes model IDs (strips provider prefix and tier suffix) for same-agent/different-model session matching; pi/lifecycle.go backfills Model on TurnEnd via ExtractModel.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cmd/entire/cli/agent/pi/reviewer.go New Pi review adapter: builds argv/env and parses Pi's JSON event stream into review events.
cmd/entire/cli/agent/pi/reviewer_test.go Unit tests for adapter name, argv/env shape, JSON event parsing, and message-end fallback text.
cmd/entire/cli/agent/pi/generate.go Implements GenerateText so Pi can act as a master / summary provider.
cmd/entire/cli/agent/pi/lifecycle.go Populates Model on TurnEnd by extracting from the captured Pi session transcript.
cmd/entire/cli/agent/text_generator_cli.go Registers Pi binary (pi) as a summary-capable provider.
cmd/entire/cli/review_bridge.go Wires pi.NewReviewer() into launchableReviewerFor.
cmd/entire/cli/review/manifest.go Tighter model-ID normalization (strip provider prefix / tier suffix) for session matching.
cmd/entire/cli/review/manifest_test.go Adds disambiguation test for same-agent/different-model worker → session mapping.
cmd/entire/cli/review/profile.go Adds Pi to default profile agents and master preference order; updates error messaging.
cmd/entire/cli/review/picker.go Updates guided-setup error message to mention Pi.
cmd/entire/cli/review/cmd.go Doc-comment update listing Pi among adapter-backed agents.
cmd/entire/cli/review/types/template.go Doc-comment update listing Pi as a template user.
cmd/entire/cli/explain_summary_provider.go Adds Pi to the "no summary-capable provider" error guidance.
cmd/entire/cli/settings/settings.go Doc comment lists Pi as a valid summary provider.
cmd/entire/cli/setup.go --summarize-agent help text mentions Pi.
docs/architecture/review-command.md Documents Pi as a prompt/model-driven worker in profiles and adapter list.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 27f6781. Configure here.

Comment thread cmd/entire/cli/review/manifest.go
@dipree dipree force-pushed the review-pi-reviewer branch from 27f6781 to c660e06 Compare June 2, 2026 10:19
@dipree
Copy link
Copy Markdown
Contributor Author

dipree commented Jun 2, 2026

Addressed the Cursor Bugbot finding and rebased onto the updated #1312.

  • Model tier suffix dropped in matching (manifest.go): the model matcher now lives on the base PR and strips the provider prefix + thinking-level suffix. Since a session's recorded model name never carries the thinking-level suffix, two pi workers that differ only by :high vs :low genuinely share the same session model id — so disambiguation falls back to start-time + the used-session set in matchReviewSessionState, which still links each worker to a distinct session (no omission). This is documented on compactReviewModelID.

Also fixed the lint findings in reviewer_test.go (unchecked type assertions → comma-ok). golangci-lint run ./cmd/entire/cli/agent/pi/... ./cmd/entire/cli/review/... is clean and the Pi adapter tests pass.

Note: the earlier test-core red was a flaky temp-dir cleanup in TestAttach_SessionAlreadyTracked_NoCheckpoint (unlinkat .git/objects: directory not empty), unrelated to this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants