Skip to content

otel: read the engine's provenance flags instead of guessing from values - #488

Merged
brentrager merged 1 commit into
mainfrom
use-usage-estimated-flag
Aug 18, 2026
Merged

brentrager merged 1 commit into
mainfrom
use-usage-estimated-flag

Conversation

@brentrager

Copy link
Copy Markdown
Contributor

Required follow-up to #474, closing a hole I introduced there. Consumes SmooAI/smooth-operator-core#177 (published as core 1.10.0, verified on crates.io — see below).

The hole in #474

#474 decided "was this measured?" by testing prompt_tokens > 0. That infers provenance from the value, and there are two fabrication sites in core that differ:

site prompt_tokens
streaming hardcoded 0
non-streaming estimated from the outgoing request's JSON length → plausible non-zero

So the heuristic caught the streaming path (the common one, which is why #474 was still a real improvement) and silently waved the non-streaming path through as measured.

record_turn_usage now reads AgentEvent::Completed.usage_estimated. A flag carries the fact; a heuristic guesses it — the same reason absent beats 0.

Proof, by restoring the old gate and re-running the new test:

an ESTIMATED count must be omitted however plausible it looks — this is the case
the old value-based heuristic missed; span fields: {… "gen_ai.usage.input_tokens": "372" …}

372 is invented. The old gate published it as a measurement.

Two new attributes

  • gen_ai.usage.cost_source = gateway | estimated, set alongside cost_usd from Completed.cost_estimated. Local ModelPricing returns the free tier for any model it doesn't recognise, so an estimate can be a wild under-count while looking exact. A billed surface must not render the two identically.
  • gen_ai.response.id from Completed.response_id, recorded whenever present. Joins to LiteLLM_SpendLogs.request_id, whose row carries the gateway's authoritative dollars and real token counts — so it matters most exactly when the counts are missing, and it turns "measured vs estimated" from a claim into something reconcilable after the fact.

Both had to be declared tracing::field::Empty at span creation. Span::record silently no-ops on an undeclared field, so the first run recorded neither while everything still compiled and only the assertions caught it.

Wire compatibility

TurnUsage now carries the three engine flags, but they are telemetry-only: eventual_response still serializes exactly {costUsd, promptTokens, completionTokens}. Pinned by an assertion on the object's key count, with all three flags set to non-default values in the fixture so a leak fails the test.

TurnUsage loses Copy (response_id is an owned String); the one call site already passed by reference.

Verification

gate result
cargo test -p smooai-smooth-operator-server -p smooai-smooth-operator 576 passed, 0 failed, 1 ignored, 46 suites (+1 new test)
cargo clippy --all-targets -- -D warnings clean (exit 0)
cargo fmt --all --check clean (exit 0)

Core publish verified directly rather than trusting a green workflow — literal 200 from static.crates.io for smooai-smooth-operator-core and -temporal at both 1.9.0 and 1.10.0, and the published 1.10.0 tarball was unpacked and confirmed to contain pub usage_estimated, pub response_id, ResponseId { and cost_estimated.

Cargo.lock also unifies socket2 0.5.10 → 0.6.4 for hyper-util/tokio/tokio-postgres. Both versions remain in the lock; incidental to the core bump, and I diffed the lock rather than assuming.

No changeset — the release for this repo is being driven separately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LkCz96UfUxcai5RU4LwPnG

Bumps core 1.8.0 -> 1.10.0 and consumes what SmooAI/smooth-operator-core#177
added, closing a hole in #474 that I introduced.

#474 decided "was this measured?" by testing `prompt_tokens > 0`. That
inferred provenance from the VALUE, and it only worked on one of the two
fabrication sites in core: the streaming one hardcodes `prompt_tokens = 0`,
but the non-streaming one estimates it from the outgoing request's JSON
length and so produces a plausible non-zero. The heuristic caught the first
and silently waved the second through as measured.

`record_turn_usage` now reads `AgentEvent::Completed.usage_estimated`. A
flag carries the fact; a heuristic guesses it — the same reason `absent`
beats `0`.

Two new attributes, both previously impossible without the core change:

- `gen_ai.usage.cost_source` = `gateway` | `estimated`, set alongside
  `cost_usd` from `Completed.cost_estimated`. Local `ModelPricing` returns
  the FREE tier for any model it doesn't recognise, so an estimate can be a
  wild under-count while looking exact; a billed surface must not render
  the two identically.
- `gen_ai.response.id` from `Completed.response_id`, recorded whenever
  present. It joins to `LiteLLM_SpendLogs.request_id`, whose row carries
  the gateway's authoritative dollars AND real token counts — so it matters
  most exactly when the counts above are missing, and it turns "measured vs
  estimated" from a claim into something reconcilable.

Both had to be declared `tracing::field::Empty` at span creation; `record`
silently no-ops on an undeclared field, which is how the first run recorded
neither while every test still compiled.

`TurnUsage` carries the three engine flags but they are TELEMETRY-only —
`eventual_response` still serializes exactly `{costUsd, promptTokens,
completionTokens}`, now pinned by an assertion on the object's key count
with all three flags set in the fixture. It also loses `Copy`, since
`response_id` is an owned `String`.

Verified by restoring the old `prompt_tokens > 0` gate: the new test fails
with `input_tokens: "372"` — an invented count published as measured.

576 tests pass across 46 suites (+1 new). clippy -D warnings and fmt
--check clean. Cargo.lock also unifies socket2 0.5.10 -> 0.6.4 for
hyper-util/tokio/tokio-postgres; both versions remain in the lock and it is
incidental to the core bump.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LkCz96UfUxcai5RU4LwPnG
@changeset-bot

changeset-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d16d742

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@brentrager
brentrager merged commit fb7d0d1 into main Aug 18, 2026
3 checks passed
brentrager added a commit that referenced this pull request Aug 18, 2026
#488 merged without one, same as #470, #471 and #474 before it. Four for four
on this ticket — the release here is changeset-driven, so a merged fix that
carries no changeset publishes nothing and every consumer keeps the old crate.


Claude-Session: https://claude.ai/code/session_01LkCz96UfUxcai5RU4LwPnG

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
brentrager added a commit that referenced this pull request Aug 18, 2026
Fifth in a row on this ticket to merge without one (#470, #471, #474, #488,
#490). Adding it so the parity work actually publishes.


Claude-Session: https://claude.ai/code/session_01LkCz96UfUxcai5RU4LwPnG

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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