# VSR Replatform — Decision Log

Architectural decision records (ADRs) for INT-2682 and child tickets. Each entry
records the decision, rationale, alternatives considered, and impact.

---

## INT-2823 — Catalog browse performance mitigation (2026-06-28)

### D001: Retire `V_PRODUCTS`; re-point the single product mapping at raw `PRODUCT` / nav-view columns

- **Decision:** Drop the curated `V_PRODUCTS` view. The one product row→GraphQL
  mapping in `SnowflakeDataSource` / `productResolvers` now reads raw `PRODUCT`
  (and per-nav view) columns directly via a shared `PRODUCT_PROJECTION`.
- **Rationale:**
  - `V_PRODUCTS` was a thin rename layer over `PRODUCT`; the precomputed per-nav
    views already expose `p.*`, so a second curated view added cost without value.
  - A single projection keeps detail/search/related/by-UPC and browse consistent.
- **Alternatives considered:**
  - *Keep `V_PRODUCTS` and add nav views on top* — rejected: two overlapping
    abstractions, double maintenance, and `V_PRODUCTS` re-derives columns the nav
    views already carry.
- **Impact:** `V_PRODUCTS` removed from `infra/sql/snowflake-views.sql`. Dependent
  views (`V_DEALS`, `V_MERCHANDISE_ROLLUP`, `V_INVENTORY`) already read `PRODUCT`
  directly, so retirement breaks nothing.

### D002: Use `TRIM(ARTICLE_NO)` as `Product.id` (not `SELECTION_ID`)

- **Decision:** Product identity is `ARTICLE_NO` (trimmed), not `SELECTION_ID`.
- **Rationale:** `ARTICLE_NO` is the catalog's true unique key (38,340 rows,
  38,340 distinct, 0 null). Source values carry trailing whitespace, so all
  reads/keys `TRIM(ARTICLE_NO)`.
- **Alternatives considered:**
  - *Keep `SELECTION_ID`* — rejected: not the product key; ambiguous across
    rollup/merch rows.
- **Impact:** `SELECTION_ID` is retained internally only for the
  `V_MERCHANDISE_ROLLUP` self-join (`ROLLUP_NO = SELECTION_ID`), passed via an
  internal `_rollupParentId` on the mapped product.

### D003: Data-drive catalog nav + sidebar from precomputed Snowflake objects

- **Decision:** Catalog browse reads precomputed per-nav views
  (`V_{ALL|MUSIC|VIDEO}_{VIEW}`) filtered by `CATEGORY`. The primary/secondary nav
  comes from `NAVIGATION_PRIMARY` / `NAVIGATION_SECONDARY`; sidebar buckets +
  counts come from `NAV_CATEGORIES`.
- **Rationale:**
  - Precomputed slices + counts remove the client-side `productFacets` tally,
    which capped facet sums at the page size and produced biased buckets.
  - One source of truth for nav structure and counts (including On Deal), and the
    nav adapts to data (e.g. Video has no New Releases / Exclusive slice).
- **Alternatives considered:**
  - *Keep client-side facet aggregation* — rejected: sample-biased counts, payload
    bloat.
  - *Hardcode nav arrays in the frontend* — rejected: drifts from data; Video
    subset was already wrong in the hardcoded list.
- **Impact:** Added `navigation` + `navCategories` GraphQL queries, a
  `navigationResolvers`, and a `navigation.graphql` schema. Removed the
  `productFacets` query/resolver/`FacetBucket` type. `VsrLayout` and `CatalogPage`
  render from the nav queries.

### D004: Read stock from `PRODUCT.AVAILABLE_QT`; drop the per-product `V_INVENTORY` call

- **Decision:** Stock status is derived from `PRODUCT.AVAILABLE_QT` (available when
  > 0). The per-product `V_INVENTORY` lookup is removed from the catalog path.
- **Rationale:** `AVAILABLE_QT` is present on `PRODUCT` (and inherited by every nav
  view via `p.*`), so stock comes for free with the browse read — no extra query.
- **Alternatives considered:**
  - *Keep `V_INVENTORY` per-row* — rejected: an extra round-trip per product for a
    value already on the row.
