Backend catalog recent updates

tests/test_barcode_enrichment_guards.py

@@ -0,0 +1,185 @@
+"""Two guards on the barcode stage: it may not erase, and it may not guess loosely.
+
+Both of these were latent behind `ENABLE_BARCODE_LOOKUP`, which is false in
+production. Switching it on to fill barcodes for newly uploaded products would
+have triggered both at once, so they are fixed before that flag is ever flipped.
+
+1. THE WIPE
+   `BarcodeResult.as_product_fields()` always returns its full nine keys, and a
+   failed lookup makes every one of them None. `EnrichmentStage.apply` merged
+   that dict straight in, so a MISS replaced the barcode the shop had typed with
+   NULL. Verified end to end before the fix: a sheet sending 8901262010016
+   stored None. Since the cascade misses far more often than it hits, enabling
+   the stage would have destroyed more real barcodes than it found.
+
+2. THE LOOSE GATE
+   `matching.is_match` defaults to a 0.45 name-similarity floor. That is a fair
+   general default, but by the time a candidate reaches the gate its brand and
+   pack size have already matched, so the name carries the whole decision. At
+   0.45, more than half the accepted matches in this catalogue were a different
+   product - which is how 33 of the 95 stored barcodes came to be wrong.
+
+Every product name below is real, taken from the live catalogue and from what
+Open Food Facts actually returns for those codes.
+"""
+from __future__ import annotations
+
+import asyncio
+
+import pytest
+
+from app.services.enrichment.barcode import stage as barcode_stage
+from app.services.enrichment.barcode.models import (
+    BarcodeCandidate,
+    BarcodeResult,
+    LookupStatus,
+)
+from app.services.enrichment.barcode.service import BarcodeLookupService
+
+
+class _Service:
+    """Stands in for the cascade. Records what it was asked to look up."""
+
+    def __init__(self, result=None):
+        self.result = result or BarcodeResult.null_result(LookupStatus.NOT_FOUND)
+        self.asked = []
+
+    def lookup_one(self, brand, title, size, category=""):
+        self.asked.append(title)
+        return self.result
+
+
+@pytest.fixture
+def stage_on(monkeypatch):
+    """Turn the stage on and hand back the stub service it will use."""
+    service = _Service()
+    monkeypatch.setattr(barcode_stage, "ENABLE_BARCODE_LOOKUP", True)
+    monkeypatch.setattr(barcode_stage, "get_default_service", lambda: service)
+    return service
+
+
+def _apply(product, brand="Amul"):
+    stage = barcode_stage.BarcodeEnrichmentStage()
+    return asyncio.run(stage.apply(product, brand))
+
+
+# ---------------------------------------------------------------------------
+# 1. The wipe
+# ---------------------------------------------------------------------------
+def test_a_failed_lookup_does_not_erase_the_sheets_barcode(stage_on):
+    """The regression, stated as plainly as it happened."""
+    product = {"product_name": "Amul Butter 500g", "size": "500g",
+               "barcode": "8901262010016", "barcode_type": "EAN-13"}
+
+    _apply(product)
+
+    assert product["barcode"] == "8901262010016"
+    assert product["barcode_type"] == "EAN-13"
+
+
+def test_a_product_that_already_has_a_barcode_is_never_looked_up(stage_on):
+    """Not merely harmless - the lookup is skipped outright.
+
+    A barcode the shop supplied is better evidence than anything the cascade
+    can find: they are holding the pack. Skipping saves the network call too,
+    which on a 2000-row sheet is the difference that matters.
+    """
+    _apply({"product_name": "Amul Butter 500g", "size": "500g",
+            "barcode": "8901262010016"})
+
+    assert stage_on.asked == [], "a row with a barcode reached the network"
+
+
+def test_a_blank_barcode_still_gets_looked_up(stage_on):
+    """The guard must not turn the stage off for the rows it exists to serve."""
+    _apply({"product_name": "Amul Ghee 1L", "size": "1L", "barcode": ""})
+
+    assert stage_on.asked == ["Amul Ghee 1L"]
+
+
+def test_a_found_barcode_fills_a_blank(stage_on, monkeypatch):
+    found = BarcodeResult(barcode="8901262010023", barcode_type="EAN-13",
+                          barcode_verified=True)
+    monkeypatch.setattr(barcode_stage, "get_default_service",
+                        lambda: _Service(result=found))
+    product = {"product_name": "Amul Ghee 1L", "size": "1L"}
+
+    _apply(product)
+
+    assert product["barcode"] == "8901262010023"
+
+
+def test_a_stage_may_still_correct_a_value_just_not_blank_it():
+    """The merge rule is narrow on purpose.
+
+    Overwriting a value with a DIFFERENT value is what correcting a field
+    means and stays allowed; only blanking a held value is refused. A rule
+    that froze every populated field would break legitimate enrichment.
+    """
+    from app.services.enrichment.base import EnrichmentStage, StageOutcome
+
+    class _Correcting(EnrichmentStage):
+        name = "test"
+
+        @property
+        def enabled(self):
+            return True
+
+        async def enrich_one(self, product, brand):
+            return StageOutcome(stage_name=self.name,
+                                fields={"category": "Dairy", "barcode": None})
+
+    product = {"category": "General", "barcode": "8901262010016"}
+    asyncio.run(_Correcting().apply(product, "Amul"))
+
+    assert product["category"] == "Dairy", "a real correction was refused"
+    assert product["barcode"] == "8901262010016", "a held value was blanked"
+
+
+# ---------------------------------------------------------------------------
+# 2. The gate
+# ---------------------------------------------------------------------------
+def _match(off_name, off_brand, off_size, our_brand, our_title, our_size):
+    candidate = BarcodeCandidate(barcode="8906021120418", source_name="OFF",
+                                 candidate_title=off_name,
+                                 candidate_brand=off_brand,
+                                 candidate_size=off_size)
+    return BarcodeLookupService._first_validated_match(
+        [candidate], our_brand, our_title, our_size, set())
+
+
+@pytest.mark.parametrize("off_name,our_title,size", [
+    # Every one of these is a barcode currently stored on the wrong product.
+    ("Chicken Kabab/65 Masala", "Aachi Chicken Masala 50g", "50g"),
+    ("Chicken Curry Masala", "Aachi Chicken Masala 200g", "200g"),
+    ("Tata Tea Gold Care", "Tata Tea Gold 500g", "500g"),
+    ("MTR Chana Masala", "MTR Masala 300g", "300g"),
+    ("PEPER NOTEN", "Lion Dates Powder 100g", "100g"),
+])
+def test_a_different_product_is_no_longer_accepted(off_name, our_title, size):
+    """Brand and size agree in every case; only the name separates them."""
+    brand = our_title.split()[0]
+
+    assert _match(off_name, brand, size, brand, our_title, size) is None
+
+
+def test_the_right_product_is_still_accepted():
+    result = _match("Aachi Mutton Masala", "Aachi", "100g",
+                    "Aachi", "Aachi Mutton Masala 100g", "100g")
+
+    assert result is not None
+    assert result.barcode
+
+
+def test_both_directions_share_one_floor():
+    """A barcode good enough to store must be good enough to read back.
+
+    Two independently-declared copies that drifted apart would give exactly
+    that contradiction, so the forward and reverse paths import the same
+    setting rather than each keeping a constant.
+    """
+    from app.infrastructure.settings import BARCODE_MIN_NAME_SIMILARITY as configured
+    from app.services.enrichment.barcode.service import BARCODE_MIN_NAME_SIMILARITY as forward
+    from app.services.nutrition_data_service import BARCODE_MIN_NAME_SIMILARITY as reverse
+
+    assert forward == reverse == configured

