Skip to content

test: cover OpenAI compatible execution validation - #412

Merged
kstonekuan merged 6 commits into
Hebbian-Robotics:mainfrom
msparsh13:fix/issue-407
Sep 5, 2026
Merged

test: cover OpenAI compatible execution validation#412
kstonekuan merged 6 commits into
Hebbian-Robotics:mainfrom
msparsh13:fix/issue-407

Conversation

@msparsh13

Copy link
Copy Markdown
Contributor

Summary

Add regression coverage for validation performed by OpenAICompatibleExecution.__post_init__.

The tests now cover previously untested invalid configurations, including model whitespace, invalid response format types, malformed API key environment variable names, invalid max_tokens / max_retries values, boolean integer edge cases, and non-finite temperatures.

Validation

Ran:

  • uv run pytest tests/test_build_ai_vlm_checks.py -q
  • uv run pytest -q
  • uv run ruff check --fix
  • uv run ruff format
  • uv run ty check

All relevant checks passed.

Checklist

  • I added or updated outcome-focused tests for changed business logic.
  • I updated documentation for changed behavior, flags, formats, or requirements.
  • I ran uv run ruff check --fix, uv run ruff format, and uv run ty check.
  • I ran the relevant pytest suite.
  • I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
  • I preserved stored-data compatibility or documented an explicit version change.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

👋 Hi @msparsh13 — thank you so much for your first contribution to HFlow!

A maintainer will review your pull request as soon as possible. In the meantime:

💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game.

We are excited to have you here and appreciate your help making the project better! 🙌

@Sagar-024

Sagar-024 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Hi @msparsh13, welcome and nice work. The new rows read exactly like the validation function's docs.

Two small things before merge:

  1. max_tokens has the same bool guard (build_ai_vlm_checks.py:161) but no row with max_tokens=True. Worth adding.
  2. Heads up, fix(build-ai): cover completeness knobs in the check version contract #411 adds tests to this same file, so whichever lands second needs a quick rebase. Nothing in your rows overlaps.

Optional: the response_format row matches on a prefix only. Pinning the full message would catch a future rewording.

Good first PR, hope to see more.

kstonekuan and others added 2 commits September 5, 2026 00:55
The twelve-case parametrize decorator sat on
test_openai_compatible_execution_accepts_valid_configuration, which takes
no arguments, so the file did not collect at all: "function uses no
argument 'endpoint'". The refusal test below it had lost its decorator
and still had the old three-parameter signature.

Decorator moved onto the refusal test, signature widened to the eight
fields the table supplies, and the orphaned copy removed.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging with a fixup. Welcome, and the table itself is exactly right: every one of the nine refusals is now the only failure when its own condition is neutered, including both isinstance(..., bool) clauses separately from the type checks around them.

model empty / padded                       caught
response_format wrong type                 caught
api_key env var name malformed             caught
max_tokens not an int / <= 0               caught
max_retries not an int / negative          caught
temperature not finite                     caught
max_tokens bool guard alone                caught
max_retries bool guard alone               caught

uncovered refusals: 0

Those last two are the ones I most wanted and the easiest to leave out, because a test with max_tokens="32" looks like it covers the same ground and does not.

The fixup, dd5ea87. The file did not collect as submitted:

ERROR collecting tests/test_build_ai_vlm_checks.py
In test_openai_compatible_execution_accepts_valid_configuration: function uses no argument 'endpoint'

The twelve-case decorator had ended up on ..._accepts_valid_configuration, which takes no arguments, while the refusal test below still carried the old three-parameter signature and no decorator. Everything you wrote was right, it was assembled in the wrong order. I moved the decorator onto the refusal test, widened its signature to the eight fields the table supplies, and deleted the orphaned copy.

Worth saying plainly, because your PR description lists uv run pytest -q as passing: it does not pass on the submitted commit, it errors during collection, so the whole file is skipped rather than failing loudly. That is a nasty failure mode to hit as a newcomer, since a collection error scrolls past differently from a test failure. Nothing about it reflects on the work.

One thing to know rather than fix: #414 landed a couple of hours before this and covers max_retries=True too, so that one case is now asserted twice, and there are two positive-control tests. Harmless duplication, and I would rather leave both than delete someone else's test to tidy mine.

Gate on the merged result: ruff check, ruff format --check, ty check clean, 1557 passed / 6 skipped.

Closes #407. If you want another, the starter list is at https://github.com/Hebbian-Robotics/hflow/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22, and #408 is the closest in shape to what you just did. Two tips for next time: only an assignee reserves an issue, so anything unassigned is fair game, and we cap non-collaborators at one open PR at a time so a bot will politely close a second and invite you back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants