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
|
||||
|
||||
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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
+19
-2
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user