feat(audio): report transcription audio length as a cost basis - #868
Conversation
Models in the whisper family bill by audio length, not tokens: the
response reports `usage: {type: "duration", seconds: N}` and no token
counts at all. The emitted UsageEvent carried tokens only, so cp-api
received a row with nothing to price it with and every such request
settled at $0.
`UsageEvent` gains `audio_duration_seconds`, populated on
/v1/audio/transcriptions and /translations. It is skipped on the wire
when zero, so token-only events are byte-identical to before and an
older cp-api simply ignores the field.
The length is read from the upstream response where it exists — the
`json` format's `usage.seconds`, `verbose_json`'s top-level `duration`.
Neither is present on `text` / `srt` / `vtt`, whose bodies are not JSON,
so those fall back to reading the uploaded file's own properties. That
fallback is not an optimisation: without it the caller decides whether a
request is metered by choosing a response format, which is the same
shape of bypass as an unbilled stream.
Reading the file is metadata-only — `lofty` parses container and codec
headers without decoding, so an arbitrary caller upload costs
microseconds, and anything unrecognised yields no duration rather than
an error. The probe runs only when the response carried no duration, so
the common `json` path never pays for it.
TTS is unchanged: /v1/audio/speech bills per input character, not by the
length of the audio it produced.
Refs api7/AISIX-Cloud#1138, #457
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAudio requests now derive duration from upstream metadata or uploaded-file metadata. The duration flows into usage events and OTLP spans. Tests cover supported response formats, fallback probing, invalid uploads, and end-to-end reporting. ChangesAudio duration metering
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant MultipartDispatch
participant UpstreamResponse
participant lofty
participant UsageEvent
participant OTLPSpan
MultipartDispatch->>UpstreamResponse: Read usage.seconds or duration
MultipartDispatch->>lofty: Probe uploaded audio if duration is unavailable
MultipartDispatch->>UsageEvent: Set audio_duration_seconds
UsageEvent->>OTLPSpan: Export aisix.audio.duration_seconds
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
#865 landed the SSE usage fallback on the same statement this branch extends with the duration cost basis, and both branches appended tests at the same anchor. Resolution keeps both: the usage extraction now reads tokens from JSON, then from a streamed body's terminal event, while the duration is measured independently.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/aisix-proxy/src/audio.rs (1)
758-775: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid parsing the response body as JSON twice.
Line 758 parses
body_bytesinto aserde_json::Valueforextract_token_usage. Line 767 callsupstream_duration_seconds(&body_bytes), which parses the same bytes again. Everyjson-format transcription or translation response pays for two full JSON parses of the same body.Parse once and reuse the value for both extractions.
♻️ Proposed refactor to parse the body once
- let usage = serde_json::from_slice::<Value>(&body_bytes) - .ok() - .as_ref() - .and_then(extract_token_usage) - .or_else(|| extract_sse_token_usage(&upstream_headers, &body_bytes)); + let parsed_body = serde_json::from_slice::<Value>(&body_bytes).ok(); + let usage = parsed_body + .as_ref() + .and_then(extract_token_usage) + .or_else(|| extract_sse_token_usage(&upstream_headers, &body_bytes)); // Duration cost basis (`#457`): what the upstream reported, else what // the uploaded file says. The probe runs only when the response // carried nothing, so the common `json` path never pays for it. - let duration_seconds = upstream_duration_seconds(&body_bytes) + let duration_seconds = parsed_body + .as_ref() + .and_then(upstream_duration_from_value) .or_else(|| { fields .iter() .find(|(name, ..)| name == "file") .and_then(|(.., data)| probe_audio_duration_seconds(data)) }) .unwrap_or(0.0);And split
upstream_duration_secondsso the byte-slice entry point (kept for the existing unit tests) delegates to aValue-based helper:-fn upstream_duration_seconds(body: &[u8]) -> Option<f64> { - let json = serde_json::from_slice::<Value>(body).ok()?; - let from_usage = json +fn upstream_duration_seconds(body: &[u8]) -> Option<f64> { + let json = serde_json::from_slice::<Value>(body).ok()?; + upstream_duration_from_value(&json) +} + +fn upstream_duration_from_value(json: &Value) -> Option<f64> { + let from_usage = json .get("usage") .and_then(|u| u.get("seconds")) .and_then(Value::as_f64); let seconds = from_usage.or_else(|| json.get("duration").and_then(Value::as_f64))?; (seconds.is_finite() && seconds > 0.0).then_some(seconds) }🤖 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 `@crates/aisix-proxy/src/audio.rs` around lines 758 - 775, Refactor the response handling around extract_token_usage and upstream_duration_seconds to deserialize body_bytes into one serde_json::Value and reuse it for both token-usage and duration extraction. Split upstream_duration_seconds so its byte-slice API remains available for existing tests while delegating to a Value-based helper, and update the main flow to call that helper with the shared parsed value.tests/e2e/src/cases/audio-duration-cost-basis-e2e.test.ts (1)
189-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd E2E coverage for an unsupported uploaded file.
The PR states that unsupported uploads must produce no duration without failing the request. The Rust unit tests cover this at the
probe_audio_duration_secondslevel, but this E2E suite does not exercise the full request path (200 response,audio_duration_secondsomitted from the emitted span) for a file lofty cannot parse.Add a third case reusing the
textUpstream(or a similar no-usage upstream): upload non-audio bytes as thefilefield withresponse_format: "text", assert the response is still200, and assert the resulting span carries noaisix.audio.duration_secondsattribute (or reports0).As per coding guidelines, "Tests must cover boundary cases (empty values, min/max), invalid inputs, combination scenarios, and extreme cases (high load, failures)" for
**/*.{test,spec}.{js,ts,jsx,tsx}.🤖 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 `@tests/e2e/src/cases/audio-duration-cost-basis-e2e.test.ts` around lines 189 - 251, Add a third scenario in the duration-billing test after the existing text response case, using the text-transcribe/no-usage upstream and non-audio bytes as the uploaded file with response_format set to text. Assert the request returns HTTP 200, retrieve its emitted span via the request ID, and verify aisix.audio.duration_seconds is absent or zero.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.
Inline comments:
In `@crates/aisix-proxy/src/audio.rs`:
- Around line 1240-1263: Update probe_audio_duration_seconds to handle valid
WebM/Matroska uploads unsupported by lofty, either by adding duration coverage
for WebM or by preventing those uploads from receiving a zero billing duration.
Preserve the existing zero-cost behavior only for formats whose duration cannot
be identified, and ensure text, SRT, and VTT responses use the recovered WebM
duration.
---
Nitpick comments:
In `@crates/aisix-proxy/src/audio.rs`:
- Around line 758-775: Refactor the response handling around extract_token_usage
and upstream_duration_seconds to deserialize body_bytes into one
serde_json::Value and reuse it for both token-usage and duration extraction.
Split upstream_duration_seconds so its byte-slice API remains available for
existing tests while delegating to a Value-based helper, and update the main
flow to call that helper with the shared parsed value.
In `@tests/e2e/src/cases/audio-duration-cost-basis-e2e.test.ts`:
- Around line 189-251: Add a third scenario in the duration-billing test after
the existing text response case, using the text-transcribe/no-usage upstream and
non-audio bytes as the uploaded file with response_format set to text. Assert
the request returns HTTP 200, retrieve its emitted span via the request ID, and
verify aisix.audio.duration_seconds is absent or zero.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e452a567-8498-487d-99a5-6ded769daf7e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlcrates/aisix-obs/src/otlp_http_sink.rscrates/aisix-obs/src/usage.rscrates/aisix-proxy/Cargo.tomlcrates/aisix-proxy/src/audio.rstests/e2e/src/cases/audio-duration-cost-basis-e2e.test.ts
`lofty` reads every container the transcription endpoints accept except Matroska/WebM, and WebM is not a corner case: `MediaRecorder` produces `audio/webm;codecs=opus` by default, so it is what a browser uploads. A WebM transcription asked for as `text` / `srt` / `vtt` therefore reported no length and billed nothing — the exact bypass the file probe exists to close. Adds a header-only EBML reader: EBML root → Segment → Info, taking Duration (a float in TimecodeScale units) and TimecodeScale (nanoseconds, default 1,000,000). Clusters are never entered and no audio is decoded. Two things make this a structural walk rather than a byte scan. A file's SeekHead carries the Info id inside its SeekID payloads, so scanning for the id lands on a pointer and reads garbage. And a live muxer — which is what a browser is — leaves Segment at the unknown-size marker, so its extent has to be inferred from the parent rather than read. The input is a caller upload, so the walk is total: every read is bounds-checked, declared sizes are clamped to the enclosing element rather than trusted, and the element count is capped. Reported by CodeRabbit on #868.
1 participant
Problem
Models in the whisper family bill by audio length, not tokens. Their response reports
{"text": "...", "usage": {"type": "duration", "seconds": 11}}and no token counts at all. The emitted
UsageEventcarried tokens only, so cp-api received a row with nothing to price it with — every whisper-class request settles at $0. Verified against real OpenAI through a real DP in api7/AISIX-Cloud#1138:whisper-1recordedprompt=0 completion=0whilegpt-4o-transcribe(which does report tokens) recorded26/12on the same audio.Change
UsageEventgainsaudio_duration_seconds, populated on/v1/audio/transcriptionsand/v1/audio/translations. Skipped on the wire when zero, so token-only events are byte-identical to before and an older cp-api ignores the field.Two sources, in order:
jsonusage.secondsverbose_jsondurationtext/srt/vttThe file fallback is not an optimisation. Those three formats answer with a body that is not JSON and carries no usage whatsoever, so without it the caller decides whether a request is metered by choosing a response format — the same shape of bypass as an unbilled stream.
Reading the file is metadata-only:
lofty(MIT OR Apache-2.0,default-features = false, 4 new crates, no C dependency) parses container/codec headers without decoding, so an arbitrary caller upload costs microseconds and anything unrecognised yields no duration rather than an error. The probe runs only when the response carried no duration, so the commonjsonpath never pays for it.This is the DP half of #457. cp-api pricing the field is the paired CP change.
Unchanged:
/v1/audio/speech— TTS bills per input character, not by the length of the audio it produced.Baseline
LiteLLM prices transcription as
cost_per_second × duration, taking the duration from the response and falling back to computing it from the file withsoundfilewhen absent (calculate_request_duration). Same two sources, same order; ours differs only in that the fallback is always available rather than an optional dependency.Tests
None.audio_duration_seconds = 11with zero tokens; atextresponse with no usage falls back to the uploaded file's 3s.tests/e2e/src/cases/audio-duration-cost-basis-e2e.test.ts— realaisixbinary + etcd + mock upstreams, asserting both paths through the OTLP fan-out. Fails before this change (the attribute does not exist →NaN).Refs api7/AISIX-Cloud#1138, #457
Summary by CodeRabbit