- **Impact:** `AVAILABLE_QT` is surfaced through the existing `stockStatus` field
  (fed via `cdsQty`), so `StockBadge` thresholds are unchanged. `V_INVENTORY` is
  retained for non-catalog consumers but annotated as retired from this path.

### D005: No Dashboard — `/` lands on ALL PRODUCTS → New Releases → first category

- **Decision:** Remove the Dashboard landing. `/` resolves to ALL PRODUCTS → New
  Releases → the first category (ascending, i.e. the oldest street-date week),
  sourced from `NAV_CATEGORIES`.
- **Rationale:** The Dashboard was a placeholder; the catalog is the primary task
  surface. The first-category landing matches the legacy New-Releases-first flow.
- **Alternatives considered:**
  - *Auto-select first category inside `CatalogPage` for any empty category* —
    rejected: would change explicit-nav behavior (clicking a tab should show all,
    with the sidebar to filter). A dedicated landing redirect keeps explicit nav
    unchanged.
- **Impact:** `app.tsx` adds a `LandingRedirect` at `/` and removes the
  `/dashboard` route + `DashboardPage` import. `DashboardPage.tsx` is left in the
  tree (unrouted) pending an explicit removal.

---

## INT-2845 — Data-drive NAVIGATION_SECONDARY columns (2026-06-29)

Three `NAVIGATION_SECONDARY` columns (`view_nm`, `add_to_cart_method`,
`lines_per_webpage`) were read and exposed via GraphQL by INT-2823 but did not
drive behavior — each was overridden by a hardcoded value. INT-2845 makes the
table the single source of truth so these behaviors are tunable in-data.

### D006: `products` browse query keys on `(primaryNav, secondaryNav)`; view name sourced from `view_nm`

- **Decision:** Replace the `products(mediaType, viewType)` arguments with
  `products(primaryNav, secondaryNav)` (matching `navCategories`). The server
  resolves the backing view from `NAVIGATION_SECONDARY.VIEW_NM` for that nav pair,
  validates it (`/^V_[A-Z0-9_]+$/` + non-null), then interpolates it into the
  query. The static `MEDIA_SEGMENT` / `VIEW_SEGMENT` / `resolveViewName` maps and
  the `MediaType` / `ViewType` enums are removed.
- **Rationale:**
  - The view name now comes from data, not a code map — adding/retiring a nav view
    is a table change, not a code change.
  - `view_nm` is read server-side from our own table (clients pass only nav names,
    bound as parameters), and is whitelist-validated before reaching SQL — no
    client-controlled identifier is interpolated.
  - Unifies browse and `navCategories` on the same `(primaryNav, secondaryNav)` key.
- **Alternatives considered:**
  - *Keep `mediaType`/`viewType` and map to `view_nm` server-side* — rejected:
    reintroduces a hardcoded `(media,view) → nav` mapping, defeating the goal.
  - *Pass `view_nm` from the client* — rejected: a client-supplied SQL identifier is
    an injection surface even with validation; sourcing it server-side is safer.
- **Impact:** `product.graphql`, `productResolvers`, `SnowflakeDataSource`
  (`ProductFilter`, `getProducts`, new `resolveNavViewName`), and the
  `GET_PRODUCTS` query + `CatalogPage` variables. The `BEST_SELLERS` order-by
  special case now keys off the secondary-nav name.

### D007: Page size driven by `lines_per_webpage`; browse fetch deferred until nav loads

- **Decision:** `CatalogPage` sources page size from the current nav row's
  `linesPerWebpage` (replacing the `PAGE_SIZE = 30` constant), threaded into both
  the products query and the "Showing X–Y" math. The browse products fetch is
  deferred (`skip`) until the `navigation` query resolves so the correct page size
  is used on the first request. Search has no nav row and uses a fixed page size.
- **Rationale:** The table value is honored as-is (New Releases = 1000, others =
  50; tunable in-data). Deferring avoids a double-fetch / page-size flash. The
  `navigation` query is served from the Apollo cache (already fetched by
  `VsrLayout`), so the lookup is effectively free.
