API-contract integration tests, and fold in the measured properties

.github/workflows/ci.yml

@@ -23,7 +23,12 @@ jobs:
         run: ruff check .
       - name: Install runtime deps
         run: pip install -r requirements.txt
-      - name: Pytest (unit)
+        # fastapi + httpx back the TestClient integration tests (tests/test_api_contract.py).
+        # Without them that file skips itself rather than failing, but then CI silently stops
+        # checking every endpoint contract — so install them explicitly.
+      - name: Install test-only deps
+        run: pip install fastapi httpx pydantic
+      - name: Pytest (unit + integration)
         run: |
           if git ls-files '*test_*.py' '*_test.py' 'tests/**/*.py' | grep -qv '^tests/e2e/'; then
             pytest -q --ignore=tests/e2e

tests/test_api_contract.py

@@ -0,0 +1,154 @@
+"""Integration tests: every endpoint's CONTRACT, through FastAPI's TestClient.
+
+The suite already had unit tests (rules, physchem, stereo-safety) and Playwright e2e tests that
+drive a real browser against a real server. This is the missing middle: it exercises the actual
+HTTP layer — routing, request parsing, response shape, status codes — without a browser, without a
+port, and without waiting fifty seconds for models to load.
+
+That gap mattered. A response that silently drops a field, renames a key, or starts returning 200
+with an error body would sail past the unit tests (which never touch HTTP) and might well pass the
+e2e tests too (which assert on rendered pixels, not payloads). The workbench, the MCP server and
+the Claude skill all consume these payloads by key, so a renamed field is a breaking change that
+nothing else in the pyramid would catch.
+
+Runs in CI with FLAVORMANCER_NO_MODELS=1: endpoints that need trained heads answer honestly with
+`available: false` rather than 500, and that degradation is itself worth asserting — it is what
+keeps a partially-built install usable instead of broken.
+"""
+import os
+
+import pytest
+
+# Import the app the same way uvicorn does. NO_MODELS keeps this fast and CI-friendly; the tests
+# below assert the *contract*, which must hold whether or not the heads are present.
+os.environ.setdefault("FLAVORMANCER_NO_MODELS", "1")
+
+# importorskip is not enough here: `fastapi.testclient` imports cleanly and then starlette raises
+# a RuntimeError if httpx is missing, which pytest reports as a collection ERROR rather than a
+# skip. Catch both so an environment without the test extras skips this file instead of failing.
+try:
+    from fastapi.testclient import TestClient
+except (ImportError, RuntimeError) as exc:  # pragma: no cover — environment-dependent
+    pytest.skip(f"FastAPI TestClient unavailable ({exc})", allow_module_level=True)
+
+import app as flavor_app
+
+VANILLIN = "COc1cc(C=O)ccc1O"
+
+
+@pytest.fixture(scope="module")
+def client():
+    with TestClient(flavor_app.app) as c:
+        yield c
+
+
+# --- liveness -------------------------------------------------------------------------------
+
+def test_healthz_is_always_answerable(client):
+    """/healthz must answer even while models are loading — it is what the container's health
+    check polls, and a 503 during a legitimate 50s warm-up would kill the container."""
+    r = client.get("/healthz")
+    assert r.status_code == 200, r.text
+
+
+def test_status_reports_load_progress(client):
+    r = client.get("/api/status")
+    assert r.status_code == 200
+    body = r.json()
+    for key in ("loaded", "total", "phase", "ready"):
+        assert key in body, f"/api/status lost `{key}` — the warming page polls it"
+    assert isinstance(body["ready"], bool)
+
+
+# --- head catalog ---------------------------------------------------------------------------
+
+def test_heads_catalog_shape(client):
+    """The catalog is consumed by the modal, the studios, the MCP server and the skill. Its four
+    categories and their per-head fields are a contract, not an implementation detail."""
+    r = client.get("/api/heads")
+    assert r.status_code == 200
+    body = r.json()
+    for group in ("taste", "aroma", "mouthfeel", "safety"):
+        assert group in body, f"/api/heads lost the `{group}` group"
+        assert isinstance(body[group], list)
+
+
+def test_every_head_publishes_its_calibration(client):
+    """Since #261 a head must say where its bar sits and how precise it is there. Dropping these
+    would silently return the project to reporting AUROC alone — the exact failure ACCURACY.md
+    exists to prevent."""
+    body = client.get("/api/heads").json()
+    for group in ("taste", "aroma", "mouthfeel", "safety"):
+        for head in body[group]:
+            assert "head" in head
+            assert "confident_capable" in head, (
+                f"{group}/{head.get('head')} has no confident_capable flag")
+            thr = head.get("threshold")
+            if thr is not None:
+                assert 0.0 < float(thr) <= 1.0, f"{head['head']} threshold {thr} out of range"
+
+
+# --- prediction -----------------------------------------------------------------------------
+
+def test_predict_returns_the_documented_blocks(client):
+    r = client.post("/api/predict", json={"smiles": VANILLIN})
+    assert r.status_code == 200, r.text
+    body = r.json()
+    assert body.get("smiles")
+    # physchem and the applicability gate are computed from structure alone, so they are present
+    # even with no trained heads at all — that is what makes a models-less install still useful.
+    assert "physchem" in body
+    assert "applicability" in body
+
+
+def test_predict_rejects_an_unparseable_smiles(client):
+    """Garbage in must produce a clear error, not a 500 and not a confident-looking empty read."""
+    r = client.post("/api/predict", json={"smiles": "not-a-molecule"})
+    assert r.status_code < 500, "an unparseable SMILES must not crash the service"
+    body = r.json()
+    assert "error" in body or body.get("applicability", {}).get("in_domain") is False, (
+        "an unparseable SMILES should surface an error, not a silent empty result")
+
+
+def test_predict_requires_smiles(client):
+    r = client.post("/api/predict", json={})
+    assert r.status_code < 500
+
+
+# --- search surfaces ------------------------------------------------------------------------
+
+def test_design_search_contract(client):
+    """The reverse search backs the studios. Its envelope (items / requested / total_matches)
+    is what the UI paginates against."""
+    r = client.get("/api/design", params={"descriptors": "vanilla", "limit": 3})
+    assert r.status_code == 200
+    body = r.json()
+    for key in ("items", "requested", "total_matches", "offset", "limit"):
+        assert key in body, f"/api/design lost `{key}`"
+    assert isinstance(body["items"], list)
+    assert len(body["items"]) <= 3, "limit must be honoured"
+
+
+def test_design_with_an_unknown_descriptor_is_empty_not_an_error(client):
+    r = client.get("/api/design", params={"descriptors": "notarealnote"})
+    assert r.status_code == 200
+    assert r.json()["items"] == []
+
+
+def test_studio_terms_groups(client):
+    r = client.get("/api/studio_terms")
+    assert r.status_code == 200
+    body = r.json()
+    for group in ("flavors", "notes", "mouthfeel", "taste"):
+        assert group in body, f"/api/studio_terms lost `{group}` — a chip family would vanish"
+
+
+# --- degradation ----------------------------------------------------------------------------
+
+def test_model_backed_endpoints_degrade_honestly(client):
+    """With no heads loaded, endpoints that need them must say so rather than 500 or, worse,
+    return an empty read that looks like a confident 'no aroma'."""
+    body = client.post("/api/predict", json={"smiles": VANILLIN}).json()
+    aroma = body.get("aroma")
+    if isinstance(aroma, dict) and aroma.get("available") is False:
+        assert aroma.get("note"), "an unavailable modality must explain why"

training/build_enrichment.py

@@ -248,6 +248,7 @@ def _taste_by_skel():
     for _skel, _row in _by_skel("iupac_backfill.parquet", ["common_name", "iupac_name"]).items():
         tgt = props.setdefault(_skel, {})
         for _k in ("common_name", "iupac_name"):
+            # isinstance(..., str) is the right guard here — it treats NaN as absent correctly
             if not isinstance(tgt.get(_k), str) and isinstance(_row.get(_k), str):
                 tgt[_k] = _row[_k]
     # MEASURED boiling/melting points from PUG-View (build_measured_properties.py). The property
@@ -257,7 +258,11 @@ def _taste_by_skel():
                                 ["melting_point_c", "boiling_point_c"]).items():
         tgt = props.setdefault(_skel, {})
         for _k in ("melting_point_c", "boiling_point_c"):
-            if tgt.get(_k) is None and _row.get(_k) is not None:
+            # `is None` is WRONG here and silently discarded the entire crawl: a missing value
+            # read from parquet is NaN, not None, so the guard never fired for any row that
+            # already existed in properties.parquet with an empty cell — which is all of them.
+            # Same NaN trap that once left ~500 molecules unnamed (see _pick_name).
+            if pd.isna(tgt.get(_k)) and not pd.isna(_row.get(_k)):
                 tgt[_k] = _row[_k]
     taste_doc = _taste_by_skel()
     curated = _curated_names()   # human names for the molecules we hand-curated