tests/test_nutrition_by_barcode.py

@@ -0,0 +1,245 @@
+"""Looking a product up by the barcode on its pack, rather than by its name.
+
+WHAT IS NEW HERE
+----------------
+Every other Open Food Facts call in this project searches by brand and product
+name and scores whatever comes back. That is why OFF-sourced rows in
+`nutrition_facts` sit at match confidences as low as 0.32 - the module's own
+`MIN_MATCH_CONFIDENCE`. A barcode is the identifier printed on the pack, so
+`/api/v2/product/{code}` returns that product or nothing.
+
+WHY THERE IS STILL A GATE
+-------------------------
+The lookup is exact. The STORED BARCODE is not. Measured across all 95 barcoded
+catalogue rows, a third of the records found this way described a different
+product, because the barcode on our row was wrong. Every string in the tests
+below is real - taken from that run, not invented - so if the gate is loosened
+these fail with the actual products it would let through.
+
+No network: `requests.get` is stubbed at the boundary, as the existing nutrition
+tests do.
+"""
+from __future__ import annotations
+
+import pytest
+
+from app.services import nutrition_data_service as nds
+from app.services.enrichment.barcode.sources import open_food_facts as off
+
+
+class _Resp:
+    def __init__(self, payload, status_code=200):
+        self._payload = payload
+        self.status_code = status_code
+
+    def json(self):
+        return self._payload
+
+
+def _product(name, brands, quantity, nutriments=None, code="8906021122290"):
+    return {
+        "code": code,
+        "product_name": name,
+        "brands": brands,
+        "quantity": quantity,
+        "countries_tags": ["en:india"],
+        "nutriments": nutriments if nutriments is not None else {
+            "energy-kcal_100g": 388.2,
+            "proteins_100g": 12.0,
+            "fat_100g": 15.0,
+            "carbohydrates_100g": 45.0,
+        },
+    }
+
+
+@pytest.fixture
+def off_returns(monkeypatch):
+    """Stub the HTTP boundary. Returns a setter taking the JSON body."""
+    box = {}
+
+    def _get(url, **kwargs):
+        if "status" in box and box["status"] is None:
+            raise TimeoutError("simulated network timeout")
+        return _Resp(box.get("body", {"status": 0}))
+
+    monkeypatch.setattr(off.requests, "get", _get)
+    return box
+
+
+# ---------------------------------------------------------------------------
+# The lookup itself
+# ---------------------------------------------------------------------------
+def test_a_known_barcode_returns_the_product(off_returns):
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Aachi Mutton Masala", "Aachi", "100g")}
+
+    product = off.fetch_product_by_barcode("8906021122290")
+
+    assert product["product_name"] == "Aachi Mutton Masala"
+
+
+def test_an_unknown_barcode_is_none_not_an_error(off_returns):
+    """OFF answers HTTP 200 with `status: 0` for a code it has never seen.
+
+    Checking the HTTP code alone would treat that as a hit and hand the caller
+    an empty product. About a third of our barcodes land here, so this is the
+    ordinary path, not an exceptional one.
+    """
+    off_returns["body"] = {"status": 0, "status_verbose": "product not found"}
+
+    assert off.fetch_product_by_barcode("0000000000000") is None
+
+
+def test_an_empty_barcode_makes_no_network_call(monkeypatch):
+    called = []
+    monkeypatch.setattr(off.requests, "get",
+                        lambda *a, **k: called.append(1) or _Resp({"status": 0}))
+
+    assert off.fetch_product_by_barcode("") is None
+    assert off.fetch_product_by_barcode("   ") is None
+    assert not called, "a blank barcode should never reach the network"
+
+
+def test_a_network_failure_returns_none_rather_than_raising(off_returns):
+    """One unreachable host must not take down a whole backfill run."""
+    off_returns["status"] = None  # makes the stub raise
+
+    assert off.fetch_product_by_barcode("8906021122290") is None
+
+
+# ---------------------------------------------------------------------------
+# The gate - every string below came from the real catalogue
+# ---------------------------------------------------------------------------
+def test_the_right_product_is_accepted(off_returns):
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Aachi Mutton Masala", "Aachi", "100g")}
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "8906021122290", "Aachi", "Aachi Mutton Masala 100g", "100g")
+
+    assert facts["data_status"] == "verified"
+    assert facts["data_source"] == nds.BARCODE_DATA_SOURCE
+    assert facts["source_ref"] == "8906021122290"
+    assert facts["calories_kcal"] == 388.2
+
+
+def test_a_different_product_under_the_same_barcode_is_refused(off_returns):
+    """The real failure this gate exists for.
+
+    Barcode 8906021120418 is stored on our "Aachi Chicken Masala 50g", but Open
+    Food Facts holds it as "Chicken Kabab/65 Masala" - a different spice blend.
+    Brand and pack size both agree, so the name is the only thing that can tell
+    them apart, and it scores 0.538.
+    """
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Chicken Kabab/65 Masala", "Aachi", "50g")}
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "8906021120418", "Aachi", "Aachi Chicken Masala 50g", "50g")
+
+    assert facts["data_status"] == "unavailable"
+    assert "calories_kcal" not in facts
+
+
+def test_a_near_miss_variant_is_refused(off_returns):
+    """"Tata Tea Gold" and "Tata Tea Gold Care" are different products.
+
+    This one scores 0.761 - above matching.py's own 0.45 default, which is why
+    the barcode path sets its own, higher floor rather than reusing it.
+    """
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Tata Tea Gold Care", "Tata", "500g")}
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "8901030873829", "Tata", "Tata Tea Gold 500g", "500g")
+
+    assert facts["data_status"] == "unavailable"
+
+
+def test_nonsense_is_refused(off_returns):
+    """Barcode 20086039 is stored on our Lion Dates Powder; OFF has it as a
+    Dutch confection. Scores 0.097."""
+    off_returns["body"] = {"status": 1, "product": _product(
+        "PEPER NOTEN", "Favorina", "300 g")}
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "20086039", "Lion Dates", "Lion Dates Powder 100g", "50g")
+
+    assert facts["data_status"] == "unavailable"
+
+
+def test_a_matching_record_with_no_usable_values_is_unavailable(off_returns):
+    """Our product, but Open Food Facts holds a near-empty record for it.
+
+    Distinct from a mismatch and must not be reported as one: the barcode is
+    right, there is simply nothing to import. Real case - OFF has five
+    nutriment keys for Aachi Chicken Masala 100g and no values among them.
+    """
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Aachi Mutton Masala", "Aachi", "100g", nutriments={"nova_group": 4})}
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "8906021122290", "Aachi", "Aachi Mutton Masala 100g", "100g")
+
+    assert facts["data_status"] == "unavailable"
+
+
+def test_the_floor_can_be_relaxed_per_call(off_returns):
+    """The threshold is a judgement, not a constant, so it is a parameter.
+
+    "Aachi Biryani Masala" against our "Aachi Biryani Masala 50 g" is plainly
+    the same product and scores 0.716 - below the conservative default. The
+    backfill script exposes this as --min-similarity for exactly this reason.
+    """
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Aachi Biryani Masala", "Aachi", "50 g")}
+    args = ("8906021120272", "Aachi", "Aachi Biryani Masala 50 g", "50 g")
+
+    assert nds.fetch_verified_nutrition_by_barcode(*args)["data_status"] == "unavailable"
+    relaxed = nds.fetch_verified_nutrition_by_barcode(*args, min_name_similarity=0.70)
+    assert relaxed["data_status"] == "verified"
+
+
+# ---------------------------------------------------------------------------
+# The two paths must stay interchangeable
+# ---------------------------------------------------------------------------
+def test_the_barcode_path_returns_the_same_shape_as_the_name_path(off_returns):
+    """Both feed the same `upsert_nutrition_facts`, so they cannot drift.
+
+    If the barcode path ever grew a key the name path lacks, the writer would
+    silently drop it - the INSERT is built from a fixed column list.
+    """
+    off_returns["body"] = {"status": 1, "product": _product(
+        "Aachi Mutton Masala", "Aachi", "100g")}
+
+    by_barcode = nds.fetch_verified_nutrition_by_barcode(
+        "8906021122290", "Aachi", "Aachi Mutton Masala 100g", "100g")
+
+    # `name_similarity` is diagnostic and unique to this path; everything else
+    # must exist on the name path too.
+    extra = set(by_barcode) - set(_name_path_keys())
+    assert extra == {"name_similarity"}, f"unexpected new keys: {extra}"
+
+
+def _name_path_keys():
+    """The keys `fetch_verified_nutrition` produces on a successful match."""
+    return {
+        "data_status", "data_source", "source_ref", "source_url",
+        "match_confidence", "serving_size_g", "serving_size_label",
+        "extended_nutrients", "per_serving", "ingredients_text",
+        "off_nutriscore", "allergens", "off_labels_tags",
+        "off_ingredients_analysis_tags", "off_categories_tags", "fetched_at",
+    } | set(nds._build_flat_fields({}, "100g"))
+
+
+def test_a_disabled_open_facts_makes_no_call(monkeypatch):
+    called = []
+    monkeypatch.setattr(off.requests, "get",
+                        lambda *a, **k: called.append(1) or _Resp({"status": 0}))
+    monkeypatch.setattr(nds, "USE_OPEN_FACTS", False)
+
+    facts = nds.fetch_verified_nutrition_by_barcode(
+        "8906021122290", "Aachi", "Aachi Mutton Masala 100g", "100g")
+
+    assert facts["data_status"] == "unavailable"
+    assert not called