Repository navigation
feat(studio): add model picker and reload shortcut to API monitor - #11223
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add ModelSelector and a reload previous model shortcut to the API monitor header toolbar. Reuses the shared Chat runtime to avoid dead-end states when the resident model crashes or is unloaded. Fixes unslothai#11189
e2278b9 to
01399f3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
NilayYadav
left a comment
There was a problem hiding this comment.
Review + reproduction evidence
Pinned review: 7fa1f1ffc8 (PR head b8dcb645d8 plus two fixes I pushed). Three findings on the new controls, all reproduced by hand and fixed in 36dcad8366 and 7fa1f1ffc8.
I ran the flow on fresh upstream main (496acd8f) and on main plus this branch, installed with install.sh --local --no-torch on Modal (CPU, llama.cpp), Chromium 1440x900, unsloth/Qwen3-0.6B-GGUF Q4_K_M and unsloth/SmolLM2-135M-Instruct-GGUF Q4_K_M:
- main: the API page toolbar has Pause, Unload, Refresh, Clear log and API settings only. With no model loaded there is no way to load one from this page.
- PR head, core flow works: pick Qwen3 Q4_K_M in the new picker, the monitor reports
unsloth/Qwen3-0.6B-GGUF:Q4_K_M · 40,960 ctx; Unload givesNone; Reload (tooltipReload unsloth/Qwen3-0.6B-GGUF · Q4_K_M (no model loaded)) brings the same quant back.
Findings on the PR head, each reproduced with the same steps:
- The picker eject unloads the wrong model after an API auto-switch. With "Switch model by request" on, load Qwen3 from the picker, then an API client requests SmolLM2. The trigger shows SmolLM2, but its eject called
ejectModel(), which unloads Chat's stale selection. Server log:Unloaded model: unsloth/Qwen3-0.6B-GGUF(a 200 no-op), and/api/inference/statusstill reportedactive_model: unsloth/SmolLM2-135M-Instruct-GGUF15 s later. The trigger eject now goes through the page's existingunloadResidentpath, the same one the Unload button uses. After the fix, the same steps leave nothing loaded. - The per-row Eject unloads the wrong model with several models kept loaded. With "Keep multiple models loaded" on and Qwen3 plus SmolLM2 resident, clicking
Eject unsloth/Qwen3-0.6B-GGUFunloaded SmolLM2 (Unloaded GGUF model: .../models--unsloth--SmolLM2-135M-Instruct-GGUF/...) and left Qwen3 loaded. The wrapper dropped the row's id. It now forwards it, and the same click unloads Qwen3 (Unloaded GGUF model: unsloth/Qwen3-0.6B-GGUF) and keeps SmolLM2. - The trigger showed the quant twice and could show a stale one. It read
Qwen3-0.6B-GGUF:Q4_K_M GGUF · Q4_K_M. After an API switch to Q8_0 it readQwen3-0.6B-GGUF:Q8_0 GGUF · Q4_K_M, because the chip came from Chat's store. The id and the quant are now both taken from the monitor's<id>:<quant>, so it readsQwen3-0.6B-GGUF GGUF · Q4_K_Mand the loaded row is matched in the list.
I also made the load call match Chat's picker: it now forwards loadId, isVision and downloadPresentation, and passes previousConfig, so a failed or cancelled switch rolls back to the resident model's settings. Reload passes previousConfig too.
- Typecheck passes on
mainplus this branch. eslint is clean on the new files. biome reports the same 15 diagnostics onapi-monitor-page.tsxas onmain. tests/api-monitor-reload-last-model.test.ts: 6/6 pass, including a newsplitActiveModelcase. On the Actions A/B mirrors the file passes on the PR side and fails withERR_MODULE_NOT_FOUNDwith the source reverted. The two red jobs on both sides are not from this PR: locale parity on thesettings.resources.*.modelsDiskkeys, and the Windowssystem-status-verdict.test.ts"no tile fabricates a usage percentage" case.- Frontend only, no backend change. The controls render only on the API page, not in the floating monitor overlay.
Two further review rounds on a mirror. The first raised three items: the duplicate quant, the dropped loadId and the missing previousConfig. All three are fixed above. The second, on 7fa1f1ffc8, raised two:
- Set
throwOnErroron the load calls: not taken.selectModelalready reports failures itself (setModelsErrorand the error toast). With the flag, a pick replaced by a newer one would also throw "Model selection was superseded by a newer choice." and show up as a false inline error. - The initial
refresh({ includeLoras: false })skipslistLoras(). If someone opens/api-monitorbefore Chat has refreshed the store, trained LoRAs are missing from this picker. I could not reproduce it here (no trained LoRA on a CPU box), so I am leaving it as a suggestion:
refresh();
(api-model-load-controls.tsx, the first line of the mount effect.)
Conclusion: the gap in #11189 is real on main, and the PR closes it. With the fixes above, load, unload, reload and both eject paths behave correctly on current main. Not covered by hand: an external provider selected in Chat (no provider key on the test box), and Windows/macOS beyond the Actions unit jobs.
This comment has been minimized.
This comment has been minimized.
…the staged pick on any eject
Review rounds 11-19Reviewed on a mirror PR from 9 findings fixed, 5 declined with the reason above. |
…elling (#12858) #11223 added modelLoading to the unload button's disabled= (no manual unload while a model is loading), and the contract test pinned the old string, so test_monitor_can_unload_the_loaded_model fails on main. The test now reads that button's disabled= as a set of || terms and requires unloading and !data?.active_model to stay among them.
## Problem Five checks fail on main, so the `Repo tests (CPU, studio)`, `Repo tests (CPU, rest)` and `workflow-trigger lint` shards are red on every open PR (for example #14472, #14482, #14488, #14490 and #14494). All five reproduce on main `4d7ebf5cea`. | Test | Introduced by | Fault | |---|---|---| | `tests/security/test_lint_risky_loaders.py::test_the_tree_matches_the_baseline` | #14437 | Baseline. Two new dynamic-import sites: `import_module` in `studio/backend/core/rag/embeddings.py`, and a third `import_from_string` in `unsloth/models/sentence_transformer.py` (`_require_word_tokenizer`). Both are safe, see below. | | `tests/studio/test_tauri_branding_contract.py::test_desktop_surfaces_do_not_restore_studio_branding` | #14473 | Product. "Studio" became "Unsloth Studio" in the Markdown import errors and the Tauri webview reject message. The desktop contract allows "Unsloth" there, never "Unsloth Studio". "Unsloth" meets both #14473's rule (no bare "Studio") and this one. | | `tests/studio/test_usage_examples_model_source_contract.py::test_auto_switch_section_sits_above_the_usage_examples` | #14455 | Test. The redesign moved the usage examples above the monitor link on purpose, and its own `api-settings-tab.test.ts` pins that. | | `tests/studio/test_usage_examples_model_source_contract.py::test_monitor_can_unload_the_loaded_model` | #14455 | Test. Unloading moved from a standalone button on the page into the model picker's eject (`ApiModelLoadControls`). The `/unload` path itself is unchanged. | | `tests/studio/test_settings_compact_overflow_contract.py::test_api_monitor_entries_and_expanded_text_can_shrink` | #14455 | Test. Entry rows are now rounded cards (no `border-b`), and the prompt/reply box's `rounded-lg` became conditional, so the class literals the test matched are gone. The overflow guards themselves are still in the source. | ## Fix - **Risky loaders:** two baseline entries with reasons. - `embeddings.py` imports only the three `sentence_transformers.models` paths written out in a literal tuple, to wrap their `import_from_string` with the trust gate. - `_require_word_tokenizer` imports only after its caller has refused any `tokenizer_class` outside `sentence_transformers.*`, and only dotted identifiers. - **Branding:** "Unsloth Studio" to "Unsloth" in those seven Markdown import errors and the Rust reject message. `conversation-markdown-import.test.ts` now matches `/Unsloth Markdown/`. - **#14455 contracts:** rewritten to guard what the redesign keeps: - the auto-switch section still follows the monitor link, and the usage examples come before both; - the page passes `unloadActiveModel` to the load controls as `onUnloadActive`, and eject is withheld while `busy` (`modelLoading || unloading`, #11223's rule); - the `/unload` call still reads the internal id from status; - entry rows keep `w-full min-w-0`; - the prompt/reply box keeps `max-h-72 overflow-auto break-words`; - the model id keeps `min-w-0 break-all`. ## Verification - The four Python files: 78 passed (5 failed on main). - `studio/frontend/tests/conversation-markdown-import.test.ts`: 20 passed. - Sabotage, each run against the rewritten tests, each failing exactly one test: - removing `min-w-0` from the entry row, or `break-words` from the text box: the overflow test fails; - dropping `unloading` from `busy`, or passing `onEject` while busy: the unload test fails; - putting the auto-switch section above the monitor link: the order test fails.


Summary
Adds a model picker and a Reload action to the Studio API monitor toolbar. Previously the API page only offered Unload: once a model was unloaded or the backend crashed, the page showed "No model loaded" and loading one meant going to Chat and back.
Fixes #11189
Changes
studio/frontend/src/features/api-monitor/components/api-model-load-controls.tsx:ApiModelLoadControls, the sharedModelSelectorplus a Reload button, mounted in the API monitor toolbar.selectModel(context, quant, GPU placement, trust, running-chat confirm, rollback to the resident config on a failed or cancelled switch); NPU picks go throughloadNpuModel.<id>:<quant>), so it follows API auto-switches. Its eject unloads the resident model through the page's own Unload path; per-row Eject unloads that row's model.studio/frontend/src/features/api-monitor/reload-last-model.ts: resolves Reload's target from the durable last-local-load record (including a quant-less direct.gguffile), not the bounded, clearable monitor ring buffer.chat,hub,model-pickerfeature indexes: export the helpers the controls use.studio/frontend/tests/api-monitor-reload-last-model.test.ts: reload target resolution and delegation.Testing
cd studio/frontend npm run typecheck node --experimental-strip-types --test tests/api-monitor-reload-last-model.test.tsEvidence (head
403d0137b)Fresh Studio, CPU llama.cpp,
unsloth/Qwen3-0.6B-GGUFQ4_K_M. Studio regression A/B against the merge base: no functional regression; the only visual change is the API monitor toolbar.API monitor toolbar, merge base vs this branch:
Picker opened from the API page:
Loaded from the API page picker; the trigger names the quant once:
After Unload, Reload still points at the last model:
After Reload, the same model and quant are back:
"Switch model by request" on: an API request for
unsloth/SmolLM2-135M-Instruct-GGUF:Q4_K_Mswitches the resident model and the trigger follows:The trigger eject then unloads the resident SmolLM2: