Skip to content

fix(nemo-gym): refresh final-token route across turns - #3485

Open
zpqiu wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
zpqiu:agent/fix-multiturn-final-token-route
Open

fix(nemo-gym): refresh final-token route across turns#3485
zpqiu wants to merge 2 commits into
NVIDIA-NeMo:mainfrom
zpqiu:agent/fix-multiturn-final-token-route

Conversation

@zpqiu

@zpqiu zpqiu commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Refresh the previous assistant turn final routed-expert row from the next turn prompt prefill.
  • Extend the existing router-replay slicing test with a two-turn regression that replaces the prior dummy route while preserving normal token slicing.

Why

The final generated token in a request is not fed back through the model, so router replay pads its route with a valid dummy row. On the next model request that token is part of the prompt and receives a real prefill route, but NeMo Gym previously sliced only the unseen prompt suffix and left the historical dummy route in the training trajectory. Replaying that dummy route can change hidden states and inflate rollout-versus-training logprob error.

The fix updates only the routed-expert metadata for the last already-seen token. It does not modify seen_token_ids or change token slicing.

Concrete two-turn failure

Assume one MoE layer with top-k=2.

The first request has:

prompt tokens:     [1, 2]
generated tokens: [3]

routes:
token 1 -> [10, 11]  (real prompt-prefill route)
token 2 -> [20, 21]  (real prompt-prefill route)
token 3 -> [ 0,  1]  (dummy padding)

Token 3 is sampled from the logits produced while processing token 2. Because the request ends immediately afterward, token 3 is never itself processed by the model in this request, so no real route exists yet.

After a tool result adds tokens [4, 5], the second request sends the full history:

prompt tokens:     [1, 2, 3, 4, 5]
generated tokens: [6, 7]

second-request prefill route:
token 3 -> [30, 31]  (now real)

At this point, seen_token_ids == [1, 2, 3]. The existing suffix slicing correctly starts at index 3 and adds only the new prompt tokens [4, 5], but that also means token 3 is not appended again. Before this fix, its already-stored route was never updated:

before: training replay keeps token 3 -> [0, 1]   (wrong dummy route)
after:  previous assistant token 3 -> [30, 31]   (real prefill route)

This is not a tokenization or sequence-offset change. The token history remains [1, 2, 3, 4, 5, 6, 7]; only the route metadata for the previous turn final token is refreshed from:

routed_experts[len(seen_token_ids) - 1]

The stale dummy route is harmful even though it belongs to a historical token: R3 forces training replay through those dummy experts, changing that token's hidden state and therefore the logprobs of later tokens that attend to it.

Signed-off-by: alexchiu <7390474+zpqiu@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown

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.

@zpqiu

zpqiu commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test e3ba8c2

Signed-off-by: alexchiu <7390474+zpqiu@users.noreply.github.com>
@zpqiu

zpqiu commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 09a2e40

@zpqiu
zpqiu marked this pull request as ready for review August 4, 2026 06:29
@zpqiu
zpqiu requested review from a team as code owners August 4, 2026 06:29
@zpqiu zpqiu added the CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) label Aug 4, 2026
@zpqiu

zpqiu commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 09a2e40

@zpqiu
zpqiu requested review from ZhiyuLi-Nvidia and zyzhou5 August 4, 2026 06:33
@zpqiu zpqiu added CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) and removed CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version) labels Aug 4, 2026
@zpqiu

zpqiu commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 09a2e40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant