VideoEpisodeSpec accepts bools and NaN that its sibling validator refuses #274

Current behavior

src/hflow/testing.py has two spec validators sitting 30 lines apart. They disagree about the same bad inputs.

_validate_synthetic_episode_spec (:382) opens by calling _require_float and _require_int on every numeric field, then checks each value is finite, then checks the ranges. _validate_video_episode_spec (:352) does none of that. It goes straight to spec.source_start_s < 0.0 and compares whatever it was handed.

Measured on current main (7dedc63), the same value through each validator:

VIDEO  duration_s=True            ACCEPTED
VIDEO  image_hz=True              ACCEPTED
VIDEO  duration_s=float("nan")    ACCEPTED
VIDEO  image_hz=float("inf")      ACCEPTED

SYNTH  duration_s=True            ValueError: duration_s must be an int or float, got bool
SYNTH  image_hz=True              ValueError: image_hz must be an int or float, got bool
SYNTH  duration_s=nan             ValueError: duration_s must be finite, got nan
SYNTH  image_hz=inf               ValueError: image_hz must be finite, got inf

duration_s=True is accepted and silently means one second, because True == 1. duration_s=nan is accepted because every comparison against NaN is false, so nan <= 0.0 does not trip the range check that is supposed to catch it.

One case refuses for the wrong reason and is worth seeing before you fix it: image_width=True does raise, but the message is image dimensions must be even so the canonical H.264 transform can use yuv420p. True is odd. The type error is being caught by an evenness rule, by luck.

Why the asymmetry rather than the individual holes

_require_int and _require_float are defined at :60 and :65 of this same file. The module docstring at :34-36 explains they were deliberately copied in here rather than imported, so the incubating video-measurements package could stay extractable. They exist, they are two lines each, and one of the two validators in the file calls them.

bool subclassing int is a defect this repository has now closed in about eight places. This is the ninth, and it is the one sitting next to the fix.

VideoEpisodeSpec is not internal-only: examples/egocentric/prepare.py builds one, alongside tests/test_motion.py and tests/test_testing.py.

Nothing would catch a regression either

tests/test_testing.py:285-340, test_synthesize_episode_refuses_invalid_spec, is a parametrized table of sixteen rejected SyntheticEpisodeSpec values, including duration_s at zero, negative, NaN, and True. There is no equivalent for VideoEpisodeSpec. The pattern to copy is in the same file as the gap.

What to build

Bring _validate_video_episode_spec up to what its sibling already does, and pin both.

The direct move is to call _require_float and _require_int on the video spec's numeric fields and add the finiteness check. Worth thinking past before you write it: the two validators would then open with the same eight lines, and this issue exists because they drifted apart once already. Consider whether the shared part belongs in one place that both call. Say which you chose and why.

Decide deliberately what source_start_s, image_width, and image_height should accept, and say so in the PR. They are not covered by the transcript above and they are not all the same case.

Leave the "dimensions must be even" rule alone. It is correct and it is about H.264, not about types. Once a real type check runs first, image_width=True will raise the right error, and a test should pin that it is now the type error rather than the evenness one.

Definition of done

1. _validate_video_episode_spec refuses a bool and a non-finite value for every numeric field it validates, with a message naming the field.
2. A parametrized rejection table for VideoEpisodeSpec, following test_synthesize_episode_refuses_invalid_spec. Prove each row bites: remove the guard it covers, watch that one row go red, put it back.
3. Every existing test passes untouched. No currently valid spec becomes invalid.
4. image_width=True raises the type error, not the evenness error, and a test says so.
5. If you unify the shared validation, the synthetic spec's existing sixteen-row table still passes unchanged. That table is the contract for the half that already worked.

Validation

uv sync --locked --all-extras
uv run ruff check --fix
uv run ruff format
uv run ty check
uv run pytest -q

Run plain uv run pytest -q from the repo root: pytest tests alone misses packages/hflow-server/tests.

Notes

No ffmpeg run and no fixture needed for the validation itself: these are pure functions over a frozen dataclass. synthesize_episode in the existing table is called only to reach the validator, and the refusal happens before any encoding.

Do not change SyntheticEpisodeSpec's behavior. It is the half that is already right, and it is the reference for what the other half should do.