fix(vision): NaN/inf/bool di uno strumento non diventano più una quota (#16)
runner.run_graph esclude bool e NaN/inf dagli output numerici, con il motivo in RunOutcome.invalid_outputs. vision_service tratta una quota mappata con valore None/assente/non finito come "non prodotta" (422), invece di arrivare a float(None) e sollevare TypeError. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ to the worker over HTTP and stays ignorant of how a number was produced.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import math
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from uuid import uuid4
|
from uuid import uuid4
|
||||||
|
|
||||||
@@ -21,6 +22,21 @@ from src.backend.models.orm.vision import VisionReferenceImage, VisionResult
|
|||||||
from src.backend.services import measurement_service
|
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:
|
async def _call_worker(image_bytes: bytes, graph: dict) -> dict:
|
||||||
"""The one place that talks to the worker; the tests replace it.
|
"""The one place that talks to the worker; the tests replace it.
|
||||||
|
|
||||||
@@ -159,8 +175,13 @@ async def execute_task(
|
|||||||
outputs = report["outputs"]
|
outputs = report["outputs"]
|
||||||
|
|
||||||
# Every quote or none. A half-measured piece read as a whole one is worse
|
# Every quote or none. A half-measured piece read as a whole one is worse
|
||||||
# than a piece not measured at all.
|
# than a piece not measured at all. A quote counts as produced only if its
|
||||||
missing = [q.vision_output for q in quotes if q.vision_output not in outputs]
|
# 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:
|
if missing:
|
||||||
failed = ", ".join(f["tool_name"] for f in report["failures"])
|
failed = ", ".join(f["tool_name"] for f in report["failures"])
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
|
|||||||
@@ -159,6 +159,50 @@ async def test_a_missing_output_names_itself(
|
|||||||
assert (await db_session.execute(select(Measurement))).scalars().all() == []
|
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
|
@pytest.mark.asyncio
|
||||||
async def test_a_task_without_a_graph_is_refused(
|
async def test_a_task_without_a_graph_is_refused(
|
||||||
client, db_session, measurement_tec_user, worker_says,
|
client, db_session, measurement_tec_user, worker_says,
|
||||||
|
|||||||
+19
-2
@@ -6,6 +6,7 @@ database, no filesystem beyond reading the pinned submodule's commit.
|
|||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import math
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import time
|
import time
|
||||||
@@ -34,6 +35,12 @@ class RunOutcome:
|
|||||||
failures: list[ToolFailure] = field(default_factory=list)
|
failures: list[ToolFailure] = field(default_factory=list)
|
||||||
engine_version: str = ""
|
engine_version: str = ""
|
||||||
duration_ms: int = 0
|
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)
|
@lru_cache(maxsize=1)
|
||||||
@@ -109,6 +116,7 @@ def run_graph(
|
|||||||
|
|
||||||
outputs: dict[str, float] = {}
|
outputs: dict[str, float] = {}
|
||||||
failures: list[ToolFailure] = []
|
failures: list[ToolFailure] = []
|
||||||
|
invalid_outputs: dict[str, str] = {}
|
||||||
for result in report:
|
for result in report:
|
||||||
if not result.ok:
|
if not result.ok:
|
||||||
failures.append(ToolFailure(
|
failures.append(ToolFailure(
|
||||||
@@ -118,12 +126,21 @@ def run_graph(
|
|||||||
))
|
))
|
||||||
continue
|
continue
|
||||||
for name, value in result.outputs.items():
|
for name, value in result.outputs.items():
|
||||||
if isinstance(value, (int, float)):
|
key = f"{result.tool_id}.{name}"
|
||||||
outputs[f"{result.tool_id}.{name}"] = float(value)
|
# 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(
|
return RunOutcome(
|
||||||
outputs=outputs,
|
outputs=outputs,
|
||||||
failures=failures,
|
failures=failures,
|
||||||
engine_version=engine_version(),
|
engine_version=engine_version(),
|
||||||
duration_ms=elapsed_ms,
|
duration_ms=elapsed_ms,
|
||||||
|
invalid_outputs=invalid_outputs,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -72,6 +72,53 @@ def test_a_broken_tool_does_not_sink_the_others():
|
|||||||
assert any(key.endswith(".diameter") for key in outcome.outputs)
|
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():
|
def test_the_engine_says_which_version_it_is():
|
||||||
version = engine_version()
|
version = engine_version()
|
||||||
assert len(version) == 40 # a full git SHA
|
assert len(version) == 40 # a full git SHA
|
||||||
|
|||||||
@@ -48,6 +48,9 @@ async def run(
|
|||||||
) from exc
|
) from exc
|
||||||
|
|
||||||
return {
|
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,
|
"outputs": outcome.outputs,
|
||||||
"failures": [
|
"failures": [
|
||||||
{"tool_id": f.tool_id, "tool_name": f.tool_name, "error": f.error}
|
{"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,
|
"engine_version": outcome.engine_version,
|
||||||
"duration_ms": outcome.duration_ms,
|
"duration_ms": outcome.duration_ms,
|
||||||
|
"invalid_outputs": outcome.invalid_outputs,
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user