From 58a66cc1a4605d996cf03f29a23617638d561c6d Mon Sep 17 00:00:00 2001 From: Rayan-and-beyond <263488867+Rayan-and-beyond@users.noreply.github.com> Date: Sat, 12 Sep 2026 06:04:26 +0300 Subject: [PATCH 1/2] refactor(video): reuse field guards in VideoImportConfig (#508) --- src/hflow/importers/video.py | 28 ++++++++++++++----------- tests/test_video_import.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/hflow/importers/video.py b/src/hflow/importers/video.py index c178560..c110315 100644 --- a/src/hflow/importers/video.py +++ b/src/hflow/importers/video.py @@ -12,6 +12,11 @@ from mcap.writer import Writer from mcap_protobuf.schema import build_file_descriptor_set +from hflow._field_guards import ( + require_finite_float, + require_int_in_range, + require_positive_int, +) from hflow._pinned_asset import sha256_hex_of_file from hflow.ffmpeg import ffmpeg_path, ffmpeg_version from hflow.ffmpeg._process import media_input_was_rejected, run_media_command @@ -52,10 +57,7 @@ def __post_init__(self) -> None: ("source_start_s", self.source_start_s), ("image_hz", self.image_hz), ): - if isinstance(value, bool) or not isinstance(value, int | float): - raise ValueError(f"{name} must be a finite number") - if not math.isfinite(value): - raise ValueError(f"{name} must be a finite number") + require_finite_float(value, name) if self.duration_s <= 0 or self.source_start_s < 0: raise ValueError("duration_s must be positive and source_start_s nonnegative") if not 0 < self.image_hz <= NANOSECONDS_PER_SECOND: @@ -66,14 +68,16 @@ def __post_init__(self) -> None: ("image_width", self.image_width), ("image_height", self.image_height), ): - if isinstance(value, bool) or not isinstance(value, int) or value <= 0 or value % 2: - raise ValueError(f"{name} must be a positive even integer") - if ( - isinstance(self.start_time_ns, bool) - or not isinstance(self.start_time_ns, int) - or not 0 <= self.start_time_ns <= _MAXIMUM_TIMESTAMP_NS - ): - raise ValueError("start_time_ns must be an unsigned 64-bit integer") + require_positive_int(value, name) + # H.264 chroma subsampling needs even dimensions. Evenness is a + # domain rule about this config, not a shared numeric invariant, + # so it stays here with its own message instead of moving into + # _field_guards (see #508). + if value % 2: + raise ValueError(f"{name} must be an even integer, got {value}") + require_int_in_range( + self.start_time_ns, "start_time_ns", minimum=0, maximum=_MAXIMUM_TIMESTAMP_NS + ) if self.frame_count > (1 << 32): raise ValueError("the excerpt exceeds the MCAP sequence number range") final_timestamp_ns = _sample_timestamp_ns(self, self.frame_count - 1) diff --git a/tests/test_video_import.py b/tests/test_video_import.py index 7ec7367..8eefecf 100644 --- a/tests/test_video_import.py +++ b/tests/test_video_import.py @@ -215,12 +215,18 @@ def test_invalid_sources_and_incomplete_excerpts_publish_nothing( [ {"duration_s": 0}, {"duration_s": float("nan")}, + {"duration_s": "fast"}, {"source_start_s": -1}, + {"source_start_s": False}, {"image_hz": 0}, {"image_hz": float("inf")}, + {"image_width": 0}, {"image_width": 3}, + {"image_height": 3}, {"image_height": True}, {"start_time_ns": -1}, + {"start_time_ns": True}, + {"start_time_ns": (1 << 64)}, {"start_time_ns": (1 << 64) - 1}, {"camera_name": ""}, {"metadata": (("task", "one"), ("task", "two"))}, @@ -344,3 +350,37 @@ def test_tagged_video_duration_is_shared_by_probe_and_import( ) assert isinstance(outcome, ImportedVideoEpisode) assert len(hflow.Episode(outcome.path).channel("/camera/compressed")) == 4 + + +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("image_width", 0, "image_width must be > 0, got 0"), + ("image_width", 3, "image_width must be an even integer, got 3"), + ("image_height", -4, "image_height must be > 0, got -4"), + ("image_height", 3, "image_height must be an even integer, got 3"), + ], +) +def test_image_dimensions_distinguish_non_positive_from_odd( + field: str, value: object, message: str +) -> None: + """Positivity comes from the shared guard; evenness keeps its own message.""" + with pytest.raises(ValueError, match=f"^{message}$"): + replace(VideoImportConfig(duration_s=1), **{field: value}) + + +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("duration_s", "fast", "duration_s must be an int or float, got str"), + ("duration_s", float("nan"), "duration_s must be finite, got nan"), + ("source_start_s", False, "source_start_s must be an int or float, got bool"), + ("image_hz", float("inf"), "image_hz must be finite, got inf"), + ], +) +def test_finite_fields_name_the_field_and_the_defect( + field: str, value: object, message: str +) -> None: + """The shared guard splits the old blanket message into type vs finiteness.""" + with pytest.raises(ValueError, match=f"^{message}$"): + replace(VideoImportConfig(duration_s=1), **{field: value}) From 1de6754293f7cbd65f80a1e78c72d4f3847b0356 Mon Sep 17 00:00:00 2001 From: Rayan-and-beyond <263488867+Rayan-and-beyond@users.noreply.github.com> Date: Sun, 13 Sep 2026 06:46:56 +0300 Subject: [PATCH 2/2] test(video): pin start_time_ns upper-bound guard --- tests/test_video_import.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_video_import.py b/tests/test_video_import.py index 8eefecf..74a5ce6 100644 --- a/tests/test_video_import.py +++ b/tests/test_video_import.py @@ -384,3 +384,11 @@ def test_finite_fields_name_the_field_and_the_defect( """The shared guard splits the old blanket message into type vs finiteness.""" with pytest.raises(ValueError, match=f"^{message}$"): replace(VideoImportConfig(duration_s=1), **{field: value}) + + +def test_start_time_upper_bound_uses_field_guard() -> None: + """The field guard owns the start_time_ns upper-bound refusal.""" + value = 1 << 64 + with pytest.raises(ValueError) as exc_info: + replace(VideoImportConfig(duration_s=1), start_time_ns=value) + assert str(exc_info.value) == (f"start_time_ns must be in [0, {value - 1}], got {value}")