training/predict.py

@@ -266,9 +266,22 @@ def _load_all_models():
     """Discover and load every trained head (taste + tox + aroma), updating LOAD_PROGRESS as each
     lands, then set MODELS_READY. Runs on a daemon thread from import so the port binds instantly.
 
-    Loading is SERIAL on purpose: joblib.load is dominated by GIL-bound Python unpickling, so a
-    thread pool only adds contention (measured ~34 s serial vs ~86 s across 14 threads). The win
-    from cores comes at INFERENCE time (predict_proba releases the GIL) — see _aroma_scores_canon."""
+    Loading is SERIAL, and BOTH ways of parallelising it have now been measured and rejected:
+
+      threads    WORSE than serial (~34 s serial vs ~86 s across 14 threads). joblib.load is
+                 dominated by GIL-bound Python unpickling, so threads only add contention.
+      processes  3.1x FASTER in isolation — 40 files take 11.5 s serial and 3.7 s across a
+                 ProcessPoolExecutor, transfer of the deserialised forests included. But this
+                 function runs DURING MODULE IMPORT, and forking while the interpreter holds the
+                 import lock deadlocks: the children inherit a locked import machinery they can
+                 never acquire. Tried it; the parent and every worker hung indefinitely.
+
+    The process pool is the right answer, but only once loading is deferred out of import time
+    (a FastAPI startup hook, or an explicit warm() the server calls) — see #225. Until then serial
+    is correct, and the warming page makes the ~50 s visible rather than mysterious.
+
+    The win from cores comes at INFERENCE time instead (predict_proba releases the GIL) — see
+    _aroma_scores_canon."""
     jobs = []  # (kind, name, path)
     if TASTE.exists():
         jobs += [("taste", p.stem.replace("_rf", ""), p) for p in TASTE.glob("*_rf.joblib")]
@@ -323,7 +336,14 @@ def _load_all_models():
 # only need featurization (_feat / _MORGAN), like the parallel index builder, which loads each head
 # in its own worker process rather than in this parent.
 if os.environ.get("FLAVORMANCER_NO_MODELS") == "1":
-    pass
+    # Skip loading, but still mark READY. Leaving the event unset meant the app's warming gate
+    # returned 503 to every request FOREVER — a models-less install was not "degraded", it was
+    # dead, which is the opposite of what the docs promised. Structure-derived answers (physchem,
+    # the sour/salty rules, applicability, substructure) need no heads at all and should be served.
+    with _LOAD_LOCK:
+        LOAD_PROGRESS["phase"] = "ready (no models)"
+        LOAD_PROGRESS["ready"] = True
+    MODELS_READY.set()
 elif os.environ.get("FLAVORMANCER_BLOCKING_LOAD") == "1":
     _load_all_models()
 else: