fix(billing): preserve generated schedule edits - #3211
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Confidence score: 2/5
server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.tscan emit a persisted entity-scoped schedule without its entity scope becauseentity_idis null, risking incorrectly targeted or unusable billing requests — resolve the public entity ID from theentityScheduleskey.server/tests/scenarios/billing/schedule-playground-scenario.test.tschecks only per-phase item counts, so duplicated data could pass while phase identity is wrong; assert the expected phase-to-customer_product_idsmapping for both the response and persisted schedule.
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="server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.ts:100">
P1: When an entity-scoped schedule was persisted by create-schedule, this mapping emits it without an entity scope because the schedule row's `entity_id` is null. Resolve the public entity ID from the `entitySchedules` key and `fullCustomer.entities` before serializing the schedule, otherwise a repeated edit can treat entity phases as customer-level.</violation>
</file>
<file name="server/tests/scenarios/billing/schedule-playground-scenario.test.ts">
<violation number="1" location="server/tests/scenarios/billing/schedule-playground-scenario.test.ts:187">
P2: This end-to-end persistence check only asserts the count of `customer_product_ids` per phase ([3,3,3,3]) for both the response and the persisted schedule. It never verifies phase identity, so a regression that duplicates the same three products across phases, reorders them, or swaps plans between phases would still pass. Assert the actual phase-to-customer-product mapping (e.g. that each phase resolves to the distinct core/analytics/support products and that persisted phases match the response) to actually cover the phase-identity fix this PR is about.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
| ? firstPersistedStartsAt | ||
| : generatedStartsAt; | ||
| let persistedStartsAt: number | undefined; | ||
| if (index === 0) persistedStartsAt = firstPersistedStartsAt; |
Contributor
There was a problem hiding this comment.
When a generated edit removes or reorders the existing first phase, the phase now at index zero unconditionally inherits the removed phase's persistedStartsAt. The request builder then treats that timestamp as authoritative, causing the replacement phase to be backdated or updated as the wrong persisted phase.
Knowledge Base Used: Billing lifecycle and payment flows
Prompt To Fix With AI
This is a comment left during a code review.
Path: vite/src/components/forms/create-schedule/utils/scheduleFormFromRequestBody.ts
Line: 97
Comment:
**First phase identity transfers**
When a generated edit removes or reorders the existing first phase, the phase now at index zero unconditionally inherits the removed phase's `persistedStartsAt`. The request builder then treats that timestamp as authoritative, causing the replacement phase to be backdated or updated as the wrong persisted phase.
**Knowledge Base Used:** [Billing lifecycle and payment flows](https://app.greptile.com/autumn-org-2/-/custom-context/knowledge-base/useautumn/autumn/-/docs/billing-lifecycle.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Confidence score: 3/5
setupGenerationContext.tscan omitentity_idfrom emitted schedules when an entity-scoped product has onlyinternal_entity_id, potentially generating schedules for the wrong scope; preserve the resolved entity mapping before merging product data.generate-create-schedule-e2e.test.tsdoes not exercise the pagination path because it creates fewer than the default 15-product limit, leaving the related regression insufficiently covered; increase the fixture count beyond the limit and assert the paginated behavior.
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="server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.ts">
<violation number="1" location="server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.ts:124">
P1: When an entity-scoped customer product has `internal_entity_id` but no persisted `entity_id`, this map drops the entity mapping even though the loaded entity can resolve it. The emitted schedule then omits `entity_id`, so generated edits can treat an entity schedule as customer-scoped; resolve the API ID from `entities` with `customerProduct.entity_id` as fallback.</violation>
</file>
<file name="server/tests/integration/billing/generateRequest/generate-create-schedule-e2e.test.ts">
<violation number="1" location="server/tests/integration/billing/generateRequest/generate-create-schedule-e2e.test.ts:337">
P2: This new test creates only 12-13 customer products, which stays within CusService.getFull's default customer-product limit (15). Because every product is already in the paginated `customer_products` set, the `current_plans` assertion passes even without the scheduledProductIds merge that this PR adds, so the test gives false confidence that the merge works. Size the fixture beyond the limit (e.g. more phases/plans so scheduled products exceed 15) or assert a scenario where a scheduled product would otherwise be dropped.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+124
to
+129
| product.internal_entity_id && product.entity_id | ||
| ? [[product.internal_entity_id, product.entity_id]] | ||
| : [], | ||
| ), | ||
| ); | ||
| const compactSchedule = ( |
Contributor
There was a problem hiding this comment.
P1: When an entity-scoped customer product has internal_entity_id but no persisted entity_id, this map drops the entity mapping even though the loaded entity can resolve it. The emitted schedule then omits entity_id, so generated edits can treat an entity schedule as customer-scoped; resolve the API ID from entities with customerProduct.entity_id as fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/billing/v2/actions/generateRequest/setup/setupGenerationContext.ts, line 124:
<comment>When an entity-scoped customer product has `internal_entity_id` but no persisted `entity_id`, this map drops the entity mapping even though the loaded entity can resolve it. The emitted schedule then omits `entity_id`, so generated edits can treat an entity schedule as customer-scoped; resolve the API ID from `entities` with `customerProduct.entity_id` as fallback.</comment>
<file context>
@@ -87,22 +89,66 @@ export const setupGenerationContext = async ({
+ );
+ const entityIdByInternalId = new Map(
+ customerProducts.flatMap((product) =>
+ product.internal_entity_id && product.entity_id
+ ? [[product.internal_entity_id, product.entity_id]]
+ : [],
</file context>
Suggested change
| product.internal_entity_id && product.entity_id | |
| ? [[product.internal_entity_id, product.entity_id]] | |
| : [], | |
| ), | |
| ); | |
| const compactSchedule = ( | |
| const entityIdByInternalId = new Map<string, string>( | |
| customerProducts.flatMap((product) => { | |
| if (!product.internal_entity_id) return []; | |
| const entityId = | |
| entities.find( | |
| (entity) => entity.internal_id === product.internal_entity_id, | |
| )?.id ?? product.entity_id; | |
| return entityId | |
| ? [[product.internal_entity_id, entityId] as const] | |
| : []; | |
| }), | |
| ); |
| expect( | ||
| response.phases | ||
| .flatMap(({ customer_product_ids }) => customer_product_ids) | ||
| .every((id) => currentPlanIds.has(id)), |
Contributor
There was a problem hiding this comment.
P2: This new test creates only 12-13 customer products, which stays within CusService.getFull's default customer-product limit (15). Because every product is already in the paginated customer_products set, the current_plans assertion passes even without the scheduledProductIds merge that this PR adds, so the test gives false confidence that the merge works. Size the fixture beyond the limit (e.g. more phases/plans so scheduled products exceed 15) or assert a scenario where a scheduled product would otherwise be dropped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/tests/integration/billing/generateRequest/generate-create-schedule-e2e.test.ts, line 337:
<comment>This new test creates only 12-13 customer products, which stays within CusService.getFull's default customer-product limit (15). Because every product is already in the paginated `customer_products` set, the `current_plans` assertion passes even without the scheduledProductIds merge that this PR adds, so the test gives false confidence that the merge works. Size the fixture beyond the limit (e.g. more phases/plans so scheduled products exceed 15) or assert a scenario where a scheduled product would otherwise be dropped.</comment>
<file context>
@@ -260,6 +285,59 @@ test(`${chalk.yellowBright("billing.generate schedule: generated edit previews,
+ expect(
+ response.phases
+ .flatMap(({ customer_product_ids }) => customer_product_ids)
+ .every((id) => currentPlanIds.has(id)),
+ ).toBe(true);
+}, 120_000);
</file context>
charlietlamb
deleted the
charlie/billing-generate-schedule-regression
branch
September 2, 2026 19:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
bun test src/components/forms/create-schedule/utils/scheduleFormFromRequestBody.test.ts(5 passed)Summary by cubic
Fixes
billing.generateso generated schedule edits keep the persisted schedule topology and phase identity, and no longer replace a requested first-phase start date with the current time.Bug Fixes
Refactors
Written for commit 36b51e1. Summary will update on new commits.
Greptile Summary
This PR adds persisted schedule topology to billing-generation context and carries phase identity through repeated generated edits.
Confidence Score: 4/5
The PR is not yet safe to merge because removing or reordering the first generated phase can still transfer the old phase’s persisted identity to its replacement.
Generated phase zero inherits the previous first phase’s persisted timestamp without confirming that it is the same phase, and request construction then treats an already-started inherited timestamp as authoritative.
Files Needing Attention: vite/src/components/forms/create-schedule/utils/scheduleFormFromRequestBody.ts; vite/src/components/forms/create-schedule/hooks/useCreateScheduleRequestBody.ts
Important Files Changed
Reviews (4): Last reviewed commit: "refactor(billing): extract schedule cont..." | Re-trigger Greptile
Context used (3)