[REF-1467] fix: dynamic billing bugs for Kling/Wan/Fish/Fal/NanoBanana tools

[alchemistklk]

Summary

• Fix 6 code-level bugs in the dynamic billing engine affecting 10 tools (REF-1459–REF-1468)
• Add oneOf/anyOf schema support with path-aware branch selection in getFieldTypeFromSchema
• Fix pricing tier matching with strict-decimal-only coercion (matchesTierValue)
• Add fail-closed unitField validation with phase-aware lazy schema parsing
• Add video category to extractAndAggregateValue array aggregation
• Add per-rule finite validation with actionable error context
• Fix processBilling: change discountedPrice > 0 → !== undefined, default from 1 → 0, add source flags + warning policy, validate inputs, add finiteness guards

Test plan

• 73 calculation tests passing (26 new tests in suites 12–16)
• 15 service-level tests passing (new billing.service.spec.ts)
• All 88 tests pass together
• Type-check (pre-existing TS6305 monorepo build order issues unrelated to billing)
• CI pipeline validation

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed default pricing calculation to prevent unintended charges when no billing configuration is present.
  • Improved validation of direct price inputs to reject invalid values.
  • Enhanced handling of edge cases (NaN, Infinity) in billing calculations.

• Improvements

  • Strengthened free tool access override logic.
  • Improved schema validation for complex billing configurations.

[linear]

REF-1467 【Kling/kling-o1-image-to-video】按秒计费没生效，5秒/10秒视频计费都是81分

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors the billing service to validate direct pricing inputs, introduce explicit billing source tracking, and enhance schema-aware billing calculations. Adds comprehensive test suites covering pricing paths, dynamic and legacy billing fallbacks, free toolset handling, and schema traversal scenarios.

Changes

Cohort / File(s)	Summary
BillingService Tests apps/api/src/modules/tool/billing/billing.service.spec.ts	New comprehensive test suite for processBilling covering direct pricing, dynamic billing via calculateCreditsFromRules, legacy per-call billing fallback, no-charge configs, missing billing source warnings, free toolset behavior, NaN/Infinity handling, and default pricing scenarios.
BillingService Implementation apps/api/src/modules/tool/billing/billing.service.ts	Refactored pricing logic with default costs initialized to 0, explicit validation for direct price inputs (finite and non-negative), new state flags for billing source tracking (resolvedFromDirectPrice, resolvedFromLegacyBilling, resolvedFromExplicitNoCharge), adjusted direct-price path handling, explicit no-charge override, dynamic billing gating, and diagnostic warnings for missing billing sources.
Billing Calculation Tests apps/api/src/modules/tool/utils/billing-calculation.spec.ts	Expanded test coverage with floating-point tolerant assertions, new suites for oneOf/anyOf schema traversal, tier coercion with string-numeric matching, video category array aggregation, and unitField validation edge cases.
Billing Calculation Implementation apps/api/src/modules/tool/utils/billing-calculation.ts	Enhanced billing calculation with strict decimal-based tier value matching (DECIMAL_RE and matchesTierValue), unit-field schema validation for additive rules, per-rule finite validation, extended aggregation for video categories, and utilities for complex JSON Schema handling (getSchemaNodeType, parsePathToken, resolveCompositeForPath, path-aware composite resolution).

Sequence Diagram

sequenceDiagram
    participant Client
    participant BillingService
    participant DirectPrice as Direct Price<br/>Validator
    participant DynamicCalc as Dynamic<br/>Calculator
    participant LegacyBilling as Legacy<br/>Billing
    participant Cache as Redis<br/>Cache

    Client->>BillingService: processBilling(input)
    
    activate BillingService
    BillingService->>DirectPrice: Validate direct price input
    alt Direct price provided & valid
        DirectPrice-->>BillingService: finalDiscountedPrice set
        BillingService->>BillingService: resolvedFromDirectPrice = true
    else Direct price invalid
        DirectPrice-->>BillingService: error (invalid finite value)
    else No direct price
        BillingService->>BillingService: Check explicit no-charge
        alt No-charge enabled
            BillingService->>BillingService: resolvedFromExplicitNoCharge = true
        else Try dynamic billing
            BillingService->>DynamicCalc: calculateCreditsFromRules(schemas)
            alt Dynamic succeeds
                DynamicCalc-->>BillingService: finalDiscountedPrice updated
                BillingService->>BillingService: resolvedFromDynamic = true
            else Dynamic fails
                BillingService->>LegacyBilling: calculateCredits fallback
                LegacyBilling->>Cache: Query accumulator
                Cache-->>LegacyBilling: usage data
                LegacyBilling-->>BillingService: legacy credits
                BillingService->>BillingService: resolvedFromLegacyBilling = true
            end
        end
    end
    
    BillingService->>BillingService: Apply free access override
    BillingService->>BillingService: Final validation (non-finite → 0)
    BillingService-->>Client: pricing result with source flags
    deactivate BillingService

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

• fix: missing billing info for composio-based tools #1772 — Modifies BillingService.processBilling to add optional resultId/version plumbing, directly intersecting with the main PR's pricing/validation refactor.
• [REF-1454] feat: Add toolCallId linkage for PTC billing tracking #2238 — Also modifies BillingService.processBilling to add toolCallId propagation, which runs parallel to the main PR's billing source logic changes.
• fix: restore PTC billing linkage and improve script result targeting #2249 — Touches BillingService.processBilling for toolCallId propagation into function calls, complementary to the main PR's internal pricing/validation refactor.

