fix(openai_utils): strip redundant outer tool-call names - #3061
Merged
Conversation
`65129dd4` ("chore(deps): pin openai to 2.44.0", #2456, which closed
#2452) added `model_config = ConfigDict(extra="forbid")` to
`NeMoGymChatCompletionCreateParamsNonStreaming`. Pydantic propagates that
config into nested `TypedDict`s, so an unknown key anywhere in the
request -- including inside `tool_calls` -- became a hard 422 rather than
being ignored.
Agents echo tool calls back from whatever the provider returned, so
provider-specific keys on a tool call are routine. No benchmark under
`benchmarks/` emits that shape, which is why this went unnoticed, but any
external tool-calling agent hits it. Tau2/Tau3, whose agent lives in a
separate checkout, had all 776 rollouts of a run rejected.
Opt the two tool call shapes out of the inherited config so unknown keys
are dropped instead of rejected. The request model itself stays strict:
an unknown top-level request field is still a validation error, so the
schema-auditing intent of #2456 is preserved.
Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
5 tasks
ananthsub
reviewed
Sep 3, 2026
…ng extras Review feedback: `extra="ignore"` was too broad. It silently removed every non-OpenAI field from a tool call, and the config propagated into the nested `function` TypedDict as well, so unknown fields disappeared from `model_dump()`. That is worse than the bug it fixed. Tool calls carry provider state that a client must echo back verbatim -- Gemini returns `tool_calls[].extra_content.google.thought_signature` and requires the exact value on the next request -- so dropping it corrupts an otherwise valid trajectory and fails later, out of context. Declare the one known redundant field instead. `tool_calls[].name` round-trips, and an unknown field on a tool call is an error again, which fails loudly at the point of the mistake. Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
ananthsub
reviewed
Sep 3, 2026
Follow-up to #3061. Strips the non-standard outer `tool_calls[].name` field before strict validation while preserving nested function/custom-call names and all other strict unknown-field checks. The sanitizer is copy-on-write so caller input is not mutated. Tests cover function and custom calls, preservation of nested names, rejection of unrelated extra fields, and the provider-boundary payload.
ananthsub
changed the title
Contributor
|
/ok to test 7e5a057 |
ananthsub
approved these changes
Sep 3, 2026
ananthsub
enabled auto-merge (squash)
September 3, 2026 20:53
j-nolan
added a commit
to j-nolan/Gym
that referenced
this pull request
Sep 4, 2026
NVIDIA-NeMo#3061 added guards that the tool-call name normalization must not loosen the request model. Relaxing the schema loosens it deliberately, so those assertions now describe behaviour the model no longer has. Invert the two that assert rejection, and add one that pins down what did not change: a tool call missing its required function is still rejected, so carrying unknown keys has not turned off structural validation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
j-nolan
added a commit
to j-nolan/Gym
that referenced
this pull request
Sep 4, 2026
The model server forwards to engines that accept fields the OpenAI types do not define, and callers send them. vLLM's chat_template_kwargs selects a chat template variant and is how a caller turns a model's thinking mode on; clients also label tool results with the tool's name. Forbidding extras rejects the request outright, so the caller loses the field and the conversation with it. Requests are inbound from callers Gym does not control, so carrying what is not modelled is the right default for a proxy. Being strict about what Gym itself sends is unaffected, and structure is still validated: a tool call missing its required function is still rejected, which the new test pins down. This supersedes adding fields one at a time. Two have needed it already, name and chat_template_kwargs, and each was a separate change for a field vLLM already accepts. NVIDIA-NeMo#3061 added guards that tool-call name normalization must not loosen the request model. That loosening is now deliberate, so those assertions are inverted to describe it. Signed-off-by: James Nolan <jnolan@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Some external chat-completions agents replay assistant tool calls with a redundant outer
namealongside the standard nestedfunction.nameorcustom.name. OpenAI's tool-call schema does not define the outer field.PR #2456 configured
NeMoGymChatCompletionCreateParamsNonStreamingto reject unknown fields. Pydantic applies that policy to its nestedTypedDictmodels, so Gym now rejects the entire request with HTTP 422 whentool_calls[].nameis present. Before that strict configuration was added, Gym accepted the request and omitted the redundant field before sending it to the model provider.Fix
The tool-call list now removes only the redundant outer
tool_calls[].namebefore strict schema validation. The normalization is copy-on-write, so it does not mutate the caller's request object.The nested
function.nameorcustom.nameremains unchanged and continues to identify the tool. Every other unknown request or tool-call field remains a validation error. This avoids silently discarding provider state that may be required when an agent replays a tool call.Relationship to #2963
This change and #2963 handle different fields:
nameinside an assistant message'stool_calls[]entry.namein the subsequentrole: "tool"result message.Neither change replaces the other, and they can merge independently.
Validation
Regression coverage verifies function and custom tool calls, preservation of their nested names, rejection of unrelated unknown fields, caller-input immutability, and the payload received by the model server. The original Tau3 benchmark failure was reproduced with the same request shape, and removing the redundant outer field prevents the validation failure.
Checklist