From 1e5048d792a57871828c9a8b6044abd2f66b54c1 Mon Sep 17 00:00:00 2001 From: Omkar Joshi <103182931+omkarjoshi0304@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:05:28 +0100 Subject: [PATCH 1/3] Register high-level inference models as LLM resources during synthesis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: apply_high_level_inference() built providers.inference entries from allowed_models but never registered those models in registered_resources.models. Llama Stack can only discover models via list_models() at startup, which requires live provider connectivity — causing inference failures whenever endpoints are unreachable during startup. Solution: Register each allowed_models entry as an LLM resource pointing at its provider, deduped against existing model_ids. When a later high-level entry reuses the same provider_id, evict the predecessor's models to prevent stale entries. Models from baseline, native_override, or BYOK configs are preserved. Implementation: Extract provider-entry construction and replace-or-append logic into helpers. Introduce _LLMModelRegistrar class to own model registration/eviction bookkeeping instead of threading mutable state through functions. --- src/llama_stack_configuration.py | 169 ++++++++++++++++++---- tests/unit/test_llama_stack_synthesize.py | 127 ++++++++++++++++ 2 files changed, 265 insertions(+), 31 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index f0b1b6175..e604057b1 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -1064,6 +1064,130 @@ def _strip_default_openai_inference(ls_config: dict[str, Any]) -> None: ] +def _replace_or_append_inference_provider( + inference_list: list[Any], entry: dict[str, Any] +) -> None: + """Replace an inference entry with the same provider_id, else append. + + Baseline ${env.OPENAI_API_KEY:+openai} matches as "openai" (LCORE-3607). + + Parameters: + inference_list: Mutable providers.inference list. + entry: New provider entry to install. + """ + provider_id = entry["provider_id"] + for index, existing in enumerate(inference_list): + if not isinstance(existing, dict): + continue + existing_id = _matchable_provider_id(existing.get("provider_id")) + if existing_id == provider_id: + logger.info( + "Replacing existing inference provider with " + "provider_id=%r; a later high-level entry overwrote it", + provider_id, + ) + inference_list[index] = entry + return + inference_list.append(entry) + + +def _build_inference_entry( + provider: dict[str, Any], emitted_id: str, ls_provider_type: str +) -> tuple[dict[str, Any], list[str]]: + """Build a providers.inference entry from one high-level provider. + + Parameters: + provider: One high-level ``inference.providers`` entry. + emitted_id: The provider_id to emit (explicit id or hyphenated type). + ls_provider_type: Llama Stack provider_type from :data:`PROVIDER_TYPE_MAP`. + + Returns: + tuple[dict[str, Any], list[str]]: The provider entry, and its + ``allowed_models`` (empty list when unset). + """ + entry: dict[str, Any] = { + "provider_id": emitted_id, + "provider_type": ls_provider_type, + } + + provider_config: dict[str, Any] = {} + if provider.get("extra"): + provider_config.update(provider["extra"]) + if provider.get("api_key_env"): + key_field = API_KEY_FIELD_MAP.get(ls_provider_type, "api_key") + provider_config[key_field] = "${env." + provider["api_key_env"] + "}" + allowed_models = provider.get("allowed_models") or [] + if allowed_models: + provider_config["allowed_models"] = allowed_models + if provider_config: + entry["config"] = provider_config + + return entry, allowed_models + + +class _LLMModelRegistrar: # pylint: disable=too-few-public-methods + """Owns ``registered_resources.models`` writes for one synthesis call. + + ``apply_high_level_inference`` needs to register a model per + ``allowed_models`` entry, dedupe against models registered before the + call (baseline, native_override, BYOK, ...), and — when a later + high-level entry reuses a ``provider_id`` — evict the models it + registered for that provider's earlier declaration. Bundling those three + pieces of state here keeps that bookkeeping out of the caller instead of + threading a list, a set, and a dict through free-function parameters. + """ + + def __init__(self, ls_config: dict[str, Any]) -> None: + self._models = ls_config.setdefault("registered_resources", {}).setdefault( + "models", [] + ) + self._known_ids = { + m.get("model_id") for m in self._models if isinstance(m, dict) + } + self._owned_by_provider: dict[str, list[str]] = {} + + def sync(self, provider_id: str, allowed_models: list[str]) -> None: + """Register ``allowed_models`` for ``provider_id``, replacing its prior set. + + A later high-level entry with the same emitted ``provider_id`` fully + replaces the earlier one's provider config (see + ``_replace_or_append_inference_provider``), so any model this + registrar added for it earlier in the same call is stale and must be + evicted first — otherwise a model no longer served by the replaced + provider would linger in ``registered_resources.models``. + + Parameters: + provider_id: provider_id of the inference provider offering the + models. + allowed_models: Model names to register. + """ + stale = set(self._owned_by_provider.pop(provider_id, [])) + if stale: + self._models[:] = [ + m + for m in self._models + if not (isinstance(m, dict) and m.get("model_id") in stale) + ] + self._known_ids.difference_update(stale) + + added = [] + for model_name in allowed_models: + if model_name in self._known_ids: + continue + self._models.append( + { + "model_id": model_name, + "model_type": "llm", + "provider_id": provider_id, + "provider_model_id": model_name, + } + ) + self._known_ids.add(model_name) + added.append(model_name) + if added: + self._owned_by_provider[provider_id] = added + + def apply_high_level_inference( ls_config: dict[str, Any], inference: dict[str, Any] ) -> None: @@ -1082,6 +1206,14 @@ def apply_high_level_inference( Secrets are emitted as ``${env.}`` references, never resolved values (R6). + Each of the provider's ``allowed_models`` is also registered as an LLM + entry in ``registered_resources.models`` (skipping any ``model_id`` already + present), so the model is usable even when the provider endpoint is + unreachable at startup — Llama Stack's auto-discovery otherwise requires a + live connection to list models. Replacing a provider (same emitted id) + also evicts the LLM entries this function registered for the earlier + declaration, so a superseded provider's models don't linger. + Parameters: ls_config: The OGX configuration being synthesized (modified in place). @@ -1097,43 +1229,18 @@ def apply_high_level_inference( providers_section = ls_config.setdefault("providers", {}) inference_list = providers_section.setdefault("inference", []) + model_registrar = _LLMModelRegistrar(ls_config) for provider in providers: provider_type = provider["type"] emitted_id = provider.get("id") or provider_type.replace("_", "-") ls_provider_type = PROVIDER_TYPE_MAP[provider_type] - entry: dict[str, Any] = { - "provider_id": emitted_id, - "provider_type": ls_provider_type, - } + entry, allowed_models = _build_inference_entry( + provider, emitted_id, ls_provider_type + ) - provider_config: dict[str, Any] = {} - if provider.get("extra"): - provider_config.update(provider["extra"]) - if provider.get("api_key_env"): - key_field = API_KEY_FIELD_MAP.get(ls_provider_type, "api_key") - provider_config[key_field] = "${env." + provider["api_key_env"] + "}" - if provider.get("allowed_models"): - provider_config["allowed_models"] = provider["allowed_models"] - if provider_config: - entry["config"] = provider_config - - # Replace a baseline provider with the same id, else append. - # Baseline ${env.OPENAI_API_KEY:+openai} matches as "openai" (LCORE-3607). - for index, existing in enumerate(inference_list): - if not isinstance(existing, dict): - continue - existing_id = _matchable_provider_id(existing.get("provider_id")) - if existing_id == emitted_id: - logger.info( - "Replacing existing inference provider with " - "provider_id=%r; a later high-level entry overwrote it", - emitted_id, - ) - inference_list[index] = entry - break - else: - inference_list.append(entry) + _replace_or_append_inference_provider(inference_list, entry) + model_registrar.sync(emitted_id, allowed_models) logger.info( "Applied %d high-level inference provider(s) to synthesized config", diff --git a/tests/unit/test_llama_stack_synthesize.py b/tests/unit/test_llama_stack_synthesize.py index 365c11ad2..80db9cbd9 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -524,6 +524,133 @@ def test_apply_high_level_inference_empty_is_noop() -> None: assert ls_config["providers"]["inference"] == [{"provider_id": "x"}] +def test_apply_high_level_inference_registers_llm_model() -> None: + """An allowed model is registered as an LLM resource pointing at its provider.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + { + "type": "openai", + "api_key_env": "OPENAI_API_KEY", + "allowed_models": ["gpt-4o-mini"], + } + ] + } + apply_high_level_inference(ls_config, inference) + models = ls_config["registered_resources"]["models"] + assert models == [ + { + "model_id": "gpt-4o-mini", + "model_type": "llm", + "provider_id": "openai", + "provider_model_id": "gpt-4o-mini", + } + ] + + +def test_apply_high_level_inference_registers_multiple_allowed_models() -> None: + """Every allowed model for a provider is registered, not just the first.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + { + "type": "vllm", + "id": "vllm-prod", + "allowed_models": ["model-a", "model-b"], + } + ] + } + apply_high_level_inference(ls_config, inference) + model_ids = {m["model_id"] for m in ls_config["registered_resources"]["models"]} + assert model_ids == {"model-a", "model-b"} + assert all( + m["provider_id"] == "vllm-prod" + for m in ls_config["registered_resources"]["models"] + ) + + +def test_apply_high_level_inference_no_allowed_models_no_registration() -> None: + """A provider without allowed_models registers no LLM model.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = {"providers": [{"type": "sentence_transformers"}]} + apply_high_level_inference(ls_config, inference) + assert ls_config["registered_resources"]["models"] == [] + + +def test_apply_high_level_inference_skips_already_registered_model() -> None: + """A model already present in registered_resources.models is not duplicated.""" + ls_config: dict[str, Any] = { + "providers": {"inference": []}, + "registered_resources": { + "models": [ + { + "model_id": "gpt-4o-mini", + "model_type": "llm", + "provider_id": "stale-provider", + "provider_model_id": "gpt-4o-mini", + } + ] + }, + } + inference = {"providers": [{"type": "openai", "allowed_models": ["gpt-4o-mini"]}]} + apply_high_level_inference(ls_config, inference) + models = ls_config["registered_resources"]["models"] + assert len(models) == 1 + assert models[0]["provider_id"] == "stale-provider" + + +def test_apply_high_level_inference_replacing_provider_evicts_its_stale_models() -> ( + None +): + """A later entry with the same provider_id drops the earlier entry's models.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + { + "type": "vllm", + "id": "vllm-shared", + "allowed_models": ["model-old"], + }, + { + "type": "vllm", + "id": "vllm-shared", + "allowed_models": ["model-new"], + }, + ] + } + apply_high_level_inference(ls_config, inference) + models = ls_config["registered_resources"]["models"] + model_ids = {m["model_id"] for m in models} + assert model_ids == {"model-new"} + assert all(m["provider_id"] == "vllm-shared" for m in models) + + +def test_apply_high_level_inference_replacing_provider_keeps_baseline_models() -> None: + """Eviction only removes models this call registered, not pre-existing ones.""" + ls_config: dict[str, Any] = { + "providers": {"inference": []}, + "registered_resources": { + "models": [ + { + "model_id": "baseline-model", + "model_type": "llm", + "provider_id": "vllm-shared", + "provider_model_id": "baseline-model", + } + ] + }, + } + inference = { + "providers": [ + {"type": "vllm", "id": "vllm-shared", "allowed_models": ["model-a"]}, + {"type": "vllm", "id": "vllm-shared", "allowed_models": ["model-b"]}, + ] + } + apply_high_level_inference(ls_config, inference) + model_ids = {m["model_id"] for m in ls_config["registered_resources"]["models"]} + assert model_ids == {"baseline-model", "model-b"} + + def test_provider_type_map_covers_every_literal_value() -> None: """Every UnifiedInferenceProvider.type value has a PROVIDER_TYPE_MAP entry.""" literal_values = set( From e6763915df384977402dbfbe64f7aa80d525684d Mon Sep 17 00:00:00 2001 From: Omkar Joshi <103182931+omkarjoshi0304@users.noreply.github.com> Date: Tue, 11 Aug 2026 10:07:14 +0100 Subject: [PATCH 2/3] Add --synthesize flag to CLI for unified synthesis mode Problem: The CLI (python llama_stack_configuration.py -c config.yaml) only called generate_configuration(), the legacy enrichment mode that requires an already-built run.yaml as input. There was no CLI path to synthesize_configuration()/synthesize_to_file(), the unified mode that builds run.yaml from lightspeed-stack.yaml alone, forcing consumers to import the module instead of using the documented script interface. Solution: Add a --synthesize flag to the CLI. When set, the CLI builds the config via synthesize_to_file() from -c alone (ignoring -i), instead of enriching an existing run.yaml. Handle empty or comment-only -c files by loading them as {} rather than None to avoid opaque AttributeError crashes in synthesize_to_file(). Implementation: Add --synthesize argument to argparse, add guard for empty config file (yaml.safe_load returns None), and route to the appropriate function (synthesize_to_file vs generate_configuration). --- src/llama_stack_configuration.py | 22 +++-- tests/unit/test_llama_stack_synthesize.py | 99 +++++++++++++++++++++++ 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index e604057b1..01b0292d6 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -1593,7 +1593,7 @@ def generate_configuration( def main() -> None: """CLI entry point.""" parser = ArgumentParser( - description="Enrich OGX config with Lightspeed values", + description="Enrich or synthesize OGX config from Lightspeed values", ) parser.add_argument( "-c", @@ -1605,20 +1605,32 @@ def main() -> None: "-i", "--input", default="run.yaml", - help="Input OGX config (default: run.yaml)", + help="Input OGX config for legacy enrichment mode; ignored " + "with --synthesize (default: run.yaml)", ) parser.add_argument( "-o", "--output", default="run_.yaml", - help="Output enriched config (default: run_.yaml)", + help="Output config file (default: run_.yaml)", + ) + parser.add_argument( + "--synthesize", + action="store_true", + help="Build a complete run.yaml from -c alone instead of enriching " + "an existing run.yaml given by -i", ) args = parser.parse_args() with open(args.config, "r", encoding="utf-8") as f: - config = yaml.safe_load(f) + config = yaml.safe_load(f) or {} - generate_configuration(args.input, args.output, config) + if args.synthesize: + synthesize_to_file( + config, args.output, config_file_dir=str(Path(args.config).parent) + ) + else: + generate_configuration(args.input, args.output, config) if __name__ == "__main__": diff --git a/tests/unit/test_llama_stack_synthesize.py b/tests/unit/test_llama_stack_synthesize.py index 80db9cbd9..e4804e1ba 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -11,6 +11,7 @@ import logging import os import stat +import sys from pathlib import Path from typing import Any, Optional, get_args @@ -25,6 +26,7 @@ deep_merge_list_replace, ensure_mcp_tool_runtime, load_default_baseline, + main, migrate_config_dumb, synthesize_configuration, synthesize_to_file, @@ -1212,6 +1214,103 @@ def test_migrate_config_dumb_rejects_non_mapping_inputs(tmp_path: Path) -> None: migrate_config_dumb(str(empty_run), lcs_path, out_path) +# --------------------------------------------------------------------------- +# CLI entry point +# --------------------------------------------------------------------------- + + +def test_main_synthesize_flag_builds_run_yaml( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """--synthesize builds a complete run.yaml from -c alone.""" + config_path = tmp_path / "lightspeed-stack.yaml" + config_path.write_text( + yaml.dump( + { + "llama_stack": { + "config": { + "baseline": "empty", + "native_override": {"version": 2, "apis": ["inference"]}, + } + } + } + ), + encoding="utf-8", + ) + output_path = tmp_path / "run.yaml" + + monkeypatch.setattr( + sys, + "argv", + [ + "llama_stack_configuration.py", + "-c", + str(config_path), + "-o", + str(output_path), + "--synthesize", + ], + ) + main() + + result = yaml.safe_load(output_path.read_text(encoding="utf-8")) + assert result == {"version": 2, "apis": ["inference"]} + + +def test_main_default_uses_legacy_enrichment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Without --synthesize, the CLI keeps calling legacy generate_configuration.""" + config_path = tmp_path / "lightspeed-stack.yaml" + config_path.write_text(yaml.dump({}), encoding="utf-8") + input_path = tmp_path / "run.yaml" + input_path.write_text(yaml.dump({"version": 2}), encoding="utf-8") + output_path = tmp_path / "run_.yaml" + + monkeypatch.setattr( + sys, + "argv", + [ + "llama_stack_configuration.py", + "-c", + str(config_path), + "-i", + str(input_path), + "-o", + str(output_path), + ], + ) + main() + + result = yaml.safe_load(output_path.read_text(encoding="utf-8")) + assert result["version"] == 2 + + +def test_main_synthesize_flag_handles_empty_config_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An empty/comment-only -c file loads as {} instead of crashing on None.""" + config_path = tmp_path / "lightspeed-stack.yaml" + config_path.write_text("# only a comment\n", encoding="utf-8") + output_path = tmp_path / "run.yaml" + + monkeypatch.setattr( + sys, + "argv", + [ + "llama_stack_configuration.py", + "-c", + str(config_path), + "-o", + str(output_path), + "--synthesize", + ], + ) + main() + + assert output_path.exists() + + # --------------------------------------------------------------------------- # reference profiles (LCORE-2346) # --------------------------------------------------------------------------- From 71a5d83d829f18eb973bd435a1bd7492690fc95c Mon Sep 17 00:00:00 2001 From: Omkar Joshi <103182931+omkarjoshi0304@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:50:01 +0100 Subject: [PATCH 3/3] Address synthesis registration review feedback Problem: Dedup on bare model_id silently dropped the same model served by two providers; duplicate provider_ids were resolved as last-wins instead of rejected; embedding models were registered as llm type; passing -i with --synthesize silently ignored -i. Solution: Key dedup on (provider_id, model_id). Add check_unique_provider_ids validator to InferenceConfiguration to reject duplicate emitted ids at load time, replacing the eviction class with a plain function. Guard embedding providers from llm registration. Error when -i and --synthesize are both supplied. --- src/llama_stack_configuration.py | 200 +++++++++++++------- src/models/config.py | 29 +++ tests/unit/test_llama_stack_synthesize.py | 211 ++++++++++++++++------ 3 files changed, 310 insertions(+), 130 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 01b0292d6..f59fe5b58 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -56,6 +56,11 @@ "remote::vllm": "api_token", } +# High-level inference `type` values that serve embeddings rather than LLMs. +# Their `allowed_models` must not be registered as `llm` model resources, +# which would give Llama Stack a mis-typed model that routes incorrectly. +EMBEDDING_PROVIDER_TYPES: frozenset[str] = frozenset({"sentence_transformers"}) + # Package-relative path to the built-in default baseline run.yaml shipped with # LCORE, used when unified mode selects baseline "default" or "byo-llm" without # a profile. "byo-llm" loads this file then strips the conditional OpenAI row. @@ -1125,67 +1130,70 @@ def _build_inference_entry( return entry, allowed_models -class _LLMModelRegistrar: # pylint: disable=too-few-public-methods - """Owns ``registered_resources.models`` writes for one synthesis call. +def _register_high_level_models( + existing_models: list[Any], provider_id: str, allowed_models: list[str] +) -> list[dict[str, Any]]: + """Build LLM resource entries for allowed_models not already registered. - ``apply_high_level_inference`` needs to register a model per - ``allowed_models`` entry, dedupe against models registered before the - call (baseline, native_override, BYOK, ...), and — when a later - high-level entry reuses a ``provider_id`` — evict the models it - registered for that provider's earlier declaration. Bundling those three - pieces of state here keeps that bookkeeping out of the caller instead of - threading a list, a set, and a dict through free-function parameters. - """ + Each allowed model is registered as an ``llm`` resource pointing at + ``provider_id`` so it is usable even when the provider endpoint is + unreachable at startup — Llama Stack's auto-discovery otherwise needs a + live connection to list models. - def __init__(self, ls_config: dict[str, Any]) -> None: - self._models = ls_config.setdefault("registered_resources", {}).setdefault( - "models", [] + Deduplication is keyed on ``(provider_id, model_id)`` rather than the bare + ``model_id``: Llama Stack scopes model identifiers by provider (it builds + ``f"{provider_id}/{model_id}"``), so the same model name served by two + providers is registered once per provider instead of being dropped for the + second one. + + Parameters: + existing_models: Models already in ``registered_resources.models`` + (baseline, native_override, BYOK, and any registered earlier in + this synthesis pass); read only for deduplication, never mutated. + provider_id: Emitted provider_id the models are served by. + allowed_models: Model names to register. + + Returns: + New model resource dicts to append; empty when all are already known. + """ + known = { + (m.get("provider_id"), m.get("model_id")) + for m in existing_models + if isinstance(m, dict) + } + new_entries: list[dict[str, Any]] = [] + for model_name in allowed_models: + key = (provider_id, model_name) + if key in known: + logger.debug( + "Model %r already registered for provider_id=%r; skipping", + model_name, + provider_id, + ) + continue + known.add(key) + new_entries.append( + { + "model_id": model_name, + "model_type": "llm", + "provider_id": provider_id, + "provider_model_id": model_name, + } ) - self._known_ids = { - m.get("model_id") for m in self._models if isinstance(m, dict) - } - self._owned_by_provider: dict[str, list[str]] = {} + return new_entries - def sync(self, provider_id: str, allowed_models: list[str]) -> None: - """Register ``allowed_models`` for ``provider_id``, replacing its prior set. - A later high-level entry with the same emitted ``provider_id`` fully - replaces the earlier one's provider config (see - ``_replace_or_append_inference_provider``), so any model this - registrar added for it earlier in the same call is stale and must be - evicted first — otherwise a model no longer served by the replaced - provider would linger in ``registered_resources.models``. +def _emitted_provider_id(provider: dict[str, Any]) -> str: + """Return the provider_id emitted for one high-level provider entry. - Parameters: - provider_id: provider_id of the inference provider offering the - models. - allowed_models: Model names to register. - """ - stale = set(self._owned_by_provider.pop(provider_id, [])) - if stale: - self._models[:] = [ - m - for m in self._models - if not (isinstance(m, dict) and m.get("model_id") in stale) - ] - self._known_ids.difference_update(stale) - - added = [] - for model_name in allowed_models: - if model_name in self._known_ids: - continue - self._models.append( - { - "model_id": model_name, - "model_type": "llm", - "provider_id": provider_id, - "provider_model_id": model_name, - } - ) - self._known_ids.add(model_name) - added.append(model_name) - if added: - self._owned_by_provider[provider_id] = added + Parameters: + provider: One high-level ``inference.providers`` entry. + + Returns: + The provider_id to emit. + """ + explicit_id = (provider.get("id") or "").strip() + return explicit_id or provider["type"].replace("_", "-") def apply_high_level_inference( @@ -1206,13 +1214,19 @@ def apply_high_level_inference( Secrets are emitted as ``${env.}`` references, never resolved values (R6). - Each of the provider's ``allowed_models`` is also registered as an LLM - entry in ``registered_resources.models`` (skipping any ``model_id`` already - present), so the model is usable even when the provider endpoint is - unreachable at startup — Llama Stack's auto-discovery otherwise requires a - live connection to list models. Replacing a provider (same emitted id) - also evicts the LLM entries this function registered for the earlier - declaration, so a superseded provider's models don't linger. + Each LLM provider's ``allowed_models`` is also registered as an LLM entry + in ``registered_resources.models`` (deduped by ``(provider_id, model_id)``), + so the model is usable even when the provider endpoint is unreachable at + startup — Llama Stack's auto-discovery otherwise requires a live connection + to list models. Embedding provider types (see :data:`EMBEDDING_PROVIDER_TYPES`) + are skipped, since their models are not LLMs. Duplicate emitted provider ids + are rejected upstream by ``InferenceConfiguration`` validation, so no + same-id eviction is needed here. + + These registrations run before the ``native_override`` merge and + ``registered_resources.models`` is a list, so an operator override that + supplies its own ``models`` replaces them wholesale (R5 list-replacement + precedence) — intended, so an override always wins. Parameters: ls_config: The OGX configuration being synthesized (modified in @@ -1227,20 +1241,58 @@ def apply_high_level_inference( if not providers: return + # Validate for duplicate emitted ids before mutating ls_config. + # InferenceConfiguration catches this at Pydantic load time, but callers + # such as the CLI may pass raw dicts that bypass that validation. + seen_ids: set[str] = set() + for provider in providers: + emitted = _emitted_provider_id(provider) + if emitted in seen_ids: + raise ValueError( + f"duplicate inference provider id {emitted!r}: two " + "inference.providers entries resolve to the same provider_id; " + "set a distinct 'id' on one of them" + ) + seen_ids.add(emitted) + providers_section = ls_config.setdefault("providers", {}) inference_list = providers_section.setdefault("inference", []) - model_registrar = _LLMModelRegistrar(ls_config) + # (emitted_id, allowed_models) pairs to register as LLM resources, collected + # while emitting provider entries and applied after the loop. + to_register: list[tuple[str, list[str]]] = [] for provider in providers: provider_type = provider["type"] - emitted_id = provider.get("id") or provider_type.replace("_", "-") + emitted_id = _emitted_provider_id(provider) ls_provider_type = PROVIDER_TYPE_MAP[provider_type] entry, allowed_models = _build_inference_entry( provider, emitted_id, ls_provider_type ) - _replace_or_append_inference_provider(inference_list, entry) - model_registrar.sync(emitted_id, allowed_models) + + if not allowed_models: + continue + if provider_type in EMBEDDING_PROVIDER_TYPES: + logger.debug( + "Skipping LLM model registration for embedding provider " + "type=%r (provider_id=%r)", + provider_type, + emitted_id, + ) + continue + to_register.append((emitted_id, allowed_models)) + + # Bind (creating if needed) registered_resources.models only when there is + # something to register, so a config that registers nothing keeps its + # original shape instead of gaining an empty models block. + if to_register: + models_list = ls_config.setdefault("registered_resources", {}).setdefault( + "models", [] + ) + for emitted_id, allowed_models in to_register: + models_list.extend( + _register_high_level_models(models_list, emitted_id, allowed_models) + ) logger.info( "Applied %d high-level inference provider(s) to synthesized config", @@ -1604,9 +1656,9 @@ def main() -> None: parser.add_argument( "-i", "--input", - default="run.yaml", - help="Input OGX config for legacy enrichment mode; ignored " - "with --synthesize (default: run.yaml)", + default=None, + help="Input OGX config for legacy enrichment mode " + "(default: run.yaml); not valid with --synthesize", ) parser.add_argument( "-o", @@ -1622,6 +1674,14 @@ def main() -> None: ) args = parser.parse_args() + if args.synthesize and args.input is not None: + parser.error( + "-i/--input is not valid with --synthesize; synthesize builds " + "the config from -c alone" + ) + + # An empty or comment-only -c file loads as {} rather than None; this + # applies to both modes (legacy previously raised on config.get()). with open(args.config, "r", encoding="utf-8") as f: config = yaml.safe_load(f) or {} @@ -1630,7 +1690,7 @@ def main() -> None: config, args.output, config_file_dir=str(Path(args.config).parent) ) else: - generate_configuration(args.input, args.output, config) + generate_configuration(args.input or "run.yaml", args.output, config) if __name__ == "__main__": diff --git a/src/models/config.py b/src/models/config.py index 8edf2fd0a..30dcada6b 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -1833,6 +1833,35 @@ def check_default_model_and_provider(self) -> Self: ) return self + @model_validator(mode="after") + def check_unique_provider_ids(self) -> Self: + """Reject two high-level providers that resolve to the same provider_id. + + The synthesizer emits each provider under its explicit ``id`` when set, + otherwise the ``type`` with underscores hyphenated. Two entries + resolving to the same emitted id would collide in the synthesized + ``providers.inference`` list (the later one silently overwriting the + earlier), so reject the ambiguity here rather than resolving it as + last-wins at synthesis time. + + Raises: + ValueError: If two providers resolve to the same emitted id. + + Returns: + self (Self): The validated configuration instance. + """ + seen: set[str] = set() + for provider in self.providers: + emitted_id = provider.id or provider.type.replace("_", "-") + if emitted_id in seen: + raise ValueError( + f"duplicate inference provider id {emitted_id!r}: two " + "inference.providers entries resolve to the same " + "provider_id; set a distinct 'id' on one of them" + ) + seen.add(emitted_id) + return self + class CompactionConfiguration(ConfigurationBase): """Configuration for conversation history compaction. diff --git a/tests/unit/test_llama_stack_synthesize.py b/tests/unit/test_llama_stack_synthesize.py index e4804e1ba..51cd1ef3f 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -18,6 +18,7 @@ import pytest import yaml from ogx.core.stack import replace_env_vars +from pydantic import ValidationError from llama_stack_configuration import ( CONDITIONAL_OPENAI_PROVIDER_ID, @@ -31,7 +32,7 @@ synthesize_configuration, synthesize_to_file, ) -from models.config import UnifiedInferenceProvider +from models.config import InferenceConfiguration, UnifiedInferenceProvider OPENAI_CONDITIONAL_PROVIDER_ID = CONDITIONAL_OPENAI_PROVIDER_ID OPENAI_CONDITIONAL_API_KEY = "${env.OPENAI_API_KEY:=}" @@ -368,6 +369,28 @@ def test_apply_high_level_inference_uses_explicit_id() -> None: assert entry["provider_type"] == "remote::vllm" +def test_apply_high_level_inference_strips_whitespace_from_explicit_id() -> None: + """A raw-dict caller's padded id is stripped, not emitted verbatim.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = {"providers": [{"type": "vllm", "id": " vllm-prod "}]} + apply_high_level_inference(ls_config, inference) + entry = ls_config["providers"]["inference"][0] + assert entry["provider_id"] == "vllm-prod" + + +def test_apply_high_level_inference_detects_duplicate_after_stripping() -> None: + """Ids that only differ by incidental whitespace are treated as duplicates.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + {"type": "vllm", "id": "shared"}, + {"type": "ollama", "id": " shared "}, + ] + } + with pytest.raises(ValueError, match="duplicate inference provider id"): + apply_high_level_inference(ls_config, inference) + + def test_apply_high_level_inference_same_type_distinct_ids() -> None: """Two providers of the same type with distinct ids both appear.""" ls_config: dict[str, Any] = {"providers": {"inference": []}} @@ -395,32 +418,28 @@ def test_apply_high_level_inference_same_type_distinct_ids() -> None: assert by_id["vllm-staging"]["config"]["url"] == "http://staging:8000" -def test_apply_high_level_inference_duplicate_id_last_wins( +def test_apply_high_level_inference_replaces_baseline_provider_id( caplog: pytest.LogCaptureFixture, ) -> None: - """Duplicate id keeps the last entry and logs an info message.""" - ls_config: dict[str, Any] = {"providers": {"inference": []}} - inference = { - "providers": [ - { - "type": "vllm", - "id": "vllm-shared", - "api_key_env": "FIRST_KEY", - }, - { - "type": "vllm", - "id": "vllm-shared", - "api_key_env": "SECOND_KEY", - }, - ] + """A high-level provider that matches a baseline provider_id replaces it.""" + ls_config: dict[str, Any] = { + "providers": { + "inference": [ + { + "provider_id": "vllm", + "provider_type": "remote::vllm", + "config": {"api_token": "${env.OLD_KEY}"}, + } + ] + } } + inference = {"providers": [{"type": "vllm", "api_key_env": "NEW_KEY"}]} with caplog.at_level("INFO", logger="lightspeed_stack.llama_stack_configuration"): apply_high_level_inference(ls_config, inference) entries = ls_config["providers"]["inference"] assert len(entries) == 1 - assert entries[0]["provider_id"] == "vllm-shared" - assert entries[0]["config"]["api_token"] == "${env.SECOND_KEY}" - assert "provider_id='vllm-shared'" in caplog.text + assert entries[0]["config"]["api_token"] == "${env.NEW_KEY}" + assert "provider_id='vllm'" in caplog.text def test_apply_high_level_inference_merges_extra() -> None: @@ -572,15 +591,46 @@ def test_apply_high_level_inference_registers_multiple_allowed_models() -> None: def test_apply_high_level_inference_no_allowed_models_no_registration() -> None: - """A provider without allowed_models registers no LLM model.""" + """A provider without allowed_models leaves registered_resources untouched.""" ls_config: dict[str, Any] = {"providers": {"inference": []}} inference = {"providers": [{"type": "sentence_transformers"}]} apply_high_level_inference(ls_config, inference) - assert ls_config["registered_resources"]["models"] == [] + # Nothing registered -> no empty registered_resources.models block is added. + assert "registered_resources" not in ls_config + + +def test_apply_high_level_inference_skips_embedding_provider_models() -> None: + """allowed_models on an embedding provider is not registered as an llm.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + {"type": "sentence_transformers", "allowed_models": ["all-MiniLM-L6-v2"]} + ] + } + apply_high_level_inference(ls_config, inference) + assert ls_config["providers"]["inference"][0]["provider_id"] == ( + "sentence-transformers" + ) + assert "registered_resources" not in ls_config + + +def test_apply_high_level_inference_same_model_distinct_providers() -> None: + """The same model name served by two providers is registered once each.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = { + "providers": [ + {"type": "openai", "id": "openai", "allowed_models": ["gpt-4o"]}, + {"type": "azure", "id": "azure", "allowed_models": ["gpt-4o"]}, + ] + } + apply_high_level_inference(ls_config, inference) + models = ls_config["registered_resources"]["models"] + by_provider = {(m["provider_id"], m["model_id"]) for m in models} + assert by_provider == {("openai", "gpt-4o"), ("azure", "gpt-4o")} def test_apply_high_level_inference_skips_already_registered_model() -> None: - """A model already present in registered_resources.models is not duplicated.""" + """A model already registered for the same provider_id is not duplicated.""" ls_config: dict[str, Any] = { "providers": {"inference": []}, "registered_resources": { @@ -588,7 +638,7 @@ def test_apply_high_level_inference_skips_already_registered_model() -> None: { "model_id": "gpt-4o-mini", "model_type": "llm", - "provider_id": "stale-provider", + "provider_id": "openai", "provider_model_id": "gpt-4o-mini", } ] @@ -598,59 +648,68 @@ def test_apply_high_level_inference_skips_already_registered_model() -> None: apply_high_level_inference(ls_config, inference) models = ls_config["registered_resources"]["models"] assert len(models) == 1 - assert models[0]["provider_id"] == "stale-provider" + assert models[0]["provider_id"] == "openai" -def test_apply_high_level_inference_replacing_provider_evicts_its_stale_models() -> ( - None -): - """A later entry with the same provider_id drops the earlier entry's models.""" - ls_config: dict[str, Any] = {"providers": {"inference": []}} - inference = { - "providers": [ - { - "type": "vllm", - "id": "vllm-shared", - "allowed_models": ["model-old"], - }, - { - "type": "vllm", - "id": "vllm-shared", - "allowed_models": ["model-new"], - }, - ] - } - apply_high_level_inference(ls_config, inference) - models = ls_config["registered_resources"]["models"] - model_ids = {m["model_id"] for m in models} - assert model_ids == {"model-new"} - assert all(m["provider_id"] == "vllm-shared" for m in models) - - -def test_apply_high_level_inference_replacing_provider_keeps_baseline_models() -> None: - """Eviction only removes models this call registered, not pre-existing ones.""" +def test_apply_high_level_inference_registers_model_for_new_provider() -> None: + """A model present under another provider is still registered for this one.""" ls_config: dict[str, Any] = { "providers": {"inference": []}, "registered_resources": { "models": [ { - "model_id": "baseline-model", + "model_id": "gpt-4o-mini", "model_type": "llm", - "provider_id": "vllm-shared", - "provider_model_id": "baseline-model", + "provider_id": "other-provider", + "provider_model_id": "gpt-4o-mini", } ] }, } inference = { "providers": [ - {"type": "vllm", "id": "vllm-shared", "allowed_models": ["model-a"]}, - {"type": "vllm", "id": "vllm-shared", "allowed_models": ["model-b"]}, + {"type": "openai", "id": "openai", "allowed_models": ["gpt-4o-mini"]} ] } apply_high_level_inference(ls_config, inference) - model_ids = {m["model_id"] for m in ls_config["registered_resources"]["models"]} - assert model_ids == {"baseline-model", "model-b"} + keys = { + (m["provider_id"], m["model_id"]) + for m in ls_config["registered_resources"]["models"] + } + assert keys == {("other-provider", "gpt-4o-mini"), ("openai", "gpt-4o-mini")} + + +def test_inference_config_rejects_duplicate_explicit_provider_ids() -> None: + """Two providers with the same explicit id are rejected at validation.""" + with pytest.raises(ValidationError, match="duplicate inference provider id"): + InferenceConfiguration( + providers=[ + {"type": "vllm", "id": "vllm-shared", "allowed_models": ["a"]}, + {"type": "openai", "id": "vllm-shared", "allowed_models": ["b"]}, + ] + ) + + +def test_inference_config_rejects_duplicate_type_derived_provider_ids() -> None: + """Two providers of the same type with no id collide on the derived id.""" + with pytest.raises(ValidationError, match="duplicate inference provider id"): + InferenceConfiguration( + providers=[ + {"type": "vllm", "allowed_models": ["a"]}, + {"type": "vllm", "allowed_models": ["b"]}, + ] + ) + + +def test_inference_config_allows_distinct_provider_ids() -> None: + """Distinct ids (and an id that avoids a type-derived clash) validate.""" + config = InferenceConfiguration( + providers=[ + {"type": "vllm", "allowed_models": ["a"]}, + {"type": "vllm", "id": "vllm-staging", "allowed_models": ["b"]}, + ] + ) + assert len(config.providers) == 2 def test_provider_type_map_covers_every_literal_value() -> None: @@ -1286,6 +1345,38 @@ def test_main_default_uses_legacy_enrichment( assert result["version"] == 2 +def test_main_synthesize_flag_rejects_duplicate_provider_ids( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """CLI synthesis raises ValueError for duplicate emitted provider_ids.""" + config_path = tmp_path / "lightspeed-stack.yaml" + config_path.write_text( + "inference:\n" + " providers:\n" + " - type: vllm\n" + " id: shared\n" + " allowed_models: [a]\n" + " - type: openai\n" + " id: shared\n" + " allowed_models: [b]\n", + encoding="utf-8", + ) + monkeypatch.setattr( + sys, + "argv", + [ + "llama_stack_configuration.py", + "-c", + str(config_path), + "-o", + str(tmp_path / "run.yaml"), + "--synthesize", + ], + ) + with pytest.raises(ValueError, match="duplicate inference provider id"): + main() + + def test_main_synthesize_flag_handles_empty_config_file( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: