Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Jan 27, 2026

TL;DR

Added billing UI components and updated the Cloud SDK dependency to support new billing features.

What changed?

  • Added several new billing-related components:
    • BillingPlanBadge
    • BillingPlans
    • BillingStatus
    • BillingUsageGauge
    • CurrentBillCard
    • ManageBillingButton
    • UsageCard
  • Added a new billing route for projects
  • Added @radix-ui/react-collapsible dependency
  • Updated @rivet-gg/cloud SDK from version f01ac23 to d2c864d
  • Added a collapsible UI component
  • Updated various dependencies including lightningcss and terser

How to test?

  1. Navigate to a project's billing page
  2. Verify the billing components display correctly
  3. Test the billing plan selection and usage visualization
  4. Verify the manage billing button works as expected

Why make this change?

This change introduces a new billing UI to allow users to view their current billing status, usage metrics, and manage their billing plans directly within the application. The updated Cloud SDK provides the necessary backend support for these new billing features.

Copy link
Contributor Author

jog1t commented Jan 27, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 27, 2026

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4050

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@4050

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4050

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4050

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4050

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4050

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4050

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4050

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4050

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4050

commit: 56694d7

@claude
Copy link

claude bot commented Jan 27, 2026

Pull Request Review: Billing UI Implementation

Overview

This PR adds comprehensive billing UI components to the dashboard, including plan selection, usage tracking, and billing management. The implementation introduces several new React components with usage metrics visualization and integrates with the updated Cloud SDK.


Positive Aspects ✅

  1. Well-structured component architecture - Clean separation of concerns with modular components
  2. Strong type safety - Effective use of TypeScript with proper type definitions
  3. Good UX considerations - Animated metrics, collapsible sections, and responsive design
  4. Efficient data fetching - Uses React Query with proper caching strategies
  5. BigInt handling - Correctly uses BigInt for precise financial calculations

Issues & Concerns

🔴 Critical Issues

1. Arithmetic Error in Billing Calculations (billing-page.tsx:82)

const overage = usage > includedInPlan ? usage - includedInPlan : 0n;
return (overage * pricePerBillionUnits) / 1_000_000_000n;

Problem: Division with BigInt truncates the result, potentially losing precision in financial calculations.

Example: If overage is 500,000,000n and price is 49n:

  • (500_000_000n * 49n) / 1_000_000_000n = 24n (loses 0.5 cents)

Impact: Users could be undercharged or overcharged depending on usage patterns.

Fix: Perform multiplication first, then divide, or add proper rounding:

// Option 1: Round to nearest cent
return (overage * pricePerBillionUnits + 500_000_000n) / 1_000_000_000n;

// Option 2: More explicit rounding
const totalCents = (overage * pricePerBillionUnits) / 1_000_000_000n;
const remainder = (overage * pricePerBillionUnits) % 1_000_000_000n;
return remainder >= 500_000_000n ? totalCents + 1n : totalCents;

2. Division by Zero Risk (usage-card.tsx:75-78)

const calculatePercent = (value: bigint, max: bigint): number => {
  if (max === 0n) return 0;
  return Number((value * 10000n) / max) / 100;
};

Problem: While the function handles max === 0n, the calling code at line 134-137 can still pass 0:

const maxValue = includedInPlan
  ? current > includedInPlan ? current : (includedInPlan * 100n) / 75n
  : current || 1n;  // Falls back to 1n, but current could be 0n initially

Impact: Edge case when current === 0n and !includedInPlan could still trigger division issues.

3. Inconsistent Plan Pricing Documentation (billing.ts:20-36)

The comments state:

  • Free plan: "$5 dollars worth" of actor_awake
  • Pro/Team: "$20 dollars worth" of actor_awake

But the actual values are:

  • Free: 360_000_000n seconds = 100k hours
  • Pro/Team: 1_440_000_000n seconds = 400k hours

With pricing at $0.05 per 1k hours:

  • 100k hours × $0.05/1k = $5 ✓
  • 400k hours × $0.05/1k = $20 ✓

The math is correct, but consider adding explicit price calculations in comments for clarity.

4. Potential Memory Leak (billing-page.tsx:88-89)

const { data } = useSuspenseQuery({
  ...dataProvider.currentProjectBillingDetailsQueryOptions(),
});

If this page is unmounted during data fetch with Suspense, React Query may not properly clean up. Consider adding error boundaries and loading states.

⚠️ High Priority Issues

5. Missing Error Handling (hooks.ts:15-45)

The useAggregatedMetrics function aggregates metrics from multiple namespaces but doesn't handle:

  • Failed queries (some queries might fail while others succeed)
  • Partial data scenarios
  • Network errors

Recommendation:

const aggregated = metricQueries.reduce((acc, query) => {
  if (query.data && !query.error) {
    query.data.forEach((metric) => {
      if (!acc[metric.name]) {
        acc[metric.name] = 0n;
      }
      acc[metric.name] += metric.value;
    });
  }
  return acc;
}, {} as Record<Rivet.namespaces.MetricsGetRequestNameItem, bigint>);

// Check if any queries are still loading or have errors
const hasErrors = metricQueries.some(q => q.error);
const isLoading = metricQueries.some(q => q.isLoading);

6. Race Condition in Plan Changes (billing-plans.tsx:18-25)

The mutation invalidates queries on success, but there's no optimistic update or loading state coordination:

const { mutate, isPending, variables } = useMutation({
  ...dataProvider.changeCurrentProjectBillingPlanMutationOptions(),
  onSuccess: async () => {
    await queryClient.invalidateQueries(
      dataProvider.currentProjectBillingDetailsQueryOptions(),
    );
  },
});

Issue: Between mutation completion and query refetch, the UI shows stale data.

Fix: Add optimistic updates:

onMutate: async (newPlan) => {
  await queryClient.cancelQueries(
    dataProvider.currentProjectBillingDetailsQueryOptions()
  );
  const previousData = queryClient.getQueryData(
    dataProvider.currentProjectBillingDetailsQueryOptions()
  );
  queryClient.setQueryData(
    dataProvider.currentProjectBillingDetailsQueryOptions(),
    (old) => ({ ...old, billing: { ...old.billing, futurePlan: newPlan.plan } })
  );
  return { previousData };
},
onError: (err, newPlan, context) => {
  queryClient.setQueryData(
    dataProvider.currentProjectBillingDetailsQueryOptions(),
    context.previousData
  );
},

7. Type Safety Issue (billing-plans.tsx:45-46)

isLoading: variables?.__from === plan && isPending,

The __from property is not part of the official type definition. This is a code smell indicating improper state tracking.

Better approach: Track the loading state for each plan separately in component state.

8. Security: External Link Without Security Attributes (billing-plans.tsx:70)

window.open("https://www.rivet.dev/sales", "_blank");

Issue: Opening external links without noopener and noreferrer can expose security vulnerabilities.

Fix:

const salesWindow = window.open("https://www.rivet.dev/sales", "_blank");
if (salesWindow) {
  salesWindow.opener = null;
}

Or better, use a proper Link component with security attributes.

📋 Medium Priority Issues

9. Inefficient Data Fetching (hooks.ts:16-29)

The useAggregatedMetrics hook fetches metrics for ALL namespaces on every render where namespaces change. This could be expensive for projects with many namespaces.

Consider:

  • Pagination for metrics fetching
  • Memoization of namespace list
  • Implementing virtual scrolling for large datasets
  • Caching aggregated results

10. Inconsistent Date Handling (billing-page.tsx:128-137)

periodStart={
  data.billing.currentPeriodStart
    ? new Date(data.billing.currentPeriodStart)
    : startOfMonth(new Date())
}

Issue: Mixing API dates with calculated fallbacks can lead to inconsistent billing periods if the API is temporarily unavailable or returns null.

Recommendation: Always use API values or show an error state. Don't silently fall back to calculated dates for billing information.

11. Accessibility Issues (usage-card.tsx:54)

The SVG gauge has aria-label but the interactive elements lack proper ARIA attributes:

<Button variant="ghost" size="icon" className="size-auto p-0">
  <Icon icon={faInfoCircle} />
</Button>

Fix: Add proper labels:

<Button 
  variant="ghost" 
  size="icon" 
  className="size-auto p-0"
  aria-label="More information about month-to-date charges"
>
  <Icon icon={faInfoCircle} />
</Button>

12. Hardcoded Magic Numbers (usage-card.tsx:136)

: (includedInPlan * 100n) / 75n

The 75n appears to be calculating 133% of the included plan (100/75 ≈ 1.33) but this is not documented.

Recommendation: Extract to a named constant with documentation:

// Show gauge up to 133% of included amount for better visualization
const GAUGE_MAX_MULTIPLIER = 75n; // Represents 100/75 = 1.33x
const maxValue = includedInPlan
  ? current > includedInPlan 
    ? current 
    : (includedInPlan * 100n) / GAUGE_MAX_MULTIPLIER
  : current || 1n;

13. Potential Performance Issue (usage-card.tsx:85-101)

The AnimatedMetric component uses Framer Motion for every metric value, which re-animates on every render:

useEffect(() => {
  animate(motionValue, numValue, { duration: 1, ease: "circIn" });
}, [motionValue, numValue]);

Issue: If parent re-renders frequently, animations restart constantly.

Fix: Add proper dependency handling or debouncing.

🔧 Minor Issues

14. Unused Import (billing-page.tsx:4)

import { faPencil } from "@rivet-gg/icons";

This icon is used, so this is fine. ✓

