From b563aac8669000f24351000b46d68e9d1c9ca3ec 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 | 160 ++++++++++++++++++---- tests/unit/test_llama_stack_synthesize.py | 127 +++++++++++++++++ 2 files changed, 260 insertions(+), 27 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 1a563bc33..0ab2bcbdc 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -998,6 +998,125 @@ def deep_merge_list_replace( return result +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. + + 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 isinstance(existing, dict) and existing.get("provider_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: @@ -1014,6 +1133,14 @@ def apply_high_level_inference( appended. 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 Llama Stack configuration being synthesized (modified in place). @@ -1029,39 +1156,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. - for index, existing in enumerate(inference_list): - if isinstance(existing, dict) and existing.get("provider_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 15927ae99..6a56967e4 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -412,6 +412,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 5bf13c4af97a49423e86bf00fdc224618ab9df06 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 | 101 ++++++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 0ab2bcbdc..c704800a3 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -1481,7 +1481,7 @@ def generate_configuration( def main() -> None: """CLI entry point.""" parser = ArgumentParser( - description="Enrich Llama Stack config with Lightspeed values", + description="Enrich or synthesize Llama Stack config from Lightspeed values", ) parser.add_argument( "-c", @@ -1493,20 +1493,32 @@ def main() -> None: "-i", "--input", default="run.yaml", - help="Input Llama Stack config (default: run.yaml)", + help="Input Llama Stack 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 6a56967e4..632b19686 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -6,8 +6,11 @@ write-to-file step (persistent path, mode 0600). """ +# pylint: disable=too-many-lines + import os import stat +import sys from pathlib import Path from typing import Any, Optional, get_args @@ -20,6 +23,7 @@ deep_merge_list_replace, ensure_mcp_tool_runtime, load_default_baseline, + main, migrate_config_dumb, synthesize_configuration, synthesize_to_file, @@ -915,6 +919,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 3db88130b7276c8a755a80c981da473e3207831a 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 | 189 ++++++++++++++-------- src/models/config.py | 29 ++++ tests/unit/test_llama_stack_synthesize.py | 189 +++++++++++++++------- 3 files changed, 277 insertions(+), 130 deletions(-) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index c704800a3..431208214 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" without a profile. DEFAULT_BASELINE_RESOURCE: Path = Path(__file__).parent / "data" / "default_run.yaml" @@ -1054,67 +1059,57 @@ 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", [] - ) - 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. + 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. - 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: + 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. - 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, - } + 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, ) - self._known_ids.add(model_name) - added.append(model_name) - if added: - self._owned_by_provider[provider_id] = added + continue + known.add(key) + new_entries.append( + { + "model_id": model_name, + "model_type": "llm", + "provider_id": provider_id, + "provider_model_id": model_name, + } + ) + return new_entries def apply_high_level_inference( @@ -1133,13 +1128,19 @@ def apply_high_level_inference( appended. 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 Llama Stack configuration being synthesized (modified in @@ -1154,9 +1155,27 @@ 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 = (provider.get("id") or "").strip() or provider["type"].replace( + "_", "-" + ) + 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"] @@ -1165,9 +1184,31 @@ def apply_high_level_inference( 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", @@ -1492,9 +1533,9 @@ def main() -> None: parser.add_argument( "-i", "--input", - default="run.yaml", - help="Input Llama Stack config for legacy enrichment mode; ignored " - "with --synthesize (default: run.yaml)", + default=None, + help="Input Llama Stack config for legacy enrichment mode " + "(default: run.yaml); not valid with --synthesize", ) parser.add_argument( "-o", @@ -1510,6 +1551,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 {} @@ -1518,7 +1567,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 757eeaa13..fe4b56ec3 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -1820,6 +1820,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 632b19686..3ae74a472 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -16,6 +16,7 @@ import pytest import yaml +from pydantic import ValidationError from llama_stack_configuration import ( PROVIDER_TYPE_MAP, @@ -28,7 +29,7 @@ synthesize_configuration, synthesize_to_file, ) -from models.config import UnifiedInferenceProvider +from models.config import InferenceConfiguration, UnifiedInferenceProvider # --------------------------------------------------------------------------- # ensure_mcp_tool_runtime @@ -285,32 +286,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: @@ -462,15 +459,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": { @@ -478,7 +506,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", } ] @@ -488,59 +516,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: @@ -991,6 +1028,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: