Skip to content

fix(translation): preserve OpenAI Chat tool error flags - #920

Closed
ayushag-nv wants to merge 1 commit into
mainfrom
ayushag/preserve-chat-tool-error
Closed

ayushag-nv wants to merge 1 commit into
mainfrom
ayushag/preserve-chat-tool-error

Conversation

@ayushag-nv

@ayushag-nv ayushag-nv commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Why

OpenAI Chat tool messages carrying is_error: true currently decode to a tool result with no error flag. Routing algorithms then lose the caller's failure signal.

What

Preserve boolean is_error values when decoding tool messages. Missing, null and non-boolean values remain unset.

How

Read the field into ToolResult.is_error, matching the Anthropic decoder. This changes decoding only; it does not add fields to outgoing OpenAI requests.

Notes for reviewers

Where to Start Review

The one-line change in codecs/openai_chat/buffered.rs, followed by the table-driven regression in tests/request_translation.rs.

Test Plan

The regression failed before the fix. All 233 translation-crate tests, formatting and strict Clippy pass.

This enables removal of the compatibility bridge in PreProc #919 after a published SDK release includes the fix. The example remains on SDK 0.3.0 for now.

Summary by CodeRabbit

  • Bug Fixes

    • OpenAI Chat tool results now retain is_error when its value is true or false. Missing, null, or non-boolean values remain unset.
  • Tests

    • Added coverage for boolean, missing, null, and string values.

Signed-off-by: ayushag <ayushag@nvidia.com>
@ayushag-nv
ayushag-nv requested a review from a team as a code owner October 6, 2026 16:26
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA-NeMo/Switchyard/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: b091c5c1-3168-4a04-ae68-534fad2b240d
📥 Commits

Reviewing files that changed from the base of the PR and between b9e7ccc and 42f97f2.

📒 Files selected for processing (2)
  • crates/switchyard-translation/src/codecs/openai_chat/buffered.rs
  • crates/switchyard-translation/tests/request_translation.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


Walkthrough

OpenAI Chat tool-result decoding now preserves is_error when the message contains a boolean. Tests verify that missing and non-boolean values decode to None.

Changes

Tool-result error flag decoding

Layer / File(s) Summary
Decode and test tool-result error flags
crates/switchyard-translation/src/codecs/openai_chat/buffered.rs, crates/switchyard-translation/tests/request_translation.rs
The decoder preserves boolean is_error values. Tests verify that missing, null, and string values decode to None.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Merge Risk: ⚪ Minimal · up to 42f97

The change is limited to decoding tool-result error flags, with regression coverage for boolean and non-boolean values. No actionable merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preserving OpenAI Chat tool error flags during translation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the message flow,
True and false now safely show.
Missing flags stay tucked away,
Strings and nulls do not hold sway.
The tests hop through each case,
Then leave a neat and quiet trace.

Comment @coderabbitai help to get the list of available commands.

@ayushag-nv

Copy link
Copy Markdown
Contributor Author
Moved the decoder fix and regression test into #919 as requested. Closing this standalone PR; the fix is now carried by the example PR.

@ayushag-nv ayushag-nv closed this Oct 6, 2026
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.

1 participant