Repository navigation
refactor(agents): move the agent server modules into nemo_gym/agents - #5228
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
ananthsub
marked this pull request as ready for review
October 28, 2026 21:36
32 of 41 tasks
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5228-aa13c06419b7.docs.buildwithfern.com/nemo/gym |
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 28, 2026 21:56
7632984 to
f42bf49
Compare
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 28, 2026 22:26
f42bf49 to
431b22a
Compare
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 29, 2026 14:19
1f9f755 to
94444d8
Compare
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 29, 2026 15:14
94444d8 to
ed65bc8
Compare
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 29, 2026 15:51
ed65bc8 to
b9b56aa
Compare
ananthsub
force-pushed
the
ananthsub/layout-agents
branch
from
October 29, 2026 17:01
b9b56aa to
335b7a1
Compare
yaoyu-33
reviewed
Nov 2, 2026
yaoyu-33
left a comment
Contributor
There was a problem hiding this comment.
Thanks, this one is clean. Verdict: ship. There are three small NOTEs inline and no blockers or risks.
What we checked at 5a77563ff2de
- The move is mechanical.
- All 19 renamed files are identical apart from their import lines, and both
.shscripts are byte-identical. - Across all 169 changed Python files, the AST differs only in imports and in the intended strings: lazy exports,
MOVED_MODULES, scaffold templates, patch targets and logger names. - All 633 rewritten import bindings resolve to the same objects as before.
- The 202 moved test IDs are unchanged.
- All 19 renamed files are identical apart from their import lines, and both
- Old import paths keep working.
- Every old path returns the same module with one DeprecationWarning. That includes each
nemo_gym.agent_utils.*submodule andnemo_gym.sandbox.agent_tools, also when imported asfrom nemo_gym.sandbox import agent_tools. mock.patchthrough an old path still reaches the running code. Because these are whole-module moves, no patch target stops taking effect.- A base-tree pickle of
AgentOutcome/SandboxCommandloads at head.
- Every old path returns the same module with one DeprecationWarning. That includes each
- Public API, packaging and CI wiring.
nemo_gym.agents._PUBLIC_API(42 names) matches itsTYPE_CHECKINGblock, and each name is used by non-test code outside the library.lint-importsis KEPT.fern/docs.ymlredirects all 11 moved pages in both forms.- The session-routing CI filter is updated.
- The wheel ships
harness_runner.shandprocess_supervisor.shunderagents/utils. - The runners that execute inside sandboxes import nothing this PR moves.
- Tests pass.
- The unit suite passes, and so do the server suites of every agent and environment server touched here, locally and in CI.
- With this PR's moved-path DeprecationWarnings turned into errors, no test imports an old path.
- CI.
- The one red required job,
Server suite (shard 5), fails only onresponses_api_models/vllm_modeltest_cancelled_chat_keepalive_retains_uncertain_capture[False]. This PR does not touch that server, and the test passes 5/5 locally and on #5225 and #5229. kilocode conformanceis the known non-blockingmodel_errorprobe.
- The one red required job,
Stack-wide items rooted in other PRs (not for this PR)
- #5167,
scripts/migrate_imports.py- Text imports are rewritten to the package API only when they fit on one line, so multi-line samples in
.md/.mdxkeep the submodule path. Thepatterns.mdnote inline comes from this. - Earlier leftovers of the same kind: the server_runtime samples at
patterns.md:24/56and several fern tutorial pages from #5214 (e.g.single-step-environment.mdx:192).
- Text imports are rewritten to the package API only when they fit on one line, so multi-line samples in
- #5189,
AGENTS.md:194-197- The rule says server tests import from
nemo_gym.<package>, and that a missing name gets added to_PUBLIC_API. - Server tests here import private names from submodules (
_AgentSessionRecord,parse_cleanup_receipt,AGENT_SESSION_COOKIE_KEY, ...), and the facade test only enforces exported names. - The rule could say that server tests may import non-exported names from the defining module.
- The rule says server tests import from
- #5224 / #5225,
nemo_gym/models/__init__.py- Two entries name modules that only re-export them:
CONTEXT_LENGTH_EXCEEDED_ERROR_CODE(:89) points atbase_responses_api_model. It is defined inmodels/token_capture/records.py.MAX_NUM_TRIES(:95) points atopenai_utils. It is defined inclient/http_client.py.
- The root
_LAZY_EXPORTShas a defining-module check, but no package facade test does. A check with an allowlist for intentional re-exports such asBodywould catch these.
- Two entries name modules that only re-export them:
yaoyu-33
previously approved these changes
Nov 2, 2026
yaoyu-33
left a comment
Contributor
There was a problem hiding this comment.
Approving at 5a77563ff2de. The team review found no blockers or risks; the remaining inline notes are optional and can be handled here or in a follow-up.
base_responses_api_agent.py, agent_outcome.py, agent_activation_types.py and skills.py move as they are, sandbox/agent_tools.py becomes agents/agent_tools.py, and the agent_utils package becomes agents/utils. Record the six moves in MOVED_MODULES (agent_utils as a package), rewrite in-repo references with scripts/migrate_imports.py, point the lazy exports at agents.base_responses_api_agent, make the contract's agents layer the nemo_gym.agents package, and redirect the old library reference pages in fern/docs.yml. Regenerate the public API of each layout package that exists here from what non-test code outside nemo_gym imports, and point code outside nemo_gym, tests included, at nemo_gym.<package>. Signed-off-by: Ananth Subramaniam <ansubramania@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.
What and why
Moves what agent servers share into a new
nemo_gym/agents/package: the agent server base, agent outcomes and activations, sandbox sessions and the tools agents call in sandboxes, and skills.Where this PR sits
The stack lands bottom-up; each PR's base is the branch of the PR before it, so each diff shows only that PR's change.
Where the package sits
An arrow means "imports"; each package imports only packages below it. Yellow: created by this PR. Green: created by an earlier PR in the stack. Grey: still to come.
What moves
Whole modules, unchanged:
nemo_gym/base_responses_api_agent.pynemo_gym/agents/base_responses_api_agent.pynemo_gym/agent_outcome.pynemo_gym/agents/agent_outcome.pynemo_gym/agent_activation_types.pynemo_gym/agents/agent_activation_types.pynemo_gym/skills.pynemo_gym/agents/skills.pynemo_gym/sandbox/agent_tools.pynemo_gym/agents/agent_tools.pynemo_gym/agent_utils/nemo_gym/agents/utils/How to review
One commit. Its changes:
MOVED_MODULESinnemo_gym/_moved_paths.py.scripts/migrate_imports.py.agentslayer becomes thenemo_gym.agentspackage.nemo_gym.sandboxpackage. Withagent_toolsgone, none of its submodules moves elsewhere, so its__init__.pyis now checked too.fern/docs.ymlredirects the old library reference pages to the new ones.agent_utilsis recorded as a package, so its submodules, such asnemo_gym.agent_utils.sandbox_session, resolve too. Its harness and supervisor shell scripts move with it; the agents upload them from the host by path, so no sandbox imports them by name.nemo_gym/__init__.pypoint atagents.base_responses_api_agent;nemo_gym.agents.utilsand theagents/utils/files in its heading, cross-reference, shared-module table and diagram;nemo_gym.agentsandnemo_gym.server_runtime, becausemigrate_importsrewrites only one-line text imports.tests/unit_tests/agents/;test_sandbox_agent_tools.pybecomestest_agent_tools.pywith its module.Public API
Every layout package that exists after this PR has a lazy public API in its
__init__.py: the names code outsidenemo_gym/imports from it (servers and their tests, benchmarks, scripts, and code samples in docs and skills), plus every name it already exported, each served from the submodule that defines it on first use. Code outsidenemo_gym/and its tests (servers and their tests, benchmarks, scripts, and docs) imports those names fromnemo_gym.<package>, so later moves inside a package leave it alone. Code insidenemo_gym/, and the library's tests undertests/, import from the module that defines each name, so the import-linter contract sees every dependency and a test reads what it patches. A patch target names the module the code under test looks the name up in.AGENTS.mdstates the rule.Validation
pytest -n 16 tests/unit_tests, also with-W errorfor the moved paths. The only failures are environment-specific and unrelated to the move: this machine's hostname resolves to127.0.1.1, which the sandbox host check refuses (the Harbor run summary tests and the benchmark listing test), and two Docker PTY tests.lint-imports.pre-commit runon the changed files.Rollout evidence: N/A. The modules move unchanged.
Compatibility
nemo_gym.agents.*. A logging config set onnemo_gymstill covers them.python scripts/migrate_imports.py <paths>.Issue
Part of #5166.