fix(products): conservative dialect detection — canonical-distinctive columns veto Shopify (SLICE-8 review)

Addresses adversarial-review findings: a canonical file carrying a stray
Shopify-signature name (e.g. SEO Title) no longer misdetects as Shopify and
silently drops canonical Type / corrupts the weight unit (kg->g). A
canonical-distinctive column (Description/Variant Cost/Variant Weight/Google
Product Category/Variant Volume/Tax ID/Position) now vetoes detection, so
detection leans conservative (under-detection warns; over-detection corrupts).
Also closes the dual-named-column shadow (Body (HTML)+Description) and the
stale-weight-unit-on-clear case. Tests cover all three.

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

backend/app/domains/products/codec.py

@@ -73,9 +73,13 @@ def parse_csv(data: bytes) -> ParsedFile:
                 c: (raw[col_index[c]].strip() if col_index[c] < len(raw) else "")
                 for c in col_index
             }
-            # Shopify grams carry an implicit unit; canonical needs it explicit (§6.5.1).
-            if dialect == "shopify" and cells.get("Variant Weight"):
-                cells["Variant Weight Unit"] = "g"
+            # Shopify grams carry an implicit unit; canonical needs it explicit. In a
+            # Shopify file "Variant Weight" can only come from the Variant Grams rename
+            # (a canonical-named Variant Weight would have vetoed Shopify detection), so
+            # weight and unit move together: a value -> "g"; a cleared grams (present-but-
+            # empty) clears the unit too, never leaving a stale unit (§6.5.1).
+            if dialect == "shopify" and "Variant Weight" in cells:
+                cells["Variant Weight Unit"] = "g" if cells["Variant Weight"] else ""
             rows.append(Row(line_number=reader.line_num, cells=cells))
     except csv.Error:
         raise FileRejected("not_csv", "This file isn't readable as CSV.") from None

backend/app/domains/products/dialect_shopify.py

@@ -38,12 +38,36 @@ _SIGNATURE: frozenset[str] = frozenset(
     }
 )
 
+# Canonical-distinctive columns — names Shopify renames away (so a real Shopify export
+# never carries them) plus canonical-only columns Shopify has no equivalent for. Their
+# presence proves the file is canonical and VETOES Shopify detection: detection must
+# lean conservative, because under-detection is safe (Shopify-named columns are warned
+# as not-imported) while over-detection corrupts (canonical Type / weight-unit are
+# dropped). This closes the misdetection hazard — a canonical file that merely contains
+# a stray signature-named column (e.g. `SEO Title`) is never misread as Shopify (§7.4).
+_CANONICAL_SIGNATURE: frozenset[str] = frozenset(
+    {
+        "Description",
+        "Variant Cost",
+        "Variant Weight",
+        "Google Product Category",
+        "Variant Volume",
+        "Variant Volume Unit",
+        "Variant Tax ID 1",
+        "Variant Tax ID 2",
+        "Variant Position",
+    }
+)
+
 
 def is_shopify_header(header: list[str]) -> bool:
-    """True iff the header carries a Shopify-only signature column. Otherwise the
-    file is treated as canonical — shared columns map identically, so an ambiguous
-    header never misparses (§7.4)."""
+    """True iff the header carries a Shopify-only signature column AND no
+    canonical-distinctive column. A canonical signal vetoes Shopify detection so an
+    ambiguous or hybrid header is treated as canonical — where Shopify-named columns
+    are warned as not-imported rather than silently remapped (never misparses, §7.4)."""
     cols = {h.strip() for h in header}
+    if cols & _CANONICAL_SIGNATURE:
+        return False
     if cols & _SIGNATURE:
         return True
     return any(c.startswith("Google Shopping / ") for c in cols)

backend/tests/test_products_dialect_shopify.py

@@ -76,3 +76,46 @@ def test_shopify_fixture_maps_exhaustively():
     ):
         assert col in parsed.unknown_columns, col
     assert "Variant Compare At Price" not in first
