Throw error for invalid currency conversion inputs with tests

[pavankumar-vh]

Overview

Fix currency conversion functions in common/src/util/currency.ts to throw errors instead of silently returning 0 for invalid inputs, with comprehensive test coverage.

Bug Description

The previous version silently returned 0 when:

• centsPerCredit was 0 or negative (division by zero or negative rate)
• credits or amountInCents was NaN or Infinity

This is dangerous in money-conversion paths because misconfigurations would silently produce wrong values rather than failing loudly. A bad Stripe price setup or calculation error would result in users getting 0 credits/cents without any indication that something went wrong.

Fix

Changed to throw explicit errors with descriptive messages so invalid inputs are caught immediately during development and testing.

if (!(centsPerCredit > 0)) {
  throw new Error(
    `convertCreditsToUsdCents: centsPerCredit must be positive, got ${centsPerCredit}`,
  )
}
if (!Number.isFinite(credits)) {
  throw new Error(
    `convertCreditsToUsdCents: credits must be finite, got ${credits}`,
  )
}

Testing

Added comprehensive test coverage for:

• Normal conversion cases
• Non-positive centsPerCredit (zero, negative)
• NaN inputs (credits, amountInCents)
• Infinity inputs (positive and negative)

All 10 tests pass.

Call Site Audit

Searched for call sites of both functions:

• convertCreditsToUsdCents: No external call sites found in codebase (only used internally)
• convertStripeGrantAmountToCredits: No external call sites found in codebase (only used internally)

Both functions appear to be used internally within the codebase, and throwing errors will help catch configuration issues early.

Files Changed

• common/src/util/currency.ts - Added validation and error throwing
• common/src/util/__tests__/currency.test.ts - Added comprehensive test coverage

Scope

This change only touches common/ which is an approved contribution area per the Contributing Guide.

[codebuff-team]

Thanks for adding tests alongside the fix — that's the right instinct and the test file is clean and readable.

The core issue: these functions live in common/ but are almost certainly consumed by billing/checkout code paths (Stripe grant amounts, credit purchases) that this PR's "call site audit" says it couldn't find. That's a red flag rather than reassurance — if a function used in money-handling has no visible call sites in this mirror, it's because the real call sites are in packages/billing/ or similar, which is out of scope here and not visible to you. Changing these functions to throw instead of returning 0 is a behavioral change that could crash a live request path (e.g., a webhook handler) instead of degrading gracefully, depending on how the caller handles errors. That tradeoff needs to be evaluated by someone who can see the actual call sites, not asserted from the mirror alone.

Substantively, throwing on non-finite/non-positive inputs is a defensible improvement over silently returning 0 — I'd lean toward porting the validation logic itself. But:

1. The PR should not claim confidence about "no external call sites" — that claim isn't verifiable from this repo and shouldn't be part of the justification.
2. Consider whether these should be assertion-style checks (throw only in dev/test) vs. runtime guards that a caller can catch and handle, since billing paths often want to log-and-fallback rather than crash.
3. Minor: missing newline at end of the new test file.

Worth a maintainer's attention to evaluate against the real call sites, but not a drop-in port as-is — needs-work rather than a straightforward port-candidate.