Skip to content

fix(agent-sessions): token, cost and call roll-up correctness - #1122

Merged
JeremyFunk merged 11 commits into
mainfrom
fix/agent-sessions-usage-netting
Sep 29, 2026
Merged

JeremyFunk merged 11 commits into
mainfrom
fix/agent-sessions-usage-netting

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Agent Sessions token, cost and call roll-ups double-counted in several framework shapes, and the sessions list and the session page netted usage by different rules. One commit per bug, each with its own regression test. Numbers below come from the replayed framework captures in the EU org (service docs-verify-<framework>).

#28 Vercel AI SDK session totals were exactly 2x (session page) — 595edb0

  • Symptom: every Vercel AI SDK session showed twice its tokens: a2 680 for two chat calls totalling 340, a1 3930 for 1965, b 7306 for 3653. The per-model table was correct.
  • Cause: with usage: true the SDK stamps ai.usage.outputTokenDetails.reasoningTokens="0" on the step span between invoke_agent and chat. It decodes to a reasoning bucket of 0, so spanTokenBuckets returned a zero total and countableUsageSpans (packages/agent-sessions/src/session-summary.ts:486) treated the step as a reporter. chargeToNearestReporter (:591) charged each chat to its step, which netted to nothing, and the agent span kept its whole roll-up.
  • Fix: a span whose buckets total zero is not a reporter. This was already the list SQL's rule (usageReportersExpr only admits Tokens > 0).
  • Test: session-summary.test.ts › "does not let a zero-usage step between the agent and its calls absorb them". It uses the a2 capture's figures, and returns 680 without the fix.

#7 / #34 List usage netted one level deep — 0808692, review follow-up 3efc7fa

  • Symptom: the list showed exactly twice the session page's tokens for Strands (a1 4712/262 vs 2356/131; b 6146/1246 vs 3073/623; g 444/152 vs 222/76) and for the Vercel AI SDK (a2 680 vs 340, before fix: use Schema.Class members in AlertDestinationCreateRequest #28 also doubled the page).
  • Cause: childClaimsExpr (packages/query-engine-integrations/src/ai/ai-span-columns.ts:121) charged each reporter's claim to its direct ParentSpanId. Some frameworks put a span between the agent and its calls: Strands execute_event_loop_cycle, the Vercel AI SDK step N, smolagents Step N. The claims were keyed on those spans, no reporter collected them, and the agent span kept its roll-up. The session page walks the whole ancestry.
  • Fix:
    • Each trace also collects two links maps (usageLinksExpr), one for tokens and one for cost. A span id maps to itself when it reported that measure, and to its parent otherwise.
    • The session level climbs 4 links from each reporter's parent (past up to three non-reporting spans; the deepest shape seen is two). It appends the nearest token-reporting ancestor as tuple element 12 and the nearest cost-reporting ancestor as element 13 (sessionReportersExpr).
    • childClaimsExpr enters each reporter twice: its tokens under one ancestor, its cost under the other. That matches the session page, which charges each measure to its own nearest reporter.
    • The "no reporting ancestor" test for unreported calls also reads elements 12 and 13.
    • The lambda captures the per-trace maps, so the cost scales with reporters × rows per trace, not per session.
  • Verification (the compiled SQL run through run_sql against real data):
    • EU replay: the list now matches the session page for every Strands, Vercel AI SDK and smolagents session (e.g. Strands a1 2356/131, Vercel a1 1965, b 3653).
    • One production-org week, netting every session (a cost sort's work): totals are the same as before (no deep chains there). Read time under the 2-thread / 512MB interactive profile went from ~220–370ms to ~360–440ms. The two climbs are the added cost. A default page nets only its 50 rows.
  • Test: text assertions in ai-span-columns.test.ts and ai-sessions.test.ts, and the refreshed __sql_baseline__/integrations.sql.
  • Limit:
    • A parent outside the index (a span without a vendor stamp) still stops the climb. Example: the Strands TypeScript execute_agent_loop_cycle under a custom service.name, so Strands TS stays 2x on the list (1136/128 vs 568/64).
    • Stamping those spans is the ingest's job (sibling batch B1, TS Strands fingerprinting).
  • Cumulative reporters — e3f37bb:
    • Agents that live across turns report the conversation so far.
      • smolagents run(reset=False) (capture cap_a_v1, session a1): run spans report 1067 / 2243 / 4857 / 7811 cumulatively over calls that sum to 7811. Both pages showed 15978.
      • A Strands Agent reused across requests (capture strands_user, scenario a): 18044 shown against 4541 billed.
    • A reporter that is not a model call (agent, workflow, anything not flagged IsLlmCall) now claims nothing of a measure once a reporter of that measure is charged to it.
    • Model-call wrappers keep their excess, so the missing call's usage still survives under generateText over doGenerate or under a gateway generation.
    • The same rule applies on both pages (keptClaim in session-summary.ts, if(r.6 = 0 AND charged > 0, 0, …) in nettedReportersExpr).
    • Verified with the compiled page query over literal index rows on ClickHouse 25.8: smolagents 7811, three Strands turns 1031. EU replay and production totals are unchanged, because no agent span there keeps an excess.
  • Index e2e — e92c9ee: ai-trace-index-materialization.clickhouse.e2e.test.ts now seeds a reused Strands agent over an event-loop span and a fully failed OpenRouter request, then runs the real page query. Expected: 595 tokens and 2 calls; 1 call.

#36 A failed OpenRouter request counted once per attempt — 41a3592

  • Symptom: trace:83d675eb59e436078591a758e28cb09a counted 2 LLM calls for one request. LLM Generation and provider attempt 1: OpenAI are both op chat, both Error, and neither has usage.
  • Cause: a model call that reported no usage counted unless an ancestor reported usage (countedLlmCalls in session-summary.ts:549, nettedReportersExpr in ai-span-columns.ts:152). A failed generation never reports usage.
  • Fix: a model call without usage does not count when its parent is a model call. On the list, the reporter ids column now holds every reporter (usage reporters and model calls): tupleElement(reporters, 1), one lambda fewer. The check is one has on the direct parent.
  • Verification: that trace now counts 1 on the list. Other EU sessions are unchanged (e.g. inv_d7aeabdd… 16). A production week counts 378 unreported calls both before and after.
  • Test: session-summary.test.ts › "counts a gateway request whose every attempt failed once, not once per attempt", plus the SQL text tests.

#10 (second half) Cost netted differently on the list and the session page — f133741

  • Symptom: nested agents that each stamp gen_ai.usage.cost summed to different session costs on the list and the page. The page subtracted a sub-agent's cost from the orchestrator's roll-up; the list did not.
  • Cause, part 1: a sub-agent sits under its tool span, and the list's one-level netting never reached the orchestrator. Some UI/UX feedbacks #7's commit fixes this.
  • Cause, part 2: costBySpan (session-summary.ts:621) took a span stamped with a zero cost (cost >= 0) as a reporter, while the list only charges Cost > 0. A zero-cost wrapper absorbed its calls' cost, which is fix: use Schema.Class members in AlertDestinationCreateRequest #28's shape for cost.
  • Fix: a zero cost is still recorded, so the session reads "free" and not "unmeasured", but claims are no longer charged to it.
  • Test: "nets a sub-agent's cost through its tool span, and past a zero-cost wrapper", which returns 0.005 for 0.004 without the fix. Also "reads a zero cost as free, not as unmeasured".
  • The replayed LiteLLM sessions already agree (docs-litellm-b $0.00372185 on both), because the guide now prices only the outermost agent.

#21 A tool call paused for a human counted twice (session page, partial) — 40e992c, review follow-ups eeaa350, 2ad793e

  • Symptom: Strands a1 showed 4 tool calls with delete_file ×2, for 3 calls. The interrupted span ends Ok with no result, and the resumed turn's trace opens a second span with the same gen_ai.tool.call.id (call_ltoPrLQIg3ZHWkyFnBIOE65u).

  • Cause: buildSessionSummary counted every tool span (work.toolCalls and toolUsage, session-summary.ts).

  • Fix: countedToolCalls drops a tool span only when all of these hold:

    • it recorded no result and did not fail;
    • a later span with the same call id carries a result.

    Two calls that share an id and both returned stay two (parallel lanes, providers that number calls per turn), and a session captured without payloads keeps every span.

  • Test:

    • "counts a call paused for a human and resumed once, as its resumed copy";
    • "keeps two calls that share an id when both returned, and every call captured without payloads".
  • Partial:

    • Covered: Strands, which records the resumed call's result on span attributes.
    • Not covered, still counted twice:
      • Google ADK: the outcome is only in gcp.vertex.agent.tool_response, and the paused copy's confirmation request reads as a result. The ADK guide's SkipDuplicateToolSpans processor already drops the paused copy for users who follow it.
      • Strands versions that record results in span events (strands_user: 3 → 5).
      • OpenAI Agents: its tool spans carry no call id.
    • Older Mastra double-exports a failed tool span under the same span id, and that span counts twice again. Current Mastra does not do this.
    • The narrow rule is deliberate: merging two real calls would be worse than counting a paused copy.
  • List not changed (follow-up):

    • ai_trace_index has no call-id column. Deduping on the list needs a ToolCallId index column, a migration recreating the materialized view (forward-only), and a manual Tinybird deploy.
    • Until then the list and the page disagree on a human-approval duplicate (Strands a1: list 4 tool calls, page 3).
    • This is the only shared call id among tool spans across the EU replay of every framework, and a production week has none.

Known limits

  • The list's links maps are built from at most 2,000 index rows per trace (groupArray(2000)), the same cap as the reporters and the session page's span read. Past it, a missing intermediate span stops the climb.
  • A parent outside the index (a span without a vendor stamp) stops the climb, as described under Some UI/UX feedbacks #7.

Notes

  • No migration, no index or DB change: every list change is on the read path and applies to existing rows.
  • gen-ai-columns.ts is not touched.
  • Checks and findings (session-checks.ts, session-findings.ts) are unchanged.

…ren's tokens

Symptom: Vercel AI SDK sessions showed exactly twice their tokens on the
session page (docs-verify-vercel-ai-sdk a2: 680 for two chat calls totalling
340; a1 3930 for 1965).

Cause: with `usage: true` the SDK stamps
`ai.usage.outputTokenDetails.reasoningTokens="0"` on the `step` span between
`invoke_agent` and `chat`. That decodes to a reasoning bucket of 0, so
`spanTokenBuckets` returned a zero total and `countableUsageSpans` treated the
step as a reporter. `chargeToNearestReporter` then charged each chat to its
step (netting to nothing) and the agent span kept its whole roll-up.

Fix: a span whose buckets total zero is not a reporter, which is already the
list SQL's rule (`usageReportersExpr` admits `Tokens > 0`).

Seen in: Vercel AI SDK 7 capture docs_vercel-ai-sdk_a.
Symptom: the sessions list showed exactly twice the tokens of the session
page for Strands (docs-verify-strands a1 4712/262 vs 2356/131, b 6146/1246
vs 3073/623) and for the Vercel AI SDK (a2 680 vs 340).

Cause: the list SQL charged each reporter to its direct parent
(`childClaimsExpr` keyed on ParentSpanId, ai-span-columns.ts:121). Strands
puts `execute_event_loop_cycle` between `invoke_agent` and `chat`, the
Vercel AI SDK a `step` span, smolagents a `Step N` chain; the claims were
keyed on those spans, no reporter picked them up, and the agent span kept
its full roll-up. The session page walks the whole ancestry.

Fix: each trace also collects its links (span id -> itself when it
reported usage, else its parent). The session level climbs up to 8 links
from each reporter's parent to the nearest ancestor that reported and
carries it as a twelfth tuple element, which the claims and the "any
ancestor reported" test for unreported calls key on. Verified against the
EU replay: list totals now equal the session page for every Strands, Vercel
AI SDK and smolagents session. A week of the production org nets to the
same totals as before at the same read time (~370ms either way).

Limit: a parent outside the index (a span without a vendor stamp, e.g. the
Strands TypeScript loop span under a custom service name) still stops the
climb.

Seen in: captures docs_strands_{a,b,g}, docs_vercel-ai-sdk_{a,b}.
…r attempt

Symptom: an OpenRouter Broadcast request whose every provider attempt failed
counted as 2 LLM calls on the list and the session page
(trace:83d675eb59e436078591a758e28cb09a: `LLM Generation` plus
`provider attempt 1: OpenAI`, both op `chat`, both Error, no usage).

Cause: a model call that reported no usage counted unless an ancestor
reported usage (session-summary.ts `countedLlmCalls`, ai-span-columns.ts
`nettedReportersExpr`). The attempt netting only worked when the generation
above it reported usage, which a failed request never does.

Fix: a model call that reported no usage does not count when its parent is
a model call. On the list the session's reporter ids now carry every
reporter (usage and model calls), so the check is one `has` on the direct
parent next to the existing one on the charged ancestor. Verified on the
EU replay (the trace now counts 1; other sessions unchanged) and on a week
of the production org (378 unreported calls counted before and after).

Seen in: OpenRouter Broadcast capture (openrouter guide, failed request).
…n page

Symptom: nested agents that each stamp `gen_ai.usage.cost` summed to
different session costs on the list and the session page (LiteLLM guide,
orchestrator delegating to workers through tool spans): the page subtracted
the sub-agents' cost from the orchestrator's roll-up, the list did not.

Cause: two rules differed. The list charged a claim to its direct parent,
which for a sub-agent is its tool span, so nothing was netted; that half is
fixed by the full-ancestry netting in the list commit before this one. The
page also took a span stamped with a zero cost as a cost reporter
(`costBySpan`, `cost >= 0`, session-summary.ts), where the list only
charges `Cost > 0`: a zero-cost wrapper absorbed its calls' cost and left
the agent above it keeping its whole roll-up, the #28 shape for cost.

Fix: a zero cost is recorded (so the session still reads "free", not
"unmeasured") but is not a reporter claims are charged to.

Seen in: LiteLLM capture docs_litellm_b (nested agents), reconstructed as a
fixture; the replayed session already agrees because the guide now prices
only the outermost agent.
Symptom: a Strands session with one human-approved `delete_file` call
showed it twice in the session page's tool ledger and tool-call count
(docs-verify-strands a1: 4 tool calls, `delete_file` x2, for 3 calls).

Cause: the interrupted call ends its `execute_tool` span Ok with no result,
and the resumed turn's trace opens a second span under the same
`gen_ai.tool.call.id` (call_ltoPrLQIg3ZHWkyFnBIOE65u, traces df191678… and
18645746…). `buildSessionSummary` counted every tool span
(session-summary.ts `work.toolCalls`, `toolUsage`).

Fix: `countedToolCalls` takes the session's tool spans with the ones
sharing a call id collapsed to the last to start (the resumed copy, which
carries the result); spans without an id count as before. Both the count
and the ledger read it.

The list's tool-call count is not changed: `ai_trace_index` carries no
call id, so deduping it there needs a new index column and a recreated
materialized view. Across the EU replay (every framework) this is the only
shared call id among tool spans, and a week of the production org has
none.

Seen in: capture docs_strands_a (HITL resume).
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 165abbd7-2b78-4a74-b6f6-6fa5d4efd1f7

📥 Commits

Reviewing files that changed from the base of the PR and between f8b99d8 and 2ad793e.

📒 Files selected for processing (8)
  • packages/agent-sessions/src/session-summary.test.ts
  • packages/agent-sessions/src/session-summary.ts
  • packages/backend/src/services/warehouse/ai-trace-index-materialization.clickhouse.e2e.test.ts
  • packages/query-engine-integrations/src/__sql_baseline__/integrations.sql
  • packages/query-engine-integrations/src/ai/ai-sessions.test.ts
  • packages/query-engine-integrations/src/ai/ai-sessions.ts
  • packages/query-engine-integrations/src/ai/ai-span-columns.test.ts
  • packages/query-engine-integrations/src/ai/ai-span-columns.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot

maple-review-bot Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Maple review

Confidence 3/5 · needs attention
The SQL and JS netting rules were checked against each other and look consistent; the page-only tool-call dedupe leaves the list counting a paused call twice.
quality 90/100 · 1 warning · tests covered · risk medium

Nets agent-session tokens, cost and calls so a rolled-up wrapper or a zero-usage step is no longer counted twice, on both the session page and the sessions list SQL, with a regression test per bug. The two rules now agree for usage; the list still counts a paused tool call twice.

  • countableUsageSpans and costBySpan stop treating an all-zero span as a reporter
  • countedLlmCalls drops a model call whose parent is a model call
  • usageLinksExpr climbs a trace's rows so a claim is netted past non-reporting spans
  • countedToolCalls collapses tool spans sharing gen_ai.tool.call.id on the page

Findings

Warning · F1 · countedToolCalls dedupes the page only, the list still counts a paused call twice

correctness · packages/agent-sessions/src/session-summary.ts:799-808

The list's tool-call figure is toolCalls: CH.sum($.IsToolCall) over ai_trace_index rows (packages/query-engine-integrations/src/ai/ai-sessions.ts:553, summed per session at :640), which still counts both the paused and the resumed copy of one gen_ai.tool.call.id; only the page now collapses them. On a Strands / OpenAI Agents / Mastra / ADK session paused for a human the list row shows one more tool call than the page's Tools ledger Calls, and toolCallsMin/toolCallsMax (:768) plus the toolCalls histogram bucket off the inflated number — the exact list-vs-page split this pull request sets out to remove.

Dedupe where the count is made rather than on one consumer: carry `gen_ai.tool.call.id` into `ai_trace_index` and count distinct ids per session (the way the reporters are netted one level up), so the row, its filters and the histogram read one number.
What was checked
  • Element 12 (childClaimsExpr key) reads the same nearest-reporting-ancestor rule as the page's chargeToNearestReporter, and reporterIds now matches the page's reportsUsage/isLlmCall(parent) t…
  • tokens.total is the sum of the five disjoint buckets (session-summary.ts:455), so the zero rule cannot drop a span that reported cache-only tokens
  • The dedupe keeps the last-started copy only when gen_ai.tool.call.id is non-empty (session-summary.ts:805), so unkeyed tool spans are never collapsed
Copy all findings (1)
Findings from an automated review of commit 40e992c5b62c44ebbb67ce70a1f0e2ef08291194. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Warning · correctness · packages/agent-sessions/src/session-summary.ts:799-808
`countedToolCalls` dedupes the page only, the list still counts a paused call twice
The list's tool-call figure is `toolCalls: CH.sum($.IsToolCall)` over `ai_trace_index` rows (`packages/query-engine-integrations/src/ai/ai-sessions.ts:553`, summed per session at :640), which still counts both the paused and the resumed copy of one `gen_ai.tool.call.id`; only the page now collapses them. On a Strands / OpenAI Agents / Mastra / ADK session paused for a human the list row shows one more tool call than the page's Tools ledger `Calls`, and `toolCallsMin`/`toolCallsMax` (:768) plus the `toolCalls` histogram bucket off the inflated number — the exact list-vs-page split this pull request sets out to remove.
Suggested fix: Dedupe where the count is made rather than on one consumer: carry `gen_ai.tool.call.id` into `ai_trace_index` and count distinct ids per session (the way the reporters are netted one level up), so the row, its filters and the histogram read one number.

40e992c · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

readonly Tokens: Expr<number>
readonly Cost: Expr<number>
}): Expr<unknown> {
const next = CH.if_($.Tokens.gt(0).or($.Cost.gt(0)), $.SpanId, $.ParentSpanId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Cost-only wrappers double session tokens

When a cost-only wrapper separates an agent from its call, usageLinksExpr stops the token claim at that wrapper. The agent keeps its token roll-up, so the list counts the call twice.

Learn more

The session list uses usageLinksExpr to find the nearest reporting ancestor before subtracting child claims. It treats either positive tokens or positive cost as a stop, then uses the same stop for both measures in childClaimsExpr. The detail summary instead charges tokens and cost separately in countableUsageSpans and costBySpan. A cost-only intermediate span blocks a token claim from reaching its token-reporting ancestor. The inverse, a token-only intermediate span, blocks cost netting.

Example: An agent reports 100 tokens, a child wrapper reports only $0.01, and its chat child reports 100 tokens and $0.01. The list retains the agent's 100 tokens plus the chat's 100, while the detail reports 100.

Recommended fix: Resolve token and cost reporting ancestors independently in usageLinksExpr and charge each measure to its own nearest reporter in childClaimsExpr. Keep zero-valued spans transparent, and add mixed-measure ancestry tests.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 3efc7fa: tokens and cost now climb their own links maps (tokenLinks, costLinks), each reporter carries both ancestors (elements 12/13), and the child-claims sumMap enters each reporter under its token ancestor with its tokens and under its cost ancestor with its cost. Same totals on the EU replay and a production week.

Comment on lines +804 to +806
const callId = span.genAi.toolCallId
if (callId === undefined || callId === "") unkeyed.push(span)
else byCallId.set(callId, span)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Parallel tool calls disappear from summary

When separate traces reuse a tool ID, countedToolCalls keeps only the later call. Parallel lanes then lose a tool event and undercount their work.

Learn more

Tool-call IDs do not uniquely identify invocations across a session. sessionToolResults already handles reused IDs by preferring a result from the same trace, and its tests use two different tool results for toolu_1 in two traces. The new map keys solely on the ID, so these independent calls become one; the paused/resumed case also spans traces, so simply including the trace ID in the key would undo the intended deduplication.

Example: Lane A in trace A runs run_sql with toolu_1 at 0 ms, and lane B in trace B runs run_sql with toolu_1 at 1,000 ms. The summary keeps only lane B, although both ran.

Recommended fix: Match interrupted and resumed spans only when there is evidence of the same invocation, such as an incomplete earlier call followed by a resumed result in the corresponding continuation. Preserve separate same-ID calls from concurrent or retried traces; test both cases together.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in eeaa350: only a tool span that recorded no result and did not fail is dropped, and only when a later span with the same call id carries a result (the human-approval pause shape). Two same-id calls that both returned are both counted, and a session captured without payloads keeps every span; test added for both.

const next = CH.if_($.Tokens.gt(0).or($.Cost.gt(0)), $.SpanId, $.ParentSpanId)
const link = CH.compileFnCall<unknown>("tuple", $.SpanId, next)
return CH.untypedExpr(
`CAST(groupArray(${MAX_USAGE_REPORTERS_PER_TRACE})(${compile(link.toFragment())}), 'Map(String, String)')`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Large traces lose token roll-up links

For traces exceeding 2,000 spans, usageLinksExpr can omit an intermediate span while retaining its descendant's usage claim. The missing link prevents ancestor netting, so the list double-counts the agent's tokens.

Learn more

The trace index groups reporting spans separately from the new map of all spans. usageReportersExpr can hold 2,000 selected reporters, while usageLinksExpr holds at most 2,000 of all index rows. The two aggregates have no ordering or shared sampling rule. When an intermediary between a retained reporter and its agent is missing from the map, sessionReportersExpr cannot resolve that ancestor. The resulting child claim never subtracts from the agent's roll-up.

Example: A trace has an agent claiming 100 tokens, 2,000 unrelated index spans, and a chat claiming the same 100 tokens beneath an intermediate step. If the map omits the step but the reporter array includes both claims, the list shows 200 tokens.

Recommended fix: Build a complete ancestry lookup for every retained reporter, or derive each reporter's nearest ancestor before imposing a cap. Include a test with more index rows than the cap and an intermediate zero-usage step.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Leaving this as is. 2,000 per trace is the same cap the reporters already carry (MAX_USAGE_REPORTERS_PER_TRACE = AI_SESSION_SPANS_MAX_SPANS), and the session page reads at most that many spans too, so past it the two pages already disagree by design (documented on the constant). The largest trace in a production week has 567 index rows.

@maple-review-bot maple-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 inline note from Maple's review. The score and summary are in the review comment above.

* (Strands, OpenAI Agents, Mastra, Google ADK) — so spans sharing an id are one
* call, the last to start standing for it. Spans without an id count as they are.
*/
function countedToolCalls(ordered: readonly AiSessionSpan[]): readonly AiSessionSpan[] {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

countedToolCalls dedupes the page only, the list still counts a paused call twice

F1 · Warning · correctness

The list's tool-call figure is toolCalls: CH.sum($.IsToolCall) over ai_trace_index rows (packages/query-engine-integrations/src/ai/ai-sessions.ts:553, summed per session at :640), which still counts both the paused and the resumed copy of one gen_ai.tool.call.id; only the page now collapses them. On a Strands / OpenAI Agents / Mastra / ADK session paused for a human the list row shows one more tool call than the page's Tools ledger Calls, and toolCallsMin/toolCallsMax (:768) plus the toolCalls histogram bucket off the inflated number — the exact list-vs-page split this pull request sets out to remove.

Dedupe where the count is made rather than on one consumer: carry `gen_ai.tool.call.id` into `ai_trace_index` and count distinct ids per session (the way the reporters are netted one level up), so the row, its filters and the histogram read one number.
Prompt for an AI agent
In `packages/agent-sessions/src/session-summary.ts:799-808`: `countedToolCalls` dedupes the page only, the list still counts a paused call twice.

The list's tool-call figure is `toolCalls: CH.sum($.IsToolCall)` over `ai_trace_index` rows (`packages/query-engine-integrations/src/ai/ai-sessions.ts:553`, summed per session at :640), which still counts both the paused and the resumed copy of one `gen_ai.tool.call.id`; only the page now collapses them. On a Strands / OpenAI Agents / Mastra / ADK session paused for a human the list row shows one more tool call than the page's Tools ledger `Calls`, and `toolCallsMin`/`toolCallsMax` (:768) plus the `toolCalls` histogram bucket off the inflated number — the exact list-vs-page split this pull request sets out to remove.

Suggested fix: Dedupe where the count is made rather than on one consumer: carry `gen_ai.tool.call.id` into `ai_trace_index` and count distinct ids per session (the way the reporters are netted one level up), so the row, its filters and the histogram read one number.

Verify the problem exists at that location before changing it, and keep the fix to those lines.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Known and stated in the PR body (#21 section): ai_trace_index has no call id, so the list needs a new index column and a recreated materialized view (plus a manual Tinybird deploy) to do the same. That is out of proportion for this batch: it is the only shared call id among tool spans across the EU replay of every framework, and a production week has none. Left as a follow-up.

…ge charges them

Review follow-up to the #7 list netting. The list climbed one chain for
both measures (nearest ancestor with tokens or cost), while the session page
charges tokens to the nearest token reporter and cost to the nearest cost
reporter. A wrapper that priced but did not count (or the reverse) between
an agent and its call stopped the other measure's claim, and the agent kept
its roll-up of it on the list only.

Each trace now carries two links maps (`tokenLinks`, `costLinks`); each
reporter carries both ancestors (elements 12 and 13), and the child-claims
sumMap enters each reporter twice, its tokens under one and its cost under
the other. The climb is 4 links (past up to three non-reporting spans; the
deepest shape seen is two) to keep the added map lookups down: a
production week netted in full reads ~360-440ms against ~220-370ms before
the netting change, same totals; EU replay totals unchanged.
Review follow-up to #21. Keying the tool calls on `gen_ai.tool.call.id`
alone merged any two calls sharing an id, and ids are not unique across a
session for every emitter (parallel lanes, providers that number calls per
turn). Only a span that recorded no result and did not fail is now dropped,
and only when a later span with the same id carries a result — the shape a
human-approval pause leaves. A session captured without payloads keeps
every span.
@maple-review-bot

maple-review-bot Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Maple review

Confidence 3/5 · needs attention
F1 stays open: the list SQL still counts a paused tool call twice while only the page dedupes it.
quality 90/100 · 1 warning · tests partial · risk medium

Nets agent-session usage the same way on the session page and the sessions list: zero-usage spans and zero-cost wrappers stop being reporters, and the list now climbs each measure to its nearest reporting ancestor. Tokens/cost numbers agree across both views, but the list still counts a paused tool call twice.

  • countableUsageSpans/costBySpan drop zero-token and zero-cost spans as reporters
  • sessionReportersExpr gains per-measure ancestor elements 12/13 from usageLinksExpr
  • countedToolCalls merges a resumed tool call by gen_ai.tool.call.id on the page
  • countedLlmCalls stops counting a gateway attempt whose parent is a model call

Still open from earlier reviews

What was checked
  • climb starts at r.2 (ParentSpanId), so the first hop lands on the parent, not the reporter itself (ai-span-columns.ts:152)
  • childClaimsExpr keys [c.12, c.13] pair with values [c.3, 0.]/[0., c.4], charging tokens and cost to their own ancestors (ai-span-columns.ts:172-174)
  • countedToolCalls keeps the recorded copy and a later paused copy but drops an earlier unrecorded one (session-summary.ts:802-813)

eeaa350 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.

…ted (cumulative reporters)

#7, cumulative reporters. Symptom: agents that live across turns report the
conversation so far on their agent span, and both pages counted that
excess over the agent's own calls again.
- smolagents `run(reset=False)` (capture cap_a_v1, session a1): run spans
  report 1067, 2243, 4857, 7811 cumulatively over calls summing to 7811;
  both pages showed 15978 (2.05x).
- Strands with one Agent reused across requests (capture strands_user,
  scenario a): 18044 shown against 4541 billed (~4x).

Cause: a wrapper kept whatever it reported above the reporters beneath it
(`countableUsageSpans` / `costBySpan`, `nettedReportersExpr`), which is
right for a model-call wrapper whose child call reported nothing and wrong
for an agent whose excess is earlier turns.

Fix: a reporter that is not a model call (an agent, a workflow, any wrapper
the index does not flag `IsLlmCall`) claims nothing of a measure once a
reporter of that measure is charged to it; model-call wrappers (an SDK's
`generateText` over `doGenerate`, a gateway generation over its attempts)
keep their excess. Same rule on the session page (`keptClaim`) and the
list (`if(r.6 = 0 AND charged > 0, 0, …)` per measure). Verified with the
compiled page query over literal index rows on ClickHouse 25.8 (smolagents
7811, Strands 1031 for three turns); the EU replay and a production week
have no agent span with an excess, so their totals are unchanged.
…covers (#21)

OpenAI Agents stamps no gen_ai.tool.call.id on its tool spans, so its
human-approval copies still count twice; the comment no longer lists it.
)

Seeds a reused Strands agent over an event-loop span (two turns) and an
OpenRouter request whose generation and provider attempt both failed, under
their own org, and runs the real compiled page query: 595 tokens and 2 calls
for the Strands session, 1 call for the failed request. The unit tests only
compare the netting SQL's text. The suite runs in CI once the ClickHouse E2E
job lists this file.
@maple-review-bot

maple-review-bot Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Maple review

Confidence 3/5 · needs attention
The netting rewrite is mirrored in SQL and in TS and both sides have regression tests; the one open defect is the list-side tool-call double count, which the change does not touch.
quality 90/100 · 1 warning · tests covered · risk medium

Nets agent-session usage per measure on both the list SQL and the session page: zero-usage step spans no longer act as reporters, the list charges each claim to the nearest token- or cost-reporting ancestor, and a wrapper keeps nothing of a measure its children reported. Logic and its mirror agree; the list still double-counts a paused tool call.

  • usageLinksExpr adds a per-trace Map from span id to itself-or-parent, for tokens and cost
  • sessionReportersExpr stores each reporter's nearest token- and cost-reporting ancestor as elements 12 and 13
  • childClaimsExpr charges each reporter's tokens under one ancestor and its cost under the other
  • Page: countedToolCalls drops a paused tool call when a later copy has its result

Still open from earlier reviews

What was checked
  • childClaimsExpr value arrays line up with nettedReportersExpr elements 2–8 and claims use the matching child element
  • nettedReportersExpr counts reproduce countedLlmCalls: parent-is-call, ancestor-reported and netted-excess branches line up
  • The new branch in listKeys… replaced by: indexTraces groups by traceId, so each reporter's small links map is looked up in its own trace

e92c9ee · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.

…e covers

Covered: Strands, which records the resumed call's result on span
attributes. Not covered, still counted twice: Google ADK (outcome only in
gcp.vertex.agent.tool_response, and the paused copy's confirmation request
reads as a result), Strands versions that record results in span events,
and OpenAI Agents (no gen_ai.tool.call.id).
@maple-review-bot

maple-review-bot Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Maple review

Confidence 3/5 · needs attention
The only file changed since the last review carries doc-only edits; the open list/page tool-call mismatch is the one thing left to settle.
quality 90/100 · 1 warning · tests covered · risk medium

The new head only documents which paused tool-call shapes countedToolCalls merges; the summary code is otherwise what the earlier reviews already read. Safe to merge, except that the list/page tool-call mismatch raised as F1 is still unfixed.

Still open from earlier reviews

What was checked
  • Read the head countedToolCalls and the session-page wiring: the paused copy is dropped only when a later same-id copy recorded a result and the paused one neither recorded nor failed (`session-summa…
  • Confirmed the list still counts tool calls as CH.sum($.IsToolCall) per index row (ai-sessions.ts:554), so F1's defect is unchanged at this head
  • costBySpan's zero-cost handling and keptClaim match the netting the list applies (Cost > 0 / token reporters), read at session-summary.ts:636

2ad793e · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple to ask about one.

@JeremyFunk
JeremyFunk merged commit 3b7241b into main Sep 29, 2026
37 checks passed
@JeremyFunk
JeremyFunk deleted the fix/agent-sessions-usage-netting branch September 29, 2026 11:08
JeremyFunk added a commit that referenced this pull request Sep 29, 2026
…blind runs

Blind runs of 11 frameworks (a fresh agent applying only the skill to an
open-source example) surfaced stale Maple limitations and missing setup
guidance.

- Remove statements fixed by #1120, #1121, #1122 and #1127 (2x token totals,
  Unidentified vendors, dropped plain-text tool payloads, OpenInference
  transcripts, check-headline caveats) from the skills and docs pages.
- Add to every skill: wrong-region 401 hint, load .env before the exporter,
  fail fast on a missing key, verification without Maple access, a driver for
  apps without a scriptable entry point, and a non-crashing TS shutdown.
- Apply the verified framework-specific fixes for vercel-ai-sdk,
  cloudflare-agents, mastra, langchain, openai-agents, google-adk,
  claude-agent-sdk and pydantic-ai.
JeremyFunk added a commit that referenced this pull request Sep 30, 2026
…1115)

* docs(agent-tracing): per-framework agent tracing guides and skills (WIP)

* docs(agent-tracing): apply verifier fixes from end-to-end runs of every guide

* docs(agent-tracing): editorial pass, cross-links from instrumentation and onboarding

* docs(agent-tracing): align guides with what Agent Sessions shows for each framework

* docs(agent-tracing): cut the human guides to a 5-minute setup, move detail into the skills

* docs(agent-tracing): trim the Vercel AI SDK setup to three packages, keep the span processor variant for serverless

* docs(agent-tracing): drop package trivia from the Vercel AI SDK install step

* docs(agent-tracing): say when to use the OpenTelemetry guide instead of listing languages

* docs(agent-tracing): cut claims a reader setting up tracing doesn't need

* docs(agent-tracing): one quick-setup wording across guides, with the EU region hint

* feat(docs): render install commands as npm/pnpm/bun and pip/uv tabs

* docs(agent-tracing): TypeScript for LangChain.js, OpenAI Agents, ADK, Cloudflare Agents and Genkit; filter guides by language

* docs(agent-tracing): list guides per language on the overview without a selector

* docs(agent-tracing): list the any-language guide once, under other languages and frameworks

* docs(agent-tracing): say what each Cloudflare Agents package is for

* skills(agent-tracing): tell agents how to send redacted feedback on a skill

* skills(agent-tracing): send feedback through the MCP only

* skills(agent-tracing): drop the feedback section for now

* skills(agent-tracing): drop fixed Maple gaps, add setup gotchas from blind runs

Blind runs of 11 frameworks (a fresh agent applying only the skill to an
open-source example) surfaced stale Maple limitations and missing setup
guidance.

- Remove statements fixed by #1120, #1121, #1122 and #1127 (2x token totals,
  Unidentified vendors, dropped plain-text tool payloads, OpenInference
  transcripts, check-headline caveats) from the skills and docs pages.
- Add to every skill: wrong-region 401 hint, load .env before the exporter,
  fail fast on a missing key, verification without Maple access, a driver for
  apps without a scriptable entry point, and a non-crashing TS shutdown.
- Apply the verified framework-specific fixes for vercel-ai-sdk,
  cloudflare-agents, mastra, langchain, openai-agents, google-adk,
  claude-agent-sdk and pydantic-ai.

* docs(agent-tracing): drop stale payload rules, build the Claude SDK env per call

- opentelemetry: tool results may be plain strings; Maple no longer drops
  plain-text tool payloads.
- claude-agent-sdk: build the telemetry env per query() and fail fast on a
  missing key, matching the skill.

* docs(agent-tracing): drop setup steps Maple no longer needs

- GenAI semconv flag is recommended, not required, for LangChain (Python), LlamaIndex and smolagents; openai-agents keeps it for agent lanes and finish reasons
- smolagents: stop zeroing run-span token usage
- Vercel AI SDK / Cloudflare Agents: runtimeContext groups sessions, drop enrichSpan
- Genkit: pass string tool results through unwrapped
- LangChain.js: correct the GenAiSpans rationale
- Strands TS: note zero tokens with api: "chat" behind OpenAI-compatible gateways

* skills(agent-tracing): pass LangChain.js tool results through as plain text

* docs(agent-tracing): drop workarounds and caveats the agent-session fixes made stale

- Session keys: every vendor now falls back to gen_ai.conversation.id; drop the
  "Maple ignores gen_ai.conversation.id" lines (agno, crewai, dspy, smolagents,
  spring-ai, strands) and the OpenInference Haystack session.id caveat.
- Tokens: usage counts only on the model-call span, so drop Strands'
  gen_ai_use_latest_invocation_tokens, the per-request TS agent rationale and
  the 1.54 floor, pydantic-ai's aggregated-usage warning and the Anthropic cache
  double-count caveats. Hand-written spans send semconv totals (input includes
  cache, output includes reasoning); the Anthropic/Gemini mappings and the ADK
  TS processor follow that.
- Cost: LiteLLM's litellm.cost.total and Pydantic AI's operation.cost are read;
  drop the LiteLLM turn-cost recipe.
- Detection: LangChain.js, Genkit and .NET Semantic Kernel get their framework
  label; OpenAI Agents TS gets agent lanes; LangChain.js groups by session.id
  without GenAiSpans' conversation-id copy.
- Classification: drop DSPy's adapter marker, Spring AI's advisor rename,
  LangChain's ChatPromptTemplate step, the MAF workflow.build instruction,
  Mastra scorer and LangChain turn-label caveats, and MapleSpanFixes' tool
  argument fix (the tool-errors view decodes arguments like the session page).

* skills(agent-tracing): leave ADK TS output tokens as reported; Maple adds thinking

* skills(agent-tracing): trim to what an implementing agent needs (#1174)

- cut human-guide links, backend background, tested-version notes, restated code
- drop the Go reference; other languages follow the generic steps
- Do-not lists keep only silent, non-obvious mistakes not stated in the steps
- inline the GenkitForMaple processor instead of pointing at the guide
- OTLP header: quoted literal space everywhere (every targeted SDK accepts it)
- add Cloudflare Agents and Genkit to the OpenTelemetry skill's framework list
- smolagents: enable_genai_semconv is required

* docs(agent-tracing): ADK header uses a quoted literal space, not %20

* docs(agent-tracing): keep tool error text out of Haystack spans with content off, note Spring AI's

* docs(agent-tracing): export ADK env vars, name MAPLE_INGEST_KEY, drop the removed Go reference

* skills(agent-tracing): tolerate malformed tool arguments, route provider SDKs through the router, raw skill URL

* docs(agent-tracing): tighten the OpenTelemetry guide's intro, content and check wording

* docs(agent-tracing): use the private ingest key from the environment, never inline

Agent tracing runs server-side, so guides and skills now use the private key (maple_sk_) as MAPLE_INGEST_KEY in the repo's secret/env convention. The user sets it themselves instead of pasting it into the prompt; skills create a gitignored .env and .env.example when the repo has none.

* docs(onboard): servers use the private ingest key from MAPLE_INGEST_KEY, browsers the public key

maple-onboard and the language style skills now read the private key (maple_sk_) from MAPLE_INGEST_KEY on servers, with the agent-tracing secret rules: repo secret/env convention, gitignored .env plus .env.example when there is none, fail fast when unset, never in source or asked for in chat. Browser and mobile code keep the inline public key. Landing docs and the agent-tracing overview prompt follow.

* docs(agent-tracing): warn and disable export when MAPLE_INGEST_KEY is unset

Instrumentation must never crash or block the app. Replace every throw,
exit, panic and ${VAR:?} on a missing key with one warning plus a skipped
Maple exporter, and never send an empty bearer.

* skills(agent-tracing): read the Spring key from Boot's environment so .env works

* Revert "skills(agent-tracing): read the Spring key from Boot's environment so .env works"

This reverts commit dc45d34.

* Revert "docs(agent-tracing): warn and disable export when MAPLE_INGEST_KEY is unset"

This reverts commit 307aa7a.

* Revert "docs(onboard): servers use the private ingest key from MAPLE_INGEST_KEY, browsers the public key"

This reverts commit 814e177.

* Revert "docs(agent-tracing): use the private ingest key from the environment, never inline"

This reverts commit 821d778.

* docs(agent-tracing): warn and disable export when the ingest key is unset or setup fails

Instrumentation must never crash or block the app. Code that reads
MAPLE_INGEST_KEY logs one warning and skips the Maple exporter instead of
throwing, exiting or panicking, and Go/Rust setup errors are logged, not
fatal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant