fix: refuse to start a run on an uncalibrated embryo

gently/app/tools/acquisition_tools.py

@@ -11,6 +11,7 @@
 
 import numpy as np
 
+from gently.harness import calibration_gate
 from gently.harness.tools.helpers import ctx_get, get_embryo_or_error
 from gently.harness.tools.registry import ToolCategory, ToolExample, tool
 
@@ -103,9 +104,17 @@ async def acquire_volume(
     num_slices: int = 50,
     exposure_ms: float = 10.0,
     z_buffer_um: float | None = None,
+    allow_uncalibrated: bool = False,
     context: dict | None = None,
 ) -> str:
-    """Acquire single volume - moves to embryo first, uses calibration"""
+    """Acquire single volume - moves to embryo first, uses calibration.
+
+    Refuses an uncalibrated embryo unless ``allow_uncalibrated``. The four
+    galvo/piezo parameters below used to fall back to literals with the comment
+    "use defaults if not calibrated" — so a volume acquired from an embryo that
+    had never been calibrated used invented geometry and reported success. The
+    resulting data is indistinguishable from real data, which is the problem.
+    """
     agent = ctx_get(context, "agent")
     client = ctx_get(context, "client")
 
@@ -116,13 +125,18 @@ async def acquire_volume(
     if err:
         return err
 
+    if not allow_uncalibrated and not calibration_gate.is_calibrated(embryo):
+        return f"Error: {calibration_gate.refusal_detail([embryo_id])}"
+
     try:
         # Move to embryo position first
         pos = embryo.stage_position
         if pos and pos.get("x") is not None and pos.get("y") is not None:
             await client.move_to_position(pos["x"], pos["y"])
 
-        # Get calibration parameters (use defaults if not calibrated)
+        # Calibrated by the time we get here, unless the caller explicitly
+        # accepted a guess. The literals below are that guess, and they are
+        # only ever reached on that path.
         cal = embryo.calibration or {}
         galvo_amplitude = cal.get("galvo_amplitude", 0.5)
         galvo_center = cal.get("galvo_center", 0.0)

gently/harness/calibration_gate.py

@@ -0,0 +1,86 @@
+"""Has this embryo actually been calibrated?
+
+One predicate, used by the run routes and by `acquire_volume`, so the answer
+cannot differ between the surface an operator drives and the tool an agent
+calls.
+
+WHY THIS EXISTS
+
+Nothing checked. `POST /api/devices/timelapse/start` validated
+`interval_seconds > 0` and that the embryo ids existed, and started. And the
+acquisition layer did not fail on an uncalibrated embryo either — it invented a
+complete set of scan geometry, with the comment saying so:
+
+    # Get calibration parameters (use defaults if not calibrated)
+    galvo_amplitude = cal.get("galvo_amplitude", 0.5)
+    galvo_center    = cal.get("galvo_center", 0.0)
+    piezo_amplitude = cal.get("piezo_amplitude", 25.0)
+    piezo_center    = cal.get("piezo_center", 50.0)
+    slope           = cal.get("slope_um_per_deg", 100.0)
+
+So a timelapse could run to completion on embryos that had never been
+calibrated, using five made-up numbers, and report success. The data looks
+exactly like real data. That is worse than a refusal, because a refusal is
+noticed.
+
+It also inverted the order the team stated out loud on 2026-08-07 — Ryan: "the
+main thing is just making sure that we can get the calibration to work";
+Kesavan: "Calibration has to work. Embryo navigation has to work. Then
+timelapse setup has to work." The workflow has a hard dependency; now the code
+has one too.
+
+WHAT COUNTS AS CALIBRATED
+
+A finite `slope_um_per_deg` in `embryo.calibration`. Every successful fit
+writes it (`calibration_tools.py:891`), and it is the number the scan geometry
+is derived from — without it there is nothing to derive from but a guess.
+
+Fit *quality* is deliberately not judged here. `r_squared` is hardcoded to 0.85
+on the vision-guided path, so it is not currently a measurement, and inventing
+a threshold against a literal would be theatre. Presence is checked; quality is
+reported for the operator to judge.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+# The one field that means a fit happened.
+FIT_KEY = "slope_um_per_deg"
+
+
+def is_calibrated(embryo: Any) -> bool:
+    """True when this embryo carries a usable galvo→piezo fit."""
+    cal = getattr(embryo, "calibration", None) or {}
+    if not isinstance(cal, dict):
+        return False
+    slope = cal.get(FIT_KEY)
+    try:
+        value = float(slope)  # type: ignore[arg-type]
+    except (TypeError, ValueError):
+        return False
+    # A zero slope is not a calibration either: the scan would have no extent.
+    return value == value and value not in (float("inf"), float("-inf")) and value != 0.0
+
+
+def uncalibrated(embryos: dict[str, Any], embryo_ids: list[str] | None) -> list[str]:
+    """Which of `embryo_ids` have no fit. `None` means every known embryo.
+
+    Unknown ids are not reported here — the routes already reject those, and
+    conflating "does not exist" with "not calibrated" would send an operator
+    to the wrong fix.
+    """
+    ids = list(embryo_ids) if embryo_ids else list(embryos.keys())
+    return [eid for eid in ids if eid in embryos and not is_calibrated(embryos[eid])]
+
+
+def refusal_detail(missing: list[str]) -> str:
+    """The message an operator gets, naming the embryos and the way past it."""
+    names = ", ".join(missing)
+    return (
+        f"not calibrated: {names}. "
+        "An uncalibrated embryo has no galvo/piezo fit, so the scan geometry "
+        "would be invented rather than measured. Calibrate them, or pass "
+        "allow_uncalibrated=true to acquire anyway and accept that the volume "
+        "geometry is a guess."
+    )

gently/ui/web/routes/data.py

@@ -7,6 +7,7 @@
 import yaml
 from fastapi import APIRouter, Body, Depends, HTTPException
 
+from gently.harness import calibration_gate
 from gently.ui.web.auth import require_control
 
 logger = logging.getLogger(__name__)
@@ -100,6 +101,30 @@ def _require_agent_with_experiment():
             raise HTTPException(status_code=503, detail="Agent not ready")
         return agent
 
+    def _require_calibrated(embryo_ids, payload):
+        """Refuse to start unless every named embryo carries a real fit.
+
+        Server-side on purpose: the agent reaches these routes too, and a check
+        that lives in operate.js is a check the agent walks past. See
+        gently/harness/calibration_gate.py for why an uncalibrated run is worse
+        than a refused one — the acquisition layer invents five numbers and
+        reports success.
+
+        `allow_uncalibrated: true` still goes through. Someone who means it
+        should be able to say so; the point is that they have to say it.
+        """
+        if payload.get("allow_uncalibrated"):
+            return
+        bridge = getattr(server, "agent_bridge", None)
+        agent = bridge.agent if bridge is not None else None
+        experiment = getattr(agent, "experiment", None) if agent else None
+        embryos = getattr(experiment, "embryos", None)
+        if not embryos:
+            return  # nothing known to check against
+        missing = calibration_gate.uncalibrated(embryos, embryo_ids)
+        if missing:
+            raise HTTPException(status_code=409, detail=calibration_gate.refusal_detail(missing))
+
     @router.put("/api/embryos/{embryo_id}/position", dependencies=[Depends(require_control)])
     async def update_embryo_position(
         embryo_id: str,
@@ -1389,6 +1414,7 @@ async def operate_run_tactic(payload: dict = Body(...)):  # noqa: B008
         )
 
         agent = _require_agent_with_experiment()
+        _require_calibrated(payload.get("embryo_ids") or None, payload)
         tactic = payload.get("tactic")
         lib_id = payload.get("library_id")
         if tactic is None and lib_id:
@@ -1540,6 +1566,12 @@ async def acquire_volume(payload: dict = Body(...)):  # noqa: B008
         can send "ALL OFF" for brightfield-safe Manual-view captures.
         piezo_center and galvo_center capture at the dialled focal plane.
         """
+        # Optional on purpose: Manual-mode snapping images the current stage
+        # position with no embryo in mind, which is legitimate. When a caller
+        # DOES name an embryo, it gets checked.
+        eid = payload.get("embryo_id")
+        if eid:
+            _require_calibrated([str(eid)], payload)
         client = _resolve_client()
         if client is None:
             raise HTTPException(status_code=503, detail="Microscope not connected")
@@ -1617,6 +1649,9 @@ async def timelapse_start(payload: dict = Body(...)):  # noqa: B008
 
         stop_condition = str(payload.get("stop_condition") or "manual")
         embryo_ids = payload.get("embryo_ids") or None
+        # A timelapse is the longest-running thing this UI starts; an invented
+        # scan geometry would be baked into every timepoint of it.
+        _require_calibrated(embryo_ids, payload)
         condition_value = payload.get("condition_value")
         monitoring_mode = payload.get("monitoring_mode") or None
 

gently/ui/web/static/js/operate.js

@@ -1151,6 +1151,13 @@ const OperateManager = (function () {
                 _acquiring = true; renderSubnavMeta();
                 try {
                     await postJSON('/api/devices/acquire/volume', {
+                        // Say WHICH embryo. The route used to receive no id at
+                        // all, so it imaged wherever the stage happened to be
+                        // and could not check whether that embryo had ever been
+                        // calibrated. Manual-mode snapping still omits it on
+                        // purpose — a test shot at the current position is a
+                        // real thing to want.
+                        embryo_id: _selected,
                         num_slices: Math.max(1, Number(($('op-vol-slices') || {}).value) || 50),
                         exposure_ms: Math.max(1, Number(($('op-vol-exp') || {}).value) || 10),
                     });

tests/test_calibration_gate.py

@@ -0,0 +1,127 @@
+"""A run must not start on an uncalibrated embryo.
+
+Nothing checked. `POST /api/devices/timelapse/start` validated
+`interval_seconds > 0` and that the ids existed, and started — and the
+acquisition layer did not fail either, it invented the scan geometry:
+
+    # Get calibration parameters (use defaults if not calibrated)
+    galvo_amplitude = cal.get("galvo_amplitude", 0.5)
+    ...
+    slope           = cal.get("slope_um_per_deg", 100.0)
+
+So a timelapse could run to completion on embryos that had never been
+calibrated, using five literals, and report success. The output is
+indistinguishable from real data, which is why a refusal is the kinder answer.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass, field
+from typing import Any
+
+from gently.harness import calibration_gate
+
+
+@dataclass
+class _Embryo:
+    calibration: dict[str, Any] = field(default_factory=dict)
+
+
+def _fit(slope: float = 42.0) -> dict[str, Any]:
+    """The shape a successful fit writes (calibration_tools.py:891)."""
+    return {"slope_um_per_deg": slope, "offset_um": 1.0, "r_squared": 0.85}
+
+
+# ── is_calibrated ────────────────────────────────────────────────────────────
+
+
+def test_a_real_fit_counts() -> None:
+    assert calibration_gate.is_calibrated(_Embryo(_fit())) is True
+
+
+def test_an_empty_calibration_does_not() -> None:
+    assert calibration_gate.is_calibrated(_Embryo({})) is False
+
+
+def test_scan_parameters_without_a_fit_do_not_count() -> None:
+    """The exact trap: the literals the old code substituted are not a fit.
+
+    An embryo carrying galvo/piezo values but no slope has never been
+    calibrated — those are the invented defaults, not a measurement.
+    """
+    partial = {
+        "galvo_amplitude": 0.5,
+        "galvo_center": 0.0,
+        "piezo_amplitude": 25.0,
+        "piezo_center": 50.0,
+    }
+    assert calibration_gate.is_calibrated(_Embryo(partial)) is False
+
+
+def test_a_zero_slope_is_not_a_calibration() -> None:
+    """Zero slope means the scan has no extent — a fit that failed, not one."""
+    assert calibration_gate.is_calibrated(_Embryo(_fit(0.0))) is False
+
+
+def test_junk_in_the_slope_is_not_a_calibration() -> None:
+    junk: list[Any] = [None, "", "n/a", float("nan"), float("inf"), [], {}]
+    for bad in junk:
+        assert calibration_gate.is_calibrated(_Embryo({"slope_um_per_deg": bad})) is False, bad
+
+
+def test_a_missing_calibration_attribute_does_not_raise() -> None:
+    class Bare:
+        pass
+
+    assert calibration_gate.is_calibrated(Bare()) is False
+
+
+# ── uncalibrated() ───────────────────────────────────────────────────────────
+
+
+def _roster() -> dict[str, _Embryo]:
+    return {
+        "e1": _Embryo(_fit()),
+        "e2": _Embryo({}),
+        "e3": _Embryo(_fit()),
+        "e4": _Embryo({"galvo_center": 0.0}),
+    }
+
+
+def test_reports_only_the_uncalibrated_ones() -> None:
+    assert calibration_gate.uncalibrated(_roster(), ["e1", "e2", "e3", "e4"]) == ["e2", "e4"]
+
+
+def test_none_means_every_known_embryo() -> None:
+    """`embryo_ids: null` on the route means "all active embryos"."""
+    assert calibration_gate.uncalibrated(_roster(), None) == ["e2", "e4"]
+
+
+def test_a_fully_calibrated_subset_is_allowed_through() -> None:
+    assert calibration_gate.uncalibrated(_roster(), ["e1", "e3"]) == []
+
+
+def test_unknown_ids_are_not_reported_as_uncalibrated() -> None:
+    """ "Does not exist" and "not calibrated" send an operator to different fixes.
+
+    The routes already reject unknown ids; conflating the two here would tell
+    someone to calibrate an embryo that is not there.
+    """
+    assert calibration_gate.uncalibrated(_roster(), ["e1", "ghost"]) == []
+
+
+def test_an_empty_roster_reports_nothing() -> None:
+    assert calibration_gate.uncalibrated({}, None) == []
+    assert calibration_gate.uncalibrated({}, ["e1"]) == []
+
+
+# ── the message ──────────────────────────────────────────────────────────────
+
+
+def test_the_refusal_names_the_embryos_and_the_way_past_it() -> None:
+    msg = calibration_gate.refusal_detail(["e2", "e4"])
+    assert "e2" in msg and "e4" in msg
+    # An operator at a microscope needs the remedy in the message, not in a
+    # dependency file or a route signature.
+    assert "allow_uncalibrated" in msg
+    assert "invented" in msg or "guess" in msg