Repository navigation
refactor(checkpoint): one name per checkpoint concept before release - #4997
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. |
|
🌿 Preview your docs: https://nvidia-preview-pr-4997-839e617babff.docs.buildwithfern.com/nemo/gym |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP — LGTM — no reliability concerns.
Reviewed all 30 changed files and traced the affected checkpoint declaration, nested model-config, boundary serialization, restored-state cleanup, saved-reply, and worker-message paths. No material defects found. The intentional unreleased API and checkpoint-format breaks are documented.
Validation was source-only, including inspection of the relevant regression tests; tests were not executed in this read-only review.
Note: formal approval is not available for this request, so this review is posted as a comment only.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
bc79678 to
8c40d1f
Compare
…eing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…eing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
#4997 renamed the resources participant's verify_mode to verify_step_type, so building the participant in test_checkpoint.py raised TypeError on the merge with main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com>
…rom discovery Gym main now has NVIDIA-NeMo/Gym#4957: every checkpoint participant reports its checkpoint mode, and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. The pin also brings NVIDIA-NeMo/Gym#4997's renames: the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. NVIDIA-NeMo/Gym#3909 lets checkpointing agents and resources servers run several workers, so the guide drops the num_workers: 1 requirement. Gym's dependencies are unchanged, so uv.lock is too. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rom discovery Gym main now has NVIDIA-NeMo/Gym#4957: every checkpoint participant reports its checkpoint mode, and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. The pin also brings NVIDIA-NeMo/Gym#4997's renames: the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. NVIDIA-NeMo/Gym#3909 lets checkpointing agents and resources servers run several workers, so the guide drops the num_workers: 1 requirement. Gym's dependencies are unchanged, so uv.lock is too. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…eing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…DIA-NeMo#4792) * fix(genrm_compare): keep a completed cohort through a checkpoint GenRM computes every member's reward at once, but each reward reaches its member in its own reply. A checkpoint could land after one member recorded its reward and before the others did. After a crash those others replay verification, but the member that already recorded its reward never does, so the restored server waited for a cohort that could not complete until cohort_collection_timeout_s failed it. genrm_compare is now an exported resources server. Each session still open on a completed cohort exports its member's reward, and a restore rebuilds the completed cohort, with its group-attempt watermark, so a replayed verification gets its saved reward without running the judge again. A session whose cohort has no reward yet exports an empty state, and its members rebuild the cohort as before. While a checkpoint is open, a finished evaluation holds its rewards until resume. Otherwise a member could record a reward after this server exported, leaving the same mixed cut inside Gym. ResourcesParticipant gains session_episode(), which an exported server whose state is keyed by something other than the session uses to find what each session owns. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * fix(genrm_compare): re-export a restored cohort before its members replay ResourcesParticipant.install() calls restore_session_states() before it binds each restored session to its next attempt, so GenRM could not record a restored member under its attempt. Its export looked members up only by attempt, so until a restored member replayed /verify its session exported an empty state. A second checkpoint in that window dropped the saved cohort again, and after another crash the members waited for the missing sibling until cohort_collection_timeout_s. A restore now also records each restored session's member by session ID, and the export falls back to it. Retiring a session and pruning its cohort drop the record. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * fix(genrm_compare): key members by session, keep judge failures, drop abandoned restored cohorts Review follow-ups on the cohort checkpoint export. Members are keyed by the resources session ID that verify now reads from the request, as workplace_assistant does. A restore installs the same session IDs, and a replay carries the same cookie, so one map serves a session before and after it replays. This removes the attempt-keyed map, the rollout-context lookup and ResourcesParticipant.session_episode(). Only members of cohorts with a group ID are recorded, so cohorts without one no longer leave entries behind. A judge failure reaches callers as a masked row they record like a reward. A cohort the judge failed now exports that failure, so a replay after a restore raises the same JudgeError and returns the same masked row, and the failure is held while a checkpoint is open, as rewards are. A failed restore, or a commit that leaves a restored episode out of scope, starts the rollout over from its input under the same group attempt. A restored member whose session is retired before it replays is now dropped, and a restored cohort left with no member is dropped with the attempt watermark its restore installed, so the group re-forms as it would in a fresh process. A member that replayed stays. Tests build on the real ResourcesParticipant and call verify with a request. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * test(genrm_compare): cover checkpoint end-of-life paths and the control plane Each cohort now keeps its member sessions, and the restored ones not yet replayed, so removing a cohort removes exactly its member records. Pruning no longer rebuilds the member map whenever it evicts a cohort. New tests cover the paths review found untested: member records ending with a pruned cohort, an outcome staying held when a checkpoint closes admission again while the outcome waits for the cohort lock (for rewards and for judge failures), restored sessions that disagree on a member, and a replay through the real HTTP checkpoint control plane after one and after two checkpoint-crash-restore cycles. The README describes how GenRM takes part in partial-rollout checkpoints. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * fix(genrm_compare): name the response digest scheme in saved cohort state A checkpoint saves each member's response digest, and a replay matches its response against it. If _response_digest ever serializes responses differently, digests saved by one version would refuse every exact replay on the other. Saved members now name the scheme, json-ascii-sha256, and a restore refuses state saved under a scheme it does not know instead of installing digests it cannot compare. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * fix(genrm_compare): keep a finished cohort an open member session still needs Pruning dropped finished cohorts by age and count whether or not a member still needed the outcome. A restore installs saved cohorts as ordinary tombstones, so a member replaying /verify could prune its own cohort first, then wait for a sibling that had already finished until the collection timeout. A live cohort could likewise be evicted before a checkpoint exported it for a member that had not recorded its outcome yet. A finished cohort with a member session the checkpoint participant still holds open is now kept past cohort_result_ttl_s and outside the max_terminal_cohorts count, together with its group's attempt watermark. It becomes an ordinary tombstone once those sessions close or are retired. Checkpoint tests now give the two members different rewards, so a member receiving its sibling's outcome fails them, and cover each member recovering its own outcome after two checkpoints. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * test(genrm_compare): follow the verify_step_type rename NVIDIA-NeMo#4997 renamed the resources participant's verify_mode to verify_step_type, so building the participant in test_checkpoint.py raised TypeError on the merge with main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com> * perf(genrm_compare): check open sessions only for eviction candidates The keep check asked the participant whether every member session of every finished cohort was open, on each grouped verification and under the registry lock. Check only cohorts past cohort_result_ttl_s and, in the count pass, the oldest cohorts until the excess is dropped. A needed cohort is still never dropped, and every retained cohort keeps its group's attempt watermark. At 4096 finished cohorts of 16 members, a prune with checkpointing on drops from about 10.8 ms to 2.1 ms, matching checkpointing off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Anish Mahishi <amahishi@nvidia.com> --------- Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…eing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com>
…rom discovery Gym main now has NVIDIA-NeMo/Gym#4957: every checkpoint participant reports its checkpoint mode, and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. The pin also brings NVIDIA-NeMo/Gym#4997's renames: the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. NVIDIA-NeMo/Gym#3909 lets checkpointing agents and resources servers run several workers, so the guide drops the num_workers: 1 requirement. Gym's dependencies are unchanged, so uv.lock is too. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rom discovery Gym main now has NVIDIA-NeMo/Gym#4957: every checkpoint participant reports its checkpoint mode, and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. The pin also brings NVIDIA-NeMo/Gym#4997's renames: the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. NVIDIA-NeMo/Gym#3909 lets checkpointing agents and resources servers run several workers, so the guide drops the num_workers: 1 requirement. Gym's dependencies are unchanged, so uv.lock is too. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pt Gym's checkpoint names Gym main reports every checkpoint participant's mode (NVIDIA-NeMo/Gym#4957) and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. Gym also settled its checkpoint names (NVIDIA-NeMo/Gym#4997): the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. Checkpointing agents and resources servers may run several workers since NVIDIA-NeMo/Gym#3909, so the guide drops the num_workers: 1 requirement. Main's Gym pin already includes all three. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pt Gym's checkpoint names Gym main reports every checkpoint participant's mode (NVIDIA-NeMo/Gym#4957) and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. Gym also settled its checkpoint names (NVIDIA-NeMo/Gym#4997): the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. Checkpointing agents and resources servers may run several workers since NVIDIA-NeMo/Gym#3909, so the guide drops the num_workers: 1 requirement. Main's Gym pin already includes all three. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pt Gym's checkpoint names Gym main reports every checkpoint participant's mode (NVIDIA-NeMo/Gym#4957) and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. Gym also settled its checkpoint names (NVIDIA-NeMo/Gym#4997): the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. Checkpointing agents and resources servers may run several workers since NVIDIA-NeMo/Gym#3909, so the guide drops the num_workers: 1 requirement. Main's Gym pin already includes all three. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pt Gym's checkpoint names Gym main reports every checkpoint participant's mode (NVIDIA-NeMo/Gym#4957) and discover() returns it. The restart-only check reads that mode instead of asking every participant for its status again, and no longer depends on an agent status field that never shipped. Gym also settled its checkpoint names (NVIDIA-NeMo/Gym#4997): the policy model's checkpoint_policy moves into a checkpoint block, restored_pending is unclaimed, and control error codes carry a checkpoint_ prefix. RL reads the codes from Gym's error classes; only test fixtures named them. Checkpointing agents and resources servers may run several workers since NVIDIA-NeMo/Gym#3909, so the guide drops the num_workers: 1 requirement. Main's Gym pin already includes all three. Signed-off-by: Anish Mahishi <amahishi@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eing state (#5087) * refactor(sessions): say owner for session routing and release for freeing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * docs(sessions): define ownership by the current session holder Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
…eing state (#5087) * refactor(sessions): say owner for session routing and release for freeing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * docs(sessions): define ownership by the current session holder Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
…eing state (#5087) * refactor(sessions): say owner for session routing and release for freeing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in #4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * docs(sessions): define ownership by the current session holder Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
…ease (NVIDIA-NeMo#5083) * refactor(sessions): one name per session lifecycle concept before release Give each resources session and agent session lifecycle concept one name across code, docstrings, error messages, tests, config descriptions and docs. - Split ResourcesSessionFence into ResourcesSessionLocks, which runs a session's seed and close one at a time, and ResourcesSessionFences, which replaces ClosedResourcesSessionRecords and refuses a seed after its close until the fence expires. - Call the fence's duration the close retry window everywhere, including close_retry_window_seconds in session routing. - Add AgentSessionConflictError for agent session 409 refusals. - Use episode session, episode seed, legacy run seed, saved reply, release, lost and terminal in place of typed, untyped, close receipt, tombstone, free and final. Nothing renamed has been released, so there are no aliases. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * fix(sessions): clarify seed names and unavailable-session errors Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> * test(sessions): match the unavailable-session message in OpenClaw and OpenCode tests The agent base now refuses a retried close or a seed after the fence expires with "Agent session is no longer available". Two sandbox tests still matched "expired". Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * refactor(sessions): say owner for session routing and release for freeing state (NVIDIA-NeMo#5087) * refactor(sessions): say owner for session routing and release for freeing state Session routing called the worker that holds a session a claimant and its record a claim, while the checkpoint stack uses claim for a worker taking a restored record (unclaimed, settled in NVIDIA-NeMo#4997). Routing already said owner in x-ng-session-owner and routes_sessions_to_owner, so it now says owner everywhere: SessionClaims becomes SessionOwners, claim files become owner files in session-owners, and claimed_session_routing becomes owned_session_routing. The checkpoint stack said a retire frees a session, while the session code says release. It now says release too, and on_session_freed becomes on_session_released. A few session tests still called an episode seed typed; they now use the episode session names. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * docs(sessions): define ownership by the current session holder Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com> * refactor(sessions): carry the session names to servers merged since Servers and routing code merged after this branch opened still used the earlier names: ResourcesSessionFence in browsecomp_advanced_harness and the shared SWE sandbox-session base, ClosedResourcesSessionRecords in agentic_if, officeenv, partner_if's SWE adapter and scale_swe, and claim, claimant and ROLLOUT_CLAIM_PREFIX in the rollout-stop routing. Move them onto ResourcesSessionLocks, ResourcesSessionFences and the owner names, and drop "typed", "Environment Server session", "retention window", "remembers the close" and "close receipt" from their prose, tests and docs. terminal_bench_4 keeps its own fence bookkeeping under fence names, and nemo_user_sim's closed-session map becomes fenced_resources_session_ids. Add a Sessions section to the key terminology page with each term and the names it replaces, and point AGENTS.md at it in one line. Export the new classes from nemo_gym.server_runtime and AgentSessionConflictError from nemo_gym.agents, and record the renamed names in the import-surface fixture in place of the removed ones. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * refactor(sessions): finish the name sweep and scope the glossary to the code Review follow-ups for the session names: - The glossary's Conflict row now names the shared session code that raises ResourcesSessionConflictError and AgentSessionConflictError, instead of promising that no session code raises a plain 409. Release also covers a worker releasing ownership of a session ID, Lost says how an agent refuses a cookie for a lost session, and the note on other features' senses lists token capture and launch fences, MCP token claims, the launch claim file, the checkpoint terminal step and SandboxSession.expire(). - The harness guide says saved reply, close retry window and fence instead of "retained close", "replayed" and "final", and points 409 refusals at AgentSessionConflictError. The nemo_user_sim README names session_ttl_seconds, the window its fences really last. - partner_if's SWE adapter keeps its fences in _adapter_fences, apart from the Scale-SWE _fences it inherits, as it did before the rename. - ResourcesSessionFences.get() tells callers whose record may be None to test membership, and the TODO about sessions that are never closed moves to close_resources_session. - Test names, locals, comments and READMEs drop the remaining "typed", "legacy seed", "close receipt", "frees" and "final 422" wording. - New tests pin AgentSessionConflictError as a 409 that is still a ValueError, a fence holding a None record refusing a late seed, and the Codex refusal of a sandbox cookie that names no agent session. The agent retry tests match the exact close and seed messages. Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> * refactor(sessions): carry the session names to swebench's agent-run seed NVIDIA-NeMo#4277 gave the swebench server an agent-run seed path that still used the removed ResourcesSessionFence API (_task_session_fence.closed_episode) and the old "legacy seed" and "typed session" wording. It now tests membership in the shared base's _task_session_fences and refuses with "An agent-run seed cannot replace an episode session", as r2e_gym and swe_gym do. Its test names and docstrings say episode seed and agent-run seed, and the refusal test checks the message. A pi_agent test comment merged since says "terminal 422". Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> --------- Signed-off-by: Ananth Subramaniam <ansubramania@nvidia.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com> Co-authored-by: yaoyu-33 <yaoyu.094@gmail.com>
What does this PR do?
Gives every partial-rollout checkpoint concept one name before the feature ships in a release, and moves the policy model's checkpoint settings into a nested
checkpoint:block.The checkpoint code grew across several PRs, and the same concept ended up with different names on different servers:
checkpoint_modeon resources servers,checkpoint_sessions_supported: boolon agents, andcheckpoint_boundaries: boolon environment servers.StepMode,verify_mode, "verify mode" and "step mode" in different places.restored_pendingin code and "unclaimed" in the design.checkpoint_policyandcheckpoint_generation_cuts.Nothing here has been released, so every rename is a straight rename, with no aliases. NeMo RL is the main consumer and changes in step; the list is below.
One term per concept. The names follow the partial-rollout checkpoint design's terms. Where the design and the code used two names for one thing, this PR keeps one:
checkpoint_mode:restart_only,stateless,exportedcheckpoint_sessions_supported,checkpoint_boundaries,ResourcesCheckpointModewaitorreplay;StepTypeStepMode,verify_mode, "verify mode", "step mode"unclaimed(), status keyunclaimedrestored_pendingcut_regenerate_reasoncut_restart_reasonanswered_from_saved_reply()replayed(),_completed_activation_replyDeclarations. Agents and environment servers now declare
checkpoint_mode, exactly as resources servers do. An agent that implements the session hooks declares"exported"; an environment server whoserunrecords boundaries declares"exported"; the default stays"restart_only". Agents and environment servers accept onlyrestart_onlyandexported, and a misspelled value fails at startup, as it already does on resources servers.Policy model config. A model server instance's checkpoint settings are now one block:
ModelCheckpointConfigforbids unknown fields, so a misspelled setting fails at startup instead of being ignored.The startup check that refuses a model server outside the checkpoint gate now names
checkpoint.policy.Wire names. The resources server's step type keeps the class variable
checkpoint_verify, the status keyverifyand thex-ng-checkpoint-verifyheader. The environment server's status reply now reportsunclaimedinstead ofrestored_pending. The step type the environment server and Simple Agent record in their boundaries is nowresources_verify_step_type, which changes their exported record schema.Test servers. The checkpoint end-to-end suite's slow-verify resources server read its step type from an environment variable. It is now two servers with constant declarations,
slow_verify_server(wait) andreplay_verify_server(replay), because a step type is a property of the code.Docs. The agent harness guide names
checkpoint_mode = "exported". The Hermes agent README says that itscheckpoints_enabledturns on Hermes' own checkpoints, which are unrelated to Gym's partial-rollout checkpoints.Error codes. A checkpoint refusal reaches callers of
/run,/seed_sessionand model calls as a plain{"error": {"code", "detail"}}body, so every checkpoint error code now carries thecheckpoint_prefix and says where it came from.stale_checkpointbecomescheckpoint_stale,invalid_checkpoint_statebecomescheckpoint_invalid_state, and every other code gains the prefix:checkpoint_invalid_phase,checkpoint_deadline_exceeded,checkpoint_stale_attempt,checkpoint_admission_closed,checkpoint_retire_incomplete,checkpoint_export_failed,checkpoint_restart_in_scope,checkpoint_rollout_id_required,checkpoint_invalid_rollout_id,checkpoint_unauthorizedandcheckpoint_activation_out_of_order.checkpoint_conflict,checkpoint_errorandcheckpoint_coordinator_unavailablealready had it.Marking. A participant marks a retired attempt, and only the capture ledger and the complete-record store keep a fence, so the resources participant's
fence_retiredbecomesmark_retired_sessions.Unchanged by decision: the participant phases,
mark_retiredandRetiredAttempts(retire marks an attempt, and forget ends the mark),PrepareResult.continued, thelegacy_runnames for an agent's own/run, and rollout re-verification'sReverifyMode.Issue
No issue. The names were settled while reviewing #4957, which adds a status field that NeMo RL reads; settling them first keeps that field and NeMo RL's setup check from shipping against names that would change.
NeMo RL changes
NVIDIA-NeMo/RL#4500 (head
8ddf203f30):nemo_rl/environments/nemo_gym.py:policy_overrides["checkpoint_policy"] = Truebecomespolicy_overrides["checkpoint"] = {"policy": True}.tests/unit/environments/test_nemo_gym_utils.py: the expectedvllm_modelconfig becomes{"uses_reasoning_parser": False, "checkpoint": {"policy": True}}, and the lookup of["checkpoint_policy"]reads["checkpoint"]["policy"].tests/unit/environments/test_gym_checkpoint_adapter.py:restored_agent.restored_pending()becomesrestored_agent.unclaimed().tests/unit/experience/test_failures.py: the refusal bodies' codes becomecheckpoint_admission_closedandcheckpoint_stale_attempt.The code that recognizes a refusal compares against
AdmissionClosedError.code, so it follows the new value without a change.NVIDIA-NeMo/RL#4506 (head
039c525985):nemo_rl/environments/nemo_gym.py:policy_overrides["checkpoint_generation_cuts"] = Truebecomespolicy_overrides["checkpoint"]["generation_cuts"] = True, merged into the same block aspolicy.tests/unit/environments/test_nemo_gym_utils.py: the assertions onmodel["checkpoint_policy"]andmodel["checkpoint_generation_cuts"]readmodel["checkpoint"]["policy"]andmodel["checkpoint"]["generation_cuts"].Rollout collection (#3893), Gym's own controller, compares against
AdmissionClosedError.code, so its code follows without a change.Its collection test that expects a refusal with
admission_closedexpectscheckpoint_admission_closedinstead, and its docstrings say the new code.Open PRs that rebase onto this
#3909 (rebuilt on this branch), #4957, #4792, the sandbox stack (#4220, #4221, #4222, #4274, #4576), and the drafts #3897, #3898, #4451 and #4778. Each needs only the renames above.
Validation
pytest -n 16 tests/unit_tests. Every checkpoint, environment server, model server and token-capture test passes. The failures that remain are in sandbox, reward-profile, benchmark-listing and dataset-orchestrator tests that need optional packages or services absent from the local environment; the sandbox, reward-profile and benchmark-listing ones fail the same way onmain.responses_api_agents/simple_agent,responses_api_agents/tool_simulation_agent,environment_servers/single_agent_turn,resources_servers/blackjack.NEMO_GYM_CHECKPOINT_E2E=1 pytest tests/e2e/checkpoint -k "not real_model".pre-commit run --fileson the changed files.Rollouts
N/A: the change renames declarations, config fields and internal names and moves two config fields into a block. No rollout, verifier or checkpoint behavior changes. The checkpoint end-to-end suite, which checkpoints, kills and restores real Gym processes, covers the renamed paths.
Compatibility
checkpoint_name; callers that compare against an error class'scodefollow automatically.Checklist
pre-commit run --fileson the changed files).git commit -s).