# Advisory Handoff — Cart Loose-Charge Logic (INT-2851) **Purpose:** Guidance for a future session implementing / directing VSR cart operations. Distilled from an analysis of PR #26 (`rkordisch:feature/INT-2851-fix-loose-charge` → `theorchard/virtual-sales-rep:master`). Treat the PR as a **reference proposal**, not settled code — adopt the correct parts, avoid the pitfalls below. **Status:** Advisory only. Do not comment on or approve PR #26 from this handoff. --- ## 1. The rule to implement Loose surcharge bills **only the loose units** of a line — the remainder beyond whole cartons — not the full quantity. ``` looseUnits = bulkFactor > 0 ? (quantity % bulkFactor) : quantity looseCharge = looseChargeFlag && !waiveLooseBulk ? round(looseUnits * $0.30) : 0 ``` Ticket reference (UPC `620953711022`, carton factor 80, rate $0.30): | Quantity | Correct loose charge | | --- | --- | | 7 | $2.10 (7 loose) | | 80 | $0.00 (exact carton) | | 81 | $0.30 (1 loose) | | 163 | $0.90 (2 cartons + 3 loose) | A full carton — or any exact multiple — is never surcharged. `LOOSE_CHARGE_RATE` stays `$0.30`. This matches legacy Domino behaviour. The prior formula (`$0.30 × full quantity`) overcharged every order of a full carton or more (qty 80 billed $24 instead of $0). It was correct only when `quantity < bulkFactor`. --- ## 2. Where authority lives - **Server is authoritative.** `packages/vsr-graphql-server/src/utils/pricing.ts` computes the real charges. `getCartWithItems` recomputes charges **live** on every read — that is the source of truth for what the user is billed. - **Frontend `apps/vsr/src/utils/pricing.ts` is display-only.** Keep its `computeLooseCharge` signature in lockstep with the server (it now needs `bulkFactor`). If they diverge, the server wins. - `bulkFactor` is already exposed on `Product` (`bulkFactor: Int!`, sourced from `BULK_FACTOR_QT`) — no new catalog field is required. --- ## 3. Data-model requirements The RDS sidecar `cart_items` / `order_items` are a **distilled subset** of the legacy Snowflake `CART_DETAIL` / `ORDER_DETAIL` and had dropped `BULK_FACTOR_QT`. Cart operations need the carton factor present at compute time. - `cart_items` needs `bulk_factor_qt` (and, for legacy parity, `loose_charge_qt` / `loose_charge_am`). - `order_items` needs `bulk_factor_qt` added to its immutable snapshot. - Migration `infra/sql/005_loose_charge_bulk_factor.sql` does this idempotently. - The carton factor is **snapshotted at add-to-cart time** (consistent with how `unit_price` / `effective_price` / waiver flags are already captured), not re-derived live per read. --- ## 4. Pitfalls to avoid (learned from the proposal) ### 4.1 `DEFAULT 0` + the "all-loose" guard is a semantic landmine — HIGHEST PRIORITY `computeLooseUnits` treats `bulkFactor <= 0` as *every unit loose*. Migration 005 backfills existing rows with `bulk_factor_qt = 0`. Combined, **any cart row that predates the migration evaluates as fully loose — reintroducing the exact overcharge being fixed** — until the item is removed and re-added. **Guidance for the cart session:** - Prefer backfilling `bulk_factor_qt` from `products` for existing open carts in the migration, **or** - Explicitly treat open carts as ephemeral (dev DBs reset) and accept the window — but document that assumption where the guard lives. - Consider `DEFAULT NULL` + a `CHECK`, or a sentinel, rather than `0`, so the "unknown factor" state is not silently equal to "all loose". ### 4.2 `ON CONFLICT DO UPDATE` does not refresh `bulk_factor_qt` `addCartItem`'s upsert only bumps `quantity` and `updated_at`. On a re-add of the same UPC it keeps the **first** insert's `bulk_factor_qt`. For post-migration rows this is fine (first insert always carries the factor). But a legacy row with `bulk_factor_qt = 0` stays `0` forever through re-adds. If the cart session ever needs the carton factor to track catalog changes, the snapshot-at-add design (D011) deliberately does **not** — confirm that is the intended contract. ### 4.3 The persisted cart-level snapshot is write-only + a non-transactional double round-trip `refreshLooseChargeSnapshot` runs a **second** `UPDATE` after INSERT/UPDATE, outside a transaction, to persist `loose_charge_qt` / `loose_charge_am` on `cart_items`. But `getCartWithItems` always recomputes live, so **nothing reads those persisted cart-level columns** — they exist only for `CART_DETAIL` parity. **Guidance:** - If parity columns are required, fold the recompute into the write statement (a single SQL round-trip) rather than a follow-up `UPDATE`, to avoid a stale snapshot if the process dies mid-way. - Otherwise, question whether the cart-level persisted columns are needed at all — the `order_items` snapshot is the one that matters for order history. ### 4.4 Charge asymmetry (informational) The loose charge now bills the remainder, but `min_order_qt` in `placeOrder` still snapshots the **full** quantity. This is intentional / out of scope (MIN_ORDER flag is hard-coded false on PRODUCT via `V_PRODUCTS`), but be aware of the asymmetry if min-order logic is ever activated. --- ## 5. Recommended cart-operation shape 1. On **add to cart**: capture `product.bulk_factor_qt` into `cart_items` at insert time alongside price/flag snapshots. 2. On **read** (`getCartWithItems`): recompute all charges live from `quantity + bulk_factor_qt + flags`. This is the billed truth. 3. On **place order**: recompute once more and snapshot the **loose remainder** (not full qty) into `order_items.loose_charge_qt`, plus `bulk_factor_qt`. 4. Keep the frontend mirror's signature identical to the server's. 5. Guard `bulkFactor <= 0`, but make sure that state can only arise from genuinely unknown data — not from a `DEFAULT 0` on freshly migrated rows (see 4.1). --- ## 6. Testing expectations - Unit-test the carton cases in **both** workspaces: `7/80/81/163`, exact-carton → $0, waiver → $0, and the defensive `bulkFactor = 0` case. - Add at least one path test for the **merge-quantity / re-add** recompute — the proposal relies on live smoke for that path only. - Live smoke against the real stack (server :4001 + Postgres) for qty 7 / 80 / 81 and verify the persisted snapshot. --- ## 7. Open follow-ups - **INT-2793 / FUP-20260703-1348** — RDS `cart_items` / `order_items` are a distilled subset of legacy `CART_DETAIL` / `ORDER_DETAIL` and diverge in column set and naming. Loose-charge was one symptom (`BULK_FACTOR_QT` missing). A full RDS ↔ Snowflake column map + rename/extend is deferred there. Expect more per-column gaps until reconciled. - Decision context for all of the above: `docs/DECISION_LOG.md` **D011**. --- ## 8. Provenance - Source: PR #26 diff + `RdsDataSource.ts` at head `21dda9b` (`feature/INT-2851-fix-loose-charge`). - Files touched by the proposal: server + frontend `pricing.ts`, `RdsDataSource.ts`, `cartOrderResolvers.ts`, `product.graphql` / `cart.graphql`, `infra/sql/005_loose_charge_bulk_factor.sql`, `ProductRow.tsx`, `ProductDetailPage.tsx`, plus `CHANGELOG` / `DECISION_LOG` / `FOLLOW_UP_TICKETS`. - This handoff is advisory. No review was posted to PR #26.