Skip to content

TestParseExamples conflates hermetic snapshot-consistency checks with live models.dev catalog drift #4134

Description

@aheritier

What's wrong

pkg/config/examples_test.go's TestParseExamples validates every example's model reference by calling modelsdev.NewStore() with no WithFetcher/WithCache override. modelsdev.Store tries a live HTTP fetch to the real models.dev API first, and only falls back to the repo's embedded pkg/modelsdev/snapshot.json if that live fetch fails. GitHub Actions runners have normal internet egress, so in practice this test validates against whatever models.dev's live catalog says at the exact instant CI runs — not against the snapshot committed to the repository.

This was concretely observed on #4121: the PR's original CI run passed; a re-run of the identical commit roughly 9 hours later failed, because the live models.dev catalog had, in the interim, dropped moonshotai/Kimi-K2.5 from the nebius provider (examples/nebius.yaml references it). Nothing about the PR's diff changed between the two runs — only the live external catalog did.

Why this is a problem

This conflates two failure modes that have very different implications and should be treated differently:

  1. Snapshot-consistency failures — the PR's own change to pkg/modelsdev/snapshot.json (or another file) makes an example/config reference a model that the committed snapshot no longer has. This is a real, local problem introduced by the PR, and it's entirely reasonable to block the PR on it.
  2. Live-catalog-drift failures — the committed snapshot and the PR's diff are both fine; the external, real-time models.dev catalog has independently moved on since the snapshot was last refreshed. This is not the PR author's fault, is not something their change caused, and can strike any PR at any time regardless of what it touches — including a PR that only edits documentation with no code changes — with zero actionable signal to the author.

Today TestParseExamples can only produce failure mode #2 for any provider not in modelsDevAbsentProviders (there's no test that pins to the committed snapshot at all), so a red TestParseExamples gives no signal about which of the two happened, and a PR author sees "your PR broke this" when the actual cause is "the internet changed" — confusing and unhelpful, and blocks unrelated PRs on external drift they can't fix by rebasing.

Suggested fix direction

We likely want both kinds of coverage, but running as separate checks with separate blast radius:

  • Hermetic, PR-blocking test: run TestParseExamples-equivalent validation against the embedded/committed snapshot.json only (e.g. via modelsdev.NewDatabaseStore seeded from the embedded snapshot, or WithFetcher/WithCache pointed at a fixture that never hits the network). This is what should gate merges — it only fails when the PR's own diff introduces an inconsistency.
  • Live/scheduled, non-blocking check: keep (or add) a separate job — e.g. on a schedule, or as an informational/non-required PR check — that does hit the real models.dev API, so drift is still caught and surfaced, but it alerts the team rather than blocking whatever PR happens to be open when it fires. Its failure output should make clear to whoever reads it that this is an external-catalog issue, not something caused by any particular PR's diff.

References

Activity

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

Metadata

Metadata

Assignees

Labels

area/ciCI/CD workflows and pipelinearea/modelsLLM model integrations and model providersarea/testingTest infrastructure, CI/CD, test runners, evaluation

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions