Repository navigation
fix(cli): treat servers nested three levels deep as editable installs - #4464
Merged
Merged
Conversation
The two-level pyproject.toml check missed nested servers (e.g. conversational_tool_use/simulation), so their venvs installed nemo-gym from the package index instead of the checkout. Signed-off-by: Christian Munley <cmunley@nvidia.com>
cmunley1
marked this pull request as ready for review
October 12, 2026 08:49
Contributor
Author
|
/ok to test 77111e6 |
yaoyu-33
added
area:cli
ananthsub
approved these changes
Oct 12, 2026
Contributor
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/38258590564 |
dims
pushed a commit
to dims/Nemo-Gym
that referenced
this pull request
Oct 14, 2026
…train judge JSON, report mask_sample (NVIDIA-NeMo#4568) ## What does this PR do? The nested conversational tool-use environment (`resources_servers/conversational_tool_use_simulation` with the `conversational_tool_use/simulation` agent) now starts from a source checkout, and its rollouts are graded and masked correctly. Before this PR its stack never came up: every model server bound to the agent crashed during startup. Behind that crash were two more defects, which this PR also fixes. The trajectory judge often returned JSON that failed validation, and the server's mask decision never reached the `mask_sample` field that Gym and trainers read. Fixes NVBug [6847461](https://nvbugspro.nvidia.com/bug/6847461). Closes NVIDIA-NeMo#4497. ### Cause 1. **Startup crash.** `ServerClient.assistant_message_header` (`nemo_gym/server_utils.py:825`, added by NVIDIA-NeMo#3711) imports `responses_api_agents.{harness}` with the raw config key. Nested harnesses are keyed by directory path, so `conversational_tool_use/simulation` raises `ModuleNotFoundError` in `policy_model` setup (`base_responses_api_model.py:286`). The venv blocker in the same NVBug (a pin to `nemo-gym==<version>`) was already fixed by NVIDIA-NeMo#4464. 2. **Unparseable judge output.** The judge prompt contains `Evaluation.model_json_schema()` as text, and the reply is parsed with `model_validate_json`, with no decoding constraint (since NVIDIA-NeMo#2111). With the QA model (temperature 1.0), the step-0 judgement failed all 3 attempts in 3 of 3 runs. Replies either wrapped the instance in `{"properties": {...}}` or left a string unquoted, so the row got `verification_generation_error`. 3. **Mask dropped.** NVIDIA-NeMo#2111 reported the mask decision only as `instance_config.mask_sample`. NVIDIA-NeMo#2611 later made `BaseVerifyResponse.mask_sample` the contract. This environment was never migrated, so simulator, tool, and judge failures reached rollouts with `mask_sample: false` and reward 0. ### Fix 1. Map `/` to `.` before importing the harness package. 2. When `judge_responses_create_params.text` is not configured, the judge requests Responses `text.format` (`json_schema`) for `Evaluation`, or for `UserAgentEnvironmentEvaluation` in combined mode. A configured `text` still wins. `openai_model` forwards it to the provider; the chat converter maps it to `response_format`. 3. The resources server's `/verify` and the agent's transient-failure response set top-level `mask_sample`. `instance_config` is removed, and the checked-in `example_rollouts.jsonl` is migrated. The README states the masking rule. Rejected: catching `ImportError` in `assistant_message_header`. It would hide a declared header from any nested harness, and the new test fails on it. ### Before the fix Gym `4a101eeb5` (main), QA case `conversational_tool_use_simulation`: ```bash ./run.sh test conversational_tool_use_simulation --ref 4a101ee --no-ai-log ``` ```text nemo_gym/base_responses_api_model.py", line 286, in setup_webserver assistant_message_header=self.server_client.assistant_message_header(self.config.name), nemo_gym/server_utils.py", line 825, in assistant_message_header ModuleNotFoundError: No module named 'responses_api_agents.conversational_tool_use/simulation' RuntimeError: Process `policy_model` finished unexpectedly! ERROR (30.2s): StackStartupError: `gym env start` exited with code 1 during child startup conversational_tool_use_simulation: 0/3 passed in 90.6s ``` With only the import fixed (intermediate commit), the stack started but the judge failed to parse: ```text FAIL (78.7s): original conversation has no generation or verification failures: expected [], got ['verification_generation_error'] generation_error = 2 validation errors for Evaluation success Field required [type=missing, input_value={'properties': {'success'...tion', 'type': 'object'}, input_type=dict] ``` ### After the fix Gym `29d57f66c` (this branch before a rebase onto `977583d3d`; the one new main commit touches no file in this diff), same command: ```bash ./run.sh test conversational_tool_use_simulation --ref 29d57f66c --no-ai-log ``` ```text PASS 373.9s the canonical one-example conversation uses live roles and verifies session disposal and grading gates PASS 79.1s enable_llm_judge=false scores a complete conversation 1.0 without calling the judge PASS 49.8s complete_trajectory_combined_evaluation judges the whole conversation once with per-role verdicts conversational_tool_use_simulation: 3/3 passed in 502.8s ``` ### Validation - QA testcase `conversational_tool_use_simulation` (3 cases: canonical message-level judge, `enable_llm_judge=false`, `complete_trajectory_combined_evaluation`): 0/3 on main (stack startup), 3/3 on this branch. The canonical case also replays the recorded conversation and checks the step limit (`excessive_length`, not masked), `###TRANSFER###` with both scope labels, and rejection of malformed history (25 direct resources-server calls). - Judge-format probe against the QA endpoint, 6 calls each: prompt-only schema gave 5/6 valid replies, `text.format` gave 6/6. - After the rebase (head `a3a6ab1f4`), I reran `test_server_utils.py` (173), `gym env test` for the server (78), and the agent tests (36): all pass. - `tests/unit_tests/test_server_utils.py`: 173 passed. The new `test_model_header_imports_nested_harness_key` fails on main, and also fails on a catch-`ImportError` fix. - `gym env test --resources-server conversational_tool_use_simulation`: 78 passed. New judge structured-output tests cover both judge paths and a configured `text`. 8 mask assertions now check top-level `mask_sample`, and the example-rollout contract test checks the masking rule. - `responses_api_agents/conversational_tool_use/simulation/tests`: 36 passed. 4 mask assertions moved to `mask_sample`. - `tests/unit_tests` (Mac): same 43 pre-existing failures as main, plus the new passing test. In one full run, `test_slurm_script.py::…multi_node_chain_survives_symmetric_run_entrypoint` also failed (a `bash` subprocess with a 10 s timeout). It passes 3/3 alone on both trees and 226/226 as a file, and this diff does not touch it. - `pre-commit run --all-files`: passed, no changes - Rollout evidence: the QA case above runs live rollouts. The policy, simulator, and judge all use `nvidia/nvidia/nemotron-3.5-super-vl-preview` through `openai_model`. ### Compatibility - Consumers that read `instance_config.mask_sample` from this environment, such as older NeMo RL, no longer get a mask. They should read top-level `mask_sample`, like every other environment. - The judge request now includes `text.format`. A provider that rejects `json_schema` structured output can turn it off by setting `judge_responses_create_params.text.format.type=text`. - With a live judge, rows that it scores as user or tool failures are now masked. Before this PR they counted as reward-0 policy samples. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: qiaochuz <qiaochuz@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
jkyi-nvidia
added a commit
that referenced
this pull request
Oct 15, 2026
…_tool_use_{stage}
The four conversational tool-use agents were nested under
responses_api_agents/conversational_tool_use/, which Gym's flat
responses_api_agents/<name> layout does not expect. #4464 and #4568 added
framework workarounds for the nesting.
Move each stage to a top-level agent directory:
conversational_tool_use/domain_generation -> conversational_tool_use_domain_generation
conversational_tool_use/policy_tool_generation -> conversational_tool_use_policy_tool_generation
conversational_tool_use/scenario_generation -> conversational_tool_use_scenario_generation
conversational_tool_use/simulation -> conversational_tool_use_simulation
- Rename harness keys in agent configs and the resources-server config
(including allowed_agents), and update imports, hardcoded paths in
prepare.py, tests, scripts, and docs.
- Point each agent's requirements.txt at the repo root with ../../.
- Drop the orphaned parent __init__.py, requirements.txt, and index README
(the resources-server README already carries the same component table
and Prepare Assets section).
- Revert the nested-path workarounds in nemo_gym/server_utils.py and
nemo_gym/cli/setup_command.py, with their tests; nothing else is nested.
Metric names under the conversational_tool_use/ namespace are unchanged.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Junkeun Yi <jkyi@nvidia.com>
jkyi-nvidia
added a commit
that referenced
this pull request
Oct 16, 2026
…_tool_use_{stage}
The four conversational tool-use agents were nested under
responses_api_agents/conversational_tool_use/, which Gym's flat
responses_api_agents/<name> layout does not expect. #4464 and #4568 added
framework workarounds for the nesting.
Move each stage to a top-level agent directory:
conversational_tool_use/domain_generation -> conversational_tool_use_domain_generation
conversational_tool_use/policy_tool_generation -> conversational_tool_use_policy_tool_generation
conversational_tool_use/scenario_generation -> conversational_tool_use_scenario_generation
conversational_tool_use/simulation -> conversational_tool_use_simulation
- Rename harness keys in agent configs and the resources-server config
(including allowed_agents), and update imports, hardcoded paths in
prepare.py, tests, scripts, and docs.
- Point each agent's requirements.txt at the repo root with ../../.
- Drop the orphaned parent __init__.py, requirements.txt, and index README
(the resources-server README already carries the same component table
and Prepare Assets section).
- Revert the nested-path workarounds in nemo_gym/server_utils.py and
nemo_gym/cli/setup_command.py, with their tests; nothing else is nested.
Metric names under the conversational_tool_use/ namespace are unchanged.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Junkeun Yi <jkyi@nvidia.com>
This branch was previously deployed
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
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.
Servers three levels below the repo root (e.g. conversational_tool_use/simulation) failed the two-level
pyproject.tomlcheck, so their venvs installed nemo-gym from the package index instead of the local checkout.Checks one more level up. Unit test added.