Hebbian-Robotics / Hebbian-Robotics/hflow

checks.py still hand-rolls five numeric guards that _field_guards now covers

Open
#498 5 comments 0 reactions 0 assignees View on GitHub
enhancement help wanted
Dominant language
Python
Stars
269
Forks
150
Avg merge
8h 18m
Merged PRs (30d)
246

Description

#487 landed the guard set in `_field_guards` (#494) and converted `batching.py` and the configuration half of `build_ai_vlm_checks.py`. `checks.py` is the largest file still hand-rolling it.

## Measured on current main

38 hand-rolled `isinstance(x, bool)` sites remain across 16 files. Two groups are deliberately not targets:

- `catalog.py` (6): coerces NumPy scalars, which `_field_guards` deliberately does not. See its module docstring. Leave it.
- `build_ai_vlm_checks.py` (4): three parse external model responses and one is a genuine bool field. All four are correct as they are, and #494's review covers why.

That leaves `checks.py` as the biggest convertible one, with 5:

| line | parameter | current shape |
| --- | --- | --- |
| `:991` | `shake_threshold_dps` | bool + `np.isfinite` + `< 0` |
| `:998` | `unstable_min_duration_s` | bool + `np.isfinite` + `< 0` |
| `:1005` | `horizontal_field_of_view_degrees` | bool + `np.isfinite` + `not 0 < x <= 360` |
| `:1659` | `max_plausible_fps` | bool, then `np.isfinite` + `<= 0` |
| `:1664` | `downsample_tolerance_fps` | bool, then `np.isfinite` + `< 0` |

Four of the five map straight onto the new helpers:

```python
require_non_negative_float(shake_threshold_dps, "shake_threshold_dps")
require_non_negative_float(unstable_min_duration_s, "unstable_min_duration_s")
require_positive_float(max_plausible_fps, "max_plausible_fps")
require_non_negative_float(downsample_tolerance_fps, "downsample_tolerance_fps")
```

`horizontal_field_of_view_degrees` is the odd one: it wants a half-open range, `0 < x <= 360`. There is no helper for that. Add one only if a second caller wants the same shape; otherwise compose `require_finite_float` with the bound left in place, and say which you chose.

## What this costs, which is the part to decide

Every message changes to `_field_guards`'s spelling. `"shake_threshold_dps must be finite and non-negative"` becomes `"shake_threshold_dps must be >= 0, got -1"`. The second names the offending value, which is better, but it is a user-visible string change across five parameters and it will move test assertions.

That was accepted for #487 and it is accepted here. Two things not to lose:

- The guards must stay **above** `selected_cameras`, which is what #445 and #447 were about. A guard inside the per-camera loop does not fire on a camera-less episode.
- The messages must name the parameter as the signature spells it, never the internal settings field. That was the other half of #447.

## Definition of done

1. `rg -c "isinstance\([a-z_.]+, bool\)" src/hflow/checks.py` reports at most 1 (the field-of-view case, if you leave it composed).
2. Every refusal the file made before, it still makes: bool, non-finite, and out-of-range for all five parameters, each on a camera-less episode.
3. Tests use `match=` with anchored patterns. On an episode with cameras a camera-processing failure also raises `ValueError`, so an unanchored `pytest.raises(ValueError)` passes whether or not the guard is there. #492 has the shape to copy.
4. Mutation: neuter each guard in turn and confirm a test goes red.

## Sequencing

**Wait for #459 to merge.** It is open against the guards in `camera_frame_stats` and `camera_signal_quality` in this same file and will add more of them. Converting underneath it would conflict.

## Validation

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

Contributor guide

Open the contributing guide

Research direction

Wait for #459 to merge, then start in src/hflow/checks.py by reading _field_guards and the five validation sites around lines 991, 998, 1005, 1659, and 1664. Use #492's anchored match= test shape, covering bool, finite, and range refusals on camera-less episodes, then run the listed uv checks and pytest commands. Done means the remaining bool guard count is at most one and each guard is mutation-tested.

Written by the indexing model from the issue text.

Assessment

Tech stack
numpy, python
Domain
data-engineering, testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.