From c7481c7464161fa65daf3eaa35735ed6228eff2c Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sun, 9 Aug 2026 23:44:05 +0200 Subject: [PATCH] fix(openai): bound tool-call arguments by bytes, not provider chunking Every scheduled run of at least one production project has been failing hourly with: veryfront-cloud request failed: invalid successful stream (tool call arguments exceeded 4096 fragments) `appendOpenAIStreamToolArgument` counted every fragment against `MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS`, including the ones that carried bytes. At typical streaming delta sizes of 5-20 UTF-8 bytes, 4096 fragments is roughly 20-80 KB -- about 2-8% of the 1 MiB that `MAX_OPENAI_STREAM_TOOL_ARGUMENT_BYTES` nominally allows. The fragment cap therefore always fired first and the byte budget was unreachable. Fragment count is also the wrong thing to hold a caller to: it is chosen by the provider's tokenizer, not by the size or complexity of the arguments the agent produced, so the same tool call passed or failed depending on how the stream happened to be chunked. Both provider streams' existing tests show what the cap is actually for -- each floods `arguments: ""` / `delta: ""`. Zero-byte fragments never advance the byte budget, so they are the only ones that can arrive without bound. Count only those against the cap. Content is then bounded by bytes, and since every kept fragment is at least one byte, the chunk array cannot outgrow the byte budget either. Behaviour preserved: both existing flood tests still trip the cap unchanged, because they flood empty fragments. Adds direct unit tests for the budget, which had none -- it was only covered indirectly through the two stream parsers. The new test fails before this change with "expected no limit for 16384 small fragments". Fixes veryfront-issue-inbox#432. Note the parent run reports `completed` while the child fails, so this was invisible from run status; that reporting gap is worth a separate look. --- .../src/openai-tool-input.test.ts | 74 +++++++++++++++++++ .../ext-llm-openai/src/openai-tool-input.ts | 29 ++++++-- 2 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 extensions/ext-llm-openai/src/openai-tool-input.test.ts diff --git a/extensions/ext-llm-openai/src/openai-tool-input.test.ts b/extensions/ext-llm-openai/src/openai-tool-input.test.ts new file mode 100644 index 0000000000..5d26d026f1 --- /dev/null +++ b/extensions/ext-llm-openai/src/openai-tool-input.test.ts @@ -0,0 +1,74 @@ +import { assertEquals } from "#veryfront/testing/assert.ts"; +import { describe, it } from "#veryfront/testing/bdd.ts"; +import { + appendOpenAIStreamToolArgument, + joinOpenAIStreamToolArguments, + MAX_OPENAI_STREAM_TOOL_ARGUMENT_BYTES, + MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS, + type OpenAIStreamToolArgumentBudget, +} from "./openai-tool-input.ts"; + +function emptyBudget(): OpenAIStreamToolArgumentBudget { + return { bytes: 0, fragments: 0 }; +} + +describe("ext-llm-openai/openai-tool-input", () => { + it("accepts far more non-empty fragments than the fragment cap while under the byte budget", () => { + // The provider's tokenizer decides how finely arguments are chunked, so a + // caller cannot control fragment count. Counting non-empty fragments against + // the flood guard rejected ordinary tool calls at roughly 2-8% of the byte + // budget they were nominally allowed -- every scheduled run of one project + // failed on this hourly. The byte budget is what bounds content. + const budget = emptyBudget(); + const chunks: string[] = []; + const fragment = '{"a":1},'; + const count = MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS * 4; + + let limit: string | undefined; + for (let i = 0; i < count; i++) { + limit = appendOpenAIStreamToolArgument(budget, chunks, fragment); + if (limit) break; + } + + assertEquals(limit, undefined, `expected no limit for ${count} small fragments`); + assertEquals(chunks.length, count, "every non-empty fragment must be kept"); + assertEquals( + budget.bytes < MAX_OPENAI_STREAM_TOOL_ARGUMENT_BYTES, + true, + "the payload must still be well inside the byte budget", + ); + assertEquals(joinOpenAIStreamToolArguments(chunks).length, fragment.length * count); + }); + + it("still stops an unbounded flood of zero-byte fragments", () => { + // Zero-byte fragments never advance the byte budget, so they are the only + // ones that can arrive without bound. They are what this cap guards. + const budget = emptyBudget(); + const chunks: string[] = []; + + let limit: string | undefined; + for (let i = 0; i < MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS + 10; i++) { + limit = appendOpenAIStreamToolArgument(budget, chunks, ""); + if (limit) break; + } + + assertEquals(limit, "fragments", "empty-fragment floods must still be rejected"); + assertEquals(chunks.length, 0, "empty fragments are never appended"); + }); + + it("still enforces the byte budget", () => { + const budget = emptyBudget(); + const chunks: string[] = []; + const big = "x".repeat(MAX_OPENAI_STREAM_TOOL_ARGUMENT_BYTES); + + assertEquals(appendOpenAIStreamToolArgument(budget, chunks, big), undefined); + assertEquals(appendOpenAIStreamToolArgument(budget, chunks, "y"), "bytes"); + }); + + it("counts multi-byte characters by UTF-8 length, not code units", () => { + const budget = emptyBudget(); + const chunks: string[] = []; + appendOpenAIStreamToolArgument(budget, chunks, "€"); + assertEquals(budget.bytes, 3); + }); +}); diff --git a/extensions/ext-llm-openai/src/openai-tool-input.ts b/extensions/ext-llm-openai/src/openai-tool-input.ts index a3c4835b61..fdc5f057ab 100644 --- a/extensions/ext-llm-openai/src/openai-tool-input.ts +++ b/extensions/ext-llm-openai/src/openai-tool-input.ts @@ -13,20 +13,35 @@ export function appendOpenAIStreamToolArgument( chunks: string[], fragment: string, ): "bytes" | "fragments" | undefined { - if (budget.fragments >= MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS) { - return "fragments"; + const fragmentBytes = TOOL_ARGUMENT_ENCODER.encode(fragment).byteLength; + + // Zero-byte fragments never advance the byte budget, so they are the only + // ones that can arrive without bound -- and they are what the fragment cap + // exists to stop. Both provider streams exercise it exactly that way, by + // flooding empty deltas. + // + // Counting non-empty fragments here too made the cap bind long before the + // byte budget: at typical delta sizes, 4096 fragments is roughly 2-8% of the + // 1 MiB a tool call is nominally allowed, which left the byte limit + // unreachable. Fragment count is chosen by the provider's tokenizer rather + // than by the caller, so the same tool call passed or failed depending on how + // the stream happened to be chunked. + if (fragmentBytes === 0) { + if (budget.fragments >= MAX_OPENAI_STREAM_TOOL_ARGUMENT_FRAGMENTS) { + return "fragments"; + } + budget.fragments++; + return undefined; } - const fragmentBytes = TOOL_ARGUMENT_ENCODER.encode(fragment).byteLength; if (fragmentBytes > MAX_OPENAI_STREAM_TOOL_ARGUMENT_BYTES - budget.bytes) { return "bytes"; } - budget.fragments++; + // Content is bounded by bytes: every fragment kept here is at least one byte, + // so the array cannot outgrow the byte budget. budget.bytes += fragmentBytes; - if (fragment.length > 0) { - chunks.push(fragment); - } + chunks.push(fragment); return undefined; }