022 spec: registry-driven LLM serving + operator model selection

[kurtvalcorza]

Spec-only PR — the complete spec-kit artifact set for feature 022 (no implementation code). Reviewing the design before building it, the way 021's spec landed via #62.

Why

Driving the merged 021 console end-to-end surfaced that the LLM is the only engine that ignores the registry. Every other child (vision/embed/ASR/tabular) resolves its @serving version's artifact from MLflow on each cold load; the llama.cpp adapter alone loads a fixed GGUF file path from an env var. So four quirks:

1. Promoting an LLM in the console does nothing (env-fixed, not registry-driven) — changing the served LLM needs a host .env edit + agent restart.
2. Fine-tunes register without a task tag → show as "no renderer", not selectable.
3. Serving a fine-tune needs base + LoRA adapter — the engine couldn't express it.
4. The served-model label diverges from what actually runs → predictions logged against the wrong model+version, corrupting the quality-monitoring window.

(All four reproduced live: I trained ops-bot-v2, and only a host-level override could serve it.)

What the spec proposes

Make the LLM a registry-driven, console-operable engine like the others: promoting a text-generation version (a full base or a LoRA fine-tune) makes it serve, under the single-GPU lease, with honest agent-reported identity everywhere.

5 user stories — US1 promote-to-serve (P1), US2 honest identity (P1), US3 base+adapter serving (P2), US4 fine-tunes discoverable + backfill (P2), US5 safe switch under the lease (P3, [HW]). FR-254…275, SC-143…151, 29 tasks (T461–T489).

Artifacts in this PR

specs/022-registry-driven-llm-serving/ — spec.md, plan.md, research.md, data-model.md, contracts/ (serving-resolution + agent-identity/allow-list), quickstart.md, tasks.md, checklists/requirements.md. (+ the .specify/feature.json + CLAUDE.md marker bumps.)

Design decisions worth reviewing

• R1 (load-bearing): a platform-scoped active-serving-LLM pointer (in the relational store), not an env-pinned model name — because operators switch between distinct LLM models (base vs a differently-named fine-tune), unlike vision which pins one model name. This is the biggest commitment.
• R2: serve fine-tunes as -m base --lora adapter (register bases as first-class full-model versions; resolve the base from lineage) rather than merge-on-promote.
• Clarifications (2026-07-05): promote = immediate go-live (one action, no separate "select"); operator-only switching (a retraining policy may gate but must not auto-switch the LLM — FR-275); scope kept to the LLM serving quirks (dataset byte-download / streamed-prediction logging deferred).

Process already run on these artifacts

• /speckit-specify → /speckit-plan → /speckit-tasks → /speckit-clarify (4 decisions ratified) → /speckit-analyze (0 critical / 0 high; 5 MEDIUM/LOW findings remediated).
• Quality checklist 16/16 PASS; Constitution Check PASS (Principle II — the switch reuses the existing one-tenant swap; never two-resident; jobs never preempted; [HW]-validated in T484).

Please review the spec design — especially R1 (the active-LLM pointer vs an env-pinned name), the Principle II framing of the switch, and whether the scope boundary (LLM-only) is right.

@claude review this spec design
@codex review

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[kurtvalcorza]

@claude please review the spec design in this spec-only PR (feature 022 — making the LLM a registry-driven, console-operable engine).

Focus on the design, not code (there is none):

• R1 — the active-serving-LLM pointer. I chose a platform-scoped store pointer (not an env-pinned model name like vision) so operators can switch between differently-named LLM models. Is that the right mechanism, or is there a simpler registry-native option (e.g. a reserved alias) I'm missing?
• Principle II framing. The served-LLM switch reuses the existing host-agent swap (evict→load, never two-resident, jobs never preempted, operator-confirmed). Does the spec fully preserve the one-tenant invariant, and are FR-257/258/259 airtight?
• Base+adapter resolution (R2). Serve fine-tunes as -m base --lora adapter, base resolved from lineage as a registered base version. Any failure mode (missing base, chained parents) the spec doesn't handle?
• Coverage/consistency. Every FR-254..275 mapped to a task? Any contradiction between the clarifications (promote=go-live, operator-only) and the tasks/contracts?
• Scope. Is LLM-only the right boundary, or should any of the deferred quirks be pulled in?