- **Alternatives considered:**
  - *Fetch immediately with a fallback page size, refetch on nav load* — rejected:
    causes a double-fetch and a visible page-size change.
  - *Cap New Releases below 1000 for perf* — deferred: ~150 products per release
    week in practice; lower the table value if perf becomes an issue.
- **Impact:** `CatalogPage` queries `GET_NAVIGATION` (cache-first) and selects the
  `NavSecondary` row by `(primaryNav, secondaryNav)`.

### D008: Add-to-cart mode driven by `add_to_cart_method`; New Releases sort toggle removed

- **Decision:** The batch ("page-batch") vs single add-to-cart UX is driven by
  `add_to_cart_method` (`Multiple` → batch grid, otherwise single), replacing the
  `subcategory === 'new-releases'` slug checks. The artist/date sort toggle is
  removed from New Releases entirely; rows render in the view's native artist/title
  order.
- **Rationale:** Cart UX is a per-view data attribute, not a property of one slug.
  The sort toggle is no longer relevant — the view ordering is authoritative.
- **Alternatives considered:**
  - *Keep the sort toggle but generalize it* — rejected: the product owner
    confirmed it is obsolete.
- **Impact:** `CatalogPage` gates the batch state, quantity inputs, `orderMode`
  prop, and "ADD SELECTED TO CART" actions on `addToCartMethod === 'Multiple'`;
  the sort-toggle state and UI are deleted. New-Releases-only cosmetics not
  governed by these columns (street-date label formatting, future-date stock
  hiding) remain slug-based by design.

---

## INT-2849 — Align front end display/functionality with old site (2026-07-02)

### D009: Retire the hardcoded `SIDEBAR_CONFIG`; drive the category sidebar from data

- **Decision:** Remove `CatalogPage`'s hardcoded `SIDEBAR_CONFIG` map (per-slug
  `{ title, allLabel }`). The category sidebar is now gated on the data-driven
  `NavSecondary.includeCategories` flag (NAVIGATION_SECONDARY, INT-2823) plus a
  non-empty `navCategories` result. Its header is the secondary-nav name, and the
  "all" reset entry (All Weeks/Deals/Genres/Formats/All) and the separate
  "Media" (All/Music/Video) sidebar switcher are both removed. Browse pages now
  always land on the first category (`history.replace`), with no "view
  everything" mode; media is switched only via the primary-nav tabs.
- **Rationale:**
  - Aligns with the old site (INT-2849 changes 4–6): the header mirrors the
    secondary-nav name, there is no all-categories view, and media is not a
    per-page sidebar control.
  - Extends the INT-2823 data-driven nav pattern rather than maintaining a
    parallel hardcoded map; all browse secondary navs already report
    `includeCategories = true`.
  - Landing on the first bucket (categories are returned `ORDER BY CATEGORY ASC`)
    gives the oldest week for New Releases, as specified.
- **Alternatives considered:**
  - *Keep `SIDEBAR_CONFIG` and only relabel the title* — rejected: leaves a
    hardcoded per-slug map duplicating data already exposed by the nav query.
  - *Gate the sidebar purely on `navCategories.length > 0`* — viable, but
    `includeCategories` is the explicit intent flag; the redirect still also
    requires a non-empty result, so both signals are used.
- **Impact:** `CatalogPage` (`SIDEBAR_CONFIG` deleted, `includeCategories` added
  to the `NavSecondary` shape, category redirect effect, product-fetch `skip`
  extended while a redirect is pending). No schema or server change for 4–6.
  Change 3 (browse sort) adds `CONFIGURATION_NM ASC, UPC_CD ASC` to
  `browseOrderBy` (non-Best-Sellers), matching the nav-view DDL; Best Sellers
  keeps `RANK_IN_GENRE`. Changes 1–2 (Price Code → "Code:" above Price;
  Returnable shown only when "No") touch `ProductRow` and `ProductDetailPage`.

### D010: Show list vs effective price; read deal end date from `TODAYS_DEFAULT_DEAL_END_DATE`