Suggested Reviewers

• nettee
• mrcfps

Poem

🐰 A hop-hop-hop through billing's hall,
Where prices dance and schemas sprawl,
Direct or dynamic, legacy too,
Free toolsets hop—no charge for you!
Validation guards each path we take,
No silent charges left to make! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing dynamic billing bugs for multiple specific tools (Kling/Wan/Fish/Fal/NanoBanana), directly matching the core objective of resolving billing engine issues as described in the PR summary and raw code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/REF-1467-fix-dynamic-bil

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/api/src/modules/tool/billing/billing.service.ts (1)

481-488: Consider using the _appliedFreeAccessOverride flag.

The _appliedFreeAccessOverride variable is assigned but not used. If this is intended for future logging or debugging, consider adding it to the audit trail or removing the underscore prefix if it should be used.

💡 Suggestion: Use the flag for observability

       let _appliedFreeAccessOverride = false;
       if (FREE_TOOLSET_KEYS.includes(toolsetKey) && finalDiscountedPrice > 0) {
         const hasFreeToolAccess = await this.checkFreeToolAccess(uid);
         if (hasFreeToolAccess) {
           _appliedFreeAccessOverride = true;
           finalDiscountedPrice = 0;
+          this.logger.debug(
+            `Free access override applied for ${toolsetKey}:${toolName}, uid=${uid}`,
+          );
         }
       }

🤖 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 481 - 488,
The variable _appliedFreeAccessOverride is set when FREE_TOOLSET_KEYS includes
toolsetKey and checkFreeToolAccess(uid) returns true but never used; either
remove it entirely or surface it for observability by passing it into the
existing audit/log call (or adding one) alongside finalDiscountedPrice and
toolsetKey so callers can see that free-access override was applied; if you
intend it as a public flag rename it (appliedFreeAccessOverride) and include it
in the payload sent to the audit/event method or in processLogger.debug to avoid
dead/underscored variables.

🤖 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.ts`:
- Around line 481-488: The variable _appliedFreeAccessOverride is set when
FREE_TOOLSET_KEYS includes toolsetKey and checkFreeToolAccess(uid) returns true
but never used; either remove it entirely or surface it for observability by
passing it into the existing audit/log call (or adding one) alongside
finalDiscountedPrice and toolsetKey so callers can see that free-access override
was applied; if you intend it as a public flag rename it
(appliedFreeAccessOverride) and include it in the payload sent to the
audit/event method or in processLogger.debug to avoid dead/underscored
variables.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba7671 and 592635f.

📒 Files selected for processing (4)

• apps/api/src/modules/tool/billing/billing.service.spec.ts
• apps/api/src/modules/tool/billing/billing.service.ts
• apps/api/src/modules/tool/utils/billing-calculation.spec.ts
• apps/api/src/modules/tool/utils/billing-calculation.ts

[alchemistklk]

Code review

Found 2 issues:

1. Test mock client.eval argument indices are wrong — the mock reads args[2] (microCredits) and args[5] (scale), but the production client.eval call passes microCredits at index 4 and MICRO_CREDIT_SCALE at index 7. The mock actually reads accumulatorKey (a Redis key string) and idempotencyKey, both of which Number() converts to NaN. This means the accumulator flush and idempotency deduplication logic is not actually being tested — all mock computations silently produce NaN.

refly/apps/api/src/modules/tool/billing/billing.service.spec.ts

Lines 124 to 130 in 592635f

	exists: jest.fn().mockResolvedValue(1),
	eval: jest.fn().mockImplementation(async (...args: any[]) => {
	const microCredits = Number(args[2]);
	const scale = Number(args[5]);
	const flushCredits = Math.floor(microCredits / scale);
	const remainder = microCredits - flushCredits * scale;
	return [flushCredits, remainder, 0]; // [flush, remainder, replayed]

Production call for reference (note argument positions):

refly/apps/api/src/modules/tool/billing/billing.service.ts

Lines 672 to 683 in 592635f

	`;
	const result = (await client.eval(
	script,
	2,
	accumulatorKey,
	idempotencySetKey,
	String(microCredits),
	idempotencyKey,
	String(ACCUMULATOR_TTL_SECONDS),
	String(MICRO_CREDIT_SCALE),
	)) as [number, number, number];

2. _appliedFreeAccessOverride is declared and assigned but never read, logged, or returned. The underscore prefix suppresses linter warnings, but the variable serves no purpose — it does not influence control flow, appear in logs, or get included in the return value. This looks like incomplete scaffolding from the refactor.

refly/apps/api/src/modules/tool/billing/billing.service.ts

Lines 480 to 488 in 592635f

	// Apply toolset-based free access (specific toolsets are one month free)
	let _appliedFreeAccessOverride = false;
	if (FREE_TOOLSET_KEYS.includes(toolsetKey) && finalDiscountedPrice > 0) {
	const hasFreeToolAccess = await this.checkFreeToolAccess(uid);
	if (hasFreeToolAccess) {
	_appliedFreeAccessOverride = true;
	finalDiscountedPrice = 0;
	}
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[alchemistklk]

✅ Claude Code Reviewer (PASS) for REF-1467

No blocking findings were reported in the latest review run.

Generated by openclio monitor.