Repository navigation
feat(models): let models.json add models safely without a release - #86
Merged
Merged
Conversation
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
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.
What and why
A model should reach released apps by merging a change to
apps/desktop/resources/models.json, with no app release.The bug this fixes. Released apps never read the catalog on
main. A packaged app shipsmodels.jsonnext toapp.asar, andcontentDir()points there.getCatalogtreated that copy as a development override, so it never fetched the remote file. Now the override applies in development only (catalogOverrideDir). A packaged app fetchesmain's file and keeps its own copy as the fallback.Safety rails (
shared/modelCatalog.ts):min_app_version: a model, bundle (either list) or adapter can set this to hide itself from older apps. A bundle with a hidden part is hidden too. A prerelease counts as its release, so a nightly of 2.1.0 meets2.1.0, unless the minimum names a prerelease itself.catalogProblemschecks a remote file before the app uses it:main;min_app_versionis valid semver;schema_versionis at mostCATALOG_SCHEMA_VERSION(3).<data root>/cache/models.json(source: 'cache', already in theCatalogtype). A model added after a release stays listed offline. A saved copy older than the packaged catalog (byupdated_at) loses to it.test/shared/modelCatalog.test.tschecks the shippedmodels.jsonwith the same rules, so every PR that edits it is checked..github/workflows/catalog.ymlpurges jsDelivr's copy after a push tomainthat changes the file. It hascontents: read, a 30 s timeout per attempt and 5 retries.Docs:
docs/how-to/add-a-model.md, added to thedocs/README.mdindex.docs/reference/models.md§1: where the catalog comes from, and the two rules. Itslast-verifiedis not bumped, because I compared only §1.How it was verified
pnpm lint(0 errors, the same 12 warnings as main),pnpm format:check,pnpm typecheck,pnpm test:unit(3566 passed)pnpm test:e2e: not run, because there is no UI changepnpm test:install: not run, because no launcher, install, update or release paths changeddocs/how-to/add-a-model.md(new),docs/reference/models.md§1,docs/README.mdNew tests:
test/shared/modelCatalog.test.ts:meetsMinimumVersiontable;entriesForAppdrops a bundle with a hidden part.test/main/models/models.catalog.test.ts:min_app_versionhides entries and their bundles on a nightly.test/main/lib/content.test.ts: a packaged app has no catalog override.Platforms affected
Notes for the reviewer
models.jsonfrom GitHub or jsDelivr on catalog refresh, including at startup (8 s timeout, then the saved or packaged copy). Before this, only the network probe touched those hosts.min_app_versionsays nothing about the engine. A model the engine cannot load still needs an engine release first, as the how-to says."throttled":truein jsDelivr's response as a failed purge. I could not confirm that field against jsDelivr's docs. If the field is absent, the step just succeeds after one call.