- **Decision:** The pricing box now shows both `Price:` (list = `bulkPrice`) and,
  when a deal applies, `Effective Price:` (`dealInfo.effectivePrice`) as separate
  rows, and a minimal deal label `"{discountPct}% OFF ends {M/D}"`. Server-side,
  `dealInfo.endsAt` is now read from `TODAYS_DEFAULT_DEAL_END_DATE` (was
  `DEAL_END_DATE_1`), the deal gate no longer requires `TODAYS_DEFAULT_PROGRAM_NAME`
  (gates on `ON_DEAL='Y'` and derived `discountPct > 0`), and the programme name
  is no longer displayed.
- **Rationale:**
  - Matches the legacy VSR box (INT-2850): the old site shows list price,
    effective price, and "5% OFF ends 7/3" — and never the deal name. The new
    site had mislabelled the discounted price as "Price" and omitted the end date.
  - `DEAL_END_DATE_1` is a `VARIANT` column that Domino leaves **null** in
    practice, so the end date never rendered. `TODAYS_DEFAULT_DEAL_END_DATE`
    (a `DATE`) is the populated "today's default deal" expiry — verified live.
  - With the name no longer shown, gating on it would hide legitimately
    discounted products that lack a programme name; `ON_DEAL` + `discountPct` is
    the authoritative signal.
- **Alternatives considered:**
  - *Keep `endsAt` on `DEAL_END_DATE_1`* — rejected: null in practice; the end
    date would never display.
  - *Keep the richer `DealBanner` (was→now / Save $X)* — rejected: redundant now
    that Price and Effective Price are explicit rows, and the old site omits it.
  - *Format the end date with `new Date(iso)`* — rejected: UTC-midnight parsing
    shifts the day backwards in negative-offset timezones; `formatShortDate`
    parses `YYYY-MM-DD` by parts.
- **Impact:** Server — `PRODUCT_PROJECTION` swaps `DEAL_END_DATE_1` for
  `TODAYS_DEFAULT_DEAL_END_DATE`; `mapProduct` relaxes the gate and defaults the
  now-unused `name` to `''` (schema `DealInfo.name` stays non-null). Frontend —
  `ProductRow` + `ProductDetailPage` add the Effective Price row and set
  `Price:` to `bulkPrice`; `DealBanner` reduced to the minimal label; new
  `formatShortDate` util; `SEARCH_PRODUCTS` query gains `bulkPrice` +
  `dealInfo.endsAt`. Orphaned deal-banner/pill SCSS removed; label columns
  widened to fit "Effective Price:".

### D011: Loose surcharge bills the loose remainder; `bulk_factor_qt` snapshotted per line (INT-2851)

- **Decision:** The loose surcharge bills only the units beyond whole cartons
  (`looseUnits = bulkFactor > 0 ? quantity % bulkFactor : quantity`), not the
  full line quantity. `bulk_factor_qt` is added to `cart_items` and `order_items`
  (migration `005`) and snapshotted from the catalog at add-to-cart time. For
  legacy `CART_DETAIL` parity, `cart_items` also persists `loose_charge_qt` /
  `loose_charge_am`, refreshed on every write path (`addCartItem` /
  `updateCartItemQty`). Charges remain server-authoritative in `pricing.ts` and
  are recomputed live on every cart read (`getCartWithItems` is the billed truth).
- **Primacy:** Aligns our branch with PR #26
  (`rkordisch:feature/INT-2851-fix-loose-charge`, head `21dda9b`) so our PR
  supersedes and closes it. PR #26 is adopted as the baseline; deviations are
  limited and documented below.
- **Rationale:** The prior formula (`$0.30 × full quantity`) overcharged every
  order of a full carton or more (qty 80 @ carton 80 billed $24 instead of $0).
  Legacy Domino surcharges only the loose remainder; a full carton — or any exact
  multiple — is never charged. `bulkFactor` must be present at compute time, so it
  is captured on the line snapshot alongside price/flag fields.
