feat(shared): usage alert basis and filter - #3246
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 19 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Add a basis field to usage alerts (balance, included, recurring, usage_limit) and an optional filter that points a usage_limit alert at the usage limit sharing its feature and filter. Alerts dedup on (feature_id, basis, filter, threshold_type, threshold) across customer, entity and plan billing controls; entity usage limits dedup by filter too. Org config rejects basis usage_limit, and a plan's usage_limit alerts must match a usage limit on the same plan.
51565d2 to
66cd28f
Compare
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Confidence score: 5/5
- In
shared/api/billingControls/planBillingControls.ts, the comment references a nonexistenthandleUsageLimitAlertErrorsguard, which could mislead maintainers about where usage-limit alert validation occurs; document only this schema’s create-time validation or remove the reference.
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="shared/api/billingControls/planBillingControls.ts">
<violation number="1" location="shared/api/billingControls/planBillingControls.ts:31">
P3: This comment references a nonexistent `handleUsageLimitAlertErrors` guard, so it misleads maintainers about where catalog updates validate usage-limit alerts. Document only this schema's create-time validation, or reference the actual update validation mechanism.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ault and balance-basis list
There was a problem hiding this comment.
1 issue found across 21 files (changes from recent commits).
Confidence score: 3/5
- In
shared/models/cusModels/billingControls/identity/usageLimitIdentity.ts, accepted filter keys or values containing=,&, or|can produce identical identities for distinct limits, causing deduplication or alerts to use the wrong cap; preserve a reversible shared encoding or reject these delimiters.
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="shared/models/cusModels/billingControls/identity/usageLimitIdentity.ts">
<violation number="1" location="shared/models/cusModels/billingControls/identity/usageLimitIdentity.ts:6">
P1: When an accepted filter key or value contains `=`, `&`, or `|`, this identity is not injective. Distinct limits can be deduplicated or an alert can match the wrong cap; preserve a reversible shared encoding or reject delimiter characters before using this key for identity.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Layer 1 of 6 (schema). Stack: #3246 → #3247 → #3259 → #3248 → #3249 → #3250.
Usage alerts gain
basis(what 100% means) and an optionalfilterthat points ausage_limitalert at the usage limit sharing its feature and filter.balance(default, unchanged)includedrecurringusage_limitfilteris only valid withbasis: usage_limit.(feature_id, basis, filter, threshold_type, threshold)on customer, entity and plan controls. Entity usage limits now dedup by filter as customers already did.basis: usage_limit(there is no single cap to measure).PlanBillingControlsParamsSchemarequires a plan'susage_limitalerts to match a usage limit on the same plan.basisis stored explicitly with abalancedefault; existing tests that compare stored alerts by equality add it.No behaviour change to firing yet; that lands in #3248.
Summary by cubic
Usage alerts now support an explicit basis (
balance,included,recurring, orusage_limit) and an optional filter that selects a matching usage limit. Previously all alerts usedbalanceimplicitly; this adds schema and validation only, so non-balancebases don't change alert firing yet.Validation
filteris only valid withbasis: "usage_limit", which also requires afeature_id; org alerts reject that basis, and plan alerts must resolve to a usage limit on the same plan.(feature_id, basis, filter, threshold_type, threshold)across customer, entity, and plan controls; usage limits deduplicate by(feature_id, filter).feature_idskip entries without one.basis: "balance", so no migration is needed.Known issue
&or=, which can reject valid controls or resolve an alert against the wrong usage limit.Written for commit 81abf81. Summary will update on new commits.
Greptile Summary
Adds schema and validation support for selecting how usage-alert percentages are measured and optionally targeting a filtered usage limit.
balance,included,recurring, andusage_limitalert bases, withbalanceas the default.Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains within the scope of this follow-up review.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Usage alert input] --> B{Select basis} B -->|balance, included, recurring| C[Validate alert fields] B -->|usage_limit| D[Require feature ID] D --> E[Match feature and filter to plan usage limit] C --> F[Check duplicate identity] E --> F F --> G[Store alert with explicit basis]Reviews (9): Last reviewed commit: "refactor(shared): usage_limit alerts req..." | Re-trigger Greptile
Context used: