-
Notifications
You must be signed in to change notification settings - Fork 91
fix: prevent OpenAI tool output double-wrapping for error objects #1172
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
fix: prevent OpenAI tool output double-wrapping for error objects #1172
Conversation
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR adds a "humanizeJson" feature to prevent double-wrapping JSON tool outputs when displaying structured results. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-11-16T22:51:26.374ZApplied to files:
📚 Learning: 2026-01-13T19:28:00.789ZApplied to files:
🧬 Code graph analysis (3)packages/core/src/providers/openai/buildResponsesRequest.stripToolCalls.test.ts (2)
packages/core/src/providers/openai/buildResponsesRequest.ts (1)
packages/core/src/providers/openai/OpenAIProvider.ts (1)
🔇 Additional comments (9)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Issue Summary:: The user wants me to review PR #1172 which fixes OpenAI tool output double-wrapping for error objects. Let me analyze the PR:
The problem this PR solves: The solution:
Let me look at the files changed and evaluate the implementation:
I'll examine the test coverage for the new humanize JSON functionality. The tests validate key behaviors:
The test suite ensures robust handling of different tool output scenarios, verifying the new humanize mode works as expected across various input types. Critical issues reveal potential implementation weaknesses:
The review highlights two specific implementation concerns around null and empty value processing. While not overly complex, these edge cases require careful consideration to maintain predictable string conversion behavior. Documentation updates, though minimal, demonstrate thoughtful attention to clarifying the tool output specification. The approach balances simplicity with precise technical guidance. Comprehensive test coverage spans multiple functional paths, including error scenarios, stdout/stderr rendering, and message formatting. This systematic testing strategy suggests a robust implementation approach. The implementation carefully preserves existing module behaviors while introducing targeted enhancements for OpenAI tool interactions. Conditional logic ensures backward compatibility with previous code paths. Documentation appears comprehensive, addressing key implementation details for the new feature. The note clarifies nuanced tool result handling, explaining human-readable rendering for structured results. The approach systematically mitigates potential double-wrapping issues, with explicit tests validating error and multi-line output processing. Each implementation step seems thoughtfully designed to maintain existing system integrity while introducing targeted improvements. The test coverage and documentation strategy demonstrate a methodical approach to feature implementation, ensuring robust and clear communication of technical changes. toolResponsePayload.ts: Let me analyze this PR #1172 which fixes OpenAI tool output double-wrapping for error objects. The PR addresses issue #1171, which is about OpenAI tool outputs still getting JSON-wrapped when the tool result is an object (e.g., {error, stdout, stderr}). This reintroduces escaping and breaks real newlines. Let me examine the changes:
Key observations:
Let me check for any issues:
The PR looks good. Let me finalize the review. LLxprt PR Review – PR #1172Issue Alignment
Side Effects
Code Quality
Tests & Coverage
Verdict
Let me analyze:
I confirm the implementation precisely targets the core problem. The solution introduces a flexible mechanism for transforming complex tool output objects into more readable formats, specifically for OpenAI providers. By adding a configurable humanization mode, the PR addresses JSON-wrapping issues while maintaining backward compatibility and providing sensible defaults. LLxprt PR Review – PR #1172Issue Alignment
Side Effects
Code Quality
Tests & Coverage
Verdict
|
Proposed change
Improves OpenAI tool output rendering when a tool returns a JSON-like result object (e.g.
{ error: string },{ stdout, stderr, exitCode }).Instead of embedding a JSON string inside the
output:section (double-wrapping), the OpenAI provider now prefers a human-readable multi-line rendering that:output:\n{"error":"..."}.Implementation details
buildToolResponsePayload()with an optional humanize JSON mode for OpenAI tool outputs.humanizeJsonForDisplay()to detect common result shapes and render them as readable multi-line text:error,message, etc.exitCode,stdout,stderras stable multi-line blocksOpenAIProvider.formatToolMessage()buildResponsesRequest()forfunction_call_output{ error: "multiline..." }does not becomeoutput:\n{"error": ...}stdout/stderr/exitCodeare rendered as multi-line blocks with stable spacingdocs/tool-output-format.mdclarifying preference for human-readable rendering for structured resultsNotes
Related issue
Fixes #1171 — OpenAI tool outputs were still getting JSON-wrapped when the tool result was an object,
reintroducing over-escaping and reducing readability even after #1002.