#20 - models/api/measurement.py: value accettava NaN/Infinity/fuori scala, ogni confronto tornava falso ("pass") e poi l'INSERT falliva. Il controllo sposta in measurement_service._validate_measured_value (math.isfinite + |value|<1e6): un Field pydantic (allow_inf_nan/gt/lt) rifiuta la stessa cosa ma FastAPI, costruendo l'errore 422, riecheggia il NaN nel campo "input" e Starlette lo nega con allow_nan=False, producendo un 500 al posto del 422 - verificato con una chiamata reale, non solo a livello di modello. calculate_pass_fail e save_measurement arrotondano value a 6 decimali (stessa scala di DECIMAL(12,6)) prima di calcolare e salvare, cosi' il verdetto e il valore persistito non divergono mai. #5 - measurement_service.save_measurement: prima dell'insert verifica che la quota appartenga a un task della version_id dichiarata (422 altrimenti), e se production_run_id e' dato usa la nuova validate_run_for_version(db, run_id, version_id): il run deve esistere (404), essere della stessa ricetta della versione (422), essere misurabile - riusa production_service._require_measurable (409 se chiuso o in pausa). Import locale per evitare un ciclo con production_service, che gia' importa measurement_service. Test: gia' presenti in test_measurements.py (TestMeasurementValueBounds, TestSaveMeasurementCoherence) - finiti nel commit0dce229di un altro agente che lavorava in parallelo sullo stesso file (export CSV, #22); confermato con `git show0dce229-- test_measurements.py`, contenuto intatto. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,13 @@ class MeasurementCreate(BaseModel):
|
||||
"""Schema for creating a single measurement."""
|
||||
subtask_id: int
|
||||
version_id: int
|
||||
# Finiteness and the DECIMAL(12,6) scale bound are enforced in
|
||||
# measurement_service.save_measurement, not with a Field constraint here:
|
||||
# a NaN/Infinity that fails a pydantic Field constraint gets echoed back
|
||||
# in the validation error's "input", and FastAPI's default error response
|
||||
# then crashes with a 500 trying to JSON-encode that same NaN/Infinity
|
||||
# (json.dumps(allow_nan=False) in Starlette's JSONResponse). A plain
|
||||
# HTTPException raised after parsing sidesteps that.
|
||||
value: float
|
||||
lot_number: Optional[str] = Field(None, max_length=100)
|
||||
serial_number: Optional[str] = Field(None, max_length=100)
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
"""Measurement service - pass/fail calculation, data storage."""
|
||||
import math
|
||||
from datetime import datetime
|
||||
from decimal import Decimal
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from fastapi import HTTPException, status
|
||||
from sqlalchemy import func, select
|
||||
@@ -13,6 +15,38 @@ from src.backend.models.orm.task import (
|
||||
)
|
||||
from src.backend.models.orm.user import User
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from src.backend.models.orm.production import ProductionRun
|
||||
|
||||
|
||||
# What the Measurement.value column (DECIMAL(12,6)) can actually hold.
|
||||
_MAX_ABS_VALUE = 1_000_000
|
||||
|
||||
|
||||
def _validate_measured_value(value: float) -> None:
|
||||
"""Refuse a value that would corrupt the verdict or the INSERT.
|
||||
|
||||
NaN makes every tolerance comparison false, so a bad reading silently
|
||||
comes out as "pass"; NaN and Infinity then both fail the DECIMAL(12,6)
|
||||
column at insert time with an IntegrityError the operator never asked
|
||||
for. Raised as a plain HTTPException with a string detail rather than as
|
||||
a pydantic Field constraint on the request model: a Field rejecting NaN
|
||||
(via allow_inf_nan or a gt/lt bound) makes pydantic echo that same NaN
|
||||
back in the validation error's "input", and FastAPI's default error
|
||||
response then crashes with a 500 trying to JSON-encode it
|
||||
(Starlette's JSONResponse calls json.dumps(..., allow_nan=False)).
|
||||
"""
|
||||
if not math.isfinite(value):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
|
||||
detail="Measured value must be a finite number (no NaN/Infinity)",
|
||||
)
|
||||
if abs(value) >= _MAX_ABS_VALUE:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
|
||||
detail=f"Measured value out of range: |value| must be < {_MAX_ABS_VALUE}",
|
||||
)
|
||||
|
||||
|
||||
def calculate_pass_fail(
|
||||
value: float, subtask: RecipeSubtask
|
||||
@@ -25,7 +59,12 @@ def calculate_pass_fail(
|
||||
- If value is outside UTL/LTL -> 'fail'
|
||||
- If value is outside UWL/LWL but inside UTL/LTL -> 'warning'
|
||||
- Otherwise -> 'pass'
|
||||
|
||||
The value is rounded to 6 decimals first - the precision the DECIMAL(12,6)
|
||||
column keeps - so the verdict is computed on the same number that ends up
|
||||
stored, and a value like 10.5000004 does not fail against a 10.5 limit.
|
||||
"""
|
||||
value = round(value, 6)
|
||||
deviation = None
|
||||
if subtask.nominal is not None:
|
||||
deviation = float(Decimal(str(value)) - Decimal(str(subtask.nominal)))
|
||||
@@ -202,6 +241,35 @@ async def task_progress(
|
||||
return progress
|
||||
|
||||
|
||||
async def validate_run_for_version(
|
||||
db: AsyncSession, run_id: int, version_id: int,
|
||||
) -> "ProductionRun":
|
||||
"""The run a measurement is being filed against, checked for coherence.
|
||||
|
||||
Three things have to hold before a value goes in against a production: the
|
||||
run has to exist, it has to belong to the same recipe as the version being
|
||||
measured (a run started for another product must not collect another
|
||||
product's history), and it has to be in a state that accepts measurements
|
||||
(not closed, not paused). Shared with vision_service, which files camera
|
||||
measurements against a run the same way a manual reading does.
|
||||
"""
|
||||
# Local import: production_service imports this module at module scope,
|
||||
# so importing it back here at module scope would be circular.
|
||||
from src.backend.services import production_service
|
||||
|
||||
run = await production_service.get_run(db, run_id) # 404 if missing
|
||||
|
||||
recipe = await recipe_of_version(db, version_id)
|
||||
if recipe is None or run.recipe_id != recipe.id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
|
||||
detail="Production run belongs to a different recipe than this version",
|
||||
)
|
||||
|
||||
production_service._require_measurable(run) # 409 if closed or paused
|
||||
return run
|
||||
|
||||
|
||||
async def authorise_measurement(
|
||||
db: AsyncSession, measurement: Measurement, supervisor: User,
|
||||
) -> Measurement:
|
||||
@@ -236,6 +304,8 @@ async def save_measurement(
|
||||
the gate models "moving on" between measurements taken one after another,
|
||||
which does not exist inside a single camera acquisition.
|
||||
"""
|
||||
_validate_measured_value(value)
|
||||
|
||||
# Get subtask for tolerance values
|
||||
result = await db.execute(
|
||||
select(RecipeSubtask).where(RecipeSubtask.id == subtask_id)
|
||||
@@ -244,6 +314,22 @@ async def save_measurement(
|
||||
if subtask is None:
|
||||
raise ValueError(f"Subtask {subtask_id} not found")
|
||||
|
||||
# The quote has to belong to a task of the declared version - otherwise the
|
||||
# tolerances just read off `subtask` are not the ones this version's operator
|
||||
# was shown, and pass/fail is computed against the wrong recipe.
|
||||
task_row = await db.execute(
|
||||
select(RecipeTask.version_id).where(RecipeTask.id == subtask.task_id)
|
||||
)
|
||||
subtask_version_id = task_row.scalar_one_or_none()
|
||||
if subtask_version_id != version_id:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
|
||||
detail="Subtask does not belong to the given version",
|
||||
)
|
||||
|
||||
if production_run_id is not None:
|
||||
await validate_run_for_version(db, production_run_id, version_id)
|
||||
|
||||
recipe = await recipe_of_version(db, version_id)
|
||||
_enforce_recipe_rules(recipe, lot_number, serial_number, input_method)
|
||||
|
||||
@@ -263,6 +349,10 @@ async def save_measurement(
|
||||
),
|
||||
)
|
||||
|
||||
# Rounded once, here, so the stored value is exactly the one the verdict
|
||||
# above was computed on - calculate_pass_fail rounds internally too, but
|
||||
# matching it here keeps the persisted row from disagreeing with itself.
|
||||
value = round(value, 6)
|
||||
pass_fail, deviation = calculate_pass_fail(value, subtask)
|
||||
|
||||
measurement = Measurement(
|
||||
|
||||
Reference in New Issue
Block a user