Feat/model picker per model config v2 - #7207
Conversation
Relocate model-selector + its support files from components/assistant-ui into a self-contained features/model-picker feature (own barrel), mirroring the modular Hub layout. Pure move + import repoint; no behaviour change.
Superset PerModelConfig (customContextLength, kvCacheDtype, speculativeType, specDraftNMax, tensorParallel, chatTemplateOverride, trustRemoteCode) persisted to localStorage (unsloth_model_configs) with schema versioning + LRU budget. KV-dtype and speculative value sets match main's sidebar (no q4_0/ngram-simple). Reuses features/hub/lib/model-identity for normalization; adds storage-key layer and applyPerModelConfigToRuntime (sets tensorParallel, which the old PR omitted).
New studio/backend/picker package (schemas/service/routes) mounted at /api/picker:
- POST /api/picker/validate-chat-template (Jinja syntax validation, no false positives)
- GET /api/picker/chat-template/{model_name} (default template from tokenizer_config.json,
reusing get_cache_path/resolve_cached_repo_id_case; graceful null, no model-code exec)
Frontend api/templates.ts client + hooks/use-model-defaults lazy cache. No backend
changes to the existing inference load route (per-model load fields already supported).
Picker now sources cached + local models from useHubInventory (the Hub's shared store) via a thin adapter, replacing its own /api/models/* fetchers + module caches. Hub, download manager, and picker now share one source of truth, so completed downloads reflect in the picker automatically. Partial/live-download rows are filtered from the cached lists (unchanged rendering). Local naming/search preserved via additive LocalInventoryRow modelId/displayName. Variant expander, scan-folder management, recommended-fit, search, external providers untouched. Known minor: cached 'Downloaded date' sort tiebreak degrades to alphabetical (hub cached rows carry no mtime); default 'recent' (load-time) sort preserved.
Picking a (non-external) model now opens an in-picker config view built from main's current load controls (context length, KV cache dtype, speculative decoding, draft tokens, tensor parallel) plus a chat-template editor backed by the picker validate/default endpoints. 'Remember for this model' persists the config per model+variant; Run forwards the config to the existing load flow via meta.config. External models bypass the step. Two-view orchestration lives in model-selector (single interception point); pickers.tsx call sites untouched. trustRemoteCode dropped from PerModelConfig to preserve main's per-load consent.
handleCheckpointChange threads meta.config into the selection; stageOrLoad and the autoload/Hub-run paths now apply the picker config (explicit pick or saved remembered config) via applyPerModelConfigToRuntime before staging/loading, with keepSpeculative set so a remembered speculative mode survives the model switch. Replaces the old remembered-load-settings seeding (resolveInitialConfig now the single source). SelectedModelInput carries config.
The load knobs (context, KV cache, speculative, draft tokens, tensor parallel) and the chat-template editor now live only in the picker config step. The sheet's Model section keeps the staged Load/Cancel flow (config is applied at pick time); sampling params, system prompt, and RAG are unchanged. Deletes the superseded remembered-load-settings module + the store's applyRememberedLoadSettings action, removes the now-dead sheet state/imports, and points the settings reset at unsloth_model_configs. Delete-cleanup deferred (stale config is LRU-capped).
…odel section The downloaded-variant gear (ModelLoadSettingsAction) staged a model straight into the right-sidebar Run-settings flow -- the old 'configure before load' path now fully replaced by the in-picker config step. Removed the gear + its component. Also gate the sheet's 'Model' section to staged picks only (pendingSelection): after the load-knob strip its content is staged-only, so it was rendering an empty section header whenever a model was merely loaded.
…bled After the load-config UI moved into the picker, the store's per-model setters (setKvCacheDtype/setSpeculativeType/setSpecDraftNMax/setTensorParallel/ setCustomContextLength/setChatTemplateOverride) had zero callers (applyPerModelConfigToRuntime writes via setState), and the sheet's modelControlsDisabled was unreferenced. Verified dead across the whole tree.
Root cause: with Settings > Chat > 'Load on selection' turned OFF, the config step's load went down the deferred-staging path -- opening the right sidebar with '<model> is staged, not loaded yet / Choose Load model'. The in-picker config step IS the deliberate load action, so its Load now loads immediately (or downloads + auto-loads when not cached) regardless of the toggle. Renamed the button 'Run model' -> 'Load model' to match. Native/dropped picks still honor the toggle.
…nly load flow The in-picker config step (and the Hub Run button) now fully supersede the old stage-to-sidebar flow, so the Load-on-selection toggle is removed everywhere: - chat stageOrLoad: every pick loads immediately, or downloads + auto-loads when not cached (the previous default behaviour, now universal). - hub Run: drops the stage branch; downloaded GGUFs load directly with their saved per-model config (no collision with the chat config step — both end at selectModel). - store: removed loadOnSelection field/setter/key/default; Settings>Chat toggle and its settings-reset entry removed. - staged sidebar section is now a download-progress view (auto-loads on completion). No manual staging remains; stageModel is used only for background auto-load downloads.
…through config flow Read the embedded tokenizer.chat_template from GGUF files (read_gguf_chat_template in gguf_metadata) and use it as the per-model default. Plumb gguf_variant through the picker service, /api/picker/chat-template route, frontend templates API, and use-model-defaults so the right variant's template is fetched. Also refine the picker config-page/model-selector wiring, drop the dead ggufNativeContextLength runtime path, and add the per-model-config storage keys to the settings prefs export.
…e it has no effect Resolve the default chat template for safetensors models: prefer the modern chat_template.jinja, fall back to the tokenizer_config.json chat_template field, then chat_template.json (multimodal processor), then the GGUF embedded template. Applied to local dirs, the HF cache snapshot scan, and the HF remote fetch. Hide the chat-template editor in the picker for safetensors models — the override is only applied at load by the GGUF/llama.cpp backend, so editing it on safetensors currently has no effect. GGUF keeps the editor. Nothing removed; the dialog stays for when the safetensors apply path is wired up in a later branch.
…ceeds Set unsloth_model_configs_migrated only once writeMap confirms the migrated map persisted, so a quota/storage failure no longer marks migration done and silently drops the user's pre-existing remembered settings — the next load retries.
for more information, see https://pre-commit.ci
Sync 59 upstream commits. Resolve the lone pickers.tsx conflict by fully adopting upstream quick-eject (unslothai#6654) into the refactored picker: thread onEject + RemoveCircleIcon + floating pill through HubModelPicker, wire it from model-selector, and guard the footer eject button to non-hub tabs so the hub tab shows only the pill.
…ig' into feat/model-picker-per-model-config # Conflicts: # studio/backend/picker/service.py
Apply remembered per-model configs consistently from picker and Hub loads, keep default configs from overriding standing speculative settings, add config access for direct local GGUF files, and support saving or forgetting active model settings without a reload.
for more information, see https://pre-commit.ci
|
@codex review |
1 similar comment
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54568b8dad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for rel in _JINJA_TEMPLATE_PATHS: | ||
| template = _download_text(rel) | ||
| if template and template.strip(): | ||
| return template |
There was a problem hiding this comment.
For an uncached Hub repo with a chat_template.jinja between 64 KiB and 4 MiB plus a valid tokenizer_config.json, this returns the oversized Jinja immediately. The route then drops it at its 64 KiB response cap, so it never falls through to the valid tokenizer template (unlike the local path, which skips oversized Jinja files). Apply MAX_CHAT_TEMPLATE_BYTES to raw Jinja downloads or continue searching when its extracted template exceeds that limit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 774a860. The remote path only bounded raw chat_template.jinja downloads at MAX_TEMPLATE_METADATA_BYTES (4 MiB) and returned the first non-empty Jinja unconditionally, while the route drops any template over MAX_CHAT_TEMPLATE_BYTES (64 KiB), so an uncached repo with a 64 KiB-4 MiB Jinja plus a valid smaller tokenizer_config.json returned no template. It now gates the extracted Jinja on MAX_CHAT_TEMPLATE_BYTES and continues searching when over the cap, matching the local _chat_template_from_jinja_file gate; the 4 MiB bound stays for JSON files that embed a small template. Added a regression test in tests/test_picker_service.py.
The remote chat-template resolver bounded raw chat_template.jinja downloads only by MAX_TEMPLATE_METADATA_BYTES (4 MiB), then returned the first non-empty Jinja unconditionally. The picker route drops any template larger than MAX_CHAT_TEMPLATE_BYTES (64 KiB), so an uncached repo whose chat_template.jinja sits between 64 KiB and 4 MiB returned no template at all, even when a valid smaller tokenizer_config.json template existed. The local path already skips oversized .jinja files and falls through. Gate the extracted Jinja on MAX_CHAT_TEMPLATE_BYTES and continue searching when it exceeds the cap, matching _chat_template_from_jinja_file. The 4 MiB download bound stays for JSON files that merely embed a small template. Adds a regression test that a big Jinja plus a valid tokenizer config resolves to the tokenizer template.
for more information, see https://pre-commit.ci
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Status from my side: this is ready. Codex approves the current head (e493c30) and every inline item is addressed. All PR-specific CI is green. The only red checks are the three Core (HF=...) matrices, and those are a pre-existing main-wide failure, not something this branch introduces:
So the Core reds are not merge-blocking for this branch; they will clear once unsloth_zoo main is realigned. |
Resolves the pickers.tsx import conflict from unslothai#7266 (malformed HF token must not empty the Recommended list). The PR consolidated the picker imports onto the @/features/hub barrel, so the resolution keeps that structure and adds main's new hfApiToken there (also re-exported from the hub barrel) rather than reintroducing the deep per-module import paths. The accessToken = hfApiToken( hfToken) fix from unslothai#7266 merges in unchanged.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The v1->v2 localStorage migration (unsloth_load_settings -> unsloth_model_configs) runs on every store read, so it must migrate exactly once and never re-run, duplicate, or clobber a newer per-model config on a reload or restart. That was covered only by a manual proof, so add durable guards: - Source-contract test pinning the three idempotency layers (the in-memory legacyMigrationChecked guard, the persistent unsloth_model_configs_migrated flag set in every terminal branch, and the non-overwriting Object.hasOwn merge-skip) plus the readMap invocation. Reddens if any layer is dropped. - Playwright model-config E2E: promote the legacy-migration step to a gating check (soft_fail, which gates under the CI STUDIO_UI_STRICT=1) that the migrated value is preserved and the flag is set, then reload again with a fresh legacy seed present and assert the stored key set is unchanged, so a second reload cannot re-migrate, duplicate, or clobber.
for more information, see https://pre-commit.ci
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Resolves the hub-page.tsx import conflict from unslothai#7271 (immediate tab navigation). Keeps the PR's chat + hf-token-store imports and adds main's new hub-feed-store import (isChannelEntryFresh, useHubFeedStore, used by the merged tab-navigation body); drops main's duplicate hf-token-store import since the PR already imports hfApiToken/useHfTokenStore from ./stores.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
5 participants
Reintroduce model picker per model config
This resubmits the model picker per model config feature from #6647. That PR was squash merged as 8cbdfbe and then reverted on main in 1c7bce4, so the feature is currently not in main.
This branch is the same feature branch including danielhanchen's follow up fixes, plus four additional bug fixes: