amadan
Reduction design: Astra's review (reject, four P1s) recorded; revision 1 marked rejected; the three decisions for Paul listed
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+116
−1
docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md+7 −1docs/superpowers/specs/CODEX-FINDINGS-quantity-reduction-design-review1.md+109 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md b/docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md| index 2c18afed6..ec97ccd89 100644 |
| --- a/docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md |
| +++ b/docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md |
| @@ -1,6 +1,6 @@ |
| # Quantity reduction at renewal: design |
| -Status: design, written 2026-10-02 on `feat/carloku-quantity-reduction-20261002` (off the multi-currency branch at `68f6ee23`, which is not yet merged; this branch carries it). Nothing here is built. Billing is OFF. Nothing here writes to Stripe or to production. |
| +Status: design revision 1, written 2026-10-02 on `feat/carloku-quantity-reduction-20261002` (off the multi-currency branch at `68f6ee23`). **Rejected by Astra's design review** (`CODEX-FINDINGS-quantity-reduction-design-review1.md`, four P1s): a request can cross the renewal and its confirmation proves nothing about which quantity was invoiced; a future-dated assignment blocks every legitimate change before the renewal (the history model below is wrong); a missed window postpones the reduction by a year in silence; and running reductions on the fence proof alone leaves no path to restore seats when members return. Revision 2 is not written yet: it needs the model changed (no future-dated assignment; the reduction proven by the renewal itself; a missed reduction detected and held) and three decisions that are Paul's, listed at the end. Nothing here is built. Billing is OFF. Nothing here writes to Stripe or to production. |
| The approved pricing spec (`2026-09-07-carloku-pricing-plans-design.md`, "Stripe object model") says: "Member changes update the subscription quantity. Additions prorate; people who leave come off at renewal." Additions are built (`billingquantity.go`: the quantity worker, its operations, the fence). Reductions are not: `OUTSTANDING.md` item 14, "a renewal bills the old quantity", and team plans cannot be sold until it is closed. Today a removal is recorded as a quantity event with the effect `at_renewal`, the account's summary reads `at_renewal` when the desired quantity is below the billed one, and nothing acts on either. |
| @@ -71,3 +71,9 @@ The renewal itself: `provableRenewal` requires `current.Quantity == next.Quantit |
| - The window's length (72 hours) is a choice, stated with its reason; a shorter window lowers the exposure to a returning member's prorated seat and raises the exposure to a worker that was down. It is a constant, not a configuration. |
| - Whether the customer is told, on the billing page, that seats come off at the next renewal with the date. Copy is Paul's; the page can state it from the summary's `at_renewal` and the period end once the copy exists. |
| - Nothing here changes the runtime key's permissions: the item update is the same call the additions make, under the same deliberately withheld permission until B and the fence (ENABLEMENT §4). Reductions need the fence; they do not need B. |
| + |
| +## Decisions for Paul before revision 2 (from the review) |
| + |
| +1. **A member who leaves and is replaced before the renewal.** Lowering the quantity before the renewal and raising it again when a member returns charges a prorated seat that was already paid through the period, once per cycle, for as many seats as cycle. Options: accept that for a short window (72 hours was proposed; the exposure is per seat per cycle, not one seat); lower only at the boundary by a provider-side schedule (no re-charge, but a schedule's interaction with the addition worker's item updates and a further key permission need test-mode verification); or lower immediately on departure (largest exposure). Recommendation: the schedule, if test mode shows it coexists with item updates; else the short window with the exposure stated on the billing page. |
| +2. **A missed reduction** (the worker down across the window; a request that crossed the boundary): the renewal has billed the old quantity for a year. Refund the difference by credit note, credit it against the next invoice, or accept the charge? The console can detect and hold it; the remedy is commercial. |
| +3. **Reductions and the proration policy (B).** The review shows reductions cannot run safely without the addition path running too (seats that return must be billable again). Reductions therefore wait for both switches, unless Paul rules otherwise. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-quantity-reduction-design-review1.md b/docs/superpowers/specs/CODEX-FINDINGS-quantity-reduction-design-review1.md| new file mode 100644 |
| index 000000000..1ab53860a |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-quantity-reduction-design-review1.md |
| @@ -0,0 +1,109 @@ |
| +All references are to `37c44887`. **Design** below means `docs/superpowers/specs/2026-10-02-quantity-reduction-at-renewal-design.md`. No files were changed, tests run, or network requests made; Stripe documentation was assessed only through the committed evidence. |
| + |
| +1. **P1 — A reduction can cross renewal, and confirmation does not prove which quantity was invoiced.** |
| + **References:** Design:32–33,48; `internal/console/billingquantity.go:1026–1060,1099–1113`; `internal/billingstripe/client.go:384–390`. |
| + |
| + **Scenario:** A `20 → 10` request leaves just before renewal. Stripe creates the renewal invoice for 20, then executes the update with `none` in the new period. The response confirms item, quantity, price and currency; it establishes neither the execution period nor the invoice quantity. Design:48 would nevertheless accept the confirmed target as proof that the renewal should grant allowance for 10. |
| + |
| + Omitting `proration_date` removes the existing worker’s claimed provider-side period boundary. Local clock checks and leases cannot replace it; a slow console can also send after Stripe has crossed the boundary. After idempotency-key expiry, the same problem extends to much later requests. The committed evidence explicitly leaves that expiry defect open and the worker disabled (`2026-10-01-stripe-idempotency-evidence.md:71–85`). |
| + |
| + **Smallest acceptable change:** Specify how the execution period and actual renewal charge are established. Do not infer either from `ConfirmedAt` and a matching target. Define a held, recoverable outcome for boundary ambiguity, add the late-request-fencing prerequisite, and include boundary-crossing and clock-skew cases in test-mode verification. |
| + |
| +2. **P1 — The future assignment blocks legitimate changes before renewal.** |
| + **References:** Design:19,44,52; `internal/plans/month.go:111–125`; `internal/console/billingstore.go:621–671,709–741`. |
| + |
| + **Scenario:** At `T−48h`, reduce 12 to 10 and append revision 2 effective at renewal `T`. At `T−24h`, membership grows to 13 and the addition succeeds. Its revision 3 should take effect before `T`, but `latest <= quantityStart` fails. The store records quantity-date and clock reviews and clamps revision 3 to `T`. The account retains allowance for 12 before renewal despite the additional purchase. Cancellation or another ordinary state change before `T` encounters the same future timestamp. |
| + |
| + Simply removing the clamp is worse: sorting revision 3 before revision 2 makes `AllowanceForMonth` reject the **whole history**, including earlier months. |
| + |
| + **Smallest acceptable change:** Separate the pending renewal reduction from committed allowance revisions, or fully specify a replacement history model supporting intervening changes. Define how additions interact with seats already paid through `T`; keep the frozen-month protections. |
| + |
| + Equal timestamps themselves are safe: successive reductions and then a renewal at `T`, with increasing revisions, do not double-count allowance. However, the reduction assignment retains `PaidThrough=T`, so it contributes **no paid span after `T`** until renewal extends coverage (`billinginvoicestore.go:529–542`; `month.go:150–162`). Frozen calculations remain immutable, and a frozen renewal month prevents backdating; neither fact solves the intervening-change problem. |
| + |
| +3. **P1 — Missing the window silently postpones the reduction by another annual term.** |
| + **References:** Design:9,27–32,71; pricing design `2026-09-07-carloku-pricing-plans-design.md:410–419`; `internal/console/billingquantityaudit.go:63–66`. |
| + |
| + **Scenario:** Twenty seats fall to ten before the window. The worker is unavailable throughout the last 72 hours and returns after renewal. Stripe has charged 20 for another year. Reconciliation advances the period, and the next eligible window is almost a year away. Unprocessed removals do not themselves hold billing, so this can remain an ordinary `at_renewal` summary. |
| + |
| + **Smallest acceptable change:** Persist the intended renewal boundary and define detection, review and remediation for a missed reduction. Specify treatment of departures after the last successful pass as well. Whether to refund, credit or accept the extra charge is an owner decision; silently waiting another year does not satisfy Design:9. |
| + |
| +4. **P1 — Fence-only enablement is unsafe with the proposed return path.** |
| + **References:** Design:38,52,73; `internal/console/billingquantity.go:118–138`; `internal/console/billingmembership.go:245–256`; `implementation/carloku-paid-launch/stripe-provisioning.md:303–315`. |
| + |
| + **Scenario:** The fence is verified but B is unset. A reduction lowers 20 to 10. Once the entitlement is published, ten members return; team membership permits this. The addition worker remains disabled, so the design’s promised restoration cannot run and renewal can bill ten for twenty members. |
| + |
| + A reduction’s own request needs no proration policy, but this design relies on a functioning addition path after it. It also claims unchanged permissions while the committed provisioning plan withholds item-write permission until both switches are set. |
| + |
| + **Smallest acceptable change:** Require both switches for initiating reductions under this mechanism, or obtain an explicit owner decision for an independently safe restoration/growth policy. Update the permission-enablement contract accordingly. Engineering fence verification alone is insufficient. |
| + |
| +5. **P2 — Recovery needs explicit operation-kind dispatch throughout the worker.** |
| + **References:** Design:15,33–36,48,58; `internal/console/billingquantity.go` locations below. |
| + |
| + **Scenario:** A reduction is confirmed, renewal is reconciled, and the worker restarts before acknowledgement. The existing resolver calls `markOperationLanded` with `landed_unverified` even for a confirmed operation. That function acknowledges the operation, updates **addition** events, and files a quantity-date review. The removal can remain unprocessed while the account receives a false hold. |
| + |
| + **Smallest acceptable change:** Include this consumer inventory and its required behavior in the design: |
| + |
| + | Consumer | Existing assumption and required treatment | |
| + |---|---| |
| + | `beginQuantityOperation:525–559` | Explicitly requires `target > from` and derives generation/target from additions. Add kind-specific direction, target and generation checks; atomically verify no pending addition and no intervening membership change. Persist/reset `kind` on every upsert. | |
| + | `quantityOperationFor:459–463`, `quantityStep:903–911` | Carry no kind. Preserve it through loading, recovery and completion dispatch. | |
| + | `sendQuantityOperation:1059–1060,1121` | Always supplies `CreatedAt` as the proration date and chooses an addition outcome. Make request parameters and outcomes kind-specific. | |
| + | `resolveQuantityOperation:1166–1208` | Comparisons are largely direction-neutral, but completion is addition-specific. Define confirmed, unconfirmed, following-period and changed-target reduction outcomes. | |
| + | `markOperationLanded:775–801` | Updates additions and creates an unverified hold. Add reduction-specific recovery. | |
| + | `markQuantityAdditionsProcessed:275–354` | Uses addition peaks and the same-period requirement; acknowledgement does not check kind. Prevent it from acknowledging reductions. | |
| + | `settleQuantityOperation:1267–1280`, `fenceQuantityOperation:1290–1328`, `retireQuantityOperation:634–635` | Do **not** inherently assume `target > from`. Preserve their lease, fencing and review invariants; do not equate retirement with successful reduction. | |
| + | `closeEndedQuantityAdditions:241–252` | Event selection is addition-only; operation retirement is direction-neutral. Extend event cleanup without weakening retirement rules. | |
| + |
| +6. **P2 — Reconcile can file a false hand-lowered review or reuse stale reduction evidence.** |
| + **References:** Design:42–48; `internal/console/billingservice.go:1009–1045`; `internal/console/billingstore.go:627–655`. |
| + |
| + **Scenario A:** Stripe applies a reduction and another console reconciles before the sender persists confirmation. The operation has a live lease. A literal “skip the hand review only when confirmed” change files `billing_quantity_lowered` before the existing lease check returns pending. Later confirmation does not remove that review. |
| + |
| + **Scenario B:** Renewal is observed during the same confirmation gap. The current same-period operation matcher cannot defer it, and `provableRenewal` rejects the changed quantity. The renewal gets observation-dated and held even if confirmation arrives moments later. |
| + |
| + **Scenario C:** A confirmed, acknowledged `20 → 10` row remains. An operator subsequently raises quantity to 20 and lowers it to 10 again. Design:42’s listed matcher lacks `AckedAt == 0`; reusing that row suppresses a legitimate hand-lowered review. The renewal exception likewise needs the original `from` and full operation identity, not just target and period end. |
| + |
| + **Smallest acceptable change:** Define one outstanding-operation matcher, including kind, direction, behavior, subscription/item, both period boundaries and source/target quantities. Check live-lease ambiguity **before filing either review**, including renewal-first observations. Revalidate the operation inside the assignment transaction; a kind field alone does not prevent cross-kind attribution. |
| + |
| +7. **P2 — The commercial exposure is neither “one seat” nor bounded by one 72-hour proration.** |
| + **References:** Design:18,52–53,71; `internal/console/billingquantity.go:118–123`. |
| + |
| + **Scenario:** Reduce 20 to 10, restore 20 when members return, then repeat the departure/return cycle. Each restoration creates positive prorations; each reduction uses `none` and offsets none of them. Multiple seats and repeated cycles accumulate charges for capacity already paid through renewal. |
| + |
| + **Smallest acceptable change:** State exposure per seat **per cycle**, and put recharging already-paid seats, repeated reductions, window length and customer disclosure to the owner explicitly. Calling 72 hours a constant does not make this an engineering-only choice. Define these decisions before implementation. |
| + |
| +8. **P2 — Removal completion and cleanup are underspecified outside successful reductions.** |
| + **References:** Design:27,30–36,59; `internal/console/billingmembership.go:475–485`; `internal/console/billingquantity.go:145–147,241–243`. |
| + |
| + **Scenario:** A team at the ten-seat minimum loses one member. No reduction is necessary and the summary remains `in_sync`, so the proposed account selector never processes that removal. Similarly, a canceled subscription with only pending removals and no operation need not be selected, making the proposed extension of `closeEndedQuantityAdditions` unreachable. |
| + |
| + An unconfirmed operation whose target is merely observed also cannot truthfully produce `reduced_at_renewal` with the operation’s `none` reference. |
| + |
| + **Smallest acceptable change:** Define outcomes and selection for minimum-covered removals, departures offset by replacements, ended/replaced subscriptions, observed targets and indeterminate operations. Scope successful marks to the operation’s subscription and covered generations, and retain the existing clock-safe processing timestamp rule. |
| + |
| +9. **P2 — Adding the outcome requires a deployment compatibility rule.** |
| + **References:** Design:34,58; `internal/console/billingquantityaudit.go:20–43,46–61`; `internal/console/billingquantity.go:459–463,553–559`. |
| + |
| + **Scenario:** A new console records `reduced_at_renewal`. An older console starts against the shared database, reinstalls its older processing trigger and marks that outcome as a history gap. That gap is deliberately sticky. An older worker also cannot distinguish a reduction operation and may recover it through addition logic. |
| + |
| + **Smallest acceptable change:** Require all readers/workers to support reductions before enabling writes, explicitly prohibit incompatible rollback, and define handling of operations outstanding during deployment. Adding a defaulted column and updating one outcome constant does not establish mixed-version safety. |
| + |
| +10. **P2 — The Stripe verification list covers the ordinary path, but not every relied-on claim.** |
| + **References:** Design:17,21,62–67; `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md:105–119`. |
| + |
| + The basic mechanism is appropriate for an **immediate quantity update without a proration generated by that update**. It does not schedule a change at renewal or establish what an already-created renewal invoice contains. Checks 1, 2 and 4 correctly address the ordinary update, unchanged-period webhook and subsequent positive proration. |
| + |
| + **Scenario:** A prior addition has pending prorations, or a returning member causes an immediate invoice before renewal. “The next invoice, which is the renewal” is then an insufficient invoice contract. The absolute claim that scheduled subscriptions refuse direct item updates is also unsupported by the committed evidence and absent from the verification list. |
| + |
| + **Smallest acceptable change:** Specify the renewal’s recurring line separately from other invoice items. Add verification for boundary crossing, existing pending prorations, the exact restricted-key permissions and all `none` fence cases. Cite and verify the precise schedule restriction used to reject that alternative. Keep expiry, indeterminate-500 and key-rotation limitations explicit; a passing ordinary fence test does not settle them. |
| + |
| +Claims the design makes that the code contradicts: |
| + |
| +- **“The month … gets … the new quantity’s [allowance] from [period end]” — Design:44.** The reduction row alone has `EffectiveAt == PaidThrough`, so its paid span is empty. Renewal must supply the extended period (`billinginvoicestore.go:530–542`; `plans/month.go:156–162`). |
| +- **“A reduction outstanding does not hold growth, because it charges nothing” — Design:54.** Membership admission does not inspect operation kind or charging behavior. It requires a published entitlement matching current billing state. Reconciliation can therefore temporarily hold growth while publication catches up (`billingmembership.go:233–238,283–308`). |
| +- **“The seats it bounds” — Design:23.** Team quantity is not a purchased membership cap: Community and Business use unlimited `MaxMembers` (`entitlementstore.go:130–134,163–168`; `billingmembership.go:245–256`). |
| +- **“The recorded period” is checked by `quantityProviderItem` — Design:31.** It compares period start, not period end (`billingquantity.go:1020–1023`). |
| +- **The listed unsettled-review identifier — Design:60.** The actual identifier is `billing_quantity_operation_unsettled:<serial>` (`billingquantity.go:111–112`). The existing paths can also produce period and clock reviews omitted from that list. |
| +- **“Recorded when observed” for every undatable reduction — Design:46.** The generic fallback can clamp forward to the latest assignment (`billingstore.go:726–730`); observation time is not guaranteed. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |