Skip to content

fix(server): Pi extension notifications render as notices, not tool calls - #15368

Open
blue-az wants to merge 1 commit into
pingdotgg:mainfrom
blue-az:fix/pi-notify-system-notice
Open

blue-az wants to merge 1 commit into
pingdotgg:mainfrom
blue-az:fix/pi-notify-system-notice

Conversation

@blue-az

@blue-az blue-az commented Oct 3, 2026 •

Copy link
Copy Markdown

Problem

Pi extension notifications (ctx.ui.notify) are unreadable in T3 Code. The Pi adapter emits each notify as a dynamic_tool item named "notify", so clients fold it into "Used N tools" and show the message as raw tool-input JSON with literal \n escapes. docs/user/providers-pi.md says notifications appear in the work log, but in practice an extension's output is hidden behind two expanders and arrives as JSON.

Repro: enable Pi, open a thread in a project with a Pi extension that calls ctx.ui.notify from a slash command (here, the operator-control-plane extension's /op:status), and run the command.

Change

PiAdapterV2 now emits notify as a system_notice turn item, the existing type the Claude adapter uses for usage-limit notices. Web and mobile already render system_notice as readable text outside the folded work log, so this needs no contract or client change. Items persisted earlier as dynamic_tool still replay unchanged.

Scope and approval

No prior issue. I think this fits the small, obvious-bug exception: one adapter emits the wrong item type for a provider event, and the fix maps it onto an existing type the clients already render.

One question for maintainers: web styles every system_notice as runtime.warning (orange, alert icon), so info-level notifies now look like warnings. In the screenshots, doctor PASS is shown in orange. system_notice has no level field, so Pi's notifyType (info/warning/error) is dropped. Carrying it through would mean an optional field in packages/contracts plus styling in web and mobile. I kept that out of this PR; happy to follow up if you want that shape.

Verification

  • vp test run packages/provider-pi/src/server/adapter.test.ts: 75/75 pass, including the multiline notification assertion and the ordering-barrier test updated to wait for a system_notice.
  • vp run --filter @t3tools/provider-pi typecheck: passes; only existing Effect suggestions are reported.
  • vp lint packages/provider-pi/src/server/adapter.ts packages/provider-pi/src/server/adapter.test.ts: passes with one pre-existing unused layer warning in adapter.ts.
  • vp fmt --check packages/provider-pi/src/server/adapter.ts packages/provider-pi/src/server/adapter.test.ts: passes.
  • Manual web verification with Pi 1.0.0 and /op:status was performed before the rebase. It was not repeated after the provider package move; mobile was not checked.

Before (main): the report is folded into "Used 2 tools" and shows as raw JSON.

before, collapsed
before, expanded

After (this branch): notices appear inline, and the expanded report keeps its line breaks. The older turn above it still replays as before.

after, collapsed
after, expanded

Done with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 3, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d8126d3

Macroscope's review found this PR approvable — This is a narrow Pi adapter bug fix that maps existing notifications to the already-supported system_notice type, preserving message content while avoiding folded tool-call rendering. The change is covered by a focused test and does not alter schemas, defaults, or production infrastructure.

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

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6e81aaa6-1820-45b5-ae5f-2873d3a67f9c

📥 Commits

Reviewing files that changed from the base of the PR and between d8126d3 and 7edfd22.


📒 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; 9 remain after this review.



📝 Walkthrough

Walkthrough

The Pi adapter now emits notify events as completed system_notice turn items. Updated tests check the new item type and verify the full notification text.

Changes

Pi notification handling

Layer / File(s) Summary
Map notifications to system notices
packages/provider-pi/src/server/adapter.ts, packages/provider-pi/src/server/adapter.test.ts
Pi notify events now create completed system_notice items with the notification message as the title and content. Tests check the item type and full notification text.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge


Merge Risk: ⚪ Minimal · up to 7edfd

Pi notifications now appear as readable notices with their full text. The supplied contract, visibility, and test evidence shows no material merge-blocking risk.

Architecture Summary

Architecture risk: 🔵 Low · up to 7edfd

The change affects 1 system.

Changed systems: packages/provider-pi

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/provider-pi (library) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/provider-pi/src/server/adapter.test.ts: The ordering-barrier test now waits for a system_notice turn item instead of a dynamic_tool item named notify.
  • observed — Modified behavior in packages/provider-pi/src/server/adapter.test.ts: The command-only prompt test now supplies and expects the full notification message "/op:status done\n\nTask: alpha" instead of "done".
  • observed — Modified behavior in packages/provider-pi/src/server/adapter.ts: The extension UI comment now describes notify as a system notice rather than a completed activity item.
  • observed — Modified behavior in packages/provider-pi/src/server/adapter.ts: Pi notify events now create a system_notice item titled with the message and carrying that message directly; the previous dynamic_tool representation with notification input was removed.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check Passed The title clearly and concisely states the main change: Pi extension notifications render as system notices instead of tool calls.
Description check Passed The description covers the problem, change, scope and approval rationale, and verification. It also identifies the styling limitation and the checks that were not repeated or performed.


✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
@kushaldotdev

Copy link
Copy Markdown

Heads-up: this is now conflicting with main (mergeable: false) after the provider package rework — Pi moved to packages/provider-pi (#17302), followed by the Effect convention refactors (#17542, #17427, #17381, #17375).

Would you be able to rebase and push? Macroscope's verdict was recorded against the old head, so a new push re-triggers the review path. This is still the fix for notifications rendering as tool input, which several Pi threads still hit.

…alls

Pi `notify` was emitted as a `dynamic_tool` item, so clients showed it as a
collapsed "Notify" tool call with the message as raw JSON input. Emit it as
a `system_notice`, which web and mobile already render as readable text
outside the folded work log.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blue-az
blue-az force-pushed the fix/pi-notify-system-notice branch from d8126d3 to 7edfd22 Compare October 9, 2026 20:10

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:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants