Skip to content

fix(core): decouple model catalog refresh from persistence - #45363

Open
kitlangton wants to merge 1 commit into
v2from
models-cache
Open

fix(core): decouple model catalog refresh from persistence#45363
kitlangton wants to merge 1 commit into
v2from
models-cache

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

Why

The models.dev catalog is larger than Durable Object SQLite's 2 MB value limit. Although failed KV writes are now tolerated, refresh still discarded the downloaded catalog and reloaded the old persisted value or bundled snapshot, so successful downloads could leave models permanently stale.

Persistence should help the next startup, not determine whether a running server can adopt a catalog update. This replaces the storage choice from #41649 while retaining support for filesystem-less runtimes.

What Changes

Situation Behavior
Bun/Node startup Seed from a source-specific file cache, falling back to the bundled snapshot. Readers need not wait for the background HTTP refresh.
workerd startup Seed from the bundled snapshot; downloaded updates stay in memory, not in each Durable Object's database.
Successful changed response Decode and normalize, adopt in memory, publish Refreshed, then attempt persistence.
Cache read/write failure Fall back during startup or retain the newly adopted live catalog. Cache failures do not prevent model updates.
Invalid response Preserve the previous live and persisted catalog.
Unchanged response Advance in-memory freshness without rewriting the cache, renormalizing, or publishing; forced refresh still bypasses this shortcut.
Explicit models.file Remain authoritative; refresh rereads the file without HTTP or implicit-cache access.
Cancelled reader Shared initialization continues in the service scope, so later reads and refreshes remain usable.

Cache Ownership

ModelsDev owns the live catalog, digest, refresh timing, and single-flight coordination. The catalog-specific ModelsDevCache interface only reads and writes raw responses. Its local adapter uses atomic temporary-file replacement under Global.cache/models-dev/<source-hash>.json, with file mtime supplying startup freshness; workerd explicitly installs a dependency-free disabled adapter.

sequenceDiagram
    participant ModelsDev
    participant Upstream as Catalog Endpoint
    participant Consumers
    participant Cache as Optional Local Cache
    ModelsDev->>Upstream: Fetch /api.json
    Upstream-->>ModelsDev: Raw catalog
    ModelsDev->>ModelsDev: Validate, normalize, adopt
    ModelsDev-->>Consumers: Refreshed
    opt Local cache enabled
        ModelsDev->>Cache: Atomic best-effort write
    end
Loading

Scope

No general object-store abstraction or new infrastructure. workerd restarts use the bundle and refetch; existing KV catalog entries are ignored, not migrated or deleted. The first native startup after upgrading falls back to the bundle and refreshes rather than importing the old KV cache.

Verification

cd packages/core
bun run test
bun run test test/models.test.ts test/models-cache.test.ts test/plugin/models-dev.test.ts test/catalog.test.ts
bun run test test/pty/pty-session.test.ts
bun typecheck

cd ../server
bun run ../core/script/test.ts
bun run ../core/script/test.ts test/workerd.test.ts
bun typecheck
bun run probe:workerd

cd ../sdk
bun run ../core/script/test.ts
  • Full Core suite: 2,328 passed, 16 skipped, one PTY event timeout (retains exited sessions until removed). All 7 tests in that PTY file pass on isolated rerun; the earlier full run passed without that timeout.
  • Focused catalog/cache/plugin tests: 53 passed, including failed persistence, pending-write notification delivery, cancelled readers, startup concurrency, source isolation, corrupt data, and explicit files.
  • Workerd tests: 2 passed. The bundle probe passes without static Bun builtin imports.
  • SDK suite: 24 passed using the isolated test harness.
  • Server suite: 33 passed, 1 skipped, 3 existing OAuth callback failures. All three reproduce unchanged on base 018b4c40f3 in a separate clean worktree: two missing /cancel requests and the both-ports-busy response returning 200 instead of 400.
  • Core and Server typechecks, formatting, and diff checks pass. Targeted lint reports no errors and only existing warnings in model normalization code.
  • Repository pre-push typechecks: 32 tasks passed.

The Server and SDK suites use Core's existing isolated-home runner because their plain test scripts otherwise load local user configuration.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant