mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-17 04:43:52 +00:00
3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
6b0035b8e6
|
studio: apply saved per-model settings on API loads, add API monitor (#7473)
* studio: apply saved per-model settings on API loads, add API monitor Per-model settings were mirrored to the server as two fields only, llama_extra_args and max_seq_length. Everything else lived in browser localStorage, so a model loaded by an API request came up with app defaults for context length, KV cache dtype, speculative decoding, tensor parallel and GPU placement. Store the full config server side and map it onto the same LoadRequest the picker builds, so a remote load and a picker load of the same model produce the same command line. Entries are keyed per quant, falling back to the bare repo id so existing entries keep resolving. Also moves the API monitor out of the settings tab: a full page at /api-monitor, plus a floating panel that opens itself when traffic arrives, and a settings page per model reachable from the Hub. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: harden per-model settings against review findings and fuzzing Review findings (PR #7473): - Look up overrides under the concrete load path with its quant, not just the advertised repo id, so local folders and non-active HF caches are found. - Carry bare-repo launch flags into the first per-quant save. Auto-switch prefers the qualified entry, so without this the flags were silently dropped and no UI could show or restore them. The bare id is only derived when the suffix looks like a quant, so a Windows drive letter is not split. - Drop a saved gpu_ids pin that no longer resolves instead of 400ing the whole load. A pin outlives the machine it was made on. - Build the displayed API base from getApiBase() on desktop; the Tauri webview origin is not the API server. - Keep a partial download's isDownloaded when opening settings, so the loader still reports download progress. - Only prefer the loaded quant when the loaded model is this row. Q4_K_M exists in most repos, so an unguarded match targeted the wrong variant. Found by simulation: - A lone surrogate in a chat template raised UnicodeEncodeError on the byte check, an unhandled 500. Now a validation error, in all three call sites. - _bounded_int accepted bools as GPU ids, truncated fractional floats, and raised OverflowError on Infinity, which json.loads accepts. - api_monitor stored a non-string model verbatim; the monitor page then threw on toLowerCase and rendered nothing. Coerced at the boundary and the filter no longer trusts network data. - The overlay store now uses storage that cannot throw. Safari private mode and blocked-cookie origins make localStorage throw on access, which broke the opt-out toggle. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: address second review round on per-model settings - Probe actual Vulkan devices in the GPU reconciliation helper. Vulkan ordinals are their own index space, so resolve_requested_gpu_ids only rejects malformed ones; the helper said "usable" and the load then 400d on the ggml probe it had skipped. - Send explicit save/remove intent. A save whose config is entirely default carries no fields, which is shape-identical to "forget this model", so it was wiping launch flags the UI cannot show or restore. A bare model_id without the flag still removes, keeping the original contract. - Backfill existing per-model settings into the server map once after upgrade. Without it, settings saved before this change never reached the server, so an API load used app defaults while the UI still showed the model as remembered. Never overwrites a server entry and retries until it fully succeeds. - Stop polling the monitor endpoint when auto-open is off and the panel is closed. It cannot open or display anything in that state, so every open Studio window was polling every five seconds for nothing. * studio: address third review round on per-model settings - Fall back to a case-insensitive override lookup. The browser normalizes ids to lowercase before storing, so the backfill wrote "org/model:q4_k_m" while the resolver asked for "Org/Model:Q4_K_M" and never matched, which made the whole migration a no-op. Exact match still wins and an ambiguous fallback matches nothing, so two POSIX paths differing only in case stay distinct. - Record a detail revision only when a fetch actually started. requestDetail's in-flight guard can refuse, and recording anyway meant a revision that landed during an earlier fetch was skipped for good once updated_at stopped moving, leaving a terminal request showing a stale running payload. - Discard superseded settings opens. The GGUF variant lookup is async, so opening a second row while the first was pending let whichever finished last win, and the page could then save or load settings for the wrong model. - Raise the model_id cap to PATH_MAX plus a quant suffix. A local model's id is its filesystem path and LoadRequest.model_path is unbounded, so the old 512 limit 422d the server sync while the local save succeeded, leaving the UI showing settings the API would never apply. * studio: only open the API monitor for real API clients CI caught this: the Chat UI Playwright run failed because the floating panel opened during an ordinary chat turn and its Expand button covered the composer's Send button. The panel opened because Studio's own chat goes through the same tracked endpoints as the OpenAI-compatible API, so any conversation looked like API traffic. That is wrong regardless of the click interception: this panel exists for when Unsloth is being used as an API server, not when someone is using Unsloth. Record on each monitor entry whether the caller authenticated with an sk-unsloth key rather than a UI session, and auto-open only for those. The discriminator already existed for other routes; this reuses it. * studio: address fourth review round on per-model settings - Never case-fold a filesystem path when looking up an override. Two POSIX paths differing only in case are two different files, so a near miss must load defaults rather than replay another model's context and GPU pin. Repo ids still fold, which is what the migration needs. - Match a quant suffix against the loader's own quant pattern instead of a length heuristic. "/models/foo:bar.gguf" is one valid POSIX filename, and splitting it grafted /models/foo's launch flags onto an unrelated model. - Retry a load once without the saved gpu_ids when the loader rejects the pin. The pre-flight check cannot mirror every rule the loader applies (a Vulkan diffusion GGUF refuses GPU selection outright, and the rules move), so this stops chasing them one at a time: a stale placement preference must never be the reason a request cannot be served. - Make an explicit remove win over config fields sent in the same payload. - Seed only finished requests on the monitor's first snapshot. A request still running when Studio loads is traffic the user has not seen, not history. - Nudge the detail effect when the in-flight guard refuses a fetch. Nothing else changes its deps when the older fetch settles, so a terminal reply could stay truncated forever. - Invalidate pending row lookups when opening settings from a detail card, not just from a row. - Mark the covered detail pane inert so it leaves the focus order. - Refuse to open settings for a variant-required GGUF whose quant could not be resolved: the picker matches variants exactly and would never find the saved config, while the API falls back to the bare key and would apply it. - Mirror to the server only for GGUFs. The auto-switch resolver indexes GGUFs, so a safetensors config was being advertised as applied on API load when no API request could ever apply it. * studio: clear server overrides for models evicted from local storage Saving a config can push the browser's map over its entry or byte budget, at which point older models are silently dropped and the save still reports success. Their server-side overrides survived, so API loads kept applying settings that nothing in the UI showed any more and nothing could forget. savePerModelConfig now reports what it evicted, and the caller clears those server entries alongside the one it saved. Removal also had to resolve the way lookup does. Storage keys are normalized, so an evicted entry comes back lowercased while the server may hold the repo's real casing; deleting only the literal key would leave the entry a load still resolves to. Read and remove now share resolve_model_override_key, so what a load applies and what forgetting clears cannot disagree. Path ids still match exactly, so one file is never forgotten by clearing its case-variant neighbour. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: pin the GGUF mirror gate and eviction cleanup with contract tests Both were flagged in review with no test holding them in place. Also corrects the gpu_ids preflight docstring: it claims to mirror every rule the loader applies, which it deliberately does not, and the retry below is what covers the rest. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve the override key on save and match standalone gguf settings The remove branch already resolved the key a load would use, but the save branch wrote payload.model_id literally. The browser normalizes casing before storing, so a backfilled key and a later UI save left two entries for one model; with two equivalent keys present resolve_model_override_key finds no unique match, so any third casing resolved to no override at all and the model silently loaded with defaults. A standalone .gguf gets variants=() from the resolver, so variant is None and only the bare ids were tried. The picker keys the same file by the quant label it derives from the filename, which is never empty, so those settings lived under <path>:LABEL and nothing reached them. Try the filename-derived key after the variant-qualified ones and before the bare ones, so older bare entries still work. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Match server overrides by identity during the one-time backfill app_settings carries no schema version, so an install that predates identity normalization holds rows keyed by whatever id was typed, such as Unsloth/Repo-GGUF:Q4_K_M, while this browser only ever stores the folded form. The exact property lookup therefore reported "not on the server" for a row that is, and the backfill PUT over it, replacing server settings the file documents as the newer authority. The migration runs once on every existing profile, so this lands on exactly the upgrades it was written to protect. Fold both sides before comparing, splitting on the last colon because a quant label never contains one. A repo id and a Windows path fold, a POSIX path does not, which is the same rule the backend resolves by. Verified with the real module under node: the legacy-casing, variant-casing and Windows-path cases go from overwriting to skipping, a second run stays clean, and a genuinely new model, a different quant of the same repo, a POSIX path differing only in case, and a bare legacy key all still migrate. * Fold case-insensitive path ids the way the browser already does resolve_model_override_key refused the case fallback for every filesystem path, but only a POSIX path is case-sensitive. A Windows drive path, a UNC share and a WSL drive path each name one file whatever the casing, and the browser folds exactly those three before storing. A Windows user's migrated entry was therefore keyed lowercase while an API auto-switch resolved the same file with its on-disk casing, so the lookup missed and the saved launch flags silently stopped applying until the settings were saved again. Fold those three shapes here too, normalizing the separator as the browser does so C:/Models/Foo.gguf and c:\models\foo.gguf agree. POSIX stays case-sensitive, /mnt/data stays an ordinary mount rather than a WSL drive, and an ambiguous fold still matches nothing so a load takes defaults instead of guessing. The existing Windows test asserted the opposite. It carried no rationale, unlike its POSIX sibling, and get_model_override's docstring already scopes the rule to POSIX, so it read as an over-generalisation of the POSIX case. * Close clearApiMonitor The merge that brought main into this branch dropped the closing brace, so the function body ran straight into the interface declared below it and the frontend did not compile at all: tsc reports TS1005 at the end of the file and vite fails the build. Reproduced against the pushed head and clean with the brace restored. * Carry legacy flags across a scanner-derived GGUF variant label A .gguf whose filename holds no recognizable quant token still gets a label from the scanner, which falls back to the filename stem, so the UI stores keys like "/models/custom.gguf:custom". _bare_model_id accepted only known quant tokens, so the first per-quant save did not carry over llama_extra_args stored under the bare id, and auto-switch prefers the qualified entry, so those flags were silently dropped with no UI able to restore them. Accept a suffix that is exactly the label the scanner derives for that filename. Requiring the head to be a .gguf and the suffix to match exactly is what keeps an arbitrary colon-containing POSIX path out: "/models/foo:bar.gguf" splits to a head that is not a .gguf. The filename is taken by splitting on both separators, since a "C:\\..." key is written on Windows but may be read back by a backend that is not. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep model lifecycle rows out of the request statistics A load, unload or download is recorded in the monitor but is not an HTTP call. It reads as running for as long as the load takes, so it was counted as an in-flight request with no client waiting, and a multi-minute download was folded into Avg latency and the error rate. The backend already excludes these rows from active_count for the same reason, so the page was also disagreeing with the number the API itself reports. Requests counted them too, so that is now the non-lifecycle count rather than the raw entry count. Also limit the API-reach sentence on the Hub settings page to GGUF models. The Hub opens that page for every downloaded model, but ModelConfigPage mirrors settings to the server only when target.isGguf, because auto-switch indexes GGUFs only, so a safetensors user was told the settings apply to an API request that cannot reach them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Accept bpw variants, fold POSIX quant suffixes, migrate bare GGUF configs Three gaps the previous round left, all in the same key-resolution rule, so the rule now lives in one place as split_quant_suffix. A quant label may carry a bits-per-weight modifier, because two files at the same base quant are kept distinct by it: utils/models/model_config.py preserves IQ4_XS-3.53bpw while hub/utils/gguf.py strips it, and both forms reach the override keys. The known-quant pattern accepted neither, so _bare_model_id missed the bare entry and the first qualified save dropped its launch flags. On POSIX the browser lowercases the quant but keeps the path casing, so a migrated "/models/Foo:q4_k_m" was unreachable from the scanner's "/models/Foo:Q4_K_M". Only the quant suffix folds now, and only when it really is a quant, so "/models/foo:Bar.gguf" stays a distinct filename and the path itself stays case-sensitive. A standalone .gguf picked directly has no quant to choose between and is stored with a null variant. The backfill filter read that as safetensors and skipped it, and the done flag is set on the same pass, so those settings stayed browser-only for good while auto-switch kept loading the model with defaults. * Let remove win over flag validation, and make Clear log clear shared rows An explicit remove ran the launch-flag validation first, so a form still carrying a rejected flag raised a 400 and left the override in place. Nothing is stored on that path, so there is nothing to validate; remove now short-circuits it, which is what the branch below already claims to do. Clear log dropped only the caller's own rows, but a lifecycle row is shared: it is visible to everyone and owned by no one, so those rows survived and the reload straight after the click brought them back, leaving the button visibly ineffective. Deleting them is not an option either, since that erases another caller's history. They are now hidden per subject, so the clear is true for that caller and harmless to the rest. A shared row that is still running is live state rather than history, so it stays visible, and the hidden ids are pruned against the ring buffer so they cannot accumulate. * Move the lifecycle labels out of the lazily loaded monitor page The overlay is mounted from __root.tsx and imported two label helpers from the page, so the page and its dependency graph were pulled into the eagerly loaded bundle and the route's lazyRouteComponent bought nothing: every route paid for the monitor page even when it was never opened. Measured on a production vite build, the async api-monitor chunk was 0.20 kB, meaning the implementation had landed in the main bundle. The helpers now live in their own module. The same build gives an 18.83 kB api-monitor chunk and a main bundle 18 kB smaller (3.9 kB gzipped). * Order override writes per model Saving twice quickly, or saving while the one-time backfill is still running, started independent requests with no sequencing, so the older response could commit last and resurrect the entry the newer one meant to replace or remove. An API-driven load then applies context or GPU settings the user has already changed, with nothing in the UI showing it. Writes now chain per override key. The chain hangs off the settled tail, so a failed write cannot cancel the next one, and only the last writer clears the slot so a queue that is still building keeps its order. Different models still overlap. Verified against the real module under node: two saves for one model with the first made slow commit oldest-first and never overlap, where the previous version committed them in the wrong order; a rejected write still lets the next succeed; and two models still run concurrently. * Key write queues by identity, and reach unknown GGUF labels in either casing Two holes in the previous two commits, both found by the same review round. The per-model write queue keyed on the literal spelling, so the backfill's legacy casing and a UI save's normalized one opened two queues for one model and raced exactly as before. It now keys on the folded identity, which is what the backend resolves by. A .gguf with no recognizable quant token is labelled by its filename stem, and v2 storage lowercases that label while the scanner probes with the filename's own casing. Folding only recognized quant labels therefore left the migrated entry unreachable for precisely the files that need the stem fallback. The suffix rule now also accepts a case-insensitive match against the label the scanner derives for that filename, which keeps an ordinary colon out because the head still has to be a .gguf. _bare_model_id drops onto the same shared rule rather than repeating half of it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard the backfill, the detail settings entry point and the detail retry Three separate reports, all confirmed against head. listPerModelConfigs reported future-schema records. loadPerModelConfig refuses to apply one and eviction refuses to drop one, because this client cannot interpret that schema, so handing it to the backfill would persist a partial reading of it server-side and let an API-triggered load apply settings the same client will not apply locally. It is skipped there now, matching the other two paths. The detail view's on-device card passes a null variant while its own lookup is pending or after it failed, and this entry point opened the editor anyway. That saves a bare-model config, which the picker never finds because it matches variants exactly, while the API's bare-key fallback would apply it. openModelSettings already refuses with a toast for exactly that reason; this path now refuses the same way. A failed detail fetch was never retried. The revision is recorded when the fetch starts, and on failure the entry stays missing, so selectedIsMissing does not change and a terminal row's updated_at does not advance: nothing was left to re-run the effect, and the full prompt and reply stayed unavailable until another row was selected. The in-flight flag settling is the trigger now, and the attempt count bounds it, because the usual failure is an entry that has aged out of the ring buffer and will never arrive however often it is asked for. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Point the lifecycle contracts at the module the labels moved to Extracting the labels out of the monitor page left two contracts asserting they were still in it, so the staged run went red on all three platforms. They read the new module now, and the page contract additionally pins that it imports from there rather than redefining them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Re-read local state before backfilling, and stop advertising Ollama as API loadable The backfill wrote the snapshot it took before fetchModelOverrides resolved, so a save or a forget during that round trip was undone: the write is queued behind the interactive one and commits last, leaving the browser showing the new settings while an API load applied the old ones. Each write now re-reads the model's current local config and skips it if it has gone or gone back to defaults. Verified against the real module under node: the write carries maxSeqLength 9999 where it previously carried the stale 1000. target.isGguf was also standing in for "an API request can load this". It cannot for an Ollama model: local_model_resolver skips Ollama's scanner on purpose, so those models are never in the auto-switch index, yet the mirror ran and the settings page told the user the API would apply them. The target now carries apiLoadable, set from the inventory source the row already has, and both the mirror and that sentence read it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Key the Hub's per-model settings by the repo id, not by the load path A repo cached outside the active HF cache reports load_id as its snapshot path (cache_inventory), which is what the loader needs, but the chat picker and the auto-switch index both name that repo by repo_id. The new Hub settings page was saving under the load id, so the settings landed on a key no other load reads: the picker, an auto-load and an OpenAI-compatible request all fell back to defaults, and a server override already stored under the repo id could win against the save. ModelPickTarget now carries configId for the case where the storage identity is not the loadable one. Every read, write and server mirror in ModelConfigPage uses it; the chat template and GGUF header probes keep target.id, since they have to open the model. The Hub sets it for cache rows and resolves its own load through the same helper, so a config saved from the settings page is the one a later load finds. Rows whose load id is already their identity, which is every local row and every repo in the active cache, are unaffected. Verified against the real per-model-config module under node: saved under the snapshot path, a picker read reports remembered=false and the default max sequence length; saved under the repo id it reports remembered=true and 8192. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments across the API monitor and per-model settings work Pass over every comment this branch touches. Collapse the multi-paragraph rationales to the point they were making, drop prop docs that only restated the prop name, and reflow the rest onto fewer lines. No code changes. * Recognise an absolute path id in either platform spelling _is_abs_path_id decides whether an id is a host path that must not be published through /v1/models, and it asked pathlib.Path, which follows the running OS. A Windows backend therefore read "/home/me/x.gguf" as a relative name and a POSIX one read "C:\models\x.gguf" the same way, and in both cases the path was advertised verbatim as a model id. Ids outlive the machine that wrote them: settings sync, a WSL session and a copied config all carry the other platform's spelling, which is why the model-override identity in this PR already folds Windows drive, UNC and WSL paths. The backend now agrees with it, reading the value as both a POSIX and a Windows path. Neither reading can misfire on a repo id, which has no leading separator, drive letter or UNC prefix. The two tests this fixes on Windows are older than this PR; the new one pins the contract in both directions. * Give the idle-unload test a wall-clock deadline The drive loop waited a fixed 200 iterations of a 10 ms sleep. Windows rounds that sleep up to the roughly 15.6 ms scheduler tick, on this loop and on the idle loop under test alike, so the unload got far less real time there than the count suggests and the test failed on the Windows runner for being slow rather than wrong. It now waits on time.monotonic with a generous ceiling and still breaks as soon as the KV file is gone, so the fast path costs nothing. The test is older than this PR; the deadline is the only change to it. * Split a quant suffix the way the backend does, and gate the detail card too The backfill took a key's identity by splitting on its last colon, so anything else that ends in one was read as a quant separator. A Windows path made "C:\models\foo.gguf" into model "C" with variant "\models\foo.gguf", and an ordinary colon inside a POSIX filename folded "/models/foo:Bar.gguf" and "/models/foo:bar.gguf" onto one key, so whichever of the two was already on the server made the other look migrated and left its API loads on defaults. splitQuantSuffix now mirrors split_quant_suffix on the backend: the suffix has to be a known quant label, with or without a bits-per-weight modifier, or the head has to be a .gguf carrying a stem label. Checked against the backend over twelve keys, including every case above, with identical answers on both sides. Settings also opens from the on-device detail card, and that constructor never set apiLoadable, so an Ollama model reached the server mirror and the "API loads use these settings" line from that entry point even though the auto-switch resolver skips Ollama's scanner. It now reads the same source the row menu does. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Judge the config storage actually keeps, not the one on screen savePerModelConfig normalizes before deciding whether a config is default, and the runtime hands the settings page Speculative Decoding "auto", which canonicalizes to null. The page judged the raw object instead, so a model sitting at defaults looked non-default: turning on "Remember for this model" reported saved while the local write had dropped the entry, reopening showed it as not remembered, and the mirror sent the server a speculative_type "auto" override the browser did not have. That disagreement between the two is the one thing the mirror is written to avoid. The page now normalizes once and uses that object for the default check, the local write and the server mirror alike. Driving the real module under node: the raw object reads as non-default, storage stores nothing, and only the normalized reading agrees with storage. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Shorten the comments on the newest round of changes Comment-only pass over the quant-suffix split, the storage-shape normalization and the wall-clock deadline in the idle-unload test. Same points, fewer lines. * Key one config per model, and stop the backfill replacing newer server state Five follow-ups on the per-model settings map. The one-time localStorage backfill read the override map once and then wrote each model in turn, so a save by another tab during that pass was replaced by this browser's older copy, against the migration's own "never overwrites" contract. Re-fetching per model would cost a round trip each; instead the PUT takes only_if_absent and the server tests and writes under one transaction. gpu_ids arrived unbounded and normalize_model_override de-duplicated it by scanning the list it was building, so a large authenticated array cost roughly 20x what the same work costs with a set (4.5s against 0.27s for a million entries). The payload now bounds the field to the number of ids the normalizer can store, and the dedupe uses a set. A settings target opened from the Chat model picker carried no apiLoadable, so the isGguf fallback mirrored an Ollama GGUF to the server. Ollama's blobs reach that picker as custom-folder GGUFs under a .studio_links / ollama_links dir, which local_model_resolver refuses to index, so the mirror advertised a load the API can never make. The picker, the sidebar editor and the backfill now all use the same classification. The Hub settings page compared the loaded model to settingsTarget.id, but a GGUF loaded from an inactive HF cache or straight off disk loads by path while /status reports the clean public id, so the page ignored the live launch config and showed saved or default values. It now also matches the settings identity and the public id the backend would report. A standalone .gguf gets a filename-derived format_variant from the inventory, so the Hub row menu stored its settings under <path>:Q4_K_M while the Chat picker, the detail card and the backfill all used the bare path. The row menu now uses the bare path too. Tests: publicModelId / residentModelIdMatches / isOllamaLinkPath / settingsGgufVariantForRow in studio/frontend/tests, the create-only write and the gpu_ids bound in studio/backend/tests, and the wiring in tests/studio/test_model_picker_contracts.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Read the concrete model key first, and fill settings in field by field Three follow-ups on the last round. The auto-switch load tried the advertised id before the concrete load path, so an entry under the alias shadowed the settings the user had just saved and kept shadowing them. The settings page keys every local row by the path being loaded, while the alias is derived: the /v1/models name a hand-written overrides PUT is written against, and for a loose .gguf only its filename stem. Each pair now reads the path first, and the alias, the bare ids and the older filename-label key are all still read after it, so a cached repo (keyed by its repo id, which is the alias) resolves exactly as before. publicModelId collapses two paths that share a filename or a directory basename onto one id, so the resident check added last round could mark the wrong catalog row as loaded and seed its editor with another model's live launch config, then save that under this model's key. The Hub page now records the loadable identifier /status reports, as every other status reader already does, so the literal comparison names one row; the public-id pass only accepts a namespaced repo id, the one collapse that cannot name two models. The override map shipped before this browser mirror did, holding only llama_extra_args and max_seq_length, so an upgraded install can have a server entry for a model whose context, KV cache, speculative and GPU settings live only in localStorage. The backfill read key presence as done, skipped exactly those models and then marked itself complete, so their API loads lost the settings for good. Filling them in from the browser would reopen the race the conditional write just closed, so the merge is the server's: the PUT flag is now fill_absent_fields, and studio_db merges field by field under the write's own transaction, where a stored value always wins. A fill with an entry already there also stops replaying that entry's stored flags through validation, so one denylisted since it was saved cannot 400 the one-time migration. Tests: the key order and the fill in studio/backend/tests, the collapse in studio/frontend/tests, and the wiring in tests/studio/test_model_picker_contracts.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Attribute API-key traffic on the lifecycle rows it creates The overlay opens on API-key traffic only, and record_lifecycle never set that flag. Auto-switch and auto-download run before the endpoint opens its request row, so a switch or a download that is refused never reaches api_monitor.start and its lifecycle row is the whole trace of the request. The monitor therefore stayed shut on exactly the failures automatic observability is for. The row now carries the attribution: a load takes it from the request that drove it, and auto-download passes it directly, since only an API request reaches that path at all. A manual unload and an idle unload are not API traffic and stay unattributed, so neither pops the overlay. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Treat a loaded standalone GGUF as resident despite its derived quant A loose .gguf keys its settings by the bare path with no variant, since the path already names the one file. The loader still derives a label from the filename and /status reports it, so the settings page compared a derived quant against a deliberate null, never matched, and withheld the live launch config from the very file that was loaded. Applying from that page could then write the saved values over what the resident model is running with. The variant equality is skipped for that case only. Every other target, a repo row or a directory, still has to agree on the quant, because there the variant is what tells two loaded copies apart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Show the Hub the settings the model is really running with Two ways the Hub's per-model settings page could offer a resident model's saved or default values as if they were live, and then write them back over the running config on Apply. ModelConfigPage seeds its editable state from loadedConfig in a useState initializer, so it reads that prop once per mounted instance. The sidebar entry keys its instance on a signature of the live config; the Hub's keyed on the model and quant only. Open the page before /api/inference/status has hydrated, or while that same target is still loading, and loadedConfig flips from null to the live config after mount with nothing to remount on: the editor keeps the values it seeded from and Apply reloads the model with them. Both hosts now mount under one shared key that includes the live config, so the arrival of that config re-seeds the editor and a repeated poll of the same values does not. The live config itself comes out of the chat runtime store, and landing straight on /hub is the one entry point where nothing has applied the status yet: useChatModelRuntime has no mount sync and the chat page is a different route. The Hub's own status effect pinned the checkpoint and stopped there, so the resident check passed while kv cache, speculative decoding, tensor parallel and every GPU placement field still held their defaults. It now applies the whole status, the same call the chat runtime's refresh makes, and holds off when a load owns the store or an external provider is selected so it cannot fight either. * Pop the monitor open only for the traffic a caller made Lifecycle rows are shared so a load or a download shows up in everyone's monitor list, which is deliberate. Since they started carrying via_api_key they also carry the flag the floating panel auto-opens on, and that reached every authenticated subject: another logged-in browser sprang open for API traffic it had nothing to do with. The row now records the caller that drove it and reports the attribution only to them. Visibility is untouched, so the row still appears for everybody, and a subject-scoped Clear hides a shared row it owns rather than deleting it out of everyone else's history. Auto-download had the flag hardcoded on, reasoning that only an API request gets that far. Only a /v1 request does, which is not the same thing: Studio's own chat calls those same endpoints with a session JWT, so a chat that named a model this server does not have popped the panel open mid-chat, which is exactly what via_api_key exists to prevent. The attribution now comes from the request that asked for the download. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Require the scanner's own label before folding a .gguf colon suffix The frontend splitter accepted any suffix after a .gguf head, while the backend requires that suffix to be the label the scanner derives from the filename. A colon is legal in a POSIX filename and POSIX is case sensitive, so "/models/llama.gguf:Bar.gguf" and "/models/llama.gguf:bar.gguf" are two real, distinct files. The one-time backfill folded both onto one override key, since the variant half is stored lowercased, and then re-read the local configs by that key, so the first entry was sent twice, the second file's context and KV cache settings never left the browser, and the done flag was set anyway. splitQuantSuffix now ports extract_quant_label for a bare filename, shard suffix and float-precision fallback included, and takes the suffix only when it equals that label. Checked against the backend over twenty-nine keys with identical answers on both sides, up from twelve, and the backfill now migrates both files with their own settings. The identity helpers come straight from features/hub/lib/model-identity rather than the hub barrel, which also re-exports the download manager and its React components. Same bindings, and it puts the module within reach of a test. The 27 new frontend assertions cover the split case by case against the backend's answers, and pin the storage rule the backfill's re-read depends on: two spellings of one repo id or one Windows path keep a single record, a POSIX path keeps two, and importing the legacy load settings never adds a duplicate. * Point the parallel-slots contract at the shared config signature main added nParallel to sidebar-model-config.tsx's own configSignature while this branch moved that helper into config-signature.ts so the hub, the sidebar and the model config page key one editor instance the same way. The behaviour is unchanged, so the assertion follows the expression to where it now lives and pins the sidebar to the shared key. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refresh Hub inference status after API-driven model switches The Hub read /api/inference/status once, on mount. An OpenAI-compatible request can auto-switch the resident model at any time, and nothing else on /hub reads status, so every 'loaded' marker and, more importantly, the per-model settings page kept describing the model that was resident when the Hub opened. The cost is concrete: with a stale checkpoint the newly loaded model fails settingsTargetIsResident, its settings page is handed loadedConfig=null, ModelConfigPage seeds the editor from saved or default values, and Apply reloads the model with them over the launch settings the API selected. Re-read on the moments this tab could have missed a switch -- regaining focus or visibility, and opening a model's settings page -- rather than polling on a timer. adoptResidentModelStatus already stands down for an external selection and for a load this tab started, so re-running it never fights the model the user is switching to, and applying an unchanged status leaves the live config identical, so the settings editor is not remounted under a draft. * Mirror per-model parallel slots to API loads and keep launch flags on eviction Parallel decode slots are a per-model setting the picker sends on every GGUF load, but the server-side override the OpenAI-compatible auto-switch path reads never carried them, so an API load of the same model fell back to the server-wide --parallel default. llama_extra_args could not stand in either, since --parallel is on the managed denylist. A config whose only change was the slot count also serialized to an empty payload, so the one-time backfill sent nothing and still marked itself done. Carry n_parallel through the serializer, the override schema, the normalizer and the load kwargs, and list it on the monitor so such an entry no longer reads as app defaults. Evicting a model to stay inside the browser's storage budget also sent the same full remove an explicit Forget does, which wiped llama_extra_args set through the settings API that no UI can show or restore, for a model the user never touched in that save. Eviction now clears only the mirrored fields; the route already carries stored launch flags over and drops the row once nothing is left. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep native-leased GGUFs off the API mirror and date the monitor's first snapshot Two identities the OpenAI-compatible auto-switch can never reach were being treated as if it could. A dropped or file-picked GGUF loads through a signed native-path lease, and /api/inference/status reports model_identifier as null for it, so the checkpoint the browser keys settings by is the bare file name the backend echoes back. local_model_resolver._build_index keys a standalone GGUF by its on-disk path and by its .gguf-stripped stem, so that name is never an index key: mirroring it wrote a server override no load can read, and the monitor's "Settings applied on API load" list then advertised it as live. The save gates on the lease token rather than the name, since the label falls back to a plain string with no suffix, and the one-time backfill, which has no token to read, goes by the identity shape. The floating monitor also wrote off every finished row of its first snapshot as history. That snapshot is not taken at mount: a hidden tab issues no fetch at all and an unreachable backend fails one, so the first API call of the session can start and finish before it lands and never open the panel. Date the backlog from when the poll stood up instead, on the server's own clock minus a browser duration, so a browser that disagrees with the server cancels out rather than replaying the whole ring buffer. A backend with no clock field keeps the old behaviour. The decision moves into its own module so it can be driven without a browser, which the overlay's .tsx dependency graph rules out. * Record a monitor row for a /v1 call refused by auto-download * Key a standalone GGUF's settings the same way on every surface llama_cpp falls back to _extract_quant_label(gguf_path) when a load names no variant, and /status echoes that as gguf_variant, so the sidebar wrote settings under <path>:Q4_K_M while the Hub row, the picker and the backfill used the bare path. The auto-switch lookup reads target_id before target_id:file_variant, so the bare entry always won and a sidebar edit could never reach an API load. The sidebar now nulls the variant for the settings identity, the same rule settingsGgufVariantForRow applies to a Hub row, sharing one definition. The displayed label still carries the quant. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Judge residency on settled state, not a snapshot Three places read the resident model from state that had moved on. The Hub's variant lookup closed over rowIsActive and activeGgufVariant before awaiting listGgufVariants, while the same click also starts a status refresh, so the two race and the network call usually loses. The residency test now happens after the await against the settled store, and skips an external checkpoint. adoptResidentModelStatus returned early on an empty status, before the external and modelLoading guards, so an unload from another tab or over the API left the store pinned and the settings page went on treating that row as resident. The empty branch now sits after both guards and clears only a local checkpoint. The monitor's Unload targets the resident local model from /status but cleared the store checkpoint unconditionally, and clearCheckpoint drops the persisted external selection too, so unloading wiped an external pick it never touched. It now clears only when the store holds the model that was unloaded. * Reject booleans for the numeric override fields Pydantic parses non-strictly and bool subclasses int, so a payload with max_seq_length true was stored as 1, a one-token context, and gpu_ids [true] as [1], an unintended GPU pin. _bounded_int already rejects bools for exactly that reason, but never saw one: coercion happens at the route boundary first, which left that guard unreachable through this path. A mode=before validator rejects only booleans, including inside the gpu_ids list, so every other lax conversion still runs and tensor_parallel, remove and fill_absent_fields keep working. * Match both spellings when the monitor unloads a model My earlier fix compared the store checkpoint only against the identifier /status reports, which is the concrete load path. For a GGUF loaded through auto-switch or from a non-active cache the store can hold the advertised repo id instead, so the comparison failed and the store stayed pinned to a model that had just been freed. Match the load path and the public alias, and keep the external guard that prompted the narrower comparison. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Read status before the settings target resolves a quant Two fixes to the API monitor and the Hub settings open. The overlay rearm only cleared `seeded`, so returning from the full page ran the initial-watch seed, and that seed holds running rows back on purpose: at a session's first snapshot such a row started while Studio was loading and nobody has seen it. Off the full page the opposite is true, since that page was showing the same feed with its running rows, so the overlay reopened on exactly the request the user had been reading. The seed now records whether it follows a rearm. Opening a cache row's settings resolves its quant from the store, and the comment there claimed the open kicks a status refresh that lands during the variant lookup. It does not: that refresh comes from an effect keyed on `settingsTarget`, which cannot run until the target exists. The Hub has no polling timer either, re-reading on focus and visibility only, so a window that keeps focus holds a checkpoint from before any API-driven switch for as long as the user sits there, and the editor opens on the quant of whichever model that switch displaced. The handler now reads status itself, alongside the variant lookup. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Clear the derived gguf key on forget and key cached repos by repo id Three fixes. A standalone .gguf is keyed by its bare path, but a load also reads the `<path>:LABEL` entry derived from the filename, which is how the picker keyed the same file before and what the backfill carries over from an upgraded browser. `resolve_model_override_key` cannot fold a bare path onto that spelling, so forgetting cleared a key that was never there and left the real one applying to every later API load, with the settings gone from the UI and nothing left to reach them. The remove branch now clears the derived key too, only for a bare `.gguf` path and only through the resolver, so an ambiguous fold removes nothing. `modelConfigIdentity` decided from the view kind what its own comment says depends on the row. Discover resolves a repo in an inactive HF cache into a discover-kind view whose resource is still the cache row with its snapshot-path run id, so Run from Discover read a key the Downloaded row never writes and loaded with default context, GPU and template, silently. Keyed off the resource instead. The detail view's settings button took the card's quant as given. That quant is a choice only when the user picked it in the selector; otherwise it comes off a store nothing re-reads while the window keeps focus. The card now says which it is, and a derived one defers to a fresh status read. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Consolidate the tests without changing what they prove Test-only. No file under studio/frontend/src or the backend's routes, core, hub, utils or storage is touched. Backend: the override PUT was spelled out at 29 sites as a two-call expression with a local import each time, and 39 tests mocked the store by hand when a fixture does it. A `_put` helper and an `override_store` fixture take both. -186 lines, 273 tests still pass, and the assert count is unchanged at 624. Frontend: eight test files become five plus a shared kit holding the bundler-resolver registration, the localStorage fake and the chat-runtime store fakes that three files had each written out. The resident-status pair merge into one file, and the three identity/storage files into another. 62 tests, 139 assertions, both unchanged. Prose: multi-line docstrings and comment blocks in the test suites keep their opening statement, the rest being recoverable from history. That is most of the remaining reduction, because these tests are close to one line per assertion already. Two consolidations were measured and rejected rather than shipped. A table-driven form of the source-contract tests generates 930 lines to replace 773, since a row costs what an assert line costs. Parametrising the backend key-folding and carry-over families saves nothing once the helper above removes their boilerplate: what is left is the per-case reason, not repetition. Every mutation these tests were written to catch still reddens: reverting new-traffic.ts, adopt-inference-status.ts, settings.py and hub-page.tsx to their pre-fix parents each fails the expected tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve a local quant folder's variants by its path A local row carries a repo id only inside the HF cache: getLocalHubId returns null unless the source is hf_cache. A plain folder of quants, under the models dir, a custom folder or LM Studio, therefore has none, while the backend still marks it as needing one, since requires_variant is scan_path.is_dir(), and leaves format_variant null because a directory is not the single-file case. settingsGgufVariantForRow then returns null, the guard is entered, the lookup is skipped for want of an id, and the toast is all that is left. The row menu offers Settings on every non-dataset row, so for these folders it could never do anything. The listing takes a path in that position and scans it, before the repo-id validation that would otherwise reject one, and that is already the request the on-device card makes: its fetch state is keyed on the model id, which for a local row without a hub id is the load path. So both surfaces now choose from the same quant set, and the settings key, the row's load id, does not move. * Stop clearing the checkpoint on an empty status, and forget every spelling Three fixes. An empty /status is not the same as the model going away. With idle unload on, the server frees the resident GGUF and keeps a stash the next request reloads, while get_status never consults that stash and InferenceStatusResponse carries no field that tells the two apart. The store is what names the model meanwhile, for the usage examples and for the reload itself, and usage-examples.tsx says as much in its own words. The chat runtime, which this adoption mirrors, resets only capability flags on an empty status and leaves the checkpoint pinned; every other clearCheckpoint caller is tied to the action that caused the unload. The clear I added two rounds ago is removed, along with the test that pinned it. Forgetting an override cleared only the key a load resolves to. An exact hit short-circuits before the fold, so with two spellings of one model in the map, from a build whose setter stored the literal id, the one named went and the survivor became the sole fold match: the next load applied what had just been forgotten. The folding rule is factored into one helper and the remove branch clears every key under the identity. POSIX paths still stand alone, so two files never clear each other. Both settings handlers now read status before building a target rather than only in the variant branch. Every path out of them seeds the editor from the store and Apply reloads with what it seeded, so a target built on a pre-switch read could reload a resident model from defaults and overwrite its saved config if applied before the read landed. A quant the user picked in the card is still preserved. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments across this branch Collapse multi-line comment blocks to one or two lines and drop comments the code already states. The reasons behind the non-obvious rules are kept, just shorter: the empty-status checkpoint rule, POSIX vs Windows path folding, the legacy path:LABEL override key, reading status before building a settings target, and the rearm seed. Comments only, no code changes. * Never let the monitor's api-key check fail a load _request_used_api_key decides one monitor label, and it already wraps the header read in a try because a caller may hand it anything. The unpack sat outside that guard, so a request object whose headers answer with a non-string reached `scheme, _, token = header.partition(" ")` and raised ValueError out of _load_model_impl. That is exactly how the load routes are driven in tests: a MagicMock request answers every attribute, so `or ""` never fires and the unpack blows up. Twelve tests across test_nvfp4_load_error_message.py, test_validate_gguf_runtime_message.py and test_chat_load_during_training.py failed on this branch and pass on its merge base, which is where this came from. The full backend suite now matches that baseline exactly at 17 pre-existing failures in the same six modules, down from 29. Made the helper total rather than teaching three test modules to build a better double, because a best-effort label must not take a load down for any caller. * Read an empty status against the idle setting, and keep repo quants Two fixes. An empty /status means one of two things and the payload cannot say which. Armed, the idle loop frees the model but keeps a stash the next request reloads, and the store is what names it meanwhile. Disarmed, nothing will bring it back, so the model is gone and leaving the row resident seeds the settings editor from a launch config nothing is running. Neither always-clear nor never-clear is right, and the only endpoint that knows is /openai-auto-switch, which already reports idle_unload_active for the usage examples. The Hub reads it once and the empty-status branch clears only while the loop is disarmed, which is the default. isStandaloneGgufPath tested the .gguf suffix alone. Repo ids ending in .gguf are real on the Hub, an iMat repo among them, and those hold every quant of a model, so reading one as a single file dropped its variant and saved Q4 and Q8 under the same key. The id now also has to name something on this machine: a path anchor, a drive, a second separator, or the bare filename a picked file is echoed back as. A repo id has none of those. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Merge main, and read backend sources as UTF-8 in the contract tests _read_backend arrived with the local MTP drafter work and reads without an encoding, three lines below _read, which passes one. On Windows that decodes as cp1252, and routes/inference.py holds byte 0x81 at offset 103058, so the Windows job died with UnicodeDecodeError before any assertion ran rather than reporting a contract. Same failure on main today; it shows up here because this branch touches the file the helper reads. * Fix six issues the review found, one of them in my own last fix chat_template_override: I gated the re-seed on seedLoadParams and said it implied hydratingExistingModel. It does not, it is !modelLoading, so the re-seed also fired on an ordinary poll and overwrote an unsaved edit on every refresh. It needs both, which is what the neighbouring fields do. The idle-unload setting was read once and cached for the life of the page, so a timeout changed from Settings while the Hub stayed mounted was never seen, and a transient failure stuck on the default forever. It is read with every status read now, keeping the last answer on failure rather than falling back to the default: the default is disarmed, which is the side that clears the checkpoint. Forgetting repo:QUANT cleared the bare repo entry unconditionally, which strips the quants inheriting from it. It is only cleared when no other qualified key survives, so the forget has to be the last word on the model. Provenance would be the better test but nothing records it: the carry-over copies the flags without marking the copy. ApiMonitor.clear dropped an own row that was still running, losing the request outright, since active_count falls to zero and the finish that follows has no entry to land on. The shared pass in the same function already keeps a running row; both follow that rule now. The scoped-clear test used a running row only because start() is the only way to make one, so it finishes the request first and the running rule gets its own test. A cached repo is keyed by its repo id now and used to be keyed by the snapshot path, and resolveInitialConfig has no fallback, so an upgrade read as never remembered. adoptLegacyConfigKey moves the record once, before either entry point reads the new key; a newer save under the new key wins. The server override mirror sent gpu_ids without their namespace. The same integers are Vulkan ordinals under Vulkan and device indices elsewhere, and the server cannot tell, so only a physical pin travels. An absent kind is a record written before the field existed, which was physical only. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix the argument order in adoptLegacyConfigKey, which broke the build savePerModelConfig takes (modelId, ggufVariant, config); the new adoptLegacyConfigKey passed (modelId, config, ggufVariant). tsc caught it, so `npm run typecheck` and `npm run build` both fail at head, and since `npm run build` is the first thing studio setup does, the failure cascades: 30 of 51 checks are red on eb4c3eac8, including startup on all three platforms, the wheel and Tauri builds, Chat UI Tests and every connection job. The runtime effect is worse than the type error suggests. normalize() receives a string, hits its `typeof raw !== "object"` guard and returns an all-defaults config, isDefaultConfig() then reads that as default and takes the delete branch, so nothing is written at all. deletePerModelConfig on the next line still drops the real legacy record, and savePerModelConfig has already returned true. The migration that exists to carry settings across the re-key destroys them instead, and reports success. The contract test asserted the call as a literal source substring, so it passed on the broken code. Corrected alongside, but a substring assertion cannot catch an argument order, so this also adds tests/adopt-legacy-config-key.test.ts, which drives the real storage module and asserts the moved config's values rather than the presence of a key. Two of its four cases fail on the previous head and pass here; the other two cover the paths that never reach savePerModelConfig. Verified: typecheck clean, 103/103 node tests, production build clean, 122 passed across the three tests/studio contract files, and 316 passed across test_openai_auto_switch.py and test_api_monitor.py. * Take the cosmetic churn back out of the test diff The test diff carried 90 pre-existing functions that this branch only reworded or re-wrapped comments in. That is what made the three test files read as 2446 lines of change when the behavioural part is far smaller, and it is the reason the deletion count looked like coverage had been removed: 342 of the deletions were comments being reflowed to a wider budget, not assertions going away. Restored those functions to their merge-base text wherever the change was provably cosmetic. Two rules decided it: comments never reach the AST, so they drop out for free, and a docstring only counts as cosmetic when its word sequence is unchanged, so the 16 docstrings this branch genuinely rewrote are left alone. The executable AST of all three files is byte-identical to the previous head. Also pointed the 19 backend source reads the new tests had inlined at the _read_backend helper the file already owns, which is what the pre-existing tests in it use. Seven of them spanned three physical lines to repeat a path and an encoding argument. The two inlined reads inside pre-existing tests are left as they are, since rewriting those would add diff rather than remove it. No test and no assertion was removed: 278/633, 107/250 and 102/547 before and after, per file. The three files go from 2446 to 2094 lines of diff, and the PR from 9854 to 9502. Verified: 2141 passed and 3 skipped across tests/studio, 486 passed across test_openai_auto_switch.py, test_api_monitor.py, test_openai_auto_download.py and test_parallel_slots_per_load.py. * Fix three ways a saved per-model config is lost or ignored Three review findings, each a case where the settings a user saved stop being the settings a load applies. routes/settings.py, utils/openai_auto_switch_settings.py. A repo cached outside the active HF cache has two spellings: the snapshot path it loads from, and its repo id. The one-time backfill mirrors whichever one localStorage held, a later Settings save writes the other, and nothing retired the first, so the override map ended up holding both. The load ladder reads the path before the advertised id, so every API auto-load kept applying the pre-migration config and the save looked like it had done nothing. Reordering the ladder would only move the bug: an inactive-cache repo is sometimes legitimately keyed by its path, when inventory-dedupe keeps no cached row for it. So a save or a forget now retires the other spelling of the same (repo, quant), which is the one-entry-per-model invariant this route already enforces for casing folds, the <path>:LABEL legacy key and the bare repo id. The retired entry hands over its llama_extra_args first, since the page can neither show nor restore them. The one-time fill retires nothing, or the migration would delete what it is meant to carry. per-model-config.ts. adoptLegacyConfigKey saved under the new id before deleting the old one, so at the entry or byte budget the map briefly held both copies and the save evicted the oldest unrelated model to fit: silently, still reporting success, and with no eviction list to hand back, so that model's server override kept applying with nothing in the UI able to forget it. The key is renamed in one write now, which cannot grow the entry count. Reachable well before the entry cap via the byte cap: 17 models each carrying a 60 KB chat template sit under 1 MiB. apply-inference-status-to-store.ts. An API auto-switch hands the loader the concrete snapshot path, so /status reports that path while the model's settings are keyed by its repo id. Reading remembered slots by the raw identifier missed the record, cleared nParallel on the model change, and the next save wrote the blank back over both localStorage and the server entry. Resolved through the cached-repo alias now, and only for namespaced ids: a file stem two models can share must never be read, or one model's settings apply to another. Two further findings are not changed here, with the reasoning in the PR thread: the duplicate-key collapse cannot affect a load, because the override keys come from the resolver and never from the request's spelling; and preserving a bare repo entry for a quant that inherits it is unsatisfiable alongside test_forget_clears_the_bare_repo_entry_the_quant_inherited_from, since neither case leaves anything in storage to tell them apart. Verified: 492 passed across the four backend suites this touches, 2142 passed and 3 skipped across tests/studio, 118 node tests, typecheck, build and ruff clean. Each fix was confirmed against its own tests on the unfixed source first: 4 fail for the alias retirement, 2 for the eviction, 5 for the slot lookup, and the two over-deletion guards pass either way. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments this branch added Same reasons, fewer lines: collapse the multi-line explanations added by this branch and drop three field docs that only restated the field name. No code, docstring or test changes. * Stop a lower-priority override key holding newer data than the one a load reads Four defects, all one shape: a write leaves a key the loader reads before, or instead of, the key holding the values the user actually meant. Standalone GGUF saves now consult the filename-derived legacy key. The loader reads the bare path ahead of "<path>:LABEL", and the settings page never sends llama_extra_args, so the backend carry-over was the only thing preserving those flags; for a colon-free path neither _bare_model_id nor cached_repo_alias_keys fires, so the new bare entry won and the flags became unreachable. The removal branch already called _legacy_standalone_gguf_key; the save branch now does too. The one-time fill no longer creates a snapshot-path key over a repo-id entry. resolve_model_override_key cannot fold a path onto a repo id, so a backfill from a pre-upgrade browser wrote a brand-new higher-priority key and skipped alias retirement, shadowing newer server values on every API load. The redirect is direction-aware: only the path spelling outranks, so a repo-id fill over an existing path entry still creates its own key, and two snapshot paths never redirect since neither is knowably the load path. Carried pass-through flags no longer shadow first-class controls. A save keeps stored llama_extra_args while writing the field just edited, and the auto-switch mapper sent both, so a stale "--ctx-size 8192" landed after Unsloth's own flags and won llama.cpp's last-wins parse against a freshly saved 32768. The /load route already strips exactly these groups off inherited extras; that stripper is imported rather than mirrored, gated per group on the field the override supplies, so a flag with no first-class field behind it still passes through. A same-model reload now advances the chat template. hydratingExistingModel is computed from checkpoint and quant alone, so a reload from another client left the Hub presenting the old template as live, and the next Apply persisted and reloaded it over the new one. The baseline always advances; the control advances only while it still equals the old baseline, so a mid-edit value survives. Blank and null compare equal, matching cleanTemplate and the backend normalisation. The resolver is a separate module because the applier imports the model-picker barrel, which is unimportable under node --experimental-strip-types. Each fix was checked against its own test on the unfixed source first: 2 fail for the settings-key pair, 2 for the shadowing flags, 5 for the template seed. 326 backend, 316 llama-server/chat-load, 103 contract and 127 node tests pass, with typecheck, build and ruff clean. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop the monitor acting on a snapshot the API has already moved past Two defects on the new monitor, both where this PR's own feature is the concurrent actor: an API auto-switch runs while the page is watching it. Turning automatic opening off no longer arms a delayed pop. The poll stood down without re-arming the watch, so API-key calls landing during the opt-out stayed unseen and the first snapshot after re-enabling reopened the panel for an hour-old burst. The suppression guard could not stop it either, since lastNewEntryAt is frozen for the whole opt-out and quietFor always exceeds REARM_QUIET_MS. That contradicts the invariant stated a few lines above it, that a backlog built while the poll stood down is not new traffic, which the other stand-down already enforces. Re-arming alone is not enough: autoOpen is an observer dependency, so the re-enable render folds the stale pre-opt-out snapshot first and spends the re-arm before any fresh poll resolves. observeResponse now folds each snapshot at most once, and standDownWatch re-arms only a watch that has seeded, so a session that merely starts opted out keeps its first-snapshot semantics. Unload no longer reports success over a model it did not free. The button named the checkpoint it read from /status, but /unload takes the lifecycle gate the auto-switch holds across a whole swap, and the switch does substantial awaited work before teardown, so status answers with the outgoing model for a wide window. Naming a model a load has replaced is a 200 no-op, so the button reported success while the new model kept its VRAM. There is no unload-current route, so unloadResident reads, unloads and re-reads, retrying once and surfacing the model still resident instead of swallowing it. Steady-state cost is one extra GET /status per click and never an extra unload. Note on the contract test this moved. test_monitor_unload_clears_only_the_model _it_freed asserted a verbatim source line, so it passed on the racy single-pass unload and broke only when the text moved; it never covered its own name. The assertion now matches the new shape and its docstring points at the node test that exercises the race. Each fix failed its own test on the unfixed source first: 2 for the re-arm, of which one pins that the naive one-line remedy is still broken, and 3 for the unload. 2142 tests/studio, 326 backend, 137 node tests pass, with typecheck and build clean. * Release a settings open on the refresh that superseded it, and report a refused clear A superseded status read no longer releases the caller awaiting it. The drop branch was a plain generation check that wrote nothing and still resolved, and settingsOpenSeq is bumped only by opening settings, so a focus or visibility refresh could supersede a settings-open read while passing that guard. Every other superseder is either covered by settingsOpenSeq or starts after the awaited read and therefore wins; focus and visibility are the only uncoordinated ones. Half of the reported harm does not occur: the editor cannot keep the old model's config, because loadedConfigSignature is folded into the React key so the view remounts when the live config lands, and setting settingsTarget fires a fresh refresh that always starts last. What does not self-heal is the quant baked into settingsTarget by the stale read, which is what runSettingsTarget passes to selectModel and what settings are then persisted under. That is the harm test_detail_settings_defers_a_derived_quant_to_a_fresh_status_read already guards elsewhere, so a dropped response now resolves with the refresh that superseded it. Only a strictly newer sequence hands back a promise, so the unmount cleanup, which bumps the sequence without starting a read, cannot make the last refresh await itself. A refused clear no longer fails silently. clearApiMonitor rejects on a network error and on any non-ok DELETE, clear() had no catch, and the page discards the promise with void, so the detail pane emptied while the log stayed put with no message and an unhandled rejection behind it. The hook already owns an error state the banner renders, so the failure routes through it and the reload is skipped, since nothing was deleted. One limit worth stating: when the DELETE alone fails while polling still succeeds, the next poll clears the shared banner within about a second, so the message flashes. That is inherent to reusing the existing error state, and it still beats silence. The case it genuinely fixes is a paused monitor, where the poll returns early while the button stays enabled, so the failure was permanent. Each fix failed its own test on the unfixed source first: 2 of 6 for the supersession, 2 of 3 for the clear. 2142 tests/studio and 146 node tests pass, typecheck and build clean, and no contract assertion needed changing. * Serialize override writes so two spellings cannot retire each other The override save handler is a plain def, so FastAPI runs it in a threadpool and two requests really do interleave. Each individual write is atomic, since set_model_override goes through upsert_app_setting_map_entry under BEGIN IMMEDIATE, but nothing spans calls: the target write, the alias read and the alias delete are three separate transactions, and the only lock in either module guards the two second memo. So when one client saves a cached quant by repo id while another saves it by snapshot path, and both writes land before either cleanup, the repo-id request deletes the path row and the path request then deletes the repo row. Both calls return 200 and the store ends empty. The two spellings are not hypothetical. The shipped UI keys a cached repo by repo id, but the path spelling is why cached_repo_alias_keys exists at all, the browser's write queue is keyed by spelling so the two go out in independent queues rather than serialized, and the eviction mirror sends dropped.modelId straight from localStorage, which can still be a legacy snapshot-path key because the openModelSettings branch never adopts it. It is also a documented REST endpoint any client can call. The server is a single uvicorn process with no workers configured, so a module level lock is a complete fix. It is applied as a decorator rather than a with block for two reasons: the handler body is not reindented, which keeps the verbatim-source contract assertions intact, and the remove branch's four-write sequence is covered by the same lock. The new test pins both threads just after their target write with a barrier, falling back on a broken barrier once the writes serialize so the fixed path cannot deadlock, and locks the fake store per entry to mirror BEGIN IMMEDIATE so only the gap between calls is on trial. It fails on the unfixed source with an empty store. 327 backend, 103 contract and 160 adjacent settings-route tests pass, ruff clean, and no contract assertion needed changing. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> |
||
|
|
06829c2627
|
Studio: tighten the comments added by the OpenAI model-admission work (#7501)
Comment-only follow-up to #7454. That change carried 523 comment lines, many of them three and four line preambles where one line says the same thing. This collapses them and drops the ones restating what the code already says, for a net 77 lines. Scope is limited to comments #7454 itself introduced. The files it touched hold about 3,761 comments in total; the rest predate it and are untouched, verified by checking that every removed line is one that commit added. Nothing that records why a non-obvious decision was made was dropped, only compressed. Still stated: the normcase-before-versus-after Windows separator trap, the innermost-indexed-model rule for nested directories, an HTTPException being a decision rather than a failure to decide, that only an explicit False is anonymous to huggingface_hub while None borrows the server owner's login, the fail-closed tri-state custom-code gate, and the regressions each test was written for. Code is provably unchanged: comment_tools.py check reports 17/17 files comments-only. Backend CI command 10337 passed, 0 failed. tsc -b clean. |
||
|
|
da447d47ba
|
Studio: fix the "No model loaded" error, and optionally auto-download a model named in an API request (#7454)
* Studio: say which model is missing instead of "No model loaded" A /v1 request naming a model that is not downloaded returned the generic "No model loaded. Call POST /inference/load first.", which cannot fix it. Return 404 model_not_found naming the model and listing what can serve, and make the API usage examples name a model the server actually has. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: page the API monitor, show model load/unload, pin the example quant The monitor rendered all 50 retained entries in one scroller: page it 5 at a time, freezing history while paged back so live traffic cannot reorder it. Add model load/unload rows so the feed shows what the server is doing, and stop the header rendering the loaded model as a raw host path. Advertise each model's GGUF quant on /v1/models so the example pins repo:QUANT, and move the auto-switch section above the monitor with shorter copy. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: optionally download a model named in an OpenAI API request Auto-switch only ever loaded models already on disk, so naming one this server does not have either 404s or, when something else is loaded, gets quietly answered by the resident model. Add openai_api_auto_download_model (off by default, gated on auto-switch). When on, a /v1 request naming a GGUF repo that is not downloaded starts a background fetch and returns 503 with Retry-After and a typed model_downloading code. The resident model keeps serving in the meantime, and the retry after the download completes is served by the new model through the existing auto-switch path. The download reuses the Hub manager's service layer, which already does repo-id validation, casing, claim bookkeeping, disk preflight, resume and cancel. The in-loader download is deliberately not used: it silently falls back to a smaller quant under low disk, which is wrong when the caller named an exact one. Admission is narrow, since a request only needs an API key: - namespace/name only, so gpt-4 and other foreign ids fall through to the resident model exactly as before - GGUF only, decided from the remote file list rather than the repo name - anything declaring auto_map is refused, so trust_remote_code stays a deliberate opt-in in the UI and can never be granted over the API - a single download at a time, plus a free-disk reserve - one model_info call answers existence, gating and the quant list, so a missing repo, a gated repo and a wrong quant each get their own error With the setting off every one of these paths is byte-identical to before. Also: - monitor rows for downloads, with a live percentage - public_model_id resolves an HF cache snapshot to its repo id, so a cache-loaded model is no longer labelled with a commit sha; this drops the duplicate helper added for the monitor and fixes the same leak in the inference status response - the unedited sk-unsloth-YOUR_KEY from the copyable examples now says so instead of "Invalid or expired API key"; every other bad key keeps the generic message * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: add an Unload button to the API monitor The monitor names the loaded model but offered no way to free it. Idle auto-unload is the only existing release path, and it needs a TTL and a wait. The button sits next to Refresh, appears only while a model is loaded and is disabled mid-unload. /unload matches on the internal identifier, which this response deliberately omits because it would be a host path, so the click reads it from /api/inference/status the same way the chat runtime does rather than widening the monitor payload. Also stamp the manual unload row with the quant, read before the teardown clears it, so it reads repo:QUANT like the load row it pairs with. * Studio: keep the API monitor Unload button visible when idle It only rendered while a model was loaded, which hid the one manual release path at exactly the moment someone goes looking for it. Render it always, disabled with a "No model is loaded" tooltip when there is nothing to free. * Studio: never answer a named model with a different one Asking for a model this server is not serving returned 200 from whatever was resident. Requesting gemma-4-E2B-it-GGUF:UD-Q6_K_XL while UD-Q4_K_XL was loaded got a confident answer from the wrong quant, with nothing in the response saying so. A name carrying a namespace (org/model, optionally :QUANT) is a concrete reference, so 404 instead, with the reason: - wrong quant -> names the quants that are actually downloaded - not on disk -> lists what is available - on disk but auto-switch off -> says to turn it on Ids without a namespace (gpt-4, claude-3, default) are foreign labels rather than references, so they still fall through to the resident model and drop-in clients are unaffected. A bare org/model is still satisfied by any loaded quant of that repo; only an explicit :QUANT must match. The check runs whatever the auto-switch and auto-download toggles are, since serving the wrong weights is wrong in every configuration. It is skipped when nothing is loaded, where the existing no-model-loaded error already says the right thing, and when the model is on disk with auto-switch on, where a failed swap should still fall back. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: use a simpler prompt in the API usage examples "What is Unsloth Studio?" rather than "Can Unsloth Studio do API calling?". One constant feeds all nine snippet tabs. * Studio: only refuse a model reference meant for this server A namespace alone was treated as a concrete model reference, so a /v1 request naming anthropic/claude-3.5-sonnet, openai/gpt-4o or any other LiteLLM or OpenRouter style vendor/model id started returning 404 instead of being answered by the resident model. Refuse only on evidence the caller meant this server: an explicit GGUF quant label, or a repo that is actually on disk here. gpt-4 and vendor/model alike fall through again, while the wrong-quant and wrong-repo cases this PR exists for still refuse. Also from review: - Release the single download slot by object identity, not repo id. A stale watcher could clear a newer download of the same repo and let a second multi-GB fetch start alongside it. - Catch BaseException around admission: CancelledError is not an Exception, so a cancelled request stranded the slot for the process lifetime. - Honour the download service's accepted=False, which it returns without raising for a cross-variant conflict, instead of promising a download that was never dispatched. - Treat a failed status probe as unknown rather than idle, so a transient read cannot fail the monitor row and free the slot under a live worker. - Check gated repos with auth_check. The Hub serves metadata for a gated repo without granting its files, so the licence gate was being reported as the unrelated custom-code refusal. - Size the disk reserve from the download plan, which includes the mmproj and MTP companions the worker fetches with every quant. - Never fetch under the server's own HF token. The repo is named by whoever holds an API key, so the ambient token let that key pull the owner's private repos. - Refuse an explicit quant on a backend with no quant identity, gated on the suffix really being a quant so Ollama style :latest tags still match. - Raise instead of falling through when the diagnosis fails: the mismatch is already established by then, only the wording is uncertain. - Report a failed switch as 503 model_switch_failed rather than answering as the resident model. - Fail an open monitor row under the same lock as the check, so a finish landing in between cannot stamp an error onto a completed row. - Usage examples never emit a hardcoded model id: the catalog is tri-state and the panel asks for a model to be loaded instead of printing one the server cannot serve. It also refreshes when the loaded model changes. - Keep the monitor pager reachable while frozen entries expire. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: scope the auto-download 404 cache to the caller's credentials The Hub answers 404 for a private repo the caller cannot see, so caching that verdict per repo alone let one anonymous request mark a private repo unservable for everyone for the whole TTL. A later caller sending a valid X-Unsloth-HF-Token skipped the probe and fell through to the resident model instead of downloading what it asked for. Keyed on the repo id plus a digest of the token now, so the token itself is never held. Two more from the same review: - Clear the chat runtime checkpoint after unloading from the API monitor, as the chat eject flow already does. The store went on treating the freed checkpoint as loaded and the usage examples kept naming it. - Point gated and not-found callers at the X-Unsloth-HF-Token header. Automatic download deliberately ignores the server's own Hugging Face identity, so telling the user to add a token in Studio sent them round the same 403 forever. * Studio: tighten the comments added by this branch * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: keep API auto-download off the server's Hugging Face identity Passing None for the caller's token was not anonymous. spawn_worker substitutes the backend's HF_TOKEN for a falsy one, and HfApi(token=None) falls back to a cached login, so a repo named by an API-key holder could still be fetched under the owner's Hub identity and land in the shared catalog. The metadata probe and auth_check now pass an explicit False, and dispatch threads allow_ambient_token=False so the worker stays anonymous too. The flag defaults to True, so the UI download path keeps the ambient fallback that private repos rely on. Three more from the same review: - Require an exact hf_variant match only when the suffix is really a quant. The llama.cpp branch still compared Ollama style :latest and :8b against the loaded quant and refused the resident model, which is the opposite of what looks_like_quant classifies them as. - Decode an HF cache repo id only when the models-- component is followed by snapshots. An ordinary directory whose name merely starts with models-- was being read as an encoded repo id. - Return the probing response before consulting the job registry when an adopted claim has no variant yet. A stale error on the whole-repo key could otherwise release the slot the first request's probe still holds, letting a second large download start beside it. * Studio: stop treating a namespace as what decides model intent The rule refused a reference only when it carried a namespace, which was wrong in both directions. vendor/model is how LiteLLM and OpenRouter name every provider, and a standalone or custom-folder GGUF is advertised without one, so asking for a path-free local id such as model-Q4_K_M was answered by whatever else happened to be resident. The slashless early return is gone and the same evidence test now applies to every id: an explicit quant, or a model that actually resolves here. gpt-4 and default still fall through because they are not local, not because of their shape. Also: - Recognise bits-per-weight quant labels. _extract_quant_label emits IQ4_XS-3.53bpw and the resolver and downloader both accept it, but _GGUF_KNOWN_QUANT_RE has no bpw group, so looks_like_quant rejected a reference the rest of the machinery understands. - Upper-case the synthetic names handed to _pick_best_gguf. Its preference tokens are upper case and matched case-sensitively, so a repo with lower-case filenames skipped the preference and took the first entry, which can be F16. - Only offer a downloaded but unloaded model as a runnable example when auto-switch is on. It is off by default, so the copied snippet hit the no-model-loaded error, which is the failure this branch exists to fix. The tool-passthrough cancel test stubbed asyncio.to_thread module-wide, so it cancelled at the first thread hop rather than the generation hop it means to test. Model resolution runs off the loop before the monitor row opens, so that stub now passes the resolver through. * Studio: tighten the comments added since the last pass * Studio: match a resident model through its resolver alias A manual load stores the model by its on-disk path while the resolver and /v1/models advertise it as publisher/model, so _loaded_satisfies could not recognise the alias. Reducing the resolution to a boolean then threw away the load path that would have proved the match, and the request was refused with 404 for a model the server was serving at that moment. Common for LM Studio models and custom-folder aliases. The resolved path is compared against the resident backend before anything is refused. Also: - Size disk admission on what is left to fetch. expected_bytes is the whole plan, so a resumed quant or a companion already pulled in by another quant was charged for twice and could 507 a download that fits. Cached blobs are subtracted through existing_blob_bytes, the same accounting the worker's own preflight does, and it falls open to the full size when no blob hashes are available. - Report a cancelled download as cancelled. The catch-all sent every state other than complete or idle through fail_open, so a deliberate cancel rendered as a download failure rather than the monitor's cancelled state. - Keep polling the servable ids while nothing is loaded. The poll settled as soon as auto-switch was on, so turning it back off left the examples naming an unloaded model until something else remounted the panel. * Studio: shorten the comments added in the last pass * Studio: keep the FLA fast-path tests hermetic across transformers versions _discover_fla_model_types scans the *installed* transformers for modeling files importing `from fla.`, so `models/qwen3_5/` only exists from transformers 5.x. The backend supports transformers>=4.51, and on a 4.x install the Qwen3.5 gate returns False, so 14 tests in test_training_worker_flash_attn.py silently exercised a no-op instead of the install path and failed their call-count assertions. Pin the discovered model_type set in those 14 tests, the same way test_hook_does_not_install_tilelang_for_model_outside_allowlist already pins it against newly added FLA model_types. Test-only change: the production gate and the _discover_fla_model_types unit tests are untouched. * Studio: keep the /v1 admission check off the model-scanning path The admission check added here runs on every /v1 request, including with auto-switch off, where the route used to return straight away. It called resolve_local_gguf, whose index is cached for 5s and otherwise rebuilt by walking ./models and every HF cache root, under a lock the next caller waits on. On an install with a large cache that scan measured 6.1s, longer than the TTL that is meant to amortise it, so steady traffic would keep rebuilding it. Answer from the last built index instead and never rebuild from the request path: a stale answer is fine here, since what is on disk barely moves and a finished download already invalidates the index. The first request, before any scan has completed, warms the index on a background thread and skips the check rather than blocking on it. That also makes the lookup a dict read, so it no longer needs handing to a thread. Cold resolve on this box goes from 6152495us to 0.4us, and the whole hook now costs the same for a foreign label as for the resident model. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: fix the admission hook's cold, stale and contended index paths Five review items, four of them on the admission hook added here. Skipping the check until the first scan lands also skipped explicit quant mismatches, so the first request after startup could ask for :Q8_0 while Q4_K_M was resident and be answered by it. The early return was redundant as well: with an empty index resolved is None and here is False, so the gate below already lets a bare name through and refuses an explicit quant, which is what the except branch has always concluded. Dropped it and index_is_built with it. index_is_built took _lock, which _index holds for the whole scan, so once a warm was running every later request blocked on the event loop for exactly as long as the scan it was there to avoid. The warm now has its own lock and reads the timestamp unlocked, which is safe because _scan is only ever rebound. Warming only when the index had never been built left a model fetched in the Hub UI, or dropped into a scan folder, invisible for the life of the process, since only the auto-download watcher calls invalidate_index. Warm on staleness too, and unconditionally, so it refreshes within a TTL without a scan on the request path. Rescanning is capped at a tenth of the scan's own duration: a big install takes longer to scan than the TTL, and warming on the TTL alone would keep a thread scanning continuously. An Ollama-style tag names no quant, so the resolver misses it and auto-download saw a model the resident one already answers to, then 404'd it for having no such quant. Return early when the loaded model satisfies the reference. Frontend: a cancelled download said "Model download failed", because the label collapsed everything non-completed into failure. The backend tests get an autouse fixture that stops the warm from walking the developer's real HF caches; that scan starved the loop under the timing sensitive streaming tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make /v1/models and the admission hook agree on what is local Three review items, all on the seam between the catalog scan and the resolver index, which run on separate schedules. /v1/models can advertise a local GGUF the resolver has not indexed yet. A bare id carries no quant to refuse on, so a client asking for one it had just been handed was answered by the resident model instead. The hook now reads the catalog cache as evidence too, never scanning it. It takes the path rather than a yes/no because the converse also happens: the catalog can list the resident weights under an alias the loaded entry does not answer to, and those must stay served. That alias was also emitted twice by /v1/models, once as the loaded basename a manual load records and once as publisher/model marked unloaded, because the dedup only compared ids. Compare the path as well. A directly loaded standalone .gguf takes its quant from the filename, but the resolver stores such files with no quants, so the advertised <stem>:<quant> stopped resolving as soon as anything else loaded. Advertise a quant only when that reference resolves, and downgrade only on a definite answer so a cold index leaves the metadata alone. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten the comments this branch adds Collapse the multi-line notes in the auto-download path, the /v1 admission hook and their tests to one line each, keeping the reason and dropping the restatement. No behaviour change. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: four admission and catalog fixes from review Lowercasing paths in _resolves_to_resident made /srv/models/Foo and /srv/models/foo the same weights on any case-sensitive filesystem, so a request for one could be answered by the other and /v1/models could mark the wrong entry loaded. That helper now backs residency as well as admission, so use os.path.normcase, which folds case only where the filesystem does. Advertising a quant whenever the resolver could not disprove it kept the bug it was meant to fix: a standalone .gguf loaded before the first scan still got <stem>:<quant> published, and the usage examples persist that. No proof is not proof, so omit it and warm the index instead. A 401 from an expired or invalid X-Unsloth-HF-Token skipped the 403 and 404 branches and surfaced as "could not reach Hugging Face, retry shortly". It now says to replace the token, kept apart from the gated refusal since a rejected credential is not an unaccepted licence. An image request naming an undownloaded text-only GGUF started the whole download and only then hit the capability guard, which never sees a remote target, so every retry 400d and the bytes were wasted. Thread require_vision into admission and check it against the mmproj companions the disk preflight already asks build_gguf_variant_plans for. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make the Hub error fixture carry a status on both hub majors The 401 test built HfHubHTTPError directly, which works on 0.x and fails on 1.x where response is required and keyword-only, so all four Python jobs failed while the same test passed locally. _hub_error already handled both constructors, but the 0.x branch left the exception with no response at all, and hf_error_status reads the status off it for the types that do not encode it in their name. So it could only produce a usable error on 1.x, which is why the test bypassed it. Attach the status when the constructed exception lacks it, and use the helper. Cover the helper itself against stand-ins for both constructor shapes, since whichever hub is installed only ever exercises one of them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: invalidate on every download, resolve bare tags, keep polling Three review items. Only the API auto-download watcher dropped the resolver cache, so a GGUF fetched in the Hub UI stayed absent to the cache-only request path and the request was answered by whatever was resident. finalize_worker_exit is the one point every download worker exits through, so invalidate there. That closes the window without leaning on the TTL, which the scan-duration throttle can stretch past 5s on an install where the scan itself takes longer than that. A downloaded but unloaded GGUF asked for as org/model:latest missed the resolver, since the suffix was always treated as an exact quant. With auto-download on that probed the Hub and returned a 404 for a quant that was never a quant; with it off it refused without switching. Fall back to the base entry when the suffix is not quant-shaped, and keep exact matching for real quants so a swap can never serve the wrong weights under the right name. The usage examples stopped polling once a model was resident, but idle unload frees one without touching the store, so nothing re-ran the effect and the examples kept naming a model that could no longer be reloaded. Slow the poll to 60s instead of stopping it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: hold the download slot while it is in use, and keep quants to llama.cpp _loaded_satisfies refuses a quant reference against the Transformers backend by name, but the path match did not carry that rule. A Transformers model active from a directory that also holds GGUF exports therefore matched a request for one of those quants and answered it with the safetensors weights. Only llama.cpp has a quant identity, so admission now passes llama_only whenever the reference is quant-qualified. A bare name still matches either backend, and /v1/models residency keeps the default so a loaded Transformers model is still reported loaded. The 24 hour watch window was bounding ownership of the single-flight slot when it should only have been bounding progress reporting, so a legitimately slow download had its slot handed back while the worker was still writing, admitting a second multi-gigabyte download beside it. Resolve the row on the clock, but keep the slot on a slower poll until the job is actually terminal. Past the deadline an unknown state does release it, since it means the worker cannot be probed and holding it on that forever would wedge auto-download. * Studio: keep what the resolver already knew when a download lands Invalidating cleared the index to empty. The request path reads that cache without scanning, so from a completed download until the rebuild landed it had no evidence about any local model, not just the new one, and a bare request for any of them was answered by whatever was resident. Wiring the hook into the shared completion path in the last commit widened that from auto-download to every download. Mark the scan stale and keep the entries instead. Both _index and warm_index_soon rebuild on a zero stamp, while the request path still sees everything it knew a moment ago. Only a completed download invalidates, and that only ever adds models, so nothing retained goes false. Warm from the completion hook too, so the rebuild starts when the download lands rather than when the next request happens to need it. * Studio: match the quant, not just the directory, and default-select bare tags Two quants of one repo share a directory, so the path match could not tell them apart and an explicit :Q8_0 was answered by a resident Q4_K_M that _loaded_satisfies had already refused by name. The llama_only fix in the last commit only ruled out the wrong backend, not the wrong quant on the right one. Both path matches now require the resident hf_variant to equal the requested quant whenever the reference is quantified; a bare name still matches on the path alone, since it claims nothing about the weights. The local resolver already treated a tag that names no quant as meaning the repo, but remote admission still looked for a quant literally called "latest", so the same reference resolved locally and 404d remotely. Branch on looks_like_quant there too. A real quant the repo does not have is still a 404 and never a substitution, which is what separates this from the loader's low-disk fallback. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: one quant preference, and stop trusting a stale checkpoint list_local_gguf_variants sorts by descending size, so the head of variants was the biggest quant, often F16, while remote admission and a plain load both rank through _pick_best_gguf. A bare id therefore meant a different quant depending on which side answered it, and the local answer was the one that could evict a working model and then fail or OOM starting an F16 next to a usable Q4. /v1/models advertised that same head for pinning. Pull the ranking into one preferred_quant helper and have both sides use it. The usage examples returned a stored checkpoint without ever consulting /v1/models, and the polling added last round was gated on not having one, so for a stored checkpoint it never ran. An idle unload then left the panel showing a snippet that could not run. Poll whenever mounted, and prefer the checkpoint only while the catalog still backs it or switching can reload it. A catalog that has not answered yet is not evidence against it. The static contract pinned the old dependency array, so it now asserts the intent it documents: a finished load re-runs the fetch, and the effect is not gated on having no checkpoint. * Studio: fix the Windows path compare, and advertise a label the worker knows The case fix normalized the separator to "/" and then called os.path.normcase, which on Windows folds case and rewrites the separator back to a backslash, so the descendant checks compared against a "/" the path no longer had. A manually loaded GGUF reached through an alias then read as a different model, giving a false 404 and an alias marked unloaded. Run normcase first and normalize the separator after it. There are two quant-label extractors and they only agree while a recognized quant token is present. With none, _extract_quant_label takes the last hyphenated segment, "7b" of llama-7b, while build_gguf_variant_plans and the worker key the whole stem: the plan lookup missed and the job exited on a variant it had no shards for. Use the canonical extractor for the unrecognized case only. Checked across real filenames first, the two match on every recognized quant and part on bpw-qualified labels, which _extract_quant_label keeps apart on purpose so byteshape's IQ4_XS at 3.53, 3.97 and 4.19 bpw stay separate variants. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: a stored checkpoint needs catalog evidence, not just the switch setting Preferring it whenever switching was on short-circuited the catalog check, so a checkpoint the store still held after the model was deleted or moved kept being named even though /v1/models had already proved it absent, and the snippets 404d instead of falling back to a model that is actually there. A lookup rather than a disjunction, which settles the whole matrix in one place: no answer yet keeps the checkpoint, since that is not evidence against it; listed and resident keeps it; listed but unloaded keeps it only when switching can reload it; absent falls back whatever the setting says. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: normalize the quote style pre-commit would have rewritten * Studio: cover the model that just landed, and pin the quant the catalog has Retaining the index on invalidation protects what was already scanned and by construction cannot contain the model that just finished downloading, so a bare request for it in the window before the rebuild was still answered by the resident model. Record the repo at the completion hook and treat that as admission evidence alongside the resolver and the catalog; the next completed scan clears the notes, since the index then covers them. Publishing a rebuilt index before completion becomes observable would have closed it too, but that blocks the download worker for the length of the scan. Catalog membership proves the repo, not the saved quant, and the examples then pinned the stored one. A quant deleted while another quant of the same repo remained produced repo:deleted-quant, a missing-quant 404 with a runnable alternative listed right beside it. Pin what the catalog advertises: for a resident entry that is the resident quant, for an unloaded one it is a quant actually on disk. The store is only consulted before /v1/models has answered. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: apply three rules everywhere they belong, not only where reported The trust probe was the last credential handoff still passing a raw token. huggingface_hub reads None as "use the cached login", so a caller-named repo was read with this server's Hugging Face identity whenever the caller sent none, which is exactly the isolation the metadata probe and the worker already keep. It takes _hub_token now. Enumerated the rest of that path while there: auth_check, model_info and spawn_worker were already correct. finalize_worker_exit is shared with dataset downloads, so the resolver hook fired for every completed dataset, scanning the model directories for nothing and recording the dataset id as local-model evidence, which turns a bare /v1 request naming that id into a refusal instead of a foreign-id fallthrough. Gated on repo_type. _already_serving decided "bare" on the presence of a colon while _loaded_satisfies and the resolver decide it on whether the suffix names a quant, so org/model:latest against a serving Q8_0 read as a mismatch and swapped in the preferred Q4_K_M for a request either one answers. That rule now lives in four places, each fixed in its own round, so this time I looked for the rest and found a fifth: describe_local_miss splits on the bare colon and its docstring claims it splits like the resolver. It no longer did, and would report a missing quant named "latest". Fixed here too, unreported. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: probe before refusing busy, and scan once when the index is cold The busy refusal fired before anything established the requested label was a model at all, so any namespaced id a drop-in client sends was told to wait out an unrelated download for as long as it ran. Probe first and refuse only a label the Hub actually serves as GGUF; anything else falls through to the resident model as before. A probe failure answers "not downloadable", since stranding ordinary traffic costs more than missing a busy refusal. Treating an unbuilt index as "nothing here" let the first request after startup be answered by the resident model under another model's name. That was a deliberate trade to keep the scan off the request path, and it was the wrong one. Cold, the scan now runs once on a thread, bounded so a pathological install falls through rather than hanging the request. Built, the request path still never scans, so the latency fix stands. The watcher freed the slot the moment it saw an error, while Retry-After is thirty times the poll interval, so the client came back to an empty slot and restarted the same failing download instead of being told. Hold the failure on the slot until a retry surfaces it, and let another repo take it after three retry intervals so a client that never returns cannot keep it. The watcher also invalidated on completion, which now lands after finalize_worker_exit's warm and marks that fresh scan stale, pushing a synchronous rescan onto the retry. Removed. _loaded_satisfies lowercased paths as well as aliases, so it returned satisfied before the case-preserving compare below could run. Both now go through one helper: paths compare with normcase, aliases stay case-insensitive. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: an unfinished scan is not absence, and a decided refusal is not a failure Bounding the cold scan then reading the bound as "not here" left the same hole one branch over. A timeout now answers 503 model_indexing with a Retry-After and leaves the warm running. A foreign label sent inside that window is asked to retry rather than falling through, which is a real cost, but the window is one request on an install whose scan exceeds ten seconds and it clears itself, where answering with the wrong weights does not. That uncovered a worse one. Every check here runs inside a broad except whose job is "could not verify, so fall through", so an HTTPException raised in the block was logged as a verification failure and the request was answered by the resident model. Any refusal decided in there was being swallowed. Re-raise it ahead of that handler. Canonicalizing generic labels made them real variant keys, but the matcher still decided on shape, so repo:llama-13b fell past an exact match and fetched llama-7b. Match exactly first, whatever the shape; a quant-shaped suffix that matches nothing is still a miss and never a swap. Marking a catalog alias loaded while publishing the preferred on-disk quant claimed alias:Q4 was loaded while Q8 was serving, and requiring the resident quant to match then made pinning it a 404. Advertise the resident variant when the entry resolves to the resident model. * Studio: keep the asyncio.timeout fallback tests runnable on Python 3.10 Both tests deleted asyncio.timeout to force _wall_clock_timeout down its pre-3.11 branch, but monkeypatch.delattr raises when the attribute is already absent. On Python 3.10, the one version the fallback exists for, there is nothing to delete, so the two tests errored with AttributeError before reaching the code they cover. Passing raising=False makes the deletion a no-op there and leaves the assertions running against the same branch on every version. Every other delattr in the repo already passes raising=False for exactly this reason. Verified with asyncio.timeout removed from the interpreter: the two tests fail with the CI AttributeError before this change and pass after, and the file still runs 89 passed on 3.13 where the deletion is real. * Studio: decide GGUF residency, servability and variant keys by one rule each Four admission and catalog fixes, each closing a gap between two places that were answering the same question differently. The /v1/models catalog asked _resolves_to_resident without llama_only, so a Transformers model live from a directory that also holds GGUF exports marked a GGUF alias loaded and gave it a GGUF quant. The usage examples then pinned alias:quant that nothing could serve with switching off. Every entry in that loop is advertised as GGUF, so residency there is llama.cpp residency. The busy probe accepted any .gguf sibling while admission excludes mmproj, MTP drafters and big-endian builds. A repo holding only companions is not downloadable, so it was held at model_download_busy for the length of an unrelated download instead of falling through to the resident model as it does when no download is running. It now reuses _gguf_variants, the same filter. split_model_ref refused any slash-bearing suffix, but an unrecognized GGUF below a subdirectory keys on its path (build/llama-13b), which is_valid_gguf_variant allows and the catalog advertises. Pinning such a variant could not parse, so only the default-ranked one was reachable. A slash-bearing suffix is now a variant exactly when a real Hub repo precedes it, which still leaves C:/models/x.gguf a path rather than a quant. The usage examples treated a downloaded-but-unloaded model as runnable only under auto-switch, but a standalone UNSLOTH_MODEL_IDLE_TTL reloads exactly what it freed on the next request. The panel hid runnable examples after an idle unload. Tracked apart from auto-switch, because the stash restores the stored checkpoint only and never an arbitrary catalog entry. Also stub the index walk in the three cold-index tests that missed it: a real multi-root scan inside the cold-wait budget made them time out into a 503 under load rather than assert what they are there for. One of them flaked locally. Verified each fix is load-bearing by reverting it and watching its test fail. Backend CI command: 10195 passed, 0 failed. tsc -b and the frontend build clean. * Studio: bound the Hub admission probes and stop guessing at nested model paths Three review fixes plus a test-isolation one. _resolves_to_resident matched on a path prefix, so two separately indexed models that nest (/models/A alongside /models/A/sub/B) both satisfied it: loading B made a request for A resident and answered it with B's weights, and the catalog marked A loaded. A prefix match now counts only when no catalog entry sits deeper, which is the innermost indexed model that actually owns the file. With nothing indexed there is no nesting to tell apart, so the directory-to-weights match this exists for is unchanged. auth_check and hf_hub_download take no timeout of their own, and both ran while the provisional single-flight slot was held, so an unresponsive Hub stalled the request far past the metadata budget and reported every other model busy for the duration. Both are bounded now. Each default errs the safe way: an unchecked repo is not a cleared one, so the custom-code probe refuses on timeout, while a slow gated-repo check stays inconclusive because the download's own auth is the real gate. The usage examples caught a failed refresh into an empty catalog and a disabled auto-switch, which made a transient error authoritative and blanked every example while the model was still servable. The catalog is deliberately tri-state; a failure now keeps the last answer and retries. Also start the backend tests from a built, empty model index. Stubbing only the background warm still left the cold path walking real caches synchronously inside the admission wait, so on a large install a test asserted against a 503 "still indexing" instead of its subject. _build_index is untouched, so the tests that call it directly still exercise the real walk. Verified each fix is load-bearing by reverting it and watching its test fail. tsc -b clean. Backend CI command green apart from two failures reproduced only on this box (a real model-dir scan and an orphan-process cleanup), neither touched by this PR; staging CI is the gate for those. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |