Studio: kill in-flight llama-server before spawning a new one (#5171)

* Studio: kill in-flight llama-server before spawning a new one

Two rapid Apply clicks in the chat settings panel can race two
load_model calls. Both pass the Phase 1 _kill_process (because neither
has stored its Popen handle yet), both download / read metadata, and
both reach Phase 3 and spawn a server. Only the last reference is
tracked in self._process. The first server becomes an orphan that
holds the model in RAM until the kernel OOM kicks in. Addresses #5161.

Two complementary changes in studio/backend/core/inference/llama_cpp.py:

1. At load_model entry, set the existing _cancel_event so any in-flight
   load aborts at its next checkpoint, then bind a fresh Event for
   the new load. Subsequent _kill_process and download phases pick up
   the new event.
2. Inside the Phase 3 lock, immediately before subprocess.Popen, run a
   defensive _kill_process that removes any orphan handle a racing
   load might have stored after the first kill ran.

Speculative decoding cleanup (also touched while in this code path):

The chat UI used to send "ngram-mod" as the wire value when the
speculative dropdown was On, and the backend mapped that to the
4-flag combo --spec-type ngram-mod --spec-ngram-size-n 24 --draft-min
48 --draft-max 64. Switch the wire value to "default" and have the
backend pass the single llama-server flag --spec-default. That flag
expands to the exact same params (see common/arg.cpp:3905-3914 in
llama.cpp). Default-on for non-vision models is preserved.

Verified:
- Unit test: planted Popen orphan handle is terminated before
  self._process is overwritten.
- All ten spec-cmd mappings produce the expected llama-server args
  ("default" -> --spec-default, "off" / null -> no flag, vision ->
  always disabled, manual "ngram-mod" / "ngram-simple" still work).

* Address review feedback on PR #5171

The previous attempt at cancelling in-flight loads via
``self._cancel_event.set()`` followed by
``self._cancel_event = threading.Event()`` was broken in two ways
(flagged independently by Gemini and Codex):

1. Sub-methods like _download_gguf consult ``self._cancel_event``
   on every check. After the rebind, the in-flight thread reads the
   FRESH unset Event, not the one we just set, so cancellation never
   propagates.

2. Worse, if unload_model() lands between ``set()`` and the rebind,
   unload's signal hits the OLD event and is then immediately
   discarded when load_model swaps in a fresh Event. The user's
   stop-request silently no-ops.

Revert to the original ``self._cancel_event.clear()``. The Phase 3
defensive ``_kill_process()`` introduced in this branch still closes
the orphan-process race that #5161 reports: even if two concurrent
loads both pass Phase 1 with self._process == None, the loser's
Phase 3 kill terminates the winner's Popen handle before overwriting
it, so we end up with exactly one llama-server process.

* Studio: coerce legacy speculative-type values for the simplified dropdown

The Speculative Decoding control was simplified to On (default) / Off,
but the backend still accepts and reports the older manual modes
(ngram-mod, ngram-simple). When a load response or status refresh comes
back with one of those values -- whether from an external API caller, a
model loaded before this PR landed, or a not-yet-upgraded backend -- the
controlled Select renders with an empty trigger because the value is not
in the SelectItem list.

Add a tiny normaliser at both entry points (status refresh + post-load)
so legacy manual modes coerce to "default". The user sees "On" instead
of a blank dropdown, and reapplying lets llama.cpp pick its own preferred
strategy via --spec-default.

Reviewer-flagged finding on PR #5171.

studio/backend/core/inference/llama_cpp.py

@@ -1574,11 +1574,21 @@ class LlamaCppBackend:
             # ref: https://github.com/ggml-org/llama.cpp/blob/master/docs/speculative.md
             # ref: https://github.com/ggml-org/llama.cpp/pull/19164
             # ref: https://github.com/ggml-org/llama.cpp/pull/18471
+            # ``"default"`` -> let llama-server pick a sensible spec
+            # config via ``--spec-default``. Explicit type names are
+            # passed through with the manual draft tuning we've shipped
+            # historically so power users keep their overrides.
             _valid_spec_types = {"ngram-simple", "ngram-mod"}
-            if speculative_type and speculative_type in _valid_spec_types:
-                if not is_vision:  # spec decoding disabled for vision models
-                    cmd.extend(["--spec-type", speculative_type])
-                    if speculative_type == "ngram-mod":
+            normalized_spec = (
+                speculative_type.lower().strip() if speculative_type else None
+            )
+            if normalized_spec and normalized_spec != "off" and not is_vision:
+                if normalized_spec == "default":
+                    cmd.append("--spec-default")
+                    self._speculative_type = "default"
+                elif normalized_spec in _valid_spec_types:
+                    cmd.extend(["--spec-type", normalized_spec])
+                    if normalized_spec == "ngram-mod":
                         cmd.extend(
                             [
                                 "--spec-ngram-size-n",
@@ -1589,7 +1599,7 @@ class LlamaCppBackend:
                                 "64",
                             ]
                         )
-                    self._speculative_type = speculative_type
+                    self._speculative_type = normalized_spec
                 else:
                     self._speculative_type = None
             else:
@@ -1750,6 +1760,12 @@ class LlamaCppBackend:
             if gpu_indices is not None:
                 env["CUDA_VISIBLE_DEVICES"] = ",".join(str(i) for i in gpu_indices)
 
+            # Defensive kill: if a concurrent load slipped past Phase 1
+            # (because its `self._process` was None at the time) and
+            # already stored a Popen handle here, drop that orphan
+            # before we overwrite the reference. See issue #5161.
+            self._kill_process()
+
             self._stdout_lines = []
             self._process = subprocess.Popen(
                 cmd,

studio/frontend/src/features/chat/chat-settings-sheet.tsx

@@ -1037,7 +1037,7 @@ export function ChatSettingsPanel({
                           <SelectValue />
                         </SelectTrigger>
                         <SelectContent>
-                          <SelectItem value="ngram-mod">On</SelectItem>
+                          <SelectItem value="default">On</SelectItem>
                           <SelectItem value="off">Off</SelectItem>
                         </SelectContent>
                       </Select>

studio/frontend/src/features/chat/hooks/use-chat-model-runtime.ts

@@ -24,6 +24,19 @@ import type {
   InferenceParams,
 } from "../types/runtime";
 
+// The simplified Speculative Decoding control surfaces "default" (which
+// maps to llama.cpp's --spec-default) and "off". A backend status / load
+// response can still report the older manual modes (ngram-mod,
+// ngram-simple) when a model is loaded via the API or carried over from an
+// older Studio version. The Select would render an empty trigger for those
+// values, so coerce them to "default" -- llama.cpp's own --spec-default
+// picks an equivalent strategy and keeps the dropdown coherent.
+function normalizeSpeculativeType(v: string | null | undefined): string | null {
+  if (v == null) return null;
+  if (v === "default" || v === "off") return v;
+  return "default";
+}
+
 type SelectedModelInput = {
   id: string;
   isLora?: boolean;
@@ -279,7 +292,7 @@ export function useChatModelRuntime() {
         const ggufNativeContextLength = statusRes.is_gguf
           ? (statusRes.native_context_length ?? null)
           : null;
-        const currentSpecType = statusRes.speculative_type ?? null;
+        const currentSpecType = normalizeSpeculativeType(statusRes.speculative_type);
         useChatRuntimeStore.setState({
           supportsReasoning,
           reasoningAlwaysOn,
@@ -492,7 +505,7 @@ export function useChatModelRuntime() {
               }
             }
             const loadedKv = loadResponse.cache_type_kv ?? null;
-            const loadedSpec = loadResponse.speculative_type ?? null;
+            const loadedSpec = normalizeSpeculativeType(loadResponse.speculative_type);
             const nativeCtx = loadResponse.is_gguf
               ? (loadResponse.context_length ?? 131072)
               : null;

studio/frontend/src/features/chat/stores/chat-runtime-store.ts

@@ -270,7 +270,7 @@ export const useChatRuntimeStore = create<ChatRuntimeStore>((set) => ({
   toolCallTimeout: loadInt(TOOL_CALL_TIMEOUT_KEY, 5),
   kvCacheDtype: null,
   loadedKvCacheDtype: null,
-  speculativeType: "ngram-mod",
+  speculativeType: "default",
   loadedSpeculativeType: null,
   customContextLength: null,
   defaultChatTemplate: null,
@@ -365,7 +365,7 @@ export const useChatRuntimeStore = create<ChatRuntimeStore>((set) => ({
       toolStatus: null,
       kvCacheDtype: null,
       loadedKvCacheDtype: null,
-      speculativeType: "ngram-mod",
+      speculativeType: "default",
       loadedSpeculativeType: null,
       customContextLength: null,
       defaultChatTemplate: null,