- **Divergence from PR #26 (documented in code):**
  - *`addCartItem` refreshes `bulk_factor_qt` on the `ON CONFLICT` re-add path*
    (PR #26 bumps only quantity). PR #26 would keep a stale carton factor on
    re-add — and permanently `0` for any pre-migration row — re-introducing the
    overcharge. Our refresh makes the factor track the current catalog and lets
    the migration-window `0` case self-heal (handoff §4.2). Commented in
    `RdsDataSource.addCartItem`.
  - *GraphQL contract left unchanged* (as PR #26 intends): `CartItem` does not
    expose `bulkFactor` / `looseUnits`; only doc comments are corrected. The
    frontend uses `Product.bulkFactor` + the display mirror.
- **Alternatives considered:**
  - *Store only `bulk_factor_qt` and never persist `loose_charge_qt` / `_am` on
    `cart_items`* — considered (handoff §4.3 flagged the extra write) but **not**
    taken: PR #26 has primacy and the columns give legacy `CART_DETAIL` parity for
    INT-2793. Kept consistent via `refreshLooseChargeSnapshot`; pricing stays
    single-sourced in `pricing.ts` (not duplicated in SQL).
  - *Re-derive `bulkFactor` live from the catalog per read* — rejected: breaks the
    snapshot-at-add contract used for price and waiver flags; a placed order must
    reflect the carton factor in effect when the item was added.
  - *Treat `bulkFactor <= 0` as "no loose units"* — rejected: legacy treats absent
    carton grouping as every-unit-loose.
- **Impact:** `cart_items` gains `bulk_factor_qt` / `loose_charge_qt` /
  `loose_charge_am`; `order_items` gains `bulk_factor_qt` (migration `005`).
  `computeLooseUnits` added, `computeLooseCharge` takes `bulkFactor`, and
  `CartItemCharges` gains `looseChargeQt` (server + frontend mirror);
  `RdsDataSource` adds `refreshLooseChargeSnapshot` and threads `bulk_factor_qt`;
  `placeOrder` snapshots the loose remainder into `order_items.loose_charge_qt`.
  Min-order charge still snapshots full quantity (MIN_ORDER hard-coded false on
  `V_PRODUCTS`) — out of scope.

---

### D012: Remote dev RDS is the default database target; localhost is vestigial (INT-2851)

- **Decision:** The dev stack runs against **remote** data by default — the dev
  RDS instance on AWS for cart/order writes and live Snowflake `VIRTUALSALESREP.DEV`
  for reads. Local Docker Postgres (`:5432`) is demoted to a vestigial, secondary
  lab-only fallback. `DATABASE_URL` in `.env.local` selects the target; QA and PROD
  follow the same shape (scaffolded, credentials provisioned separately).
- **Rationale:**
  - Development should exercise the same data services the app uses in higher
    environments, surfacing RDS/Snowflake divergences early rather than against a
    hand-seeded local DB.
  - A single lever (`DATABASE_URL`) keeps target selection explicit and auditable;
    `src/dataSources/db.ts` auto-enables TLS for `*.rds.amazonaws.com` hosts
    (`rejectUnauthorized: false` in dev, strict in prod) so no `sslmode` needs to
    live in the URL.
  - Establishes a clean dev → qa → prod promotion path.
- **Alternatives considered:**
  - *Keep local Postgres as the default* — rejected: hides RDS-specific behavior
    (TLS, network, managed-PG quirks) until late; contradicts the remote-first goal.
  - *Set `NODE_ENV=production` locally to force SSL* — rejected: also flips
    `rejectUnauthorized: true` (dev RDS CA is not in the local trust store) and
    changes unrelated prod semantics. TLS is instead keyed on the host.
  - *Put `sslmode=require` in the connection string* — rejected: `pg` then verifies
    the CA chain and fails on the dev RDS cert; SSL is controlled in `db.ts` instead.
- **Impact:** `.env.shadow` documents remote-first `DATABASE_URL` (localhost
  commented, vestigial) plus QA/PROD templates; `db.ts` gains host-based TLS
  selection; the `dev-stack` skill and the DB-structure report + `tools/vsr-db-bridge`
  sidecar are remote-first (dev primary, qa/prod pathway, local secondary).