diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 1a563bc33..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" @@ -998,6 +1003,115 @@ 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 + + +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. + + 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. + + 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, + } + ) + return new_entries + + def apply_high_level_inference( ls_config: dict[str, Any], inference: dict[str, Any] ) -> None: @@ -1014,6 +1128,20 @@ def apply_high_level_inference( appended. Secrets are emitted as ``${env.}`` references, never resolved values (R6). + 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 place). @@ -1027,41 +1155,60 @@ 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", []) + # (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("_", "-") 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 + ) + _replace_or_append_inference_provider(inference_list, entry) - 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) + 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", @@ -1375,7 +1522,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", @@ -1386,21 +1533,41 @@ def main() -> None: parser.add_argument( "-i", "--input", - default="run.yaml", - help="Input Llama Stack config (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", "--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() + 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) + 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 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 15927ae99..3ae74a472 100644 --- a/tests/unit/test_llama_stack_synthesize.py +++ b/tests/unit/test_llama_stack_synthesize.py @@ -6,13 +6,17 @@ 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 import pytest import yaml +from pydantic import ValidationError from llama_stack_configuration import ( PROVIDER_TYPE_MAP, @@ -20,11 +24,12 @@ deep_merge_list_replace, ensure_mcp_tool_runtime, load_default_baseline, + main, migrate_config_dumb, synthesize_configuration, synthesize_to_file, ) -from models.config import UnifiedInferenceProvider +from models.config import InferenceConfiguration, UnifiedInferenceProvider # --------------------------------------------------------------------------- # ensure_mcp_tool_runtime @@ -281,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: @@ -412,6 +413,173 @@ 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 leaves registered_resources untouched.""" + ls_config: dict[str, Any] = {"providers": {"inference": []}} + inference = {"providers": [{"type": "sentence_transformers"}]} + apply_high_level_inference(ls_config, inference) + # 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 registered for the same provider_id is not duplicated.""" + ls_config: dict[str, Any] = { + "providers": {"inference": []}, + "registered_resources": { + "models": [ + { + "model_id": "gpt-4o-mini", + "model_type": "llm", + "provider_id": "openai", + "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"] == "openai" + + +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": "gpt-4o-mini", + "model_type": "llm", + "provider_id": "other-provider", + "provider_model_id": "gpt-4o-mini", + } + ] + }, + } + inference = { + "providers": [ + {"type": "openai", "id": "openai", "allowed_models": ["gpt-4o-mini"]} + ] + } + apply_high_level_inference(ls_config, inference) + 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: """Every UnifiedInferenceProvider.type value has a PROVIDER_TYPE_MAP entry.""" literal_values = set( @@ -788,6 +956,135 @@ 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_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: + """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) # ---------------------------------------------------------------------------