feat: add self-describing billed usage to TokenUsage#896
Open
season179 wants to merge 3 commits into
Open
Conversation
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
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 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: wrap transcription call
OtelMiddleware->>LocalCaptureTracer: record billed quantity/unit
OtelTranscriptionRoute-->>PlaywrightTest: { ok, spans }
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
testing/e2e/global-setup.tsParsing error: "parserOptions.project" has been provided for 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.githubusercontent.com/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
- 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.
Contributor
|
@tombeckenham leaving this one to you! |
# Conflicts: # docs/config.json
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
billed: { quantity, unit }for media and transcription results (with legacy billed fields still populated).tanstack.ai.usage.billed_quantity/tanstack.ai.usage.billed_unit, including for duration-billed transcription.usage.billedinstead ofusage.unitsBilled./api/otel-transcription.