fix(sdk): stop discarding a cost that arrives without a token count - #5721
Open
mmabrouk wants to merge 1 commit into
Open
fix(sdk): stop discarding a cost that arrives without a token count#5721mmabrouk wants to merge 1 commit into
mmabrouk wants to merge 1 commit into
Conversation
dosubot
Bot
added
the
size:M
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Railway Preview Environment
|
`record_usage` returned as soon as the token total was falsy, and it wrote the cost
only after that check. So a run that reports a cost but no trustworthy token split
lost the cost entirely.
That combination is deliberate upstream, not an edge case. The runner returns exactly
`{total: 0, cost: ...}` to say "I have a billed figure but no split I trust", and it
has a test pinning that shape. The SDK then threw the whole record away, so the cost
lived on the runner's own spans and never reached the workflow root. Trace-focused
analytics read root spans only, and the roll-up works per OTLP request, so nothing
downstream could recover it.
Now the early return fires only when neither a token count nor a cost is present, and
the cost is written independently of the token total.
Missing and measured zero stay distinct, and the two are drawn differently on purpose.
A cost is decided by presence, so a reported 0.0 is stamped as a real measurement, a
free model or a fully cached turn, while an absent cost stamps nothing. That matches
the API's own convention, which tests for the attribute rather than its value. A token
total of zero is read as absence, because it is the runner's own sentinel for "no
trustworthy split" and no model call spends zero tokens; writing zeros there would
assert a run that consumed nothing and would poison the token roll-up. Inside a
reported split, a zero input or output is written as the measurement it is, since a
split can honestly be one sided.
The body also moved inside the existing error handling. The old code read the token
total before the guard, so a malformed record raised into the caller instead of being
logged.
Tests: 2,396 passing in the SDK suite, and 100 passing in the service's own agent
tests, which drive this recorder seam. Six new tests cover a cost-only record, a
tokens-only record, records with neither, a reported zero cost against an absent one,
a one-sided split, and a malformed record.
Claude-Session: https://claude.ai/code/session_01RkWWQUNNzRbaB5jnCAdjYA
mmabrouk
force-pushed
the
fix/sdk-cost-only-usage
branch
from
August 3, 2026 22:28
1a5b90a to
345667a
Compare
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
1 participant
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.
Stacked on #5708. Set the base to that branch, so this diff shows only its own change.
The symptom
A run that reports a cost but no trustworthy token split loses the cost entirely on the workflow root span.
That combination is deliberate upstream, not an edge case. The runner returns exactly
{total: 0, cost: ...}to say "I have a billed figure but no split I trust", and it has a test pinning that shape (services/runner/tests/unit/sandbox-agent-usage.test.ts:72-76).record_usagereturned as soon as the token total was falsy, and wrote the cost only after that check. So the cost lived on the runner's own spans and never reached the root. Trace-focused analytics read root spans only, and the roll-up works per OTLP request while the workflow root ships in its own request, so nothing downstream could recover it.The change
The early return now fires only when neither a token count nor a cost is present, and the cost is written independently of the token total.
Missing versus measured zero, drawn deliberately
The two are decided differently, and the reason matters.
Cost is decided by presence. A reported
0.0is stamped as a real measurement, because a free model or a fully cached turn genuinely costs zero. An absent cost stamps nothing. This matches the API's own convention, which tests for the attribute rather than its value.A token total of zero is read as absence. It is the runner's own sentinel for "no trustworthy split", and no model call spends zero tokens. Writing zeros would assert a run that consumed nothing and would poison the token roll-up.
Inside a reported split, a zero input or output is written as the measurement it is, because a split can honestly be one-sided, and the runner has a test for exactly that.
One incidental hardening
The body moved inside the existing error handling. The old code read the token total before the guard, so a malformed record raised into the caller instead of being logged. This function is documented as best effort and must never raise.
Verification
Note for the reviewer
The runner's ACP path still writes
input_tokens = 0on a cost-only run, which is the opposite of what the root span now does. That is pre-existing and out of scope here, but the two should be made to agree.