Skip to content

fix: preserve streamed tool call arguments - #3762

Merged
diegosouzapw merged 7 commits into
cheaper-inference:release/v3.8.24from
Mffff4:fix/tool-call-integrity
Jun 13, 2026
Merged

diegosouzapw merged 7 commits into
cheaper-inference:release/v3.8.24from
Mffff4:fix/tool-call-integrity

Conversation

@Mffff4

@Mffff4 Mffff4 commented Jun 13, 2026

Copy link
Copy Markdown

Summary

  • Fixed integrity preservation of tool call arguments during streaming (SSE parsing, transformations, and response translation).

Related Issues

Validation

  • npm run lint
  • npm run test:unit
  • npm run test:coverage
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Tests Added Or Updated

  • tests/e2e-tool-calls.sh (new)
  • tests/unit/responses-transformer.test.ts
  • tests/unit/sse-parser.test.ts
  • tests/unit/sseTextTransform.test.ts
  • tests/unit/streamingPiiTransform.test.ts
  • tests/unit/translator-resp-openai-responses.test.ts

Coverage Notes

  • Added unit and e2e tests covering SSE stream parsing, PII transform behavior with tool calls, and payload translation to ensure tool call arguments are preserved intact.

Reviewer Notes

  • Added docs/fixes/TOOL_CALL_INTEGRITY.md detailing the integrity mechanism for streamed tool calls.

