From 58abf4ca503663327bb8456f50c84a88f7ede23a Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Tue, 28 Jul 2026 17:32:46 +0000 Subject: [PATCH] Tighten the comments across the API monitor and per-model settings work Pass over every comment this branch touches. Collapse the multi-paragraph rationales to the point they were making, drop prop docs that only restated the prop name, and reflow the rest onto fewer lines. No code changes. --- studio/backend/core/inference/api_monitor.py | 23 +-- studio/backend/routes/inference.py | 43 ++--- studio/backend/routes/settings.py | 71 +++---- studio/backend/tests/test_api_monitor.py | 12 +- .../backend/tests/test_openai_auto_switch.py | 178 ++++++++---------- .../utils/openai_auto_switch_settings.py | 102 ++++------ studio/frontend/src/app/routes/__root.tsx | 10 +- studio/frontend/src/app/routes/api.tsx | 5 +- .../api-monitor/api-monitor-overlay.tsx | 44 ++--- .../features/api-monitor/api-monitor-page.tsx | 64 +++---- .../components/saved-model-settings.tsx | 11 +- .../src/features/api-monitor/lifecycle.ts | 17 +- .../src/features/api-monitor/overlay-store.ts | 22 +-- .../features/api-monitor/use-api-monitor.ts | 57 +++--- .../frontend/src/features/chat/types/api.ts | 4 +- .../features/hub/catalog/download-card.tsx | 1 - .../hub/catalog/hub-model-settings-view.tsx | 18 +- .../hub/catalog/local-on-device-card.tsx | 6 +- .../features/hub/catalog/model-inspector.tsx | 2 +- .../hub/catalog/models-catalog-lists.tsx | 1 - .../hub/catalog/models-catalog-rows.tsx | 10 +- .../features/hub/catalog/models-catalog.tsx | 1 - studio/frontend/src/features/hub/hub-page.tsx | 102 +++++----- .../api/migrate-model-overrides.ts | 49 +++-- .../model-picker/api/model-overrides.ts | 67 +++---- .../components/model-config-page.tsx | 34 ++-- .../model-selector/model-row-menu.tsx | 3 +- .../components/model-selector/types.ts | 11 +- .../model-config/per-model-config.ts | 14 +- .../settings/components/monitor-link.tsx | 8 +- tests/studio/test_model_picker_contracts.py | 17 +- ...test_settings_compact_overflow_contract.py | 12 +- ...st_usage_examples_model_source_contract.py | 18 +- 33 files changed, 435 insertions(+), 602 deletions(-) diff --git a/studio/backend/core/inference/api_monitor.py b/studio/backend/core/inference/api_monitor.py index f18dd449cd..92104df688 100644 --- a/studio/backend/core/inference/api_monitor.py +++ b/studio/backend/core/inference/api_monitor.py @@ -50,9 +50,8 @@ class ApiMonitorEntry: started_at: float updated_at: float subject: Optional[str] = None - # True when the caller used an sk-unsloth key rather than a UI session. The - # floating panel only opens itself for these: Studio's own chat goes through - # the same endpoints, and popping the monitor open mid-chat is noise. + # True for sk-unsloth key callers, not UI sessions. The floating panel only + # auto-opens for these, so Studio's own chat does not pop it mid-chat. via_api_key: bool = False # Monotonic anchors so duration math survives wall-clock steps (NTP). started_monotonic: float = 0.0 @@ -125,11 +124,8 @@ class ApiMonitor: enabled: bool = True, ): self._entries: deque[ApiMonitorEntry] = deque() - # Shared rows one subject has cleared. A shared row belongs to everyone, - # so dropping it would erase another caller's history, but leaving it - # means "Clear log" visibly does nothing to it: the frontend reloads - # straight after and the row comes back. Hiding it per subject is the - # only thing that is both true for that caller and safe for the others. + # Shared rows one subject cleared. Deleting them would erase another + # caller's history; keeping them makes "Clear log" look broken on reload. self._hidden_shared: dict[str, set[str]] = {} self._max_entries = max(0, max_entries) self._lock = threading.Lock() @@ -153,8 +149,8 @@ class ApiMonitor: id = f"apireq_{uuid.uuid4().hex[:12]}", endpoint = endpoint, method = method, - # str(): a raw JSON body can carry any type here, and the field is - # rendered in the UI, where a non-string breaks the whole monitor. + # str(): a raw JSON body can carry any type, and a non-string + # breaks the UI that renders it. model = str(model) if model else "default", prompt = _trim(prompt, _MAX_PROMPT_CHARS), status = "running", @@ -411,9 +407,7 @@ class ApiMonitor: self._entries.clear() self._hidden_shared.clear() return - # A shared row that is still running is a load in progress, not - # history, so it stays visible; clearing the log is about what has - # already happened. + # A running shared row is a load in progress, not history, so it stays. hidden = self._hidden_shared.setdefault(subject, set()) for entry in self._entries: if entry.shared and entry.subject != subject and entry.status != "running": @@ -446,8 +440,7 @@ class ApiMonitor: kept.append(entry) terminal_seen += 1 self._entries = kept - # The hidden sets only ever name rows that exist, so they stay bounded - # by the ring buffer rather than growing for the life of the process. + # Keep hidden sets to live rows so they stay bounded by the ring buffer. live = {entry.id for entry in kept} for subject, hidden in list(self._hidden_shared.items()): hidden &= live diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index bc9b430619..f951f91c04 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -4354,20 +4354,14 @@ async def _maybe_auto_switch_model( if _already_serving(): _record_serving_alias() return - # Apply this model's saved launch config so an API-driven swap - # loads it exactly as the picker would: context, KV dtype, - # speculative decoding, chat template and GPU placement, not - # just the two legacy flags. Look the config up under the - # variant-qualified id first (two quants of one repo can carry - # different configs), then the bare ids. Both the advertised - # repo id and the concrete load path are tried: a local folder - # or a non-active HF cache is configured against its path. - # A standalone .gguf needs no quant sub-selection, so the - # resolver reports variant=None for it. The picker still - # keys its config by the quant label it derives from the - # filename (LocalModelInfo.format_variant), which is never - # empty, so those settings live under ":LABEL" and no - # bare key would ever reach them. + # Apply this model's saved launch config so an API swap loads + # it exactly as the picker would. Try variant-qualified keys + # first (two quants of one repo can differ), then bare ids, and + # both the repo id and the load path (a local folder or a + # non-active HF cache is configured against its path). + # A standalone .gguf resolves with variant=None, but the picker + # keys it by the quant label derived from the filename, so its + # settings live under ":LABEL" and no bare key reaches them. file_variant = None if not variant and target_id.lower().endswith(".gguf"): from hub.utils.gguf import extract_quant_label @@ -4389,7 +4383,7 @@ async def _maybe_auto_switch_model( load_kwargs.update( model_override_load_kwargs( override, - # variant is set for every GGUF the resolver returns; the + # Set for every GGUF the resolver returns; the # reload-stash path carries the quant it froze. is_gguf = bool(variant) or target_id.lower().endswith(".gguf"), ) @@ -4398,9 +4392,8 @@ async def _maybe_auto_switch_model( if saved_gpu_ids and not await _override_gpu_ids_still_resolve( saved_gpu_ids ): - # A pin saved before a GPU was removed, before a - # visibility-mask change, or on another host. Dropping the - # one dead field beats 400ing the whole load. + # Stale pin (GPU removed, mask changed, another host). + # Dropping the one dead field beats 400ing the whole load. load_kwargs.pop("gpu_ids", None) logger.warning( "Dropping saved gpu_ids %s for %s: not available here.", @@ -4418,12 +4411,10 @@ async def _maybe_auto_switch_model( current_request_counted = True, ) except HTTPException as exc: - # The pre-flight check above cannot mirror every rule the - # loader applies to gpu_ids (a Vulkan diffusion GGUF refuses - # GPU selection outright, and the rules move). Rather than - # duplicating them, retry once without the saved pin: a - # stale placement preference must never be the reason an - # API request cannot be served. + # The pre-flight check cannot mirror every gpu_ids rule the + # loader applies (a Vulkan diffusion GGUF refuses GPU + # selection outright). Retry once without the saved pin: a + # stale placement preference must never block a request. if not ( exc.status_code == 400 and load_kwargs.get("gpu_ids") @@ -4719,12 +4710,12 @@ async def _override_gpu_ids_still_resolve(gpu_ids: List[int]) -> bool: is_vulkan = LlamaCppBackend._is_vulkan_backend() if get_device() == DeviceType.XPU and not is_vulkan: - # gpu_ids is rejected outright on XPU. + # Rejected outright on XPU. return False resolved = resolve_requested_gpu_ids(gpu_ids, is_vulkan = is_vulkan) if is_vulkan and resolved: # Vulkan ordinals are their own index space, so resolve() only rejects - # malformed ones. Presence needs the same ggml probe the load does. + # malformed ones; presence needs the ggml probe the load runs. binary = LlamaCppBackend._find_llama_server_binary() if binary: probed = { diff --git a/studio/backend/routes/settings.py b/studio/backend/routes/settings.py index 9639cee82d..a5e3cab832 100644 --- a/studio/backend/routes/settings.py +++ b/studio/backend/routes/settings.py @@ -132,16 +132,14 @@ class OpenAIAutoSwitchResponse(BaseModel): auto_download_model: bool = DEFAULT_OPENAI_AUTO_DOWNLOAD_ENABLED -# A quant suffix, as modelOverrideKey builds it. Matched against the loader's own -# quant pattern rather than a length heuristic: a POSIX path may legitimately -# contain a colon ("/models/foo:bar.gguf"), and treating "bar.gguf" as a quant -# would graft an unrelated model's launch flags onto this one. +# A quant suffix, as modelOverrideKey builds it. Matched against the loader's quant +# pattern, not a length heuristic: a POSIX path may hold a colon +# ("/models/foo:bar.gguf") and would otherwise inherit another model's flags. _MAX_VARIANT_SUFFIX_LEN = 64 -# A local model's id is its filesystem path, optionally with a quant suffix, and +# A local model's id is its path plus an optional quant suffix, and # LoadRequest.model_path is unbounded. A limit under PATH_MAX would 422 the server -# sync while the local save succeeded, leaving the UI showing settings the API -# never applies. +# sync while the local save succeeded. MAX_MODEL_OVERRIDE_KEY_LEN = 4096 + 1 + _MAX_VARIANT_SUFFIX_LEN @@ -156,9 +154,8 @@ class ModelOverridePayload(BaseModel): """ model_id: str = Field(..., min_length = 1, max_length = MAX_MODEL_OVERRIDE_KEY_LEN) - # None means "leave the stored value alone": the settings UI has no control - # for launch flags, so a save from it must not wipe flags set through this - # API. An explicit [] clears them (that is how "forget this model" arrives). + # None means "leave the stored value alone": the settings UI has no control for + # launch flags and must not wipe them. An explicit [] clears them (forget). llama_extra_args: Optional[list[str]] = None # ge=1: 0 is not a valid sequence length, and the setter drops a falsy value, # so reject it at the boundary instead of accepting then silently discarding it. @@ -168,25 +165,22 @@ class ModelOverridePayload(BaseModel): speculative_type: Optional[str] = Field(default = None, max_length = 32) spec_draft_n_max: Optional[int] = Field(default = None, ge = 1, le = 16) tensor_parallel: bool = False - # Validated in bytes below, not by max_length: pydantic counts characters, - # so a multi-byte template could pass here and then be silently dropped by - # the normalizer (which measures UTF-8) while the request still returned 200. + # 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. chat_template_override: Optional[str] = None gpu_memory_mode: Optional[Literal["auto", "manual"]] = None # -1 is Auto (llama.cpp --fit sizes the offload); the normalizer treats it as unset. gpu_layers: Optional[int] = Field(default = None, ge = -1, le = 1024) n_cpu_moe: Optional[int] = Field(default = None, ge = 0, le = 1024) gpu_ids: Optional[list[int]] = None - # Explicit intent. A save whose config is entirely default carries no fields - # at all, which is indistinguishable from "forget this model" by shape alone. - # None keeps the original contract: a bare model_id means remove. + # Explicit intent: an all-default save carries no fields, which is shape + # identical to "forget this model". None keeps the legacy contract. remove: Optional[bool] = None @field_validator("chat_template_override") @classmethod def _limit_chat_template_bytes(cls, value: Optional[str]) -> Optional[str]: - # Mirrors LoadRequest.normalize_blank_chat_template_override so the same - # template is accepted or rejected identically on both paths. + # Mirrors LoadRequest.normalize_blank_chat_template_override. if value is None: return None size = chat_template_byte_length(value) @@ -357,10 +351,8 @@ def _bare_model_id(model_id: str) -> Optional[str]: """``repo`` for a ``repo:QUANT`` key, or None when there is no quant suffix.""" from utils.openai_auto_switch_settings import split_quant_suffix - # Must actually look like a quant, not just like a short path segment. The - # label may carry a bits-per-weight modifier ("IQ4_XS-3.53bpw"), which keeps - # two files at the same base quant distinct, and a .gguf with no recognized - # token is labelled by its stem, so both forms count. + # Must look like a quant, not just a short path segment. Both a bits-per-weight + # modifier ("IQ4_XS-3.53bpw") and a stem fallback label count. split = split_quant_suffix(model_id) return split[0] if split is not None else None @@ -373,10 +365,8 @@ def update_openai_auto_switch_override( from utils.openai_auto_switch_settings import get_model_override try: - # A payload carrying only model_id is the documented "remove", so it - # wipes everything. Otherwise it is a real save, and omitted launch flags - # are carried over from the stored entry (the settings UI cannot express - # them and must not delete them). + # Only model_id is the documented "remove". Otherwise omitted launch flags + # carry over from the stored entry, since the settings UI cannot express them. requested_extra_args = payload.llama_extra_args saved_fields = payload.model_dump( exclude = {"model_id", "llama_extra_args", "remove"}, exclude_none = True @@ -390,33 +380,24 @@ def update_openai_auto_switch_override( if requested_extra_args is None and not is_removal: requested_extra_args = get_model_override(payload.model_id).get("llama_extra_args") if requested_extra_args is None: - # First per-quant save for a model whose flags were stored under the - # bare repo id. Auto-switch prefers the qualified entry, so without - # this the flags are silently dropped and no UI can restore them. + # First per-quant save for flags stored under the bare repo id. + # Auto-switch prefers the qualified entry, so carry them over. bare_id = _bare_model_id(payload.model_id) if bare_id: requested_extra_args = get_model_override(bare_id).get("llama_extra_args") - # Not validated on an explicit remove: nothing is stored, so the only - # effect would be a 400 that leaves the override in place, which is the - # opposite of what remove means. A stale form still carrying a rejected - # flag must not be able to block forgetting a model. + # Not validated on an explicit remove: nothing is stored, so a 400 would only + # leave the override in place. A stale flag must not block forgetting. extra_args = [] if payload.remove is True else validate_extra_args(requested_extra_args) if payload.remove is True: - # An explicit remove wins over anything else in the payload: a stale - # form field must not turn "forget this model" into an update that - # keeps it. Only the explicit flag short-circuits; the legacy - # inferred path still just gates launch-flag carry-over. - # Remove the key a load would actually resolve to, not just the - # literal one sent: the browser normalizes casing before storing, so - # the two can differ and a stale entry would survive forgetting. + # An explicit remove wins over any other field in the payload. Remove the + # key a load resolves to, not the literal one sent: the browser normalizes + # casing before storing, so a stale entry would survive forgetting. target_id = resolve_model_override_key(payload.model_id) or payload.model_id set_model_override(target_id, llama_extra_args = [], max_seq_length = None) else: - # Save under the key a load would resolve to, for the same reason the - # removal branch does. The browser normalizes casing before storing, - # so saving the literal id leaves a second entry for one model, and - # two equivalent keys make every other casing ambiguous: the lookup - # then matches neither and the model silently loses its settings. + # Save under the key a load resolves to, as the removal branch does. + # Saving the literal id leaves two keys for one model, which makes every + # other casing ambiguous and silently loses the settings. target_id = resolve_model_override_key(payload.model_id) or payload.model_id set_model_override( target_id, diff --git a/studio/backend/tests/test_api_monitor.py b/studio/backend/tests/test_api_monitor.py index 821861457c..2b611928e9 100644 --- a/studio/backend/tests/test_api_monitor.py +++ b/studio/backend/tests/test_api_monitor.py @@ -261,9 +261,8 @@ def test_api_monitor_append_reply_exact_cap_then_more_marks_truncated(): def test_api_monitor_clear_is_scoped_to_one_subject(): - # Every other read on the monitor is subject-scoped. An unscoped clear from - # the route would let one caller erase another's history and zero their - # active count in the middle of a generation. + # Every other read is subject-scoped; an unscoped clear from the route would let + # one caller erase another's history mid-generation. monitor = ApiMonitor(max_entries = 4) alice = monitor.start( endpoint = "/v1/chat/completions", @@ -292,9 +291,8 @@ def test_api_monitor_clear_is_scoped_to_one_subject(): def test_api_monitor_records_whether_the_caller_used_an_api_key(): - # Studio's own chat hits these endpoints with a session JWT. The floating - # panel keys its auto-open off this flag, so mislabelling in-app chat as API - # traffic pops the panel over the composer mid-conversation. + # Studio's own chat hits these endpoints with a session JWT, and the floating + # panel keys its auto-open off this flag, so mislabelling it pops the panel. monitor = ApiMonitor(max_entries = 4) ui = monitor.start( endpoint = "/api/inference/chat", @@ -494,7 +492,7 @@ def test_clear_hides_shared_lifecycle_rows_for_that_caller_only(): monitor.clear(subject = "alice") assert monitor.snapshot(subject = "alice") == [] - # Bob's view is untouched: the row is hidden for alice, not deleted. + # Hidden for alice, not deleted, so bob's view is untouched. assert {e["id"] for e in monitor.snapshot(subject = "bob")} == {shared} assert monitor.get(shared, subject = "alice") is None assert monitor.get(shared, subject = "bob") is not None diff --git a/studio/backend/tests/test_openai_auto_switch.py b/studio/backend/tests/test_openai_auto_switch.py index 80d076a865..73a6e6fb4f 100644 --- a/studio/backend/tests/test_openai_auto_switch.py +++ b/studio/backend/tests/test_openai_auto_switch.py @@ -4150,9 +4150,7 @@ def test_env_idle_below_floor_is_clamped(monkeypatch): assert settings.get_auto_unload_idle_seconds() == 0 -# --------------------------------------------------------------------------- # Per-model launch config: normalization, LoadRequest mapping, key resolution. -# --------------------------------------------------------------------------- def test_normalize_model_override_drops_unusable_fields_and_keeps_the_rest(): @@ -4178,8 +4176,8 @@ def test_normalize_model_override_drops_unusable_fields_and_keeps_the_rest(): def test_normalize_model_override_rejects_oversized_chat_template(): small = settings.normalize_model_override({"chat_template_override": "{{ bos }}"}) assert small["chat_template_override"] == "{{ bos }}" - # The limit is bytes, not characters: a multi-byte template just under the - # character limit can still be over the byte limit. + # The limit is bytes, not characters, so a multi-byte template just under the + # character limit can still be over. huge = "é" * settings.MAX_CHAT_TEMPLATE_OVERRIDE_BYTES assert "chat_template_override" not in settings.normalize_model_override( {"chat_template_override": huge} @@ -4189,15 +4187,15 @@ def test_normalize_model_override_rejects_oversized_chat_template(): def test_spec_draft_n_max_only_stored_for_mtp_modes(): mtp = settings.normalize_model_override({"speculative_type": "mtp", "spec_draft_n_max": 4}) assert mtp["spec_draft_n_max"] == 4 - # A non-MTP mode ignores the draft count at load time, so storing it would - # show the user an edit that never takes effect. + # A non-MTP mode ignores the draft count, so storing it shows an edit that + # never takes effect. ngram = settings.normalize_model_override({"speculative_type": "ngram", "spec_draft_n_max": 4}) assert "spec_draft_n_max" not in ngram def test_resolve_fit_max_seq_length_hands_sizing_to_fit_under_manual_auto_layers(): - # Manual GPU memory with Auto layers means llama.cpp --fit owns the context, - # so the load sends the context pin (or 0), not the stored max seq length. + # Manual GPU memory with Auto layers hands the context to llama.cpp --fit, so + # the load sends the context pin (or 0), not the stored max seq length. override = {"gpu_memory_mode": "manual", "max_seq_length": 8192} assert settings.resolve_fit_max_seq_length(override, is_gguf = True) == 0 assert ( @@ -4228,8 +4226,8 @@ def test_model_override_load_kwargs_gates_gpu_placement_on_gguf(): assert gguf["gpu_layers"] == 20 assert gguf["gpu_ids"] == [0, 1] - # A safetensors model loads through HF auto-placement; inheriting a GGUF GPU - # pin here would silently change where the weights land. + # A safetensors model loads through HF auto-placement, so a GGUF GPU pin would + # silently change where the weights land. safetensors = settings.model_override_load_kwargs(override, is_gguf = False) assert safetensors["max_seq_length"] == 4096 assert "gpu_layers" not in safetensors @@ -4237,14 +4235,14 @@ def test_model_override_load_kwargs_gates_gpu_placement_on_gguf(): assert "n_cpu_moe" not in safetensors assert "gpu_memory_mode" not in safetensors - # Every key it produces has to be a real LoadRequest field, or the load call - # raises TypeError at the moment the user's request arrives. + # Every key must be a real LoadRequest field, or the load raises TypeError when + # the user's request arrives. LoadRequest(model_path = "unsloth/B-GGUF", **gguf) def test_auto_switch_prefers_variant_qualified_override(monkeypatch): - # Settings are saved per quant, so Q4_K_M and Q8_0 of the same repo are - # different entries; the bare repo id is only the fallback. + # 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. backend = _FakeBackend(None) rec = _LoadRecorder(backend) _wire( @@ -4284,9 +4282,8 @@ def test_auto_switch_falls_back_to_bare_repo_override(monkeypatch): def test_override_route_preserves_launch_flags_across_a_settings_only_update(monkeypatch): - # The settings page has no control for llama_extra_args, so saving from it - # omits the field. Omitted must mean "leave it alone", or every save from the - # UI would quietly wipe flags set elsewhere. + # The settings page has no control for llama_extra_args, so it omits the field. + # Omitted must mean "leave it alone", or every UI save wipes flags set elsewhere. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4304,8 +4301,8 @@ def test_override_route_preserves_launch_flags_across_a_settings_only_update(mon assert entry["llama_extra_args"] == ["--flash-attn"] assert entry["max_seq_length"] == 4096 - # An explicit empty list is how the UI says "forget this model", and with no - # other fields left that removes the entry outright. + # An explicit empty list is the UI's "forget this model", and with no other + # fields left it removes the entry outright. gone = settings_route.update_openai_auto_switch_override( settings_route.ModelOverridePayload(model_id = "unsloth/B-GGUF", llama_extra_args = []), "tester", @@ -4314,8 +4311,8 @@ def test_override_route_preserves_launch_flags_across_a_settings_only_update(mon def test_override_found_under_a_concrete_path_with_variant(monkeypatch): - # A local folder or non-active HF cache resolves to a public repo id plus a - # concrete path. Settings saved against the path must still be found. + # A local folder or non-active HF cache resolves to a repo id plus a concrete + # path, and settings saved against the path must still be found. backend = _FakeBackend(None) rec = _LoadRecorder(backend) _wire( @@ -4333,8 +4330,8 @@ def test_override_found_under_a_concrete_path_with_variant(monkeypatch): def test_repo_qualified_override_beats_path_qualified(monkeypatch): - # Ordering is most specific first, and the public repo id is the name the - # user configured against in the picker. + # Most specific first, and the public repo id is what the picker configured + # against. backend = _FakeBackend(None) rec = _LoadRecorder(backend) _wire( @@ -4355,10 +4352,9 @@ def test_repo_qualified_override_beats_path_qualified(monkeypatch): def test_first_quant_save_keeps_legacy_bare_repo_launch_flags(monkeypatch): - # Flags were stored under the bare repo id before per-quant settings existed. - # The first save from the settings page writes repo:QUANT, and auto-switch - # then prefers that entry, so the flags must come with it or they are - # silently disabled with no UI able to show or restore them. + # Flags predating per-quant settings live under the bare repo id. The first save + # writes repo:QUANT, which auto-switch prefers, so the flags must come with it + # or they are silently disabled with no UI able to restore them. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4374,8 +4370,8 @@ def test_first_quant_save_keeps_legacy_bare_repo_launch_flags(monkeypatch): def test_bare_repo_carry_over_does_not_split_a_windows_path(monkeypatch): - # "C:\models\x.gguf" has a colon that is not a variant separator. Splitting - # naively would look up "C" and, worse, could graft another model's flags on. + # The colon in "C:\models\x.gguf" is not a variant separator: splitting naively + # looks up "C" and could graft another model's flags on. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4404,8 +4400,8 @@ def test_windows_path_with_quant_still_carries_over(monkeypatch): def test_stale_gpu_ids_are_dropped_not_fatal(monkeypatch): - # A pin saved on a two-GPU box, replayed on a one-GPU box. Before this the - # whole load 400d; the contract is that one dead field degrades to defaults. + # A two-GPU pin replayed on a one-GPU box used to 400 the whole load; the + # contract is that one dead field degrades to defaults. backend = _FakeBackend(None) rec = _LoadRecorder(backend) _wire( @@ -4455,8 +4451,7 @@ def test_usable_gpu_ids_are_kept(monkeypatch): def test_override_gpu_ids_probe_never_raises(monkeypatch): - # The probe runs on the load path, so any hardware error must read as - # "unusable" rather than escaping as a 500. + # On the load path, so a hardware error must read as "unusable", not a 500. import utils.hardware.hardware as hw def boom(*args, **kwargs): @@ -4467,9 +4462,8 @@ def test_override_gpu_ids_probe_never_raises(monkeypatch): def test_vulkan_ordinal_absent_from_the_probe_is_unusable(monkeypatch): - # resolve_requested_gpu_ids only rejects malformed Vulkan ordinals, so - # presence needs the same ggml probe the load itself runs. Without it this - # helper says "fine" and the load 400s on the check it skipped. + # resolve_requested_gpu_ids only rejects malformed Vulkan ordinals, so presence + # needs the ggml probe the load runs, or the load 400s on the skipped check. from core.inference.llama_cpp import LlamaCppBackend monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda: True)) @@ -4485,8 +4479,8 @@ def test_vulkan_ordinal_absent_from_the_probe_is_unusable(monkeypatch): def test_vulkan_probe_without_a_binary_does_not_block_the_load(monkeypatch): - # No binary means nothing to probe with. Refusing here would drop a valid - # pin on every load, so the later path stays the authority. + # Nothing to probe with, and refusing would drop a valid pin on every load, + # so the later path stays the authority. from core.inference.llama_cpp import LlamaCppBackend monkeypatch.setattr(LlamaCppBackend, "_is_vulkan_backend", staticmethod(lambda: True)) @@ -4495,9 +4489,8 @@ def test_vulkan_probe_without_a_binary_does_not_block_the_load(monkeypatch): def test_default_save_preserves_flags_instead_of_removing(monkeypatch): - # "Remember for this model" is on but every value is default, so the payload - # carries no fields. That is shape-identical to a removal, and guessing wrong - # wipes launch flags no UI can show or restore. + # "Remember for this model" with all-default values sends no fields, which is + # shape-identical to a removal; guessing wrong wipes unrecoverable launch flags. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4552,7 +4545,7 @@ def test_remove_false_with_real_fields_saves_normally(monkeypatch): def test_override_lookup_falls_back_to_case_insensitive(monkeypatch): - # The browser lowercases ids before storing them, so the backfill writes + # The browser lowercases ids, so the backfill writes # "unsloth/qwen3-8b-gguf:q4_k_m" while the resolver asks for the repo's real # casing. Without this fallback every migrated entry is invisible. _mock_override_store(monkeypatch) @@ -4570,8 +4563,8 @@ def test_exact_override_match_beats_a_case_variant(monkeypatch): def test_ambiguous_case_fallback_matches_nothing(monkeypatch): - # Two POSIX paths differing only in case are two different files. Guessing - # between them would apply one model's settings to another. + # Two POSIX paths differing only in case are two files, so guessing between + # them applies one model's settings to another. _mock_override_store(monkeypatch) settings.set_model_override("/models/foo.gguf", max_seq_length = 1024) settings.set_model_override("/models/FOO.gguf", max_seq_length = 8192) @@ -4590,14 +4583,14 @@ def test_request_used_api_key_distinguishes_key_from_session(): assert inference_route._request_used_api_key(_Req("Bearer eyJhbGciOiJIUzI1NiJ9.x")) is False assert inference_route._request_used_api_key(_Req("")) is False assert inference_route._request_used_api_key(_Req(None)) is False - # A malformed request object must read as "not an API key", never raise, since - # this runs on the hot path of every tracked request. + # Runs on the hot path of every tracked request, so a malformed request object + # must read as "not an API key" rather than raise. assert inference_route._request_used_api_key(object()) is False def test_case_fallback_never_applies_to_a_posix_path(monkeypatch): - # Two files that differ only in case are two different models on Linux, so a - # near miss must load defaults rather than another model's context and GPU pin. + # Two files differing only in case are two models on Linux, so a near miss must + # load defaults rather than another model's context and GPU pin. _mock_override_store(monkeypatch) settings.set_model_override("/models/foo.gguf", max_seq_length = 8192, gpu_ids = [1]) assert settings.get_model_override("/models/Foo.gguf") == {} @@ -4605,10 +4598,9 @@ def test_case_fallback_never_applies_to_a_posix_path(monkeypatch): def test_case_fallback_does_apply_to_a_windows_path(monkeypatch): - # NTFS is case-insensitive, so these name one file, and the browser folds - # drive paths before storing. Treating them as two models would leave every - # migrated Windows entry unreachable until the user saved it again, which is - # the opposite of the POSIX rule and for the opposite reason. The separator + # NTFS is case-insensitive, so these name one file and the browser folds drive + # paths before storing. Treating them as two would strand every migrated Windows + # entry: the opposite of the POSIX rule, for the opposite reason. The separator # is interchangeable there too. _mock_override_store(monkeypatch) settings.set_model_override(r"c:\models\foo.gguf", max_seq_length = 8192) @@ -4625,16 +4617,16 @@ def test_case_fallback_applies_to_unc_and_wsl_drive_paths(monkeypatch): def test_a_plain_posix_path_under_mnt_stays_case_sensitive(monkeypatch): - # Only /mnt/ is a WSL drive mount. /mnt/data is an ordinary Linux - # mount point and stays case-sensitive like any other POSIX path. + # Only /mnt/ is a WSL drive mount; /mnt/data is an ordinary Linux mount + # point and stays case-sensitive. _mock_override_store(monkeypatch) settings.set_model_override("/mnt/data/models/foo.gguf", max_seq_length = 8192) assert settings.get_model_override("/mnt/data/models/Foo.gguf") == {} def test_an_ambiguous_windows_case_fallback_still_matches_nothing(monkeypatch): - # Two stored keys folding to one leaves no single answer, so the load takes - # defaults rather than guessing between them. + # Two stored keys folding to one has no single answer, so the load takes + # defaults rather than guessing. _mock_override_store(monkeypatch) settings.set_model_override(r"c:\models\foo.gguf", max_seq_length = 1024) settings.set_model_override("C:/models/FOO.gguf", max_seq_length = 8192) @@ -4649,10 +4641,9 @@ def test_case_fallback_still_covers_repo_ids(monkeypatch): def test_explicit_remove_is_not_blocked_by_stale_invalid_flags(monkeypatch): - # remove is the operation discriminator, so a form still carrying a rejected - # launch flag must not turn "forget this model" into a 400 that leaves the - # override in place. Nothing is stored on this path, so there is nothing to - # validate. + # remove is the operation discriminator, so a rejected launch flag must not turn + # "forget this model" into a 400 that leaves the override in place. Nothing is + # stored on this path, so there is nothing to validate. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4667,8 +4658,8 @@ def test_explicit_remove_is_not_blocked_by_stale_invalid_flags(monkeypatch): def test_explicit_remove_wins_over_config_fields_in_the_same_payload(monkeypatch): - # remove is the operation discriminator, so a stale form field alongside it - # must not quietly turn "forget this model" into an update. + # remove is the operation discriminator, so a stale field alongside it must not + # turn "forget this model" into an update. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4683,8 +4674,8 @@ def test_explicit_remove_wins_over_config_fields_in_the_same_payload(monkeypatch def test_posix_colon_in_a_path_is_not_treated_as_a_quant(monkeypatch): - # "/models/foo:bar.gguf" is one valid POSIX filename, not repo + quant. - # Splitting it would graft /models/foo's launch flags onto a different model. + # "/models/foo:bar.gguf" is one POSIX filename, not repo + quant; splitting it + # grafts /models/foo's launch flags onto a different model. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4697,11 +4688,10 @@ def test_posix_colon_in_a_path_is_not_treated_as_a_quant(monkeypatch): def test_unknown_quant_label_on_a_gguf_still_carries_flags_over(monkeypatch): - # A .gguf whose filename holds no recognizable quant token is still labelled - # by the scanner, which falls back to the stem, so the UI saves under - # "/models/custom.gguf:custom". Refusing that suffix dropped the bare entry's - # legacy flags on the first save, and auto-switch then prefers the qualified - # entry, so nothing was left that could restore them. + # A .gguf with no recognizable quant token is labelled by its stem, so the UI + # saves under "/models/custom.gguf:custom". Refusing that suffix dropped the bare + # entry's legacy flags on the first save, and auto-switch prefers the qualified + # entry, so nothing was left to restore them. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4716,10 +4706,10 @@ def test_unknown_quant_label_on_a_gguf_still_carries_flags_over(monkeypatch): def test_bpw_qualified_variants_still_carry_flags_over(monkeypatch): - # utils/models/model_config.py keeps a bits-per-weight modifier on the label - # so two files at the same base quant stay distinct, and that form reaches - # the override keys. The known-quant pattern does not accept it, so the bare - # entry was missed and the first qualified save dropped its launch flags. + # utils/models/model_config.py keeps a bits-per-weight modifier on the label to + # keep two files at the same base quant distinct, and that form reaches the + # override keys. The known-quant pattern rejects it, so the bare entry was missed + # and the first qualified save dropped its launch flags. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4736,9 +4726,9 @@ def test_bpw_qualified_variants_still_carry_flags_over(monkeypatch): def test_a_posix_path_variant_folds_while_the_path_does_not(monkeypatch): - # The browser lowercases the quant but keeps POSIX path casing, so the - # migrated key is "/models/Foo:q4_k_m" while the scanner asks for - # "/models/Foo:Q4_K_M". The path itself must still be case-sensitive. + # The browser lowercases the quant but keeps POSIX path casing, so the migrated + # "/models/Foo:q4_k_m" must answer the scanner's "/models/Foo:Q4_K_M" while the + # path itself stays case-sensitive. _mock_override_store(monkeypatch) settings.set_model_override("/models/Foo:q4_k_m", max_seq_length = 8192) assert settings.get_model_override("/models/Foo:Q4_K_M")["max_seq_length"] == 8192 @@ -4746,10 +4736,9 @@ def test_a_posix_path_variant_folds_while_the_path_does_not(monkeypatch): def test_an_unknown_gguf_label_is_reachable_in_either_casing(monkeypatch): - # A .gguf with no recognizable quant token is labelled by its stem, and v2 - # storage lowercases that label while the scanner probes with the filename's - # own casing. Folding only recognized quant labels left the migrated entry - # unreachable for exactly the files that need the fallback. + # A .gguf with no recognizable quant token is labelled by its stem, and v2 storage + # lowercases that label while the scanner keeps the filename casing. Folding only + # recognized labels stranded exactly the files that need the fallback. _mock_override_store(monkeypatch) settings.set_model_override("/models/CustomModel.gguf:custommodel", max_seq_length = 8192) got = settings.get_model_override("/models/CustomModel.gguf:CustomModel") @@ -4759,16 +4748,16 @@ def test_an_unknown_gguf_label_is_reachable_in_either_casing(monkeypatch): def test_a_posix_colon_filename_is_not_folded_as_a_variant(monkeypatch): - # "/models/foo:Bar.gguf" is one filename, not path + quant, so folding its - # tail would let it reach a different file's settings. + # "/models/foo:Bar.gguf" is one filename, not path + quant, so folding its tail + # would reach a different file's settings. _mock_override_store(monkeypatch) settings.set_model_override("/models/foo:bar.gguf", max_seq_length = 8192) assert settings.get_model_override("/models/foo:Bar.gguf") == {} def test_a_suffix_the_scanner_would_not_derive_carries_nothing_over(monkeypatch): - # Only the exact label the scanner derives for this filename is accepted, so - # an unrelated colon suffix cannot reach into another model's flags. + # Only the scanner's exact label is accepted, so an unrelated colon suffix cannot + # reach another model's flags. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4783,8 +4772,8 @@ def test_a_suffix_the_scanner_would_not_derive_carries_nothing_over(monkeypatch) def test_unknown_quant_label_carries_over_for_a_windows_path(monkeypatch): - # The key is written on Windows but may be read back by a backend that is - # not, where a backslash is an ordinary filename character. + # Written on Windows but read back on a backend where a backslash is an ordinary + # filename character. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4811,9 +4800,8 @@ def test_real_quant_suffix_on_a_path_still_carries_flags_over(monkeypatch): def test_load_retries_without_gpu_ids_when_the_loader_rejects_the_pin(monkeypatch): - # The pre-flight check cannot mirror every rule the loader applies (a Vulkan - # diffusion GGUF refuses GPU selection outright). A stale placement preference - # must never be the reason a request cannot be served. + # The pre-flight check cannot mirror every loader rule (a Vulkan diffusion GGUF + # refuses GPU selection), and a stale pin must never block a request. from fastapi import HTTPException backend = _FakeBackend(None) @@ -4887,9 +4875,8 @@ def test_a_non_gpu_load_failure_is_not_retried(monkeypatch): def test_removal_clears_the_entry_a_load_would_actually_resolve(monkeypatch): - # The browser normalizes casing before storing, so a forget request can carry - # a different casing than the stored key. Removing only the literal key would - # leave the entry a load still resolves to, with no UI able to clear it. + # The browser normalizes casing before storing, so a forget can carry a different + # casing; removing only the literal key leaves an entry loads still resolve to. import routes.settings as settings_route _mock_override_store(monkeypatch) @@ -4905,10 +4892,9 @@ def test_removal_clears_the_entry_a_load_would_actually_resolve(monkeypatch): def test_save_updates_the_existing_case_variant_instead_of_forking_it(monkeypatch): - # The backfill stores normalized (lowercase) keys while a later UI save carries - # the catalog's casing. Writing that literally leaves two keys for one model, - # and with two equivalent keys present any third casing resolves ambiguously, - # so the model silently loses every saved setting on the API path. + # The backfill stores lowercase keys while a later UI save carries the catalog's + # casing. Writing that literally leaves two keys for one model, which makes any + # third casing ambiguous and silently loses every setting on the API path. import routes.settings as settings_route _mock_override_store(monkeypatch) diff --git a/studio/backend/utils/openai_auto_switch_settings.py b/studio/backend/utils/openai_auto_switch_settings.py index 3f39525f7e..5cbdbfa2af 100644 --- a/studio/backend/utils/openai_auto_switch_settings.py +++ b/studio/backend/utils/openai_auto_switch_settings.py @@ -250,24 +250,18 @@ def set_openai_auto_switch( # --- Per-model launch config ------------------------------------------------- # # An override is the server-side twin of the UI's per-model config (the browser -# localStorage map behind features/model-picker/model-config). The UI mirrors -# every save here so a model loaded by an OpenAI-compatible API request gets the -# same launch settings a user would get loading it from the picker; without this -# the API path could only ever apply the two legacy fields below. +# localStorage map behind features/model-picker/model-config). The UI mirrors every +# save here so an OpenAI-compatible API load gets the same launch settings the +# picker would apply, rather than only the two legacy fields below. # -# Legacy entries hold just {llama_extra_args, max_seq_length}; every field is -# optional and absent means "fall back to the app default", so old entries keep -# loading correctly. A write is a full replace of the fields it expresses, so the -# route carries `llama_extra_args` over when the payload omits it (the settings -# UI has no control for launch flags and must not wipe them). +# Legacy entries hold just {llama_extra_args, max_seq_length}. Every field is +# optional and absent means "app default". A write replaces the fields it +# expresses, so the route carries `llama_extra_args` over when the payload omits it. # -# Known gap: the picker resolves a couple of knobs as "per-model value, else the -# user's global preference" -- GPU memory mode and speculative decoding, whose -# globals live in browser localStorage. An override deliberately stores only an -# explicit per-model choice (so the model keeps following later global changes), -# and the server cannot see the globals at all. So for a model that follows the -# global on one of those two, an API load falls back to the app default rather -# than the user's global. Every other field matches the picker exactly. +# Known gap: the picker falls back to a global preference for GPU memory mode and +# speculative decoding, and those globals live in browser localStorage. An override +# stores only an explicit per-model choice, so an API load of a model that follows +# the global gets the app default instead. Every other field matches the picker. # Mirrors _valid_cache_types in core/inference/llama_cpp.py. VALID_KV_CACHE_DTYPES = frozenset( @@ -287,7 +281,7 @@ VALID_SPECULATIVE_TYPES = frozenset( "ngram-simple", } ) -# Only these two consume spec_draft_n_max (mirrors MTP_SPECULATIVE_TYPES in the UI). +# Only these consume spec_draft_n_max (mirrors MTP_SPECULATIVE_TYPES in the UI). MTP_SPECULATIVE_TYPES = frozenset({"mtp", "mtp+ngram", "draft-mtp"}) VALID_GPU_MEMORY_MODES = frozenset({"auto", "manual"}) @@ -303,12 +297,10 @@ def _clean_str(value: Any, allowed: frozenset[str]) -> Optional[str]: def _bounded_int(value: Any, *, minimum: int, maximum: int) -> Optional[int]: - # bool is a subclass of int, so `gpu_ids: [true, false]` would otherwise pin - # the model to GPUs 1 and 0. + # bool subclasses int, so `gpu_ids: [true, false]` would pin GPUs 1 and 0. if isinstance(value, bool): return None - # int(1.5) is 1, which would silently turn a fractional context into a - # useless one. Only exact integers count. + # int(1.5) is 1, which would silently mangle a fractional context. if isinstance(value, float) and not value.is_integer(): return None try: @@ -349,8 +341,7 @@ def normalize_model_override(payload: dict[str, Any]) -> dict[str, Any]: speculative_type = _clean_str(payload.get("speculative_type"), VALID_SPECULATIVE_TYPES) if speculative_type: entry["speculative_type"] = speculative_type - # Only meaningful for the MTP modes; storing it otherwise would resurface - # in the UI as an edit the loader silently ignores. + # MTP-only; storing it otherwise shows an edit the loader ignores. if speculative_type in MTP_SPECULATIVE_TYPES: spec_draft_n_max = _bounded_int(payload.get("spec_draft_n_max"), minimum = 1, maximum = 16) if spec_draft_n_max: @@ -361,8 +352,8 @@ def normalize_model_override(payload: dict[str, Any]) -> dict[str, Any]: template = payload.get("chat_template_override") if isinstance(template, str) and template.strip(): - # JSON can carry lone surrogates, which encode() rejects outright. Such a - # template can never render, so it is dropped like any other bad field. + # A lone surrogate from JSON breaks encode(), and such a template can + # never render, so drop it like any other bad field. try: template_bytes = len(template.encode("utf-8")) except UnicodeEncodeError: @@ -370,13 +361,12 @@ def normalize_model_override(payload: dict[str, Any]) -> dict[str, Any]: if template_bytes <= MAX_CHAT_TEMPLATE_OVERRIDE_BYTES: entry["chat_template_override"] = template - # Only "manual" is a real override: persisting "auto" would pin the model and - # stop it following later changes to the global GPU memory preference. + # Only "manual" is a real override: "auto" would pin the model and stop it + # following the global GPU memory preference. if _clean_str(payload.get("gpu_memory_mode"), VALID_GPU_MEMORY_MODES) == "manual": entry["gpu_memory_mode"] = "manual" - # -1 is Auto (llama.cpp --fit owns layer sizing), which is also the default, - # so only a pinned count >= 0 is worth storing. + # -1 is Auto (llama.cpp --fit), which is the default, so only >= 0 is stored. gpu_layers = _bounded_int(payload.get("gpu_layers"), minimum = 0, maximum = 1024) if gpu_layers is not None: entry["gpu_layers"] = gpu_layers @@ -387,9 +377,8 @@ def normalize_model_override(payload: dict[str, Any]) -> dict[str, Any]: gpu_ids = payload.get("gpu_ids") if isinstance(gpu_ids, (list, tuple)) and gpu_ids: - # De-duplicate, preserving order: resolve_requested_gpu_ids rejects a - # repeated id outright, so storing [0, 0] would make every later API load - # of this model fail with a 400 that the picker never hits. + # De-duplicate, preserving order: resolve_requested_gpu_ids rejects a repeat, + # so storing [0, 0] would 400 every later API load of this model. cleaned_ids: list[int] = [] for gid in gpu_ids: parsed = _bounded_int(gid, minimum = 0, maximum = 1024) @@ -417,10 +406,8 @@ def resolve_fit_max_seq_length(override: dict[str, Any], *, is_gguf: bool) -> Op ) if manual_auto_layers: return override.get("custom_context_length") or 0 - # max_seq_length wins where both are set. The UI only ever sends it for a - # non-GGUF model (a GGUF's context is `custom_context_length`), so in - # practice the two never collide from that path; a hand-written or legacy - # entry that sets it on a GGUF is honoured, which is this API's contract. + # max_seq_length wins where both are set. The UI only sends it for non-GGUF + # models, so the two only collide in a hand-written or legacy entry. return override.get("max_seq_length") or override.get("custom_context_length") @@ -471,10 +458,9 @@ def _looks_like_filesystem_path(model_id: str) -> bool: return len(model_id) >= 3 and model_id[1] == ":" and model_id[2] in ("\\", "/") -# The three path shapes whose filesystem is case-insensitive, matching the rule -# the browser applies in features/hub/lib/model-identity.ts. Kept in step with -# it: the browser folds these before storing, so the two sides have to agree on -# which paths fold or a stored key becomes unreachable. +# The three case-insensitive path shapes. Must stay in step with +# features/hub/lib/model-identity.ts, which folds these before storing, or a +# stored key becomes unreachable. _WINDOWS_DRIVE_PATH = re.compile(r"^[A-Za-z]:[\\/]") _WSL_DRIVE_PATH = re.compile(r"^/mnt/[A-Za-z](?:/|$)") @@ -502,10 +488,9 @@ def _fold_case_insensitive_path(model_id: str) -> Optional[str]: return trimmed.casefold() -# A quant label may carry a bits-per-weight modifier, because two files at the -# same base quant are kept distinct by it ("IQ4_XS-3.53bpw"). The two label -# helpers disagree on whether to keep it, so anything reading a stored key has -# to accept both forms. +# A quant label may carry a bits-per-weight modifier ("IQ4_XS-3.53bpw") to keep two +# files at the same base quant distinct. The two label helpers disagree on whether +# to keep it, so readers of a stored key must accept both forms. _BPW_SUFFIX = re.compile(r"-[0-9]+(?:\.[0-9]+)?bpw$", re.IGNORECASE) _MAX_QUANT_SUFFIX_LEN = 64 @@ -529,13 +514,11 @@ def split_quant_suffix(value: str) -> Optional[tuple[str, str]]: _BPW_SUFFIX.sub("", tail) ): return head, tail - # A .gguf whose filename holds no recognizable quant token is still labelled - # by the scanner, which falls back to the stem, so keys like - # "/models/CustomModel.gguf:custommodel" exist. Storage lowercases that - # label while the scanner probes with the filename's own casing, so the - # comparison is case-insensitive. Requiring the suffix to be exactly that - # label is what keeps an ordinary colon out: "/models/foo:bar.gguf" splits - # to a head that is not a .gguf at all. + # A .gguf with no recognizable quant token is labelled by its stem, so keys like + # "/models/CustomModel.gguf:custommodel" exist. Storage lowercases the label + # while the scanner keeps the filename casing, hence the case-insensitive + # compare. Requiring exactly that label keeps an ordinary colon out: + # "/models/foo:bar.gguf" splits to a head that is not a .gguf. if not head.lower().endswith(".gguf"): return None filename = head.replace("\\", "/").rsplit("/", 1)[-1] @@ -590,12 +573,10 @@ def resolve_model_override_key(model_id: str) -> Optional[str]: return model_id if not isinstance(model_id, str): return None - # A POSIX path is case-sensitive and names a different file, so matching - # "/models/Foo.gguf" against an entry saved for "/models/foo.gguf" would - # replay another model's context and GPU pin. A Windows drive path, a UNC - # share and a WSL drive path are not case-sensitive, and the browser folds - # exactly those before storing, so refusing to fold them here would leave - # every migrated Windows entry unreachable until the user saved it again. + # A POSIX path is case-sensitive, so folding "/models/Foo.gguf" onto + # "/models/foo.gguf" would replay another model's settings. Windows drive, UNC + # and WSL paths are not, and the browser folds exactly those before storing, so + # not folding them here would strand every migrated Windows entry. if _looks_like_filesystem_path(model_id): folded = _fold_case_insensitive_path(model_id) if folded is not None: @@ -603,10 +584,9 @@ def resolve_model_override_key(model_id: str) -> Optional[str]: def fold(key: str) -> Optional[str]: return _fold_case_insensitive_path(key) else: - # POSIX: the path itself stays case-sensitive, but the browser - # lowercases the quant suffix while keeping the path casing, so a - # migrated "/models/Foo:q4_k_m" has to stay reachable from the - # scanner's "/models/Foo:Q4_K_M". + # POSIX: the path stays case-sensitive, but the browser lowercases the + # quant suffix, so "/models/Foo:q4_k_m" must be reachable from + # the scanner's "/models/Foo:Q4_K_M". folded = _fold_posix_path_variant(model_id) def fold(key: str) -> Optional[str]: diff --git a/studio/frontend/src/app/routes/__root.tsx b/studio/frontend/src/app/routes/__root.tsx index 7d89c0e83d..ad7845ebf9 100644 --- a/studio/frontend/src/app/routes/__root.tsx +++ b/studio/frontend/src/app/routes/__root.tsx @@ -70,10 +70,9 @@ const CHAT_ONLY_ALLOWED = new Set([ // Export stays reachable on chat-only hosts so the page can show its own grayed-out reason // instead of a silent redirect; it self-gates via export capability, so nothing runs. "/export", - // Chat-only hosts (Intel Macs, Apple Silicon without MLX, no-GPU boxes) serve - // the OpenAI-compatible API exactly like any other host, so the monitor has to - // be reachable there. Without this the floating panel's own "Expand" button - // and the Settings > API card both redirect to /chat. + // Chat-only hosts (Intel Macs, Apple Silicon without MLX, no-GPU boxes) serve the + // OpenAI-compatible API like any other host, so the monitor must be reachable + // there or the overlay's "Expand" and the Settings > API card redirect to /chat. "/api-monitor", ]); @@ -177,8 +176,7 @@ function RootLayout() { }, [documentTitle]); // Settings saved before the server-side override map existed live only in this - // browser, so an API load would use app defaults while the UI still showed the - // model as remembered. Backfill once, after auth. + // browser, so an API load would use app defaults. Backfill once, after auth. useEffect(() => { if (isAuthFlowRoute) { return; diff --git a/studio/frontend/src/app/routes/api.tsx b/studio/frontend/src/app/routes/api.tsx index 3a4da07f4d..d37d739122 100644 --- a/studio/frontend/src/app/routes/api.tsx +++ b/studio/frontend/src/app/routes/api.tsx @@ -12,9 +12,8 @@ const ApiMonitorPage = lazyRouteComponent( export const Route = createRoute({ getParentRoute: () => rootRoute, - // Not "/api": the backend owns that prefix (and "/v1"), and its SPA fallback - // deliberately 404s those paths so API clients get an API-shaped error rather - // than an HTML page. A deep link to /api would never reach the router. + // Not "/api": the backend owns that prefix (and "/v1") and its SPA fallback 404s + // those paths, so a deep link to /api would never reach the router. path: "/api-monitor", staticData: { title: "API" }, beforeLoad: () => requireAuth(), diff --git a/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx b/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx index 93d774cf7e..ae2344f4bd 100644 --- a/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx +++ b/studio/frontend/src/features/api-monitor/api-monitor-overlay.tsx @@ -30,11 +30,11 @@ import { computeStats } from "./use-api-monitor"; // Live cadence while the panel is on screen. const OPEN_POLL_MS = 1500; -// While closed the poll only has to notice that traffic started, so it backs off. +// Closed, the poll only has to notice traffic started, so it backs off. const IDLE_POLL_MS = 5000; // Requests shown in the panel; the rest are one click away on the full page. const VISIBLE_ENTRIES = 4; -// How long the API must be quiet before a dismissed panel will open itself again. +// Quiet time before a dismissed panel re-arms. const REARM_QUIET_MS = 60_000; const API_INFERENCE_PREFIX_RE = /^\/api\/inference/; @@ -90,7 +90,7 @@ function StatCell({ > {value} - {/* Sentence case: Unsloth metric rows read as words, not headers. */} + {/* Sentence case: metric rows read as words, not headers. */} {label} @@ -109,11 +109,10 @@ export function ApiMonitorOverlay(): ReactElement | null { ReturnType > | null>(null); - // One loop for both jobs: panel contents while open, traffic watch while - // closed. Stands down on the full page, which polls for itself. + // One loop for both jobs: panel contents while open, traffic watch while closed. + // Stands down on the full page, which polls for itself. useEffect(() => { - // Opted out and closed: the panel can neither open nor show anything, so - // polling would be pure background load on every open Studio window. + // Opted out and closed: nothing to open or show, so polling is pure load. if (onFullPage || (!autoOpen && !isOpen)) { return; } @@ -154,11 +153,11 @@ export function ApiMonitorOverlay(): ReactElement | null { const entries = useMemo(() => data?.entries ?? [], [data]); const stats = useMemo(() => computeStats(entries), [entries]); - // Ids already seen. A set, not "the newest id": finishing moves an entry to - // the front, so the head flips without any new traffic. + // Ids already seen. A set, not "the newest id": finishing moves an entry to the + // front, so the head flips without any new traffic. const seenIdsRef = useRef>(new Set()); - // Seeded on the first response even when empty, so the first request of a - // fresh session is not mistaken for history. + // Seeded on the first response even when empty, so the first request of a fresh + // session is not mistaken for history. const seededRef = useRef(false); const lastNewEntryAtRef = useRef(0); @@ -169,9 +168,8 @@ export function ApiMonitorOverlay(): ReactElement | null { const ids = data.entries.map((entry) => entry.id); if (!seededRef.current) { seededRef.current = true; - // Seed finished requests only. A request that is still running when the - // first snapshot lands started while Studio was loading, so it is live - // traffic the user has not seen, not history to adopt silently. + // Seed finished requests only: one still running at the first snapshot started + // while Studio was loading, so it is unseen live traffic, not history. seenIdsRef.current = new Set( data.entries .filter((entry) => entry.status !== "running") @@ -182,9 +180,8 @@ export function ApiMonitorOverlay(): ReactElement | null { } } const seen = seenIdsRef.current; - // Only API-key traffic counts. Studio's own chat goes through these same - // endpoints, and this panel is about serving other clients, not about the - // request the user is watching stream in front of them. + // Only API-key traffic counts: Studio's own chat uses these same endpoints, and + // this panel is about serving other clients. const hasNewTraffic = data.entries.some( (entry) => entry.via_api_key && !seen.has(entry.id), ); @@ -199,8 +196,7 @@ export function ApiMonitorOverlay(): ReactElement | null { if (!autoOpen || isOpen) { return; } - // A dismissal holds for that burst and re-arms only once the API goes - // quiet, so the next request cannot re-open it a second later. + // A dismissal holds for the burst and re-arms only once the API goes quiet. if (suppressed && quietFor < REARM_QUIET_MS) { return; } @@ -247,10 +243,8 @@ export function ApiMonitorOverlay(): ReactElement | null { initial={{ opacity: 0, scale: 0.94 }} animate={{ opacity: 1, scale: 1 }} exit={{ opacity: 0, scale: 0.94 }} - /* Panel language borrowed from the sidebar's user menu and the - model selector: no hard border, an inset hairline plus a soft - drop shadow (menu-soft-surface), a 20px corner, and the heading - font throughout. */ + /* Panel language from the sidebar's user menu and the model selector: + menu-soft-surface, a 20px corner, and the heading font throughout. */ className="menu-soft-surface pointer-events-auto fixed bottom-4 right-4 flex w-[400px] max-w-[calc(100vw-2rem)] cursor-default select-none resize flex-col overflow-hidden rounded-[20px] border-0 p-2.5 font-heading ring-0" >
@@ -302,7 +296,7 @@ export function ApiMonitorOverlay(): ReactElement | null { {data?.active_model ?? "No model loaded"}

- {/* Metrics on a soft tile, as the Hub and Train pages group readouts. */} + {/* Soft tile, as the Hub and Train pages group readouts. */}
- {/* Closing only silences this burst; this is the permanent off. */} + {/* Closing silences this burst; this is the permanent off. */}