-
Notifications
You must be signed in to change notification settings - Fork 0
fix(openai): bound tool-call arguments by bytes, not provider chunking #3521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an OpenAI-compatible provider emits very small nonempty deltas, this condition bypasses the fragment cap entirely, so both stream parsers can retain up to 1,048,576 separately allocated strings before the byte budget fires. The payload is limited to 1 MiB, but the array and per-string overhead can consume tens of megabytes per concurrent request, whereas the previous implementation retained at most 4,096 chunks. Allow provider-controlled chunking without retaining every fragment separately, for example by coalescing chunks while maintaining the byte limit.
Useful? React with 👍 / 👎.