diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index c2faad2011..f06de5aff9 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -41,6 +41,8 @@ from utils.openai_auto_switch_settings import ( DEFAULT_OPENAI_AUTO_DOWNLOAD_ENABLED, DEFAULT_OPENAI_AUTO_SWITCH_ENABLED, MAX_GPU_ID, + PARALLEL_SLOTS_MAX, + PARALLEL_SLOTS_MIN, get_auto_unload_idle_seconds, get_auto_unload_keep_kv, get_model_overrides, @@ -170,6 +172,11 @@ class ModelOverridePayload(BaseModel): kv_cache_dtype: Optional[str] = Field(default = None, max_length = 32) speculative_type: Optional[str] = Field(default = None, max_length = 32) spec_draft_n_max: Optional[int] = Field(default = None, ge = 1, le = 16) + # Parallel decode slots (llama-server --parallel), GGUF-only like the picker. + # None follows the server-wide default set at launch. + n_parallel: Optional[int] = Field( + default = None, ge = PARALLEL_SLOTS_MIN, le = PARALLEL_SLOTS_MAX + ) tensor_parallel: bool = False # Validated in bytes below, not by max_length: pydantic counts characters, so a # multi-byte template would pass here and be dropped by the UTF-8 normalizer. @@ -435,6 +442,7 @@ def update_openai_auto_switch_override( kv_cache_dtype = payload.kv_cache_dtype, speculative_type = payload.speculative_type, spec_draft_n_max = payload.spec_draft_n_max, + n_parallel = payload.n_parallel, tensor_parallel = payload.tensor_parallel, chat_template_override = payload.chat_template_override, gpu_memory_mode = payload.gpu_memory_mode, diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index e4124d1383..883ee47e3b 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4258,6 +4258,82 @@ def test_model_override_load_kwargs_gates_gpu_placement_on_gguf(): LoadRequest(model_path = "unsloth/B-GGUF", **gguf) +def test_saved_parallel_slots_reach_an_api_load(monkeypatch): + # Parallel decode slots are a per-model setting the picker sends on every GGUF + # load. Without them here an API auto-switch of the same model silently falls + # back to the server-wide --parallel default, and llama_extra_args cannot stand + # in for it: --parallel is on the managed denylist. + backend = _FakeBackend(None) + rec = _LoadRecorder(backend) + _wire( + monkeypatch, + enabled = True, + resolves_to = ("unsloth/B-GGUF", "Q4_K_M", "unsloth/B-GGUF"), + backend = backend, + recorder = rec, + ) + monkeypatch.setattr(settings, "get_model_override", lambda mid: {"n_parallel": 8}) + + _run_hook("unsloth/B-GGUF") + assert rec.calls[0].n_parallel == 8 + + +def test_parallel_slots_are_stored_and_gated_on_gguf(): + override = settings.normalize_model_override({"n_parallel": 8}) + assert override == {"n_parallel": 8} + # Blank, out of range and non-integer all mean "follow the server-wide default". + for bad in (None, 0, -1, settings.PARALLEL_SLOTS_MAX + 1, "many", True): + assert "n_parallel" not in settings.normalize_model_override({"n_parallel": bad}) + + gguf = settings.model_override_load_kwargs(override, is_gguf = True) + assert gguf["n_parallel"] == 8 + # A safetensors load has no llama-server slots, exactly as the picker gates it. + assert "n_parallel" not in settings.model_override_load_kwargs(override, is_gguf = False) + LoadRequest(model_path = "unsloth/B-GGUF", **gguf) + + +def test_override_route_persists_parallel_slots(monkeypatch): + # The mirror the picker writes has to carry the field, or a config whose only + # change is the slot count saves as an empty entry. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/B-GGUF:Q4_K_M", n_parallel = 8), + "tester", + ) + assert resp.overrides["unsloth/B-GGUF:Q4_K_M"] == {"n_parallel": 8} + + +def test_eviction_cleanup_clears_mirrored_fields_but_keeps_launch_flags(monkeypatch): + # Dropping a local entry to stay inside the browser's storage budget is not the + # user forgetting the model, so the cleanup sends remove=false with no fields. + # That must stop the mirrored settings applying without taking launch flags the + # settings API set and the settings page can neither show nor restore. + import routes.settings as settings_route + + _mock_override_store(monkeypatch) + settings.set_model_override( + "unsloth/B-GGUF:Q4_K_M", + llama_extra_args = ["--flash-attn"], + custom_context_length = 32768, + kv_cache_dtype = "q8_0", + ) + resp = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/B-GGUF:Q4_K_M", remove = False), + "tester", + ) + assert resp.overrides["unsloth/B-GGUF:Q4_K_M"] == {"llama_extra_args": ["--flash-attn"]} + + # Nothing server-owned left, so the row goes rather than lingering empty. + settings.set_model_override("unsloth/C-GGUF:Q4_K_M", custom_context_length = 32768) + gone = settings_route.update_openai_auto_switch_override( + settings_route.ModelOverridePayload(model_id = "unsloth/C-GGUF:Q4_K_M", remove = False), + "tester", + ) + assert "unsloth/C-GGUF:Q4_K_M" not in gone.overrides + + def test_auto_switch_prefers_variant_qualified_override(monkeypatch): # Settings are per quant, so Q4_K_M and Q8_0 of one repo are separate entries # and the bare repo id is only the fallback. diff --git a/studio/backend/tests/test_parallel_slots_per_load.py b/studio/backend/tests/test_parallel_slots_per_load.py index f4f2d31c6f..87fd2091cb 100644 --- a/studio/backend/tests/test_parallel_slots_per_load.py +++ b/studio/backend/tests/test_parallel_slots_per_load.py @@ -152,6 +152,14 @@ def test_frontend_mirror_matches_shared_bounds(): assert (int(low.group(1)), int(high.group(1))) == (PARALLEL_MIN, PARALLEL_MAX) +def test_override_mirror_matches_shared_bounds(): + # The API auto-switch override map mirrors the bounds rather than importing them: + # llama_server_args owns the extra-args allow-list that module stays out of. + from utils.openai_auto_switch_settings import PARALLEL_SLOTS_MAX, PARALLEL_SLOTS_MIN + + assert (PARALLEL_SLOTS_MIN, PARALLEL_SLOTS_MAX) == (PARALLEL_MIN, PARALLEL_MAX) + + def test_preset_model_reuses_shared_bounds(): # Bounds drifting from PARALLEL_MIN/MAX would 422 valid presets on every sync. from routes.chat_history import ChatPresetLoadConfig diff --git a/studio/backend/utils/openai_auto_switch_settings.py b/studio/backend/utils/openai_auto_switch_settings.py index 936da15707..07df0a584d 100644 --- a/studio/backend/utils/openai_auto_switch_settings.py +++ b/studio/backend/utils/openai_auto_switch_settings.py @@ -285,6 +285,12 @@ VALID_SPECULATIVE_TYPES = frozenset( MTP_SPECULATIVE_TYPES = frozenset({"mtp", "mtp+ngram", "draft-mtp"}) VALID_GPU_MEMORY_MODES = frozenset({"auto", "manual"}) +# Mirrors PARALLEL_MIN/MAX in core/inference/llama_server_args.py (the LoadRequest +# n_parallel bounds). Mirrored rather than imported: that module owns the extra-args +# allow-list this one must stay out of. test_parallel_slots_per_load.py pins them together. +PARALLEL_SLOTS_MIN = 1 +PARALLEL_SLOTS_MAX = 64 + MAX_SEQ_LENGTH_CEILING = 1048576 MAX_CHAT_TEMPLATE_OVERRIDE_BYTES = 65_536 # Highest device index a stored gpu_ids entry may name. Also bounds how many @@ -350,6 +356,14 @@ def normalize_model_override(payload: dict[str, Any]) -> dict[str, Any]: if spec_draft_n_max: entry["spec_draft_n_max"] = spec_draft_n_max + # Blank means "follow the server-wide --parallel default", which is also what an + # out-of-range value falls back to. + n_parallel = _bounded_int( + payload.get("n_parallel"), minimum = PARALLEL_SLOTS_MIN, maximum = PARALLEL_SLOTS_MAX + ) + if n_parallel: + entry["n_parallel"] = n_parallel + if _coerce_bool(payload.get("tensor_parallel")): entry["tensor_parallel"] = True @@ -446,6 +460,9 @@ def model_override_load_kwargs(override: dict[str, Any], *, is_gguf: bool) -> di kwargs[target] = override[source] if is_gguf: + # Slots are a llama-server flag, and the picker sends them for GGUF only. + if override.get("n_parallel") is not None: + kwargs["n_parallel"] = override["n_parallel"] if override.get("gpu_memory_mode") is not None: kwargs["gpu_memory_mode"] = override["gpu_memory_mode"] if override.get("gpu_layers") is not None: diff --git a/studio/frontend/src/features/api-monitor/components/saved-model-settings.tsx b/studio/frontend/src/features/api-monitor/components/saved-model-settings.tsx index 492f158826..1ede884013 100644 --- a/studio/frontend/src/features/api-monitor/components/saved-model-settings.tsx +++ b/studio/frontend/src/features/api-monitor/components/saved-model-settings.tsx @@ -32,6 +32,9 @@ function describeOverride(override: ApiModelOverride): string[] { : `spec ${override.speculative_type}`, ); } + if (override.n_parallel) { + parts.push(`${override.n_parallel} parallel slots`); + } if (override.tensor_parallel) { parts.push("tensor parallel"); } diff --git a/studio/frontend/src/features/model-picker/api/model-overrides.ts b/studio/frontend/src/features/model-picker/api/model-overrides.ts index 3f58bbc66b..f7535718d7 100644 --- a/studio/frontend/src/features/model-picker/api/model-overrides.ts +++ b/studio/frontend/src/features/model-picker/api/model-overrides.ts @@ -34,6 +34,8 @@ export interface ApiModelOverride { // biome-ignore lint/style/useNamingConvention: API schema spec_draft_n_max?: number; // biome-ignore lint/style/useNamingConvention: API schema + n_parallel?: number; + // biome-ignore lint/style/useNamingConvention: API schema tensor_parallel?: boolean; // biome-ignore lint/style/useNamingConvention: API schema chat_template_override?: string; @@ -102,6 +104,10 @@ export function toApiOverride(config: PerModelConfig | null): ApiModelOverride { if (config.specDraftNMax && config.specDraftNMax > 0) { payload.spec_draft_n_max = config.specDraftNMax; } + // Blank follows the server-wide --parallel default, which is the app default here. + if (config.nParallel && config.nParallel > 0) { + payload.n_parallel = config.nParallel; + } if (config.tensorParallel) { payload.tensor_parallel = true; } @@ -143,6 +149,15 @@ export interface PutModelOverrideOptions { * without the server losing anything it holds. */ fillAbsentFields?: boolean; + /** + * Clear the fields this UI mirrors but leave the server's own launch flags alone. + * + * Dropping a local entry to stay inside the storage budget is not the user asking + * to forget that model, so it must not take `llama_extra_args` the settings API + * set and the settings page can neither show nor restore. The route still drops + * the row outright once nothing is left in it. + */ + keepLaunchFlags?: boolean; } export async function putModelOverride( @@ -195,11 +210,13 @@ async function sendModelOverride( // Say which operation this is: an all-default save carries no fields, which is // shape-identical to "forget this model", and guessing wrong wipes launch flags // the UI cannot show or restore. - remove: config === null, + remove: config === null && !options?.keepLaunchFlags, // Launch flags have no UI control, so the backend preserves them when omitted. // Forgetting means forgetting all of it, so that path sends an explicit []. - // biome-ignore lint/style/useNamingConvention: API schema - ...(config === null ? { llama_extra_args: [] } : {}), + ...(config === null && !options?.keepLaunchFlags + ? // biome-ignore lint/style/useNamingConvention: API schema + { llama_extra_args: [] } + : {}), ...toApiOverride(config), }), }); @@ -222,8 +239,9 @@ export function syncModelOverride( modelId: string, ggufVariant: string | null | undefined, config: PerModelConfig | null, + options?: PutModelOverrideOptions, ): void { - void putModelOverride(modelId, ggufVariant, config).catch( + void putModelOverride(modelId, ggufVariant, config, options).catch( (error: unknown) => { console.warn( "Failed to mirror model settings to the server; an API load of this model will use defaults.", diff --git a/studio/frontend/src/features/model-picker/components/model-config-page.tsx b/studio/frontend/src/features/model-picker/components/model-config-page.tsx index 50a93be1e5..d0c402279d 100644 --- a/studio/frontend/src/features/model-picker/components/model-config-page.tsx +++ b/studio/frontend/src/features/model-picker/components/model-config-page.tsx @@ -949,8 +949,12 @@ export function ModelConfigPage({ } // Saving can push the local map over budget and drop other models, whose server // entries would keep being applied with nothing in the UI able to forget them. + // Not a Forget though: the user never asked to drop these, so only the mirrored + // fields go and launch flags set through the API stay. for (const dropped of evicted) { - syncModelOverride(dropped.modelId, dropped.ggufVariant, null); + syncModelOverride(dropped.modelId, dropped.ggufVariant, null, { + keepLaunchFlags: true, + }); } if (effectivePersistenceOnly) { if (saveFailed) { diff --git a/tests/studio/test_model_picker_contracts.py b/tests/studio/test_model_picker_contracts.py index 41a2d5881f..1306189561 100644 --- a/tests/studio/test_model_picker_contracts.py +++ b/tests/studio/test_model_picker_contracts.py @@ -693,6 +693,37 @@ def test_parallel_slots_setting_wired_end_to_end(): assert "key={modelConfigInstanceKey(modelId, ggufVariant, loadedConfig)}" in sidebar +def test_parallel_slots_reach_an_api_load_through_the_server_mirror(): + """The server mirror is the hop an OpenAI-compatible auto-switch load reads, + and it is the browser's only way to express a per-model setting to a load no + browser makes. A slot count missing from it silently reverts that load to the + server-wide --parallel default, and llama_extra_args cannot stand in because + --parallel is denylisted. A config whose only change is the slot count also + serializes to an empty payload, so the one-time backfill sends nothing and + still marks itself done.""" + api = " ".join(_read("features/model-picker/api/model-overrides.ts").split()) + assert "n_parallel?: number;" in api + assert ( + "if (config.nParallel && config.nParallel > 0) { payload.n_parallel = config.nParallel; }" + in api + ) + # The monitor lists what a remote load applies, so an entry holding only slots + # must not read as "App defaults". + monitor = " ".join(_read("features/api-monitor/components/saved-model-settings.tsx").split()) + assert "if (override.n_parallel) {" in monitor + + route = (WORKDIR / "studio" / "backend" / "routes" / "settings.py").read_text(encoding = "utf-8") + assert "n_parallel: Optional[int] = Field(" in route + assert "n_parallel = payload.n_parallel," in route + store = ( + WORKDIR / "studio" / "backend" / "utils" / "openai_auto_switch_settings.py" + ).read_text(encoding = "utf-8") + assert 'entry["n_parallel"] = n_parallel' in store + # GGUF-only, like the picker: a safetensors load has no llama-server slots. + gguf_block = store.split(" if is_gguf:", 1)[1] + assert 'kwargs["n_parallel"] = override["n_parallel"]' in gguf_block + + def test_parallel_slots_control_cleared_when_the_load_never_sent_them(): """`nParallel` is the editable control ("blank = follow the server default") and `loadedNParallel` the rollback baseline. A success path that sends no @@ -931,13 +962,28 @@ def test_evicted_local_configs_drop_their_server_overrides(): """savePerModelConfig evicts older models when the map exceeds its budget. Those models keep a server override that API loads still apply, with nothing left in the UI showing it or able to forget it, so eviction has to propagate. + + It propagates as a clear, not a Forget: saving one model silently drops the + oldest OTHER model, and sending the full remove would also take llama_extra_args + that only the settings API writes and no UI can show or restore. """ src = " ".join(_read("features/model-picker/components/model-config-page.tsx").split()) assert "const evicted: { modelId: string; ggufVariant: string | null }[] = [];" in src assert ( - "for (const dropped of evicted) { syncModelOverride(dropped.modelId, dropped.ggufVariant, null); }" - in src + "for (const dropped of evicted) { syncModelOverride(dropped.modelId, " + "dropped.ggufVariant, null, { keepLaunchFlags: true, }); }" in src ) + api = " ".join(_read("features/model-picker/api/model-overrides.ts").split()) + assert "keepLaunchFlags?: boolean;" in api + # remove=false with no fields: the route re-supplies the stored flags and drops + # the row outright once nothing server-owned is left in it. + assert "remove: config === null && !options?.keepLaunchFlags," in api + assert ( + "...(config === null && !options?.keepLaunchFlags ? { llama_extra_args: [] } : {})," + in api.replace("// biome-ignore lint/style/useNamingConvention: API schema ", "") + ) + route = (WORKDIR / "studio" / "backend" / "routes" / "settings.py").read_text(encoding = "utf-8") + assert 'requested_extra_args = stored.get("llama_extra_args")' in route, "the rule this mirrors" # The eviction path must report what it dropped, decoded back into a model id # and variant rather than the normalized storage key.