[REF-1460][REF-1462][REF-1466] fix: tool billing floor, STT fieldPath, and kling routing#2260
Conversation
…, and kling routing - Add minimum 1 credit floor for dynamic billing when sub-credit amounts would never reach the micro-credit accumulator flush threshold (REF-1460) - Fix fish_audio STT billing rule fieldPath from "data.duration" to "duration" to match actual response schema (REF-1462, DB fix) - Update kling-o1-image-to-video description to include "O1" for proper LLM tool selection (REF-1466, DB fix) - Add 4 new tests for minimum credit floor edge cases REF-1465 (wan-video-to-video 405): Investigated - endpoint config is correct per fal.ai docs. Needs production log analysis for root cause. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request adds a minimum credit floor of 1 to dynamic billing calculations in the billing service. When dynamic pricing yields a positive value below 1, it is floored to 1 credit. Zero and values ≥ 1 remain unchanged. New test cases validate the flooring behavior across different calculation scenarios. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/api/src/modules/tool/billing/billing.service.ts (1)
423-429: Extract the minimum dynamic charge into a named constant.This logic is correct, but the hardcoded
1policy value appears inline. Defining a constant keeps billing policy centralized and reduces drift risk.♻️ Proposed refactor
const PROVIDER_BILLING_CACHE_TTL_MS = 5 * 60 * 1000; const MICRO_CREDIT_SCALE = 1_000_000; const ACCUMULATOR_TTL_SECONDS = 24 * 60 * 60; +const MIN_DYNAMIC_CREDIT_FLOOR = 1; ... - if (finalDiscountedPrice > 0 && finalDiscountedPrice < 1) { - finalDiscountedPrice = 1; + if ( + finalDiscountedPrice > 0 && + finalDiscountedPrice < MIN_DYNAMIC_CREDIT_FLOOR + ) { + finalDiscountedPrice = MIN_DYNAMIC_CREDIT_FLOOR; }As per coding guidelines, "Avoid magic numbers and strings - use named constants in TypeScript/JavaScript."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/billing/billing.service.ts` around lines 423 - 429, Introduce a named constant for the minimum per-tool dynamic charge instead of the inline literal `1`: define (and export if appropriate) a constant like `MIN_DYNAMIC_TOOL_CHARGE = 1` near the top of the Billing service or in the module-level billing policy/constants file, then replace the hardcoded checks that reference `1` (e.g., the `finalDiscountedPrice` floor in billing.service where `finalDiscountedPrice > 0 && finalDiscountedPrice < 1` is set) with `MIN_DYNAMIC_TOOL_CHARGE`, and ensure any related unit tests or documentation reference the new constant.apps/api/src/modules/tool/billing/billing.service.spec.ts (1)
410-457: Consider parameterizing tests 17–19 to reduce repetition.These cases share identical setup/assertion shape and are a good fit for
test.each.🧹 Optional refactor
- test('17: Zero dynamic billing stays at 0 (no minimum applied)', async () => { - calculateCreditsFromRules.mockResolvedValueOnce(0); - const result = await service.processBilling( - baseOptions({ - input: { text: '' }, - output: {}, - requestSchema: '{"type":"object","properties":{"text":{"type":"string"}}}', - responseSchema: '{"type":"object","properties":{}}', - }), - ); - expect(result.success).toBe(true); - expect(result.discountedPrice).toBe(0); - }); - - test('18: Dynamic billing at exactly 1 stays at 1 (floor not applied)', async () => { - calculateCreditsFromRules.mockResolvedValueOnce(1); - const result = await service.processBilling( - baseOptions({ - input: { text: 'medium text' }, - output: {}, - requestSchema: '{"type":"object","properties":{"text":{"type":"string"}}}', - responseSchema: '{"type":"object","properties":{}}', - }), - ); - expect(result.success).toBe(true); - expect(result.discountedPrice).toBe(1); - }); - - test('19: Dynamic billing above 1 (1.5) stays unchanged', async () => { - calculateCreditsFromRules.mockResolvedValueOnce(1.5); - const result = await service.processBilling( - baseOptions({ - input: { text: 'longer text' }, - output: {}, - requestSchema: '{"type":"object","properties":{"text":{"type":"string"}}}', - responseSchema: '{"type":"object","properties":{}}', - }), - ); - expect(result.success).toBe(true); - expect(result.discountedPrice).toBe(1.5); - }); + test.each([ + { dynamicCredits: 0, expected: 0, text: '' }, + { dynamicCredits: 1, expected: 1, text: 'medium text' }, + { dynamicCredits: 1.5, expected: 1.5, text: 'longer text' }, + ])( + 'dynamic billing boundary: $dynamicCredits -> $expected', + async ({ dynamicCredits, expected, text }) => { + calculateCreditsFromRules.mockResolvedValueOnce(dynamicCredits); + const result = await service.processBilling( + baseOptions({ + input: { text }, + output: {}, + requestSchema: '{"type":"object","properties":{"text":{"type":"string"}}}', + responseSchema: '{"type":"object","properties":{}}', + }), + ); + expect(result.success).toBe(true); + expect(result.discountedPrice).toBe(expected); + }, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/modules/tool/billing/billing.service.spec.ts` around lines 410 - 457, Parameterize tests 17–19 by replacing the three near-identical test blocks with a single test.each table that feeds dynamic billing input and expected discountedPrice; use the existing calculateCreditsFromRules.mockResolvedValueOnce values (0, 1, 1.5) and call service.processBilling with baseOptions (keeping the same input/output/requestSchema/responseSchema shapes), then assert result.success and result.discountedPrice for each row. Locate the blocks referencing service.processBilling, calculateCreditsFromRules.mockResolvedValueOnce, and baseOptions and convert them into a single test.each with rows for [0, 0], [1, 1], and [1.5, 1.5] to remove duplication while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/modules/tool/billing/billing.service.spec.ts`:
- Around line 410-457: Parameterize tests 17–19 by replacing the three
near-identical test blocks with a single test.each table that feeds dynamic
billing input and expected discountedPrice; use the existing
calculateCreditsFromRules.mockResolvedValueOnce values (0, 1, 1.5) and call
service.processBilling with baseOptions (keeping the same
input/output/requestSchema/responseSchema shapes), then assert result.success
and result.discountedPrice for each row. Locate the blocks referencing
service.processBilling, calculateCreditsFromRules.mockResolvedValueOnce, and
baseOptions and convert them into a single test.each with rows for [0, 0], [1,
1], and [1.5, 1.5] to remove duplication while preserving behavior.
In `@apps/api/src/modules/tool/billing/billing.service.ts`:
- Around line 423-429: Introduce a named constant for the minimum per-tool
dynamic charge instead of the inline literal `1`: define (and export if
appropriate) a constant like `MIN_DYNAMIC_TOOL_CHARGE = 1` near the top of the
Billing service or in the module-level billing policy/constants file, then
replace the hardcoded checks that reference `1` (e.g., the
`finalDiscountedPrice` floor in billing.service where `finalDiscountedPrice > 0
&& finalDiscountedPrice < 1` is set) with `MIN_DYNAMIC_TOOL_CHARGE`, and ensure
any related unit tests or documentation reference the new constant.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/modules/tool/billing/billing.service.spec.tsapps/api/src/modules/tool/billing/billing.service.ts
Summary
processBilling()when dynamic billing resolves to a positive sub-credit amount (0 < credits < 1). Without this floor, short TTS inputs produce micro-credits that never reach the accumulator flush threshold.fieldPathfrom"data.duration"to"duration"intool_billingtable — the response schema hasdurationat top level, not underdata.. Schema validation was failing → falling back to legacy 50-credit billing.kling-o1-image-to-videodescription to explicitly mention "Kling O1" so the LLM can correctly match@kling-o1-image-to-videoto the right tool.DB Changes (applied to local + dev)
tool_billing(local):fish_audio/speech-to-textfieldPath"data.duration"→"duration"tool_methods(local + dev):kling-o1-image-to-videodescription updated to include "O1"Test plan
getFieldTypeFromSchema()with STT schema, kling description differentiation@kling-o1-image-to-videoroutes correctly after description update on prod🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests