Repository navigation
build(agentic): upgrade pi to 1.4.0 - #1950
Conversation
pi 1.4.0 persists the prompt and tool loadout as role: system message entries. The pi reader skips them, so no projection carries the prompt and none becomes an empty compaction summary. The entries stay in the log verbatim, because pi rebuilds a resumed session by walking parentId links and a missing entry would cut the history before it. PiAssistantMessage.stopReason adds pending and deferred.
@agentic-kit/pi moves its dev dependency on pi-coding-agent to 1.4.0. The peer floor stays >=0.79.0, because constructive-db still runs 0.79.6. @agentic-kit/cli pins pi-coding-agent 1.4.0 exactly. Both build, lint and test green with no source changes.
A pi 1.4.0 response with stop reason pending or deferred has not arrived yet, so the turn did not end. The span projector gives such a model span status open with no end, the same as an unsettled tool call.
|
Review complete. 🟡 2 medium 💬 Inline comments (1)
📍 Findings outside the diff (1) — 🟡 1 medium — defects on lines GitHub can't attach comments to🟡 Medium — Superseded // agentic/run-log/src/projectors/spans.ts
150 if (event.kind === 'model-response') {
151 const span: ModelSpan = {
152 id: `model:${event.entryId ?? String(seq)}`,
153 kind: 'model',
154 name: event.model ?? 'model',
155 // The turn began no earlier than the previous entry was written; with
156 // no previous entry the interval is unknown and the span is a point.
157 startedAt: previous ?? at,
158 startSeq: seq,
159 timing: 'bounded',
160 status: modelStatus(event.stopReason),
161 ...(event.model ? { model: event.model } : {}),
The change bumps
Reviewed commit: 005d019 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
This PR upgrades the agentic packages to pi-coding-agent 1.4.0 and adapts run-log transcript reading and span projection to the new entry shape.
Key findings
- 🟡
abortedand unknown stopReasons map took— spans.ts:109 - 🟡 Superseded
pendingmodel span never gets closed — spans.ts:150
| /** A pending or deferred response has not arrived yet, so its turn has not ended. */ | ||
| const modelStatus = (stopReason: string | undefined): SpanStatus => { | ||
| if (stopReason === 'pending' || stopReason === 'deferred') return 'open'; | ||
| return stopReason === 'error' ? 'error' : 'ok'; | ||
| }; |
There was a problem hiding this comment.
🟡 bug · medium
aborted and unknown stopReasons map to ok
The rewritten modelStatus helper (agentic/run-log/src/projectors/spans.ts:109-113) returns 'ok' for every stopReason except pending/deferred/error, so a user-aborted turn (stopReason: 'aborted', a member of the union at agentic/run-log/src/transcripts/pi-entry.ts:92) is closed with status: 'ok' and a duration. Unrecognized stopReasons from a newer writer are likewise flattened to success, unlike the projector's otherwise-explicit unknown handling. Trace consumers cannot distinguish a completed turn from a cancelled one.
📋 Prompt for AI Agents
In agentic/run-log/src/projectors/spans.ts at modelStatus (lines 109-113), map stopReason 'aborted' to a non-success status — either return 'error' or extend SpanStatus (line 31) with 'aborted' — and add a final branch for unrecognized stopReason values that keeps the span open or returns an 'unknown' status, so aborted and future stopReason semantics are not reported as success. Update spans projection tests to cover an 'aborted' entry and a fabricated stopReason value.
There was a problem hiding this comment.
Not changing this here. aborted already mapped to ok before this PR; only the pending/deferred branch is new. The raw stopReason stays on the span, so consumers can tell an aborted turn from a completed one. Adding an aborted status widens SpanStatus, which downstream consumers switch on, so it should be its own change.
There was a problem hiding this comment.
The concern stands. The current diff still shows the same return stopReason === 'error' ? 'error' : 'ok' fallthrough, meaning 'aborted' maps to 'ok'. The argument that the raw stopReason field is still on the span is reasonable for consumers who inspect that field directly, but it does not address the semantic mismatch of status: 'ok' on an aborted turn — consumers switching on status alone will misread it as success. The fix (adding an explicit 'aborted' branch or an unknown-fallback) is not visible in this hunk. If it landed elsewhere in the file or in a related file, I will need more context to verify.
|
Reply to the out-of-diff finding about a superseded |
Summary
@agentic-kit/run-logreads pi 1.4.0 system messages and stop reasons from transcripts.@agentic-kit/piand@agentic-kit/climove to pi 1.4.0.Dependents
The desktop (airpage) and constructive-db PRs depend on this PR and on the run-log publish that follows the merge. Lerna publishes new
@agentic-kit/run-log,piandcliversions from main. Versions are not bumped by hand here.Test plan
agentic/run-log: build, lint, test (118 passed)agentic/pi: build, lint, test (75 passed)agentic/cli: build, lint, test (51 passed)