Repository navigation
refactor(models): give token capture its own lazy public API - #5525
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
November 3, 2026 19:16
ananthsub
force-pushed
the
ananthsub/token-capture-public-api
branch
2 times, most recently
from
November 3, 2026 19:44
24c180f to
a3e4e85
Compare
Token capture is a namespace of its own, like a task format, so code outside the library now imports its names from nemo_gym.models.token_capture instead of nemo_gym.models or a submodule. Frameworks implement its protocols, so every name it serves must import without Gym's server stack or an OpenAI client. - nemo_gym.models.token_capture serves a lazy _PUBLIC_API built with lazy_exports. It keeps every name its eager __all__ had and adds the rollout finalizer and retirement functions, the conformance check, and the protocol and record names nemo_gym.models served. Importing the package loads none of its modules, so the finalizer no longer needs to stay out of it. - It also exports RolloutRemovalPayload, which TokenSource and CaptureLedger return from retire and delete, and RolloutRetiredError, which a CaptureLedger raises for a retired rollout. A framework can implement those protocols without importing the protocols module. - nemo_gym.models.token_capture.staging, the framework hand-off, keeps a public API of its own, now lazy. The terminal resolver moves to token capture's API, where it is defined. The old token_id_capture paths for it and for canonicalize_tool_arguments resolve through moved-name entries. - The external capture handlers run inside the model server, and their module needs the OpenAI SDK through the chat completions converter. nemo_gym.models, the model server's public API, serves them instead of token capture's. - nemo_gym.models no longer re-exports token capture's protocol and record names. No release shipped them, so they need no moved-path entries. - The dependency check imported only the package, which now loads none of its modules. It now loads every name both token capture public APIs serve, and also fails on the OpenAI SDK, httpx, and Starlette. - The vLLM model server, its tests, the Claude Code agent tests, and the external agent harness guide's samples import from the new APIs. Code inside nemo_gym/ and its tests imports from the defining modules. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
ananthsub
force-pushed
the
ananthsub/token-capture-public-api
branch
from
November 3, 2026 19:53
a3e4e85 to
e5e832a
Compare
32 of 41 tasks
zyzhou5
added a commit
to zyzhou5/Gym
that referenced
this pull request
Nov 4, 2026
…IA-NeMo#5525 main's token-capture public API (NVIDIA-NeMo#5525) no longer serves CaptureStore from nemo_gym.models.token_capture; nemo_gym.models does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Zeyu Zhou <zezhou@nvidia.com>
adil-a
added a commit
that referenced
this pull request
Nov 7, 2026
Conflicts: - nemo_gym/models/token_capture/sink.py: main renamed rollout_id to capture_key in the unresolved-parent warning; kept the rename and this branch's reason and request-item count. - responses_api_agents/opencode_agent/configs/opencode_agent.yaml: kept main's new owner-extension keys and this branch's removal of the opencode_config.tools map. Follow-ups for main's API changes: tests build TokenEntry with capture_key, and the opencode_agent test imports the token-capture names from nemo_gym.models.token_capture, their public API since #5525. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Adil Asif <adasif@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 changed and why
Token capture is a namespace of its own, the way a task format such as
nemo_gym.data.tasks.harboris, so it now has its own lazy public API. Code outsidenemo_gym/imports token capture names fromnemo_gym.models.token_capture, not fromnemo_gym.modelsor from a token capture module. Before this change, no package served the rollout finalizer, so the external agent harness guide had to importnemo_gym.models.token_capture.deliverydirectly. The package layout docs (#5385) ran into that.nemo_gym.models.token_capturereplaces its eager imports and__all__with a_PUBLIC_APIbuilt withlazy_exports, plus aTYPE_CHECKINGblock, as in the Harbor package. It serves every name its old__all__listed, and the protocol and record names thatnemo_gym.modelsserved. It also addsfinalize_rollout_token_capture,retire_rollout_token_capture,ConformanceError,ConformanceWarning,TerminalAttribution, andresolve_terminal.RolloutRemovalPayloadandRolloutRetiredError.TokenSourceandCaptureLedgerreturn the first fromretireanddelete, and aCaptureLedgerraises the second for a retired rollout. Neither the old__all__nornemo_gym.modelsexported them, so a framework implementing those protocols had to import token capture'sprotocolsmodule.delivery.pyimports only token capture modules andnemo_gym.core.episode, which depends on Pydantic alone, so nothing keeps it out of the public API. The package docstring no longer says it needs Gym's server stack.ExternalCaptureHandler,VLLMWorkerCaptureHandler,make_external_capture_handler,validate_framework_context_options) run inside the model server. Their module needs the OpenAI SDK through the chat completions converter. Sonemo_gym.models, the model server's public API, serves them instead of token capture's.nemo_gym.models.token_capture.staging, the hand-off to training frameworks, keeps a public API of its own, now lazy too. Its docstring already describes it as a namespace separate from token capture's records. It also serves the three staging names thatnemo_gym.modelsserved and that its old__all__did not list.resolve_terminal,TerminalAttribution) is defined in token capture, so token capture's API serves it. A lazy API serves only its own package's modules. The old pathnemo_gym.token_id_capture.staging, which NeMo RL importsresolve_terminalfrom, still resolves through a new moved-name entry and warns.canonicalize_tool_arguments, which the oldnemo_gym.token_id_capturepackage imported without exporting it.nemo_gym.modelsno longer re-exports token capture's protocol and record names. No release shipped them: v0.7.0 has nonemo_gym.modelspackage, and the moves ship in 0.8.0. So they need no moved-path entries.nemo_gym/and its tests imports from the modules that define each name, so the library modules and tests that imported from the token capture packages now import from those modules.Tracking issue: #5166.
Compatibility
from nemo_gym.models import <token capture protocol or record name>no longer works. That form never shipped in a release.nemo_gym.models.token_captureor itsstagingpackage no longer imports their modules eagerly.nemo_gym.token_id_capturepaths keep resolving with a deprecation warning. That includesnemo_gym.token_id_capture.staging.resolve_terminal, which NeMo RL imports, andnemo_gym.token_id_capture.canonicalize_tool_arguments.scripts/migrate_imports.pyreads public APIs only from top-level packages, so it does not write the token capture or Harbor subpackage form. That predates this change.Validation
tests/unit_tests/test_public_api_facades.pynow also covers the two new public APIs. It checks that each name is the object its module defines, that importing either package loads none of its modules, and that theTYPE_CHECKINGimports match_PUBLIC_API. It also checks that code outside the library imports these names only from a public API, and that every name it imports resolves.tests/unit_tests/models/test_token_capture.pyloads every name both public APIs serve in a fresh interpreter. It fails if Ray, FastAPI, Starlette, uvicorn, aiohttp, requests, httpx, the OpenAI SDK, or torch was imported, so an external capture handler added back to token capture's API fails it. Before this change the check imported only the package, which now loads none of its modules.tests/unit_tests/test_moved_paths.pychecks that every recorded old path and NeMo RL's imports still resolve.pytest tests/unit_tests, compared withupstream/mainin the same environment, to check that the change adds no failures.pytest responses_api_models/vllm_model/tests responses_api_agents/claude_code_agent/tests, which run the vLLM model server and the Claude Code agent against the new imports.lint-imports, which checks that each package imports only packages below it.pre-commit run --files <changed files>.Rollout evidence
N/A: this changes import paths only, with no change to agent, model server, or verifier behavior.