diegosouzapw and others added 2 commits June 13, 2026 00:07
Bumps [esbuild](https://github.com/evanw/esbuild) from 0.28.0 to 0.28.1.
- [Release notes](https://github.com/evanw/esbuild/releases)
- [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md)
- [Commits](evanw/esbuild@v0.28.0...v0.28.1)

---
updated-dependencies:
- dependency-name: esbuild
  dependency-version: 0.28.1
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@Mffff4
Mffff4 requested a review from diegosouzapw as a code owner June 13, 2026 10:54

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a critical issue where streaming tool call arguments were corrupted or duplicated during SSE translation. It introduces a byte-preserving bypass for machine JSON fields (toolArgs and partialJson) in the text and PII transformation pipelines, and implements an overlap-aware delta assembler (appendToolCallArgumentDelta) to handle repeated snapshots. The changes are accompanied by comprehensive unit and E2E tests, as well as Dockerfile updates to support cloakbrowser. Feedback on the changes includes a high-severity fix for a fragile jq filter in the E2E test script that can cause false warnings, and a performance optimization for the overlap detection algorithm to prevent blocking the event loop.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/e2e-tool-calls.sh
Comment thread open-sse/utils/toolCallArguments.ts Outdated
Mffff4 and others added 3 commits June 13, 2026 14:00
…truncation

The fuzzy suffix/prefix-overlap heuristic in appendToolCallArgumentDelta
dropped bytes from legitimate incremental tool-call deltas: when a
fragment's leading bytes repeated the buffer's tail it silently collapsed
them (`ls -ll` -> `ls -l`, `xx` -> `x`), trading the visible duplication
bug from #3701 for a silent truncation bug across all four streaming paths
(sseParser, responsesTransformer, openai-responses, openai-to-claude) —
including the openai-to-claude path that feeds Claude Code / Droid.

Restrict dedup to the unambiguous snapshot case only (identical repeat or
a growing superset that still starts with the buffer); append every other
fragment verbatim. Add a direct unit test for the accumulator covering
incremental fragments with repeated chars, plus an incremental-fragment
transformer test — both fail against the old heuristic. Also strengthen the
snapshot-dedup transformer test to assert the streamed delta events
reconstruct the arguments exactly once.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.24 June 13, 2026 11:33
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @Mffff4 — this is a valuable fix and the diagnosis of #3701 is spot on. The most important part of your change is correct and lands as-is: routing tool-call arguments / partial_json around the PII and text-stream transforms (they're machine JSON, not human text) and emitting only the incremental slice (emittedDelta) instead of re-emitting the whole accumulated buffer. Those were the real sources of the duplicated-character corruption, and your tests for the PII bypass and the per-emit slicing are genuine regression guards.

During review I found one issue in the accumulator that I fixed in-place on your branch (pushed as a commit authored by you, with me as co-author) so the PR could merge cleanly:

open-sse/utils/toolCallArguments.ts — the fuzzy suffix/prefix-overlap dedup silently dropped bytes from legitimate incremental deltas. Providers like gpt-5.5 (Codex) and Kimi stream function.arguments as small incremental fragments, not full snapshots. When a fragment's leading byte(s) coincided with the buffer's trailing byte(s), the existing.endsWith(next) branch + the overlap loop collapsed them:

Deltas Expected Old heuristic
{"a":" x x "} {"a":"xx"} {"a":"x"}
{"cmd":"l l -l"} {"cmd":"ll -l"} {"cmd":"l -l"}

That turns a visible duplication bug into a silent truncation bug (ls -ll → ls -l) across all four streaming paths — including openai-to-claude.ts, which feeds the exact Claude Code / Droid clients in the issue. The fix restricts dedup to the unambiguous snapshot case only (an identical repeat, or a growing superset that still starts with the buffer) and appends every other fragment verbatim:

if (next === existing) return existing;     // identical snapshot repeat
if (next.startsWith(existing)) return next; // growing snapshot
return existing + next;                       // incremental delta — verbatim

I also added tests/unit/tool-call-arguments.test.ts (direct fragment-level coverage proving no byte loss) and an incremental-fragment test in responses-transformer.test.ts; both fail against the old heuristic and pass with the fix, and I strengthened your snapshot-dedup test to assert the streamed .delta events reconstruct the arguments exactly once.

Validated: 85/85 across the touched suites, typecheck:core clean, lint clean, no PII-redaction regression (content/reasoning redaction untouched; only machine tool-JSON bypasses it, by design). Merging into release/v3.8.24 for the upcoming release. Really appreciate the thorough write-up and the e2e harness. 🙌

@diegosouzapw
diegosouzapw merged commit 06e85a9 into cheaper-inference:release/v3.8.24 Jun 13, 2026
2 checks passed
diegosouzapw added a commit that referenced this pull request Jun 13, 2026
… v3.8.24 merges

These 5 files exceeded the frozen baseline solely because of already-merged,
already-reviewed v3.8.24 PRs that landed after this branch froze its baseline:
  - src/lib/providers/validation.ts        4209→4302  (#3767 qwen-web fix)
  - src/shared/components/RequestLoggerV2.tsx 1232→1276 (#3760 logs light-mode)
  - open-sse/executors/antigravity.ts      1553→1572  (#3766 antigravity refresh)
  - open-sse/executors/base.ts             1199→1205  (#3756 xhigh effort)
  - open-sse/translator/response/openai-responses.ts 873→878 (#3762 tool-call integrity)

None are touched by this branch; the bump reflects current release reality.
@Mffff4

Mffff4 commented Jun 13, 2026

Copy link
Copy Markdown
Author

Awesome, looking forward to the release so I can get back to using it properly)

diegosouzapw added a commit that referenced this pull request Jun 13, 2026
…contributor credits

- Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance
- Add bullets for every PR landed since v3.8.23 that was missing:
  marketplace (#3656), strict-mode CC defaults (#3776), emergency-fallback flag (#3752),
  xhigh effort (#3756), Codex memory WS (#3749), IPv6 egress (#3777),
  marketplace SSRF (#3774), CodeQL/Dependabot (#3778), anthropic sampling (#3780),
  thinking passthrough (#3775), mcp dist entry (#3765), streamed tool args (#3762),
  logs light-mode (#3760), clean-history purge (#3751), quality-gates (#3757),
  docs gaps (#3453), file-size re-baseline (#3770), E415 publish guard, i18n prune
- Move misplaced #3775 bullet out of [Unreleased] into [3.8.24]
- Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
@diegosouzapw diegosouzapw mentioned this pull request Jun 13, 2026
diegosouzapw pushed a commit that referenced this pull request Jun 24, 2026
…penai-to-claude streaming (#4951)

The regex-based strip introduced by #1852 matched patterns like `"key":""` and
`"key":[]` to remove spurious placeholder fields. However, since #3762 introduced
snapshot-dedup via appendToolCallArgumentDelta, the deltaStr computed for
single-chunk providers (e.g. deepseek-v4-flash-free) is the FULL args JSON, not a
raw incremental fragment. The strip therefore deleted any field whose value happened
to be "" or [], silently corrupting intentional empty-string arguments:

  {"file_path":"","content":"text"} → {"content":"text"}

Removed the strip entirely. The #3762 snapshot-dedup already collapses repeated
noise chunks into a zero-length delta (suppressed by the `!deltaStr` guard), so
the strip is no longer needed. Legitimate empty-string / empty-array argument values
now pass through unchanged.

Closes #4951
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
Integrated into release/v3.8.24 — tool-call argument dedup made snapshot-only to prevent silent truncation (fix-in-place + tests).
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…contributor credits

- Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance
- Add bullets for every PR landed since v3.8.23 that was missing:
  marketplace (cheaper-inference#3656), strict-mode CC defaults (cheaper-inference#3776), emergency-fallback flag (cheaper-inference#3752),
  xhigh effort (cheaper-inference#3756), Codex memory WS (cheaper-inference#3749), IPv6 egress (cheaper-inference#3777),
  marketplace SSRF (cheaper-inference#3774), CodeQL/Dependabot (cheaper-inference#3778), anthropic sampling (cheaper-inference#3780),
  thinking passthrough (cheaper-inference#3775), mcp dist entry (cheaper-inference#3765), streamed tool args (cheaper-inference#3762),
  logs light-mode (cheaper-inference#3760), clean-history purge (cheaper-inference#3751), quality-gates (cheaper-inference#3757),
  docs gaps (cheaper-inference#3453), file-size re-baseline (cheaper-inference#3770), E415 publish guard, i18n prune
- Move misplaced cheaper-inference#3775 bullet out of [Unreleased] into [3.8.24]
- Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Repeated / duplicated characters in CLI tool-use output when using OmniRoute models

2 participants