Studio: send with no model loaded only auto-loads on-device models, never downloads the hard-coded default#7379
Studio: send with no model loaded only auto-loads on-device models, never downloads the hard-coded default#7379shimmyshimmer wants to merge 6 commits into
Conversation
…ard-coded default Fixes #7374. Pressing Send with no model loaded could miss local models (models dir, LM Studio, custom scan folders, inactive HF caches) and then download unsloth/Qwen3.5-4B-MTP-GGUF from Hugging Face without consent. - autoLoadSmallestModel is now autoLoadOnDeviceModel: adopts the server active model, then the remembered on-device model, then the smallest complete chat-capable on-device model (GGUF first), across managed HF caches and the unified backend local inventory. - Inventory failures fail closed instead of reading as an empty cache. - The remote Qwen fallback is removed from the send path; with no valid candidate the user is asked to select or explicitly download a model. - last-local-model-load records loadId/inventoryId/source with backward compatible v1 parsing; indexed local loads are remembered while native path lease picks stay excluded. - New source-contract tests in tests/studio/test_model_picker_contracts.py.
for more information, see https://pre-commit.ci
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939ea605ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [row.load_id, row.id, row.path, row.model_id].some( | ||
| (value) => !!value && targets.has(value.toLowerCase()), | ||
| ); |
There was a problem hiding this comment.
When a scanned directory produces multiple LocalModelInfo rows that share the same path/load id (for example a folder containing both GGUF and safetensors/adapter weights), this predicate matches only by identifiers and ignores remembered.kind. Interactive local loads currently do not persist an inventoryId, so a remembered non-GGUF load can resolve to the GGUF row (or vice versa), making send-with-no-model attempt the wrong backend/variant or skip the actual last-used model. Include the remembered kind/model format in this match before accepting a row.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. A folder with both GGUF and safetensors weights does produce two rows sharing the load target, and interactive local loads do not persist an inventoryId. matchesRememberedLocalRow now requires the row format to agree with the remembered kind before identifier matching, covered by test_remembered_local_row_match_requires_kind_agreement and a dual-format simulation. Fixed in c4f182a.
| const cascadeLocalRows = localRows.filter( | ||
| (row) => row.capabilities?.requires_variant !== true, | ||
| ); |
There was a problem hiding this comment.
When the only on-device models are local GGUF directories from LM Studio/custom/models_dir and there is no remembered model yet, the backend marks those directory rows as requires_variant (see studio/backend/hub/services/models/common.py:528), so this filter removes them before the fallback can list/select a quant. Pressing Send then reports no model loaded even though picker-visible on-device models exist; handle these rows like cached GGUF repos by listing variants, or only skip rows that truly cannot be resolved automatically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against common.py: requires_variant is set for every directory-based GGUF row, so the previous filter dropped all LM Studio and models-dir folders from the fallback. The cascade now resolves the smallest complete quant through the same gguf-variants call the picker card uses (repo id or local path plus local_path), and only skips rows with no resolvable quant. Non-GGUF variant-requiring rows stay excluded since they have no background resolution path. Fixed in c4f182a.
| ...cascadeLocalRows | ||
| .filter((row) => row.model_format !== "gguf") | ||
| .map((row) => ({ |
There was a problem hiding this comment.
For users with a local LoRA/adapter folder in models_dir, LM Studio, or a custom scan folder, the backend advertises adapters as can_chat (studio/backend/hub/services/models/common.py:129), so this non-GGUF fallback can pick an adapter as the smallest on-device row. Loading that adapter path causes the inference loader to treat it as a LoRA adapter and resolve/load its base model; if the base is only a Hugging Face id, pressing Send can still initiate the kind of implicit remote fetch this change is meant to avoid. Filter model_format === "adapter" out of background auto-load unless the base is known to be local/already cached.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: adapters are advertised can_chat and the load route auto-detects is_lora from adapter_config.json, so an uncached Hub base could be fetched implicitly. model_format adapter rows are now excluded from background auto-load entirely; adapters remain interactive-only. Fixed in c4f182a.
Allows behavioral simulations to drive the real auto-load implementation directly instead of asserting on source text only. No runtime change.
…ip adapters Review follow-ups, all verified against the backend inventory services: - Directory-based local GGUFs (LM Studio, models dir, custom folders) are flagged requires_variant by the backend, so the fallback now resolves the smallest complete quant through the same variants API the picker card uses instead of dropping every directory row. - A folder holding both GGUF and safetensors weights yields two rows with the same load target; remembered-model matching now requires the row format to agree with the remembered kind. - Adapter rows are chat-capable but load by resolving their base model, which for a Hub-id base would start an implicit remote fetch; adapters are excluded from background auto-load. Contract tests extended to pin all three behaviors.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db7855538a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| matchesRememberedLocalRow(candidateRow, lastLoaded), | ||
| ); | ||
| if (row) { | ||
| markSeen(row.load_id, row.id, row.path, row.model_id); |
There was a problem hiding this comment.
When the last-used model is a local GGUF directory, this marks its row as seen before the remembered quant is resolved or loaded. If that remembered quant was deleted/partial or the load fails but another complete quant remains in the same LM Studio/models_dir/custom folder, the fallback loop later hits isSeen(...) and skips the row, so Send reports no model instead of choosing the smallest remaining quant. Only mark the row as deduped after a successful load, or don't use this seen set for the remembered-row retry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, that markSeen ran before the remembered quant was resolved or loaded, so a folder with another complete quant could never fall back. The remembered-local retry no longer pre-marks the row; only the exact failed candidate key is excluded, mirroring the managed-cache remembered path. Verified with a simulation where the remembered Q8_0 fails to load and the same folder's Q4_K_M then loads. Fixed in bf46d8b.
| if (repo.partial) return false; | ||
| if (repo.capabilities?.can_chat === false) return false; | ||
| return !isHiddenModelId(repo.repo_id); |
There was a problem hiding this comment.
When a LoRA adapter repo is present in the Hugging Face cache, the cached-models inventory classifies it as model_format: "adapter" and leaves it chat-capable, but this filter only rejects partial or explicitly non-chat cached rows. The fresh evidence is that the fallback now builds modelRepos directly from /api/hub/cached-models, so the smallest cached adapter can still be selected; /api/inference/load then auto-detects adapter_config.json and resolves the base model, which can trigger the same implicit Hub fetch that local adapter filtering was meant to prevent. Exclude cached model_format === "adapter" rows here too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed: the cached inventory classifies adapter repos with the same capabilities mapping, so they are chat-capable there too. isAutoLoadableCachedRepo now rejects model_format adapter, matching the local-row exclusion, with a simulation asserting a cached adapter never reaches validate or load. Fixed in bf46d8b.
| const variants = await listGgufVariants(row.model_id || row.id, undefined, { | ||
| preferLocalCache: true, | ||
| localPath: row.path, |
There was a problem hiding this comment.
When an LM Studio/models_dir/custom GGUF folder has a model_id and that repo is also present in the HF cache, this resolves variants using the repo id even though the candidate will be loaded from row.path. The backend's prefer_local_cache flow checks the HF cache before the supplied localPath, unlike the picker which passes the local path as the repo id, so auto-load can choose a cached quant that is not in the local folder and then fail /load with row.path + quant despite other local quants being available. Resolve local rows from row.id/row.path or otherwise force the path scan first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against the variants service: prefer_local_cache checks the HF cache before the supplied local_path, so a row with a Hub model_id could resolve a quant absent from its own folder. Resolution now uses the row's local path as the repo id, which routes the backend straight to the filesystem scan of the folder the candidate loads from. Covered by a simulation where the folder and cache differ. Fixed in bf46d8b.
…apters Second round of review follow-ups, verified against the backend services: - A failed remembered local quant now excludes only that exact candidate key instead of marking the whole row seen, so another complete quant in the same folder can still load (mirrors the managed-cache remembered path). - Quant resolution for a local GGUF folder scans the folder itself via a local-path repo id; the cache-first prefer_local_cache flow could return a cache quant missing from the folder when the row also has a Hub model_id. - Cached adapter repos are chat-capable in the cached inventory and resolve a base model on load, so they are excluded from background auto-load like local adapter rows. Contract tests extended for all three.
|
CI triage for the failures seen on the previous run: all five are pre-existing on main and unrelated to this change.
Everything exercised by this PR passes: Frontend build + bundle sanity, Source lint, pre-commit.ci, both other Chat UI shards, and the studio contract tests (48 passing, including the new autoload guards). |
1 participant
Summary
Fixes #7374.
Pressing Send in a new chat with no model loaded could report "No downloaded models found" and start downloading
unsloth/Qwen3.5-4B-MTP-GGUF(UD-Q4_K_XL) from Hugging Face, even when the user had a perfectly loadable local model on disk. Two underlying problems:listCachedGguf/listCachedModels), so models in the models dir, LM Studio dirs, custom scan folders, and direct local GGUFs were invisible to it. Both requests were also wrapped in.catch(() => []), so an inventory error read as "nothing on device" and fell straight through to the remote download.Changes
studio/frontend/src/features/chat/api/chat-adapter.tsautoLoadSmallestModel()is renamed toautoLoadOnDeviceModel()and now resolves candidates in this order: model already active on the inference server (unchangedtryAdoptServerActiveModel), then the last successfully loaded on-device model, then the smallest complete chat-capable on-device model (GGUF group first, then safetensors), merging managed-cache rows with the backend indexed local inventory (/api/hub/local), the same non-React inventory API the unified picker uses.models_dir/lmstudio/custom,capabilities.can_chattrue, not partial, not hidden infra, no variant requirement, big-endian GGUFs excluded. Rows load via the backendload_idtarget and record theirinventory_id.MAX_AUTO_LOAD_ATTEMPTS, per-model GPU/context/template/speculative settings, and theloadResp.model || loadIdruntime identity handling for inactive-cache rows.studio/frontend/src/features/chat/utils/last-local-model-load.tsloadId,inventoryId, andsourcealongside the existing fields. Parsing stays on theunsloth.last-local-model-load.v1key and is backward compatible: legacy records default to the managed-cache source and keep working.ggufVariant: nullbecause its load target identifies the file.studio/frontend/src/features/chat/hooks/use-chat-model-runtime.tslocal) are now remembered. Native file-picker selections remain excluded (their access depends on a signed expiring path lease), and other arbitrary paths keep theisLocalModelPath()protection.studio/frontend/src/features/chat/types/api.tsGgufVariantDetailgains the optionalpartialflag the backend already sends, so partial variants are skipped during auto-load.tests/studio/test_model_picker_contracts.pyTesting
cd studio/frontend && npm run typecheckpasses.npx biome checkon the four changed frontend files: no new diagnostics; pre-existing chat-file diagnostics are reduced (the removed download block carried two of them).pytest tests/studio/test_model_picker_contracts.py -q: 45 passed (34 existing plus 11 new).npm run buildsucceeds and the rebuilt Studio serves normally.Behavioral simulations
Beyond the source-contract tests, the fix was exercised end to end in an isolated sandbox (esbuild bundles of the real frontend modules, run under Node with a mocked backend and a controllable localStorage, one fresh process per scenario). The same harness was also built against the pre-PR source (a worktree of main) to prove the reported bug reproduces there.
Pre-PR bug reproduction (all reproduce on main, all impossible after this PR):
unsloth/Qwen3.5-4B-MTP-GGUFis issued.Post-PR scenarios (40 scenarios plus 14 persistence unit cases, all passing):
load_idand exactcache_path, models dir, LM Studio, and custom scan folders, including a local GGUF with a null variant.can_chatfalse,requires_variantrows, big-endian GGUFs, ollama link rows, and hf_cache-source local rows never auto-load.MAX_AUTO_LOAD_ATTEMPTScap, trust and security gated candidates cascade without loading and without any download.Browser and platform compatibility: the changed modules compile cleanly for chrome90, firefox90, safari14, and edge90 esbuild targets, and the full vite production build passes against the project browserslist. The frontend never touches the filesystem directly, so Linux, macOS, and Windows differences reduce to backend-provided path strings, which the path-shape scenarios cover. The Python contract suite was additionally run inside a fresh uv virtualenv: 45 passed.
The
autoLoadOnDeviceModelexport exists so these simulations can drive the real implementation; it has no runtime effect.