Tests that need a live embed backend must SKIP, not FAIL, so the CI deselect list can be deleted

[jaylfc]

CARD TITLE (intent, not commit subject): Tests that need a live embed backend must SKIP, not FAIL, so the CI deselect list can be deleted

Autonomous build of board card tsk-hfy6dg.

Files:
.github/workflows/ci.yml | 19 +-----------
tests/conftest.py | 54 +++++++++++++++++++++++++++++++++
tests/test_catalog_pipeline.py | 2 +-
tests/test_hermes_audit_batch2.py | 6 ++--
tests/test_http_server_registry_auth.py | 2 +-
tests/test_project_storage.py | 8 ++---
tests/test_reindex.py | 2 +-
7 files changed, 65 insertions(+), 28 deletions(-)

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b764e1a-a108-4102-8d69-05eb5c53af2d

📥 Commits

Reviewing files that changed from the base of the PR and between 2151279 and 075b39c.

📒 Files selected for processing (7)

• .github/workflows/ci.yml
• tests/conftest.py
• tests/test_catalog_pipeline.py
• tests/test_hermes_audit_batch2.py
• tests/test_http_server_registry_auth.py
• tests/test_project_storage.py
• tests/test_reindex.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (7 files)

• .github/workflows/ci.yml
• tests/conftest.py
• tests/test_catalog_pipeline.py
• tests/test_hermes_audit_batch2.py
• tests/test_http_server_registry_auth.py
• tests/test_project_storage.py
• tests/test_reindex.py

---

_{Reviewed by step-3.7-flash · Input: 82.2K · Output: 31.4K · Cached: 348.4K}

[jaylfc]

Approved. The card's goal is met and I measured it rather than reading it.

My card (tsk-hfy6dg). The claim to test is "the deselect list can be deleted", so that is what I ran.

Acceptance, on a box with NO embed backend (verified: no model.onnx at either candidate path, and localhost:7832 unreachable):

full suite, no --deselect flags:   1301 passed, 12 skipped, 0 failed  (137s)

The ten formerly-deselected tests skip instead of failing, and the CI workflow's entire deselect block plus its "do not grow this list" comment are gone. That was the point of the card.

The check that actually matters, because "10 skipped" is an ambiguous result

Ten skips are equally consistent with "the guard works" and "the guard always skips", and the second would be a silent loss of ten tests' coverage while looking greener than before. So I ran both halves:

A. no backend (real state of this box)      -> ssssssssss   10 skipped
B. TAOSMD_ONNX_PATH -> a dir with model.onnx ->  2 failed   (tests RAN)
C. same two tests, no backend                ->  ss          2 skipped

B is the one that makes A meaningful: with the detector satisfied the tests execute rather than skip, so the skip is conditional on the backend and not unconditional.

Coverage did not silently widen either. Exactly 10 call sites request the fixture, distributed 1/3/1/4/1 across test_catalog_pipeline / test_hermes_audit_batch2 / test_http_server_registry_auth / test_project_storage / test_reindex, which matches the deselect list file for file. No test that used to run is now skipped.

The skip message is also the right kind: it names both ways to get a backend (scripts/setup.sh or TAOSMD_ONNX_PATH, or qmd serve) rather than just reporting absence.

Defect, not blocking, and carded as follow-up

_has_onnx_model() tests only that the file EXISTS, never that it loads. My step B above is the proof: an EMPTY model.onnx satisfied the detector, so the tests ran and FAILED rather than skipping, which is the exact failure mode this card exists to remove, reappearing in a narrower case.

An empty file is artificial, but a truncated one is not: an interrupted scripts/setup.sh leaves precisely that, and the user then sees two confusing failures instead of a skip that tells them to re-run setup. Worth noting the PR is still a strict improvement here, because before it those tests failed unconditionally.

Suggested fix in the follow-up card: treat an embed failure at fixture time as a skip too, so the guard keys on "can this backend actually embed" rather than "is there a file".

Minor, same card: _has_qmd_service() is a function-scoped fixture, so it re-probes per test. Connection-refused returns instantly (the 10 skips took 1.6s total), but on a network where the port is filtered rather than refused each probe waits the full 2s timeout. Session-scoping or caching the answer costs nothing.

Merging.