022: registry-driven LLM serving + operator model selection (offline slice, T461–T488)#65
022: registry-driven LLM serving + operator model selection (offline slice, T461–T488)#65kurtvalcorza wants to merge 2 commits into
Conversation
…8, offline slice) Make the text-generation engine registry-driven like the other four: promoting an LLM version from the console IS the go-live action (FR-254/255), with honest served identity end-to-end and base+LoRA-adapter resolution from lineage. Setup + foundational: - platformlib/store: ActiveServingLLM singleton pointer table + accessors (T461; mirrored in infra/postgres/init.sql; additive, no version bump) - scripts/register_base_gguf.py: register local zoo bases as kind=full-model versions with base_id tags, idempotent, never downloads (T462, R2/R7) - platformlib/llmresolve.py: ONE registry walk (name -> @Serving -> kind -> base) shared by agent + gateway so the two resolution sites cannot drift - hostagent/serving_llm.py: cold-load resolver -> {base_gguf, adapter_gguf, model_name, version}; ResolutionError (refuse, FR-265) vs ResolutionUnavailable (env fallback, surfaced) (T463) US1 promote -> serve: - llama adapter rebinds base/adapter/alias/version from the registry at each cold load (TTL-cached on the health path); MODEL/LORA/MODEL_ALIAS env knobs become the fallback default; spawn appends --lora for adapters (T465) - swap.reload_serving_llm: cross-tenant preempt_for reuse + the same-tenant FORCE-RELOAD (unload -> ensure_loaded) preempt_for silently no-ops on; idempotent no-op, target-probe before evict, jobs never preempted, operator confirm (preempt=true) for any displacement (T466, PR #64 s1) - agent POST /control/reload (secret-gated, both transports) (T466) - gateway promote route: pre-gate resolvability refusal (409, alias unmoved), pointer write + reload request after a promoted alias move (T467, T474) US2 honest identity: - agent /engines/llm/health reports model_name + registry_version + base + adapter of what is ACTUALLY resident (T469) - gateway gpu_state / serving/state / /infer response / prediction logging all consume the agent-reported identity; degrade to unknown, never a config guess (T470/T471 - kills the live mislabeled-window divergence) US3/US4 fine-tunes first-class: - adapter path: base_model lineage -> registered full-model base, single hop, adapter chains refused (T473); finetune flow stamps task/serving_engine at registration (T476); scripts/backfill_llm_task_tags.py for legacy versions, idempotent + non-clobber (T477); kind->task fallback in resolve/list (T478); serving/tasks carries kind+lineage and filters text-generation to the active pointer - no stale duplicate LLM entries (T479, FR-276) US5 console switch (offline slice): - PromoteGate: promote-as-switch with the ConfirmDialog naming the displaced model, resident-vs-promoted delta, reload outcome surfaced (T482) - allow-list delta EMPTY: rides existing promote + serving/state (T483) - StreamPanel shows base-vs-adapter provenance (FR-268/274) Polish: - boundary guards: FR-273 no new resident process; FR-275 the policy/auto- promote path structurally cannot switch the served LLM (T485) - README serving section + tasks.md; AGENT_CONTROL_SECRET into gateway env Tests: 534 passed, 39 skipped (was 471/39); the 3 remaining failures are the pre-existing PIL-missing environment ones, unchanged. next build green. [HW] tail (T484/T489) + the live quickstart drills stay open for the GPU box. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hoYEa8FN5s8eGqa78Fxjx
|
@claude please review this PR — the offline slice of increment 022 (registry-driven LLM serving). Focus areas where a second pair of eyes is most valuable:
Backend Generated by Claude Code |
|
Claude finished @kurtvalcorza's task in 6m 16s —— View job Review complete
Overall this is a well-reasoned slice — the swap/reload logic, the shared resolver, and the identity plumbing are carefully thought through, and the docstrings/comments make the intent easy to follow. Found three concrete gaps in the focus areas, ranked by severity. 1.
|
…, stream trace identity Three concrete gaps from the @claude review, all in the flagged focus areas: 1. hostagent/swap.py:reload_serving_llm — the same-tenant force-reload unloaded the working model then called ensure_loaded() with NO rollback; a target that passes available() (file check) can still fail at llama-server spawn/readiness (corrupt GGUF, OOM, port), leaving the llm engine cold/empty — and rebind() had already discarded the old binding. Now snapshot the prior binding BEFORE rebind, and on a post-probe load failure restore + best-effort reload it (mirroring preempt_for's cross-tenant rollback), then propagate the error. Adds LlamaAdapter.snapshot_binding/restore_binding (restore freezes the binding so the recovery reload re-spawns the restored model, not the bad target). New test exercises the post-probe spawn failure -> rollback path. (Principle II / FR-257; data-model.md "serving is never left empty".) 2. platformlib/llmresolve.py:_is_not_found — the "not found" substring fallback was broader than the module contract: a transport 404 from a misconfigured MLFLOW_TRACKING_URI/proxy got mis-normalized to "does not exist" -> refuse (ResolutionError) instead of propagating -> env-default fallback (ResolutionUnavailable). Narrowed to the mlflow-specific RESOURCE_DOES_NOT_EXIST code (attribute or verbatim string). Tests cover a "not found"-carrying transport error and the unit classifier. 3. gateway/app/routers/stream.py — the MLflow trace still attributed to the static SERVING_MODEL at both trace sites, inconsistent with infer.py and this file's own prediction log. Resolve the agent-reported identity ONCE up front (agent reachable post-health-check; a request holds the GPU lease for its whole generation so identity can't drift) and thread it through both the trace and the log; the pre-generation failure trace reports "unknown" (agent down). Drops the detached _log task (identity is pre-resolved; log_prediction is already fire-and-forget) and the now-unused background import. (FR-261/262.) The minor list_tasks stale-pointer note is left as-is: a broken pointer already surfaces via the agent's ResolutionError, and dedup-to-one is the intended FR-276 behavior even then. pytest: 537 passed, 39 skipped (was 534/39; +3 new tests). The 3 failures remain the pre-existing PIL-missing environment ones, unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hoYEa8FN5s8eGqa78Fxjx
|
Thanks — all three findings were valid and are fixed in
The minor
Generated by Claude Code |
|
@claude please re-review — commit
Generated by Claude Code |
|
Claude finished @kurtvalcorza's task in 3m 29s —— View job Re-review complete
All three fixes in 1. Same-tenant reload rollback — freeze is sufficient, double-failure is clean
Double-failure (recovery One nuance worth noting (not a bug, but worth being aware of): 2.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Route LLM overrides through the switch confirmation
When the gate-blocked version is an LLM and any serving model is resident, this override path bypasses promoteLLMAware/askSwitch and sends preempt: false. The API still moves @serving and the active pointer, but the agent refuses the reload with “confirmation required”; because the version is now marked serving, the console disables the promote button, leaving no UI path to send the required preempt: true and the override does not go live until a cold/manual reload.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| instead of a spawn crash. Rebinds first (TTL-bounded) so the swap/reload target-probe | ||
| checks the artifacts that WOULD load — an unresolvable serving-LLM target or a missing | ||
| base/adapter GGUF refuses here rather than evicting a working holder (FR-265).""" | ||
| self.rebind() |
There was a problem hiding this comment.
When an LLM child is already resident, /engines/llm/health still calls available(), so this rebinds to the newly selected pointer and validates the new artifact instead of the loaded child. If a promote writes the pointer but the immediate reload is refused because the new full-model/adapter source is missing, health starts returning 503 even though the old child was intentionally not evicted and can still serve; the gateway's serving.health() then rejects /infer, turning a refused switch into an outage. The resident health path should remain based on _loaded or avoid rebinding while resident.
Useful? React with 👍 / 👎.
| state = null; | ||
| } | ||
| setLiveState(state); | ||
| if (state?.resident) { |
There was a problem hiding this comment.
When a non-LLM serving engine such as vision holds the GPU, /serving/state reports holder: 'vision' but resident: false because resident is only whether the LLM runtime is loaded. This check therefore skips the confirmation and promotes with preempt: false; the backend moves the alias and active pointer, then /control/reload refuses because a resident engine would be displaced, so the operator's requested LLM switch does not go live from the normal promote flow.
Useful? React with 👍 / 👎.
| if len(text_gen) > 1: | ||
| active = active_serving_llm_name() | ||
| keep = next((e for e in text_gen if e["model"] == active), text_gen[0]) | ||
| out = [e for e in out if e["task"] != llmresolve.TEXT_GENERATION or e is keep] |
There was a problem hiding this comment.
This only applies the active-pointer filter when two or more text-generation aliases exist. On an upgraded install with one pre-022 promoted LLM alias and no serving_llm pointer, or whenever the sole promoted alias is not the active model, /serving/tasks still advertises that stale alias as the live LLM even though the agent serves the configured default/active pointer, so the Infer tab can show and route against the wrong registry identity.
Useful? React with 👍 / 👎.
| conn = s.connect() | ||
| try: | ||
| s.set_serving_llm(conn, model_name, time.time(), actor) |
There was a problem hiding this comment.
On a Postgres volume created before this migration, infra/postgres/init.sql will not run again, and this new pointer path opens a connection but never calls store.bootstrap() like the existing quality/policy/job clients do. A first LLM promote on such an upgraded install can move the MLflow alias and then fail here with relation "serving_llm" does not exist, leaving the switch unapplied until some other subsystem happens to bootstrap the additive DDL.
Useful? React with 👍 / 👎.
| # 022 (FR-255): the gated promote requests the agent's /control/reload so a promoted LLM | ||
| # goes live at once. Empty when the agent runs open (the default); must MATCH the agent's | ||
| # AGENT_CONTROL_SECRET when one is set (sent as X-Agent-Control). | ||
| AGENT_CONTROL_SECRET: ${AGENT_CONTROL_SECRET:-} |
There was a problem hiding this comment.
The agent still accepts SWAP_CONTROL_SECRET as a compatibility fallback, and hostagent/run.sh sources .env, but the gateway container only receives AGENT_CONTROL_SECRET here. In an existing deployment that set only SWAP_CONTROL_SECRET, the agent protects /control/reload while the gateway sends no X-Agent-Control header, so an LLM promote moves the alias/pointer but every immediate reload is refused with 403.
Useful? React with 👍 / 👎.
| onClick={() => promoteLLMAware(v)} | ||
| disabled={v.serving || busy === v.version} |
There was a problem hiding this comment.
With the new active pointer, v.serving only means this model has its own @serving alias; it does not mean this LLM is the active served model. When two LLM models each retain a serving alias and the operator wants to switch back to the inactive one, this LLM-aware promote handler is disabled by the adjacent v.serving check, so the UI provides no way to re-promote the same version to rewrite the active pointer and reload it.
Useful? React with 👍 / 👎.
2 participants
Implements the OFFLINE-executable slice of increment 022 registry-driven LLM serving (
specs/022-registry-driven-llm-serving/) — the LLM becomes a registry-driven, console-operable engine like vision/embed/ASR/tabular. Promote = go live (Clarifications 2026-07-05): one operator action moves the@servingalias, records the active serving LLM, and triggers the immediate controlled reload.What landed, by phase
Setup + Foundational (T461–T464)
platformlib/store: ActiveServingLLM singleton pointer table + accessors (mirrored ininfra/postgres/init.sql; additive, no schema-version bump).platformlib/llmresolve.py: ONE registry walk (name →@serving→ kind → base) shared by agent and gateway — they cannot drift (the gateway image only shipsgateway/app+platformlib, so the shared home is platformlib, stdlib-only, duck-typed client).hostagent/serving_llm.py: the cold-load resolver →{model_name, version, base_gguf, adapter_gguf|null};ResolutionError(refuse, FR-265) vsResolutionUnavailable(env-default fallback, surfaced in health — never silent).scripts/register_base_gguf.py: registers the local zoo bases askind=full-modelversions withbase_idtags; idempotent; never downloads (R7).US1 — promote → it actually serves (T465–T467)
MODEL/LORA/MODEL_ALIASenv knobs are now the fallback default only;spawn()appends--lorafor adapters.swap.reload_serving_llm: cross-tenant displacement reusespreempt_for; the same-tenant model switch force-reloads (unload →ensure_loaded) — the casepreempt_forsilently no-ops on (spec review PR 022 spec: registry-driven LLM serving + operator model selection #64 §1). Idempotent no-op, target-probe before any evict, jobs never preempted, operator confirm (preempt=true) required for any displacement. Exposed as secret-gatedPOST /control/reloadon both agent transports.US2 — honest served identity (T469–T471)
model_name/registry_version/base/adapter;/serving/state, the/inferresponse, and prediction logging (REST + stream) all consume the agent-reported identity, degrading tounknown— never aSERVING_MODELconfig guess. Kills the live divergence that corrupted a served fine-tune's quality window.US3/US4 — fine-tunes first-class (T473–T479)
base_modellineage → registered full-model base in ONE hop (adapter chains refused); the fine-tune flow stampstask/serving_engineat registration;scripts/backfill_llm_task_tags.pyfixes legacy versions (idempotent, non-clobber); kind→task fallback in the resolve/list surfaces;serving/taskscarries kind+lineage and filters text-generation to the active pointer (FR-276 — no stale duplicate LLM entries).US5 offline slice + polish (T481–T483, T485–T488)
POST models/:name/promoteandGET serving/state(the contract's preferred outcome).registry.promote— the choke-point the scheduler/suggestion paths call — structurally cannot switch the served LLM).AGENT_CONTROL_SECRETpassed to the gateway in compose; constitution confirmed unchanged (the switch reuses the existing one-tenant swap).Verification
pytest: 534 passed, 39 skipped (baseline before this change: 471/39). The only 3 failures are the pre-existingPIL-missing environment failures, identical before/after — zero regressions, Principle II admission/swap tests included.next build(type-check): green. Allow-list conformance grep: everyui/gateway call resolves to an existinggw-allowlist.tsentry. (next lintremains unconfigured in-repo — no ESLint config/dependency has ever existed inui/; unchanged here.)Deliberate scope notes
c28ca97LORAenv prototype — that commit is local-only (not in this remote); the adapter work here implements--lorasupport outright, subsuming it.pytesthalves are green and each is annotated intasks.md.🤖 Generated with Claude Code
https://claude.ai/code/session_013hoYEa8FN5s8eGqa78Fxjx
Generated by Claude Code