🧪 test(models): pin the OpenRouter catalog fetch tests to repo-controlled ids - #111
Conversation
…lled ids The live-fetch and permissive tests asserted a frozen catalog snapshot: they built preferred_ids from the unpinned remote manifest and then checked that specific model ids appeared in the result. When the curated list rotated, the mocked live entries were never in preferred_ids, so they were silently skipped and the assertions failed with no real defect — reddening the integration base and every PR targeting it. Rewrite both as behavior contracts. Each test now pins the curated list to repo-controlled placeholder ids (and patches the remote manifest to empty) so preferred_ids is deterministic, then asserts the invariant under test rather than a snapshot: free/paid tags are recomputed from live pricing, and an entry with no supported_parameters is retained while an explicit list omitting tools is dropped. Adding or removing a model from the real catalog can no longer red these tests. No production code changed — the fetch/filter path was correct; only the tests were frozen.
cwest
left a comment
There was a problem hiding this comment.
No changes needed.
The diagnosis holds up against the code. fetch_openrouter_models builds preferred_ids from get_curated_openrouter_models() (imported lazily at hermes_cli/models.py:1359) and falls back to the in-repo OPENROUTER_MODELS only when that returns empty. The two old tests mocked the live payload but left preferred_ids sourced from the ambient manifest, so once the curated list rotated away from the mocked ids the entries were skipped at models.py:1392 and the assertion compared against whatever the manifest happened to hold. That is the snapshot-freeze the repo guidance rules out.
The rewrite pins both the curated source (get_curated_openrouter_models -> [] and OPENROUTER_MODELS -> a fixed test-vendor/* list) and the live payload, then asserts the relationship rather than a frozen result: zero-priced entries tag free, non-zero stay paid, curated[0] carries recommended, a missing supported_parameters field is retained, and an explicit list omitting tools is dropped. Every assertion maps to a real branch in models.py:1390-1406, and the patch targets are the right ones given the lazy import.
I ran the negative control and the pricing invariant through mutation checks on a throwaway copy of models.py: making the tools filter unconditionally permissive turns test_permissive_when_supported_parameters_missing red, and inverting the free/paid tag turns test_live_fetch_recomputes_free_tags red. Both catch the regression they claim to. tests/hermes_cli/test_models.py is 87 passed locally, and the id rotation this test used to freeze on can no longer red it since the two rewritten tests carry no third-party id whose presence is outside this repo's control. CI is green and the branch is clean.
Why
cwest/integrationwas RED on its own:tests/hermes_cli/test_models.pyyielded
2 failed, 85 passed(CI slice 7/8), so every PR targeting the branchinherited a failing required check regardless of its content.
Both failures were
AssertionError: assert 'qwen/qwen3.7-max' in ['anthropic/claude-opus-4.8'].Root cause
fetch_openrouter_modelsbuilds itspreferred_idslist from the curatedcatalog (
get_curated_openrouter_models()remote manifest, falling back to thein-repo
OPENROUTER_MODELSsnapshot), then keeps only the ids the live payloadalso carries. The two failing tests mocked the live payload but did not pin
the curated source — so
preferred_idscame from whatever the ambient remotemanifest currently returns. When the curated list rotated, the mocked live
entries were no longer in
preferred_idsand were silently skipped, failing anassertion with no underlying defect.
This is exactly the snapshot-not-contract class AGENTS.md calls out ("Behavior
contracts over snapshots. Tests should assert how two pieces of data must
relate, not freeze a current value — model lists…"). The sibling test
test_filters_out_models_without_tool_supportalready got this right by pinningthe curated list; the two failing tests were never updated to match.
The fetch/filter path itself is correct, so no production code changed — the
fix is entirely in the test file.
What
Rewrite both tests as behavior contracts:
(
test-vendor/...) viamonkeypatch.setattr(OPENROUTER_MODELS, …)and patchget_curated_openrouter_modelsto empty, sopreferred_idsis deterministicand independent of the live catalog / remote manifest.
test_live_fetch_recomputes_free_tags— tags are recomputed from livepricing (zero-priced →
free, non-zero → paid), and every covered curatedid is retained.
test_permissive_when_supported_parameters_missing— an entry with nosupported_parametersis retained, while an explicit list omittingtoolsis dropped (negative control preserves the fix(gateway): hide models without tool support from list Kilo-Org/kilocode#9068 behavior).
The rewritten tests contain no third-party model id whose presence is outside
this repo's control; adding or removing a model from the curated list can no
longer red them.
Verification
2 failed, 85 passedon this branch'sbase (
bb87eebfc), matching CI slice 7/8, before the fix.scripts/run_tests.sh tests/hermes_cli/test_models.py→ 87 passed, 0failed after the fix.
hermes_cli/models.pyis untouched (git diff --statshows only the testfile), confirming this was the behavior half, not a lookup defect.
tests/hermes_clifailures (WSL gateway, service manager, signalhandler, qwen-oauth resolution) are pre-existing on the pristine base —
verified by reverting this fix and re-running those four files (same 7
failures) — and are unrelated to the OpenRouter catalog / out of this card's
scope.