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.
This commit is contained in:
parent
41b40bff71
commit
5e090b951b
8 changed files with 187 additions and 7 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.",
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue