Centralize numeric range validation - #494
kstonekuan merged 4 commits into
Conversation
There was a problem hiding this comment.
The guard set and composition are right, and require_non_negative_real keeps batching's numbers.Real contract rather than narrowing it.
Three conversions are not. Every place you needed a cast or a try/except is a place the guard does not belong: :366, :520, :789 are the only bool sites in that file outside a __post_init__, and all three parse an external model response. _field_guards says it is for caller-constructed configuration.
The cost shows in your own test edit: parse_hand_count_response("two") went from "hand count must be 0, 1, or 2" to "must be an int, got str". The second is a fact about Python types; the first is the contract a model failed.
Revert those three, keep the ten. Separately, have require_int return the value instead of None and the remaining seven casts go away.
|
Got it, thanks fir review @kstonekuan , I reverted the three response-handling sites and updated the guards to return the narrowed value so the casts are gone. Pushed the changes |
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.
There was a problem hiding this comment.
LGTM, merging.
Casts gone entirely, and the three response sites are byte-identical to main.
Pushed one commit: skip_black_frames was the only guard left unheld, and type(x) is not bool went back to isinstance (bool cannot be subclassed, so they are equivalent).
2 participants
Adds shared numeric guards for finite-value and bounds validation, then uses them in
build_ai_vlm_checks.pyandbatching.py.This removes duplicated bool checks and standardizes numeric validation errors while preserving existing valid-input behavior, including batching's support for fractional values.
catalog.pyremains unchanged.Validation
uv sync --locked --all-extrasuv run ruff checkuv run ruff format --checkuv run ty checkuv run pytest -q— 1,780 passed, 7 skippedCloses #487