test: make TestParseExamples validate against the committed models.dev snapshot - #4147
Merged
Conversation
EmbeddedSnapshot exposes the build-time snapshot.json catalog as a read-only singleton, and Fetch hits the live models.dev API directly, surfacing a network failure instead of silently degrading to a cache or the embedded snapshot. Together with the existing NewDatabaseStore these let callers build a Store that is either provably hermetic or provably live, instead of Store's usual best-effort network-then-cache-then-snapshot fallback. Groundwork for #4134.
TestParseExamples used to resolve model references via modelsdev.NewStore(), which tries a live HTTP fetch to models.dev before falling back to the embedded snapshot. On CI's normal internet egress this meant the test validated against whatever the live catalog said at the instant it ran, not against the snapshot committed to the repo — so an unrelated, unmodified PR could start failing hours after a green run once the live catalog moved on (observed on #4121). TestParseExamples now resolves against modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot()) only, so it is hermetic and safe as a required, PR-blocking check: it can only fail when the PR's own diff introduces an inconsistency. The shared skip-and-resolve logic is extracted into catalogModelRefs() and reused by a new, opt-in TestExamplesAgainstLiveModelsDev (gated on CHECK_MODELS_DEV_LIVE, mirroring the existing CHECK_MODELS_SNAPSHOT_FRESHNESS pattern), which hits the live API and reports drift as explicitly external, not PR-caused. The bedrock TestDetectCachingSupport_* tests had the identical live NewStore() bug; switched to the same hermetic store for the same reason. Fixes #4134.
Adds `task check-models-live` and a new, non-blocking models-live-check workflow (workflow_dispatch + a daily schedule) that runs TestExamplesAgainstLiveModelsDev against the real models.dev API. Deliberately not part of ci.yml or any required check: a failure here means the external catalog drifted, which can strike any PR (or none) at any time and gives the PR author no actionable signal — see #4134. The failure step appends a summary explaining that explicitly.
aheritier
added
area/ci
aheritier
marked this pull request as ready for review
September 3, 2026 15:31
trungutt
approved these changes
Sep 3, 2026
aheritier
added a commit
that referenced
this pull request
Sep 3, 2026
accounts/fireworks/models/kimi-k2-instruct does not exist in the fireworks-ai models.dev catalog and never has. Replace it with accounts/fireworks/models/kimi-k3, which is present in the current snapshot, across DefaultModels, examples/fireworks.yaml, tests, and the Fireworks provider docs (including the stale Available Models table, which also listed two other nonexistent ids). Also teach examples_test.go to resolve the actual models.dev catalog id for providers where it diverges from the docker-agent provider name (fireworks -> fireworks-ai, together -> togetherai, moonshot -> moonshotai, chatgpt -> openai, opencode-zen -> opencode) instead of unconditionally skipping validation, so this class of drift is caught automatically going forward. vercel now validates directly (models.dev catalogs it as-is); dmr/ovhcloud/cloudflare-* remain skipped with updated, accurate comments. Rebased onto main after #4147 (issue #4134/#4133) merged and added TestDefaultModelsExistInModelsDev, which validates DefaultModels directly against the models.dev provider id without the catalog-alias resolution above; apply the same modelsDevCatalogProviders mapping there so fireworks/together/moonshot/chatgpt/opencode-zen resolve correctly instead of failing on the raw docker-agent provider name. Fixes #4132
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4134.
Problem
TestParseExamplesresolved example model references viamodelsdev.NewStore(),which tries a live HTTP fetch to models.dev before falling back to the embedded
snapshot.json. On CI's normal internet egress this meant the test validatedagainst whatever the live catalog said at the instant it ran, not the snapshot
committed to the repo — so an unrelated, unmodified PR could go from green to red
hours later purely because the live catalog moved on (observed on #4121, where
nebiusdropped a model between two runs of the same commit).Fix
pkg/modelsdev: newEmbeddedSnapshot()(the committed catalog, no network/disk)and
Fetch()(always hits the live API, surfaces failures instead of degrading).TestParseExamplesnow resolves viaNewDatabaseStore(EmbeddedSnapshot())—hermetic, safe as a required/blocking check. Shared skip logic extracted into
catalogModelRefs.TestExamplesAgainstLiveModelsDev(gated onCHECK_MODELS_DEV_LIVE,same pattern as the existing
CHECK_MODELS_SNAPSHOT_FRESHNESS) hits the live APIand reports drift with an explicit "this is external catalog drift, not this PR"
message.
task check-models-live+ scheduled/dispatch-onlymodels-live-check.ymlworkflow runs it daily. Deliberately not added to
ci.yml, so live drift cannever block a PR.
Scope note
pkg/model/provider/bedrock/client_test.go's threeTestDetectCachingSupport_*tests had the identical live-
NewStore()bug; fixed the same way as same-root-causescope. A repo-wide search found no other live models.dev lookups left in tests.
Testing
task build,task test(full suite, excluding two pre-existing environmentfailures unrelated to this diff —
pkg/cacheandpkg/rag/treesitter, verifiedpresent on
maintoo),task lint,./scripts/workflow-lint.sh, andtask check-models-live(live network) all pass. VerifiedTestParseExamplesstays green with an unreachable proxy (proving hermeticity).