-
Notifications
You must be signed in to change notification settings - Fork 3.1k
improvement(billing): migrate to decimaljs from number.parseFloat #2588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR migrates billing calculations from Key changes:
Implementation pattern: Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant BillingAPI
participant Decimal Utils
participant Database
Client->>BillingAPI: Request billing summary
BillingAPI->>Database: Fetch user/org usage data
Database-->>BillingAPI: Return decimal strings
BillingAPI->>Decimal Utils: toDecimal(value)
Decimal Utils-->>BillingAPI: Decimal object
BillingAPI->>Decimal Utils: Perform calculations (plus, minus, max)
Decimal Utils-->>BillingAPI: Decimal results
BillingAPI->>Decimal Utils: toNumber(decimal)
Decimal Utils-->>BillingAPI: Number for response
BillingAPI-->>Client: Return billing summary
Client->>BillingAPI: Deduct credits
BillingAPI->>Decimal Utils: toDecimal(cost)
Decimal Utils-->>BillingAPI: Decimal object
BillingAPI->>Decimal Utils: toFixedString(cost)
Decimal Utils-->>BillingAPI: String for SQL
BillingAPI->>Database: Execute SQL with decimal string
Database-->>BillingAPI: Return old/new balance
BillingAPI->>Decimal Utils: toDecimal(oldBalance)
Decimal Utils-->>BillingAPI: Process comparison
BillingAPI->>Decimal Utils: toNumber(result)
Decimal Utils-->>BillingAPI: Final result
BillingAPI-->>Client: Return deduction result
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 files reviewed, 2 comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
apps/sim/lib/billing/core/billing.ts, line 70 (link)logic: inconsistent with decimal.js migration - uses
Math.maxon regular numbers instead of maintaining decimal precision
5 files reviewed, 2 comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
apps/sim/lib/billing/credits/balance.ts, line 165 (link)style: inconsistent precision handling -
costandcreditsUsedare already numbers (converted from Decimal earlier), but this calculation should ideally use Decimal throughout to avoid potential precision loss before the final conversionNote: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
5 files reviewed, 1 comment
Summary
Type of Change
Testing
Tested manually
Checklist