Repository navigation
fix(claude-code): count a run's token usage once - #5158
Conversation
parse_stream_json added the result event's usage, which is already the run's cumulative total, to the usage of every assistant event, and stream-json repeats a message's usage on each of its content-block events. A Terminal-Bench-Science trial (Claude Code 2.1.207) reported 44.7M input tokens for a run whose result event says 15.6M. The result event's usage is now the total. Without one (the CLI was killed or timed out), each assistant message counts once, with its latest usage; subagent messages still count, as before. Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
Signed-off-by: gwarmstrong <gwarmstrong@users.noreply.github.com>
|
/ok to test 85fe581 |
|
🌿 Preview your docs: https://nvidia-preview-pr-5158-85fe58102c99.docs.buildwithfern.com/nemo/gym |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
SHIP WITH CARE
No implementation defect found in the result-total selection, interrupted-run deduplication, or downstream response construction. Both added tests provide concrete regression oracles.
RISK — Document the required real-smoke validation
The PR lists unit tests and marks rollouts N/A. However, the corrected counters are published as runtime response.usage by _response_from_output. The protected AGENTS.md requires representative real smoke rollouts for behavior-changing agent work; its metadata exemption covers catalog or manifest changes, not runtime accounting. The reported Terminal-Bench-Science discrepancy establishes the original problem but does not establish validation of the correction at this head.
Correction: Record a representative real-model smoke rollout using this head, inspecting the native stream, resulting Gym response, and verifier outcome. No implementation patch is indicated by this finding.
Preservation and acceptance: For a completed run, demonstrate that response input/output counts equal the native result counters and that total_tokens equals their sum, without adding assistant usage again. Confirm that the same stream retains its assistant text and tool-result sequence and that verifier behavior remains appropriate for the task. Retain the assertions (60, 9) and (90, 3) in the completed-run and interrupted-run regressions, respectively.
Review completed by static inspection; tests and rollouts were not executed here.
Review generated by codex. This was a full review. Request a full review with /review reset=true.
|
@gwarmstrong Thanks! To address the autobot you may consider temporarily adding claude code to the conformance checks: https://github.com/NVIDIA-NeMo/Gym/blob/main/scripts/harness_conformance/registry.py - this will exercise the harness under specific smoke test scenarios without starting a real run. You don't need to merge that specific change. |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40736195282 |
|
Smoke on this head (85fe581, in a build on main c155b77) with Claude Code 2.1.207 against a self-hosted DeepSeek V4.1 Flash served by vLLM: two Terminal-Bench-Science trials that ran to a 30 minute cap and were graded, plus a short local session that ends on its own so its stream has a The double count is gone: on the completed session's stream the old parser gives 110,447 input tokens, against a real 66,169. Two gaps remain. The |
|
Follow-up to the smoke note above: the fix is in #5227. This PR's change stays: it removed a real double count, since stream-json repeats a message's usage on every content block. What this PR got wrong, measured against a recording proxy between Claude Code 2.1.207 and a self-hosted DeepSeek V4.1 Flash:
The tests here used hand-written events with no cache fields and with output in the per-message usage, so they encoded the same assumptions. The follow-up does three things:
Its tests are built from real sessions, with expected values computed from what the server sent. |
What does this PR do?
parse_stream_jsonadded theresultevent's usage, which is already the run's cumulative total, to the usage of every assistant event, and stream-json repeats a message's usage on each of its content-block events. A Terminal-Bench-Science trial (Claude Code 2.1.207) reported 44.7M input tokens for a run whoseresultevent says 15.6M.The
resultevent's usage is now the total. Without one (the CLI was killed or timed out), each assistant message counts once, with its latest usage; subagent messages still count, as before.No issue: found in Terminal-Bench-Science runs; the numbers are above.
Validation
claude_code_agent: 163 passed, including two new cases: a run with aresultevent, and a killed run without one whose content-block events repeat each message's usage.pre-commitclean.Rollouts
N/A (usage accounting).
Compatibility and benchmark impact
Reported token usage for Claude Code runs drops to the real totals; rewards are unchanged.