feat(fal): surface billed cost as result.usage.unitsBilled#723
Conversation
The fal adapters discarded fal's response headers, so the actual billed cost of a generation was unrecoverable through the SDK. fal returns the real billed quantity in the `x-fal-billable-units` header on the result fetch; this surfaces it as `result.usage.unitsBilled` so consumers can compute exact media-generation cost without wrapping `fetch` themselves. - `TokenUsage` gains an optional `unitsBilled` (a bare count of priced units, sibling to `durationSeconds`; the unit name itself is provider -defined and looked up via the pricing API, not carried here). - A `config.fetch` wrapper reads `x-fal-billable-units` off every fal response, keyed by `x-fal-request-id` (the same value the client surfaces as `Result.requestId`), so the adapter's lookup always matches the fetch the units came from. `config.fetch` is used rather than `responseHandler` because the queue ops clobber a global handler. - All five fal media adapters (image, audio, video, speech, transcription) populate `result.usage.unitsBilled` when fal reports it. - `VideoUrlResult` gains a `usage` slot; `getVideoJobStatus` now emits the `video:usage` event and returns `usage` on completion. Closes #722 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR surfaces Fal's billed units as result.usage.unitsBilled across fal adapters, adds billing utilities and a fetch wrapper to capture x-fal-billable-units, wires the wrapper into the fal client, updates adapters and video status to propagate usage, and adds tests, e2e validation, and docs. ChangesFal billable units surface and tracking
Sequence DiagramsequenceDiagram
participant configureFalClient
participant createBillingFetch
participant FalAPI
participant BillingRegistry
participant FalAdapter
participant Activity
configureFalClient->>createBillingFetch: configure fetch via fal.config({ fetch })
FalAdapter->>createBillingFetch: request result using configured fetch
createBillingFetch->>FalAPI: network request
FalAPI-->>createBillingFetch: response with x-fal-request-id + x-fal-billable-units
createBillingFetch->>BillingRegistry: recordBillableUnitsFromResponse(response)
createBillingFetch-->>FalAdapter: return response (body intact)
FalAdapter->>BillingRegistry: takeBillableUnits(requestId)
BillingRegistry-->>FalAdapter: unitsBilled
FalAdapter-->>Activity: return result including usage.unitsBilled
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
🚀 Changeset Version Preview3 package(s) bumped directly, 28 bumped as dependents. 🟥 Major bumps
🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 5a0b9db ☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit f243ebf
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
Surface the new `result.usage.unitsBilled` in the media example so the
billed quantity is visible after a generation — a caption under each
generated image/video ("Billed N fal units"). Verified against the live
fal API: a fal-ai/flux/schnell generation reports unitsBilled: 1.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ai-fal/tests/speech-adapter.test.ts (1)
1-307:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd coverage for
result.usage.unitsBilledin Fal speech adapter tests.
packages/ai-fal/src/adapters/speech.tsbuilds and conditionally returnsusage(buildFalUsage(takeBillableUnits(response.requestId))), butpackages/ai-fal/tests/speech-adapter.test.tshas no assertions forresult.usage.unitsBilled(when billable units are present) and no assertions thatusageis omitted when they’re absent.Add tests mirroring the audio adapter pattern:
- Seed billable units with
seedBillableUnits- Assert
generateSpeech(...).usage.unitsBilledis present when billable units are reported- Assert
usageisundefinedwhen billable units are absent🤖 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-fal/tests/speech-adapter.test.ts` around lines 1 - 307, Add two tests in packages/ai-fal/tests/speech-adapter.test.ts to cover usage: one that seeds billable units (use seedBillableUnits with the response.requestId returned by the mocked subscribe call, e.g., 'req-speech-123') then call generateSpeech with the falSpeech adapter and assert result.usage.unitsBilled equals the seeded value; and a second test where no billable units are seeded (mockSubscribe returns a response without billable info) then assert result.usage is undefined. Ensure tests reference generateSpeech and falSpeech and use the requestId from createMockSpeechResponse so the adapter’s takeBillableUnits/buildFalUsage path is exercised.packages/ai-fal/tests/transcription-adapter.test.ts (1)
1-296:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing billing/usage tests for the Fal transcription adapter
packages/ai-fal/tests/transcription-adapter.test.tscovers transcription/segments behavior, but has no assertions that the adapter surfacesresult.usage.unitsBilled(or omitsusagewhen unbilled).- Add tests matching the existing
audio-adapter/image-adapter/video-adapterpattern:
- Seed billable units for the mocked fal
requestIdviaseedBillableUnits- Assert
generateTranscription(...).usageequals the expectedunitsBilledshape when billed- Assert
generateTranscription(...).usageisundefinedwhen fal does not report billable units🤖 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-fal/tests/transcription-adapter.test.ts` around lines 1 - 296, Add billing/usage tests to the Fal transcription adapter tests: when mocking fal.subscribe responses include a requestId and call seedBillableUnits(requestId, units) before invoking generateTranscription to assert result.usage equals the expected { unitsBilled: units } shape; also add a test where no billable units are seeded and assert generateTranscription(...).usage is undefined. Update tests that use falTranscription and generateTranscription (and mockSubscribe) to seed billable units for the mocked requestId in the billed-case test and leave it unseeded for the unbilled-case test so assertions mirror the patterns used in audio-adapter/image-adapter/video-adapter tests.
🧹 Nitpick comments (3)
packages/ai-fal/tests/audio-adapter.test.ts (1)
7-16: 💤 Low valueConsider extracting seedBillableUnits to a shared test utility.
The
seedBillableUnitshelper is duplicated across audio, image, and video adapter tests. Extracting it to a shared test util (e.g.,tests/test-utils.ts) would reduce duplication and ensure consistency.🤖 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-fal/tests/audio-adapter.test.ts` around lines 7 - 16, Extract the duplicated seedBillableUnits helper into a shared test utility file (e.g., tests/test-utils.ts) so all adapter tests reuse it; move the function that calls recordBillableUnitsFromResponse(new Response(...)) into that file, export it (seedBillableUnits), and update audio-adapter.test.ts, image and video adapter tests to import seedBillableUnits from the shared util instead of declaring their own copies. Ensure the exported helper accepts (requestId: string, units: string) and preserves the same header keys ('x-fal-request-id' and 'x-fal-billable-units') so behavior remains identical.packages/ai-fal/tests/image-adapter.test.ts (1)
7-21: 💤 Low valueConsider extracting seedBillableUnits to a shared test utility.
The
seedBillableUnitshelper is duplicated across audio, image, and video adapter tests. The JSDoc here is helpful—preserving it when extracting to a shared util (e.g.,tests/test-utils.ts) would benefit all tests.🤖 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-fal/tests/image-adapter.test.ts` around lines 7 - 21, Extract the duplicated seedBillableUnits helper into a shared test utility module (e.g., tests/test-utils.ts) and export it for reuse; move the JSDoc block along with the function to preserve documentation and behavior, keep the implementation calling recordBillableUnitsFromResponse(new Response(...)) intact, then import seedBillableUnits from the new utility in the audio, image, and video adapter tests and remove the duplicate definitions there so all tests reference the single exported function.testing/e2e/src/routes/api.fal-billable-units.ts (1)
59-66: 💤 Low valueError responses return HTTP 200 instead of an error status.
Line 65 returns
status: 200even when generation fails. This makes it impossible to distinguish success from failure at the HTTP level — clients must parse the JSON body to check theokfield.If this is intentional for test simplicity, consider adding a comment explaining the rationale.
📝 Consider using a 5xx status for server errors
} catch (error) { return new Response( JSON.stringify({ ok: false, error: error instanceof Error ? error.message : String(error), }), - { status: 200, headers: { 'Content-Type': 'application/json' } }, + { status: 500, headers: { 'Content-Type': 'application/json' } }, ) }🤖 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.fal-billable-units.ts` around lines 59 - 66, The catch block currently returns a Response with status: 200 even on errors; update the error Response in the catch clause that constructs new Response({...}) to use an appropriate error HTTP status (e.g., 500) instead of 200 so failures are distinguishable at the HTTP level, and if you intentionally want 200 for tests add a short comment explaining that choice; adjust only the status property in the Response created inside the catch (error) block that serializes { ok: false, error: ... }.
🤖 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.
Inline comments:
In @.changeset/fal-billable-units-usage.md:
- Line 7: Update the summary sentence to refer to billed "units" (or "billed
units") rather than "billed cost" to match the actual surfaced field;
specifically, change the wording to say that the fal adapters expose the billed
units on the generation result (e.g., result.usage or result.usage.unitsBilled)
by reading the x-fal-billable-units response header so the summary aligns with
the bullets and avoids implying a currency amount.
In `@testing/e2e/src/routes/api.fal-billable-units.ts`:
- Around line 27-42: The current handler patches globalThis.fetch (using
originalFetch, FAL_QUEUE_PREFIX, mockBase) which creates a race when requests
run concurrently; instead remove the global patch and supply a per-request
custom fetch to the adapter — create a local wrapper fetch that rewrites URLs
starting with FAL_QUEUE_PREFIX to mockBase + sliced path and pass that wrapper
into the adapter/config used in this route (rather than mutating
globalThis.fetch), eliminating the try/finally restore and preventing
cross-request interference.
---
Outside diff comments:
In `@packages/ai-fal/tests/speech-adapter.test.ts`:
- Around line 1-307: Add two tests in
packages/ai-fal/tests/speech-adapter.test.ts to cover usage: one that seeds
billable units (use seedBillableUnits with the response.requestId returned by
the mocked subscribe call, e.g., 'req-speech-123') then call generateSpeech with
the falSpeech adapter and assert result.usage.unitsBilled equals the seeded
value; and a second test where no billable units are seeded (mockSubscribe
returns a response without billable info) then assert result.usage is undefined.
Ensure tests reference generateSpeech and falSpeech and use the requestId from
createMockSpeechResponse so the adapter’s takeBillableUnits/buildFalUsage path
is exercised.
In `@packages/ai-fal/tests/transcription-adapter.test.ts`:
- Around line 1-296: Add billing/usage tests to the Fal transcription adapter
tests: when mocking fal.subscribe responses include a requestId and call
seedBillableUnits(requestId, units) before invoking generateTranscription to
assert result.usage equals the expected { unitsBilled: units } shape; also add a
test where no billable units are seeded and assert
generateTranscription(...).usage is undefined. Update tests that use
falTranscription and generateTranscription (and mockSubscribe) to seed billable
units for the mocked requestId in the billed-case test and leave it unseeded for
the unbilled-case test so assertions mirror the patterns used in
audio-adapter/image-adapter/video-adapter tests.
---
Nitpick comments:
In `@packages/ai-fal/tests/audio-adapter.test.ts`:
- Around line 7-16: Extract the duplicated seedBillableUnits helper into a
shared test utility file (e.g., tests/test-utils.ts) so all adapter tests reuse
it; move the function that calls recordBillableUnitsFromResponse(new
Response(...)) into that file, export it (seedBillableUnits), and update
audio-adapter.test.ts, image and video adapter tests to import seedBillableUnits
from the shared util instead of declaring their own copies. Ensure the exported
helper accepts (requestId: string, units: string) and preserves the same header
keys ('x-fal-request-id' and 'x-fal-billable-units') so behavior remains
identical.
In `@packages/ai-fal/tests/image-adapter.test.ts`:
- Around line 7-21: Extract the duplicated seedBillableUnits helper into a
shared test utility module (e.g., tests/test-utils.ts) and export it for reuse;
move the JSDoc block along with the function to preserve documentation and
behavior, keep the implementation calling recordBillableUnitsFromResponse(new
Response(...)) intact, then import seedBillableUnits from the new utility in the
audio, image, and video adapter tests and remove the duplicate definitions there
so all tests reference the single exported function.
In `@testing/e2e/src/routes/api.fal-billable-units.ts`:
- Around line 59-66: The catch block currently returns a Response with status:
200 even on errors; update the error Response in the catch clause that
constructs new Response({...}) to use an appropriate error HTTP status (e.g.,
500) instead of 200 so failures are distinguishable at the HTTP level, and if
you intentionally want 200 for tests add a short comment explaining that choice;
adjust only the status property in the Response created inside the catch (error)
block that serializes { ok: false, error: ... }.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25de0a13-4f5d-49cb-8492-129b9345c07c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.changeset/fal-billable-units-usage.mddocs/config.jsondocs/media/audio-generation.mddocs/media/image-generation.mddocs/media/video-generation.mdpackages/ai-event-client/src/index.tspackages/ai-fal/src/adapters/audio.tspackages/ai-fal/src/adapters/image.tspackages/ai-fal/src/adapters/speech.tspackages/ai-fal/src/adapters/transcription.tspackages/ai-fal/src/adapters/video.tspackages/ai-fal/src/utils/billing.tspackages/ai-fal/src/utils/client.tspackages/ai-fal/src/utils/index.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/utils.test.tspackages/ai-fal/tests/video-adapter.test.tspackages/ai/skills/ai-core/media-generation/SKILL.mdpackages/ai/src/activities/generateVideo/index.tspackages/ai/src/types.tstesting/e2e/global-setup.tstesting/e2e/package.jsontesting/e2e/src/routeTree.gen.tstesting/e2e/src/routes/api.fal-billable-units.tstesting/e2e/tests/fal-billable-units.spec.ts
fal's generated `size`/`resolution` type for `xai/grok-imagine-image`
offers `16:9_1K` / `16:9_4K`, but the live API rejects those resolutions
("Input should be '1k' or '2k'") — the vendor enum is out of sync with
the API. `'16:9_4K'` therefore type-checked but 422'd at runtime. Pass
`aspect_ratio: '16:9'` via modelOptions instead and let the endpoint
default the resolution; verified against the live fal API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address CodeRabbit + review feedback: dependency-inject the underlying
fetch rather than mutating globals.
- `createBillingFetch(baseFetch = globalThis.fetch)` now takes the fetch to
delegate to, and `FalClientConfig` gains an optional `fetch` override that
`configureFalClient` wraps for usage capture.
- The E2E route passes a per-request redirecting `fetch` via
`falImage(model, { fetch })` instead of swapping `globalThis.fetch` —
removing the concurrency race CodeRabbit flagged (no global mutation, no
try/finally restore).
- `billing.test.ts` injects a fake fetch directly instead of
`vi.stubGlobal('fetch')`.
- Changeset: reword "billed cost" → "billed units" to match the surfaced
`usage.unitsBilled` field.
Verified: full `pnpm test:pr` green, fal E2E spec green, and a live
fal-ai/flux/schnell call still reports unitsBilled via the default
(no-override) path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…test (#725) Follow-up to #723 addressing CodeRabbit review feedback. - Add unit coverage for `result.usage.unitsBilled` on the fal speech and transcription adapters (billed + unbilled cases), mirroring the existing audio/image/video adapter tests. - Remove the e2e `fal-billable-units` spec, its `/api/fal-billable-units` route, and the hand-rolled `/fal-queue` aimock mount. aimock has no seam to stamp the `x-fal-billable-units` response header the feature reads, so any e2e test required manipulating `fetch` to redirect fal's hardcoded `queue.fal.run` URLs. The billed-units behavior is now covered by unit tests across all five fal adapters instead. - Drop the now-orphaned `@tanstack/ai-fal` dependency from the e2e app and regenerate the route tree. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🎯 Changes
Closes #722.
The fal adapters discarded fal's response headers, so the actual billed cost of a generation was unrecoverable through the SDK — apps had to wrap
fetchand scrapex-fal-billable-unitskeyed by request id. This surfaces that value asresult.usage.unitsBilledso consumers can compute exact media-generation cost directly.Core type (
@tanstack/ai-event-client)TokenUsagegains an optionalunitsBilled?: number— a bare count of priced units for usage-based (non-token) billing, sitting besidedurationSeconds. It's a quantity, not money (distinct fromcost/costDetails), and the unit name (megapixels, seconds, …) is provider-defined and looked up via the pricing API, not carried here.fal adapters (
@tanstack/ai-fal)config.fetchwrapper (utils/billing.ts) readsx-fal-billable-unitsoff every fal response, keyed byx-fal-request-id— the same value the client surfaces asResult.requestId, so the adapter's lookup always matches the fetch the units came from (no URL parsing, no app-side request-id registry).config.fetchis used rather than a globalresponseHandlerbecause the queue ops override the response handler per request.falImage,falAudio,falVideo,falSpeech,falTranscription— populateresult.usage.unitsBilledwhen fal reports it.Video activity (
@tanstack/ai)VideoUrlResultgains an optionalusageslot; threaded into the streaminggeneration:resultchunk, andgetVideoJobStatusnow emits the (previously defined-but-unemitted)video:usageevent and returnsusageon completion.Tests / docs / skill
billing.test.ts+ usage-attachment tests across image/audio/video; updated config-shape assertions for the newconfig.fetch.result.usage.unitsBilledreaches a consumer end-to-end throughgenerateImage.docs/config.jsonupdatedAt) and the media-generation agent skill.✅ Checklist
pnpm run test:pr.🚀 Release Impact
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Examples
Tests