+
+
+# --- Detection robustness (review findings #1, #2): bias conservative ----------------
+
+
+def test_canonical_distinctive_column_vetoes_shopify_detection():
+    # A canonical file carrying a stray Shopify-signature name (`SEO Title`) stays
+    # canonical because it also has canonical-distinctive columns — no misparse,
+    # no dropped Type, no corrupted weight unit (review finding #2).
+    header = ["Handle", "Title", "Type", "Variant Weight", "Variant Weight Unit", "SEO Title"]
+    assert is_shopify_header(header) is False
+
+
+def test_dual_named_file_stays_canonical_and_warns_shopify_name():
+    # A malformed file with BOTH `Body (HTML)` and canonical `Description`: the
+    # canonical column vetoes detection, so `Body (HTML)` is honestly warned as an
+    # unknown column rather than silently shadowing `Description` (review finding #1).
+    data = (
+        b"Handle,Title,Body (HTML),Description,Variant Price\n"
+        b"mug,Moon Mug,FROM_BODY,FROM_DESC,18.00\n"
+    )
+    parsed = parse_csv(data)
+    assert parsed.dialect == "canonical"
+    assert parsed.rows[0].cells["Description"] == "FROM_DESC"
+    assert "Body (HTML)" in parsed.unknown_columns
+
+
+def test_real_shopify_export_still_detected():
+    # The conservative veto must not break a genuine Shopify export (no canonical-
+    # distinctive columns present).
+    parsed = parse_csv(_FIXTURE.read_bytes())
+    assert parsed.dialect == "shopify"
+
+
+def test_shopify_grams_clear_clears_weight_unit_together():
+    # An empty Variant Grams (a clear) clears the synthesized unit too — weight and
+    # unit move together, never a stale unit (review finding #3).
+    data = b"Handle,Title,Body (HTML),Variant Grams\nmug,Moon Mug,<p>x</p>,\n"
+    parsed = parse_csv(data)
+    assert parsed.dialect == "shopify"
+    cells = parsed.rows[0].cells
+    assert cells["Variant Weight"] == ""
+    assert cells["Variant Weight Unit"] == ""

docs/products-domain.md

@@ -72,11 +72,16 @@ mapping (mirrored by `tests/test_products_dialect_shopify.py` and
 
 - **Detection** — `is_shopify_header()` returns `shopify` iff the header carries a
   Shopify-only *signature* column (`Body (HTML)`, `Variant Grams`, `Cost per item`,
-  `Gift Card`, a `Google Shopping / …` column, …). Everything else is `canonical`.
-  Detection is by signature, not an exact header set, so extra market/region columns
-  don't defeat it; an ambiguous header (only columns common to both dialects) falls
-  through to `canonical`, which maps those shared columns identically — so it never
-  misparses (§7.4).
+  `Gift Card`, a `Google Shopping / …` column, …) **and no canonical-distinctive
+  column**. A canonical-distinctive name — one Shopify renames away (`Description`,
+  `Variant Cost`, `Variant Weight`, `Google Product Category`) or a canonical-only
+  column Shopify lacks (`Variant Volume`, `Variant Tax ID 1/2`, `Variant Position`) —
+  **vetoes** Shopify detection. Detection is by signature, not an exact header set, so
+  extra market/region columns don't defeat it; the veto biases it conservative, because
+  under-detection is safe (Shopify-named columns are warned as not-imported) while
+  over-detection corrupts (canonical `Type` / `Variant Weight Unit` would be dropped).
+  So a hybrid or ambiguous header falls through to `canonical`, which maps shared
+  columns identically and warns the rest — it never misparses (§7.4).
 - **Mapping** — `map_shopify_header(header)` returns `(mapped, not_imported)`:
   `mapped[i]` is the canonical name for column `i` (or `None` when it has no
   canonical home), and `not_imported` is the warning list surfaced as the draft's