From c3a085f1f7739e17b3446773e42b6dd23035792b Mon Sep 17 00:00:00 2001 From: Rakes Date: Mon, 31 Aug 2026 10:45:00 +0530 Subject: [PATCH] fix(lerobot): refuse non-finite FPS values in meta/info.json The LeRobot metadata boundary checked that fps is numeric and positive but not finite, so NaN and +/-inf passed validation and failed later with unrelated errors (no meta/episodes parquet files found, or a raw int() conversion error). Mirror the TransformConfig boundary: require fps to be finite as well as numeric and positive, and name the bad value with a complete rule in the error. Regression coverage parametrizes NaN, +inf, -inf, bool, missing, nonnumeric, zero, and negative fps, asserting refusal before any episode metadata discovery or output is written. Fixes #299 --- src/hflow/importers/lerobot.py | 4 +- tests/test_lerobot_converter.py | 91 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/hflow/importers/lerobot.py b/src/hflow/importers/lerobot.py index f23e3553a..994b610e5 100644 --- a/src/hflow/importers/lerobot.py +++ b/src/hflow/importers/lerobot.py @@ -16,6 +16,7 @@ import json import logging +import math import os import struct import subprocess @@ -313,11 +314,12 @@ def _ensure_source_archive(dataset_source: DatasetSource, cache_dir: Path) -> _S if ( isinstance(frames_per_second, bool) or not isinstance(frames_per_second, int | float) + or not math.isfinite(frames_per_second) or frames_per_second <= 0 ): raise ValueError( f"LeRobot meta/info.json has invalid fps={frames_per_second!r}; " - "expected a positive number" + "FPS must be finite and positive" ) data_path_template = dataset_information.get("data_path") if not isinstance(data_path_template, str) or not data_path_template.strip(): diff --git a/tests/test_lerobot_converter.py b/tests/test_lerobot_converter.py index 7d506fa49..44d584aab 100755 --- a/tests/test_lerobot_converter.py +++ b/tests/test_lerobot_converter.py @@ -395,3 +395,94 @@ def test_converter_output_remuxes_without_tail_loss( check=True, ) assert "nb_read_frames=90" in probe.stdout, probe.stdout + + +@pytest.mark.parametrize( + "bad_fps", + [ + float("nan"), + float("inf"), + float("-inf"), + True, + None, + "30", + 0, + -30, + ], + ids=[ + "nan", + "positive-infinity", + "negative-infinity", + "boolean", + "missing", + "nonnumeric", + "zero", + "negative", + ], +) +def test_info_json_refuses_non_finite_or_non_positive_fps( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, bad_fps: object +) -> None: + """Invalid fps metadata must be refused before any episode discovery runs.""" + corpus = _build_fake_corpus(tmp_path) + info = dict(corpus["info"]) + if bad_fps is None: + info.pop("fps") + else: + info["fps"] = bad_fps + expected_value = info.get("fps") + monkeypatch.setattr( + prep, "_hf_repo_info", lambda repo, rev: {"sha": rev, "license": "apache-2.0"} + ) + monkeypatch.setattr(prep, "_fetch_info_json", lambda repo, rev, cache: info) + + def fail_tree(repo: str, rev: str, path: str) -> list[dict]: + raise AssertionError("episode metadata discovery must not run after invalid fps") + + monkeypatch.setattr(prep, "_hf_tree", fail_tree) + + dataset_source = prep.DatasetSource(repo_id="fake/repo", revision="abc", license="apache-2.0") + cache_dir = tmp_path / "cache" + with pytest.raises(ValueError, match="fps") as excinfo: + prep._ensure_source_archive(dataset_source, cache_dir) + + message = str(excinfo.value) + assert "FPS must be finite and positive" in message + assert repr(expected_value) in message + # Refusal happens before episode metadata discovery: no downloads, no output. + assert not cache_dir.exists() or not (cache_dir / "meta" / "episodes").exists() + + +def test_info_json_accepts_normal_positive_fps( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + corpus = _build_fake_corpus(tmp_path) + corpus["info"]["fps"] = 30 + monkeypatch.setattr( + prep, "_hf_repo_info", lambda repo, rev: {"sha": rev, "license": "apache-2.0"} + ) + monkeypatch.setattr(prep, "_fetch_info_json", lambda repo, rev, cache: corpus["info"]) + monkeypatch.setattr( + prep, + "_hf_tree", + lambda repo, rev, path: ( + [{"path": "meta/episodes/chunk-000/file-000.parquet", "type": "file"}] + if "episodes" in path + else [{"path": "meta/info.json", "type": "file"}] + ), + ) + + def fake_dl(url: str, dest: Path, **kw: object) -> None: + import shutil + + dest.parent.mkdir(parents=True, exist_ok=True) + if "meta/episodes" in url: + shutil.copy( + str(tmp_path / "meta" / "episodes" / "chunk-000" / "file-000.parquet"), dest + ) + + monkeypatch.setattr(prep, "_download_file", fake_dl) + + dataset_source = prep.DatasetSource(repo_id="fake/repo", revision="abc", license="apache-2.0") + source_archive = prep._ensure_source_archive(dataset_source, tmp_path / "cache") + assert source_archive["fps"] == 30