test(modelregistry): clear ZERO_DISABLE_MODELS_FETCH in the models.dev tests by Vasanthdev2004 · Pull Request #1169 · Twigpine/zero

Vasanthdev2004 · 2026-10-21T07:13:15Z

What changed

Four tests in internal/modelregistry/modelsdev_test.go now clear ZERO_DISABLE_MODELS_FETCH with t.Setenv, the way TestRefreshModelsDevCacheFetchesAndCaches and TestRefreshModelsDevCacheRejectsOversizedResponse already do:

TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering
TestDefaultModelEntriesAppliesFreshCache
TestCachedModelsDevProvidersIgnoresStaleCache
TestCachedModelsDevProvidersRequiresOptIn

The change is test-only.

Why

ZERO_DISABLE_MODELS_FETCH turns off both RefreshModelsDevCache and the cached overlay in cachedModelsDevProviders, and these four tests read it from whatever shell runs them. I hit this running the whole suite with the variable exported. On main:

--- FAIL: TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering (0.00s)
    modelsdev_test.go:154: bad body must return an error
--- FAIL: TestDefaultModelEntriesAppliesFreshCache (0.00s)
    modelsdev_test.go:252: fresh cache must overlay the registry: {ContextWindow:200000 MaxOutputTokens:64000}

The other two still pass, but only because the nil they expect comes from the variable, not from the check each one is named for. With the variable set on main, deleting the staleness check (|| time.Since(info.ModTime()) > modelsDevMaxAge) or the opt-in check (!modelsDevEnabled.Load() ||) from cachedModelsDevProviders leaves both tests green.

Verification

On Windows, with ZERO_DISABLE_MODELS_FETCH=1 exported:

main: two of the four tests fail as above, and both deletions above go unnoticed.
This branch: all four pass. Deleting the staleness check fails TestCachedModelsDevProvidersIgnoresStaleCache ("stale cache must be ignored"), and deleting the opt-in check fails TestCachedModelsDevProvidersRequiresOptIn ("overlay must be opt-in"). Skipping the body validation in RefreshModelsDevCache, or not applying the overlay in DefaultModelEntries, fails the other two.

With the variable unset, all four pass and catch the same four deletions on both main and this branch.

go vet ./... for windows, linux and darwin, gofmt and git diff --check are clean.
go test ./... passes with the variable unset, and again with it exported for the whole run. internal/modelregistry also passes under -race.
go run ./cmd/zero-release build and smoke pass, and the pinned golangci-lint run over internal/modelregistry reports nothing. govulncheck ./... reports only GO-2026-6809, the Go 1.26.9 crypto/x509 advisory main has too, which fix: bump Go to 1.26.10 for GO-2026-6809 #1168 fixes.

Summary by CodeRabbit

Tests
- Improved model registry cache test coverage by ensuring fetch, stale-cache, overlay, and fresh-cache scenarios run independently of environment configuration.

…v tests The variable turns off both the models.dev fetch and the cached overlay, and four tests in modelsdev_test.go inherited it from the shell. With it set, TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering and TestDefaultModelEntriesAppliesFreshCache fail, and TestCachedModelsDevProvidersIgnoresStaleCache and TestCachedModelsDevProvidersRequiresOptIn pass without reaching the check they are named for: the nil they expect comes from the variable. Clear it in each of them, as the two sibling refresh tests already do.

github-actions · 2026-10-21T07:15:55Z

Zero automated PR review

Verdict: No blockers found

Blockers

None found.

Validation

[pass] Diff hygiene: git diff --check
[pass] Tests: go test ./...
[pass] Build: go run ./cmd/zero-release build
[pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: 7d16cc85feb1
Changed files (1): internal/modelregistry/modelsdev_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

coderabbitai · 2026-10-21T07:16:27Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: Twigpine/zero/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: 4e7b79df-9aed-44ce-90e6-0a8a9018d55c

📥 Commits

Reviewing files that changed from the base of the PR and between 32f7c5d and 7d16cc8.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: Twigpine/zero/.coderabbit.yaml
Review profile: CHILL
Plan: Essentials
Run ID: c0fb8818-c334-4e08-b64d-726d3b1cf110

📥 Commits

Reviewing files that changed from the base of the PR and between cd3e0b0 and 32f7c5d.

📒 Files selected for processing: 1

internal/modelregistry/modelsdev_test.go

Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

Walkthrough

The model registry cache tests now clear ZERO_DISABLE_MODELS_FETCH before checking invalid responses, stale-cache rejection, overlay opt-in, and fresh-cache application.

Changes

Model registry cache tests

Layer / File(s)	Summary
Fetch environment setup for cache tests `internal/modelregistry/modelsdev_test.go`	Four tests clear `ZERO_DISABLE_MODELS_FETCH` before checking invalid responses, stale-cache rejection, overlay opt-in, and fresh-cache application.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: lhk0504

Merge Risk: ⚪ Minimal · up to 32f7c

The test updates are scoped and improve reliability across inherited environment configurations.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: clearing ZERO_DISABLE_MODELS_FETCH in the models.dev tests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…ble-env

jatmn

LGTM

Vasanthdev2004 requested a review from jatmn October 21, 2026 07:13

coderabbitai Bot approved these changes Oct 21, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into test/clear-fetch-disa…

7d16cc8

…ble-env

kevincodex1 approved these changes Oct 21, 2026

View reviewed changes

jatmn approved these changes Oct 21, 2026

View reviewed changes

jatmn merged commit 1d233c0 into main Oct 21, 2026
27 of 29 checks passed

Navigation Menu

Repository navigation

test(modelregistry): clear ZERO_DISABLE_MODELS_FETCH in the models.dev tests - #1169

test(modelregistry): clear ZERO_DISABLE_MODELS_FETCH in the models.dev tests#1169
jatmn merged 2 commits into
mainfrom
test/clear-fetch-disable-env

Vasanthdev2004 commented Oct 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

github-actions Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading

Review skipped

Uh oh!

jatmn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Vasanthdev2004 commented Oct 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

Verification

Summary by CodeRabbit

Uh oh!

github-actions Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Zero automated PR review

Blockers

Validation

Scope

Uh oh!

coderabbitai Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

jatmn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Vasanthdev2004 commented Oct 21, 2026 •

edited by coderabbitai Bot

Loading

github-actions Bot commented Oct 21, 2026 •

edited

Loading

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading