Stop a lower-priority override key holding newer data than the one a … · unslothai/unsloth@ea63c8b · GitHub
Skip to content

Commit ea63c8b

Browse files
committed
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.
1 parent a25af76 commit ea63c8b

6 files changed

Lines changed: 403 additions & 10 deletions

File tree

‎studio/backend/routes/settings.py‎

Lines changed: 43 additions & 0 deletions

‎studio/backend/tests/test_openai_auto_switch.py‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4272,6 +4272,75 @@ def test_model_override_load_kwargs_gates_gpu_placement_on_gguf():
42724272
LoadRequest(model_path = "unsloth/B-GGUF", **gguf)
42734273

42744274

4275+
def test_a_carried_ctx_flag_cannot_outrank_a_freshly_saved_context(monkeypatch):
4276+
# The settings page has no control for pass-through flags, so a save carries over the
4277+
# ones already stored while writing the field the user just edited, leaving one entry
4278+
# holding both. Sent together they reach llama-server with the extras appended last,
4279+
# and its parser takes the final -c, so the stale flag would pin the old context.
4280+
_mock_override_store(monkeypatch)
4281+
_put("unsloth/B-GGUF:Q4_K_M", llama_extra_args = ["--ctx-size", "8192", "--top-k", "40"])
4282+
saved = _put("unsloth/B-GGUF:Q4_K_M", max_seq_length = 32768)
4283+
entry = saved.overrides["unsloth/B-GGUF:Q4_K_M"]
4284+
assert entry["max_seq_length"] == 32768
4285+
assert entry["llama_extra_args"] == ["--ctx-size", "8192", "--top-k", "40"]
4286+
4287+
backend = _FakeBackend(None)
4288+
rec = _LoadRecorder(backend)
4289+
_wire(
4290+
monkeypatch,
4291+
enabled = True,
4292+
resolves_to = ("unsloth/B-GGUF", "Q4_K_M", "unsloth/B-GGUF"),
4293+
backend = backend,
4294+
recorder = rec,
4295+
)
4296+
4297+
_run_hook("unsloth/B-GGUF")
4298+
request = rec.calls[0]
4299+
assert request.max_seq_length == 32768
4300+
# The shadowing flag goes; the sampling flag beside it is nobody's first-class field.
4301+
assert request.llama_extra_args == ["--top-k", "40"]
4302+
4303+
4304+
def test_load_kwargs_strip_only_the_shadow_groups_the_override_supplies():
4305+
# Same rule the /load route applies to inherited extras: one group per first-class
4306+
# field actually being sent, so a flag with nothing to shadow still passes through.
4307+
override = {
4308+
"llama_extra_args": [
4309+
"-c",
4310+
"8192",
4311+
"--cache-type-k",
4312+
"f16",
4313+
"--spec-type",
4314+
"ngram",
4315+
"--jinja",
4316+
"--split-mode",
4317+
"row",
4318+
"--top-p",
4319+
"0.9",
4320+
],
4321+
"max_seq_length": 32768,
4322+
"kv_cache_dtype": "q8_0",
4323+
"speculative_type": "mtp",
4324+
"chat_template_override": "{{ bos_token }}",
4325+
"tensor_parallel": True,
4326+
}
4327+
stripped = settings.model_override_load_kwargs(override, is_gguf = True)
4328+
assert stripped["llama_extra_args"] == ["--top-p", "0.9"]
4329+
4330+
# Nothing is supplied to shadow them, so every flag survives.
4331+
kept = settings.model_override_load_kwargs(
4332+
{"llama_extra_args": override["llama_extra_args"]}, is_gguf = True
4333+
)
4334+
assert kept["llama_extra_args"] == override["llama_extra_args"]
4335+
4336+
# And one field strips one group.
4337+
ctx_only = settings.model_override_load_kwargs(
4338+
{"llama_extra_args": override["llama_extra_args"], "max_seq_length": 32768},
4339+
is_gguf = True,
4340+
)
4341+
assert ctx_only["llama_extra_args"] == override["llama_extra_args"][2:]
4342+
4343+
42754344
def test_saved_parallel_slots_reach_an_api_load(monkeypatch):
42764345
# Parallel decode slots are a per-model setting the picker sends on every GGUF load.
42774346
backend = _FakeBackend(None)
@@ -4495,6 +4564,25 @@ def test_the_retired_snapshot_path_entry_hands_over_its_launch_flags(override_st
44954564
assert f"{_LEGACY_SNAPSHOT}:Q4_K_M" not in resp.overrides
44964565

44974566

4567+
def test_a_standalone_gguf_save_keeps_its_filename_label_launch_flags(override_store):
4568+
# An early build keyed a loose .gguf by the quant label from its filename, and the bare
4569+
# path the picker writes today is read before that key (see
4570+
# test_the_filename_label_key_no_longer_shadows_the_bare_path), so writing the bare entry
4571+
# without the legacy flags puts them out of reach of a page that cannot show or restore
4572+
# them. The forget path already consults the same derived key.
4573+
path = "/models/Qwen3-8B-Q4_K_M.gguf"
4574+
settings.set_model_override(
4575+
f"{path}:q4_k_m",
4576+
llama_extra_args = ["--flash-attn"],
4577+
max_seq_length = 4096,
4578+
)
4579+
4580+
resp = _put(path, max_seq_length = 32768)
4581+
entry = resp.overrides[path]
4582+
assert entry["llama_extra_args"] == ["--flash-attn"]
4583+
assert entry["max_seq_length"] == 32768
4584+
4585+
44984586
def test_a_snapshot_path_save_retires_the_repo_id_entry(override_store):
44994587
# The same rule the other way round: a row the picker still keys by its path (an inactive
45004588
# cache reached as a local row) must not be shadowed, so the last spelling saved survives.
@@ -4537,6 +4625,26 @@ def test_the_one_time_fill_retires_nothing(override_store):
45374625
assert resp.overrides["unsloth/B-GGUF:Q4_K_M"]["max_seq_length"] == 32768
45384626

45394627

4628+
def test_a_fill_never_creates_a_snapshot_path_key_over_a_repo_id_entry(override_store):
4629+
# A fill only adds, so it cannot retire the other spelling the way a save does. Creating
4630+
# the snapshot path key would leave two entries for one quant, and the loader reads the
4631+
# load path before the advertised repo id, so an upgraded browser's pre-upgrade copy would
4632+
# shadow the newer server config on every API load. Fill into the entry that is already
4633+
# there instead: nothing outranks it and the fields it lacks still arrive.
4634+
settings.set_model_override("unsloth/B-GGUF:Q4_K_M", max_seq_length = 32768)
4635+
4636+
resp = _put(
4637+
f"{_LEGACY_SNAPSHOT}:Q4_K_M",
4638+
max_seq_length = 2048,
4639+
kv_cache_dtype = "q8_0",
4640+
fill_absent_fields = True,
4641+
)
4642+
assert f"{_LEGACY_SNAPSHOT}:Q4_K_M" not in resp.overrides
4643+
entry = resp.overrides["unsloth/B-GGUF:Q4_K_M"]
4644+
assert entry["max_seq_length"] == 32768
4645+
assert entry["kv_cache_dtype"] == "q8_0"
4646+
4647+
45404648
def test_stale_gpu_ids_are_dropped_not_fatal(monkeypatch):
45414649
# A two-GPU pin on a one-GPU box used to 400 the load; one dead field degrades to defaults.
45424650
backend = _FakeBackend(None)

‎studio/backend/utils/openai_auto_switch_settings.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,33 @@ def model_override_load_kwargs(override: dict[str, Any], *, is_gguf: bool) -> di
456456
kwargs["n_cpu_moe"] = override["n_cpu_moe"]
457457
if override.get("gpu_ids") is not None:
458458
kwargs["gpu_ids"] = override["gpu_ids"]
459+
460+
if kwargs.get("llama_extra_args"):
461+
# One entry can hold a pass-through flag *and* the first-class field it shadows: the
462+
# settings page has no control for flags, so a save carries the stored ones over
463+
# (routes/settings.py) while writing the field just edited, and a legacy or
464+
# API-authored entry can start out that way. Sending both explicitly puts the flag
465+
# after Unsloth's own on the command line, where llama.cpp's last-wins parse hands it
466+
# the load, so a stale "--ctx-size 8192" would quietly outrank a freshly saved 32768.
467+
# The /load route strips exactly these groups off inherited extras
468+
# (_resolve_inherited_extra_args); the stripper is imported rather than mirrored so
469+
# the two paths cannot drift over which flag belongs to which group -- the allow-list
470+
# this module stays out of is validate_extra_args, which remains the caller's job.
471+
from core.inference.llama_server_args import strip_shadowing_flags
472+
473+
kwargs["llama_extra_args"] = strip_shadowing_flags(
474+
kwargs["llama_extra_args"],
475+
# Only the groups this override actually supplies, as the route gates on its
476+
# request's set fields: a flag with no first-class field behind it is the user's
477+
# only way to set that knob and still passes through.
478+
strip_context = "max_seq_length" in kwargs,
479+
strip_cache = "cache_type_kv" in kwargs,
480+
strip_spec = "speculative_type" in kwargs or "spec_draft_n_max" in kwargs,
481+
strip_template = "chat_template_override" in kwargs,
482+
# Sent only when on, so it is always the Tensor Parallelism toggle overriding the
483+
# flag; an override that leaves the toggle off keeps a row/none/layer split mode.
484+
strip_split_mode = bool(kwargs.get("tensor_parallel")),
485+
)
459486
return kwargs
460487

461488

‎studio/frontend/src/features/chat/lib/apply-inference-status-to-store.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
} from "../types/api";
2626
import type { ChatModelSummary } from "../types/runtime";
2727
import { sameGpuSelection } from "@/hooks/gpu-selection";
28+
import { resolveChatTemplateSeed } from "./resolve-chat-template-seed";
2829

