Eight of the nine refusals in OpenAICompatibleExecution are untested, including both bool-is-an-int guards

[kstonekuan]

OpenAICompatibleExecution is where a caller's model endpoint configuration is parsed, so its __post_init__ is a trust boundary: it decides what reaches a remote model call. It has nine refusals. One is tested.

The existing test covers two cases, and only one of them is a __post_init__ refusal:

tests/test_build_ai_vlm_checks.py:107-112
    ("localhost:8000/v1",        "model", "absolute http")           <- _require_absolute_http_url
    ("http://localhost:8000/v1", " ",     "model must not be empty") <- the one covered refusal

Verification

Against main at 44e4f50, each refusal's condition replaced with if False: in turn, running tests/test_build_ai_vlm_checks.py:

model empty                                caught
model padded                               NOTHING NOTICED
response_format wrong type                 NOTHING NOTICED
api_key_environment_variable malformed     NOTHING NOTICED
max_tokens not an integer                  NOTHING NOTICED
max_tokens <= 0                            NOTHING NOTICED
max_retries negative                       NOTHING NOTICED
temperature not finite                     NOTHING NOTICED
max_tokens bool guard alone                NOTHING NOTICED

Eight of nine can be deleted with the suite still green.

The two worth caring about most

max_tokens and max_retries each carry a separate isinstance(..., bool) clause:

src/hflow/build_ai_vlm_checks.py:160   if not isinstance(self.max_tokens, int) or isinstance(self.max_tokens, bool):
src/hflow/build_ai_vlm_checks.py:164   if not isinstance(self.max_retries, int) or isinstance(self.max_retries, bool):

bool subclasses int, so True passes isinstance(x, int) and compares equal to 1. Without that clause max_tokens=True is accepted and becomes a one-token request. Removing the max_tokens bool clause alone leaves the suite green, so the clause that exists precisely because this is easy to get wrong is the clause nothing holds. This repo has hit that family repeatedly (#392 for interval bounds, #364 for a schema version), which is why it is worth its own case rather than folding into a general type check.

What to do

Extend the parametrization at tests/test_build_ai_vlm_checks.py:107 (or add a second one beside it, since these cases need to vary fields other than endpoint and model) to cover the eight uncovered refusals, asserting the message rather than just ValueError.

Pattern to copy

The parametrized refusal table already in that file, at tests/test_build_ai_vlm_checks.py:107-123. For the bool-is-an-int case specifically, tests/test_catalog_curation.py has the same shape for interval bounds.

Definition of done

1. Each of the eight refusals is the only failure when its own condition is neutered. Check this rather than assuming it; some of these fields interact.
2. max_tokens=True and max_retries=True are refused, each as its own case, and each fails when only the isinstance(..., bool) clause is dropped rather than the whole condition.
3. Messages are asserted, not just the exception type.
4. A valid configuration still constructs (one positive case, so the parametrization cannot pass by refusing everything).

Not in scope

HFlowHostedExecution's refusals at :184-195 and :211. Same file, same shape, and also uncovered, but they belong to a different class and a different endpoint contract; they can follow as their own issue.

Validation

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

See CONTRIBUTING.md.

[chiruu12]

I'd like to take this one.

Plan is to extend the parametrised case list in tests/test_build_ai_vlm_checks.py to cover the eight uncovered refusals, with max_tokens=True and max_retries=True as their own cases rather than folded into the type checks. I'll verify each one by neutering its clause and confirming the new case fails, so none of them lands as a test that passes with the guard deleted.

[kstonekuan]

Resolved by #412. All nine refusals in OpenAICompatibleExecution.__post_init__ are now individually held, including both isinstance(..., bool) clauses separately from the type checks around them.

[chiruu12]

Withdrawing this. #412 was already open when I commented and covers the same eight refusals. I checked the issue for linked PRs and missed the open one, which is my error. Leaving it to @msparsh13.