Stop three tests failing on every pull request

[mihow]

Summary

Three tests fail on pull requests for reasons unrelated to what those pull requests change. One fails on every run: it downloads a thumbnail from Wikimedia, which now refuses arbitrary thumbnail widths and returns an HTTP 400 pointing at its allowed-sizes policy. The other two fail intermittently: they test whichever sample image the filesystem happens to list first, and one of the sample images is an empty frame with nothing for the detector to find. This makes all three reliable, by serving the thumbnail from a local test server and by having tests pick their images deterministically. No application code changes; the fixes are entirely in the tests and their helpers.

None of this was caused by an open pull request. The last test run on main (2026-04-14) passed. The Wikimedia change happened after that, and main has not had a test run since, so its green status is out of date rather than evidence that the suite still passes.

The two intermittent failures were not network problems at all. The two API tests asked for "the first" trap image and then checked that the ML pipeline found insects in it. The helper picked that image from an unsorted directory listing, so which image a test got depended on filesystem ordering, which differs between checkouts and between runs. When the listing put the empty Vermont frame first, the pipeline correctly returned no detections and the test had nothing to assert on. Whenever it put a different image first, the same tests passed, which is why they looked flaky rather than broken.

List of Changes

#	Change (what it means for the reader)	How (implementation)
1	Tests that check the ML pipeline found insects always run against an image that actually contains insects, instead of whichever image the filesystem happened to return first.	test_logits_in_classification_response and test_config_num_classification_predictions pass an explicit filename via a new IMAGE_WITH_DETECTIONS constant in trapdata/api/tests/utils.py.
2	Every test that pulls sample images gets the same images on every machine and every run. This includes the Antenna worker and memory-leak tests, which now also receive images in sorted order; they already allow for an image without detections, so what they check is unchanged.	get_test_image_urls sorts the glob result and accepts an optional filenames argument, added last so existing callers are unaffected. The helpers pass subdir, num and filenames to each other by keyword.
3	The source image URL test no longer reaches out to the public internet, so it cannot break again when a third-party host changes what it will serve.	test_url writes an image to a temporary directory and serves it through StaticFileTestServer, which the repository already uses for this purpose. The same download-and-open code path is exercised, and the image has unequal dimensions so a mixed-up image would be caught, not only a failed download.

Verification

When this pull request was opened (2026-08-12), run against this branch with the GPU disabled, matching the CPU-only CI runners:

• Full suite: 39 passed, 1 skipped, 0 failed.
• The three previously failing tests passed.
• Negative control: temporarily pointing IMAGE_WITH_DETECTIONS at the empty frame reproduced the exact CI failure (AssertionError: No detections found in response), confirming the image choice is what these tests turn on.
• Pinned tooling was clean on the changed files: black 22.3.0, isort 5.11.5, flake8 4.0.0 with bugbear and comprehensions, autoflake 1.4.

Rechecked on 2026-09-11, during review:

• The three tests pass on this branch, and with this branch merged into Restore model downloads after the storage move and let deployments choose where models come from #168. The pipeline tests used locally cached model weights, so this confirms the test changes, not the downloads.
• Restore model downloads after the storage move and let deployments choose where models come from #168's CI, which does not include this branch, fails only TestSourceImageSchema::test_url, with 54 other tests passing on Python 3.10 and 3.12.

Notes for reviewers

• Merge this together with Restore model downloads after the storage move and let deployments choose where models come from #168, this one first. The model weights these tests download were hosted on an object store that was retired in the recent storage move, and that host now returns HTTP 403. CI has no model cache, so any fresh CI run of this branch on its own is expected to fail on those downloads. The green checks from 2026-08-12 predate the move, and the run triggered by the latest commit here is likely to go red for this reason. Restore model downloads after the storage move and let deployments choose where models come from #168 points model downloads at the new store, and its only failing test is the one this pull request fixes. Merging this first and then updating Restore model downloads after the storage move and let deployments choose where models come from #168's branch should give a fully green run.
• The detector threshold and the empty Vermont frame are both left as they are. An image with no insects in it is a reasonable thing to keep in the sample set, and the detector returning nothing for it is correct behaviour; the bug was that a test could land on it by accident.
• The ML tests still download model weights from a remote object store at test time. That dependency is what broke after the storage move; caching the weights in CI would remove it and is worth a separate change.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f2546da9-c236-404e-8b9c-4e4fe51f853b

📥 Commits

Reviewing files that changed from the base of the PR and between a33746a and a1f1ad9.

📒 Files selected for processing (3)

• trapdata/api/tests/test_api.py
• trapdata/api/tests/test_models.py
• trapdata/api/tests/utils.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.}

[mihow]

Claude says: Reviewed in place of the CodeRabbit pass that was rate-limited. This looks good to merge. It is posted as a comment review because it comes from the author's account, which cannot approve its own pull request.

What's solid

• Each fix targets the actual cause. Sorted selection removes the dependence on directory order, the two pipeline tests name an image that has detections, and test_url serves a generated image from the existing local test server (free port, context manager) while exercising the same download-and-open path.
• The unequal image dimensions in test_url are a nice touch: they would catch a mixed-up image, not only a failed download.
• filenames is added as the last parameter, so existing callers are unaffected. The six callers in trapdata/antenna/tests/ now also receive sorted images; I checked that each tolerates an image without detections (len(response.detections) >= 0, or an explicit empty check), so the deterministic order does not introduce a new failure mode.

Worth knowing before merge

1. The green checks predate the object store move. They ran on 2026-08-12. The CI workflow has no model cache, and the insect-orders pipeline these tests use downloads its weights from the previous object store host, which now returns 403 for both the classifier and the detector weights; the URLs in #168 return 200. A re-run of this pull request on its own would likely fail on those downloads. The two pull requests complement each other: #168's CI currently fails only TestSourceImageSchema::test_url, which this one fixes. Suggest merging this first and then updating #168's branch, which should come back green.
2. Description wording. "Three tests have been failing on every pull request" holds for test_url, but the two order-dependent tests fail intermittently rather than every time; both passed in #168's latest CI run. The fix is still the right one, since it removes the flakiness. It may also be worth a line pointing at #168, because the remote-weights dependency mentioned under "Notes for reviewers" has since become a real failure.

Smaller cleanups (low priority)

Three inline suggestions: an existence check for explicitly named fixtures, an assertion that can no longer fail, and passing arguments by keyword. None of them block merging.

[mihow]

Claude says: Small robustness suggestion: an explicitly named file is not checked for existence here, so if the fixture is ever renamed or removed, the test would likely fail as a 404 from the test server inside the pipeline rather than as a clear missing-fixture error. For example:

paths = [images_dir / filename for filename in filenames]
missing = [path.name for path in paths if not path.exists()]
assert not missing, f"Test images not found in {images_dir}: {missing}"

It may also be worth saying in the docstring that num is ignored when filenames is given.

[mihow]

Claude says: With an explicit filename this assertion can no longer fail, because the helper always returns one URL per filename. It could be dropped, or made meaningful again by the existence check suggested in utils.py. The same applies at line 191.

[mihow]

The remaining CI failures are due to the new model URLs. Merging this, then will work on #168