15. Inconsistent String Formatting (usage-card.tsx:24-26)

function stripTrailingZeros(value: number, decimals: number): string {
  return value.toFixed(decimals).replace(/\.?0+$/, "");
}

Consider edge cases:

  • stripTrailingZeros(0, 2) returns "0"
  • stripTrailingZeros(1.0, 2) returns "1"
  • stripTrailingZeros(1.10, 2) returns "1.1"

Looks good! ✓

16. Disabled Collapsible (usage-card.tsx:167)

<Collapsible disabled open={isOpen} onOpenChange={setIsOpen}>

The collapsible is disabled but still tracks open state. This seems like dead code—either enable it or remove the state management.

17. Byte Units Inconsistency (usage-card.tsx:40-42)

Using 1000 for byte conversions (SI units) instead of 1024 (binary units):

const KB = 1000;
const MB = 1000 * 1000;

While this matches cloud provider conventions (AWS, GCP use decimal), consider documenting this choice since it differs from traditional computing conventions (where 1 KB = 1024 bytes).


Missing Elements

🧪 Test Coverage

No tests found for the billing components. Given the financial nature of this feature, comprehensive tests are essential:

Required tests:

  1. Unit tests for calculations:

    • calculateOverageCost with various scenarios
    • formatMetricValue for all metric types
    • calculatePercent edge cases
  2. Integration tests:

    • Plan upgrades/downgrades
    • Metric aggregation across namespaces
    • Billing period transitions
  3. Visual regression tests:

    • Usage gauges at different percentages
    • Plan cards in various states
  4. E2E tests:

    • Complete billing flow
    • Plan change confirmation
    • Error scenarios

📝 Documentation

  • No README or documentation for the billing feature
  • Complex billing calculations lack inline documentation
  • No migration guide for the pricing structure

🔒 Security Considerations

  • No rate limiting on billing API calls
  • No audit logging for plan changes
  • External links lack proper security attributes
  • No validation of billing data before display

Performance Considerations

  1. Bundle Size: New dependencies added (Framer Motion, additional Radix UI components). Consider:

    • Code splitting for billing routes
    • Lazy loading animations
    • Tree shaking verification
  2. Query Performance: Fetching metrics for all namespaces could be slow for large projects. Consider:

    • Pagination
    • Background refresh
    • Stale-while-revalidate strategy
  3. Animation Performance: Multiple animated components on one page. Consider:

    • will-change CSS property for GPU acceleration
    • Reducing animation complexity for low-end devices
    • Respecting prefers-reduced-motion

Recommendations Summary

Immediate Actions Required:

  1. ✅ Fix BigInt division rounding in billing calculations
  2. ✅ Add comprehensive error handling to metric aggregation
  3. ✅ Implement optimistic updates for plan changes
  4. ✅ Add test coverage (at minimum, unit tests for financial calculations)
  5. ✅ Fix security issue with external links

Before Merge:

  1. ✅ Add error boundaries for Suspense components
  2. ✅ Document the pricing structure and calculations
  3. ✅ Add accessibility attributes to interactive elements
  4. ✅ Consider date handling edge cases
  5. ✅ Review type safety issues (__from property)

Post-Merge / Follow-up:

  1. Add E2E tests for billing flows
  2. Implement monitoring/alerting for billing discrepancies
  3. Add audit logging for plan changes
  4. Performance testing with large numbers of namespaces
  5. Consider adding billing documentation for users

Conclusion

This is a solid implementation with good architecture and UX considerations. However, the critical arithmetic issue in billing calculations must be addressed before merge, as it could result in incorrect charges. The lack of test coverage for financial calculations is also a significant concern.

The code quality is generally high, but needs attention to error handling, type safety, and security best practices.

Recommendation: ⚠️ Request Changes - Address critical issues before merge.


Code Style Compliance

  • ✅ TypeScript usage is appropriate
  • ✅ Component structure follows React best practices
  • ✅ File organization is logical
  • ⚠️ Missing tests (per project guidelines, tests should accompany features)
  • ✅ Proper use of React Query patterns
  • ⚠️ Some accessibility improvements needed

Review generated by Claude Code - Let me know if you need clarification on any of these points!

@jog1t jog1t marked this pull request as ready for review January 27, 2026 18:48
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 27, 2026

Graphite Automations

"Test" took an action on this PR • (01/27/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

@jog1t jog1t force-pushed the 01-27-feat_dashboard_add_billing_screen branch from 537aefb to df3af78 Compare January 28, 2026 19:59
@jog1t jog1t force-pushed the 01-26-feat_dashboard_improve_data_providers_stability branch from 2154cae to 46d339e Compare January 28, 2026 19:59
@jog1t jog1t force-pushed the 01-27-feat_dashboard_add_billing_screen branch from df3af78 to 56694d7 Compare January 29, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants