feat: preserve additional_tools input items verbatim and surface them in the tools UI - #5103
Conversation
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR preserves OpenAI Responses ChangesAdditional tools support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ResponsesInput
participant ResponsesMessage
participant LogDetailView
ResponsesInput->>ResponsesMessage: decode additional_tools payload
ResponsesMessage-->>ResponsesInput: preserve and re-emit raw JSON
ResponsesInput->>LogDetailView: provide responses_input_history
LogDetailView->>LogDetailView: combine and expand declared tools
LogDetailView-->>LogDetailView: render counts, metadata, and tool details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "Merge branch 'dev' into 07-10-feat_adds_..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/app/workspace/logs/sheets/logDetailView.tsx (1)
2288-2288: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueArray index used as list key for
declaredTools.map.
declaredToolsis now a runtime-composed array (top-level tools + flattened namespace/input-declared tools), so index-based keys are more likely to cause incorrect reconciliation than with a staticlog.params.toolsarray (e.g. if a namespace's nested tool count changes across renders/logs). Consider keying by a more stable identifier such as`${tool?.function?.name ?? tool?.name ?? "tool"}-${i}`.As per coding guidelines: "Always use stable, unique keys in lists; never use array index as key (unless unavoidable)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/workspace/logs/sheets/logDetailView.tsx` at line 2288, Replace the index-based key in the declaredTools.map render with a stable tool-derived key, using the function name or tool name and retaining the index only as a uniqueness fallback, such as `${tool?.function?.name ?? tool?.name ?? "tool"}-${i}`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 633-646: Deduplicate tool declarations before building the
rendered tools list and badge count. Update the logic around
inputDeclaredToolEntries, flattenDeclaredTools, and declaredTools to merge
top-level and historical additional_tools using tool identity with
last-write-wins semantics, so repeated declarations represent only the final
effective tool set before JSON serialization.
---
Nitpick comments:
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Line 2288: Replace the index-based key in the declaredTools.map render with a
stable tool-derived key, using the function name or tool name and retaining the
index only as a uniqueness fallback, such as `${tool?.function?.name ??
tool?.name ?? "tool"}-${i}`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f622528-5a1e-4917-82e4-da865f0f6c13
📒 Files selected for processing (5)
core/providers/openai/tool_search_roundtrip_test.gocore/schemas/responses.gocore/schemas/responses_test.goui/app/workspace/logs/sheets/logDetailView.tsxui/lib/types/logs.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/app/workspace/logs/sheets/logDetailView.tsx (1)
264-271: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a typed tool-declaration union instead of
any[].The new helper and caller erase type checking for namespace and callable tool fields. Define a narrow tool union in
ui/lib/types/logs.ts, retaining unknown provider-specific fields asunknown.As per coding guidelines, avoid
anyin TypeScript unless it is unavoidable.Also applies to: 636-640
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/workspace/logs/sheets/logDetailView.tsx` around lines 264 - 271, Replace the any-based declarations in flattenDeclaredTools and its caller with a typed tool-declaration union defined in ui/lib/types/logs.ts, covering namespace and callable tools while preserving provider-specific fields as unknown. Update the helper and related usage to use this type, including safe access and mapping of name/tools, without introducing any.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 264-271: Replace the any-based declarations in
flattenDeclaredTools and its caller with a typed tool-declaration union defined
in ui/lib/types/logs.ts, covering namespace and callable tools while preserving
provider-specific fields as unknown. Update the helper and related usage to use
this type, including safe access and mapping of name/tools, without introducing
any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 99695cbd-cf38-4f4e-a6f3-de0fd7c99ad9
📒 Files selected for processing (5)
core/providers/openai/tool_search_roundtrip_test.gocore/schemas/responses.gocore/schemas/responses_test.goui/app/workspace/logs/sheets/logDetailView.tsxui/lib/types/logs.ts
7e341a6 to
234faaa
Compare
234faaa to
41f8736
Compare
Co-authored-by: Akshay Deo <akshay@akshaydeo.com>
…, gofmt cleanup Upstream (maximhq#5103, adds addition_tools support) renamed ResponsesMessage's private rawToolSearch field to rawPreserved when generalizing raw-byte preservation beyond tool_search. tool_search_openai_native.go (this branch) predates that rename and still referenced the old name post-rebase.
3 participants
Summary
Codex code-mode models (e.g.
gpt-5.6-sol) send anadditional_toolsinput item containing tool definitions with per-entrytypediscriminators (custom,function,namespace). Previously, Bifrost's typed schema would decode these through themcp_list_toolsshape, stripping everytools[].typefield and causing OpenAI to reject the forwarded request with"Missing required parameter: 'input[0].tools[0].type'". This PR extends the verbatim round-trip preservation already in place fortool_search_call/tool_search_outputto also coveradditional_toolsitems, and surfaces those tools in the UI alongside top-level tool declarations.Changes
ResponsesMessageTypeAdditionalTools("additional_tools") to the set of item types that Bifrost preserves verbatim (rawPreserved, formerlyrawToolSearch), preventing the typed decoder from mangling nested tool entries.rawToolSearch→rawPreservedandisToolSearchItem→isRawPreservedItemto reflect that the mechanism now covers multiple item types."tool_search_call","tool_search_output", and"additional_tools"to the TypeScriptResponsesMessageTypeunion.additional_toolsinput items are now merged with top-levelparams.toolsfor display. Namespace-grouped tools are expanded into their callable children (namespace.toolnaming). The Tools tab badge, hero stat, and tools panel all reflect the combined count and source breakdown.flattenDeclaredToolshelper that expandsnamespace-type tool entries into individually callable children for display purposes.TestResponsesInputRoundTripsAdditionalToolsItemsandTestResponsesMessagePreservesAdditionalToolsto cover the round-trip behavior, including verifying that a reused receiver does not leak preserved bytes into a subsequent decode.Type of change
Affected areas
How to test
To reproduce the original failure, send a Responses API request for a code-mode model that includes an
additional_toolsinput item withcustom,function, andnamespace-typed tool entries and confirm OpenAI no longer rejects with a missingtypeparameter error.Breaking changes
Related issues
Fixes the
"Missing required parameter: 'input[0].tools[0].type'"rejection when proxying codex code-mode model requests.Security considerations
No auth, secrets, or PII implications. The change only affects how specific input item types are serialized when forwarded to OpenAI.
Checklist
docs/contributing/README.mdand followed the guidelines