feat(sdk): add Pi wire protocol types and event classification - #154
feat(sdk): add Pi wire protocol types and event classification#154ross-rl wants to merge 2 commits into
Conversation
Hand-written Pi JSONL RPC wire types, kept field-for-field aligned with the broker's Rust bindings in `pi-codes` so the two can be diffed, plus the timeline classifier and type guards for Pi agent events. Depending on `@earendil-works/pi-coding-agent` for these types is not viable: its type graph pulls in five provider SDKs and its `RpcResponse` references internal paths no subpath export reaches. Pi ships no JSON Schema, so codegen is unavailable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| cacheRead: number; | ||
| cacheWrite: number; | ||
| total: number; | ||
| } |
There was a problem hiding this comment.
[BUG] major — usage is optional on a real assistant error frame. The current pi-codes crate marks AssistantMessage.usage with #[serde(default)], and protocol_tests.rs::assistant_message_without_usage_deserializes explicitly accepts a message_end whose assistant has stopReason: "error", errorMessage: "overloaded", and no usage. That frame cannot be represented by this interface. Please make this field optional (and update consumers to tolerate its absence), or otherwise model the wire default explicitly. Concrete failing frame: {"type":"message_end","message":{"role":"assistant","content":[],"api":"...","provider":"...","model":"...","stopReason":"error","errorMessage":"overloaded","timestamp":1}}.
There was a problem hiding this comment.
Fixed in 39d0a1e. Confirmed against the crate: AssistantMessage.usage is #[serde(default)] (types.rs:270), and assistant_message_without_usage_deserializes asserts message.usage == Usage::default() for a stopReason: "error" / errorMessage: "overloaded" frame with usage removed.
usage is now usage?: Usage, with a doc comment recording that the Rust side materialises a zeroed Usage when the field is absent — TypeScript has no equivalent of #[serde(default)], so surfacing the absence is more honest than inventing zeros the wire never sent. No SDK code read usage, so there were no consumers to update.
Regression coverage: assistantErrorMessageWithoutUsage() in src/__test-utils__/pi-fixtures.ts builds exactly the frame from your comment, and timeline-event-guards.test.ts classifies it as message_end and asserts usage === undefined alongside stopReason and errorMessage.
ross-rl
left a comment
There was a problem hiding this comment.
Review summary
Findings: 1 major.
- [BUG] major —
AssistantMessage.usageis required here but defaults when absent in the current Rust crate; see the inline comment.
Wire-fidelity verdict: Apart from that optionality mismatch, the transcription is field-for-field faithful to the current pi-codes sources and protocol_tests.rs. I checked:
- every scalar enum value and casing (
ThinkingLevel,QueueMode,StopReason,InputKind,StreamingBehavior); - every model/session field and serde optional/default behavior;
- all content-block tags and fields;
Usage/Cost;- all seven message roles and their fields;
- all 12 assistant-streaming variants, tags, terminal reason unions, and camelCase fields;
- all 11 Pi event variants and payload fields;
- response fields; and
- the five broker-modelled commands, tags, optional ids/images, and renamed fields.
The classifier, discriminated timeline union, guards, event constants, and fixtures match the modeled Rust event set. The sole mismatch comes from the crate's newer regression for an error assistant message without token usage.
The Rust crate marks the field `#[serde(default)]`, so a request that fails before any token accounting is recorded sends a message_end whose assistant message has no usage at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Fixed the one finding in 39d0a1e — Thanks for the field-by-field pass on the transcription; that was the part I most wanted a second set of eyes on. CI is green on the new head: |
What
Adds the Pi protocol layer to the SDK as a pure addition — no connection class yet, so the wire types can be reviewed against the Rust
pi-codescrate in isolation.sdk/src/pi/protocol/index.ts— hand-written Pi JSONL RPC wire types (commands, the eleven agent events, the twelveAssistantMessageEventdelta variants, the seven message variants, session state) plus the broker event-type constants (turn/start,cancel,response,PI_EVENT_TYPES).sdk/src/pi/types.ts—PiTimelineEventunion: onepi_protocoltimeline event per modelled Pi event,PiResponseTimelineEventfor acks, andmessage_updatenarrowed to text/thinking deltas, unioned with the shared system and unknown events.sdk/src/pi/classify-pi-axon-event.ts—classifyPiAxonEventbuilt on the sharedcreateClassifier, soSYSTEM_EVENTclassification wins and unmodelled Pi frames stayunknownwith their payload reachable.sdk/src/pi/timeline-event-guards.ts— per-event guards plus re-exports of the shared system/unknown guards, matching the Codex module layout.pi-codes/tests/protocol_tests.rs.Why
The broker speaks
pi_json(AXON_ATTACH_PROTOCOL_PI) and the SDK needs faithful types for it. Depending on@earendil-works/pi-coding-agentis not viable: its type graph pulls in five provider SDKs and itsRpcResponsereferences internal paths that no subpath export reaches. Pi ships no JSON Schema, so codegen is unavailable — hence hand-written types, kept field-for-field aligned withjava/rust/broker/clients/pi-codes/src/{commands,events,response,types}.rsso a reviewer can diff the two directly.Two protocol subtleties are captured in comments for reviewers:
agent_endis not turn completion —willRetry: truemeans more events follow, and onlyagent_settledends an accepted turn.steer/follow_upexist in Pi's ownRpcCommandbut not inpi-codes; they reach Pi verbatim as Control frames.Checklist
<type>(<scope>): <description>format (see above)bun run checkpasses (lint + format)bun run buildpassesbun run testpassespientry point🤖 Generated with Claude Code