2930
type LocalReasoningEffort = Extract<ReasoningEffort, "low" | "medium" | "high">;
3031

@@ -402,16 +403,18 @@ export function applyActiveModelStatusToStore(
402403
gpuStatusFields),
403404
// The one load param that only ever seeded from null, so a switch left the previous model's
404405
// template in the store, which the Hub settings page reads as the new model's loaded config:
405-
// Apply then saves A's template under B. Re-seeding needs BOTH guards the fields above use,
406-
// since seedLoadParams alone is just "no load in flight" and holds on an ordinary poll.
407-
...(status.chat_template_override !== undefined &&
408-
seedLoadParams &&
409-
(hydratingExistingModel ||
410-
(prevState.loadedChatTemplateOverride === null &&
411-
prevState.chatTemplateOverride === null)) && {
412-
chatTemplateOverride: status.chat_template_override,
413-
loadedChatTemplateOverride: status.chat_template_override,
414-
}),
406+
// Apply then saves A's template under B. A same-model reload from another client moves it
407+
// too, so the seed also follows a changed status the way the GPU group above does: baseline
408+
// always, control only while it still sits on that baseline. See resolveChatTemplateSeed.
409+
...resolveChatTemplateSeed({
410+
incoming: status.chat_template_override,
411+
previous: {
412+
chatTemplateOverride: prevState.chatTemplateOverride,
413+
loadedChatTemplateOverride: prevState.loadedChatTemplateOverride,
414+
},
415+
hydratingExistingModel,
416+
seedLoadParams,
417+
}),
415418
});
416419

417420
ensureActiveModelInStoreList(status, checkpointId);
Lines changed: 65 additions & 0 deletions

0 commit comments

Comments
 (0)