From 80f94d64b724ce774ffa473f610ffeee0403bdff Mon Sep 17 00:00:00 2001 From: Gerard Slee Date: Thu, 6 Aug 2026 13:56:12 -0400 Subject: [PATCH 1/9] feat: pi-parity for chat + agent LLM loops Centralize the OpenAI-compatible streaming/message-prep layer (sse.ts consumer, request body, truncation guard, reasoning round-trip) into a shared llm-stream module so chat-loop and pi-agent-loop can't drift again. - system prompt sent as role:"system" message (never top-level system:), incl. compaction - finish_reason capture + refuse truncated/interrupted tool calls in both loops - reasoning round-trips to the same model under its native field; converts to text cross-model; never baked into content (pi behaviour) - empty assistant turns filtered from outgoing requests - reasoning models get role:"developer" on supported providers (models.dev reasoning flag + pi detectCompat denylist); model catalog now parses it --- changelogs/v2.6.5.md | 12 + electron/ipc/chat.ts | 6 +- electron/ipc/pi-agent.ts | 4 + electron/lib/chat-loop.ts | 184 ++++------- electron/lib/compaction.ts | 9 +- electron/lib/llm-stream.test.ts | 221 +++++++++++++ electron/lib/llm-stream.ts | 402 +++++++++++++++++++++++ electron/lib/llm.ts | 2 +- electron/lib/pi-agent-loop.test.ts | 237 ++++++++++++- electron/lib/pi-agent-loop.ts | 226 +++++++------ electron/lib/tools.ts | 2 +- shared/models/model-catalog.test.ts | 6 +- shared/models/model-catalog.ts | 5 + src/components/agent/AgentChatPane.tsx | 7 +- src/components/chat/chat-panel/index.tsx | 2 + src/hooks/useChatStream.ts | 2 + 16 files changed, 1067 insertions(+), 260 deletions(-) create mode 100644 changelogs/v2.6.5.md create mode 100644 electron/lib/llm-stream.test.ts create mode 100644 electron/lib/llm-stream.ts diff --git a/changelogs/v2.6.5.md b/changelogs/v2.6.5.md new file mode 100644 index 00000000..1febdf6e --- /dev/null +++ b/changelogs/v2.6.5.md @@ -0,0 +1,12 @@ +## What's new + +### Fixes + +- **Cairn Agent no longer fails against providers that reject the top-level `system` parameter.** The agent loop sent the system prompt as an Anthropic-style top-level `system` field in the request body (`{ model, system: …, messages: … }`), which strict OpenAI-compatible providers reject outright with *unknown parameter system* — breaking the agent (and compaction) entirely on those providers, while regular chat worked because it sends the prompt as a normal `role: "system"` message. The agent loop and the context-compaction call now both send the system prompt the same way chat does, as a `role: "system"` message, so pi-agent works identically against any endpoint that chat does. +- **A cut-off agent reply can no longer execute a half-built tool call.** The agent loop never read `finish_reason` from the stream, so a model that hit its output-token limit mid-call could still have its (possibly truncated) arguments executed — the same failure chat guarded against. The agent loop now captures `finish_reason` like chat does: any tool call from a turn that ended with `finish_reason: "length"` is refused, shown as failed, and the model is asked to re-issue with complete arguments. +- **Agent reasoning now round-trips to the same model, pi-style, instead of being thrown away.** Previously the agent stripped chain-of-thought from every re-sent message, which breaks multi-turn reasoning continuity on models that expect their thinking back (Claude/OpenAI encrypted reasoning, DeepSeek/Qwen `reasoning_content`). The agent now records which reasoning field the provider used and sends it back under that same field — but only to the *same* model that produced it. If you switch provider/model mid-session, the old model's reasoning is converted to plain text (pi behaviour) so the new model keeps the context without risking an unknown-field rejection. +- **Reasoning is never baked into the reply text anymore.** When a "thinking" model exhausts its output budget having emitted only chain-of-thought, the reasoning stays in its own panel instead of being echoed back as message content — matching pi. Reasoning-only turns are still preserved and round-tripped to the same model. +- **An interrupted stream can no longer execute a half-built tool call.** A connection that drops mid-call ends the stream without any `finish_reason`. Both chat and the agent now treat that the same as an output-token cut: buffered tool calls are refused, shown as failed, and the model is asked to re-issue with complete arguments. +- **Empty assistant turns no longer poison the next agent request.** A reasoning model that stopped mid-thought can leave behind an assistant turn with neither content nor tool calls once reasoning is stripped — replaying it makes strict providers reject the whole request with *content or tool_calls must be set*. The agent now filters those turns out of outgoing requests, matching chat. +- **Chat and the agent now share one LLM streaming layer.** The SSE parsing (reasoning-field capture, `finish_reason` tracking, tool-call buffering), the request-body construction, the truncated-tool-call guard, and the message-preparation step (system prompt, reasoning round-trip, empty-turn filtering) were duplicated between `chat-loop` and the agent loop and had silently drifted. They now live in a single shared module (`llm-stream`) consumed by both loops, so a fix or improvement lands in both at once. +- **Reasoning models get the `developer` system role on supported providers (pi parity).** The system prompt is now sent as `role: "developer"` instead of `role: "system"` when the selected model is a reasoning/thinking model (from the models.dev catalog) and the provider supports it — the OpenAI convention pi follows. It falls back to `system` on providers known to reject the `developer` role (DeepSeek, NVIDIA, xAI, Together, Moonshot, ZAI, Cloudflare, Ant Ling, on-device Llama, and non-`anthropic/`/`openai/` OpenRouter models), so this can't break a provider that doesn't understand it. diff --git a/electron/ipc/chat.ts b/electron/ipc/chat.ts index 16666d29..a2692655 100644 --- a/electron/ipc/chat.ts +++ b/electron/ipc/chat.ts @@ -16,6 +16,7 @@ import { TOOLS, buildSystemPrompt, type ChatRequest } from "../lib/tools"; import { getExternalToolDefs } from "../lib/external-tools"; import { getCachedConfig, cacheLlmConnection } from "../lib/config-cache"; import { runToolLoop } from "../lib/chat-loop"; +import { resolveSystemRole } from "../lib/llm-stream"; import { resolveLlmApiKey } from "../lib/secure-store"; import { buildAttachmentParts } from "../../shared/models/pdf-attach"; import { resolveCreditSpec, probeCredits } from "../lib/provider-credits"; @@ -153,7 +154,10 @@ export function registerChatHandler(db: Database.Database, workspacePath: string : { role: "user", content: req.message }; const messages: OpenAIMessage[] = [ - { role: "system", content: buildSystemPrompt(req) }, + { + role: resolveSystemRole({ isReasoningModel: req.config?.isReasoningModel, baseUrl, provider, modelId: model }), + content: buildSystemPrompt(req), + }, ...(req.history ?? []) // Drop assistant turns that carry neither content nor tool_calls — a // thinking model that timed out or stopped mid-reasoning leaves such a diff --git a/electron/ipc/pi-agent.ts b/electron/ipc/pi-agent.ts index dca6e38f..027ce44f 100644 --- a/electron/ipc/pi-agent.ts +++ b/electron/ipc/pi-agent.ts @@ -56,6 +56,7 @@ interface PiAgentPromptRequest { temperature?: number; maxTokens?: number; autoApprove?: boolean; + isReasoningModel?: boolean; }; } @@ -75,6 +76,7 @@ interface PiAgentApprovePlanRequest { temperature?: number; maxTokens?: number; autoApprove?: boolean; + isReasoningModel?: boolean; }; } @@ -235,6 +237,7 @@ export function registerPiAgentHandler( temperature: reqConfig?.temperature ?? 0.3, maxTokens: reqConfig?.maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, + isReasoningModel: reqConfig?.isReasoningModel, }; let session = sessions.get(sessionId); @@ -330,6 +333,7 @@ export function registerPiAgentHandler( temperature: reqConfig?.temperature ?? 0.3, maxTokens: reqConfig?.maxTokens, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, + isReasoningModel: reqConfig?.isReasoningModel, }; let session = sessions.get(sessionId); diff --git a/electron/lib/chat-loop.ts b/electron/lib/chat-loop.ts index afca7bfb..0a6120bb 100644 --- a/electron/lib/chat-loop.ts +++ b/electron/lib/chat-loop.ts @@ -16,12 +16,12 @@ import type { BrowserWindow } from "electron"; import type Database from "better-sqlite3"; import type { OpenAIMessage } from "./llm"; import { buildApiUrl } from "./llm"; +import { buildChatCompletionsBody, consumeAssistantStream, failToolCallsFromTruncatedMessage, interruptedStreamToolCallError } from "./llm-stream"; import { TOOLS, type ChatRequest } from "./tools"; import { executeTool } from "../ipc/chat-executor"; import { executeExternalTool, isExternalToolName, externalToolLabel } from "./external-tools"; import { extractExternalRef } from "./external-ref"; import { externalOutputError } from "./tool-result"; -import { iterSseData } from "./sse"; import { traceTool } from "./tool-trace"; import { parseToolArgs } from "./parse-tool-args"; @@ -202,18 +202,13 @@ export async function runToolLoop( method: "POST", headers, signal, - body: JSON.stringify({ + body: JSON.stringify(buildChatCompletionsBody({ model, messages, tools: combinedTools, - tool_choice: "auto", - // Only send max_tokens when the user set an explicit cap — omitting - // it lets the model finish naturally (see maxTokens above). - ...(maxTokens ? { max_tokens: maxTokens } : {}), + maxTokens, temperature, - stream: true, - stream_options: { include_usage: true }, - }), + })), }); } catch (_err) { if (signal?.aborted) return { exhausted: true, content: "", reasoning: accumulatedReasoning }; @@ -228,118 +223,56 @@ export async function runToolLoop( const reader = response.body?.getReader(); if (!reader) return { exhausted: true, content: "No response stream", reasoning: "" }; - let contentBuffer = ""; - const toolCallBuffers: Map = new Map(); - - for await (const jsonStr of iterSseData(reader, signal ?? undefined)) { - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const chunk = JSON.parse(jsonStr) as any; - if (chunk.usage && onUsage) { - const topCost = chunk.cost as { request_cost_usd?: unknown } | undefined; - onUsage( - chunk.usage.prompt_tokens ?? 0, - chunk.usage.completion_tokens ?? 0, - chunk.usage.completion_tokens_details?.reasoning_tokens ?? 0, - typeof chunk.usage.cost === "number" - ? chunk.usage.cost - : typeof topCost?.request_cost_usd === "number" - ? topCost.request_cost_usd - : undefined, - ); - } - const delta = chunk.choices?.[0]?.delta; - // Capture the finish reason — "length" means the model hit max_tokens. - // A reasoning model can hit it having emitted only chain-of-thought. - if (chunk.choices?.[0]?.finish_reason) { - turnFinishReason = chunk.choices[0].finish_reason; - } - if (!delta) continue; - - if (delta.content) { - contentBuffer += delta.content; - accumulatedContent += delta.content; - if (onToken) onToken(delta.content); - } - - // Reasoning / thinking stream (Claude thinking_delta, OpenAI delta.reasoning, - // DeepSeek/Qwen-style delta.reasoning_content). - // Models that don't expose reasoning text simply never emit this field — - // the panel stays hidden. Reasoning is NOT merged into content/tool JSON. - const thought = delta.reasoning_content ?? delta.reasoning; - if (thought) { - accumulatedReasoning += thought; - if (onThought) onThought(thought); - } - - if (delta.tool_calls) { - for (const tc of delta.tool_calls) { - const idx: number = tc.index ?? 0; - if (!toolCallBuffers.has(idx)) { - toolCallBuffers.set(idx, { id: tc.id ?? "", name: tc.function?.name ?? "", args: "" }); - } - const buf = toolCallBuffers.get(idx)!; - if (tc.id) buf.id = tc.id; - if (tc.function?.name) buf.name = tc.function.name; - if (tc.function?.arguments) buf.args += tc.function.arguments; - // Gemini 3.x thought signature — opaque blob to round-trip back. - if (tc.thought_signature) buf.thought_signature = tc.thought_signature; - } - } - } catch { /* skip malformed SSE JSON line */ } - } + // Shared SSE parse (same as the agent loop) so reasoning-field capture, + // finish_reason tracking, and tool-call buffering can never diverge again. + const turn = await consumeAssistantStream(reader, { + signal, + onToken: (delta) => { if (onToken) onToken(delta); }, + onThought: (delta) => { if (onThought) onThought(delta); }, + onUsage: (usage) => { + if (!onUsage) return; + const topCost = usage.chunkCost as { request_cost_usd?: unknown } | undefined; + const raw = usage.raw as { cost?: unknown }; + onUsage( + usage.promptTokens, + usage.completionTokens, + usage.reasoningTokens, + typeof raw.cost === "number" + ? raw.cost + : typeof topCost?.request_cost_usd === "number" + ? topCost.request_cost_usd + : undefined, + ); + }, + }); + // Reasoning and content are accumulated here (onToken/onThought only stream). + accumulatedContent += turn.content; + accumulatedReasoning += turn.reasoning; + turnFinishReason = turn.finishReason; // Dev trace: per-tool assembled arguments. - for (const [idx, buf] of toolCallBuffers.entries()) { + for (let i = 0; i < turn.toolCalls.length; i++) { + const tc = turn.toolCalls[i]; traceTool("sse-args", { - toolIndex: idx, - toolName: buf.name, - arguments: buf.args, + toolIndex: turn.toolCallIndexes[i], + toolName: tc.function.name, + arguments: tc.function.arguments, }); } if (signal?.aborted) return { exhausted: true, content: "", reasoning: accumulatedReasoning }; - const toolCalls = toolCallBuffers.size > 0 - ? Array.from(toolCallBuffers.entries()) - .sort(([a], [b]) => a - b) - .map(([, buf]) => ({ - id: buf.id, - type: "function" as const, - function: { name: buf.name, arguments: buf.args }, - ...(buf.thought_signature ? { thought_signature: buf.thought_signature } : {}), - })) - : undefined; + const toolCalls = turn.toolCalls.length > 0 ? turn.toolCalls : undefined; assistantMsg = { role: "assistant" as const, - content: contentBuffer || null, + content: turn.content || null, // Note: reasoning is intentionally NOT included here. It is // accumulated separately in `accumulatedReasoning` and returned - // to the caller for UI/persistence. Sending it back to the API - // would violate both OpenAI and Anthropic message schemas. + // to the caller for UI/persistence (the ThinkingPanel renders it). + // Reasoning is never baked into `content` — matching pi. tool_calls: toolCalls, }; - - // Reasoning-budget exhaustion safety net (cloud/streaming). By default we - // omit max_tokens so this shouldn't happen, but a user-set cap (or a - // provider's own server-side limit) can still stop a "thinking" model with - // finish_reason:"length" having emitted only reasoning and no content. - // That empty turn is useless to show AND, if persisted, poisons the next - // request (the provider rejects an assistant message with neither content - // nor tool_calls — a 400). Surface the captured reasoning so the reply is - // never empty. - if ( - (!assistantMsg.content || !assistantMsg.content.trim()) && - !assistantMsg.tool_calls?.length && - turnFinishReason === "length" && - accumulatedReasoning.trim() - ) { - const surfaced = `*[This model hit its output-token limit before giving a final answer. Its reasoning is shown below — raise or clear "Max output tokens" in AI settings for a complete reply.]*\n\n${accumulatedReasoning.trim()}`; - assistantMsg.content = surfaced; - accumulatedContent = surfaced; - if (onToken) onToken(surfaced); - } } // No tool calls — model is ready to produce its final reply @@ -348,29 +281,26 @@ export async function runToolLoop( return { exhausted: false, content: accumulatedContent, reasoning: accumulatedReasoning }; } - // Output-token-limit truncation guard. A "length" finish means the model ran - // out of tokens mid-turn, so every tool call in this message may carry - // truncated arguments. The worst case is a cut that lands on a well-formed - // boundary: the partial JSON parses cleanly and would be executed with - // silently missing fields — something no JSON parser can detect. Mirroring - // pi's `failToolCallsFromTruncatedMessage`, refuse to execute ANY of them: - // emit the chip + a structured error so the model re-issues with complete + // Output-token-limit / interrupted-stream truncation guard. A "length" + // finish (or a stream that ended without ANY finish_reason — connection cut + // mid-call) means the tool calls in this message may carry truncated + // arguments. The worst case is a cut that lands on a well-formed boundary: + // the partial JSON parses cleanly and would be executed with silently + // missing fields — something no JSON parser can detect. Mirroring pi's + // `failToolCallsFromTruncatedMessage`, refuse to execute ANY of them: emit + // the chip + a structured error so the model re-issues with complete // arguments (or the user raises the output-token cap). - if (turnFinishReason === "length") { + const streamInterrupted = turnFinishReason === null && (assistantMsg.tool_calls?.length ?? 0) > 0; + if (turnFinishReason === "length" || streamInterrupted) { messages.push(assistantMsg); - // Tailor the guidance to the config: on Auto we send NO max_tokens, so a - // "length" cut comes from the provider's own output cap, not our setting. - const capHint = maxTokens - ? "Try raising the Max output tokens setting." - : "Max output tokens is set to Auto (no cap is sent), so this is the provider's own output limit — try a manual Max output tokens value or a model with a larger output cap."; - const truncatedError = - `Tool call not executed: the model hit its output-token limit, so the arguments may be truncated. ` + - `Re-issue the tool call with complete arguments. ${capHint}`; - for (const call of assistantMsg.tool_calls) { - emitToolCall({ tool: call.function.name, label: externalToolLabel(call.function.name, db), args: {}, callId: call.id }); - emitToolCallDone?.({ tool: call.function.name, callId: call.id, ok: false, error: truncatedError }); - messages.push({ role: "tool", tool_call_id: call.id, content: JSON.stringify({ error: truncatedError }) }); - } + const toolResults = failToolCallsFromTruncatedMessage(assistantMsg.tool_calls, { + maxTokens, + error: streamInterrupted ? interruptedStreamToolCallError() : undefined, + labelFor: (name) => externalToolLabel(name, db), + emitStart: (tool, label, callId, args) => emitToolCall({ tool, label, args, callId }), + emitEnd: (tool, label, ok, output, callId, args) => emitToolCallDone?.({ tool, callId, ok, error: output }), + }); + for (const tr of toolResults) messages.push(tr); continue; } diff --git a/electron/lib/compaction.ts b/electron/lib/compaction.ts index d6616c10..661486a9 100644 --- a/electron/lib/compaction.ts +++ b/electron/lib/compaction.ts @@ -132,8 +132,13 @@ export async function generateSummary( signal, body: JSON.stringify({ model, - system: SUMMARIZATION_SYSTEM_PROMPT, - messages: [{ role: "user", content: SUMMARIZATION_USER_PROMPT(conversationText) }], + // System prompt as a `role: "system"` message (not a top-level `system:` + // field) so strict OpenAI-compatible providers don't reject the request + // with "unknown parameter system". + messages: [ + { role: "system", content: SUMMARIZATION_SYSTEM_PROMPT }, + { role: "user", content: SUMMARIZATION_USER_PROMPT(conversationText) }, + ], max_tokens: SUMMARY_MAX_TOKENS, temperature: 0.1, // deterministic summary // Must stream to prevent proxy connection drop timeouts (e.g. gateway/reverse proxy limits on blocking sync calls returning 504 Gateway Time-out) diff --git a/electron/lib/llm-stream.test.ts b/electron/lib/llm-stream.test.ts new file mode 100644 index 00000000..885c9c2e --- /dev/null +++ b/electron/lib/llm-stream.test.ts @@ -0,0 +1,221 @@ +/** + * Unit tests for the shared LLM streaming / message-preparation layer + * (`electron/lib/llm-stream.ts`), which both the chat loop and the agent loop + * consume so their SSE parsing and reasoning handling can never diverge. + */ + +import { describe, it, expect } from "vitest"; +import { consumeAssistantStream, prepareContextMessages, resolveSystemRole, supportsDeveloperRole } from "./llm-stream"; + +function sseReader(...chunks: string[]): ReadableStreamDefaultReader { + const encoder = new TextEncoder(); + const stream = new ReadableStream({ + start(controller) { + for (const c of chunks) controller.enqueue(encoder.encode(c)); + controller.close(); + }, + }); + return stream.getReader(); +} + +describe("consumeAssistantStream", () => { + it("accumulates content, reasoning + field, and finish_reason", async () => { + const turn = await consumeAssistantStream( + sseReader( + `data: ${JSON.stringify({ choices: [{ delta: { content: "Hel" } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { content: "lo", reasoning_content: "think" } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { reasoning_content: "ing" } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: {}, finish_reason: "stop" }] })}\n\n`, + "data: [DONE]\n\n", + ), + {}, + ); + + expect(turn.content).toBe("Hello"); + expect(turn.reasoning).toBe("thinking"); + expect(turn.reasoningField).toBe("reasoning_content"); + expect(turn.finishReason).toBe("stop"); + expect(turn.toolCalls).toHaveLength(0); + }); + + it("buffers tool-call arguments split across chunks and fires onToolPending", async () => { + const pending: Array<{ name: string; callId: string }> = []; + const turn = await consumeAssistantStream( + sseReader( + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [{ index: 0, id: "call_1", function: { name: "ls", arguments: "" } }] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: "{\"path\":" } }] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: "\".\"}" } }] } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [{ delta: {}, finish_reason: "tool_calls" }] })}\n\n`, + "data: [DONE]\n\n", + ), + { onToolPending: (name, callId) => pending.push({ name, callId }) }, + ); + + expect(turn.toolCalls).toHaveLength(1); + expect(turn.toolCalls[0].function.name).toBe("ls"); + expect(turn.toolCalls[0].function.arguments).toBe('{"path":"."}'); + expect(turn.toolCallIndexes).toEqual([0]); + expect(turn.streamCallIds.get(0)).toBeDefined(); + expect(pending).toHaveLength(1); + expect(pending[0].name).toBe("ls"); + expect(pending[0].callId).toBe(turn.streamCallIds.get(0)); + }); + + it("reports usage via onUsage and captures a null finish_reason when the stream just ends", async () => { + let usage: { promptTokens: number; completionTokens: number } | undefined; + const turn = await consumeAssistantStream( + sseReader( + `data: ${JSON.stringify({ choices: [{ delta: { content: "x" } }] })}\n\n`, + `data: ${JSON.stringify({ choices: [], usage: { prompt_tokens: 10, completion_tokens: 5 } })}\n\n`, + "data: [DONE]\n\n", + ), + { onUsage: (u) => { usage = { promptTokens: u.promptTokens, completionTokens: u.completionTokens }; } }, + ); + + expect(usage).toEqual({ promptTokens: 10, completionTokens: 5 }); + expect(turn.content).toBe("x"); + expect(turn.finishReason).toBeNull(); + }); +}); + +describe("prepareContextMessages", () => { + it("prepends the system prompt as a role:system message and keeps normal turns", async () => { + const out = await prepareContextMessages({ + systemPrompt: "You are a helper.", + currentModelKey: "a::m", + messages: [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "Hello" }, + ], + }); + + expect(out).toEqual([ + { role: "system", content: "You are a helper." }, + { role: "user", content: "Hi" }, + { role: "assistant", content: "Hello" }, + ]); + }); + + it("round-trips reasoning under its native field for the same model", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + currentModelKey: "a::m", + messages: [ + { role: "assistant", content: "", reasoning: "THOUGHTS", reasoningField: "reasoning_content", reasoningModel: "a::m" }, + ], + }); + + expect(out).toHaveLength(2); // system + assistant + expect(out[1]).toMatchObject({ role: "assistant", reasoning_content: "THOUGHTS" }); + // internal metadata keys are gone + expect(JSON.stringify(out[1])).not.toContain("reasoningModel"); + }); + + it("converts reasoning from a different model to plain text (pi behaviour)", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + currentModelKey: "a::m", + messages: [ + { role: "assistant", content: "I did things.", reasoning: "OLD_THOUGHTS", reasoningField: "reasoning", reasoningModel: "b::n" }, + ], + }); + + expect(out).toHaveLength(2); + // The reasoning is NOT sent as a foreign field — it becomes text content + expect(JSON.stringify(out[1])).not.toContain("\"reasoning\":\"OLD_THOUGHTS\""); + expect(JSON.stringify(out[1])).not.toContain("reasoningField"); + expect(out[1]).toMatchObject({ role: "assistant", content: "I did things.\n\nOLD_THOUGHTS" }); + }); + + it("appends cross-model reasoning to content even when the reply was otherwise empty", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + currentModelKey: "a::m", + messages: [ + { role: "assistant", content: "", reasoning: "ONLY_THOUGHTS", reasoningField: "reasoning_content", reasoningModel: "b::n" }, + ], + }); + + expect(out).toHaveLength(2); + expect(out[1]).toMatchObject({ role: "assistant", content: "ONLY_THOUGHTS" }); + }); + + it("strips bare reasoning with no round-trip metadata", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + currentModelKey: "a::m", + messages: [ + { role: "assistant", content: "Hi", reasoning: "LEGACY_THOUGHTS" }, + ], + }); + + expect(JSON.stringify(out[1])).not.toContain("LEGACY_THOUGHTS"); + expect(out[1]).toMatchObject({ role: "assistant", content: "Hi" }); + }); + + it("drops empty assistant turns that would poison the request", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + currentModelKey: "a::m", + messages: [ + { role: "assistant", content: "" }, + { role: "assistant", content: "kept" }, + ], + }); + + expect(out).toHaveLength(2); // system + kept assistant + expect(out[1]).toMatchObject({ role: "assistant", content: "kept" }); + }); + + it("uses the requested system role for the prepended message", async () => { + const out = await prepareContextMessages({ + systemPrompt: "sys", + systemRole: "developer", + currentModelKey: "a::m", + messages: [{ role: "user", content: "Hi" }], + }); + + expect(out[0]).toEqual({ role: "developer", content: "sys" }); + }); +}); + +describe("resolveSystemRole (pi parity)", () => { + it("uses developer for reasoning models on standard OpenAI-compatible providers", () => { + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://api.openai.com/v1", provider: "openai", modelId: "gpt-5" })) + .toBe("developer"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://example.com/v1", modelId: "o3" })) + .toBe("developer"); + }); + + it("stays system for non-reasoning models", () => { + expect(resolveSystemRole({ isReasoningModel: false, baseUrl: "https://api.openai.com/v1" })) + .toBe("system"); + expect(resolveSystemRole({ baseUrl: "https://api.openai.com/v1" })) + .toBe("system"); + }); + + it("stays system on the denylisted providers (pi detectCompat)", () => { + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://api.deepseek.com/v1", modelId: "deepseek-r1" })) + .toBe("system"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://integrate.api.nvidia.com/v1" })) + .toBe("system"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://api.together.ai/v1" })) + .toBe("system"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "http://127.0.0.1:8080/v1", provider: "localllm" })) + .toBe("system"); + }); + + it("gives OpenRouter the developer role only for anthropic/openai models", () => { + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://openrouter.ai/api/v1", modelId: "anthropic/claude-sonnet-4" })) + .toBe("developer"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://openrouter.ai/api/v1", modelId: "openai/gpt-5" })) + .toBe("developer"); + expect(resolveSystemRole({ isReasoningModel: true, baseUrl: "https://openrouter.ai/api/v1", modelId: "deepseek/deepseek-r1" })) + .toBe("system"); + }); + + it("exposes the denylist predicate for consumers", () => { + expect(supportsDeveloperRole({ baseUrl: "https://api.openai.com/v1" })).toBe(true); + expect(supportsDeveloperRole({ baseUrl: "https://api.deepseek.com/v1" })).toBe(false); + }); +}); diff --git a/electron/lib/llm-stream.ts b/electron/lib/llm-stream.ts new file mode 100644 index 00000000..60b9dea3 --- /dev/null +++ b/electron/lib/llm-stream.ts @@ -0,0 +1,402 @@ +/** + * Cairn — Shared OpenAI-compatible LLM streaming / message-preparation layer. + * + * Extracted from `chat-loop.ts` and `pi-agent-loop.ts` so the two tool-call + * loops cannot silently drift again. Anything parity-critical lives here and is + * consumed by both: + * + * - `consumeAssistantStream` — SSE parse → { content, reasoning, tool calls, + * finish_reason, reasoning field }. Records WHICH reasoning field the + * provider used (`reasoning_content` / `reasoning` / `reasoning_text`) so it + * can be round-tripped verbatim (pi behaviour). + * - `buildChatCompletionsBody` — request body shape (system prompt travels as + * a `role: "system"` message, never a top-level `system:` field). + * - `failToolCallsFromTruncatedMessage` — `finish_reason: "length"` guard: + * refuse to execute ANY tool call from a turn the model had to cut short. + * - `prepareContextMessages` — system prepend + reasoning round-trip + + * empty-turn filtering around an optional context pruner. + * + * The two loops keep their own orchestration (chat runs tools sequentially, + * pi-agent in parallel with approvals/subagents), but they share this layer. + */ + +import { iterSseData } from "./sse"; +import { isSendableMessage, type OpenAIMessage } from "./llm"; + +/** Reasoning field names used by OpenAI-compatible providers, in priority order. */ +export const REASONING_FIELDS = ["reasoning_content", "reasoning", "reasoning_text"] as const; + +// ── Streaming ──────────────────────────────────────────────────────────────── + +export interface StreamToolCall { + id: string; + type: "function"; + function: { name: string; arguments: string }; + /** Gemini 3.x thought signature — opaque blob to round-trip back. */ + thought_signature?: string; +} + +export interface StreamUsage { + promptTokens: number; + completionTokens: number; + reasoningTokens: number; + /** Raw provider `usage` object (may carry provider-specific fields like cost). */ + raw: unknown; + /** Raw top-level `cost` field on the chunk, if the provider reports it there. */ + chunkCost?: unknown; +} + +export interface StreamOptions { + signal?: AbortSignal; + /** Streamed text delta (UI). */ + onToken?: (delta: string) => void; + /** Streamed reasoning/thinking delta (UI). */ + onThought?: (delta: string) => void; + /** Fired as soon as a tool call's name is first seen (live "pending" chip). */ + onToolPending?: (name: string, callId: string) => void; + /** Fired on the trailing usage chunk. */ + onUsage?: (usage: StreamUsage) => void; + /** Override callId generation for pending chips. Default `${name}:${Date.now()}:${index}`. */ + makeCallId?: (index: number, name: string) => string; +} + +export interface StreamedTurn { + content: string; + reasoning: string; + /** Which reasoning field the provider used, if any. */ + reasoningField: string | null; + /** finish_reason of the last chunk that carried one (null if the stream ended without it). */ + finishReason: string | null; + /** API-ready tool calls in stream order. */ + toolCalls: StreamToolCall[]; + /** Per toolCalls position, the original stream index (for callId lookup / tracing). */ + toolCallIndexes: number[]; + /** Original stream index → generated callId for live pending chips. */ + streamCallIds: Map; +} + +/** + * Consume an OpenAI-compatible SSE assistant stream and assemble the turn. + * Shared by chat and the agent loop so reasoning-field capture, finish_reason + * tracking, and tool-call buffering can never diverge again. + */ +export async function consumeAssistantStream( + reader: ReadableStreamDefaultReader, + opts: StreamOptions, +): Promise { + const { signal } = opts; + let content = ""; + let reasoning = ""; + let reasoningField: string | null = null; + let finishReason: string | null = null; + const toolBuffers: Map = new Map(); + const streamCallIds: Map = new Map(); + + for await (const jsonStr of iterSseData(reader, signal)) { + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const chunk = JSON.parse(jsonStr) as any; + + // Usage chunk — sent as the final SSE chunk when stream_options.include_usage is set. + if (chunk.usage) { + opts.onUsage?.({ + promptTokens: chunk.usage.prompt_tokens ?? 0, + completionTokens: chunk.usage.completion_tokens ?? 0, + reasoningTokens: chunk.usage.completion_tokens_details?.reasoning_tokens ?? 0, + raw: chunk.usage, + chunkCost: chunk.cost, + }); + } + + const choice = chunk.choices?.[0]; + // Capture the finish reason — "length" means the model hit max_tokens. + // A reasoning model can hit it having emitted only chain-of-thought. + if (choice?.finish_reason) finishReason = choice.finish_reason; + + const delta = choice?.delta; + if (!delta) continue; + + if (delta.content) { + content += delta.content; + opts.onToken?.(delta.content); + } + + // Some endpoints return reasoning in reasoning_content (llama.cpp), or + // reasoning / reasoning_text (other OpenAI-compatible endpoints). Use the + // first non-empty field and remember WHICH one so the reasoning can be + // round-tripped under the same field name on the next request. + let thoughtField: string | null = null; + for (const field of REASONING_FIELDS) { + const value = (delta as Record)[field]; + if (typeof value === "string" && value.length > 0) { thoughtField = field; break; } + } + if (thoughtField) { + reasoningField ??= thoughtField; + const thought = (delta as Record)[thoughtField] as string; + reasoning += thought; + opts.onThought?.(thought); + } + + if (delta.tool_calls) { + for (const tc of delta.tool_calls) { + const idx: number = tc.index ?? 0; + const isNew = !toolBuffers.has(idx); + if (isNew) { + toolBuffers.set(idx, { id: tc.id ?? "", name: tc.function?.name ?? "", args: "" }); + } + const buf = toolBuffers.get(idx)!; + if (tc.id) buf.id = tc.id; + if (tc.function?.name) buf.name = tc.function.name; + if (tc.function?.arguments) buf.args += tc.function.arguments; + // Gemini 3.x thought signature — opaque blob to round-trip back. + if (tc.thought_signature) buf.thought_signature = tc.thought_signature; + + // Fire pending chip as soon as we see the tool name during streaming. + if (buf.name && !streamCallIds.has(idx)) { + const callId = opts.makeCallId?.(idx, buf.name) ?? `${buf.name}:${Date.now()}:${idx}`; + streamCallIds.set(idx, callId); + opts.onToolPending?.(buf.name, callId); + } + } + } + } catch { /* skip malformed SSE JSON line */ } + } + + const entries = Array.from(toolBuffers.entries()).sort(([a], [b]) => a - b); + const toolCalls: StreamToolCall[] = entries.map(([, buf]) => ({ + id: buf.id, + type: "function", + function: { name: buf.name, arguments: buf.args }, + ...(buf.thought_signature ? { thought_signature: buf.thought_signature } : {}), + })); + const toolCallIndexes = entries.map(([idx]) => idx); + + return { content, reasoning, reasoningField, finishReason, toolCalls, toolCallIndexes, streamCallIds }; +} + +// ── Request body ───────────────────────────────────────────────────────────── + +export function buildChatCompletionsBody(opts: { + model: string; + messages: unknown[]; + tools: unknown[]; + maxTokens?: number; + temperature?: number; +}): Record { + return { + model: opts.model, + messages: opts.messages, + tools: opts.tools, + tool_choice: "auto", + // Only send max_tokens when an explicit cap is set — omitting it lets the + // model finish naturally. + ...(opts.maxTokens && opts.maxTokens > 0 ? { max_tokens: opts.maxTokens } : {}), + temperature: opts.temperature, + stream: true, + stream_options: { include_usage: true }, + }; +} + +// ── Output-token-limit truncation guard ────────────────────────────────────── + +/** Error text used when a `finish_reason: "length"` turn's tool calls are refused. */ +export function truncatedToolCallError(maxTokens?: number): string { + // Tailor the guidance to the config: on Auto we send NO max_tokens, so a + // "length" cut comes from the provider's own output cap, not our setting. + const capHint = maxTokens + ? "Try raising the Max output tokens setting." + : "Max output tokens is set to Auto (no cap is sent), so this is the provider's own output limit — try a manual Max output tokens value or a model with a larger output cap."; + return ( + `Tool call not executed: the model hit its output-token limit, so the arguments may be truncated. ` + + `Re-issue the tool call with complete arguments. ${capHint}` + ); +} + +/** Error text used when a stream ended without any finish_reason (connection cut mid-call). */ +export function interruptedStreamToolCallError(): string { + return ( + `Tool call not executed: the response stream ended before the model finished (no finish_reason received), ` + + `so the arguments may be truncated. Re-issue the tool call with complete arguments.` + ); +} + +export interface TruncatedToolCall { + id: string; + function: { name: string }; +} + +/** + * Refuse to execute ANY tool call from a turn the model had to cut short + * (`finish_reason: "length"`, or a stream that ended without any finish_reason). + * A cut that lands on a well-formed JSON boundary would otherwise execute with + * silently missing fields — no JSON parser can detect that. Emits the + * start/end callbacks so the chip appears failed, and returns the tool-result + * messages to feed back so the model re-issues. + */ +export function failToolCallsFromTruncatedMessage( + toolCalls: TruncatedToolCall[], + opts: { + maxTokens?: number; + /** Override the error text fed back to the model (e.g. interrupted-stream wording). */ + error?: string; + labelFor: (name: string) => string; + /** Map a tool call to the callId used by its live chip (defaults to `tc.id`). */ + callIdFor?: (tc: TruncatedToolCall, position: number) => string | undefined; + emitStart: (name: string, label: string, callId: string | undefined, args: Record) => void; + emitEnd: (name: string, label: string, ok: boolean, output: string, callId: string | undefined, args: Record) => void; + }, +): Array<{ role: "tool"; tool_call_id: string; content: string }> { + const error = opts.error ?? truncatedToolCallError(opts.maxTokens); + return toolCalls.map((tc, i) => { + const label = opts.labelFor(tc.function.name); + const callId = opts.callIdFor?.(tc, i) ?? tc.id; + opts.emitStart(tc.function.name, label, callId, {}); + opts.emitEnd(tc.function.name, label, false, error, callId, {}); + return { role: "tool", tool_call_id: tc.id, content: JSON.stringify({ error }) }; + }); +} + +// ── Message preparation ────────────────────────────────────────────────────── + +/** + * Whether a provider/baseUrl is a known-good `developer`-role target, mirroring + * pi's `detectCompat().supportsDeveloperRole` denylist (`openai-completions.ts`). + * Every standard OpenAI-compatible provider qualifies EXCEPT the denylisted + * "non-standard" ones (quirky system-role handling) and OpenRouter (which only + * maps the `developer` role for Anthropic/OpenAI models). + */ +export function supportsDeveloperRole(opts: { baseUrl?: string; provider?: string; modelId?: string }): boolean { + const baseUrl = opts.baseUrl ?? ""; + const provider = opts.provider ?? ""; + const isNonStandard = + provider === "nvidia" || baseUrl.includes("integrate.api.nvidia.com") || + provider === "cerebras" || baseUrl.includes("cerebras.ai") || + provider === "xai" || baseUrl.includes("api.x.ai") || + provider === "together" || baseUrl.includes("api.together.ai") || baseUrl.includes("api.together.xyz") || + baseUrl.includes("chutes.ai") || + baseUrl.includes("deepseek.com") || + provider === "zai" || baseUrl.includes("api.z.ai") || baseUrl.includes("open.bigmodel.cn") || + provider === "moonshotai" || provider === "moonshotai-cn" || baseUrl.includes("api.moonshot.") || + baseUrl.includes("api.cloudflare.com") || baseUrl.includes("gateway.ai.cloudflare.com") || + provider === "ant-ling" || baseUrl.includes("api.ant-ling.com") || + // Cairn-specific: the on-device Llama server wraps small models (Qwen/DeepSeek + // based) that don't understand the developer role. + provider === "localllm"; + const isOpenRouter = provider === "openrouter" || baseUrl.includes("openrouter.ai"); + const isOpenRouterDevModel = + isOpenRouter && (opts.modelId?.startsWith("anthropic/") || opts.modelId?.startsWith("openai/")); + return isOpenRouterDevModel || (!isNonStandard && !isOpenRouter); +} + +/** + * The `role` to send the system prompt under — `developer` for reasoning models + * on providers that support it (the OpenAI convention pi follows), else + * `system`. `isReasoningModel` comes from the models.dev catalog's `reasoning` + * flag (resolved in the renderer, which owns the catalog). + */ +export function resolveSystemRole(opts: { isReasoningModel?: boolean; baseUrl?: string; provider?: string; modelId?: string }): "system" | "developer" { + if (!opts.isReasoningModel) return "system"; + return supportsDeveloperRole(opts) ? "developer" : "system"; +} + +/** A message that may carry reasoning metadata for round-tripping. */ +export interface OutgoingMessage { + role: string; + content: string | null; + tool_calls?: unknown[]; + tool_call_id?: string; + /** Chain-of-thought text recorded from the provider's reasoning field. */ + reasoning?: string; + /** The provider field the reasoning arrived in (round-trip target). */ + reasoningField?: string; + /** Model key (`baseUrl::model`) that produced this message. */ + reasoningModel?: string; +} + +const REASONING_KEY_SET = new Set(REASONING_FIELDS); + +/** True when an assistant message is safe to send: content, tool calls, or round-tripped reasoning. */ +function isOutgoingSendable(m: Record): boolean { + if (m.role !== "assistant") return true; + if (typeof m.content === "string" && m.content.trim()) return true; + if (Array.isArray(m.tool_calls) && m.tool_calls.length > 0) return true; + for (const field of REASONING_KEY_SET) { + const v = m[field]; + if (typeof v === "string" && v.trim().length > 0) return true; + } + return false; +} + +/** + * Build the outgoing request messages: + * 1. Run the optional context pruner/compaction over reasoning-stripped + * messages (so a compaction summary can never see chain-of-thought). + * 2. Round-trip reasoning back under its native field — but ONLY for the same + * model that produced it (matching pi). Cross-model reasoning is dropped + * rather than sent as a text blob, so a provider switch mid-session can't + * 400 on an unknown field. + * 3. Drop empty assistant turns (neither content, nor tool calls, nor + * reasoning) that would poison the request with "content or tool_calls + * must be set". + * 4. Prepend the system prompt as a `role: "system"` message. + */ +export async function prepareContextMessages(opts: { + systemPrompt: string; + messages: M[]; + currentModelKey: string; + /** Role for the system prompt message (defaults to "system"). */ + systemRole?: "system" | "developer"; + pruner?: (messages: M[]) => M[] | Promise; +}): Promise { + const reasoningByMsg = new Map(); + const stripped = opts.messages.map((m): M => { + if (m.role === "assistant" && typeof m.reasoning === "string" && m.reasoning.length > 0) { + if (typeof m.reasoningField === "string" && typeof m.reasoningModel === "string") { + // Has round-trip metadata — remember the STRIPPED object (the one the + // pruner operates on) so reasoning can be re-attached after pruning. + const { reasoning: _r, reasoningField: _rf, reasoningModel: _rm, ...rest } = m; + reasoningByMsg.set(rest as M, { reasoning: m.reasoning, field: m.reasoningField, modelKey: m.reasoningModel }); + return rest as M; + } + // Bare reasoning with no round-trip metadata (legacy/persisted without + // the field name) — strip it rather than leak it to a provider that may + // reject or mis-handle an unknown assistant field. + const { reasoning: _r, ...rest } = m; + return rest as M; + } + return m; + }); + + const pruned = opts.pruner ? await opts.pruner(stripped) : stripped; + + const out: OpenAIMessage[] = [{ role: opts.systemRole ?? "system", content: opts.systemPrompt }]; + for (const m of pruned) { + if (m.role !== "assistant") { + out.push(m as unknown as OpenAIMessage); + continue; + } + const ri = reasoningByMsg.get(m); + const raw = m as unknown as Record; + let outgoing: Record; + if (ri) { + if (ri.modelKey === opts.currentModelKey) { + // Same model → round-trip reasoning under its native field (pi behaviour). + outgoing = { ...raw, [ri.field]: ri.reasoning }; + } else { + // Different model → convert reasoning to plain text content (pi behaviour), + // never sent as a foreign field the new provider may reject. + const existing = typeof raw.content === "string" && raw.content.trim() ? raw.content.trim() : ""; + outgoing = { ...raw, content: [existing, ri.reasoning].filter(Boolean).join("\n\n") }; + } + } else { + outgoing = raw; + } + if (!isOutgoingSendable(outgoing)) continue; + out.push(outgoing as unknown as OpenAIMessage); + } + return out; +} + +// Re-exported for convenience — the sendable-message predicate lives in llm.ts +// and is the same one chat.ts uses for thread history. +export { isSendableMessage }; diff --git a/electron/lib/llm.ts b/electron/lib/llm.ts index 8e0331ec..983b2c40 100644 --- a/electron/lib/llm.ts +++ b/electron/lib/llm.ts @@ -77,7 +77,7 @@ export interface LLMConfig { } export type OpenAIMessage = { - role: "system" | "user" | "assistant" | "tool"; + role: "system" | "developer" | "user" | "assistant" | "tool"; content: string | null; name?: string; tool_call_id?: string; diff --git a/electron/lib/pi-agent-loop.test.ts b/electron/lib/pi-agent-loop.test.ts index b79f3e6a..5a2a9048 100644 --- a/electron/lib/pi-agent-loop.test.ts +++ b/electron/lib/pi-agent-loop.test.ts @@ -68,18 +68,29 @@ function makeToolCtx(db: Database.Database): AgentToolContext { /** * Build a server that serves a fixed sequence of SSE response bodies. * Each call to POST /v1/chat/completions consumes the next body in the queue. + * Also records every request body so tests can assert what was actually sent. */ -function makeServer(responses: string[]): Promise<{ url: string; close: () => Promise }> { +function makeServer(responses: string[]): Promise<{ url: string; close: () => Promise; bodies: Record[] }> { let callIndex = 0; - const server = http.createServer((_req, res) => { - const body = responses[callIndex] ?? ""; - callIndex++; - res.writeHead(200, { - "Content-Type": "text/event-stream", - "Cache-Control": "no-cache", - "Connection": "keep-alive", + const bodies: Record[] = []; + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (c) => chunks.push(c as Buffer)); + req.on("end", () => { + try { + bodies.push(JSON.parse(Buffer.concat(chunks).toString("utf8")) as Record); + } catch { + bodies.push({}); + } + const body = responses[callIndex] ?? ""; + callIndex++; + res.writeHead(200, { + "Content-Type": "text/event-stream", + "Cache-Control": "no-cache", + "Connection": "keep-alive", + }); + res.end(body); }); - res.end(body); }); return new Promise((resolve) => { @@ -88,6 +99,7 @@ function makeServer(responses: string[]): Promise<{ url: string; close: () => Pr resolve({ url: `http://127.0.0.1:${port}`, close: () => new Promise((r) => server.close(() => r())), + bodies, }); }); }); @@ -179,7 +191,7 @@ function textThenToolSSE(text: string, toolName: string, toolArgs: string): stri describe("runAgentLoop — SSE streaming", () => { let db: Database.Database; - let server: { url: string; close: () => Promise }; + let server: { url: string; close: () => Promise; bodies: Record[] }; beforeEach(() => { db = makeDb(); }); afterEach(async () => { await server?.close(); db.close(); }); @@ -447,6 +459,211 @@ describe("runAgentLoop — SSE streaming", () => { expect(log.findIndex(e => e === starts[0])).toBeLessThan(firstEndIdx); expect(lastStartIdx).toBeLessThan(log.lastIndexOf(ends[ends.length - 1])); }); + + // ── 9. Output-token-limit truncation guard ───────────────────────────────── + + it("finish_reason length: tool call is NOT executed, chip fails, loop continues", async () => { + // Turn 1: tool call truncated by the output-token cap (finish_reason "length") + const lines: string[] = []; + lines.push(`data: ${JSON.stringify({ + choices: [{ delta: { tool_calls: [{ index: 0, id: "call_trunc", function: { name: "ls", arguments: "" } }] } }], + })}\n\n`); + lines.push(`data: ${JSON.stringify({ + choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: JSON.stringify({ path: "." }).slice(0, 3) } }] } }], + finish_reason: "length", + })}\n\n`); + lines.push("data: [DONE]\n\n"); + + server = await makeServer([ + lines.join(""), + textOnlySSE(["Re-issued successfully."]), + ]); + + const session = makeSession(); + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + // The truncated tool call must be failed, never executed + expect(log.some((e) => e.startsWith("tool-end:ls:false"))).toBe(true); + expect(log.some((e) => e.startsWith("tool-end:ls:true"))).toBe(false); + // Loop continues so the model can re-issue, then finishes normally + expect(log).toContain("step-start"); + expect(log).toContain("token:Re-issued successfully."); + expect(log[log.length - 1]).toBe("done"); + expect(log).not.toContain(expect.stringContaining("error:")); + }); + + // ── 10. Reasoning-only turn (length) is stored for round-trip, not surfaced ─ + + it("finish_reason length with only reasoning: reasoning stays out of content (pi behaviour)", async () => { + // A "thinking" model emits only chain-of-thought then hits the output cap. + const lines: string[] = []; + for (const char of "Let me think carefully about the answer.") { + lines.push(`data: ${JSON.stringify({ choices: [{ delta: { reasoning_content: char } }] })}\n\n`); + } + lines.push(`data: ${JSON.stringify({ choices: [{ delta: {}, finish_reason: "length" }] })}\n\n`); + lines.push("data: [DONE]\n\n"); + + server = await makeServer([lines.join("")]); + + const session = makeSession(); + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + // Reasoning is never baked into content (pi behaviour) — no surfaced token bubble + expect(log.some((e) => e.startsWith("token:*[This model hit its output-token limit"))).toBe(false); + expect(log[log.length - 1]).toBe("done"); + expect(log).not.toContain(expect.stringContaining("error:")); + + // The message stored on the session keeps an EMPTY content; the reasoning + // stays in its own field (with round-trip metadata) so it can be sent back + // to the SAME model under its native field — never baked into content. + expect(session.messages).toHaveLength(1); + const stored = session.messages[0] as { content: string; reasoning?: string; reasoningField?: string; reasoningModel?: string }; + expect(stored.content).toBe(""); + expect(stored.reasoning).toBe("Let me think carefully about the answer."); + expect(stored.reasoningField).toBe("reasoning_content"); + expect(stored.reasoningModel).toBe(`${server.url}::test`); + }); + + // ── 11. Bare reasoning without round-trip metadata is never leaked ───────── + + it("a prior reasoning-only turn (no round-trip metadata) is dropped from the request body", async () => { + server = await makeServer([textOnlySSE(["ok"])]); + + const session = makeSession(); + session.messages.push({ role: "user", content: "Continue." }); + // Prior turn produced only reasoning, but with NO reasoningField/Model (e.g. + // persisted before round-trip metadata existed) — must never reach the provider. + session.messages.push({ role: "assistant", content: "", reasoning: "SECRET_CHAIN_OF_THOUGHT" }); + + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + expect(log[log.length - 1]).toBe("done"); + expect(server.bodies).toHaveLength(1); + const sentMessages = server.bodies[0].messages as Array>; + const serialized = JSON.stringify(sentMessages); + expect(serialized).not.toContain("SECRET_CHAIN_OF_THOUGHT"); + // The user message is still sent + expect(serialized).toContain("Continue."); + }); + + // ── 12. Reasoning round-trips to the same model under its native field ───── + + it("reasoning round-trips to the SAME model under its native field (pi behaviour)", async () => { + server = await makeServer([textOnlySSE(["ok"])]); + + const session = makeSession(); + session.messages.push({ role: "user", content: "Continue." }); + session.messages.push({ + role: "assistant", + content: "", + reasoning: "SECRET_CHAIN_OF_THOUGHT", + reasoningField: "reasoning_content", + reasoningModel: `${server.url}::test`, + }); + + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + expect(log[log.length - 1]).toBe("done"); + const sentMessages = server.bodies[0].messages as Array>; + const serialized = JSON.stringify(sentMessages); + // Reasoning is sent back under the recorded field name (reasoning_content) + expect(serialized).toContain("reasoning_content"); + expect(serialized).toContain("SECRET_CHAIN_OF_THOUGHT"); + }); + + // ── 13. Cross-model reasoning is converted to text ───────────────────────── + + it("reasoning from a DIFFERENT model is sent as text, not as a foreign field", async () => { + server = await makeServer([textOnlySSE(["ok"])]); + + const session = makeSession(); + session.messages.push({ role: "user", content: "Continue." }); + // Produced by a different model — must not round-trip as a reasoning field, + // but pi converts it to plain text so the new model still sees the context. + session.messages.push({ + role: "assistant", + content: "", + reasoning: "OTHER_MODEL_THOUGHTS", + reasoningField: "reasoning", + reasoningModel: "https://other.example/v1::other-model", + }); + + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + expect(log[log.length - 1]).toBe("done"); + const sentMessages = server.bodies[0].messages as Array>; + const serialized = JSON.stringify(sentMessages); + // Not a reasoning field — appears as text content instead + expect(serialized).not.toContain("\"reasoning\":\"OTHER_MODEL_THOUGHTS\""); + expect(serialized).not.toContain("reasoningModel"); + expect(serialized).toContain("OTHER_MODEL_THOUGHTS"); + }); + + // ── 14. Interrupted stream (no finish_reason) with tool calls ────────────── + + it("stream that ends without finish_reason refuses to execute buffered tool calls", async () => { + // Tool call streamed, then the connection just ends — no finish_reason chunk. + const lines: string[] = []; + lines.push(`data: ${JSON.stringify({ + choices: [{ delta: { tool_calls: [{ index: 0, id: "call_int", function: { name: "ls", arguments: "" } }] } }], + })}\n\n`); + lines.push(`data: ${JSON.stringify({ + choices: [{ delta: { tool_calls: [{ index: 0, function: { arguments: JSON.stringify({ path: "." }) } }] } }], + })}\n\n`); + lines.push("data: [DONE]\n\n"); + + server = await makeServer([ + lines.join(""), + textOnlySSE(["Re-issued successfully."]), + ]); + + const session = makeSession(); + const { log, callbacks } = makeCallbacks(); + + await runAgentLoop( + session, "You are a test assistant.", + { baseUrl: server.url, model: "test", apiKey: "test", maxSteps: 10, temperature: 0.3 }, + callbacks, makeToolCtx(db), + ); + + // The buffered tool call must be failed, never executed + expect(log.some((e) => e.startsWith("tool-end:ls:false"))).toBe(true); + expect(log.some((e) => e.startsWith("tool-end:ls:true"))).toBe(false); + // Loop continues so the model can re-issue, then finishes normally + expect(log).toContain("step-start"); + expect(log).toContain("token:Re-issued successfully."); + expect(log[log.length - 1]).toBe("done"); + }); }); // ── Live integration tests ───────────────────────────────────────────────────── diff --git a/electron/lib/pi-agent-loop.ts b/electron/lib/pi-agent-loop.ts index 5b1245dd..be61ccf7 100644 --- a/electron/lib/pi-agent-loop.ts +++ b/electron/lib/pi-agent-loop.ts @@ -17,6 +17,14 @@ import type Database from "better-sqlite3"; import type { BrowserWindow } from "electron"; import { isLocalEndpoint, calculatePromptBreakdown, scaleBreakdown, buildApiUrl, type TokenBreakdown } from "./llm"; +import { + buildChatCompletionsBody, + consumeAssistantStream, + failToolCallsFromTruncatedMessage, + interruptedStreamToolCallError, + prepareContextMessages, + resolveSystemRole, +} from "./llm-stream"; import { readTool, readToolDefinition, writeTool, writeToolDefinition, @@ -31,7 +39,6 @@ import { import { executeTool } from "../ipc/chat-executor"; import type { ChatRequest, ToolArgs } from "./tools"; import type { SkillMeta } from "./skills"; -import { iterSseData } from "./sse"; import { traceTool } from "./tool-trace"; import { parseToolArgs } from "./parse-tool-args"; import { resultContentError } from "./tool-result"; @@ -64,12 +71,14 @@ export interface AgentLLMConfig { * value is the user's deliberate cap. */ maxTokens?: number; + /** Whether the selected model is a reasoning/thinking model (from the models.dev catalog). */ + isReasoningModel?: boolean; } // ── Message types ───────────────────────────────────────────────────────────── export interface AgentUserMessage { role: "user"; content: string } -export interface AgentAssistantMsg { role: "assistant"; content: string | null; reasoning?: string; tool_calls?: ToolCallSpec[] } +export interface AgentAssistantMsg { role: "assistant"; content: string | null; reasoning?: string; reasoningField?: string; reasoningModel?: string; tool_calls?: ToolCallSpec[] } export interface AgentToolResultMsg { role: "tool"; tool_call_id: string; content: string } export type AgentMessage = @@ -528,6 +537,10 @@ export async function runAgentLoop( // Undefined/0 → omit max_tokens (let the model finish naturally). const maxTokens = llmConfig.maxTokens && llmConfig.maxTokens > 0 ? llmConfig.maxTokens : undefined; + // Identity of this request's model — reasoning is round-tripped (pi behaviour) + // under its native field, but only to the SAME model that produced it. + const currentModelKey = `${baseUrl}::${model}`; + // Plan mode always uses 0.1 for deterministic analysis regardless of user setting const temperature = mode === "plan" ? 0.1 : (configTemp ?? 0.3); if (!apiKey && !isLocalEndpoint(baseUrl)) { @@ -551,23 +564,22 @@ export async function runAgentLoop( // assistant message and start a fresh one for this turn's tokens. if (steps > 1) callbacks.onStepStart(); - // Build messages array — apply context pruning. - // systemPrompt passes as `system:` in the request body (not as a user message). - // Strip reasoning before the pruner so custom context transforms cannot - // access thinking text. The post-pruner map is a safety net. - const stripped = session.messages.map((m) => { - if (m.role === "assistant" && "reasoning" in m) { - const { reasoning: _r, ...rest } = m; - return rest; - } - return m; - }); - const contextMessages = (await pruner(stripped)).map((m) => { - if (m.role === "assistant" && "reasoning" in m) { - const { reasoning: _r, ...rest } = m; - return rest; - } - return m; + // Build messages array — apply context pruning, reasoning round-trip, and + // empty-turn filtering via the shared helper (identical to chat's stream + // semantics). The system prompt travels as a `role: "system"` message (never + // a top-level `system:` field), reasoning round-trips to the SAME model + // under its native field, and reasoning is kept out of the pruner so + // compaction summaries can never see chain-of-thought. + const contextMessages = await prepareContextMessages({ + systemPrompt, + messages: session.messages, + currentModelKey, + systemRole: resolveSystemRole({ + isReasoningModel: llmConfig.isReasoningModel, + baseUrl, + modelId: model, + }), + pruner, }); // ── Stream assistant response ───────────────────────────────────────── @@ -587,18 +599,13 @@ export async function runAgentLoop( method: "POST", headers, signal, - body: JSON.stringify({ + body: JSON.stringify(buildChatCompletionsBody({ model, - system: systemPrompt, messages: contextMessages, tools: allTools, - tool_choice: "auto", - // Only send max_tokens when the user set an explicit cap (see maxTokens above). - ...(maxTokens ? { max_tokens: maxTokens } : {}), + maxTokens, temperature, - stream: true, - stream_options: { include_usage: true }, - }), + })), }); } catch (e) { if (signal.aborted) { callbacks.onDone(); return; } @@ -656,120 +663,110 @@ export async function runAgentLoop( const reader = response.body?.getReader(); if (!reader) { callbacks.onError("No response stream"); return; } - let contentBuffer = ""; - let reasoningBuffer = ""; - const toolCallBuffers: Map = new Map(); - // callId assigned per tool during streaming — reused at execution time - const streamCallIds: Map = new Map(); + // Shared SSE parse (identical to the chat loop) — reasoning-field capture, + // finish_reason tracking, tool-call buffering, and pending-chip callIds all + // live in `consumeAssistantStream` so the two loops cannot drift again. let toolsReadyFired = false; - - for await (const jsonStr of iterSseData(reader, signal)) { - try { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const chunk = JSON.parse(jsonStr) as any; - - // Usage chunk — sent as the final SSE chunk when stream_options.include_usage is set. - if (chunk.usage) { - const pt = chunk.usage.prompt_tokens ?? 0; - const ct = chunk.usage.completion_tokens ?? 0; - const rt = chunk.usage.completion_tokens_details?.reasoning_tokens ?? 0; - session.lastPromptTokens = pt; - // Accumulate completion + reasoning across rounds (total output for the turn). - // Prompt tokens are last-round only (= current context window usage). - session.totalCompletionTokens = (session.totalCompletionTokens ?? 0) + ct; - session.totalReasoningTokens = (session.totalReasoningTokens ?? 0) + rt; - let breakdown: TokenBreakdown | undefined; - try { - const rawBreakdown = calculatePromptBreakdown(systemPrompt, contextMessages, allTools); - breakdown = scaleBreakdown(rawBreakdown, pt); - } catch (err) { - console.error("[pi-agent] failed to calculate breakdown:", err); - } - callbacks.onUsage(pt, session.totalCompletionTokens ?? 0, session.totalReasoningTokens ?? 0, breakdown); - } - - const delta = chunk.choices?.[0]?.delta; - if (!delta) continue; - - if (delta.content) { - contentBuffer += delta.content; - callbacks.onToken(delta.content); + const turn = await consumeAssistantStream(reader, { + signal, + onToken: (delta) => callbacks.onToken(delta), + onThought: (delta) => callbacks.onThought?.(delta), + onToolPending: (name, callId) => { + if (!toolsReadyFired) { + callbacks.onToolsReady(); + toolsReadyFired = true; } - - // Reasoning / thinking stream (Claude thinking_delta, OpenAI delta.reasoning, - // DeepSeek/Qwen-style delta.reasoning_content). - // Not merged into content or tool-call JSON; surfaced as a separate panel. - const thought = delta.reasoning_content ?? delta.reasoning; - if (thought) { - reasoningBuffer += thought; - callbacks.onThought?.(thought); + callbacks.onToolPending(name, callId); + }, + onUsage: (usage) => { + const pt = usage.promptTokens; + const ct = usage.completionTokens; + const rt = usage.reasoningTokens; + session.lastPromptTokens = pt; + // Accumulate completion + reasoning across rounds (total output for the turn). + // Prompt tokens are last-round only (= current context window usage). + session.totalCompletionTokens = (session.totalCompletionTokens ?? 0) + ct; + session.totalReasoningTokens = (session.totalReasoningTokens ?? 0) + rt; + let breakdown: TokenBreakdown | undefined; + try { + const rawBreakdown = calculatePromptBreakdown(systemPrompt, contextMessages, allTools); + breakdown = scaleBreakdown(rawBreakdown, pt); + } catch (err) { + console.error("[pi-agent] failed to calculate breakdown:", err); } + callbacks.onUsage(pt, session.totalCompletionTokens ?? 0, session.totalReasoningTokens ?? 0, breakdown); + }, + }); - if (delta.tool_calls) { - for (const tc of delta.tool_calls) { - const idx: number = tc.index ?? 0; - const isNew = !toolCallBuffers.has(idx); - if (isNew) { - toolCallBuffers.set(idx, { id: tc.id ?? "", name: tc.function?.name ?? "", args: "" }); - } - const buf = toolCallBuffers.get(idx)!; - if (tc.id) buf.id = tc.id; - if (tc.function?.name) buf.name = tc.function.name; - if (tc.function?.arguments) buf.args += tc.function.arguments; - // Gemini 3.x thought signature — opaque blob to round-trip back. - if (tc.thought_signature) buf.thought_signature = tc.thought_signature; - - // Fire pending chip as soon as we see the tool name during streaming - if (buf.name && !streamCallIds.has(idx)) { - if (!toolsReadyFired) { - callbacks.onToolsReady(); - toolsReadyFired = true; - } - const callId = `${buf.name}:${Date.now()}:${idx}`; - streamCallIds.set(idx, callId); - callbacks.onToolPending(buf.name, callId); - } - } - } - } catch { /* skip malformed SSE JSON line */ } - } + const contentBuffer = turn.content; + const reasoningBuffer = turn.reasoning; + const streamCallIds = turn.streamCallIds; + const turnFinishReason = turn.finishReason; // Dev trace: per-tool assembled arguments. - for (const [idx, buf] of toolCallBuffers.entries()) { + for (let i = 0; i < turn.toolCalls.length; i++) { + const tc = turn.toolCalls[i]; traceTool("sse-args", { - toolIndex: idx, - toolName: buf.name, - arguments: buf.args, + toolIndex: turn.toolCallIndexes[i], + toolName: tc.function.name, + arguments: tc.function.arguments, }); } // ── No tool calls → turn complete ───────────────────────────────────── - if (toolCallBuffers.size === 0) { - session.messages.push({ role: "assistant", content: contentBuffer, reasoning: reasoningBuffer || undefined }); + if (turn.toolCalls.length === 0) { + // Reasoning is never baked into `content` (pi behaviour): it is streamed + // to the ThinkingPanel live and stored in its own field, then + // round-tripped to the SAME model under its native field (or converted to + // text cross-model) on the next request. + session.messages.push({ + role: "assistant", + content: contentBuffer, + reasoning: reasoningBuffer || undefined, + reasoningField: turn.reasoningField ?? undefined, + reasoningModel: currentModelKey, + }); callbacks.onDone(); return; } // ── Build tool call list from accumulated buffers ───────────────────── - const toolCalls: ToolCallSpec[] = Array.from(toolCallBuffers.entries()) - .sort(([a], [b]) => a - b) - .map(([, buf]) => ({ - id: buf.id, - type: "function" as const, - function: { name: buf.name, arguments: buf.args }, - ...(buf.thought_signature ? { thought_signature: buf.thought_signature } : {}), - })); + const toolCalls: ToolCallSpec[] = turn.toolCalls; session.messages.push({ role: "assistant", content: contentBuffer || null, reasoning: reasoningBuffer || undefined, + reasoningField: turn.reasoningField ?? undefined, + reasoningModel: currentModelKey, tool_calls: toolCalls, }); if (!toolsReadyFired) callbacks.onToolsReady(); + // ── Output-token-limit / interrupted-stream truncation guard ────────── + // Shared with the chat loop. A "length" finish (or a stream that ended + // without ANY finish_reason — connection cut mid-call) means the tool calls + // may carry truncated arguments. Refuse to execute them: emit the chip + a + // structured error so the model re-issues with complete arguments. + const streamInterrupted = turnFinishReason === null; + if (turnFinishReason === "length" || streamInterrupted) { + const toolResults = failToolCallsFromTruncatedMessage(toolCalls, { + maxTokens, + error: streamInterrupted ? interruptedStreamToolCallError() : undefined, + labelFor: (name) => isExternalToolName(name) + ? externalToolLabel(name, toolCtx.db) + : (CODING_LABELS[name]?.({}) ?? name), + callIdFor: (_tc, i) => streamCallIds.get(turn.toolCallIndexes[i] ?? i), + emitStart: (name, label, callId, args) => callbacks.onToolStart(name, label, callId, args), + emitEnd: (name, label, ok, output, callId, args) => callbacks.onToolEnd(name, label, ok, output, callId, args), + }); + for (const tr of toolResults) session.messages.push(tr); + continue; + } + // ── Execute tools in parallel ───────────────────────────────────────── + if (signal.aborted) { callbacks.onDone(); return; } // All tools in a turn fire concurrently. Results are appended to // session.messages in original source order regardless of completion order. // onToolEnd fires as each tool finishes (may be out of order for UI updates). @@ -782,7 +779,6 @@ export async function runAgentLoop( // mcp_active_writes lock (lockNote/unlockNote) plus the file-watcher's // in-flight / disk-existence checks, so a relocation's old-path unlink // is never mistaken for a delete. - if (signal.aborted) { callbacks.onDone(); return; } type ToolOutcome = { tcIdx: number; tc: ToolCallSpec; ok: boolean; resultContent: string; pendingCallId?: string }; diff --git a/electron/lib/tools.ts b/electron/lib/tools.ts index eadc03fb..902a1286 100644 --- a/electron/lib/tools.ts +++ b/electron/lib/tools.ts @@ -142,7 +142,7 @@ export interface ChatRequest { projectId?: string; workspaceId?: string; history?: ChatHistoryEntry[]; - config?: { baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number; maxTokens?: number }; + config?: { baseUrl?: string; model?: string; apiKey?: string; maxSteps?: number; temperature?: number; maxTokens?: number; isReasoningModel?: boolean }; systemPrompt?: string; /** Attachments on the current user message (base64 data URLs; kind="pdf" * becomes an Anthropic-style `document` part, images become `image_url`). */ diff --git a/shared/models/model-catalog.test.ts b/shared/models/model-catalog.test.ts index 8f17d7df..d8b69bb5 100644 --- a/shared/models/model-catalog.test.ts +++ b/shared/models/model-catalog.test.ts @@ -32,6 +32,7 @@ describe("parseModelCatalog", () => { cost: { input: 1.25, output: 10 }, modalities: { input: ["text", "image", "pdf"] }, tool_call: true, + reasoning: true, }, }, }, @@ -48,6 +49,7 @@ describe("parseModelCatalog", () => { output: 10, modes: ["text", "image", "pdf"], toolCall: true, + reasoning: true, provider: "openai", }); expect(map["claude-4"]).toEqual({ @@ -57,6 +59,7 @@ describe("parseModelCatalog", () => { output: null, modes: [], toolCall: null, + reasoning: null, provider: "anthropic", }); }); @@ -72,6 +75,7 @@ describe("parseModelCatalog", () => { output: null, modes: [], toolCall: null, + reasoning: null, provider: "deepseek", }); }); @@ -159,7 +163,7 @@ describe("resolveMaxOutputTokens", () => { describe("normalizeModelInfo", () => { it("passes well-formed entries through unchanged", () => { - const info = { context: 1000, maxOutput: 8000, input: 1, output: 2, modes: ["text"], toolCall: true, provider: "x" }; + const info = { context: 1000, maxOutput: 8000, input: 1, output: 2, modes: ["text"], toolCall: true, reasoning: true, provider: "x" }; expect(normalizeModelInfo(info as never)).toEqual(info); }); diff --git a/shared/models/model-catalog.ts b/shared/models/model-catalog.ts index 4f148ebf..aa8f4808 100644 --- a/shared/models/model-catalog.ts +++ b/shared/models/model-catalog.ts @@ -28,6 +28,8 @@ export interface ModelInfo { /** Context window in tokens (limit.context). */ modes: string[]; /** Whether the model supports tool calls, when known. */ toolCall: boolean | null; + /** Whether the model is a reasoning/thinking model (models.dev `reasoning`). */ + reasoning: boolean | null; /** models.dev provider slug that owns the catalog entry (drives the logo). */ provider: string | null; } @@ -292,6 +294,7 @@ export function normalizeModelInfo(info: ModelInfo | null | undefined): ModelInf output: typeof info.output === "number" ? info.output : null, modes, toolCall: typeof info.toolCall === "boolean" ? info.toolCall : null, + reasoning: typeof info.reasoning === "boolean" ? info.reasoning : null, provider: typeof info.provider === "string" ? info.provider : null, }; } @@ -312,6 +315,7 @@ export function parseModelCatalog(catalog: unknown): Record { cost?: { input?: unknown; output?: unknown }; modalities?: { input?: unknown }; tool_call?: unknown; + reasoning?: unknown; }; const readNum = (v: unknown): number | null => typeof v === "number" && Number.isFinite(v) ? v : null; @@ -325,6 +329,7 @@ export function parseModelCatalog(catalog: unknown): Record { output: readNum(m.cost?.output), modes: inModes, toolCall: typeof m.tool_call === "boolean" ? m.tool_call : null, + reasoning: typeof m.reasoning === "boolean" ? m.reasoning : null, provider: slug, }; } diff --git a/src/components/agent/AgentChatPane.tsx b/src/components/agent/AgentChatPane.tsx index 5c538e34..58ab772f 100644 --- a/src/components/agent/AgentChatPane.tsx +++ b/src/components/agent/AgentChatPane.tsx @@ -25,6 +25,7 @@ import { ContextRing } from "./ContextRing"; import { Tooltip } from "@/components/ui/tooltip"; import { revealNote } from "@/lib/events"; import { resolvePromptContext } from "@/lib/context-resolver"; +import { getModelInfo } from "@/lib/models-dev"; import type { PiAgentMessage, TerminalSession, TokenBreakdown, RegistryFetchResult } from "@/types"; import type { AgentConnectorMeta } from "./AgentMessageBubble"; import { redactAgentToolCall } from "@/lib/redact-agent-transcript"; @@ -521,9 +522,11 @@ export function AgentChatPane({ session, isActive }: AgentChatPaneProps) { apiKey: agentConfig.apiKey || undefined, maxSteps: agentConfig.maxSteps ?? 30, temperature: agentConfig.temperature ?? 0.3, - // Auto (default) → undefined → omit max_tokens so the model finishes naturally. - maxTokens: resolveMaxOutputTokens(agentConfig.maxOutputAuto === false ? agentConfig.maxOutputTokens : undefined), + // Auto (default) → undefined → omit max_tokens so the model finishes naturally. + maxTokens: resolveMaxOutputTokens(agentConfig.maxOutputAuto === false ? agentConfig.maxOutputTokens : undefined), autoApprove: session.autoApprove ?? agentConfig.autoApprove ?? true, + // Reasoning models get the `developer` system role (OpenAI convention). + isReasoningModel: getModelInfo(agentConfig.model)?.reasoning === true, }, }; window.electron?.piAgent.prompt(promptPayload); diff --git a/src/components/chat/chat-panel/index.tsx b/src/components/chat/chat-panel/index.tsx index 866b7289..0f670f1e 100644 --- a/src/components/chat/chat-panel/index.tsx +++ b/src/components/chat/chat-panel/index.tsx @@ -501,6 +501,8 @@ export function ChatPanel({ prefill, onPrefillConsumed, popoutMode }: ChatPanelP maxTokens: resolveMaxOutputTokens( aiConfig.maxOutputAuto === false ? aiConfig.maxOutputTokens : undefined, ), + // Reasoning models get the `developer` system role (OpenAI convention). + isReasoningModel: getModelInfo(aiConfig.model)?.reasoning === true, }, systemPrompt, images: attachmentsToSend?.map((a) => ({ name: a.name, dataUrl: a.dataUrl, kind: a.kind })), diff --git a/src/hooks/useChatStream.ts b/src/hooks/useChatStream.ts index 42ae5518..83c2d829 100644 --- a/src/hooks/useChatStream.ts +++ b/src/hooks/useChatStream.ts @@ -57,6 +57,8 @@ export interface ChatStreamRequest { temperature?: number; /** Max output/completion tokens per reply (resolved from model + user setting). */ maxTokens?: number; + /** Whether the selected model is a reasoning/thinking model (from the models.dev catalog). */ + isReasoningModel?: boolean; }; systemPrompt?: string; /** Attachments on the current user message (base64 data URLs; kind tells From 8ccd8201e07b9947eca2b70130ad31cdcc1804bb Mon Sep 17 00:00:00 2001 From: Gerard Slee Date: Thu, 6 Aug 2026 14:00:09 -0400 Subject: [PATCH 2/9] fix: auto-scroll the ask-questions form into view in chat + agent --- changelogs/v2.6.5.md | 1 + src/components/agent/AgentChatPane.tsx | 6 ++++-- src/components/chat/chat-panel/index.tsx | 5 ++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/changelogs/v2.6.5.md b/changelogs/v2.6.5.md index 1febdf6e..f093e3f9 100644 --- a/changelogs/v2.6.5.md +++ b/changelogs/v2.6.5.md @@ -10,3 +10,4 @@ - **Empty assistant turns no longer poison the next agent request.** A reasoning model that stopped mid-thought can leave behind an assistant turn with neither content nor tool calls once reasoning is stripped — replaying it makes strict providers reject the whole request with *content or tool_calls must be set*. The agent now filters those turns out of outgoing requests, matching chat. - **Chat and the agent now share one LLM streaming layer.** The SSE parsing (reasoning-field capture, `finish_reason` tracking, tool-call buffering), the request-body construction, the truncated-tool-call guard, and the message-preparation step (system prompt, reasoning round-trip, empty-turn filtering) were duplicated between `chat-loop` and the agent loop and had silently drifted. They now live in a single shared module (`llm-stream`) consumed by both loops, so a fix or improvement lands in both at once. - **Reasoning models get the `developer` system role on supported providers (pi parity).** The system prompt is now sent as `role: "developer"` instead of `role: "system"` when the selected model is a reasoning/thinking model (from the models.dev catalog) and the provider supports it — the OpenAI convention pi follows. It falls back to `system` on providers known to reject the `developer` role (DeepSeek, NVIDIA, xAI, Together, Moonshot, ZAI, Cloudflare, Ant Ling, on-device Llama, and non-`anthropic/`/`openai/` OpenRouter models), so this can't break a provider that doesn't understand it. +- **The ask-questions form now scrolls into view.** Both chat and the agent render the question form in the message thread above the input box, but if you had scrolled up when the model asked its questions, the form could arrive out of view. The thread now auto-scrolls to it so you always see the questions next to where you type your answers. diff --git a/src/components/agent/AgentChatPane.tsx b/src/components/agent/AgentChatPane.tsx index 58ab772f..51954950 100644 --- a/src/components/agent/AgentChatPane.tsx +++ b/src/components/agent/AgentChatPane.tsx @@ -193,11 +193,13 @@ export function AgentChatPane({ session, isActive }: AgentChatPaneProps) { const sendPromptRef = useRef<(text: string) => void>(() => {}); const firedSessions = useRef(new Set()); - // Scroll to bottom on new messages + // Scroll to bottom on new messages / streaming growth, and whenever the + // ask_questions form appears — otherwise it can land out of view if the user + // had scrolled up when the model asked its questions. /* eslint-disable react-hooks/exhaustive-deps */ useEffect(() => { if (isActive) messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); - }, [messages.length, messages[messages.length - 1]?.content?.length, isActive]); + }, [messages.length, messages[messages.length - 1]?.content?.length, isActive, pendingQuestions]); /* eslint-enable react-hooks/exhaustive-deps */ // Focus input when pane becomes active diff --git a/src/components/chat/chat-panel/index.tsx b/src/components/chat/chat-panel/index.tsx index 0f670f1e..d9038c56 100644 --- a/src/components/chat/chat-panel/index.tsx +++ b/src/components/chat/chat-panel/index.tsx @@ -342,7 +342,10 @@ export function ChatPanel({ prefill, onPrefillConsumed, popoutMode }: ChatPanelP const isChatActive = useCairnStore((s) => s.activeSessionId === "chat"); - useEffect(() => { if (isChatActive) messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); }, [messages, isLoading, isChatActive]); + // Scroll to the bottom when messages/loading change, and whenever the + // ask_questions form appears — otherwise it can land out of view if the user + // had scrolled up when the model asked its questions. + useEffect(() => { if (isChatActive) messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); }, [messages, isLoading, isChatActive, pendingQuestions]); useEffect(() => { if (chatOpen) inputRef.current?.focus(); }, [chatOpen]); const handleAttachImages = useCallback(async (files: File[]) => { From 69d89221df953571e92b930664ecfc85bf1215a8 Mon Sep 17 00:00:00 2001 From: Gerard Slee Date: Thu, 6 Aug 2026 14:01:38 -0400 Subject: [PATCH 3/9] fix: keep ask-questions prompt visible while answering (was textarea placeholder) --- changelogs/v2.6.5.md | 2 +- electron/lib/tool-schemas.ts | 2 +- src/components/chat/chat-panel/QuestionForm.tsx | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/changelogs/v2.6.5.md b/changelogs/v2.6.5.md index f093e3f9..7e8a2498 100644 --- a/changelogs/v2.6.5.md +++ b/changelogs/v2.6.5.md @@ -10,4 +10,4 @@ - **Empty assistant turns no longer poison the next agent request.** A reasoning model that stopped mid-thought can leave behind an assistant turn with neither content nor tool calls once reasoning is stripped — replaying it makes strict providers reject the whole request with *content or tool_calls must be set*. The agent now filters those turns out of outgoing requests, matching chat. - **Chat and the agent now share one LLM streaming layer.** The SSE parsing (reasoning-field capture, `finish_reason` tracking, tool-call buffering), the request-body construction, the truncated-tool-call guard, and the message-preparation step (system prompt, reasoning round-trip, empty-turn filtering) were duplicated between `chat-loop` and the agent loop and had silently drifted. They now live in a single shared module (`llm-stream`) consumed by both loops, so a fix or improvement lands in both at once. - **Reasoning models get the `developer` system role on supported providers (pi parity).** The system prompt is now sent as `role: "developer"` instead of `role: "system"` when the selected model is a reasoning/thinking model (from the models.dev catalog) and the provider supports it — the OpenAI convention pi follows. It falls back to `system` on providers known to reject the `developer` role (DeepSeek, NVIDIA, xAI, Together, Moonshot, ZAI, Cloudflare, Ant Ling, on-device Llama, and non-`anthropic/`/`openai/` OpenRouter models), so this can't break a provider that doesn't understand it. -- **The ask-questions form now scrolls into view.** Both chat and the agent render the question form in the message thread above the input box, but if you had scrolled up when the model asked its questions, the form could arrive out of view. The thread now auto-scrolls to it so you always see the questions next to where you type your answers. +- **The ask-questions form keeps the question visible while you answer.** The question text used to live as the *placeholder* inside the answer field, so it vanished the moment you started typing — and the form could also arrive out of view if you'd scrolled up. The question is now rendered as persistent text above each answer field (in both chat and the agent), and the thread auto-scrolls to the form so you always see what's being asked next to where you type. diff --git a/electron/lib/tool-schemas.ts b/electron/lib/tool-schemas.ts index 7b1bca96..6ed2fc57 100644 --- a/electron/lib/tool-schemas.ts +++ b/electron/lib/tool-schemas.ts @@ -466,7 +466,7 @@ export const TOOL_SCHEMAS = { questions: z.array(z.object({ id: z.string().describe("Short unique identifier, e.g. \"target_users\""), label: z.string().describe("Short bold label shown above the input, e.g. \"Target users\""), - prompt: z.string().describe("One-sentence question shown as placeholder text"), + prompt: z.string().describe("One-sentence question shown above the answer field, e.g. \"Who is this for?\""), })).describe("2–4 targeted questions"), }), }, diff --git a/src/components/chat/chat-panel/QuestionForm.tsx b/src/components/chat/chat-panel/QuestionForm.tsx index 84607cfb..180a9597 100644 --- a/src/components/chat/chat-panel/QuestionForm.tsx +++ b/src/components/chat/chat-panel/QuestionForm.tsx @@ -41,6 +41,9 @@ export function QuestionForm({ questions, onSubmit, disabled = false }: Question {questions.map((q) => (
+ {/* The question is rendered as persistent text — NOT as the textarea + placeholder — so it stays visible while the user types. */} +

{q.prompt}