Repository navigation
Refactor internal asset loading and add targeted cache versioning - #1793
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| it("does not accept an invalidated entry when remote access is disabled", async () => { | ||
| env.allowLocalModels = true; | ||
| env.allowRemoteModels = false; | ||
| cache.entries.set(`${REMOTE}tokenizer_config.json`, jsonResponse({ stale: true })); | ||
| await expect(getModelJSON(MODEL, "tokenizer_config.json")).rejects.toThrow("file was not found locally"); | ||
| expect(env.fetch).not.toHaveBeenCalled(); | ||
| expect(cache.delete).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
an offline app that can't fetch the tokenizer after upgrade will no longer work anymore. is this what we want?
There was a problem hiding this comment.
Yes, that can happen if the app updates while online but only loads the model after going offline. I think that's an acceptable tradeoff for invalidating stale assets. Apps that need guaranteed offline support should check/download the required files before switching versions or they even have some kind of model overview that shows whats (fully) cached. ModelRegistry provides the tools for that.
There was a problem hiding this comment.
If you think this we should catch this we could add an "offline fallback" where we would fetch the un-keyed version of the file if the request fails. But that would add complexity and I still think thats super rare and responsobility of the application.
There was a problem hiding this comment.
I changed my mind on this. Offline apps shouldn't break on upgrade, so I added the fallback: if the refreshed copy can't be fetched (network error, local_files_only, or allowRemoteModels=false), we serve the previously cached copy. It isn't re-stored or deleted, so the next online load replaces it. Without any cached copy it still fails as before.
| await fileCache.put(`${MODEL}/tokenizer_config.json`, jsonResponse({ stale: true })); | ||
| await fileCache.put(`${MODEL}/onnx/model.onnx`, new Response("weights")); | ||
| expect(await getModelJSON(MODEL, "tokenizer_config.json")).toEqual({ updated: true }); | ||
| expect(await fs.readFile(path.join(directory, "transformersjs_assets_v2", MODEL, "tokenizer_config.json"), "utf8")).toBe('{"updated":true}'); |
There was a problem hiding this comment.
i'm really not a big fan of having a folder which literally is named "transformersjs_assets_v2" -- when the folder is really meant to be a mirror of the repo.
seems quite hacky.
There was a problem hiding this comment.
also, this means custom caches will need to support new keys/structure... right?
There was a problem hiding this comment.
I don't think the filesystem cache necessarily needs to mirror the repo. Since this is meant to be a cache, the disk structure is imo an implementation detail.
But I do agree that custom caches shouldn't need to understand that structure.
I reworked this so versions are query parameters on cache keys, not download URLs. Custom caches still just use match/put with opaque URL keys, while FileCache handles disk mapping internally.
I know this changes how we’ve handled caching, but we need a way to invalidate stale configs: otherwise updated configs with newly required fields never reach users because persistent cache hits bypass fetching indefinitely. This seems the simplest compromise: no new cache interface or separate metadata store, and model weights and unversioned assets stay untouched.
There was a problem hiding this comment.
I reworked this: the transformersjs_assets_v2/ folder is gone. FileCache now stores versioned entries at the normal repo path and keeps the version in a small marker file next to it (no marker = unversioned). So the cache mirrors the repo again, and a pre-filled cache folder works as localModelPath. Custom caches still only see opaque match/put keys. The only change for them is that tokenizer_config.json gets a new key once, so it is re-downloaded once. Does the marker-file approach work for you?
This pulls resource resolution, fetching, and cache lookup into a shared internal
AssetLoadingContext. Public loading options and the existing cache interface stay the same.It gives us a common foundation for two ongoing discussions:
Loading, metadata checks, cache-status checks, and removal now use the same cache-key rules. Older entries are cleaned up after a successful refresh when the cache supports deletion.