Conversation
Greptile SummaryThis PR implements a new "Volume Tiered" (
Confidence Score: 5/5Safe to merge — the new volume pricing mode is implemented correctly end-to-end with no defects found. The volume-pricing logic in cost_calc.go correctly iterates tiers in order and applies the single matched tier's price to all tokens, which is exactly what the mode description promises. Validation and equality checks share the existing TieredPricing code path cleanly. Frontend components (type union, Zod schema, superRefine validation, tier UI) are updated symmetrically in both affected components. Two integration tests cover the primary scenarios (first-tier match and overflow to the unbounded tier). No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[computeItemSubtotal
quantity, pricing] --> B{pricing.Mode?}
B -->|flat_fee| C[return FlatFee]
B -->|usage_per_unit| D[return UsagePerUnit x tokens/1M]
B -->|usage_tiered| E[Tiered: iterate tiers
bill each segment separately]
B -->|usage_volume NEW| F[Volume: iterate tiers
find first matching tier]
B -->|default| G[return Zero]
E --> E1{tierUnits > 0?}
E1 -->|yes| E2[add tier.price x tierUnits/1M
to running total]
E2 --> E3{quantity <= tier.UpTo
or UpTo=nil?}
E3 -->|yes| E4[break - return total]
E3 -->|no| E1
E1 -->|no| E3
F --> F1{tier.UpTo != nil
& quantity <= tier.UpTo?}
F1 -->|yes| F3[matchedIdx = i, break]
F1 -->|no| F2{tier.UpTo == nil?}
F2 -->|yes| F3
F2 -->|no| F1
F3 --> F4[sub = matchedTier.price x ALL_tokens/1M
TierBreakdown = single entry]
F4 --> F5[return sub]
Reviews (2): Last reviewed commit: "feat: add volume price mode, close #1582" | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review
This pull request introduces a new pricing mode, usage_volume, which applies a single unit price to all tokens based on the total usage volume. The changes include updates to the frontend price editor, Zod schemas, and localization files, as well as backend logic for cost calculation and validation. My feedback focuses on two main areas: first, the cost calculation logic in internal/server/biz/cost_calc.go currently assumes that price tiers are sorted by their upTo value, which is not guaranteed and could lead to incorrect billing; I recommend adding a sorting step before processing. Second, in the frontend model-price-editor.tsx, the repeated conditional check for tiered pricing modes should be extracted into a helper function or type guard to improve maintainability and readability.
|
|
||
| return item, decimal.Zero | ||
| case objects.PricingModeVolume: | ||
| if pricing.UsageTiered != nil { |
There was a problem hiding this comment.
The current implementation for both PricingModeVolume and PricingModeTiered assumes that the price tiers are sorted by their upTo value. However, there's no guarantee that the tiers will be sorted, which could lead to incorrect price calculations.
To prevent this potential bug, you should sort the tiers before processing them. You can add this sorting logic at the beginning of the handling for both PricingModeVolume and PricingModeTiered.
Here's a suggestion for sorting:
import "sort"
// ...
// Add this at the beginning of the block for `PricingModeVolume` and `PricingModeTiered`
sort.Slice(pricing.UsageTiered.Tiers, func(i, j int) bool {
tierI := pricing.UsageTiered.Tiers[i]
tierJ := pricing.UsageTiered.Tiers[j]
if tierI.UpTo == nil {
return false // nil is considered infinity, so it should be at the end
}
if tierJ.UpTo == nil {
return true
}
return *tierI.UpTo < *tierJ.UpTo
})This will ensure the tiers are always processed in the correct order.
|
|
||
| useEffect(() => { | ||
| if (pricingMode === 'usage_tiered' && tierFields.length === 0) { | ||
| if ((pricingMode === 'usage_tiered' || pricingMode === 'usage_volume') && tierFields.length === 0) { |
There was a problem hiding this comment.
The condition (pricingMode === 'usage_tiered' || pricingMode === 'usage_volume') is used in multiple places within this file (in both PriceItemRow and PriceVariantRow). To improve code clarity and maintainability, consider extracting this logic into a helper function.
For example, you could define a type guard at the module level:
const isTieredPricingMode = (mode?: PricingMode): mode is 'usage_tiered' | 'usage_volume' =>
mode === 'usage_tiered' || mode === 'usage_volume';Then, you can simplify the conditional checks:
// In useEffect
if (isTieredPricingMode(pricingMode) && tierFields.length === 0) {
// ...
}
// In JSX
{isTieredPricingMode(pricingMode) && (
<div>...</div>
)}This makes the code more readable and easier to modify if new tiered pricing modes are introduced in the future.
No description provided.