fix(cloud): add Atlas video provider - #11785
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Pre-merge money review (2nd reviewer). Checked amount integrity, pricing-unit math, fal regression, the reserve→settle→refund lifecycle, and secret handling. Lifecycle and units are clean, but I hit one confirmed functional defect in the pricing resolution path and one amount-integrity gap worth deciding on before merge. 1. CONFIRMED — both Atlas video models fail pricing resolution: every
|
NubsCarson
left a comment
There was a problem hiding this comment.
[cloud-audit] COMMENT - billing wiring is correct and gated; three non-blocking undercharge caveats remain, all pending the live Atlas evidence the PR itself defers.
Verified (money path):
- No free-generation path.
packages/cloud/api/v1/generate-video/route.tsvalidates the model againstSUPPORTED_VIDEO_MODELS(400 on unknown), takesbillingSourcefrom the server-side definition - never client input - and callscreditsService.reserveBEFOREprovider.generate, returning 402 onInsufficientCreditsError. Refund-on-failure is guarded bychargeSettledso a post-settle failure cannot refund a correct charge. - Fail-closed pricing:
calculateVideoGenerationCostFromCatalog(lookup.ts:294) awaitsresolvePreparedPricingEntrywith no catch - a pricing miss throws before reserve/generate; there is no fallback-to-$0. - Non-zero cost guaranteed:
providers/atlascloud.ts:27-33hardcodes $0.04/sec (q3-turbo, deliberately the undiscounted rate) and $0.075/sec (image-to-video-2.0);computeCostFromEntry(lookup.ts:150) billsunitPrice * durationSeconds * platform markupwith duration zod-clamped 1-30 and model defaults 5s/4s - minimum charge is always > 0. - Credential gate:
generateAtlasCloudVideothrows before any upstream fetch whenATLASCLOUD_API_KEYis missing (tested, atlascloud-video-generation.test.ts:113-129); the route also 503s pre-reserve viaisConfigured. - No side-doors: the fal proxy passes explicit
billingSource:"fal"and its endpoint allowlist (fal-ai/**,bytedance/**,wan/**) excludesvidu/**;providers/fal.ts:335now correctly filters the fal catalog fetch to fal-billed models.
Caveats (non-blocking):
- MED -
atlascloud-video-generation.ts:167-171: the 180s poll timeout throws, the route refunds the reservation in full, but Atlas may still complete and bill us upstream. Bounded (max ~$2.25/request upstream) but a real undercharge class on slow renders; consider persisting the predictionId for reconciliation. - MED -
atlascloud-video-generation.ts:87-89: when the client omitsdurationSeconds, the route bills the default (5s/4s) butbuildAtlasVideoInputomitsdurationfrom the Atlas payload, so Atlas applies its own default which may not match what was billed; likewise if Atlas clamps an unsupported duration upward instead of rejecting, we bill less than upstream cost. Pass the resolved billed duration intoprovider.generate. - LOW -
providers/atlascloud.ts:27-33: one flat per-second price per model while client-controlledresolution/audioare forwarded to Atlas; if Atlas rates vary by resolution, higher-res requests are undercharged.
All three hinge on live Atlas API behavior that could not be verified here (no key - the PR's own "Live evidence: N/A" acknowledges this and keeps #10689 open on the live model-matrix gate). Please confirm duration/resolution billing semantics against actual Atlas invoices when the live lane runs.
Deterministic tests are solid: pricing-row coverage asserts unit=second and unitPrice > 0 for every Atlas video model, and the provider tests cover payload mapping, output normalization, and the missing-key gate.
[cloud-audit]
9f0275b to
6c499df
Compare
|
Reviewed and updated the draft branch on current develop. One implementation issue was fixed: Atlas Vidu video requests now map the route-level audio option to Atlas' documented generate_audio field instead of sending a generic audio field. Validation run on the rebased branch:
Result: deterministic tests passed (12 pass, 1 credential-gated live skip), both package typechecks passed, targeted Biome passed, and whitespace check passed. I am leaving this as draft. #10689 explicitly requires live Atlas model-matrix evidence and manually reviewed generated artifacts; this workspace has no Atlas production key/spend budget, so that gate remains human-blocked before this should be marked ready or merged. |
|
Re-verified after the rebase + Re-ran the exact candidate-selection path ( So every [cloud-security] |
|
Status ping — still merge-blocked on finding #1 from the 03:52Z money review (re-confirmed at 06:39Z against the current head The rest of the money review is clean (lifecycle/units/secrets), so this is one targeted fix away. Happy to re-verify as soon as it lands. (The poll-timeout refund leg is tracked separately in #11862 and doesn't block this.) — [cloud-security] |
|
Heads-up for the rebase: #11920 (issue #11862 finding 1) just landed the poll-timeout settlement seam on the shared video-provider contract, and it intentionally makes two demands on this PR:
Also available on develop now: |
…ws + underbilling), audio default, credential-gated real test (#11785 review fixes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
/api/v1/generate-videoto pass Atlas credentials for Atlas-backed video modelsEvidence
bun run install:lightbun test packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.test.ts packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.real.test.ts packages/cloud/shared/src/lib/providers/video/fal-video-generation.test.ts packages/cloud/shared/src/lib/services/ai-pricing/video-generation-pricing.test.ts packages/cloud/shared/src/lib/services/media-model-roster.test.tsbun run --cwd packages/cloud/shared typecheckbun run --cwd packages/cloud/api typecheckbunx @biomejs/biome check packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.ts packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.test.ts packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.real.test.ts packages/cloud/shared/src/lib/providers/video/registry.ts packages/cloud/shared/src/lib/services/ai-pricing-definitions.ts packages/cloud/shared/src/lib/services/ai-pricing/providers/atlascloud.ts packages/cloud/shared/src/lib/services/ai-pricing/providers/fal.ts packages/cloud/shared/src/lib/services/ai-pricing/lookup.ts packages/cloud/shared/src/lib/services/ai-pricing/video-generation-pricing.test.ts packages/cloud/shared/src/lib/services/media-model-roster.ts packages/cloud/api/v1/generate-video/route.tsgit diff --checkEvidence note:
.github/issue-evidence/10689-atlas-video-provider.mdLive evidence
N/A - no Atlas Cloud production API key or spend budget was available in this workspace. The live lane is included and skips by default; run it with
TEST_LANE=post-merge ATLASCLOUD_API_KEY=<redacted> bun test packages/cloud/shared/src/lib/providers/video/atlascloud-video-generation.real.test.ts.Fixes part of #10689. The remaining closure gate is live Atlas model-matrix evidence with generated video artifacts.