diff --git a/src/backend/services/vision_service.py b/src/backend/services/vision_service.py index e9fa85e..1836015 100644 --- a/src/backend/services/vision_service.py +++ b/src/backend/services/vision_service.py @@ -6,6 +6,7 @@ to the worker over HTTP and stays ignorant of how a number was produced. from __future__ import annotations import json +import math from pathlib import Path from uuid import uuid4 @@ -21,6 +22,21 @@ from src.backend.models.orm.vision import VisionReferenceImage, VisionResult from src.backend.services import measurement_service +def _is_measurable(value: object) -> bool: + """A worker output counts as a quote only if it is a finite, non-bool number. + + None (absent, or explicitly null in a malformed response), a bool (a + subclass of int), and NaN/inf all fail this - each one would otherwise + reach `float()` and either crash with a bare TypeError or silently save a + value nobody could have measured. + """ + return ( + isinstance(value, (int, float)) + and not isinstance(value, bool) + and math.isfinite(value) + ) + + async def _call_worker(image_bytes: bytes, graph: dict) -> dict: """The one place that talks to the worker; the tests replace it. @@ -159,8 +175,13 @@ async def execute_task( outputs = report["outputs"] # Every quote or none. A half-measured piece read as a whole one is worse - # than a piece not measured at all. - missing = [q.vision_output for q in quotes if q.vision_output not in outputs] + # than a piece not measured at all. A quote counts as produced only if its + # output is present *and* a finite, non-bool number: the in-process runner + # already drops NaN/inf itself (see src/vision/runner.py), but the worker + # is reached over HTTP - a different implementation of it, or a malformed + # response, could still hand back None or a non-finite float for a mapped + # key instead of omitting it outright. + missing = [q.vision_output for q in quotes if not _is_measurable(outputs.get(q.vision_output))] if missing: failed = ", ".join(f["tool_name"] for f in report["failures"]) raise HTTPException( diff --git a/src/backend/tests/test_vision_execute.py b/src/backend/tests/test_vision_execute.py index 5eb1547..e9cff58 100644 --- a/src/backend/tests/test_vision_execute.py +++ b/src/backend/tests/test_vision_execute.py @@ -159,6 +159,50 @@ async def test_a_missing_output_names_itself( assert (await db_session.execute(select(Measurement))).scalars().all() == [] +@pytest.mark.asyncio +async def test_a_null_output_counts_as_not_produced( + client, db_session, measurement_tec_user, worker_says, +): + """A worker answering with the key present but null (not omitted) must not + reach float(None) as a bare TypeError - it is "not produced", same as a + missing key (#16).""" + recipe = await create_test_recipe(db_session, measurement_tec_user.id) + version, task, first, second = await two_vision_quotes(db_session, recipe.id) + worker_says({"t1.diameter": None, "t1.radius": GOOD}) + + response = await client.post( + "/api/vision/execute", + headers=auth_headers(measurement_tec_user), + files={"image": ("p.png", an_image(), "image/png")}, + data={"task_id": str(task.id)}, + ) + + assert response.status_code == 422 + assert "t1.diameter" in response.json()["detail"] + assert (await db_session.execute(select(Measurement))).scalars().all() == [] + + +@pytest.mark.asyncio +async def test_a_non_finite_output_counts_as_not_produced( + client, db_session, measurement_tec_user, worker_says, +): + """NaN/inf must not become a saved measurement (#16).""" + recipe = await create_test_recipe(db_session, measurement_tec_user.id) + version, task, first, second = await two_vision_quotes(db_session, recipe.id) + worker_says({"t1.diameter": float("nan"), "t1.radius": GOOD}) + + response = await client.post( + "/api/vision/execute", + headers=auth_headers(measurement_tec_user), + files={"image": ("p.png", an_image(), "image/png")}, + data={"task_id": str(task.id)}, + ) + + assert response.status_code == 422 + assert "t1.diameter" in response.json()["detail"] + assert (await db_session.execute(select(Measurement))).scalars().all() == [] + + @pytest.mark.asyncio async def test_a_task_without_a_graph_is_refused( client, db_session, measurement_tec_user, worker_says, diff --git a/src/vision/runner.py b/src/vision/runner.py index 5884083..16b6826 100644 --- a/src/vision/runner.py +++ b/src/vision/runner.py @@ -6,6 +6,7 @@ database, no filesystem beyond reading the pinned submodule's commit. """ from __future__ import annotations +import math import os import subprocess import time @@ -34,6 +35,12 @@ class RunOutcome: failures: list[ToolFailure] = field(default_factory=list) engine_version: str = "" duration_ms: int = 0 + # A tool that "succeeded" but handed back NaN/inf (vs-measure emits NaN on + # an empty slice - see caliper.py:112) or a bool (a subclass of int, so + # `isinstance(x, (int, float))` alone would let it through as a quote). + # Kept apart from `failures`: the tool did not raise, it just produced a + # number the caller must not measure a part by. Value is the reason. + invalid_outputs: dict[str, str] = field(default_factory=dict) @lru_cache(maxsize=1) @@ -109,6 +116,7 @@ def run_graph( outputs: dict[str, float] = {} failures: list[ToolFailure] = [] + invalid_outputs: dict[str, str] = {} for result in report: if not result.ok: failures.append(ToolFailure( @@ -118,12 +126,21 @@ def run_graph( )) continue for name, value in result.outputs.items(): - if isinstance(value, (int, float)): - outputs[f"{result.tool_id}.{name}"] = float(value) + key = f"{result.tool_id}.{name}" + # bool is a subclass of int - a tool that outputs a flag must not + # be read as a quote. + if isinstance(value, bool) or not isinstance(value, (int, float)): + continue + value = float(value) + if not math.isfinite(value): + invalid_outputs[key] = "nan" if value != value else "infinite" + continue + outputs[key] = value return RunOutcome( outputs=outputs, failures=failures, engine_version=engine_version(), duration_ms=elapsed_ms, + invalid_outputs=invalid_outputs, ) diff --git a/src/vision/tests/test_runner.py b/src/vision/tests/test_runner.py index 73c528d..56bb778 100644 --- a/src/vision/tests/test_runner.py +++ b/src/vision/tests/test_runner.py @@ -72,6 +72,53 @@ def test_a_broken_tool_does_not_sink_the_others(): assert any(key.endswith(".diameter") for key in outcome.outputs) +class _FakeResult: + """Stands in for a vs-task tool result: only what run_graph touches.""" + + def __init__(self, tool_id, tool_name, ok, outputs=None, error=None): + self.tool_id = tool_id + self.tool_name = tool_name + self.ok = ok + self.outputs = outputs or {} + self.error = error + + +def test_a_nan_output_is_dropped_not_measured(monkeypatch): + """vs-measure emits NaN on an empty slice (caliper.py:112 per the + revisione) - a NaN must not become a quote's value (#16).""" + def fake_run(self, context=None): + return [_FakeResult("t1", "diametro", True, outputs={"diameter": float("nan")})] + monkeypatch.setattr(runner.Task, "run", fake_run) + + outcome = run_graph(a_disc(), a_circle_graph()) + + assert outcome.outputs == {} + assert outcome.invalid_outputs == {"t1.diameter": "nan"} + + +def test_an_infinite_output_is_dropped_not_measured(monkeypatch): + def fake_run(self, context=None): + return [_FakeResult("t1", "diametro", True, outputs={"diameter": float("inf")})] + monkeypatch.setattr(runner.Task, "run", fake_run) + + outcome = run_graph(a_disc(), a_circle_graph()) + + assert outcome.outputs == {} + assert outcome.invalid_outputs == {"t1.diameter": "infinite"} + + +def test_a_bool_output_is_not_treated_as_a_measurable_number(monkeypatch): + """bool is a subclass of int - a flag output must not become a quote.""" + def fake_run(self, context=None): + return [_FakeResult("t1", "flag", True, outputs={"present": True})] + monkeypatch.setattr(runner.Task, "run", fake_run) + + outcome = run_graph(a_disc(), a_circle_graph()) + + assert outcome.outputs == {} + assert outcome.invalid_outputs == {} + + def test_the_engine_says_which_version_it_is(): version = engine_version() assert len(version) == 40 # a full git SHA diff --git a/src/vision_worker/main.py b/src/vision_worker/main.py index a306042..010788a 100644 --- a/src/vision_worker/main.py +++ b/src/vision_worker/main.py @@ -48,6 +48,9 @@ async def run( ) from exc return { + # `outcome.outputs` never carries NaN/inf/bool (see runner.run_graph): + # a JSON encoder that accepts non-finite floats (Python's stdlib json + # does, by default) must never get the chance to emit them here. "outputs": outcome.outputs, "failures": [ {"tool_id": f.tool_id, "tool_name": f.tool_name, "error": f.error} @@ -55,4 +58,5 @@ async def run( ], "engine_version": outcome.engine_version, "duration_ms": outcome.duration_ms, + "invalid_outputs": outcome.invalid_outputs, }