feat(api): warn when a parent's token roll-up exceeds its own leaves - #5718
Open
mmabrouk wants to merge 1 commit into
Open
feat(api): warn when a parent's token roll-up exceeds its own leaves#5718mmabrouk 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
|
A producer that repeats a child's token count on the parent gets counted twice, and nothing said so. The agent runner did exactly that, and the resulting doubled totals reached analytics and evaluations, because `ag.metrics.tokens.cumulative.total` is a registered analytics metric. The runner is fixed separately; this makes the next occurrence loud instead of silent. After the roll-up, walk the forest once more and compare each parent's cumulative token total against the sum of its subtree's leaves. Log the offending span ids when they disagree. One extra depth-first pass per batch. The check deliberately stays quiet when the leaves measured nothing. A span that carries only a run-level total, with no instrumented children, is a sanctioned shape and not a violation. This detects rather than corrects. Adding a parent's own incremental value to its children's cumulative is the right general definition, because a parent span can legitimately make its own model call, so silently suppressing it would be wrong for honest producers. An old runner image posting to a new API will still double count, and now it will name the span while doing so. Tests: 1,640 passing in OSS and 1,899 in EE. Nine new tests drive the real ingest sequence: the corrected runner shape rolls up to the true total, the old shape yields 3,496 for a real 1,748 and is flagged, a four-turn tree counts each turn once, a leaf-only tree is unchanged, the reported cost still reaches the agent span, and a run-level-only span is not a false positive. Claude-Session: https://claude.ai/code/session_01RkWWQUNNzRbaB5jnCAdjYA
mmabrouk
force-pushed
the
fix/cost-estimation-correctness
branch
from
August 3, 2026 22:19
9b06f03 to
d7c3adc
Compare
mmabrouk
force-pushed
the
fix/token-rollup-invariant
branch
from
August 3, 2026 22:19
cee277a to
394b0e1
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 #5714. Set the base to that branch, so this diff shows only its own change.
Why
A producer that repeats a child's token count on the parent gets counted twice, and nothing said so. The agent runner did exactly that, and the doubled totals reached analytics and evaluations, because
ag.metrics.tokens.cumulative.totalis a registered analytics metric.The runner is fixed separately. This makes the next occurrence loud instead of silent.
What it does
After the roll-up, walk the forest once more and compare each parent's cumulative token total against the sum of its subtree's leaves. Log the offending span ids when they disagree. One extra depth-first pass per batch.
The check stays quiet when the leaves measured nothing. A span carrying only a run-level total, with no instrumented children, is a sanctioned shape and not a violation.
This detects rather than corrects, deliberately
Adding a parent's own incremental value to its children's cumulative is the right general definition, because a parent span can legitimately make its own model call. Silently suppressing it would give the wrong answer for honest producers.
So an old runner image posting to a new API still double counts. Now it names the span while doing so.
Verification
1,640 passing in OSS unit tests, 1,899 in EE.
Nine new tests drive the real ingest sequence: the corrected runner shape rolls up to the true total; the old shape yields 3,496 for a real 1,748 and is flagged; a four-turn tree counts each turn once; a leaf-only tree is unchanged; the reported cost still reaches the agent span; a run-level-only span is not a false positive; and the warning is actually logged.