Skip to content

fix(translator): coerce submit_pr_review functionalChanges/findings to arrays - #2242

Merged
diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.0from
NomenAK:fix/translator-tool-call-shim-2026-05-14
May 14, 2026
Merged

diegosouzapw merged 3 commits into
diegosouzapw:release/v3.8.0from
NomenAK:fix/translator-tool-call-shim-2026-05-14

Conversation

@NomenAK

@NomenAK NomenAK commented May 14, 2026

Copy link
Copy Markdown
Contributor

Problem

Some strict-schema downstream tools (e.g. submit_pr_review with required functionalChanges/findings array fields) hard-reject when those fields arrive as null, plain object, empty string, or stringified array — malformation modes that can emerge from OpenAI-shape upstream models routed through the streaming openai-to-claude translator.

When the downstream tool rejects, the caller often enters a retry loop it cannot recover from on its own.

Root cause

The streaming OpenAI → Anthropic translator at open-sse/translator/response/openai-to-claude.ts passes tool-call deltas through verbatim. When the upstream model emits a malformed required-array field, the assembled tool args land downstream in the rejected shape.

A related side effect: the strip-empty heuristic from #1852 — correct in the general case — strips legitimate empty [] from these tools' args, so the field appears omitted to the strict-schema validator.

Fix

Adds a small TOOL_SHIMS registry under open-sse/translator/helpers/toolCallShim.ts. The SSE translator applies it on the streaming OpenAI → Anthropic tool_use path. For a registered tool the shim:

  1. Buffers raw tool args per shimmed tool (rather than passing partial deltas through).
  2. Suppresses the passthrough input_json_delta events for those tools while buffering.
  3. Emits one corrective input_json_delta carrying the fully-patched JSON just before content_block_stop.

Handles five malformation modes per field: omitted, null, plain object, empty string, stringified array. Zero overhead for non-shimmed tools.

Adding another strict-schema tool to the registry is one entry.

Test plan

  • 20/20 unit + streaming integration tests in tests/unit/translator-tool-call-shim.test.ts (new, 263 lines)
  • 4 adjacent translator suites stay green: translator-resp-openai-to-claude, openai-to-claude-strip-empty, empty-tool-name-loop, context-pinning-tool-calls (23 tests combined)
  • Typecheck passes against tsconfig.typecheck-core.json
  • Verified stable for ~24h on a downstream deploy under review-harness traffic (no recurring rejection observed since deploy)

Scope and trade-offs

Intentionally surgical:

  • Does not revise the strip-empty heuristic from [BUG] WebSearch returns empty result blocks on v3.7.7 with Claude Code #1852 — that fix is correct for the 99%+ case. The shim provides the per-tool defense where the heuristic is wrong, without changing the general path.
  • Registry is hard-coded (one entry, submit_pr_review). Open to making it config-driven if you'd prefer.
  • Covers the SSE streaming path only — that's where the malformations surfaced in flight. Happy to extend to the non-streaming branch if you think it's worth it.

Happy to split into smaller commits, revise, or scope down further.

