diff --git a/docs/devel_doc/conversations_api.md b/docs/devel_doc/conversations_api.md index e7496be16..7c69de672 100644 --- a/docs/devel_doc/conversations_api.md +++ b/docs/devel_doc/conversations_api.md @@ -158,15 +158,15 @@ response = await client.responses.create( ### Conversation Storage -Conversations are stored in **two databases**: +Conversations are stored across **LCORE and Llama Stack / OGX** layers: -#### 1. Llama Stack Database (PostgreSQL `public` schema) +#### 1. Llama Stack / OGX conversations store **Tables:** - `openai_conversations`: Stores conversation metadata -- `conversation_items`: Stores individual messages/turns in conversations +- `conversation_items`: Stores individual messages/turns (durable source of truth for continue-chat) -**Configuration (in `config/llama_stack_client_config.yaml`):** +**Baseline configuration** (shipped `default_run.yaml` / typical profile): ```yaml storage: stores: @@ -175,7 +175,15 @@ storage: backend: sql_default ``` -#### 2. Lightspeed Stack Database (PostgreSQL `lightspeed-stack` schema) +In **unified mode**, when `conversation_cache` is `postgres` or `sqlite` **and** +`database` is the same type on a non-`/tmp` path, synthesis upserts +`storage.backends.conversations_default` from that cache and sets +`stores.conversations.backend: conversations_default` (unless `native_override` +retargets the store afterward). Without a matching durable `database`, +`sql_default` is left alone. See +[Conversation persistence (unified mode)](../user_doc/deployment_guide.md#conversation-persistence-unified-mode). + +#### 2. Lightspeed Stack database **Table:** `user_conversation` @@ -187,6 +195,11 @@ Stores user-specific metadata: - Message count - Topic summary +#### 3. Lightspeed conversation cache (optional) + +When configured, `conversation_cache` holds V2 Q&A history and topic summaries. +It does **not** replace the OGX conversations store used to continue chats. + --- ## API Endpoints diff --git a/docs/user_doc/deployment_guide.md b/docs/user_doc/deployment_guide.md index dc2a9a3d8..ac4bfd14b 100644 --- a/docs/user_doc/deployment_guide.md +++ b/docs/user_doc/deployment_guide.md @@ -147,6 +147,49 @@ The reference profiles are sanity-checked by the unit suite (`tests/unit/test_llama_stack_synthesize.py`), so they stay loadable as the synthesizer evolves. +### Conversation persistence (unified mode) + +When `conversation_cache` is `postgres` or `sqlite` **and** `database` is the +same backend type on a non-`/tmp` path, unified synthesis upserts an OGX backend +named `conversations_default` from that cache and points +`storage.stores.conversations` at it **before** applying +`llama_stack.config.native_override`. If `database` is missing, under `/tmp/`, +or a different type than the cache, synthesis leaves `sql_default` alone and +logs a warning. Inference/agents SQL on `sql_default` is left alone either way. +Continuing a chat after a restart (such as a Kubernetes Pod redeploy) needs the +wired OGX store; listing and ownership also need that durable matching-type +LCORE `database`. + +**Happy path.** Use unified library mode (`llama_stack.config.baseline` or a +profile), set durable `conversation_cache`, set `database` to the same backend +*type* (postgres or sqlite) on a non-ephemeral path, and do **not** set +`storage.stores.conversations` under `native_override`. See +[`examples/lightspeed-stack-unified-conversation-persistence-pg.yaml`](../../examples/lightspeed-stack-unified-conversation-persistence-pg.yaml). + +**`native_override` wins.** Dumb migration lifts a full `run.yaml` into +`native_override`, which usually still has `stores.conversations.backend: +sql_default`. That undoes enrichment; LCORE logs a warning and chats will not +survive restart until you remove that key, point it at `conversations_default`, +or accept a deliberate split. + +**`database` is a precondition for wiring.** If `database` is omitted (default +`/tmp/lightspeed-stack.db`), is under `/tmp/`, or is a different type than the +cache, synthesis does **not** retarget `stores.conversations` and warns instead. +Same-type different hosts/paths still wire and do not warn. + +**Secrets.** Prefer `${env.*}` for postgres +passwords. Literals are copied into `.generated/run.yaml` (mode 0600) and +trigger a warning. + +**SQLite.** Sharing one `db_path` between cache and OGX conversations is fine +for single-worker. Prefer Postgres for multi-worker / production. + +**Legacy two-file mode** is unchanged. Migrate to unified or edit `run.yaml` +manually. OpenAI-compatible `previous_response_id` continuation still uses an +ephemeral responses store and does not survive restart; normal continue-chat +via the `conversation` id does. + +More detail: [Conversations API Guide](../devel_doc/conversations_api.md). ### Llama Stack as a server diff --git a/examples/lightspeed-stack-unified-conversation-persistence-pg.yaml b/examples/lightspeed-stack-unified-conversation-persistence-pg.yaml new file mode 100644 index 000000000..aaa86c75b --- /dev/null +++ b/examples/lightspeed-stack-unified-conversation-persistence-pg.yaml @@ -0,0 +1,49 @@ +# Example: unified mode with durable conversation persistence (Postgres). +# +# Synthesis upserts OGX storage.backends.conversations_default from +# conversation_cache and points stores.conversations at it. Do not set +# storage.stores.conversations under native_override unless you intend to +# override that wiring (native_override wins; LCORE will warn). +# +# Keep database and conversation_cache on the same backend type so ownership +# metadata and chat continuation both survive restart. +name: Lightspeed Core Service (LCS) — unified conversation persistence +service: + host: localhost + port: 8080 + auth_enabled: false + workers: 1 + color_log: true + access_log: true +llama_stack: + use_as_library_client: true + config: + baseline: default +authentication: + module: "noop" +user_data_collection: + feedback_enabled: false + transcripts_enabled: false +database: + postgres: + host: ${env.POSTGRES_HOST:=127.0.0.1} + port: 5432 + db: ${env.POSTGRES_DB:=lightspeed} + user: ${env.POSTGRES_USER:=lightspeed} + password: ${env.POSTGRES_PASSWORD} + ssl_mode: disable + gss_encmode: disable +conversation_cache: + type: postgres + postgres: + host: ${env.POSTGRES_HOST:=127.0.0.1} + port: 5432 + db: ${env.POSTGRES_DB:=lightspeed} + user: ${env.POSTGRES_USER:=lightspeed} + password: ${env.POSTGRES_PASSWORD} + ssl_mode: disable + gss_encmode: disable +inference: + providers: + - type: openai + api_key_env: OPENAI_API_KEY diff --git a/src/app/main.py b/src/app/main.py index 378b5589f..6374fd59a 100644 --- a/src/app/main.py +++ b/src/app/main.py @@ -9,9 +9,8 @@ from fastapi import FastAPI, HTTPException from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse -from ogx_client import APIConnectionError, AsyncOgxClient from fastapi.routing import iter_route_contexts - +from ogx_client import APIConnectionError, AsyncOgxClient from starlette.types import ASGIApp, Message, Receive, Scope, Send import version diff --git a/src/constants.py b/src/constants.py index e53493c63..d2c6f2bd3 100644 --- a/src/constants.py +++ b/src/constants.py @@ -198,6 +198,17 @@ CACHE_TYPE_POSTGRES: Final[str] = "postgres" CACHE_TYPE_NOOP: Final[str] = "noop" +# Default sqlite path when DatabaseConfiguration has no backend configured. +# Ephemeral (typically tmpfs); conversation-persistence warnings treat this as non-durable. +DEFAULT_SQLITE_DATABASE_PATH: Final[str] = "/tmp/lightspeed-stack.db" + +# Dedicated OGX SQL backend for stores.conversations when conversation_cache is +# durable (RHIDP-14967). Injected only during unified synthesis; not seeded in +# default_run.yaml. +CONVERSATIONS_BACKEND_NAME: Final[str] = "conversations_default" +# Default OGX stores.conversations.table_name when the baseline/store omits one. +DEFAULT_CONVERSATIONS_TABLE_NAME: Final[str] = "openai_conversations" + # BYOK RAG # Default RAG type for bring-your-own-knowledge RAG configurations, that type # needs to be supported by Llama Stack diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 1a563bc33..4e3186f8e 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -20,9 +20,10 @@ import copy import os +import re from argparse import ArgumentParser from pathlib import Path -from typing import Any, Optional +from typing import Any, Final, Optional from urllib.parse import urljoin import yaml @@ -34,6 +35,10 @@ logger = get_logger(__name__) +_DURABLE_CACHE_TYPES: Final[frozenset[str]] = frozenset({"postgres", "sqlite"}) +# Used to warn when a postgres password is not an ${env.*} reference. +_ENV_REF_RE: Final[re.Pattern[str]] = re.compile(r"^\$\{env\.[^}]+\}$") + # Maps a UnifiedInferenceProvider.type (canonical, backend-agnostic vocabulary) # to the Llama Stack provider_type emitted by apply_high_level_inference. The # completeness of this map against UnifiedInferenceProvider.type is asserted by @@ -1110,6 +1115,197 @@ def ensure_mcp_tool_runtime(ls_config: dict[str, Any]) -> None: ) +def enrich_conversation_storage( + ls_config: dict[str, Any], + conversation_cache: Optional[dict[str, Any]], + lcs_config: dict[str, Any], +) -> None: + """Upsert ``conversations_default`` from a durable ``conversation_cache``. + + When ``conversation_cache.type`` is ``postgres`` or ``sqlite``, the selected + backend block has required fields, **and** ``lcs_config`` has a matching + durable ``database`` (same type; sqlite not under ``/tmp``), writes + ``storage.backends.conversations_default`` and sets + ``storage.stores.conversations.backend`` to that name. Leaves + ``sql_default`` and other backends untouched. Reads the raw YAML dict + slice (not a validated model) so passwords are not masked as SecretStr. + + Parameters: + ls_config: Mutable Llama Stack / OGX configuration being synthesized. + conversation_cache: Raw ``conversation_cache`` mapping from + ``lightspeed-stack.yaml``, or None. + lcs_config: Raw full ``lightspeed-stack.yaml`` dict (used to gate on + durable matching ``database``). + + Returns: + None: ``ls_config`` is modified in place. Incomplete, non-durable, or + ephemeral/mismatched-database configs are skipped with no mutation. + """ + if not isinstance(conversation_cache, dict): + return + cache_type = conversation_cache.get("type") + if cache_type not in _DURABLE_CACHE_TYPES: + return + + if _database_is_ephemeral_or_mismatched(lcs_config, str(cache_type)): + return + + block = conversation_cache.get(cache_type) + if not isinstance(block, dict): + return + + if cache_type == "sqlite": + db_path = block.get("db_path") + if not isinstance(db_path, str) or not db_path.strip(): + return + backend_cfg: dict[str, Any] = { + "type": "sql_sqlite", + "db_path": db_path, + } + else: + # host/port are optional on PostgreSQLDatabaseConfiguration (default + # localhost/5432); only require fields the model itself requires. + required = ("db", "user", "password") + for key in required: + value = block.get(key) + if not isinstance(value, str) or not value.strip(): + return + backend_cfg = { + "type": "sql_postgres", + "host": block.get("host", "localhost"), + "port": block.get("port", 5432), + "db": block["db"], + "user": block["user"], + "password": block["password"], + } + + # Treat YAML nulls (storage:, backends:, stores:) as empty dicts. setdefault + # alone is not enough: a present key with value None is returned as-is. + storage = ls_config.get("storage") or {} + ls_config["storage"] = storage + backends = storage.get("backends") or {} + storage["backends"] = backends + backends[constants.CONVERSATIONS_BACKEND_NAME] = backend_cfg + + stores = storage.get("stores") or {} + storage["stores"] = stores + conversations = stores.get("conversations") + if not isinstance(conversations, dict): + conversations = {} + stores["conversations"] = conversations + table_name = conversations.get("table_name") + if not isinstance(table_name, str) or not table_name.strip(): + conversations["table_name"] = constants.DEFAULT_CONVERSATIONS_TABLE_NAME + conversations["backend"] = constants.CONVERSATIONS_BACKEND_NAME + + logger.info( + "Conversation persistence: wired stores.conversations to %s from " + "conversation_cache.type=%r", + constants.CONVERSATIONS_BACKEND_NAME, + cache_type, + ) + + +def _database_is_ephemeral_or_mismatched( + lcs_config: dict[str, Any], cache_type: str +) -> bool: + """Return True when raw database is absent, /tmp sqlite, or type-mismatched.""" + database = lcs_config.get("database") + if not isinstance(database, dict): + return True + if database.get("postgres") is not None: + db_type = "postgres" + elif database.get("sqlite") is not None: + db_type = "sqlite" + else: + return True + if db_type != cache_type: + return True + if db_type == "sqlite": + path = (database.get("sqlite") or {}).get("db_path") + if isinstance(path, str) and ( + path.startswith("/tmp/") or path == constants.DEFAULT_SQLITE_DATABASE_PATH + ): + return True + return False + + +def warn_conversation_persistence( + ls_config: dict[str, Any], lcs_config: dict[str, Any] +) -> list[str]: + """Log post-merge conversation-persistence footguns; return message list. + + When durable ``conversation_cache`` is set but ``database`` is ephemeral or + type-mismatched (enrichment is skipped), warns that a durable matching + database is required. When a durable matching database is present but final + ``stores.conversations`` is not ``conversations_default`` (typically + ``native_override``), warns that override still owns the store. When a + postgres password is a non-``${env…}`` literal, warns to prefer an env + reference. Never logs secret values. + + Parameters: + ls_config: Final synthesized Llama Stack configuration (after override). + lcs_config: Raw full ``lightspeed-stack.yaml`` dict. + + Returns: + list[str]: Warning messages that were logged (empty when nothing to warn). + """ + messages: list[str] = [] + cache = lcs_config.get("conversation_cache") + if not isinstance(cache, dict) or cache.get("type") not in _DURABLE_CACHE_TYPES: + return messages + cache_type = str(cache["type"]) + + storage = ls_config.get("storage") or {} + stores = storage.get("stores") or {} + backends = storage.get("backends") or {} + conversations = stores.get("conversations") if isinstance(stores, dict) else None + backend_name = ( + conversations.get("backend") if isinstance(conversations, dict) else None + ) + + if _database_is_ephemeral_or_mismatched(lcs_config, cache_type): + messages.append( + "Conversation persistence: durable conversation_cache is set but " + "database is ephemeral or type-mismatched; configure a durable " + f"database of type {cache_type!r} so ownership metadata survives " + "restart." + ) + elif ( + backend_name != constants.CONVERSATIONS_BACKEND_NAME + or not isinstance(backends, dict) + or constants.CONVERSATIONS_BACKEND_NAME not in backends + ): + messages.append( + "Conversation persistence: durable conversation_cache is set but " + "native_override still owns storage.stores.conversations (or " + f"{constants.CONVERSATIONS_BACKEND_NAME} is missing). Remove or " + "retarget that key under llama_stack.config.native_override, or " + f"point it at {constants.CONVERSATIONS_BACKEND_NAME}." + ) + + password = None + backend = ( + backends.get(constants.CONVERSATIONS_BACKEND_NAME) + if isinstance(backends, dict) + else None + ) + if isinstance(backend, dict) and backend.get("type") == "sql_postgres": + password = backend.get("password") + elif cache_type == "postgres" and isinstance(cache.get("postgres"), dict): + password = cache["postgres"].get("password") + if isinstance(password, str) and password and _ENV_REF_RE.match(password) is None: + messages.append( + "Conversation persistence: literal postgres password detected in " + "conversation_cache; prefer an ${env.VAR} reference so secrets are " + "not written into the synthesized run.yaml." + ) + + for msg in messages: + logger.warning(msg) + return messages + + def _resolve_profile_path(profile: str, config_file_dir: Optional[str]) -> Path: """Resolve a ``profile:`` path against the loaded config's directory (R8). @@ -1141,8 +1337,10 @@ def synthesize_configuration( file, empty, or the built-in default), apply the existing enrichment (Azure Entra ID, BYOK RAG, Solr/OKP) for parity with legacy mode (R7), expand the high-level ``inference.providers`` section, ensure the default - MCP tool_runtime provider when the baseline was not empty, and deep-merge - the raw ``native_override`` last (R5). + MCP tool_runtime provider when the baseline was not empty, wire durable + ``conversation_cache`` into ``stores.conversations`` when ``database`` is + durable and type-matched (RHIDP-14967), deep-merge the raw + ``native_override`` last (R5), then warn on persistence footguns. Parameters: lcs_config: The full ``lightspeed-stack.yaml`` parsed into a dict. @@ -1197,6 +1395,15 @@ def synthesize_configuration( if not baseline_was_empty: ensure_mcp_tool_runtime(ls_config) + # 6b. Durable conversation_cache → conversations_default (before override), + # only when database is durable and type-matched. + cache_raw = lcs_config.get("conversation_cache") + enrich_conversation_storage( + ls_config, + cache_raw if isinstance(cache_raw, dict) else None, + lcs_config, + ) + # 7. Raw escape hatch, deep-merged last with list replacement (R5). if unified and unified.get("native_override"): ls_config = deep_merge_list_replace(ls_config, unified["native_override"]) @@ -1204,6 +1411,9 @@ def synthesize_configuration( # 8. Dedupe again in case native_override or enrichment reintroduced dupes. dedupe_providers_vector_io(ls_config) + # 9. Persistence footgun warnings (override already applied). + warn_conversation_persistence(ls_config, lcs_config) + return ls_config diff --git a/src/models/config.py b/src/models/config.py index 757eeaa13..bfe8a9da5 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -290,7 +290,7 @@ def check_database_configuration(self) -> Self: Ensure exactly one database backend is configured, defaulting to a temporary SQLite one. If neither `sqlite` nor `postgres` is set, assigns a default SQLite - configuration using the file path "/tmp/lightspeed-stack.db". If both + configuration using ``constants.DEFAULT_SQLITE_DATABASE_PATH``. If both backends are configured, raises a `ValueError`. Returns: @@ -303,8 +303,9 @@ def check_database_configuration(self) -> Self: # Default to SQLite in a (hopefully) tmpfs if no database configuration is provided. # This is good for backwards compatibility for deployments that do not mind having # no persistent database. - sqlite_file_name = "/tmp/lightspeed-stack.db" - self.sqlite = SQLiteDatabaseConfiguration(db_path=sqlite_file_name) + self.sqlite = SQLiteDatabaseConfiguration( + db_path=constants.DEFAULT_SQLITE_DATABASE_PATH + ) elif total_configured_dbs > 1: raise ValueError("Only one database configuration can be provided") diff --git a/tests/e2e/configuration/library-mode/lightspeed-stack-unified-conversation-persistence-sqlite.yaml b/tests/e2e/configuration/library-mode/lightspeed-stack-unified-conversation-persistence-sqlite.yaml new file mode 100644 index 000000000..4d0dbccf7 --- /dev/null +++ b/tests/e2e/configuration/library-mode/lightspeed-stack-unified-conversation-persistence-sqlite.yaml @@ -0,0 +1,48 @@ +name: Lightspeed Core Service (LCS) +service: + host: 0.0.0.0 + port: 8080 + auth_enabled: false + workers: 1 + color_log: true + access_log: true +llama_stack: + use_as_library_client: true + # Unified mode: run.yaml is the synthesis profile. Do not set + # storage.stores.conversations under native_override — synthesis wires + # conversations_default from conversation_cache. + config: + profile: run.yaml +user_data_collection: + feedback_enabled: true + feedback_storage: "/tmp/data/feedback" + transcripts_enabled: true + transcripts_storage: "/tmp/data/transcripts" +authentication: + module: "noop" +# Same sqlite file for LCORE database + conversation_cache (and, via synthesis, +# OGX conversations_default). Parent dir must already exist in the image +# (LCORE does not mkdir). Outside /tmp (not ephemeral) and outside ~/.llama +# (e2e may rm -rf that tree). Survives `docker restart` on the container FS. +database: + sqlite: + db_path: "/opt/app-root/src/conversation-persistence.db" +conversation_cache: + type: sqlite + sqlite: + db_path: "/opt/app-root/src/conversation-persistence.db" +inference: + default_provider: openai + default_model: gpt-4o-mini +byok_rag: + - rag_id: e2e-test-docs + rag_type: inline::faiss + embedding_model: sentence-transformers/all-mpnet-base-v2 + embedding_dimension: 768 + vector_db_id: ${env.FAISS_VECTOR_STORE_ID} + db_path: ${env.KV_RAG_PATH:=~/.llama/storage/rag/kv_store.db} + score_multiplier: 1.0 + +rag: + tool: + - e2e-test-docs diff --git a/tests/e2e/features/unified-mode-conversation-persistence.feature b/tests/e2e/features/unified-mode-conversation-persistence.feature new file mode 100644 index 000000000..f5757794e --- /dev/null +++ b/tests/e2e/features/unified-mode-conversation-persistence.feature @@ -0,0 +1,30 @@ +@cfg_unified @skip-in-server-mode +Feature: Unified mode conversation persistence across restart + + # Proves RHIDP-14967: with conversation_cache + matching database on a + # non-/tmp sqlite path, synthesis wires OGX stores.conversations to + # conversations_default so a conversation-id continue succeeds after + # lightspeed-stack process restart (docker restart). Re-run on ogx bumps. + + Background: + Given The service is started locally + And The system is in default state + And REST API service prefix is /v1 + And the Lightspeed stack configuration directory is "tests/e2e/configuration" + And The service uses the lightspeed-stack-unified-conversation-persistence-sqlite.yaml configuration + And The service is restarted + + Scenario: Continue chat after Lightspeed restart with sqlite conversation persistence + When I use "query" to ask question + """ + {"query": "Say the word apple and nothing else", "model": "{MODEL}", "provider": "{PROVIDER}", "no_tools": true} + """ + Then The status code of the response is 200 + And I store conversation details + Given The service is restarted + When I use "query" to ask question with same conversation_id + """ + {"query": "What single word did I ask you to say earlier?", "model": "{MODEL}", "provider": "{PROVIDER}", "no_tools": true} + """ + Then The status code of the response is 200 + And The body of the response contains apple diff --git a/tests/e2e/test_list.txt b/tests/e2e/test_list.txt index 2a5d97f05..5a42ec61d 100644 --- a/tests/e2e/test_list.txt +++ b/tests/e2e/test_list.txt @@ -4,38 +4,38 @@ features/info.feature features/models.feature features/rest_api.feature features/smoketests.feature -features/inline_rag.feature -features/proxy.feature -features/llama_stack_disrupted.feature features/authorized_noop_token.feature +features/conversation_cache_v2.feature features/conversations.feature -features/faiss.feature -features/opentelemetry.feature features/prompts.feature +features/faiss.feature +features/inline_rag.feature +features/byok_pdf.feature +features/vector_stores.feature +features/feedback.feature features/query.feature features/responses.feature features/responses_streaming.feature features/rlsapi_v1.feature features/streaming_query.feature -features/vector_stores.feature -features/conversation_cache_v2.feature -features/feedback.feature features/http_401_unauthorized.feature +features/authorized_rh_identity.feature features/rbac.feature features/rlsapi_v1_errors.feature -features/skills.feature -features/authorized_rh_identity.feature -features/mcp_servers_api.feature +features/llama_stack_disrupted.feature features/mcp.feature +features/mcp_servers_api.feature features/mcp_servers_api_auth.feature features/mcp_servers_api_no_config.feature -features/byok_pdf.feature +features/proxy.feature features/tls-ca.feature features/tls-mtls.feature features/tls-tlsv13.feature -features/degraded_mode_startup.feature +features/opentelemetry.feature features/unified-mode-boot.feature features/unified-mode-legacy.feature features/unified-mode-validation.feature features/unified-mode-migration.feature features/unified-mode-synthesis.feature +features/unified-mode-conversation-persistence.feature +features/skills.feature diff --git a/tests/unit/test_llama_stack_conversation_persistence.py b/tests/unit/test_llama_stack_conversation_persistence.py new file mode 100644 index 000000000..1550bc70f --- /dev/null +++ b/tests/unit/test_llama_stack_conversation_persistence.py @@ -0,0 +1,699 @@ +"""Unit tests for conversation_cache → OGX conversations wiring (RHIDP-14967).""" + +import copy +from typing import Any, Optional + +import pytest + +from constants import CONVERSATIONS_BACKEND_NAME, DEFAULT_CONVERSATIONS_TABLE_NAME +from llama_stack_configuration import ( + enrich_conversation_storage, + load_default_baseline, + synthesize_configuration, + warn_conversation_persistence, +) + +# --------------------------------------------------------------------------- +# conversation persistence from conversation_cache (RHIDP-14967) +# --------------------------------------------------------------------------- + + +def _lcs_with_matching_sqlite( + db_path: str = "/var/lib/lightspeed/app.db", +) -> dict[str, Any]: + """Build lcs_config with matching durable sqlite cache + database.""" + return { + "conversation_cache": { + "type": "sqlite", + "sqlite": {"db_path": db_path}, + }, + "database": {"sqlite": {"db_path": db_path}}, + } + + +def _lcs_with_matching_postgres( + password: str = "${env.POSTGRES_PASSWORD}", + **postgres_overrides: Any, +) -> dict[str, Any]: + """Build lcs_config with matching durable postgres cache + database.""" + postgres: dict[str, Any] = { + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": password, + } + postgres.update(postgres_overrides) + return { + "conversation_cache": {"type": "postgres", "postgres": dict(postgres)}, + "database": {"postgres": dict(postgres)}, + } + + +def _durable_pg_cache(password: str = "${env.POSTGRES_PASSWORD}") -> dict[str, Any]: + """Build a minimal durable postgres conversation_cache dict.""" + return { + "type": "postgres", + "postgres": { + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": password, + }, + } + + +def test_enrich_skips_when_database_absent() -> None: + """Cache-only (typical library E2E): do not retarget stores.conversations.""" + ls_config = load_default_baseline() + before = copy.deepcopy(ls_config) + cache = { + "type": "sqlite", + "sqlite": {"db_path": "/tmp/data/conversation-cache.db"}, + } + enrich_conversation_storage(ls_config, cache, {"conversation_cache": cache}) + assert ls_config == before + + +def test_enrich_skips_when_database_under_tmp() -> None: + """Ephemeral /tmp database blocks enrichment even with a durable cache path.""" + ls_config = load_default_baseline() + before = copy.deepcopy(ls_config) + cache = {"type": "sqlite", "sqlite": {"db_path": "/data/cache.db"}} + lcs = { + "conversation_cache": cache, + "database": {"sqlite": {"db_path": "/tmp/lightspeed-stack.db"}}, + } + enrich_conversation_storage(ls_config, cache, lcs) + assert ls_config == before + + +def test_enrich_skips_on_type_mismatch() -> None: + """Postgres cache + sqlite database must not retarget conversations.""" + ls_config = load_default_baseline() + before = copy.deepcopy(ls_config) + cache = { + "type": "postgres", + "postgres": { + "host": "h", + "db": "d", + "user": "u", + "password": "p", + }, + } + lcs = { + "conversation_cache": cache, + "database": {"sqlite": {"db_path": "/var/lib/lightspeed/app.db"}}, + } + enrich_conversation_storage(ls_config, cache, lcs) + assert ls_config == before + + +def test_enrich_runs_when_database_matches_sqlite() -> None: + """Matching durable sqlite database allows conversations_default wiring.""" + ls_config = load_default_baseline() + lcs = _lcs_with_matching_sqlite("/var/lib/lightspeed/cache.db") + enrich_conversation_storage(ls_config, lcs["conversation_cache"], lcs) + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + + +def test_enrich_conversation_storage_postgres_h1() -> None: + """H1: postgres cache upserts conversations_default and retargets store.""" + ls_config = load_default_baseline() + sql_default_before = copy.deepcopy(ls_config["storage"]["backends"]["sql_default"]) + cache = { + "type": "postgres", + "postgres": { + "host": "db.example.com", + "db": "lightspeed", + "user": "ls", + "password": "${env.POSTGRES_PASSWORD}", + }, + } + lcs = { + "conversation_cache": cache, + "database": { + "postgres": { + "host": "db.example.com", + "db": "lightspeed", + "user": "ls", + "password": "${env.POSTGRES_PASSWORD}", + } + }, + } + enrich_conversation_storage(ls_config, cache, lcs) + backend = ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME] + assert backend == { + "type": "sql_postgres", + "host": "db.example.com", + "port": 5432, + "db": "lightspeed", + "user": "ls", + "password": "${env.POSTGRES_PASSWORD}", + } + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + assert ls_config["storage"]["backends"]["sql_default"] == sql_default_before + + +def test_enrich_conversation_storage_sqlite_h2() -> None: + """H2: sqlite cache shares db_path on conversations_default.""" + ls_config = load_default_baseline() + lcs = _lcs_with_matching_sqlite("/var/lib/lightspeed/cache.db") + enrich_conversation_storage(ls_config, lcs["conversation_cache"], lcs) + assert ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME] == { + "type": "sql_sqlite", + "db_path": "/var/lib/lightspeed/cache.db", + } + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + + +def test_enrich_conversation_storage_creates_store_when_missing_h3() -> None: + """H3: missing stores.conversations is created with default table_name.""" + ls_config: dict[str, Any] = {"storage": {"backends": {}}} + lcs = _lcs_with_matching_sqlite("/data/cache.db") + enrich_conversation_storage(ls_config, lcs["conversation_cache"], lcs) + assert ls_config["storage"]["stores"]["conversations"] == { + "table_name": DEFAULT_CONVERSATIONS_TABLE_NAME, + "backend": CONVERSATIONS_BACKEND_NAME, + } + + +@pytest.mark.parametrize( + "ls_config", + [ + {"storage": None}, + {"storage": {"backends": None, "stores": None}}, + {"storage": {"backends": {}, "stores": None}}, + ], +) +def test_enrich_conversation_storage_tolerates_null_storage_nodes( + ls_config: dict[str, Any], +) -> None: + """YAML null storage/backends/stores must not crash; wire from empty dicts.""" + lcs = _lcs_with_matching_sqlite("/data/cache.db") + enrich_conversation_storage(ls_config, lcs["conversation_cache"], lcs) + assert ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME] == { + "type": "sql_sqlite", + "db_path": "/data/cache.db", + } + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + + +def test_enrich_conversation_storage_preserves_custom_table_name() -> None: + """G18: existing table_name is preserved while backend is overwritten.""" + ls_config = { + "storage": { + "backends": {}, + "stores": { + "conversations": { + "table_name": "custom_convs", + "backend": "sql_default", + } + }, + } + } + lcs = _lcs_with_matching_sqlite("/data/cache.db") + enrich_conversation_storage(ls_config, lcs["conversation_cache"], lcs) + assert ( + ls_config["storage"]["stores"]["conversations"]["table_name"] == "custom_convs" + ) + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + + +@pytest.mark.parametrize( + "cache", + [ + None, + {}, + {"type": "noop"}, + {"type": "memory", "memory": {"max_entries": 10}}, + {"type": "postgres"}, + {"type": "postgres", "postgres": {"host": "h", "db": "d", "user": "u"}}, + {"type": "sqlite", "sqlite": {}}, + ], +) +def test_enrich_conversation_storage_skips_incomplete_e1_e2( + cache: Optional[dict[str, Any]], +) -> None: + """E1/E2: non-durable or incomplete cache leaves ls_config unchanged.""" + ls_config = load_default_baseline() + before = copy.deepcopy(ls_config) + enrich_conversation_storage(ls_config, cache, {}) + assert ls_config == before + + +def test_enrich_conversation_storage_defaults_port_e5() -> None: + """E5: omitted postgres port defaults to 5432.""" + ls_config = load_default_baseline() + cache = { + "type": "postgres", + "postgres": { + "host": "h", + "db": "d", + "user": "u", + "password": "p", + }, + } + lcs = _lcs_with_matching_postgres(password="p", host="h", db="d", user="u") + enrich_conversation_storage(ls_config, cache, lcs) + assert ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME]["port"] == 5432 + + +def test_enrich_conversation_storage_defaults_host() -> None: + """Omitted postgres host defaults to localhost (mirrors model default).""" + ls_config = load_default_baseline() + cache = { + "type": "postgres", + "postgres": { + "db": "d", + "user": "u", + "password": "p", + }, + } + lcs = { + "conversation_cache": cache, + "database": { + "postgres": { + "db": "d", + "user": "u", + "password": "p", + } + }, + } + enrich_conversation_storage(ls_config, cache, lcs) + backend = ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME] + assert backend == { + "type": "sql_postgres", + "host": "localhost", + "port": 5432, + "db": "d", + "user": "u", + "password": "p", + } + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + + +def test_enrich_conversation_storage_retargets_existing_sql_postgres_e8() -> None: + """E8/G2: profile backend other_pg is overwritten to conversations_default.""" + ls_config: dict[str, Any] = { + "storage": { + "backends": { + "other_pg": { + "type": "sql_postgres", + "host": "a", + "port": 5432, + "db": "a", + "user": "a", + "password": "a", + } + }, + "stores": { + "conversations": { + "table_name": DEFAULT_CONVERSATIONS_TABLE_NAME, + "backend": "other_pg", + } + }, + } + } + cache = { + "type": "postgres", + "postgres": { + "host": "b", + "port": 5432, + "db": "b", + "user": "b", + "password": "b", + }, + } + lcs = { + "conversation_cache": cache, + "database": { + "postgres": { + "host": "b", + "port": 5432, + "db": "b", + "user": "b", + "password": "b", + } + }, + } + enrich_conversation_storage(ls_config, cache, lcs) + assert ( + ls_config["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + assert ls_config["storage"]["backends"][CONVERSATIONS_BACKEND_NAME]["host"] == "b" + + +def test_warn_when_override_clobbers_conversations_backend() -> None: + """Warn when final conversations backend is not conversations_default.""" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_sqlite", + "db_path": "/var/lib/lightspeed/x.db", + } + }, + "stores": { + "conversations": { + "backend": "sql_default", + "table_name": DEFAULT_CONVERSATIONS_TABLE_NAME, + } + }, + } + } + lcs = _lcs_with_matching_sqlite("/var/lib/lightspeed/x.db") + msgs = warn_conversation_persistence(ls_config, lcs) + assert any( + "native_override still owns storage.stores.conversations" in msg for msg in msgs + ) + + +def test_warn_when_database_absent() -> None: + """Warn when database key is absent while durable cache is set.""" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_sqlite", + "db_path": "/data/c.db", + } + }, + "stores": {"conversations": {"backend": CONVERSATIONS_BACKEND_NAME}}, + } + } + lcs = { + "conversation_cache": {"type": "sqlite", "sqlite": {"db_path": "/data/c.db"}} + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert any("database is ephemeral or type-mismatched" in msg for msg in msgs) + + +def test_warn_absent_database_does_not_blame_native_override() -> None: + """Cache-only / ephemeral DB warns about database, not about native_override.""" + ls_config = { + "storage": { + "backends": {"sql_default": {"type": "sql_sqlite", "db_path": "/x"}}, + "stores": {"conversations": {"backend": "sql_default"}}, + } + } + lcs = { + "conversation_cache": { + "type": "sqlite", + "sqlite": {"db_path": "/tmp/data/conversation-cache.db"}, + } + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert any("database is ephemeral or type-mismatched" in m for m in msgs) + assert not any("native_override still owns" in m for m in msgs) + + +def test_warn_when_database_type_mismatches_cache() -> None: + """Warn when cache is postgres and database is sqlite.""" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_postgres", + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": "p", + } + }, + "stores": {"conversations": {"backend": CONVERSATIONS_BACKEND_NAME}}, + } + } + lcs = { + "conversation_cache": _durable_pg_cache(password="p"), + "database": {"sqlite": {"db_path": "/var/lib/lightspeed/app.db"}}, + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert any("database is ephemeral or type-mismatched" in msg for msg in msgs) + + +def test_warn_no_database_warning_when_matching_postgres() -> None: + """No database warning when database and cache are both postgres.""" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_postgres", + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": "${env.P}", + } + }, + "stores": {"conversations": {"backend": CONVERSATIONS_BACKEND_NAME}}, + } + } + lcs = { + "conversation_cache": _durable_pg_cache(password="${env.P}"), + "database": { + "postgres": { + "host": "other", + "db": "other", + "user": "u", + "password": "${env.P}", + } + }, + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert not any("database is ephemeral or type-mismatched" in msg for msg in msgs) + + +def test_warn_literal_postgres_password() -> None: + """Warn for literal password; secret value must not appear in messages.""" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_postgres", + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": "s3cret", + } + }, + "stores": {"conversations": {"backend": CONVERSATIONS_BACKEND_NAME}}, + } + } + lcs = { + "conversation_cache": _durable_pg_cache(password="s3cret"), + "database": { + "postgres": { + "host": "h", + "db": "d", + "user": "u", + "password": "s3cret", + } + }, + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert any("literal postgres password" in msg for msg in msgs) + assert all("s3cret" not in msg for msg in msgs) + + +def test_warn_no_literal_password_for_env_ref_with_default() -> None: + """${env.VAR:=default} does not trigger the literal-password warning.""" + password = "${env.POSTGRES_PASSWORD:=secret}" + ls_config = { + "storage": { + "backends": { + CONVERSATIONS_BACKEND_NAME: { + "type": "sql_postgres", + "host": "h", + "port": 5432, + "db": "d", + "user": "u", + "password": password, + } + }, + "stores": {"conversations": {"backend": CONVERSATIONS_BACKEND_NAME}}, + } + } + lcs = { + "conversation_cache": _durable_pg_cache(password=password), + "database": { + "postgres": { + "host": "h", + "db": "d", + "user": "u", + "password": password, + } + }, + } + msgs = warn_conversation_persistence(ls_config, lcs) + assert not any("literal postgres password" in msg for msg in msgs) + + +def test_warn_noop_when_cache_not_durable() -> None: + """E1: no warnings when conversation_cache is not durable.""" + msgs = warn_conversation_persistence( + load_default_baseline(), + {"conversation_cache": {"type": "memory", "memory": {"max_entries": 1}}}, + ) + assert not msgs + + +def test_synthesize_wires_postgres_cache_h1() -> None: + """Pipeline H1: synthesize wires postgres cache to conversations_default.""" + lcs = { + "llama_stack": { + "use_as_library_client": True, + "config": {"baseline": "default"}, + }, + "conversation_cache": { + "type": "postgres", + "postgres": { + "host": "db.example.com", + "db": "lightspeed", + "user": "ls", + "password": "${env.POSTGRES_PASSWORD}", + }, + }, + "database": { + "postgres": { + "host": "db.example.com", + "db": "lightspeed", + "user": "ls", + "password": "${env.POSTGRES_PASSWORD}", + } + }, + } + result = synthesize_configuration(lcs) + assert ( + result["storage"]["stores"]["conversations"]["backend"] + == CONVERSATIONS_BACKEND_NAME + ) + assert ( + result["storage"]["backends"][CONVERSATIONS_BACKEND_NAME]["type"] + == "sql_postgres" + ) + + +def test_synthesize_cache_only_keeps_sql_default( + caplog: pytest.LogCaptureFixture, +) -> None: + """Library-E2E shape: sqlite cache, no database → no enrich; database warning only.""" + lcs = { + "llama_stack": { + "use_as_library_client": True, + "config": {"baseline": "default"}, + }, + "conversation_cache": { + "type": "sqlite", + "sqlite": {"db_path": "/tmp/data/conversation-cache.db"}, + }, + } + with caplog.at_level( + "WARNING", logger="lightspeed_stack.llama_stack_configuration" + ): + result = synthesize_configuration(lcs) + assert result["storage"]["stores"]["conversations"]["backend"] == "sql_default" + assert CONVERSATIONS_BACKEND_NAME not in result["storage"]["backends"] + assert any( + "database is ephemeral or type-mismatched" in r.message for r in caplog.records + ) + assert not any( + "native_override still owns storage.stores.conversations" in r.message + for r in caplog.records + ) + + +def test_synthesize_override_wins_u1(caplog: pytest.LogCaptureFixture) -> None: + """native_override restores sql_default; unused backend remains; override warning.""" + lcs = { + "llama_stack": { + "use_as_library_client": True, + "config": { + "baseline": "default", + "native_override": { + "storage": { + "stores": { + "conversations": { + "table_name": DEFAULT_CONVERSATIONS_TABLE_NAME, + "backend": "sql_default", + } + } + } + }, + }, + }, + "conversation_cache": { + "type": "sqlite", + "sqlite": {"db_path": "/data/cache.db"}, + }, + "database": {"sqlite": {"db_path": "/data/cache.db"}}, + } + with caplog.at_level( + "WARNING", logger="lightspeed_stack.llama_stack_configuration" + ): + result = synthesize_configuration(lcs) + assert result["storage"]["stores"]["conversations"]["backend"] == "sql_default" + assert CONVERSATIONS_BACKEND_NAME in result["storage"]["backends"] + assert "native_override still owns storage.stores.conversations" in caplog.text + + +def test_synthesize_migrate_shape_with_durable_cache_u2( + caplog: pytest.LogCaptureFixture, +) -> None: + """Dumb-migrate shape keeps override-owned conversations backend + override warning.""" + run_yaml = { + "version": 2, + "storage": { + "backends": { + "sql_default": { + "type": "sql_sqlite", + "db_path": "/tmp/sql.db", + } + }, + "stores": { + "conversations": { + "table_name": DEFAULT_CONVERSATIONS_TABLE_NAME, + "backend": "sql_default", + } + }, + }, + } + lcs = { + "llama_stack": { + "use_as_library_client": True, + "config": {"baseline": "empty", "native_override": run_yaml}, + }, + "conversation_cache": { + "type": "sqlite", + "sqlite": {"db_path": "/data/cache.db"}, + }, + "database": {"sqlite": {"db_path": "/data/cache.db"}}, + } + with caplog.at_level( + "WARNING", logger="lightspeed_stack.llama_stack_configuration" + ): + result = synthesize_configuration(lcs) + assert result["storage"]["stores"]["conversations"]["backend"] == "sql_default" + assert "native_override still owns storage.stores.conversations" in caplog.text