Uh oh!
There was an error while loading. Please reload this page.
feat(music): quote the price as an upper bound at fal's rate - #2003
Conversation
Tracks recoupable/api#853. Two changes to the form's quote. Rate: 0.5 -> 0.2 credits per second and no floor, matching the API. A 60s song now quotes $0.12 rather than $0.30. A quote that disagrees with what the API charges is worse than no quote. Wording: "Costs $0.12" -> "Up to $0.12 ... You are charged for the audio actually generated." The API now bills the audio fal produced, and the model routinely stops short of the requested length, so a fixed figure would overstate what most generations are billed. An upper bound is safe in the only direction that matters: nobody is charged more than they were shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017fSvwazBitPfsTQvVqpi8q
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
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 |
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 5/5
- The
components/MusicPage/__tests__/musicPricing.test.tscase named “applies no floor” does not exercise removal of the minimum: its 10s input still produces 2 credits, above the 1-credit floor enforced bycreditCostForDurationand documented inconst.ts; update the fixture to produce a below-floor value or rename and adjust the expectation.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/MusicPage/__tests__/musicPricing.test.ts">
<violation number="1" location="components/MusicPage/__tests__/musicPricing.test.ts:17">
P3: The test named "applies no floor" uses a 10s case whose value (2 credits) sits above the 1-credit minimum that creditCostForDuration still enforces via Math.max(1, ...), and const.ts still documents a floor. The test can't actually detect whether the floor was removed, and the name overstates what it verifies. If you want a test that proves the floor is gone, assert a sub-1-credit-duration case (e.g. creditCostForDuration(4) toBe(1)) or drop the "no floor" wording.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| it("applies the same floor as the API", () => { | ||
| expect(creditCostForDuration(10)).toBe(15); | ||
| it("applies no floor, matching the API", () => { |
There was a problem hiding this comment.
P3: The test named "applies no floor" uses a 10s case whose value (2 credits) sits above the 1-credit minimum that creditCostForDuration still enforces via Math.max(1, ...), and const.ts still documents a floor. The test can't actually detect whether the floor was removed, and the name overstates what it verifies. If you want a test that proves the floor is gone, assert a sub-1-credit-duration case (e.g. creditCostForDuration(4) toBe(1)) or drop the "no floor" wording.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/MusicPage/__tests__/musicPricing.test.ts, line 17:
<comment>The test named "applies no floor" uses a 10s case whose value (2 credits) sits above the 1-credit minimum that creditCostForDuration still enforces via Math.max(1, ...), and const.ts still documents a floor. The test can't actually detect whether the floor was removed, and the name overstates what it verifies. If you want a test that proves the floor is gone, assert a sub-1-credit-duration case (e.g. creditCostForDuration(4) toBe(1)) or drop the "no floor" wording.</comment>
<file context>
@@ -7,21 +7,24 @@ import {
- it("applies the same floor as the API", () => {
- expect(creditCostForDuration(10)).toBe(15);
+ it("applies no floor, matching the API", () => {
+ expect(creditCostForDuration(10)).toBe(2);
});
</file context>
sweetmantech
commented
Aug 25, 2026
Preview verification — chat#2003Verified on The drift this closes, measured on productionapi#853 merged, so production is now charging the new rate while the form still quotes the old one. Read off The API charges at most 12 credits for that request, and less when the model stops short — one of yesterday's test songs asked for 60s, produced 15.6s, and cost 4. So production currently overstates by 2.5x at best and 7.5x at worst. This PR is the fix and should follow api#853 promptly. Documented vs actual
#4 is the one that matters, since the rate is deliberately duplicated client-side so the quote can re-render on a slider drag. Every value the form shows was compared against
Exact agreement across the full 10–300s range, including both ends. ScreenshotsDesktop, Additional Settings open at the 60s default:
Mobile, 390x844:
NoteThe quote is an upper bound by design: the API charges the audio fal actually produces, so the real deduction is usually lower. Verified against a live generation on api#853 — 60s requested, 15.6s produced, 4 credits charged against the 12 quoted. The promise only ever moves in the customer's favour. |
Uh oh!
There was an error while loading. Please reload this page.
Row 8 of the chat#1999 matrix. The client half of recoupable/api#853.
Two changes
Rate.
CREDITS_PER_SECOND0.5 → 0.2 and the 15-credit floor removed, matching the API exactly. fal charges $0.002/s and a credit is $0.01, so 0.2/s is fal's rate with no markup.Wording.
Costs $0.12 for 60s (12 credits)→Up to $0.12 for 60s (12 credits). You are charged for the audio actually generated.api#853 charges for the audio fal actually produced, and the model routinely stops short — our first songs averaged 39.9s against a 60s default. A fixed "Costs" would overstate what most generations are billed. An upper bound is safe in the only direction that matters: nobody is charged more than they were shown.
This is why the earlier "the quoted price must be final" reasoning no longer applies. It existed to stop the quote being a lie; the charge now only ever moves in the customer's favour.
On the duplicated constant
lib/music/const.tsrestates the API's rate rather than fetching it, because the quote has to re-render on every slider drag. That was a deliberate call when the pricing shipped and it still holds — but it means this PR and api#853 have to land together or the form lies. The tests now name api#853 so the next person changing one finds the other.Tests
47/47 green, both suites updated first and confirmed RED.
tscandeslintclean.The form test now asserts the "up to" phrasing and the explanatory line, not just the number, so dropping the caveat while keeping the price would fail.
🤖 Generated with Claude Code
https://claude.ai/code/session_017fSvwazBitPfsTQvVqpi8q
Summary by cubic
Quotes music generation as an upper bound at fal’s rate and aligns the UI with the API. Previously: “Costs $X” at 0.5 credits/s with a 15-credit floor; now: “Up to $X” at 0.2 credits/s with no floor, and we state billing is for audio actually generated (e.g., 60s now shows $0.12).
lib/music/const.tsfor responsive quoting; keep it in sync with the API when pricing changes.Written for commit 02f4e81. Summary will update on new commits.