Skip to content

claude code token capture - #4259

Open
ertkonuk wants to merge 2 commits into
mainfrom
tkonuk/claude-code-token-capture
Open

ertkonuk wants to merge 2 commits into
mainfrom
tkonuk/claude-code-token-capture

Conversation

@ertkonuk

@ertkonuk ertkonuk commented Oct 8, 2026

Copy link
Copy Markdown

What does this PR do?

Validation

Rollouts

Compatibility and benchmark impact

Checklist

  • I have read the contributing guidelines.
  • The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
  • Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
  • Documentation added or updated, or N/A with justification.
  • Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format checks pass).
  • New source files include the required Apache-2.0 SPDX header.
  • All commits have DCO sign-off (git commit -s) (so the DCO check passes).

sdevare-nv and others added 2 commits October 8, 2026 11:25
…tion, not content

conversation_digest hashed every request item including the harness's
system prompt. OpenCode regenerates that prompt per request with today's
date, so the first model call after 00:00 UTC in any running session no
longer matched its parent's recorded context: resolve_node returned
UNRESOLVED (no_match), the call was poisoned, and the finalizer rejected
the whole rollout. In a run whose step spanned 2026-09-24 00:00 UTC, 453 of
512 rollouts (every rollout that made calls on both sides of midnight)
became placeholder rows; the healthy rollouts were exactly those that ran
entirely before or entirely after midnight.

system/developer items now contribute only their role and position to the
digest. User, assistant and tool turns are hashed as before, so a
summarized or rewritten conversation still fails verification. Reusing the
parent's tokens then serves the instruction text the model actually saw,
which is what the training rows need. FINGERPRINT_VERSION bumps to 2.

[tkonuk: FINGERPRINT_VERSION 3 here, since #3382 already bumps it to 2]
Signed-off-by: Tugrul Konuk <tkonuk@nvidia.com>
Claude Code echoes a served tool call after normalizing it: it drops a
redundant `cd <cwd> &&` from Bash commands and maps legacy tool names
(Task to Agent). The assistant fingerprint hashed the call's name and
arguments, so the echo no longer matched the served call. That request
and every later one in the conversation recorded unresolved_parent, and
the fail-closed receipt masked the rollout. In an OpenCode + Claude Code
SWE run, nearly every Claude Code rollout was poisoned this way.

A tool call with a call id is now identified by that id alone
(FINGERPRINT_VERSION 4). The backend issues the id and the harness echoes
it verbatim; the name and arguments are what the harness executed. Calls
without an id keep name and arguments as identity. conversation_digest
still covers echoed arguments, so an earlier turn rewritten between two
requests still fails context verification, and token_in splicing still
trains the tokens the model produced.

A reply that adds no model-authored content (reasoning only) left its
request's fingerprint unchanged and was indexed under it, so every
request continuing the previous turn became ambiguous. Claude Code drops
such a reply and retries with a nudge. These calls are no longer lookup
candidates (their ledger rows carry no fingerprint), and the ledger now
verifies each candidate's context before counting candidates, as
RolloutLineage.resolve_node already did.

Replaying a recorded live Claude Code session (33 requests, main agent
and 4 subagents) through the resolver: 4 unresolved calls before, 0 after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Tugrul Konuk <tkonuk@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 8, 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.

@yaoyu-33 yaoyu-33 added area:training Training framework integrations and training-data interfaces bug Something isn't working complexity:high Cross-component or shared-contract change with a large review surface needs-review PR is ready for code review and waiting on a reviewer labels Oct 8, 2026
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Oct 9, 2026
@zyzhou5

zyzhou5 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@ertkonuk Could you rebase onto main to resolve the conflicts and add a short PR summary and validation notes?

@zyzhou5
zyzhou5 requested a review from pthombre October 9, 2026 19:40
@github-actions github-actions Bot added sla:review-overdue Review response is over the one-business-day SLA and removed sla:triage-overdue Review assignment is over the one-business-day SLA labels Oct 9, 2026
@zyzhou5 zyzhou5 added waiting-on-customer Waiting on the original author to respond and removed needs-review PR is ready for code review and waiting on a reviewer labels Oct 12, 2026
return continuation


def stamp_continuation(entry: TokenEntry, request_items: list[dict]) -> TokenEntry:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should stamp_continuation use _continuation_key too? the NemoRL TQ capture ledger paths (record, _record) skip reasoning-only replies now, but the Gym-native TokenSink path still indexes them it seems

This branch has not been deployed

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

Labels

area:training Training framework integrations and training-data interfaces bug Something isn't working complexity:high Cross-component or shared-contract change with a large review surface sla:review-overdue Review response is over the one-business-day SLA waiting-on-customer Waiting on the original author to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants