Skip to content

fix(pi): Pi read, search, and edit rows show the same label on mobile as on web - #29864

Open
saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/pi-tool-row-labels
Open

saphid wants to merge 1 commit into
pingdotgg:mainfrom
saphid:fix/pi-tool-row-labels

Conversation

@saphid

@saphid saphid commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Problem

On mobile, rows for Pi's built-in tools show only the raw tool name. A Pi read shows "Read" with no file, grep/find/ls show "Grep"/"Find"/"Ls", and a single edit/write shows "Edit"/"Write". Web shows "Read src/a.ts", "Searched foo in src", and the changed file for the same items, and so does mobile for every other provider.

The cause is that the Pi adapter sets each file-change and generic tool item's title to the Pi tool name. Mobile's itemSummary treats a non-empty title as the label, while web mostly works the label out from the item. No other adapter does this: Cursor leaves title null, and Claude, OpenCode, ACP, and Cursor set read and search titles with formatReadToolLabel/formatSearchToolLabel.

Change

emitToolItem in packages/provider-pi/src/server/adapter.ts now sets title: null on file_change and dynamic_tool items, like Cursor's default. Both clients then derive the same label. MCP tools keep their title from the mcpToolPresentation spread that follows. Command items still use bash/powershell, which an existing test pins to tell the two shells apart.

Scope and approval

Very small, focused fix for an obvious bug, so no prior issue: one adapter labels tools differently from all of its siblings, and the change is 2 lines plus 2 assertions in existing tests.

Overlap search: I searched the PR index and GitHub for Pi tool titles and labels. #27607 (Pi task lists) adds a separate todo-list emitter and does not touch these lines. #16529 (Cua) and #27521 (nested Pi tools) do not change Pi item titles either.

Verification

  • Red on main: the new assertions in adapter.test.ts fail with AssertionError: expected 'read' to equal null, and the file-change assertion in "persists edit patches and write content on file change items" also fails.
  • Green: vp test run src/server/adapter.test.ts (packages/provider-pi) passes 93/93, including after rebasing on current main.
  • vp run check:changed passes: format, lint, knip, typecheck (9 packages), tests 94/94.
  • Runtime check of mobile buildThreadFeed with a throwaway test (not committed), using Pi-shaped items:
    title=raw  dynamic_tool/read summary="Read"                detail=null
    title=raw  dynamic_tool/grep summary="Grep"                detail=null
    title=raw  file_change       summary="Edit"                detail="src/a.ts"
    title=null dynamic_tool/read summary="Read src/a.ts"       detail=null
    title=null dynamic_tool/grep summary="Searched foo in src" detail=null
    title=null file_change       summary="Changed src/a.ts"    detail="src/a.ts"
    
  • Not checked: a real Pi session in the mobile app or a screenshot capture. The label change comes from the client logic shown above.

Reviewed with sol-loop: 1 round with GPT-6.1 Sol on high. Sol verdict: approved.

Created with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

… as on web

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 3, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b16fd34

Macroscope's review found this PR approvable — Pi tool rows now use existing descriptive labels instead of raw tool names, aligning mobile presentation with web. The change affects display metadata only, without changing tool execution or permissions.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The Pi adapter now emits null titles for edit/write file-change items and dynamic-tool items. Dynamic-tool names remain in toolName; tests assert the updated title values.

Changes: Pi adapter item titles

Layer / File(s) Summary
Item title mapping and assertions
packages/provider-pi/src/server/adapter.ts, packages/provider-pi/src/server/adapter.test.ts
Edit/write file-change items and dynamic-tool items use null titles. Dynamic-tool names remain in toolName. Tests assert the updated values.

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Bug fix

Suggested labels: bugfix, size:XS

Suggested reviewers: juliusmarminge, only21mil, macroscopeapp

Merge Risk: Minimal · up to b16fd

The title changes align Pi output with both clients without introducing an actionable merge risk.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the fix and the affected area: Pi read, search, and edit rows showing the same label on mobile as on web. It matches the main change in adapter.ts, where file_change and dynamic_tool i…
Description check Passed The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section gives the cause and expected behavior, with examples of the …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the fix and the affected area: Pi read, search, and edit rows showing the same label on mobile as on web. It matches the main change in adapter.ts, where file_change and dynamic_tool items now use a null title. The title is specific and concise. It uses a conventional commit prefix, which the repository template recommends.

Full details: Description check

Explanation

The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections. The Problem section gives the cause and expected behavior, with examples of the mobile and web labels. The Change section explains the 2-line fix in emitToolItem and why MCP and command items are unaffected. The Scope section gives a reason for skipping a prior issue: a very small, focused fix of an obvious bug. It also cites an overlap search. The Verification section lists the failing-then-passing tests, the full check run, a runtime check with observed output, and what was not checked. No UI screenshots are included. The template asks for before/after screenshots for UI changes. The author states that a real Pi session and screenshot capture were not checked, so this gap is disclosed but not resolved. The description also includes the required model and harness line.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ast-grep (0.51.1)

ast-grep could not parse rule config: /ast-grep-rules/ast-grep-essentials/rules/cpp/security/world-writable-file-cpp.yml

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between e1e520c and b16fd34.

Files selected for processing: 2
  • packages/provider-pi/src/server/adapter.test.ts
  • packages/provider-pi/src/server/adapter.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

This branch has not been deployed

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant