fix(responses-converter): strip strict from function tools before engine dispatch

[pthombre]

Summary

The openai-2.44 pin rework (65129dd, #2456) stopped popping strict from function tool definitions in responses_to_chat_completion_create_params. This restores the pre-#2456 strip.

strict is an OpenAI-hosted enforcement flag with no Chat Completions semantics on self-hosted engines: vLLM 0.25.1's FunctionDefinition keeps it, and chat templates that render unknown function keys (e.g. Nemotron's render_extra_keys) inject <strict>True</strict> into every <function> block. That perturbs the prompt of every tool-bearing request off-distribution.

Observed impact

In RL training (jobs 6707880/6712632 vs baseline 6682989): ~17% of sequences per step ran away to the max-token cap, collected reward-0 truncation penalties, and were masked by sequence logprob error (1e4–1e14). Restoring the strip returns the engine request to byte-parity with the pre-#2456 tree.

Question for reviewers

Was dropping the strip in #2456 intentional? If strict support on self-hosted engines is planned, a config flag may be preferable — but unconditional stripping matches pre-#2456 behavior and strict has no Chat Completions semantics today.

Changes

• nemo_gym/responses_converter.py: pop strict from function tool dicts before building NeMoGymChatCompletionToolParam
• tests/unit_tests/test_responses_converter.py: assert strict does not reach the engine params

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[pthombre]

/ok to test be5128c

[pthombre]

/claude review

[claude]

SHIP — no reliability concerns.

Focused fix: drops the strict key from function tool definitions on the Responses→Chat conversion path (responses_converter.py:307). The reasoning holds up — on self-hosted engines, chat templates that render unknown function-definition keys (Nemotron's render_extra_keys) inject strict into the prompt, perturbing every tool-bearing request off-policy. That's a silent prompt-corruption risk across the training/eval majority path, so dropping it is the right call.

One thing worth noting (NOTE, author's call): the drop is unconditional, so it also affects the OpenAI-hosted path via inference_provider, where Chat Completions strict does carry real structured-output enforcement semantics. This restores pre-openai-2.44-pin behavior as the comment says, so it's a deliberate and defensible tradeoff — the self-hosted prompt-perturbation harm outweighs losing hosted schema enforcement — but if any downstream eval relied on hosted strict structured outputs, that enforcement is now gone silently. Test updated to assert the drop; correctness of verify()/scoring is untouched. LGTM.

[pthombre]

/ok to test 8135c93

[pthombre]

/claude review

[pthombre]

/ok to test 134c3db

[claude]

HOLD — the production change is sound, but two test edits will break CI.

The fix itself (app.py) is correct. Stripping OpenAI-hosted-only strict at both vLLM boundaries — _preprocess_chat_completion_create_params (chat path) and _chat_completions_via_completions_api (completions path) — is the right place. Rendering an unknown strict key into the prompt via render_extra_keys would perturb every tool-bearing request, so this is a real correctness fix for Nemotron-style templates. Guards (.get("tools") or [], .get("function") or {}, type=="function" check) are safe against missing/custom tools, and the dedicated unit test covers them.

BLOCKER (inline): The edits to test_responses_multistep and test_responses_multistep_multiturn remove "strict": True from expected_sent_tools, but those tests mock chat_completions — the exact method the strip lives inside. On the /v1/responses path the strip is bypassed, and the responses→chat converter (responses_converter.py:302) keeps strict, so the captured outbound tools still carry it. The base tests had "strict": True here and were green; removing it makes the assertions mismatch and gym env test --resources-server vllm_model fail. Fix: revert those two expected_sent_tools edits. The new test_strip_hosted_only_tool_fields_pops_strict already covers the strip directly.

Note: the strip runs only on the real vLLM HTTP boundary. Per CLAUDE.md ("green unit tests alone are not enough"), a real rollout against a Nemotron template would confirm the prompt no longer carries strict — worth doing before merge, but not a blocker on its own.

[pthombre]

/ok to test dffac65

[pthombre]

/claude review

[claude]

SHIP — LGTM, no reliability concerns.

_strip_hosted_only_tool_fields correctly removes the OpenAI-hosted-only strict flag at the vLLM boundary, preventing chat templates with render_extra_keys (Nemotron) from injecting the unknown key into every tool-bearing prompt.

Verified coverage of both tool-forwarding paths:

• chat_completions() → _preprocess_chat_completion_create_params (app.py:501) strips before create_chat_completion (app.py:801).
• /v1/completions path (app.py:1110) strips before _render_messages_via_chat_template → apply_chat_template.

No third path hands tools to vLLM. The strip mutates the same body_dict that is forwarded, gates on type == "function", and tolerates missing/None tools/function. Hosted-provider strict semantics are preserved since this only runs in the vLLM model server. Tests assert real behavior on both the static helper and the preprocess integration.