Flag any real gap, unstated assumption, or Principle II risk. (@codex is auto-reviewing on open.)

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Claude finished @kurtvalcorza's task in 3m 22s —— View job

---

Claude's review of the 022 spec design

Todo list

• Gather context (diff, PR body, comments)
• Read spec.md, plan.md, research.md, data-model.md, contracts, tasks.md
• Review R1 — active-serving-LLM pointer mechanism
• Review Principle II framing / FR-257/258/259
• Review R2 — base+adapter resolution failure modes
• Check FR-254..275 → task coverage & clarification consistency
• Review scope boundary
• Post final review

This is a well-reasoned spec (research.md's R1–R8 rationale is unusually thorough), but I cross-checked the design against the actual llama.cpp adapter / swap / admission / registry code it says it "reuses," and found three concrete gaps between what the design assumes those components do and what they actually do today.

---

1. The "reload-on-promote" path may be a no-op in the most common case (FR-255/257, SC-144)

Research R4 and task T466 say switching the served LLM "reuses preempt_for wholesale." But preempt_for treats same-tenant as already-satisfied:

# hostagent/swap.py:56-59
holder = admission.holder()
if holder is None or holder["tenant"] == target_engine_id:
    load_ms = target.ensure_loaded()
    return {"swapped": False, "evicted": None, "load_ms": load_ms}

and ensure_loaded() short-circuits when the engine is already resident+ready, without re-spawning:

# hostagent/lifecycle.py:89-92
if self._resident() and self.adapter.ready():
    self.last_used = self.clock()
    return 0.0

Admission's tenant identity is per engine ("llm"), not per model. So the scenario in US1's own Acceptance Scenario 2 — "version A is serving, operator promotes version B" — is exactly the case where holder["tenant"] == "llm" == target_engine_id is already true (the LLM engine already holds the slot). preempt_for/ensure_loaded would treat this as a no-op and never call spawn() again, so the already-running llama-server child keeps serving the old GGUF — the promote would silently not take effect until the engine happens to idle-release and cold-load again. This is the opposite of FR-255's "immediate controlled reload."

Reusing the swap machinery correctly here needs an explicit same-tenant force-reload path (unload → ensure_loaded) distinct from preempt_for's cross-engine eviction logic — it isn't automatic from "reuse preempt_for," and none of research.md R4, contracts/serving-resolution.md, or T466 call this distinction out.

Fix this →

---

2. FR-262 (no divergence between "internal display alias" and registry record) isn't fully covered by a task

LlamaAdapter.forward()/.stream() echo self.alias in the direct /infer response's "model" field:

# hostagent/adapters/llama.py:45-46, 106-112, 123, 141-142
self.alias = os.getenv("MODEL_ALIAS", DEFAULT_ALIAS)
...
return {"text": ..., "model": self.alias, ...}

self.alias is set once, from a static env var, at __init__ — it's a separate field from whatever T463's resolver produces (model_name/version). T469 only says the agent reports actual identity "in /engines/llm/health and the aggregate health"; T471 covers gateway-side prediction logging. Neither task explicitly touches forward()/stream()'s self.alias echo. Without that, the raw per-request inference response can keep showing a static configured alias even after /serving/state and logged predictions are fixed — reproducing precisely the "display alias diverges from registry record" bug FR-262 exists to close, just one layer deeper (the adapter's own response, not just the gateway's derived fields).

Fix this →

---

3. list_tasks() isn't taught about the active-serving-LLM pointer, so it can show stale/duplicate LLM panels

promote() moves only the promoted model's own @serving alias; nothing un-sets a previously promoted model's @serving alias when a different model becomes the active LLM. registry.list_tasks() — which backs GET /serving/tasks, the endpoint the console's LLM panel (T479, FR-269/270) reads — emits one row per registered model with any promoted @serving version:

# gateway/app/registry.py:224-250 (list_tasks)
for m in models:
    sv = (dict(m.aliases or {})).get(SERVING_ALIAS)
    if not sv: continue
    ...
    out.append({"model": m.name, "task": tags.get(TASK_TAG), ...})

resolve_serving_target already has a prefer_name parameter added specifically to disambiguate "several models share a task" (its own docstring cites a prior Codex review for exactly this). list_tasks() has no equivalent. Once an operator has promoted two different text-generation models over the platform's lifetime (e.g., the base once, then ops-bot-v2 later), both can keep appearing as separate task: "text-generation" rows — the console would need to know to prefer the ActiveServingLLM pointer's model, but neither data-model.md's LLMServingTarget nor T479 mention deduplicating/filtering list_tasks() by the active pointer.

Fix this →

---

Answers to the specific questions

R1 (active-serving-LLM pointer): The mechanism itself is sound and the rejected alternatives (env-pinned name, implicit "first @serving match," a synthetic MLflow alias) are correctly dismissed — a platform pointer that spans model names is the right shape given the requirement to switch between differently-named models. The gap isn't the pointer design, it's its downstream consumers (§3 above) not yet being pointer-aware.

Principle II framing: FR-257/258/259 are airtight for the cross-tenant case (job never preempted, resident serving model requires confirm) — that logic in swap.py/admission.py is solid and well-tested per the code comments (multiple prior Codex-review hardening rounds are already reflected there). The risk isn't a Principle II violation (no two-resident window opens) — it's the narrower correctness bug in §1: the same-tenant reload can be a silent no-op, which threatens FR-255/SC-144 without ever threatening the one-tenant invariant. Worth tightening the language so "reuses the existing swap" doesn't imply preempt_for alone is sufficient.

Base+adapter resolution (R2): Missing/unavailable base is well handled (FR-265, refuse-with-reason, T474). One unstated failure mode: chained lineage (an adapter fine-tuned from another adapter, not directly from a full-model base). T473 lists "chained-parent" as a test case, but neither the contract (serving-resolution.md) nor data-model.md state whether base_model resolution walks a multi-hop chain or must point directly at a kind=full-model version. Worth an explicit line: "adapters may only reference a full-model base directly; a base_model pointing at another adapter is a resolution error" (or the alternative, if chaining is meant to be supported).

Coverage/consistency: FR-254..261, 263-275 all map cleanly to tasks. FR-262 is the one requirement with a real gap in task coverage (§2). No contradictions found between the clarifications (promote=go-live, operator-only, immediate reload) and tasks.md — T467/T475/T481/T485(b) correctly encode all three.

Scope: LLM-only is the right boundary. The two deferred items (021 FR-215 dataset byte-download, 016 streamed-prediction full logging) are genuinely orthogonal to the registry/serving-identity work here — pulling them in would dilute a well-scoped feature.

---

Summary: solid spec, no Principle II violations, but two of the "reuses existing X" claims (the swap for same-tenant reload, and list_tasks() for the console panel) don't hold up against the actual code they point to, and FR-262 has a task-coverage gap. I'd resolve findings §1 and §2 before implementation starts — they sit directly on the MVP (US1/US2) critical path; §3 matters once US4/US5 land (multiple promotable LLMs coexisting).

[kurtvalcorza]

@claude thanks — all three findings were real and code-grounded; fixed in-spec in a7b5ac8. (@codex still rate-limited, so acting on your verdict.)

§1 (headline — same-tenant reload no-op): Confirmed against swap.py:57-59 + lifecycle.py:93-95 — since admission tenancy is per-engine ("llm"), promoting model B while A is resident hits holder["tenant"]=="llm"==target → ensure_loaded() short-circuits on resident+ready → old GGUF keeps serving. Exactly the US1 MVP path. Fixed: research R4 + contracts/serving-resolution.md + T466 now split cross-tenant (reuse preempt_for) vs same-tenant model switch (explicit force-reload: unload → ensure_loaded), with the idempotent no-op narrowed to the same model+version.

§2 (FR-262 adapter response echo): T465 now sets the adapter's response model identity from the resolved model_name/version on each load, not the static MODEL_ALIAS env — so /infer's model field matches the registry record, not just /serving/state.

§3 (stale serving/tasks duplicates): Added FR-276 + T479 — serving/tasks is now filtered/preferred to the active-serving-LLM pointer (mirroring resolve_serving_target's prefer_name), so a previously-promoted LLM whose @serving alias is still set doesn't show as a duplicate live target.

R2 note: contract + data-model now state base_model must resolve directly to a kind=full-model base; an adapter-of-adapter is a resolution error (no multi-hop).

Please re-review. Your Principle-II read (no two-resident window; the risk was the same-tenant no-op, not a lease violation) matched my re-check exactly.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Claude finished @kurtvalcorza's task in 1m 18s —— View job

---

Re-review of a7b5ac8 — all 3 findings verified fixed

• Gather context (diff, PR body, comments)
• Verify §1 fix — same-tenant force-reload split (research R4, contracts/serving-resolution.md, T466)
• Verify §2 fix — T465 adapter response identity from resolved model_name/version
• Verify §3 fix — FR-276 + T479 serving/tasks filtered by active-serving-LLM pointer
• Verify R2 note — base_model must resolve directly to a full-model base (no multi-hop)
• Check clarifications consistency
• Post final verdict

I re-read the actual code each fix is grounded in (hostagent/swap.py, hostagent/lifecycle.py, hostagent/adapters/llama.py, gateway/app/registry.py) to confirm the spec text is now accurate, not just re-reading the spec diff in isolation.

§1 — same-tenant force-reload (research.md R4, contracts/serving-resolution.md, T466). Confirmed correct. swap.py:57-59 (if holder is None or holder["tenant"] == target_engine_id: load_ms = target.ensure_loaded()) really does treat same-engine-different-model as satisfied, and lifecycle.py's ensure_loaded() really does short-circuit at if self._resident() and self.adapter.ready(): return 0.0 — so the fix's characterization of the bug is exact. The proposed remedy (explicit unload→ensure_loaded() force-reload, distinct from preempt_for) is buildable from primitives that already exist: EngineRuntime.unload() is a real method swap.py itself calls on the holder, so this isn't inventing new machinery, just sequencing existing ones differently for the same-tenant case. Good catch on also probing the target before the force-reload's unload — mirrors the existing pre-evict guard.

§2 — adapter response identity (T465). Confirmed. llama.py:45 sets self.alias once from MODEL_ALIAS env, and it's echoed in forward() ("model": self.alias, ~line 108), stream()'s start/done frames (~lines 123, 141), and health(). T465 now says to set this from the resolved model_name/version on each load instead of leaving it env-pinned — that closes the gap precisely where it existed (the raw /infer//infer/stream response, one layer below the gateway-derived fields T469 already covered). Worth noting for whoever implements T465: health() also reads self.alias (line ~145), so as long as the same field is updated on load, health stays consistent for free — the task doesn't need to separately touch health().

§3 — serving/tasks dedup (FR-276, T479, data-model.md). Confirmed. registry.py already has prefer_name on resolve_serving_target (line 186) for exactly this kind of disambiguation, but list_tasks() (line 224) has no equivalent — it's a plain per-model loop with no filtering. FR-276 + the T479 addendum correctly direct list_tasks/serving/tasks to filter/prefer by the active-serving-LLM pointer, mirroring the existing pattern rather than inventing a new one. No new FR-ID collision (FR-275 was the clarification's addition, FR-276 is this fix's — sequential, checklist's ID-space note updated accordingly).

R2 note (base-resolution, no multi-hop). The contract and data-model additions are unambiguous: base_model must resolve directly to a kind=full-model version; adapter-of-adapter is a resolution error under FR-265's existing refuse-with-reason path, not a new error class. Clean.

Consistency check. No contradictions between this commit and the 2026-07-05 clarifications (promote=go-live, operator-only, immediate reload) — the force-reload path is exactly what makes "immediate reload" (FR-255) actually true in the common same-tenant case, rather than undermining it. Checklist's "16/16 PASS" claim and ID-space accounting both check out against the diff.

Verdict

All three findings from my previous pass were real, and this commit fixes them correctly and minimally — each remedy reuses an existing primitive (unload(), the prefer_name pattern, the resolved-identity plumbing already threaded through T463) rather than adding new mechanism. I don't see any new gap introduced by the fix itself. Nothing further to flag before implementation starts on US1/US2's critical path.