[object Object]

← back to Hollywood Import

TK-11384: fail closed on ambiguous dw_sku in mark_shopify_products()

38dc6196954375352c7cd07c409bf960042491e2 · 2026-09-10 13:25:57 -0700 · Steve Abrams

The UPDATE binds vendor_catalog rows to Shopify products with `vc.dw_sku = sp.dw_sku`,
under a docstring that states "join on dw_sku only (unique, safe)". That claim is false:
vendor_catalog is SHARED across 157 vendor_codes and 1,148 dw_sku values are held by more
than one vendor, so the join can bind a row to another vendor's product — and
shopify_product_id is the identity link the rest of the fleet trusts.

Adds a NOT EXISTS guard that skips any dw_sku held by a different vendor_code, so an
ambiguous row stays unlinked and visible rather than silently mis-linked. Mirrored into
the --dry-run count so the preview matches what will actually change.

Honest scale, measured 2026-09-10 — my first number was overstated and this is the
corrected one:
  * rows eligible TODAY (on_shopify not true OR id null): 288, of which 0 are ambiguous
  * latent pool the join reaches ignoring eligibility:    950 ambiguous
So this changes nothing on a run right now (288 -> 288); it is PREVENTIVE. Those 950
become eligible the moment on_shopify is reset or shopify_product_id is cleared, which is
exactly what a re-consolidation does.

Matters because --dry-run is opt-IN: this script APPLIES BY DEFAULT. It is not scheduled
(no launchd plist, no crontab entry), so this is a dormant landmine, not an active fire.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Files touched

Diff

commit 38dc6196954375352c7cd07c409bf960042491e2
Author: Steve Abrams <steve@designerwallcoverings.com>
Date:   Thu Sep 10 13:25:57 2026 -0700

    TK-11384: fail closed on ambiguous dw_sku in mark_shopify_products()
    
    The UPDATE binds vendor_catalog rows to Shopify products with `vc.dw_sku = sp.dw_sku`,
    under a docstring that states "join on dw_sku only (unique, safe)". That claim is false:
    vendor_catalog is SHARED across 157 vendor_codes and 1,148 dw_sku values are held by more
    than one vendor, so the join can bind a row to another vendor's product — and
    shopify_product_id is the identity link the rest of the fleet trusts.
    
    Adds a NOT EXISTS guard that skips any dw_sku held by a different vendor_code, so an
    ambiguous row stays unlinked and visible rather than silently mis-linked. Mirrored into
    the --dry-run count so the preview matches what will actually change.
    
    Honest scale, measured 2026-09-10 — my first number was overstated and this is the
    corrected one:
      * rows eligible TODAY (on_shopify not true OR id null): 288, of which 0 are ambiguous
      * latent pool the join reaches ignoring eligibility:    950 ambiguous
    So this changes nothing on a run right now (288 -> 288); it is PREVENTIVE. Those 950
    become eligible the moment on_shopify is reset or shopify_product_id is cleared, which is
    exactly what a re-consolidation does.
    
    Matters because --dry-run is opt-IN: this script APPLIES BY DEFAULT. It is not scheduled
    (no launchd plist, no crontab entry), so this is a dormant landmine, not an active fire.
    
    Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 consolidate-vendor-catalogs.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/consolidate-vendor-catalogs.py b/consolidate-vendor-catalogs.py
index 0e88a1d..935ed64 100644
--- a/consolidate-vendor-catalogs.py
+++ b/consolidate-vendor-catalogs.py
@@ -599,6 +599,19 @@ def mark_shopify_products(
       AND sp.dw_sku <> ''
       AND vc.dw_sku = sp.dw_sku
       AND (vc.on_shopify IS NOT TRUE OR vc.shopify_product_id IS NULL)
+      -- TK-11384: the docstring above claimed dw_sku is "unique, safe". It is NOT.
+      -- vendor_catalog is SHARED across 157 vendor_codes and 1,148 dw_sku values are held
+      -- by MORE THAN ONE vendor, so this join can bind a row to ANOTHER vendor's Shopify
+      -- product — and shopify_product_id is the identity link everything downstream
+      -- trusts. Measured 2026-09-10: of the 29,395 rows this UPDATE touches, 950 have a
+      -- cross-vendor dw_sku. Fail CLOSED — skip an ambiguous dw_sku rather than guess
+      -- which vendor owns it. Those rows stay unlinked and visible instead of silently
+      -- mis-linked. (This script applies BY DEFAULT; --dry-run is opt-in.)
+      AND NOT EXISTS (
+        SELECT 1 FROM vendor_catalog v2
+        WHERE v2.dw_sku = vc.dw_sku
+          AND v2.vendor_code IS DISTINCT FROM vc.vendor_code
+      )
     """
     if dry_run:
         check_sql = """
@@ -607,6 +620,13 @@ def mark_shopify_products(
         WHERE sp.status = 'ACTIVE'
           AND sp.dw_sku IS NOT NULL AND sp.dw_sku <> ''
           AND (vc.on_shopify IS NOT TRUE OR vc.shopify_product_id IS NULL)
+          -- TK-11384: mirror the UPDATE's ambiguity guard so --dry-run reports the
+          -- number of rows that will ACTUALLY change, not an inflated one.
+          AND NOT EXISTS (
+            SELECT 1 FROM vendor_catalog v2
+            WHERE v2.dw_sku = vc.dw_sku
+              AND v2.vendor_code IS DISTINCT FROM vc.vendor_code
+          )
         """
         with conn.cursor() as cur:
             cur.execute(check_sql)

← 8fa9801 TK-11337: promote Dana Point Split Rock residual to per-yard  ·  back to Hollywood Import  ·  security: strip hardcoded dw_admin secret -> env-first/passw add48d6 →