From 2e6f03f5ba2ec77b30976eb5f736b8b0bd4b7b03 Mon Sep 17 00:00:00 2001 From: Kaileshwar16 Date: Thu, 10 Sep 2026 16:18:37 +0530 Subject: [PATCH 1/4] Centralize numeric range validation --- src/hflow/_field_guards.py | 62 ++++++++++++- src/hflow/batching.py | 38 +++----- src/hflow/build_ai_vlm_checks.py | 96 ++++++++------------ tests/test_batching.py | 35 +++++--- tests/test_build_ai_vlm_checks.py | 142 +++++++++++++++++++++++++++--- tests/test_field_guards.py | 69 +++++++++++++++ 6 files changed, 337 insertions(+), 105 deletions(-) create mode 100644 tests/test_field_guards.py diff --git a/src/hflow/_field_guards.py b/src/hflow/_field_guards.py index 928803c6..0cbadbf2 100644 --- a/src/hflow/_field_guards.py +++ b/src/hflow/_field_guards.py @@ -1,4 +1,4 @@ -"""Shared numeric-field type guards for caller-constructed settings. +"""Shared numeric-field type and range guards for caller-constructed settings. Range checks alone (``0 <= x <= 100``) do not reject the wrong *type*: ``bool`` subclasses ``int``, so ``True``/``False`` satisfy every numeric comparison and @@ -11,8 +11,16 @@ output, so unlike ``catalog.py``'s NumPy-scalar coercion, no NumPy handling is added here: a caller building a ``np.float64`` threshold can call ``.item()`` itself, the same way any other non-native-Python value would need to. + +Range guards compose the type guards so callers can state the whole invariant +without repeating the bool exclusion. The Real guard preserves batching's +broader numeric contract without adding coercion to the native-number guards. """ +import math +from numbers import Real +from typing import cast + def require_int(value: object, name: str) -> None: """Refuse anything but a plain ``int``, ``bool`` included.""" @@ -28,3 +36,55 @@ def require_float(value: object, name: str) -> None: """ if isinstance(value, bool) or not isinstance(value, int | float): raise ValueError(f"{name} must be an int or float, got {type(value).__name__}") + + +def require_positive_int(value: object, name: str) -> None: + """Refuse anything but a strictly positive int, excluding bool.""" + require_int(value, name) + if cast(int, value) <= 0: + raise ValueError(f"{name} must be > 0, got {value}") + + +def require_non_negative_int(value: object, name: str) -> None: + """Refuse anything but a non-negative int, excluding bool.""" + require_int(value, name) + if cast(int, value) < 0: + raise ValueError(f"{name} must be >= 0, got {value}") + + +def require_int_in_range(value: object, name: str, *, minimum: int, maximum: int) -> None: + """Refuse anything but an int within the inclusive bounds, excluding bool.""" + require_int(value, name) + if not minimum <= cast(int, value) <= maximum: + raise ValueError(f"{name} must be in [{minimum}, {maximum}], got {value}") + + +def require_finite_float(value: object, name: str) -> None: + """Refuse anything but a finite int or float, excluding bool.""" + require_float(value, name) + if not math.isfinite(cast(int | float, value)): + raise ValueError(f"{name} must be finite, got {value}") + + +def require_positive_float(value: object, name: str) -> None: + """Refuse anything but a finite, strictly positive int or float.""" + require_finite_float(value, name) + if cast(int | float, value) <= 0: + raise ValueError(f"{name} must be > 0, got {value}") + + +def require_non_negative_float(value: object, name: str) -> None: + """Refuse anything but a finite, non-negative int or float.""" + require_finite_float(value, name) + if cast(int | float, value) < 0: + raise ValueError(f"{name} must be >= 0, got {value}") + + +def require_non_negative_real(value: object, name: str) -> None: + """Preserve batching's finite, non-negative Real contract, excluding bool.""" + if isinstance(value, bool) or not isinstance(value, Real): + raise ValueError(f"{name} must be a real number, got {type(value).__name__}") + if not math.isfinite(value): + raise ValueError(f"{name} must be finite, got {value}") + if value < 0: + raise ValueError(f"{name} must be >= 0, got {value}") diff --git a/src/hflow/batching.py b/src/hflow/batching.py index f5352c45..adc69afc 100644 --- a/src/hflow/batching.py +++ b/src/hflow/batching.py @@ -22,12 +22,16 @@ """ import heapq -import math from collections.abc import Iterable, Mapping from dataclasses import dataclass -from numbers import Real from pathlib import Path +from hflow._field_guards import ( + require_non_negative_int, + require_non_negative_real, + require_positive_int, +) + @dataclass(frozen=True) class PlannedBatch: @@ -54,34 +58,14 @@ def plan_batches( if (batch_count is None) == (target_batch_bytes is None): raise ValueError("pass exactly one of batch_count or target_batch_bytes") for uri, size_bytes in item_sizes.items(): - if not isinstance(size_bytes, int) or isinstance(size_bytes, bool): - raise ValueError( - f"item {uri!r} has type {type(size_bytes).__name__}, expected int bytes" - ) - if size_bytes < 0: - raise ValueError(f"item {uri!r} has negative size {size_bytes}") - - if not isinstance(stagger_interval_s, Real) or isinstance(stagger_interval_s, bool): - raise ValueError( - f"stagger_interval_s must be a number, got {type(stagger_interval_s).__name__}" - ) - if not math.isfinite(stagger_interval_s): - raise ValueError(f"stagger_interval_s must be finite, got {stagger_interval_s}") - if stagger_interval_s < 0: - raise ValueError(f"stagger_interval_s must be nonnegative, got {stagger_interval_s}") + require_non_negative_int(size_bytes, f"item {uri!r} size_bytes") + + require_non_negative_real(stagger_interval_s, "stagger_interval_s") if batch_count is not None: - if not isinstance(batch_count, int) or isinstance(batch_count, bool): - raise ValueError(f"batch_count must be an int, got {type(batch_count).__name__}") - if batch_count < 1: - raise ValueError(f"batch_count must be >= 1, got {batch_count}") + require_positive_int(batch_count, "batch_count") else: - if not isinstance(target_batch_bytes, int) or isinstance(target_batch_bytes, bool): - raise ValueError( - f"target_batch_bytes must be an int, got {type(target_batch_bytes).__name__}" - ) - if target_batch_bytes < 1: - raise ValueError(f"target_batch_bytes must be >= 1, got {target_batch_bytes}") + require_positive_int(target_batch_bytes, "target_batch_bytes") if not item_sizes: return [] diff --git a/src/hflow/build_ai_vlm_checks.py b/src/hflow/build_ai_vlm_checks.py index a275af56..28e94c56 100644 --- a/src/hflow/build_ai_vlm_checks.py +++ b/src/hflow/build_ai_vlm_checks.py @@ -40,11 +40,20 @@ from dataclasses import dataclass from enum import StrEnum from pathlib import Path -from typing import TYPE_CHECKING, Any, assert_never +from typing import TYPE_CHECKING, Any, assert_never, cast from urllib.parse import urlsplit import httpx2 +from hflow._field_guards import ( + require_finite_float, + require_float, + require_int_in_range, + require_non_negative_float, + require_non_negative_int, + require_positive_float, + require_positive_int, +) from hflow._version import __version__ from hflow._video_measurement_toolchain import measure_video_frame_statistics_for_hflow from hflow._video_measurements import FrameStatisticsSettings @@ -215,16 +224,10 @@ def __post_init__(self) -> None: raise ValueError( "api_key_environment_variable must be a valid environment variable name" ) - if not isinstance(self.max_tokens, int) or isinstance(self.max_tokens, bool): - raise ValueError("max_tokens must be an integer") - if self.max_tokens <= 0: - raise ValueError("max_tokens must be greater than zero") - if not isinstance(self.max_retries, int) or isinstance(self.max_retries, bool): - raise ValueError("max_retries must be an integer") - if self.max_retries < 0: - raise ValueError("max_retries must not be negative") - if self.temperature is not None and not math.isfinite(self.temperature): - raise ValueError("temperature must be finite") + require_positive_int(self.max_tokens, "max_tokens") + require_non_negative_int(self.max_retries, "max_retries") + if self.temperature is not None: + require_finite_float(self.temperature, "temperature") @dataclass(frozen=True) @@ -250,24 +253,12 @@ class HFlowHostedExecution: def __post_init__(self) -> None: _require_absolute_http_url(self.base_url, name="base_url") - if not isinstance(self.max_retries, int) or isinstance(self.max_retries, bool): - raise ValueError("max_retries must be an integer") - if self.max_retries < 0: - raise ValueError("max_retries must not be negative") + require_non_negative_int(self.max_retries, "max_retries") parsed_base_url = urlsplit(self.base_url) if parsed_base_url.query or parsed_base_url.fragment: raise ValueError("base_url must not contain a query string or fragment") - if not isinstance(self.check_version, int) or isinstance(self.check_version, bool): - raise ValueError("check_version must be an integer") - if self.check_version <= 0: - raise ValueError("check_version must be greater than zero") - if ( - isinstance(self.request_timeout_seconds, bool) - or not isinstance(self.request_timeout_seconds, int | float) - or not math.isfinite(self.request_timeout_seconds) - or self.request_timeout_seconds <= 0 - ): - raise ValueError("request_timeout_seconds must be finite and greater than zero") + require_positive_int(self.check_version, "check_version") + require_positive_float(self.request_timeout_seconds, "request_timeout_seconds") BuildAIExecution = OpenAICompatibleExecution | HFlowHostedExecution @@ -297,18 +288,14 @@ class FrameSampling: skip_black_frames: bool = True def __post_init__(self) -> None: - if not isinstance(self.skip_black_frames, bool): + if type(self.skip_black_frames) is not bool: raise ValueError("skip_black_frames must be a bool") - if isinstance(self.fps, bool) or not math.isfinite(self.fps) or self.fps <= 0: - raise ValueError("fps must be finite and greater than zero") - if isinstance(self.start_s, bool) or not math.isfinite(self.start_s) or self.start_s < 0: - raise ValueError("start_s must be finite and non-negative") - if self.end_s is not None and ( - isinstance(self.end_s, bool) - or not math.isfinite(self.end_s) - or self.end_s <= self.start_s - ): - raise ValueError("end_s must be finite and greater than start_s") + require_positive_float(self.fps, "fps") + require_non_negative_float(self.start_s, "start_s") + if self.end_s is not None: + require_finite_float(self.end_s, "end_s") + if self.end_s <= self.start_s: + raise ValueError("end_s must be greater than start_s") @dataclass(frozen=True) @@ -337,10 +324,7 @@ def __post_init__(self) -> None: "HFlowHostedExecution uses the hosted check's fixed prompt and does not support " "prompt overrides" ) - if isinstance(self.frame_time_seconds, bool) or not math.isfinite(self.frame_time_seconds): - raise ValueError("frame_time_seconds must be finite and non-negative") - if self.frame_time_seconds < 0: - raise ValueError("frame_time_seconds must be finite and non-negative") + require_non_negative_float(self.frame_time_seconds, "frame_time_seconds") if self.camera == "": raise ValueError("camera must be None or a non-empty topic name") @@ -376,17 +360,10 @@ def parse_hand_count_response(response_text: str) -> int: parsed_response = _parse_json_or_scalar(response_text) if isinstance(parsed_response, dict): parsed_response = parsed_response.get("hand_count") - if isinstance(parsed_response, bool): - raise ValueError("hand count must be 0, 1, or 2") - if isinstance(parsed_response, int): - hand_count = parsed_response - elif isinstance(parsed_response, str) and re.fullmatch(r"[012]", parsed_response.strip()): - hand_count = int(parsed_response) - else: - raise ValueError("hand count must be 0, 1, or 2") - if hand_count not in {0, 1, 2}: - raise ValueError("hand count must be 0, 1, or 2") - return hand_count + if isinstance(parsed_response, str) and re.fullmatch(r"[012]", parsed_response.strip()): + parsed_response = int(parsed_response) + require_int_in_range(parsed_response, "hand count", minimum=0, maximum=2) + return cast(int, parsed_response) def parse_active_manipulation_response(response_text: str) -> str: @@ -536,8 +513,11 @@ def model_output_check_result( outcome.response_metadata.response_model ) for usage_name, usage_value in outcome.response_metadata.usage.items(): - if isinstance(usage_value, int | float) and not isinstance(usage_value, bool): - measurements[f"{measurement_prefix}/usage/{usage_name}"] = usage_value + try: + require_float(usage_value, f"usage/{usage_name}") + except ValueError: + continue + measurements[f"{measurement_prefix}/usage/{usage_name}"] = cast(int | float, usage_value) observation_values: dict[str, MeasurementValue] = { "task": task.value, @@ -799,12 +779,14 @@ def _read_bounded_hosted_response(response: httpx2.Response) -> bytes: def _parse_hosted_prediction(task: EvaluationTask, value: object) -> int | str: match task: case EvaluationTask.HAND_COUNT: - if isinstance(value, bool) or not isinstance(value, int) or value not in {0, 1, 2}: + try: + require_int_in_range(value, "hand count", minimum=0, maximum=2) + except ValueError as error: raise RuntimeError( "HFlow hosted hand-visibility check returned a parsed prediction " "outside 0, 1, or 2" - ) - return value + ) from error + return cast(int, value) case EvaluationTask.ACTIVE_MANIPULATION: if not isinstance(value, str) or value not in {"yes", "no"}: raise RuntimeError( diff --git a/tests/test_batching.py b/tests/test_batching.py index c59ebb0a..167651f5 100644 --- a/tests/test_batching.py +++ b/tests/test_batching.py @@ -1,5 +1,6 @@ """Byte-balanced bin-packing and staggered batch starts.""" +from fractions import Fraction from pathlib import Path from typing import Any @@ -53,11 +54,11 @@ def test_argument_validation() -> None: plan_batches({"a": 1}) with pytest.raises(ValueError, match="exactly one"): plan_batches({"a": 1}, batch_count=1, target_batch_bytes=1) - with pytest.raises(ValueError, match="negative"): + with pytest.raises(ValueError, match="item 'a' size_bytes must be >= 0"): plan_batches({"a": -1}, batch_count=1) - with pytest.raises(ValueError, match="batch_count must be >= 1, got 0"): + with pytest.raises(ValueError, match="batch_count must be > 0, got 0"): plan_batches({"a": 1}, batch_count=0) - with pytest.raises(ValueError, match="target_batch_bytes must be >= 1, got 0"): + with pytest.raises(ValueError, match="target_batch_bytes must be > 0, got 0"): plan_batches({"a": 1}, target_batch_bytes=0) assert plan_batches({}, batch_count=3) == [] @@ -79,8 +80,8 @@ def test_batch_parameters_require_real_integers(kwargs: dict[str, Any], message: @pytest.mark.parametrize( ("size", "message"), [ - (True, "item 'a' has type bool, expected int bytes"), - (1.0, "item 'a' has type float, expected int bytes"), + (True, "item 'a' size_bytes must be an int, got bool"), + (1.0, "item 'a' size_bytes must be an int, got float"), ], ) def test_item_sizes_require_real_integers(size: Any, message: str) -> None: @@ -91,9 +92,9 @@ def test_item_sizes_require_real_integers(size: Any, message: str) -> None: @pytest.mark.parametrize( ("stagger", "message"), [ - (True, "stagger_interval_s must be a number, got bool"), - ("1.0", "stagger_interval_s must be a number, got str"), - (-1.0, "stagger_interval_s must be nonnegative, got -1.0"), + (True, "stagger_interval_s must be a real number, got bool"), + ("1.0", "stagger_interval_s must be a real number, got str"), + (-1.0, "stagger_interval_s must be >= 0, got -1.0"), (float("nan"), "stagger_interval_s must be finite, got nan"), (float("inf"), "stagger_interval_s must be finite, got inf"), ], @@ -103,8 +104,9 @@ def test_stagger_interval_requires_a_finite_nonnegative_number(stagger: Any, mes plan_batches({"a": 1}, batch_count=1, stagger_interval_s=stagger) # type: ignore[arg-type] -def test_zero_size_items_and_zero_stagger_remain_valid() -> None: - batches = plan_batches({"empty": 0}, batch_count=1, stagger_interval_s=0) +@pytest.mark.parametrize("stagger", [0, 0.0, Fraction(0)]) +def test_zero_size_items_and_zero_stagger_remain_valid(stagger: Any) -> None: + batches = plan_batches({"empty": 0}, batch_count=1, stagger_interval_s=stagger) assert batches == [PlannedBatch(items=("empty",), total_bytes=0, start_delay_s=0)] @@ -123,3 +125,16 @@ def test_plan_from_real_files(tmp_path: Path) -> None: batches = plan_batches_from_files([small, large], batch_count=2) assert batches[0].total_bytes == 10_000 assert batches[0].items == (str(large),) + + +def test_fractional_stagger_preserves_exact_delays() -> None: + batches = plan_batches( + {"a": 1, "b": 1, "c": 1}, + batch_count=3, + stagger_interval_s=Fraction(1, 3), # ty: ignore[invalid-argument-type] + ) + assert [batch.start_delay_s for batch in batches] == [ + Fraction(0), + Fraction(1, 3), + Fraction(2, 3), + ] diff --git a/tests/test_build_ai_vlm_checks.py b/tests/test_build_ai_vlm_checks.py index 7b1f10cd..045f824d 100644 --- a/tests/test_build_ai_vlm_checks.py +++ b/tests/test_build_ai_vlm_checks.py @@ -1,9 +1,11 @@ from __future__ import annotations import json -from collections.abc import Iterator +from collections.abc import Callable, Iterator +from functools import partial from pathlib import Path from types import TracebackType +from typing import Any import httpx2 import pytest @@ -174,7 +176,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, "32", 5, - "max_tokens must be an integer", + "max_tokens must be an int", ), ( "http://localhost:8000/v1", @@ -184,7 +186,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, True, 5, - "max_tokens must be an integer", + "max_tokens must be an int", ), ( "http://localhost:8000/v1", @@ -194,7 +196,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, 0, 5, - "max_tokens must be greater than zero", + "max_tokens must be > 0", ), ( "http://localhost:8000/v1", @@ -204,7 +206,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, 32, "5", - "max_retries must be an integer", + "max_retries must be an int", ), ( "http://localhost:8000/v1", @@ -214,7 +216,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, 32, True, - "max_retries must be an integer", + "max_retries must be an int", ), ( "http://localhost:8000/v1", @@ -224,7 +226,7 @@ def test_build_ai_check_version_changes_with_hosted_check_version(tmp_path: Path None, 32, -1, - "max_retries must not be negative", + "max_retries must be >= 0", ), ( "http://localhost:8000/v1", @@ -286,7 +288,7 @@ def test_openai_compatible_execution_refuses_non_integer_max_retries( ) -> None: # bool is an int subclass, so the isinstance(int) check alone would let # True through; both shapes must raise the same error. - with pytest.raises(ValueError, match="max_retries must be an integer"): + with pytest.raises(ValueError, match="max_retries must be an int"): hflow.build_ai_vlm_checks.OpenAICompatibleExecution( endpoint="https://example.com/v1", model="model", @@ -421,18 +423,138 @@ def test_hosted_unparsed_response_remains_an_evaluation_outcome() -> None: assert outcome.parse_error == 'active manipulation must be "yes" or "no"' -def test_hosted_response_refuses_a_prediction_outside_the_check_contract() -> None: +@pytest.mark.parametrize("prediction", [True, False, -1, 3, 1.0, "1", None]) +def test_hosted_response_refuses_a_prediction_outside_the_check_contract( + prediction: object, +) -> None: with pytest.raises(RuntimeError, match="outside 0, 1, or 2"): hflow.build_ai_vlm_checks._parse_hosted_check_response( hflow.build_ai_vlm_checks.EvaluationTask.HAND_COUNT, { "outcome": "parsed", - "prediction": 3, + "prediction": prediction, "raw_response": "3", }, ) +@pytest.mark.parametrize( + ("factory", "field", "boundary", "valid"), + [ + ( + partial( + hflow.build_ai_vlm_checks.OpenAICompatibleExecution, + endpoint="https://example.com/v1", + model="model", + ), + "max_tokens", + 0, + 1, + ), + ( + partial( + hflow.build_ai_vlm_checks.OpenAICompatibleExecution, + endpoint="https://example.com/v1", + model="model", + ), + "max_retries", + -1, + 0, + ), + (hflow.build_ai_vlm_checks.HFlowHostedExecution, "max_retries", -1, 0), + (hflow.build_ai_vlm_checks.HFlowHostedExecution, "check_version", 0, 1), + (hflow.build_ai_vlm_checks.HFlowHostedExecution, "request_timeout_seconds", 0, 0.5), + (hflow.build_ai_vlm_checks.FrameSampling, "fps", 0, 0.5), + (hflow.build_ai_vlm_checks.FrameSampling, "start_s", -1, 0), + (partial(hflow.build_ai_vlm_checks.FrameSampling, start_s=2), "end_s", 2, 3), + ], +) +def test_numeric_configuration_boundaries( + factory: Callable[..., Any], field: str, boundary: int, valid: int | float +) -> None: + for value in [True, False, "1", None, float("nan"), float("inf"), float("-inf"), boundary]: + if field == "end_s" and value is None: + continue + with pytest.raises(ValueError, match=field): + factory(**{field: value}) + assert getattr(factory(**{field: valid}), field) == valid + + +@pytest.mark.parametrize("value", [True, False, "1", float("nan"), float("inf"), float("-inf")]) +def test_temperature_requires_a_finite_number(value: Any) -> None: + with pytest.raises(ValueError, match="temperature"): + hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="https://example.com/v1", model="model", temperature=value + ) + + +@pytest.mark.parametrize("value", [None, -1, 0, 0.5]) +def test_temperature_accepts_optional_finite_numbers(value: float | None) -> None: + execution = hflow.build_ai_vlm_checks.OpenAICompatibleExecution( + endpoint="https://example.com/v1", model="model", temperature=value + ) + assert execution.temperature == value + + +@pytest.mark.parametrize( + "value", [True, False, "1", None, -1, float("nan"), float("inf"), float("-inf")] +) +def test_registration_refuses_invalid_frame_times(tmp_path: Path, value: Any) -> None: + application = hflow.App("frame-times", data_root=tmp_path, default_checks=()) + with pytest.raises(ValueError, match="frame_time_seconds"): + hflow.build_ai_vlm_checks.register_hand_visibility( + application, + execution=hflow.build_ai_vlm_checks.HFlowHostedExecution(), + frame_time_seconds=value, + ) + + +@pytest.mark.parametrize( + "value", [True, False, -1, 3, 1.0, None, "01", "3", float("nan"), float("inf")] +) +def test_hand_count_response_refuses_invalid_numbers(value: object) -> None: + with pytest.raises(ValueError, match="hand count must be"): + hflow.build_ai_vlm_checks.parse_hand_count_response(json.dumps({"hand_count": value})) + + +@pytest.mark.parametrize("value", [0, 1, 2, "0", "1", "2", " 1 "]) +def test_hand_count_response_accepts_integer_and_text_counts(value: int | str) -> None: + assert hflow.build_ai_vlm_checks.parse_hand_count_response( + json.dumps({"hand_count": value}) + ) == int(value) + + +def test_usage_booleans_are_observations_but_not_numeric_measurements() -> None: + checks = hflow.build_ai_vlm_checks + result = checks.model_output_check_result( + task=checks.EvaluationTask.HAND_COUNT, + requested_model="model", + outcome=checks.ParsedVisionModelOutcome( + raw_response="1", + response_metadata=checks.ModelResponseMetadata( + response_model=None, + usage={ + "tokens": 2, + "cost": 0.5, + "cached": True, + "empty": False, + "label": "text", + "missing": None, + }, + ), + predicted_value=1, + ), + observation_id="frame:0", + timestamp_ns=0, + ) + assert {key: value for key, value in result.measurements.items() if "/usage/" in key} == { + "build_ai/hand_count/usage/tokens": 2, + "build_ai/hand_count/usage/cost": 0.5, + } + assert result.observations[0].values["usage/cached"] is True + assert result.observations[0].values["usage/empty"] is False + + # --- version contract covers every knob that changes result completeness (#404) diff --git a/tests/test_field_guards.py b/tests/test_field_guards.py new file mode 100644 index 00000000..e3bcc6d5 --- /dev/null +++ b/tests/test_field_guards.py @@ -0,0 +1,69 @@ +"""Numeric guards reject invalid types before checking finite values and bounds.""" + +from collections.abc import Callable +from fractions import Fraction +from functools import partial + +import pytest + +from hflow._field_guards import ( + require_finite_float, + require_int_in_range, + require_non_negative_float, + require_non_negative_int, + require_non_negative_real, + require_positive_float, + require_positive_int, +) + + +@pytest.mark.parametrize( + ("guard", "valid_values", "invalid_values"), + [ + (require_positive_int, [1, 10**100], [0, -1, 1.0]), + (require_non_negative_int, [0, 1, 10**100], [-1, 0.0]), + ( + partial(require_int_in_range, minimum=0, maximum=2), + [0, 1, 2], + [-1, 3, 1.0], + ), + (require_finite_float, [-1, -0.5, 0, 0.0, 1, 1.5], []), + (require_positive_float, [1, 0.5], [0, 0.0, -1, -0.5]), + (require_non_negative_float, [0, 0.0, 1, 0.5], [-1, -0.5]), + ( + require_non_negative_real, + [0, 0.0, 1, 0.5, Fraction(1, 3)], + [-1, -0.5, Fraction(-1, 3)], + ), + ], + ids=[ + "positive-int", + "non-negative-int", + "int-range", + "finite-float", + "positive-float", + "non-negative-float", + "non-negative-real", + ], +) +def test_numeric_guard_contract( + guard: Callable[[object, str], None], + valid_values: list[object], + invalid_values: list[object], +) -> None: + for value in valid_values: + assert guard(value, "setting") is None + for value in [ + *invalid_values, + True, + False, + None, + "1", + 1j, + [], + float("nan"), + float("inf"), + float("-inf"), + ]: + with pytest.raises(ValueError, match=r"^setting must be "): + guard(value, "setting") From f5a3e0c0d143d2e0c7675e7aff3091d8c7c9dcc3 Mon Sep 17 00:00:00 2001 From: Kaileshwar16 Date: Thu, 10 Sep 2026 16:58:07 +0530 Subject: [PATCH 2/4] Update example tests for standardized hand count validation --- .../tests/test_evaluation.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/examples/build_ai_evaluation/tests/test_evaluation.py b/examples/build_ai_evaluation/tests/test_evaluation.py index 238f9dbd..0eb5a951 100644 --- a/examples/build_ai_evaluation/tests/test_evaluation.py +++ b/examples/build_ai_evaluation/tests/test_evaluation.py @@ -140,11 +140,20 @@ def test_hand_count_parser_accepts_published_and_compatible_shapes( assert parse_hand_count_response(response_text) == expected_hand_count -@pytest.mark.parametrize("response_text", ["3", '{"hand_count": true}', "two", ""]) +@pytest.mark.parametrize( + ("response_text", "message"), + [ + ("3", r"hand count must be in \[0, 2\], got 3"), + ('{"hand_count": true}', "hand count must be an int, got bool"), + ("two", "hand count must be an int, got str"), + ("", "hand count must be an int, got str"), + ], +) def test_hand_count_parser_rejects_values_outside_the_evaluation_contract( response_text: str, + message: str, ) -> None: - with pytest.raises(ValueError, match="0, 1, or 2"): + with pytest.raises(ValueError, match=message): parse_hand_count_response(response_text) @@ -237,7 +246,7 @@ def test_unparsed_model_judgment_is_an_explicit_recoverable_outcome() -> None: timestamp_ns=123, ) assert check_result.measurements["build_ai/hand_count/parse_error"] == ( - "hand count must be 0, 1, or 2" + "hand count must be an int, got str" ) assert check_result.observations[0].values["valid"] is False assert check_result.tags == ["build_ai/hand_count/unparsed"] @@ -417,7 +426,7 @@ def test_sample_result_outcome_variants_are_exclusive() -> None: assert not hasattr(success, "error") invalid = results[1].outcome assert isinstance(invalid, InvalidResponseSampleOutcome) - assert invalid.parse_error == "hand count must be 0, 1, or 2" + assert invalid.parse_error == "hand count must be an int, got str" assert invalid.raw_response == "not a count" assert not hasattr(invalid, "predicted_value") error = results[2].outcome From ab57b18de8063c2f12aaf93162cf8ff92e33e807 Mon Sep 17 00:00:00 2001 From: Kaileshwar16 Date: Thu, 10 Sep 2026 19:43:52 +0530 Subject: [PATCH 3/4] Address numeric guard review feedback --- .../tests/test_evaluation.py | 17 ++--- src/hflow/_field_guards.py | 65 +++++++++++-------- src/hflow/build_ai_vlm_checks.py | 33 +++++----- tests/test_build_ai_vlm_checks.py | 2 +- tests/test_field_guards.py | 4 +- 5 files changed, 61 insertions(+), 60 deletions(-) diff --git a/examples/build_ai_evaluation/tests/test_evaluation.py b/examples/build_ai_evaluation/tests/test_evaluation.py index 0eb5a951..238f9dbd 100644 --- a/examples/build_ai_evaluation/tests/test_evaluation.py +++ b/examples/build_ai_evaluation/tests/test_evaluation.py @@ -140,20 +140,11 @@ def test_hand_count_parser_accepts_published_and_compatible_shapes( assert parse_hand_count_response(response_text) == expected_hand_count -@pytest.mark.parametrize( - ("response_text", "message"), - [ - ("3", r"hand count must be in \[0, 2\], got 3"), - ('{"hand_count": true}', "hand count must be an int, got bool"), - ("two", "hand count must be an int, got str"), - ("", "hand count must be an int, got str"), - ], -) +@pytest.mark.parametrize("response_text", ["3", '{"hand_count": true}', "two", ""]) def test_hand_count_parser_rejects_values_outside_the_evaluation_contract( response_text: str, - message: str, ) -> None: - with pytest.raises(ValueError, match=message): + with pytest.raises(ValueError, match="0, 1, or 2"): parse_hand_count_response(response_text) @@ -246,7 +237,7 @@ def test_unparsed_model_judgment_is_an_explicit_recoverable_outcome() -> None: timestamp_ns=123, ) assert check_result.measurements["build_ai/hand_count/parse_error"] == ( - "hand count must be an int, got str" + "hand count must be 0, 1, or 2" ) assert check_result.observations[0].values["valid"] is False assert check_result.tags == ["build_ai/hand_count/unparsed"] @@ -426,7 +417,7 @@ def test_sample_result_outcome_variants_are_exclusive() -> None: assert not hasattr(success, "error") invalid = results[1].outcome assert isinstance(invalid, InvalidResponseSampleOutcome) - assert invalid.parse_error == "hand count must be an int, got str" + assert invalid.parse_error == "hand count must be 0, 1, or 2" assert invalid.raw_response == "not a count" assert not hasattr(invalid, "predicted_value") error = results[2].outcome diff --git a/src/hflow/_field_guards.py b/src/hflow/_field_guards.py index 0cbadbf2..be0ed7cb 100644 --- a/src/hflow/_field_guards.py +++ b/src/hflow/_field_guards.py @@ -15,20 +15,21 @@ Range guards compose the type guards so callers can state the whole invariant without repeating the bool exclusion. The Real guard preserves batching's broader numeric contract without adding coercion to the native-number guards. +Each guard returns the validated value with its refined type, without coercion. """ import math from numbers import Real -from typing import cast -def require_int(value: object, name: str) -> None: +def require_int(value: object, name: str) -> int: """Refuse anything but a plain ``int``, ``bool`` included.""" if isinstance(value, bool) or not isinstance(value, int): raise ValueError(f"{name} must be an int, got {type(value).__name__}") + return value -def require_float(value: object, name: str) -> None: +def require_float(value: object, name: str) -> int | float: """Refuse anything but a plain ``int`` or ``float``, ``bool`` included. An ``int`` is accepted for a float-declared field: it is a perfectly good @@ -36,51 +37,58 @@ def require_float(value: object, name: str) -> None: """ if isinstance(value, bool) or not isinstance(value, int | float): raise ValueError(f"{name} must be an int or float, got {type(value).__name__}") + return value -def require_positive_int(value: object, name: str) -> None: +def require_positive_int(value: object, name: str) -> int: """Refuse anything but a strictly positive int, excluding bool.""" - require_int(value, name) - if cast(int, value) <= 0: - raise ValueError(f"{name} must be > 0, got {value}") + number = require_int(value, name) + if number <= 0: + raise ValueError(f"{name} must be > 0, got {number}") + return number -def require_non_negative_int(value: object, name: str) -> None: +def require_non_negative_int(value: object, name: str) -> int: """Refuse anything but a non-negative int, excluding bool.""" - require_int(value, name) - if cast(int, value) < 0: - raise ValueError(f"{name} must be >= 0, got {value}") + number = require_int(value, name) + if number < 0: + raise ValueError(f"{name} must be >= 0, got {number}") + return number -def require_int_in_range(value: object, name: str, *, minimum: int, maximum: int) -> None: +def require_int_in_range(value: object, name: str, *, minimum: int, maximum: int) -> int: """Refuse anything but an int within the inclusive bounds, excluding bool.""" - require_int(value, name) - if not minimum <= cast(int, value) <= maximum: - raise ValueError(f"{name} must be in [{minimum}, {maximum}], got {value}") + number = require_int(value, name) + if not minimum <= number <= maximum: + raise ValueError(f"{name} must be in [{minimum}, {maximum}], got {number}") + return number -def require_finite_float(value: object, name: str) -> None: +def require_finite_float(value: object, name: str) -> int | float: """Refuse anything but a finite int or float, excluding bool.""" - require_float(value, name) - if not math.isfinite(cast(int | float, value)): - raise ValueError(f"{name} must be finite, got {value}") + number = require_float(value, name) + if not math.isfinite(number): + raise ValueError(f"{name} must be finite, got {number}") + return number -def require_positive_float(value: object, name: str) -> None: +def require_positive_float(value: object, name: str) -> int | float: """Refuse anything but a finite, strictly positive int or float.""" - require_finite_float(value, name) - if cast(int | float, value) <= 0: - raise ValueError(f"{name} must be > 0, got {value}") + number = require_finite_float(value, name) + if number <= 0: + raise ValueError(f"{name} must be > 0, got {number}") + return number -def require_non_negative_float(value: object, name: str) -> None: +def require_non_negative_float(value: object, name: str) -> int | float: """Refuse anything but a finite, non-negative int or float.""" - require_finite_float(value, name) - if cast(int | float, value) < 0: - raise ValueError(f"{name} must be >= 0, got {value}") + number = require_finite_float(value, name) + if number < 0: + raise ValueError(f"{name} must be >= 0, got {number}") + return number -def require_non_negative_real(value: object, name: str) -> None: +def require_non_negative_real(value: object, name: str) -> Real: """Preserve batching's finite, non-negative Real contract, excluding bool.""" if isinstance(value, bool) or not isinstance(value, Real): raise ValueError(f"{name} must be a real number, got {type(value).__name__}") @@ -88,3 +96,4 @@ def require_non_negative_real(value: object, name: str) -> None: raise ValueError(f"{name} must be finite, got {value}") if value < 0: raise ValueError(f"{name} must be >= 0, got {value}") + return value diff --git a/src/hflow/build_ai_vlm_checks.py b/src/hflow/build_ai_vlm_checks.py index 28e94c56..a05ca12f 100644 --- a/src/hflow/build_ai_vlm_checks.py +++ b/src/hflow/build_ai_vlm_checks.py @@ -40,15 +40,13 @@ from dataclasses import dataclass from enum import StrEnum from pathlib import Path -from typing import TYPE_CHECKING, Any, assert_never, cast +from typing import TYPE_CHECKING, Any, assert_never from urllib.parse import urlsplit import httpx2 from hflow._field_guards import ( require_finite_float, - require_float, - require_int_in_range, require_non_negative_float, require_non_negative_int, require_positive_float, @@ -360,10 +358,17 @@ def parse_hand_count_response(response_text: str) -> int: parsed_response = _parse_json_or_scalar(response_text) if isinstance(parsed_response, dict): parsed_response = parsed_response.get("hand_count") - if isinstance(parsed_response, str) and re.fullmatch(r"[012]", parsed_response.strip()): - parsed_response = int(parsed_response) - require_int_in_range(parsed_response, "hand count", minimum=0, maximum=2) - return cast(int, parsed_response) + if isinstance(parsed_response, bool): + raise ValueError("hand count must be 0, 1, or 2") + if isinstance(parsed_response, int): + hand_count = parsed_response + elif isinstance(parsed_response, str) and re.fullmatch(r"[012]", parsed_response.strip()): + hand_count = int(parsed_response) + else: + raise ValueError("hand count must be 0, 1, or 2") + if hand_count not in {0, 1, 2}: + raise ValueError("hand count must be 0, 1, or 2") + return hand_count def parse_active_manipulation_response(response_text: str) -> str: @@ -513,11 +518,9 @@ def model_output_check_result( outcome.response_metadata.response_model ) for usage_name, usage_value in outcome.response_metadata.usage.items(): - try: - require_float(usage_value, f"usage/{usage_name}") - except ValueError: + if isinstance(usage_value, bool) or not isinstance(usage_value, int | float): continue - measurements[f"{measurement_prefix}/usage/{usage_name}"] = cast(int | float, usage_value) + measurements[f"{measurement_prefix}/usage/{usage_name}"] = usage_value observation_values: dict[str, MeasurementValue] = { "task": task.value, @@ -779,14 +782,12 @@ def _read_bounded_hosted_response(response: httpx2.Response) -> bytes: def _parse_hosted_prediction(task: EvaluationTask, value: object) -> int | str: match task: case EvaluationTask.HAND_COUNT: - try: - require_int_in_range(value, "hand count", minimum=0, maximum=2) - except ValueError as error: + if isinstance(value, bool) or not isinstance(value, int) or value not in {0, 1, 2}: raise RuntimeError( "HFlow hosted hand-visibility check returned a parsed prediction " "outside 0, 1, or 2" - ) from error - return cast(int, value) + ) + return value case EvaluationTask.ACTIVE_MANIPULATION: if not isinstance(value, str) or value not in {"yes", "no"}: raise RuntimeError( diff --git a/tests/test_build_ai_vlm_checks.py b/tests/test_build_ai_vlm_checks.py index 045f824d..9121392a 100644 --- a/tests/test_build_ai_vlm_checks.py +++ b/tests/test_build_ai_vlm_checks.py @@ -513,7 +513,7 @@ def test_registration_refuses_invalid_frame_times(tmp_path: Path, value: Any) -> "value", [True, False, -1, 3, 1.0, None, "01", "3", float("nan"), float("inf")] ) def test_hand_count_response_refuses_invalid_numbers(value: object) -> None: - with pytest.raises(ValueError, match="hand count must be"): + with pytest.raises(ValueError, match=r"^hand count must be 0, 1, or 2$"): hflow.build_ai_vlm_checks.parse_hand_count_response(json.dumps({"hand_count": value})) diff --git a/tests/test_field_guards.py b/tests/test_field_guards.py index e3bcc6d5..a7eb1136 100644 --- a/tests/test_field_guards.py +++ b/tests/test_field_guards.py @@ -47,12 +47,12 @@ ], ) def test_numeric_guard_contract( - guard: Callable[[object, str], None], + guard: Callable[[object, str], object], valid_values: list[object], invalid_values: list[object], ) -> None: for value in valid_values: - assert guard(value, "setting") is None + assert guard(value, "setting") is value for value in [ *invalid_values, True, From 436343d01d6bf7411ac2d8f6dc6686133a6adc42 Mon Sep 17 00:00:00 2001 From: Kingston Date: Thu, 10 Sep 2026 12:45:39 -0700 Subject: [PATCH 4/4] test(build-ai): pin the skip_black_frames guard, and keep isinstance Deleting that guard left the whole suite green, so it gets a case: 1 and 0 are the rows that matter, since they compare equal to True and False. Reverts type(x) is not bool to isinstance. bool cannot be subclassed, so the two are equivalent here, and isinstance is the idiom everywhere else in the tree including the guards this PR adds. --- src/hflow/build_ai_vlm_checks.py | 2 +- tests/test_build_ai_vlm_checks.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/hflow/build_ai_vlm_checks.py b/src/hflow/build_ai_vlm_checks.py index a05ca12f..7b0cd1c2 100644 --- a/src/hflow/build_ai_vlm_checks.py +++ b/src/hflow/build_ai_vlm_checks.py @@ -286,7 +286,7 @@ class FrameSampling: skip_black_frames: bool = True def __post_init__(self) -> None: - if type(self.skip_black_frames) is not bool: + if not isinstance(self.skip_black_frames, bool): raise ValueError("skip_black_frames must be a bool") require_positive_float(self.fps, "fps") require_non_negative_float(self.start_s, "start_s") diff --git a/tests/test_build_ai_vlm_checks.py b/tests/test_build_ai_vlm_checks.py index 9121392a..18489730 100644 --- a/tests/test_build_ai_vlm_checks.py +++ b/tests/test_build_ai_vlm_checks.py @@ -488,6 +488,29 @@ def test_temperature_requires_a_finite_number(value: Any) -> None: ) +@pytest.mark.parametrize("value", [1, 0, "true", None, 1.0]) +def test_skip_black_frames_requires_an_actual_bool(value: Any) -> None: + # The one guard in this file that _field_guards cannot express: the field + # wants a bool, so the bool exclusion every other guard makes is inverted + # here. Deleting it left the whole suite green, hence this case. 1 and 0 + # are the interesting rows, since they compare equal to True and False. + with pytest.raises(ValueError, match="skip_black_frames must be a bool"): + hflow.build_ai_vlm_checks.FrameSampling(fps=1.0, skip_black_frames=value) + + +def test_skip_black_frames_accepts_both_bools() -> None: + # False is the row that matters: a truthiness check instead of a type + # check would let it through and a falsy-value guard would refuse it. + assert ( + hflow.build_ai_vlm_checks.FrameSampling(fps=1.0, skip_black_frames=False).skip_black_frames + is False + ) + assert ( + hflow.build_ai_vlm_checks.FrameSampling(fps=1.0, skip_black_frames=True).skip_black_frames + is True + ) + + @pytest.mark.parametrize("value", [None, -1, 0, 0.5]) def test_temperature_accepts_optional_finite_numbers(value: float | None) -> None: execution = hflow.build_ai_vlm_checks.OpenAICompatibleExecution(