fix: 🐛 only need one of amount or flat_amount or both#84
fix: 🐛 only need one of amount or flat_amount or both#84
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
- There is a concrete runtime regression risk in
atmn/src/compose/models/planModels.ts:VolumeTiernow permitsflatAmount-only tiers in TypeScript, but the Zod schema still requiresamount, so valid typed inputs can fail validation at runtime. - Because this is a high-severity, high-confidence mismatch (7/10, 9/10) in model validation behavior, this carries meaningful user-facing risk even though the change scope appears limited.
- Pay close attention to
atmn/src/compose/models/planModels.ts- align the Zod schema with the updatedVolumeTiershape to prevent runtime rejects of type-safe data.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="atmn/src/compose/models/planModels.ts">
<violation number="1" location="atmn/src/compose/models/planModels.ts:261">
P1: The new `VolumeTier` type allows `flatAmount`-only tiers, but runtime Zod schema still requires `amount`, causing type-safe code to fail validation at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Volume tier: at least one of amount or flatAmount must be present | ||
| type VolumeTier = | ||
| | { to: number | "inf"; amount: number; flatAmount?: number } | ||
| | { to: number | "inf"; amount?: number; flatAmount: number }; |
There was a problem hiding this comment.
P1: The new VolumeTier type allows flatAmount-only tiers, but runtime Zod schema still requires amount, causing type-safe code to fail validation at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At atmn/src/compose/models/planModels.ts, line 261:
<comment>The new `VolumeTier` type allows `flatAmount`-only tiers, but runtime Zod schema still requires `amount`, causing type-safe code to fail validation at runtime.</comment>
<file context>
@@ -255,14 +255,19 @@ type PriceWithGraduatedTiers = PriceBaseFields & {
+// Volume tier: at least one of amount or flatAmount must be present
+type VolumeTier =
+ | { to: number | "inf"; amount: number; flatAmount?: number }
+ | { to: number | "inf"; amount?: number; flatAmount: number };
+
+// Price with volume tiered pricing (the tier the total usage falls into applies to all units)
</file context>
| type VolumeTier = | ||
| | { to: number | "inf"; amount: number; flatAmount?: number } | ||
| | { to: number | "inf"; amount?: number; flatAmount: number }; |
There was a problem hiding this comment.
The VolumeTier type template (lines 248–250) correctly enforces that at least one of amount or flatAmount must be present. However, the Zod schema that backs runtime validation (UsageTierSchema in planModels.ts lines 7–10) does not reflect this constraint — it requires amount unconditionally and lacks flatAmount entirely.
Since atmnTypeHelpers.ts is the source template for the TypeScript type portion of planModels.ts, the corresponding Zod schema in the generator (TypeGenerator.ts line 334) must be updated in parallel to keep types and runtime validation in sync. Without this, consumers will write TypeScript code that passes the type checker but fails at runtime.
See the companion issue in TypeGenerator.ts line 334 for the schema update needed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: typegen/genUtils/atmnTypeHelpers.ts
Line: 248-250
Comment:
The `VolumeTier` type template (lines 248–250) correctly enforces that at least one of `amount` or `flatAmount` must be present. However, the Zod schema that backs runtime validation (`UsageTierSchema` in `planModels.ts` lines 7–10) does not reflect this constraint — it requires `amount` unconditionally and lacks `flatAmount` entirely.
Since `atmnTypeHelpers.ts` is the source template for the TypeScript type portion of `planModels.ts`, the corresponding Zod schema in the generator (`TypeGenerator.ts` line 334) must be updated in parallel to keep types and runtime validation in sync. Without this, consumers will write TypeScript code that passes the type checker but fails at runtime.
See the companion issue in `TypeGenerator.ts` line 334 for the schema update needed.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (2)
At runtime, a volume tier object like The schema injection (line 334) should be updated to allow Prompt To Fix With AIThis is a comment left during a code review.
Path: typegen/genUtils/TypeGenerator.ts
Line: 334
Comment:
The hardcoded `UsageTierSchema` in the `detectRequiredImports` method (which gets injected into the generated `planModels.ts`) only defines `to` and `amount`, but the new `VolumeTier` TypeScript type in this PR allows `amount` to be optional when `flatAmount` is provided.
At runtime, a volume tier object like `{ to: 100, flatAmount: 50 }` (no `amount`) will satisfy the TypeScript type but fail Zod runtime validation because `amount` is still required in the schema.
The schema injection (line 334) should be updated to allow `flatAmount` and make `amount` optional:
```suggestion
imports.push(`export const UsageTierSchema = z.object({\n to: z.union([z.number(), z.literal("inf")]),\n amount: z.number().optional(),\n flatAmount: z.number().optional(),\n}).refine(obj => obj.amount !== undefined || obj.flatAmount !== undefined, {\n message: "At least one of amount or flatAmount must be present",\n});`);
```
How can I resolve this? If you propose a fix, please make it concise.
This creates a type/schema mismatch: an object like The Alternatively, if this schema is auto-generated from a shared source, the generator template in Prompt To Fix With AIThis is a comment left during a code review.
Path: atmn/src/compose/models/planModels.ts
Line: 7-10
Comment:
The `VolumeTier` TypeScript type (lines 259–261) correctly allows objects where `amount` is absent and only `flatAmount` is provided. However, the `UsageTierSchema` Zod object (lines 7–10) still defines `amount: z.number()` as required and has no `flatAmount` field at all.
This creates a type/schema mismatch: an object like `{ to: 100, flatAmount: 50 }` will pass TypeScript's type check but will fail Zod runtime validation when validating the `tiers` field (line 53).
The `UsageTierSchema` (lines 7–10) should be updated to align with the new `VolumeTier` type:
```suggestion
export const UsageTierSchema = z.object({
to: z.union([z.number(), z.literal("inf")]),
amount: z.number().optional(),
flatAmount: z.number().optional(),
}).refine(obj => obj.amount !== undefined || obj.flatAmount !== undefined, {
message: "At least one of amount or flatAmount must be present",
});
```
Alternatively, if this schema is auto-generated from a shared source, the generator template in `typegen/genUtils/TypeGenerator.ts` (line 334) must be updated along with the shared schema source.
How can I resolve this? If you propose a fix, please make it concise. |
Summary by cubic
Fixes tier validation so volume tiers can use amount, flatAmount, or both, and clarifies graduated vs volume behavior with stricter types. Standardizes the property name to tierBehavior and updates docs.
Bug Fixes
Migration
Written for commit 4b6532c. Summary will update on new commits.
Greptile Summary
This PR improves the type definitions for volume-tiered pricing to correctly model that either
amountorflatAmount(or both) are valid per-tier, rather than requiringamountunconditionally. It also renames the old singlePriceWithTiersinto two more precise types (PriceWithGraduatedTiersandPriceWithVolumeTiers) and corrects the British-spellingtierBehaviourtotierBehavior.Key changes:
atmnTypeHelpers.ts/planModels.ts: The oldPriceWithTierstype (with a singletierBehaviour: "graduated" | "volume") is replaced by a discriminated union:PriceWithGraduatedTiers(always requiresamountper tier) andPriceWithVolumeTiers(uses the newVolumeTierunion type that acceptsamount,flatAmount, or both). TheVolumeTierunion correctly enforces that at least one of the two must be present. Note: this is a breaking change for callers who usedtierBehaviour(British spelling).TypeGenerator.ts:TierBehavioris added to the enum-to-literal-union replacement map, ensuring the corrected spelling is handled during code generation alongside the legacyTierBehaviourskey.planModels.ts: Thetiersfield JSDoc description is trimmed, removing the now-stale note abouttonot including included amounts; graduated-tier comment is made more precise.Critical issue: The
UsageTierSchemaZod object (auto-generated, used for runtime validation oftiers) still definesamount: z.number()as a required field and has noflatAmountfield at all. The new TypeScriptVolumeTiertype permits objects whereamountis absent and onlyflatAmountis provided — those objects will pass TypeScript's type checker but fail Zod runtime validation. The underlying Zod schema (hardcoded inTypeGenerator.tsline 334 and output toplanModels.tslines 7–10) must be updated to include an optionalflatAmountfield and makeamountoptional, with a.refine()check to enforce at least one is present.Confidence Score: 2/5
atmnTypeHelpers.tsare correct and will enable the desired flexibility for volume tiers (allowingamountto be optional whenflatAmountis present). However, the companion Zod schema used for runtime validation was not updated to match. Any volume tier object that passes the new TypeScript type but lacksamount(e.g.,{ to: 100, flatAmount: 50 }) will fail Zod validation at runtime, leaving the fix incomplete and potentially misleading to SDK consumers.typegen/genUtils/TypeGenerator.ts(line 334 — hardcoded UsageTierSchema),atmn/src/compose/models/planModels.ts(lines 7–10 — generated UsageTierSchema)Class Diagram
%%{init: {'theme': 'neutral'}}%% classDiagram class PriceBaseFields { +billingMethod: BillingMethod +billingUnits?: number +maxPurchase?: number } class PriceWithAmount { +amount: number +tiers?: never } class PriceWithGraduatedTiers { +amount?: never +tiers: Array~GraduatedTier~ +tierBehavior: "graduated" } class GraduatedTier { +to: number | "inf" +amount: number } class PriceWithVolumeTiers { +billingMethod: Exclude~BillingMethod,"usage_based"~ +amount?: never +tiers: Array~VolumeTier~ +tierBehavior: "volume" } class VolumeTier { +to: number | "inf" +amount?: number +flatAmount?: number } note for VolumeTier "At least one of amount or flatAmount required\n⚠ UsageTierSchema requires amount (no flatAmount)" PriceBaseFields <|-- PriceWithAmount PriceBaseFields <|-- PriceWithGraduatedTiers PriceBaseFields <|-- PriceWithVolumeTiers PriceWithGraduatedTiers --> GraduatedTier PriceWithVolumeTiers --> VolumeTierLast reviewed commit: 4b6532c