dependabot Bot and others added 2 commits May 11, 2026 21:26
Bumps [mermaid](https://github.com/mermaid-js/mermaid) from 11.14.0 to 11.15.0.
- [Release notes](https://github.com/mermaid-js/mermaid/releases)
- [Commits](https://github.com/mermaid-js/mermaid/compare/mermaid@11.14.0...mermaid@11.15.0)

---
updated-dependencies:
- dependency-name: mermaid
  dependency-version: 11.15.0
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…o arrays

Some strict-schema downstream tools (e.g. submit_pr_review with required `functionalChanges`/`findings` array fields) hard-reject when those fields arrive as null, plain object, empty string, or stringified array — malformation modes that can emerge from OpenAI-shape upstream models routed through the streaming openai-to-claude translator. Without defense, the caller enters a retry loop it cannot recover from.

Adds a small `TOOL_SHIMS` registry that the SSE translator applies on the streaming OpenAI -> Anthropic tool_use path. For registered tools the shim buffers raw args, suppresses passthrough `input_json_delta` events, then emits one corrective delta with the fully patched JSON just before `content_block_stop`. Handles five malformation modes per field. Zero overhead for non-shimmed tools.

Side benefit: neutralises the collateral damage from the strip-empty heuristic in diegosouzapw#1852 when legitimate empty arrays are required by the tool schema.

Adding another strict-schema tool to the registry is one entry.

20/20 unit + streaming integration tests in `tests/unit/translator-tool-call-shim.test.ts`. Adjacent translator suites stay green.

Verified stable for ~24h on a downstream deploy under heavy review-harness traffic.
@NomenAK
NomenAK requested a review from diegosouzapw as a code owner May 14, 2026 09:06

@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 implements a shimming layer for tool calls to handle cases where upstream models emit malformed data, specifically ensuring that fields in the submit_pr_review tool are coerced into arrays. The implementation involves buffering tool arguments and re-emitting a corrected JSON delta at the end of the stream. A review comment correctly identified a potential memory performance issue where arguments are buffered for all tool calls regardless of whether they require shimming; the reviewer suggested limiting buffering to shimmed tools and removing a redundant string check.

Comment on lines +193 to +200
// Always buffer the raw stream so shimmed tools can re-emit a
// corrected JSON at stop time.
toolInfo.argBuffer = (toolInfo.argBuffer || "") + tc.function.arguments;

if (toolInfo.shimmed) {
// Suppress passthrough; we emit one corrective delta at finish.
continue;
}

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.

high

The current implementation buffers tool arguments for all tool calls, which introduces significant memory overhead for non-shimmed tools. For large tool outputs (e.g., file contents or large data sets), this could lead to excessive memory consumption or OOM errors in high-traffic environments. Buffering should be restricted to only those tools that require shimming.

Additionally, since argBuffer is initialized to "" at line 175, the (toolInfo.argBuffer || "") check is redundant.

          if (toolInfo.shimmed) {
            // Buffer the raw stream for shimmed tools to re-emit corrected JSON at stop time.
            toolInfo.argBuffer += tc.function.arguments;
            // Suppress passthrough; we emit one corrective delta at finish.
            continue;
          }

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.0 May 14, 2026 11:02
@diegosouzapw
diegosouzapw merged commit 52285d8 into diegosouzapw:release/v3.8.0 May 14, 2026
1 of 2 checks passed
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @NomenAK — squash-merged into release/v3.8.0 as 52285d8a. The shim approach is elegant: zero overhead for non-shimmed tools, single corrective input_json_delta before content_block_stop, and the 20 unit + streaming tests cover all five malformation modes per field plus the buffering contract. Easy to add new strict-schema tools to TOOL_SHIMS later. Appreciated.

diegosouzapw added a commit that referenced this pull request May 14, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: #2135 (1proxy settings), #2227 (antigravity project ID), #2238 (Z.AI Search), #2240 (CLI Suite)
- fix: #2217, #2218, #2219, #2221, #2222, #2223, #2224, #2231, #2233, #2236, #2242, #2243
- security: #2209 (stack trace exposure)
- chore: #2228, #2234

Total contributors updated from 50+ to 55+.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…o arrays (diegosouzapw#2242)

Integrated into release/v3.8.0 — surgical streaming translator shim for submit_pr_review functionalChanges/findings array fields.
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite)
- fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243
- security: diegosouzapw#2209 (stack trace exposure)
- chore: diegosouzapw#2228, diegosouzapw#2234

Total contributors updated from 50+ to 55+.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
…o arrays (diegosouzapw#2242)

Integrated into release/v3.8.0 — surgical streaming translator shim for submit_pr_review functionalChanges/findings array fields.
Poid-ZA pushed a commit to Poid-ZA/OmniRoute that referenced this pull request Aug 5, 2026
Deep audit of all 320 commits since v3.7.9 found:
- 18 merged PRs not documented in CHANGELOG (4 features, 10 bug fixes, 1 security, 2 chores, 1 debug improvement)
- 3 contributors entirely missing from credits table (@NomenAK with 12 PRs, @kang-heewon, @one-vs)
- 4 existing contributors with inaccurate PR counts (@oyi77 8→12, @ddarkr 2→3, @andrewmunsell 2→3, @nickwizard 2→3)

New entries added:
- feat: diegosouzapw#2135 (1proxy settings), diegosouzapw#2227 (antigravity project ID), diegosouzapw#2238 (Z.AI Search), diegosouzapw#2240 (CLI Suite)
- fix: diegosouzapw#2217, diegosouzapw#2218, diegosouzapw#2219, diegosouzapw#2221, diegosouzapw#2222, diegosouzapw#2223, diegosouzapw#2224, diegosouzapw#2231, diegosouzapw#2233, diegosouzapw#2236, diegosouzapw#2242, diegosouzapw#2243
- security: diegosouzapw#2209 (stack trace exposure)
- chore: diegosouzapw#2228, diegosouzapw#2234

Total contributors updated from 50+ to 55+.
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.

2 participants