From e32ecd91b9b7216f47106f87caa04be94d75d944 Mon Sep 17 00:00:00 2001 From: ltmoerdani Date: Thu, 24 Sep 2026 06:21:15 +0700 Subject: [PATCH] fix(responses): preserve whitespace in tool-call arguments (#244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit response.function_call_arguments.delta fragments went through the trimming firstString() helper — the same per-chunk-.trim() bug class as #192, but on a path the #192 fix deliberately left alone. Responses API argument fragments are arbitrary JSON slices that can split inside string values, so per-fragment trimming corrupted tool-call input on every GPT-family model (all route to /v1/responses) on both Go and Zen, sending the model into reasoning loops. Three-layer fix: - root cause: fragments now go through firstStringRaw() - new response.function_call_arguments.done handler emits the final authoritative arguments tagged argumentsDone: true - ToolCallAccumulator treats argumentsDone as REPLACE instead of append, healing any gateway-side delta mis-join Verified against openai-node's own accumulator (raw += event.delta) and the official done-event semantics. 5 regression tests; 489/489 pass. Docs: docs/issues/107, doc 83 annotated (superseded assumption), CHANGELOG 0.7.7, devlog, ARCHITECTURE-MAP. Version bump 0.7.7. closes #244 --- ARCHITECTURE-MAP.md | 6 +- CHANGELOG.md | 6 ++ docs/devlog.md | 16 ++++- ...issue244-tool-call-arguments-whitespace.md | 67 +++++++++++++++++++ ...60826-responses-api-whitespace-stripped.md | 4 +- package-lock.json | 4 +- package.json | 2 +- src/core/routing.ts | 36 +++++++++- src/test/routing.test.ts | 39 +++++++++++ src/test/toolCallAccumulator.test.ts | 30 +++++++++ src/toolCallAccumulator.ts | 11 ++- 11 files changed, 210 insertions(+), 11 deletions(-) create mode 100644 docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md diff --git a/ARCHITECTURE-MAP.md b/ARCHITECTURE-MAP.md index abe0aa7d8..7cf56b2ca 100644 --- a/ARCHITECTURE-MAP.md +++ b/ARCHITECTURE-MAP.md @@ -37,7 +37,7 @@ Total `src/` ≈ **16,310 lines** across ~109 files (excl. tests). Grouped by do | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | **Provider** | `src/provider/` — `OpenCodeProvider.ts` (532), `chatPrep.ts` (332), `messages.ts` (446), `modelInfo.ts` (239), `historyTrim.ts` (200), `settings.ts` (260), `modelList.ts` (185), `visionProxy.ts` (345), `providerDialogs.ts` (117), `transportLog.ts` (109), `definitions.ts` (238), `tokens.ts` (106), `providerUtils.ts` (35) | ~3,017 | The `OpenCodeProvider` class implementing `LanguageModelChatProvider` (thin — delegates to siblings via deps objects), request preparation (`chatPrep`: message conversion, vision proxy, history trimming, budgets, thinking payload), model-info assembly (`modelInfo`), model-list fetch/cache (`modelList`), Manage/Test-Connection flows (`providerDialogs`), rolling transport diagnostics (`transportLog`) | `prepareChatRequest()` / `provideModelChatInformation()` / `convertMessage()` / `normalizeMessages()` / `trimOldMessagesToFitContext()` / `historyByteCapForBudget()` / `ModelListFetcher` | | **Transports** | `src/transports/` — `engine.ts` (409), `extractors.ts` (556), `extract.ts` (225), `thinkTags.ts` (139), `streamParts.ts` (88), `sse.ts` (31), `chatCompletions.ts` (64), `responses.ts` (30), `anthropic.ts` (28), `google.ts` (31) | ~1,601 | One adapter per wire format (OpenAI chat-completions, OpenAI Responses, Anthropic Messages, Google generateContent) + the shared streaming engine, SSE parser, response extractors | `streamOpenCodeResponse()` (engine), `OpenAiResponseExtractor` / `AnthropicResponseExtractor` | -| **Core (registry/routing)** | `src/core/` — `routing.ts` (441), `registry.ts` (142), `transport.ts` (70) | ~653 | Data-driven model registry (`MODEL_REGISTRY`), transport resolution (`resolveModelRouting`), Responses/Google SSE normalization, shared `StreamRequestOptions` contract | **Pure** — no `vscode` import, no side effects | +| **Core (registry/routing)** | `src/core/` — `routing.ts` (535), `registry.ts` (142), `transport.ts` (70) | ~653 | Data-driven model registry (`MODEL_REGISTRY`), transport resolution (`resolveModelRouting`), Responses/Google SSE normalization, shared `StreamRequestOptions` contract | **Pure** — no `vscode` import, no side effects | | **Models (metadata)** | `src/models/` — `metadata.ts` (523), `modelTables.ts` (141), `metadataFetcher.ts` (102), `modelLimits.ts` (52), `modelCapabilities.ts` (41), `modelNames.ts` (29), `pricing.ts` (88) | ~976 | models.dev live metadata + bundled fallback snapshot (static data tables in `modelTables.ts`), limit/capability resolution, pricing | Live fetch may fail → bundled snapshot MUST exist | | **Usage** | `src/usage/` — `tracker.ts` (585, thin class shell), `trackerTypes.ts` (96), `trackerWindows.ts` (140), `trackerSummary.ts` (238), `dashboard.ts` (19 barrel) + `dashboard/` (`webview.ts`, `webviewData.ts`, `webviewHtml.ts`, `state.ts`, `statusBar.ts`, `targetEditor.ts`, `tooltip.ts` ≈ 1,405), `history.ts` (378), `usage.ts` (146), `goUsageSync.ts` (128), `formatting.ts` (129), `usageProfile.ts` (74), `pricing.ts` (62) | ~3,571 | Go usage tracker (types/windows/summary split out of the old god file), per-profile tracking, CLI SQLite history reader, server-usage sync, status bar + usage webview + quick-pick (webview split into state/status/webview modules) | Server meters authoritative for Session/Weekly/Monthly; device-local for Today/Yesterday | | **Thinking** | `src/thinking/` — `provider.ts` (78), `base.ts` (75), `resolve.ts` (94), `deepseek.ts` (53), `glm.ts` (53), `kimi.ts` (81), `minimax.ts` (54), `mimo.ts` (72), `openai.ts` (57), `qwen.ts` (103), `fallback.ts` (39), `schema.ts` (106), `payload.ts` (26), `types.ts` (51) | ~942 | Per-family thinking strategy classes + config resolution (per-model config wins over workspace — but schema-default echoes are stripped first, `stripSchemaDefaultEcho` in `resolve.ts`, issue #226) | **Pure** — no `vscode` import; family from registry | @@ -45,7 +45,7 @@ Total `src/` ≈ **16,310 lines** across ~109 files (excl. tests). Grouped by do | **Commands** | `src/commands/` — `agentsWindow.ts` (130), `diagnostics.ts` (41), `providers.ts` (72), `thinkingPicker.ts` (31) | ~274 | Command handlers: diagnostics, agents-window BYOK bridge, provider enable/disable (state-aware toggle, base-vendor resolved — issue #228), thinking picker | Thin — delegates to provider/usage modules | | **Autocomplete** | `src/autocomplete/` — `index.ts` (157), `engine.ts` (143), `provider.ts` (127), `context.ts` (89), `usage.ts` (88), `throttle.ts` (79), `prompt.ts` (60), `types.ts` (32) | ~775 | Inline code suggestions (opt-in) — FIM emulation over chat-completions, debounce/throttle, usage counters | Separate subsystem; not wired into Go cost tracker yet | | **Extension entry** | `src/extension.ts` | 415 | Thin `activate()`/`deactivate()` — wiring only (command registration, provider registration, status bar init) | Target <300 lines; keep wiring-only | -| **Root utilities** | `src/config.ts` (278), `contextWindowHook.ts` (485), `contextWindowHookBridge.ts` (122), `errors.ts` (294), `retry.ts` (463), `utils.ts` (186), `responsesRequest.ts` (180), `toolCallAccumulator.ts` (138), `imageNormalizer.ts` (118), `visionProxyCache.ts` (79), `runtimeDiagnostics.ts` (57), `chatParts.ts` (55), `reasoningHistory.ts` (43), `tokenEstimate.ts` (39), `providerTypes.ts` (23), `openCodeAuth.ts` (20), `providerEnablement.ts` (18), `apiKeyResolution.ts` (8), `thinking.ts` (30, legacy barrel) | ~2,428 | Cross-cutting utilities (plus the two proposed-API `.d.ts` module augmentations ≈ 231 LoC — `chatProvider` v6 + `languageModelThinkingPart` v1) | `config.ts` must stay **dependency-free** | +| **Root utilities** | `src/config.ts` (278), `contextWindowHook.ts` (485), `contextWindowHookBridge.ts` (122), `errors.ts` (294), `retry.ts` (463), `utils.ts` (186), `responsesRequest.ts` (180), `toolCallAccumulator.ts` (180), `imageNormalizer.ts` (118), `visionProxyCache.ts` (79), `runtimeDiagnostics.ts` (57), `chatParts.ts` (55), `reasoningHistory.ts` (43), `tokenEstimate.ts` (39), `providerTypes.ts` (23), `openCodeAuth.ts` (20), `providerEnablement.ts` (18), `apiKeyResolution.ts` (8), `thinking.ts` (30, legacy barrel) | ~2,428 | Cross-cutting utilities (plus the two proposed-API `.d.ts` module augmentations ≈ 231 LoC — `chatProvider` v6 + `languageModelThinkingPart` v1) | `config.ts` must stay **dependency-free** | --- @@ -360,7 +360,7 @@ Reuse these before writing new logic (all under `src/` root unless noted): | `apiKeyResolution.ts` | `resolveResponseApiKey` cold-start fallback | provider | | `providerEnablement.ts` | `providerEnabledSetting()` — reads root config (never section-scoped) | extension + commands | | `reasoningHistory.ts` | `thinkingTextFromValue`, `shouldEchoThinkingHistory` (family-gated echo) | messages | -| `toolCallAccumulator.ts` | `ToolCallAccumulator` (flush only on `tool_calls` finish reason) | extractors | +| `toolCallAccumulator.ts` | `ToolCallAccumulator` (flush only on `tool_calls` finish reason; `argumentsDone` delta REPLACES pending arguments — done-event repair, #244) | extractors | | `contextWindowHook*.ts` | context-window usage injection — **monkey-patches** Copilot's internal `$handleProgressChunk` + `Set.prototype`; bridge = lazy-load seam; silent no-op if capture fails | engine/provider | | `visionProxyCache.ts` | SHA-256 image-hash → vision-proxy description cache (FIFO, cap 200) | provider | | `runtimeDiagnostics.ts` | `runtimeDiagnosticsLines()` — version/host/platform/integrity lines for the Diagnostics command | provider | diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bea58213..0d3cbcb95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to the **OpenCode Go BYOK Provider** extension are documented here. +## [0.7.7] — 2026-09-24 + +### Fixed + +- **`[Responses]` Whitespace is no longer stripped from tool-call arguments on OpenAI models (#244).** `response.function_call_arguments.delta` fragments went through the trimming `firstString()` helper — the exact per-chunk-`.trim()` bug class fixed for text deltas in #192 (doc 83), but on a path the #192 fix had deliberately left alone under the wrong assumption that arguments tolerate trimming. Responses API argument fragments are arbitrary JSON slices that can split inside string values ("hello wo + rld"), so trimming each fragment corrupted tool-call input on every GPT-family model (all route to `/v1/responses`) on both Go and Zen — and the mangled arguments sent the model into reasoning loops. Fix is three-layered: fragments now go through `firstStringRaw()` (root cause); a new `response.function_call_arguments.done` handler emits the final authoritative arguments tagged `argumentsDone: true`; and `ToolCallAccumulator` treats that flag as a REPLACE instead of an append, healing any gateway-side delta mis-join even when fragments are mangled upstream. Regression tests cover the mid-string fragment split, the done-event mapping, and the replace semantics. Documented in `docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md`. + ## [0.7.6] — 2026-09-23 ### Fixed diff --git a/docs/devlog.md b/docs/devlog.md index 82526118e..c0501dc5c 100644 --- a/docs/devlog.md +++ b/docs/devlog.md @@ -1,6 +1,20 @@ # 🧠 OPENCODE COPILOT CHAT DEVLOG -**Branch:** `chore/models-dev-data-sync` (work on `main`) | **Updated:** 2026-09-23 Asia/Jakarta | **Current Phase:** issue #226 fix — global `opencodego.thinking.*` now wins over the picker schema-default echo; verified end-to-end, pending commit. +**Branch:** `main` (fix uncommitted, branch TBD) | **Updated:** 2026-09-24 Asia/Jakarta | **Current Phase:** issue #244 fix — tool-call arguments whitespace on Responses API; implemented + tested (489/489), docs synced, CHANGELOG `[0.7.7]`, VSIX 0.7.7 built + installed locally, pending commit. + +--- + +## ✅ Issue #244 — Tool-call arguments whitespace stripped on OpenAI models — 2026-09-24 + +**Scope:** regression-class bug of #192 (doc 83). The #192 fix moved text/reasoning deltas to `firstStringRaw()` but deliberately left `arguments_delta` on the trimming `firstString()` — assuming arguments tolerate trimming. Wrong: Responses API argument fragments are arbitrary JSON slices that split **inside string values** ("hello wo + rld"), so per-fragment trim corrupted tool-call input on every GPT-family model (all route to `/v1/responses`) on Go and Zen, and the model looped retrying mangled tool calls. + +**Verification vs official sources:** openai-node `response-accumulator.ts` accumulates via raw `output.arguments += event.delta` (no trim, no falsy filter); openai-node test mocks split argument streams on whitespace boundaries — exactly the case trimming destroys; `response.function_call_arguments.done` carries the final authoritative `arguments`. Reporter diagnostics (Go `gpt-5.6-luna`, Zen `gpt-5.6-sol`) confirmed every affected request hit the `responses` endpoint with HTTP 200 + repeat-request/98% cache-hit retry patterns. + +**Fix (3 layers):** (1) root cause — `function_call_arguments.delta` → `firstStringRaw()` in `core/routing.ts`; (2) new `function_call_arguments.done` handler emitting the final arguments tagged `argumentsDone: true`; (3) `ToolCallAccumulator.collect()` treats `argumentsDone` as REPLACE-not-append — self-healing any gateway-side delta mis-join. + +**Tests:** 5 new regression tests (fragment split mid-string-value, done-event mapping, done-without-args, replace semantics, append-only unaffected). **489/489 pass**, `npm run compile` + typecheck clean. + +Docs: `docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md`, doc 83 annotated (superseded assumption), CHANGELOG `[Unreleased]`. --- diff --git a/docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md b/docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md new file mode 100644 index 000000000..0cdd3c27c --- /dev/null +++ b/docs/issues/107-20260924-issue244-tool-call-arguments-whitespace.md @@ -0,0 +1,67 @@ +# Issue #244 — Whitespace Stripped from Tool-Call Arguments on OpenAI Models (Responses API) + +**Status:** ✅ Solved +**Topic:** streaming / responses-api / tool-calls +**Updated:** 2026-09-24 +**Tags:** #responses-api #bug #whitespace #streaming #tool-calls +**GitHub Issue:** [#244](https://github.com/ltmoerdani/opencode-copilot-chat/issues/244) +**Related:** doc [83 — Responses API whitespace stripped (#192)](83-20260826-responses-api-whitespace-stripped.md) — same bug class, different delta path + +--- + +## Problem + +Reporter (KonradDebski, ext 0.7.6): all whitespace is stripped from tool calls and responses when using **OpenAI models** on both OpenCode Go and OpenCode Zen — every word concatenated (`thisiswhattheresponselookslike`). Corrupted tool-call arguments also cause the model to enter **reasoning loops** (stuck re-resolving mangled tool input). Other providers unaffected. Reproducible on a fresh VS Code profile with only this extension installed. + +## Root Cause — regression of the #192 fix's scope assumption + +Doc 83 (PR #194, merge `717f6b5`) fixed the visible-text path by introducing `firstStringRaw()` (no `.trim()`) for `output_text.delta` and reasoning deltas, and **deliberately left `arguments_delta` on the trimming `firstString()`** under the assumption that "whitespace stripping is correct for arguments". + +That assumption was wrong. OpenAI Responses API streams tool arguments as arbitrary JSON slices — fragments **can split inside JSON string values** (`"hello wo` + `rld"`). Trimming each fragment destroys the boundary whitespace: + +```ts +// src/core/routing.ts (BEFORE — bug) +if (eventType === "response.function_call_arguments.delta") { + const delta = firstString(data.delta, data.arguments_delta); // ← .trim() per fragment +``` + +Because GPT models route to the `responses` transport (`resolveModelRouting`), and in agent mode most output flows through tool calls, the corruption surfaced as "all whitespace stripped". + +## Reporter Diagnostics (from the issue, verified 2026-09-24) + +The reporter's OpenCode Diagnostics dumps (Go + Zen) corroborate the root cause: + +- **Every affected request hit the `responses` endpoint** — `gpt-5.6-luna` on `opencode.ai/zen/go/v1/responses` and `gpt-5.6-sol` on `opencode.ai/zen/v1/responses`. This is exactly the transport whose normalizer carried the bug; other families (DeepSeek, GLM, …) route to `chat-completions`/`messages`, whose tool-call arguments are appended raw — which is why the reporter saw "models from other providers work as intended". +- **All requests returned HTTP 200 with `finishReason: stop`** — the corruption happened silently in the normalization layer, invisible to the HTTP layer. Consistent with per-fragment `.trim()`, which leaves no trace on the wire. +- **Repeat-request pattern fits the reasoning loop**: Go — two `gpt-5.6-luna` calls 18 s apart with an overlapping prompt cache (37%); Zen — three `gpt-5.6-sol` calls in 4 minutes at 98.6–98.9% prompt-cache hit (identical prompts retried), then tiny completions (7–15 tokens). Classic agent retry-after-corrupted-tool-input behaviour. +- **`totalEvents: 11–98`** — many small delta fragments means many fragment boundaries, i.e. many chances for the trim to eat a space. +- Reported models carry `thinkingFamily: openai` — no think-tag filter is active for this family, ruling out `filterText` as the whitespace eater on the visible-text path. + +## Evidence (official sources, verified 2026-09-24) + +- **openai-node `response-accumulator.ts`**: the official SDK accumulates via raw `output.arguments += event.delta` — no trim, no falsy filtering, even for empty-string deltas. +- **OpenAI API reference** (`ResponseFunctionCallArgumentsDeltaEvent.delta`): "The function-call arguments delta that is added" — an incremental fragment to append. +- **openai-node test mocks** split argument streams **on whitespace boundaries**, exactly the case per-fragment trimming destroys. +- **`response.function_call_arguments.done`** carries the final authoritative `arguments` string — used as a repair net. + +## Fix + +| File | Change | +| ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/core/routing.ts` | `response.function_call_arguments.delta`: `firstString` → `firstStringRaw` (root-cause fix) | +| `src/core/routing.ts` | New handler for `response.function_call_arguments.done`: emits a `tool_calls` delta tagged `argumentsDone: true` carrying the final arguments | +| `src/toolCallAccumulator.ts` | `collect()` honours `argumentsDone` — **replaces** (not appends) pending arguments, healing any delta-join corruption even if a gateway mis-streams | + +Design note: the done-event path makes the fix **permanent** — even if some gateway trims or mangles delta fragments, the final `response.function_call_arguments.done` value overwrites the accumulated string, matching official SDK semantics (`output.arguments = event.arguments`). + +## Tests + +- `src/test/routing.test.ts` — "function_call_arguments whitespace preservation (#244)": fragments split inside a JSON string value must preserve the space; done-event mapping; done-event without arguments emits no choices. +- `src/test/toolCallAccumulator.test.ts` — "argumentsDone replaces accumulated arguments (#244)": replace semantics + append-only path unaffected. + +Verification: `npm test` 489/489 pass, `npm run compile` clean. + +## Impact + +- Models: all OpenAI-family models on Go and Zen (responses transport). Non-OpenAI providers unaffected (chat-completions arguments already appended raw). +- No changes to text/reasoning paths — the #192 fix remains intact. diff --git a/docs/issues/83-20260826-responses-api-whitespace-stripped.md b/docs/issues/83-20260826-responses-api-whitespace-stripped.md index d407c91ba..3dd1f0030 100644 --- a/docs/issues/83-20260826-responses-api-whitespace-stripped.md +++ b/docs/issues/83-20260826-responses-api-whitespace-stripped.md @@ -72,6 +72,8 @@ The bug affected two call sites in `src/core/routing.ts`: Non-text fields (`call_id`, `stop_reason`, `arguments_delta`) used `firstString()` correctly — `.trim()` is safe for identifiers and enums — and were left unchanged. +> **⚠️ Superseded assumption (2026-09-24):** leaving `arguments_delta` on the trimming `firstString()` turned out to be wrong. Responses API argument fragments can split **inside JSON string values**, so per-fragment trimming corrupts tool-call arguments and triggers reasoning loops. Fixed in issue #244 — see [doc 107](107-20260924-issue244-tool-call-arguments-whitespace.md). The text/reasoning part of this fix remains intact and in force. + ## Fix ### Changes in `src/core/routing.ts` @@ -93,7 +95,7 @@ function firstStringRaw(...values: unknown[]): string | undefined { 2. **`extractResponsesReasoningText()`** — changed to use `firstStringRaw()` for reasoning text deltas. -3. `firstString()` is **unchanged** — it continues to be used for non-text fields (`call_id`, `stop_reason`, `arguments_delta`) where `.trim()` is appropriate. +3. `firstString()` is **unchanged** in this fix — it stayed for non-text fields (`call_id`, `stop_reason`, `arguments_delta`). The `arguments_delta` part of that decision was later reversed by the #244 fix ([doc 107](107-20260924-issue244-tool-call-arguments-whitespace.md)); `call_id`/`stop_reason` still use `firstString()` today. ### Changes in `src/test/routing.test.ts` diff --git a/package-lock.json b/package-lock.json index c9c3baf98..b1ad652cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "opencode-copilot-chat", - "version": "0.7.5", + "version": "0.7.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "opencode-copilot-chat", - "version": "0.7.5", + "version": "0.7.7", "license": "MIT", "dependencies": { "@silvia-odwyer/photon-node": "^0.3.4" diff --git a/package.json b/package.json index bc4e30090..aadc7108b 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "opencode-copilot-chat", "displayName": "OpenCode for Copilot Chat: BYOK 30+ AI Models", "description": "Use 30+ frontier AI models (DeepSeek V4, Kimi K2.6, GLM-5.1, Qwen3.7, MiMo V2.5, MiniMax M2.7, free Claude Opus, GPT-5.5, Gemini 3.5, Grok) in GitHub Copilot Chat. Bring Your Own Key, no Copilot Pro needed.", - "version": "0.7.6", + "version": "0.7.7", "publisher": "ltmoerdani", "license": "MIT", "icon": "media/opencodego.png", diff --git a/src/core/routing.ts b/src/core/routing.ts index f524ae5cd..5fef65d42 100644 --- a/src/core/routing.ts +++ b/src/core/routing.ts @@ -100,7 +100,11 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown { } if (eventType === "response.function_call_arguments.delta") { - const delta = firstString(data.delta, data.arguments_delta); + // Arguments fragments can split INSIDE JSON string values (OpenAI's own + // accumulator does `output.arguments += event.delta` with no trimming), + // so per-fragment trimming corrupts values like "hello wo" + "rld" and + // triggers tool-call reasoning loops (issue #244, regression class of #192). + const delta = firstStringRaw(data.delta, data.arguments_delta); return delta ? { choices: [ @@ -144,6 +148,36 @@ export function normalizeResponsesStreamEvent(data: unknown): unknown { return text ? { choices: [{ index: 0, delta: { responseDoneText: text }, finish_reason: null }] } : { choices: [] }; } + // response.function_call_arguments.done carries the FINAL arguments string. + // Use it as an authoritative repair: the accumulator REPLACES (not appends) + // the pending arguments so any corruption from mis-joined delta fragments is + // healed at stream end (issue #244). + if (eventType === "response.function_call_arguments.done") { + const args = firstStringRaw(data.arguments); + if (args === undefined) { + return { choices: [] }; + } + return { + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: typeof data.output_index === "number" ? data.output_index : 0, + id: firstString(data.call_id, data.item_id) ?? "", + type: "function", + function: { arguments: args }, + argumentsDone: true, + }, + ], + }, + finish_reason: null, + }, + ], + }; + } + if (eventType === "response.output_item.done") { const item = data.item; if (isRecord(item) && item.type === "message" && Array.isArray(item.content)) { diff --git a/src/test/routing.test.ts b/src/test/routing.test.ts index 693d79503..97dc08a2d 100644 --- a/src/test/routing.test.ts +++ b/src/test/routing.test.ts @@ -176,3 +176,42 @@ describe("normalizeResponsesStreamEvent — output_text.delta whitespace preserv assert.equal(normalized.join(""), "thisiswhattheresponselookslike"); }); }); + +describe("normalizeResponsesStreamEvent — function_call_arguments whitespace preservation (#244)", () => { + it("preserves whitespace across argument fragments split inside JSON string values", () => { + // Regression of #244: fragments previously went through firstString() + // which trimmed each chunk, destroying spaces at fragment boundaries. + const fragments = ['{"query":"what', " is 2", " plus 2", `?"}`]; + + const normalized = fragments.map((fragment) => { + const result = normalizeResponsesStreamEvent({ + type: "response.function_call_arguments.delta", + delta: fragment, + }) as { choices: { delta: { tool_calls: { function: { arguments: string } }[] } }[] }; + return result.choices[0]?.delta.tool_calls[0]?.function.arguments ?? ""; + }); + + assert.equal(normalized.join(""), `{"query":"what is 2 plus 2?"}`); + }); + + it("maps response.function_call_arguments.done to a replacing tool_calls delta", () => { + const result = normalizeResponsesStreamEvent({ + type: "response.function_call_arguments.done", + output_index: 0, + call_id: "call_1", + arguments: `{"query":"hello world"}`, + }) as { choices: { delta: { tool_calls: Record[] } }[] }; + + const call = result.choices[0]?.delta.tool_calls[0]; + assert.equal(call.argumentsDone, true); + assert.equal((call.function as { arguments: string }).arguments, `{"query":"hello world"}`); + assert.equal(call.id, "call_1"); + }); + + it("function_call_arguments.done with no arguments emits no choices", () => { + const result = normalizeResponsesStreamEvent({ type: "response.function_call_arguments.done" }) as { + choices?: unknown[]; + }; + assert.equal(result.choices?.length, 0); + }); +}); diff --git a/src/test/toolCallAccumulator.test.ts b/src/test/toolCallAccumulator.test.ts index 6cad0b44e..c2eb5c39f 100644 --- a/src/test/toolCallAccumulator.test.ts +++ b/src/test/toolCallAccumulator.test.ts @@ -215,3 +215,33 @@ describe("ToolCallAccumulator — hasCompletePendingCalls (#184)", () => { assert.equal(acc.hasCompletePendingCalls(), true); }); }); + +describe("ToolCallAccumulator — argumentsDone replaces accumulated arguments (#244)", () => { + it("replaces pending arguments with the authoritative done value", () => { + const accumulator = new ToolCallAccumulator(); + accumulator.collect([{ index: 0, id: "call_1", type: "function", function: { name: "search", arguments: '{"query":"what' } }]); + accumulator.collect([{ index: 0, function: { arguments: " is 2 plus 2?" } }]); + + // Corrupted accumulation (deltas mis-joined by a gateway) + accumulator.collect([ + { + index: 0, + type: "function", + function: { arguments: '{"query":"what is 2 plus 2?"}' }, + argumentsDone: true, + }, + ]); + + const flushed = accumulator.flush(); + assert.equal(flushed.length, 1); + assert.deepEqual(flushed[0].input, { query: "what is 2 plus 2?" }); + }); + + it("append-only path unaffected when argumentsDone is absent", () => { + const accumulator = new ToolCallAccumulator(); + accumulator.collect([{ index: 0, id: "call_1", type: "function", function: { name: "search", arguments: '{"a":' } }]); + accumulator.collect([{ index: 0, function: { arguments: "1}" } }]); + const flushed = accumulator.flush(); + assert.deepEqual(flushed[0].input, { a: 1 }); + }); +}); diff --git a/src/toolCallAccumulator.ts b/src/toolCallAccumulator.ts index b613f2dc7..9887b3fb7 100644 --- a/src/toolCallAccumulator.ts +++ b/src/toolCallAccumulator.ts @@ -20,7 +20,7 @@ export interface FlushedToolCall { input: object; } -function isRecord(value: unknown): value is Record { +export function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null; } @@ -84,7 +84,14 @@ export class ToolCallAccumulator { pending.name += fn.name; } if (typeof fn.arguments === "string") { - pending.arguments += fn.arguments; + // `argumentsDone` marks the authoritative final-arguments event + // (response.function_call_arguments.done) — REPLACE, not append, + // so repairs accumulated delta corruption (issue #244). + if (toolCall.argumentsDone === true) { + pending.arguments = fn.arguments; + } else { + pending.arguments += fn.arguments; + } } }