diff --git a/AGENTS.md b/AGENTS.md index fc3fa8d83..ca8b025e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -397,3 +397,75 @@ Allowed prefixes: `LCORE-`, `RSPEED-`, `MGTM-`, `OLS-`, `RHIDP-`, `LEADS-`, `CWF 4. Follow existing code patterns in the module you're modifying 5. Write unit tests covering new functionality 6. Run format and verify before completion + +## Lola Skills + +These skills are installed by Lola and provide specialized capabilities. +When a task matches a skill's description, read the skill's SKILL.md file +to learn the detailed instructions and workflows. + +**How to use skills:** +1. Check if your task matches any skill description below +2. Use `read_file` to read the skill's SKILL.md for detailed instructions +3. Follow the instructions in the SKILL.md file + + + +### ixd-firefly-custom-skills + +#### api-spec-validator +**When to use:** Validate API endpoint implementations against OpenAPI or Swagger specifications for correctness and completeness. Use when checking API compliance, auditing spec drift, or verifying that route handlers match their documented contracts. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/api-spec-validator/SKILL.md` for detailed guidance. + +#### code-reviewer +**When to use:** Perform structured code reviews with a comprehensive checklist covering correctness, security, performance, and maintainability. Use when reviewing a diff, pull request, merge request, or set of changed files before merge. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/code-reviewer/SKILL.md` for detailed guidance. + +#### coverage-analyzer +**When to use:** Analyze test coverage reports to identify gaps in line, branch, and function coverage, then suggest targeted tests to close those gaps and reach 100% coverage. Use when coverage is below target, after adding new code, or when preparing for a release. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/coverage-analyzer/SKILL.md` for detailed guidance. + +#### cypress-skill +**When to use:** Generates production-grade Cypress E2E and component tests in JavaScript or TypeScript, with support for local execution. Use when the user asks to write or debug Cypress tests, set up Cypress, work with cy commands, or mentions Cypress, "component test", "E2E test", "cy.", or common APIs such as cy.visit, cy.get, or cy.intercept. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/cypress-skill/SKILL.md` for detailed guidance. + +#### evolution +**When to use:** Updates skill files based on user corrections during a session. Use when a user's guidance contradicts or extends a loaded skill's instructions, enabling skills to learn and improve from real-world usage. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/evolution/SKILL.md` for detailed guidance. + +#### flaky-test-detector +**When to use:** Identify flaky tests by running test suites multiple times and analyzing inconsistent pass/fail results. Use when tests fail intermittently, CI builds are unreliable, or when diagnosing non-deterministic test behavior. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/flaky-test-detector/SKILL.md` for detailed guidance. + +#### security-scanner +**When to use:** Scan source code for common security vulnerabilities including injection flaws, authentication issues, sensitive data exposure, XSS, and dependency CVEs. Use when performing security audits, reviewing code before deployment, or hardening an application. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/security-scanner/SKILL.md` for detailed guidance. + +#### test-runner +**When to use:** Execute project test suites for unit testing with coverage and functional testing. Use when running tests, checking coverage, verifying test results, or executing a specific test file or method. +**Instructions:** Read `.lola/modules/ixd-firefly-custom-skills/skills/test-runner/SKILL.md` for detailed guidance. + + + + + +--- +name: ixd-firefly-custom-skills +description: Shared agent skills for harness-cli and OpenCode following the Agent Skills standard +--- + +# Harness Skills Module + +This AI Context Module provides shared skills for use with [harness-cli](https://github.com/your-org/ixd-firefly-harness-engineering-cli) and [OpenCode](https://opencode.ai). + +All skills in this module follow the [Agent Skills](https://agentskills.io) open standard, making them compatible with any agent that supports the specification, including **OpenCode**, **Claude Code**, **Cursor**, **GitHub Copilot**, **Gemini CLI**, and [many others](https://agentskills.io/home). + +## Available Skills + +Skills include: `test-runner`, `coverage-analyzer`, `flaky-test-detector`, `code-reviewer`, `security-scanner`, `api-spec-validator`, `cypress-skill`, and `evolution` (enables skills to learn from user corrections during sessions). See [SKILLS_QUICKSTART.md](./SKILLS_QUICKSTART.md) for the complete list with descriptions. + +## Installation + +Install skills into your project using `harness-cli` or manually by copying skill directories into your project's `.opencode/skills/` directory. See [README.md](./README.md) for full installation and usage instructions. + + diff --git a/src/app/endpoints/rags.py b/src/app/endpoints/rags.py index 8cf5c7679..664f915f0 100644 --- a/src/app/endpoints/rags.py +++ b/src/app/endpoints/rags.py @@ -177,9 +177,7 @@ async def get_rag_endpoint_handler( logger.info("Llama stack config: %s", llama_stack_configuration) # Resolve user-facing rag_id to llama-stack vector_db_id - vector_db_id = _resolve_rag_id_to_vector_db_id( - rag_id, configuration.configuration.byok_rag - ) + vector_db_id = _resolve_rag_id_to_vector_db_id(rag_id, configuration.byok_rag) try: # try to get Llama Stack client diff --git a/src/client.py b/src/client.py index 05e6fe250..bf2665d8d 100644 --- a/src/client.py +++ b/src/client.py @@ -142,10 +142,13 @@ def _enrich_library_config(self, input_config_path: str) -> str: config = configuration.configuration # Enrichment: BYOK RAG - enrich_byok_rag(ls_config, [b.model_dump() for b in config.byok_rag]) + enrich_byok_rag( + ls_config, + [b.model_dump() for b in config.rag.byok.stores], + ) # Enrichment: Solr - enabled when "okp" appears in either inline or tool list - enrich_solr(ls_config, config.rag.model_dump(), config.okp.model_dump()) + enrich_solr(ls_config, config.rag.model_dump(), config.rag.okp.model_dump()) # Enrichment: Azure Entra ID deferred auth entra_id_config = ( diff --git a/src/configuration.py b/src/configuration.py index e95e89083..89dd2dfce 100644 --- a/src/configuration.py +++ b/src/configuration.py @@ -18,6 +18,7 @@ AuthenticationConfiguration, AuthorizationConfiguration, AzureEntraIdConfiguration, + ByokRag, CompactionConfiguration, Configuration, ConversationHistoryConfiguration, @@ -534,10 +535,10 @@ def approvals_configuration(self) -> ApprovalsConfiguration: @property def okp(self) -> "OkpConfiguration": - """Return OKP configuration.""" + """Return OKP configuration from the unified rag section.""" if self._configuration is None: raise LogicError("logic error: configuration is not loaded") - return self._configuration.okp + return self._configuration.rag.okp @property def reranker(self) -> "RerankerConfiguration": @@ -553,6 +554,22 @@ def skills(self) -> Optional[SkillsConfiguration]: raise LogicError("logic error: configuration is not loaded") return self._configuration.skills + @property + def byok_rag(self) -> list[ByokRag]: + """Return BYOK RAG stores from the unified rag configuration. + + Backward-compatible property that returns ``rag.byok.stores``. + + Returns: + list[ByokRag]: The list of BYOK RAG store configurations. + + Raises: + LogicError: If the configuration has not been loaded. + """ + if self._configuration is None: + raise LogicError("logic error: configuration is not loaded") + return self._configuration.rag.byok.stores + @property def rag_id_mapping(self) -> dict[str, str]: """Return mapping from vector_db_id to rag_id from BYOK and OKP RAG config. @@ -567,12 +584,15 @@ def rag_id_mapping(self) -> dict[str, str]: if self._configuration is None: raise LogicError("logic error: configuration is not loaded") byok_mapping = { - brag.vector_db_id: brag.rag_id for brag in self._configuration.byok_rag + brag.vector_db_id: brag.rag_id + for brag in self._configuration.rag.byok.stores } rag = self._configuration.rag okp_id = constants.OKP_RAG_ID - okp_enabled = okp_id in (rag.inline or []) or okp_id in (rag.tool or []) + inline_sources = rag.retrieval.inline.sources + tool_sources = rag.retrieval.tool.sources + okp_enabled = okp_id in inline_sources or okp_id in tool_sources okp_mapping = ( {constants.SOLR_DEFAULT_VECTOR_STORE_ID: okp_id} if okp_enabled else {} ) @@ -593,7 +613,7 @@ def score_multiplier_mapping(self) -> dict[str, float]: raise LogicError("logic error: configuration is not loaded") return { brag.vector_db_id: brag.score_multiplier - for brag in self._configuration.byok_rag + for brag in self._configuration.rag.byok.stores } @property @@ -601,14 +621,15 @@ def inline_solr_enabled(self) -> bool: """Return whether OKP is included in the inline RAG list. Returns: - bool: True if 'okp' appears in rag.inline, False otherwise. + bool: True if 'okp' appears in rag.retrieval.inline.sources, + False otherwise. Raises: LogicError: If the configuration has not been loaded. """ if self._configuration is None: raise LogicError("logic error: configuration is not loaded") - return constants.OKP_RAG_ID in self._configuration.rag.inline + return constants.OKP_RAG_ID in self._configuration.rag.retrieval.inline.sources def resolve_index_name( self, vector_store_id: str, rag_id_mapping: Optional[dict[str, str]] = None diff --git a/src/constants.py b/src/constants.py index 91aa3cb83..ee9018b81 100644 --- a/src/constants.py +++ b/src/constants.py @@ -218,14 +218,19 @@ USER_QUOTA_LIMITER: Final[str] = "user_limiter" CLUSTER_QUOTA_LIMITER: Final[str] = "cluster_limiter" -# Hard cap on total RAG chunks delivered to the LLM across all sources +# Hard cap on total RAG chunks delivered to the LLM across all sources. +# Now user-configurable via rag.retrieval.inline.max_chunks; this constant +# serves as the default value. INLINE_RAG_MAX_CHUNKS: Final[int] = 10 # RAG as a tool constants DEFAULT_RAG_TOOL: Final[str] = "file_search" +# Now user-configurable via rag.retrieval.tool.max_chunks; this constant +# serves as the default value. TOOL_RAG_MAX_CHUNKS: Final[int] = 10 # retrieved from RAG as a tool -# Inline RAG constants +# Inline RAG constants — now user-configurable via rag.byok.max_chunks and +# rag.okp.max_chunks respectively; these constants serve as default values. BYOK_RAG_MAX_CHUNKS: Final[int] = 10 # retrieved from BYOK RAG OKP_RAG_MAX_CHUNKS: Final[int] = 5 # retrieved from OKP RAG # Score multiplier applied to BYOK chunks after cross-encoder reranking (Solr chunks unchanged) diff --git a/src/llama_stack_configuration.py b/src/llama_stack_configuration.py index 68f344b1a..04777848f 100644 --- a/src/llama_stack_configuration.py +++ b/src/llama_stack_configuration.py @@ -469,6 +469,80 @@ def construct_vector_io_providers_section( return output +def _extract_byok_rag_list(config: dict[str, Any]) -> list[dict[str, Any]]: + """Extract BYOK RAG store list from either new or old config format. + + Supports both the new ``rag.byok.stores`` path and the deprecated + top-level ``byok_rag`` key. + + Parameters: + config: Raw lightspeed-stack.yaml dict. + + Returns: + List of BYOK RAG store dicts (may be empty). + """ + # New format: rag.byok.stores + rag = config.get("rag") + if isinstance(rag, dict): + byok = rag.get("byok") + if isinstance(byok, dict): + stores = byok.get("stores") + if isinstance(stores, list) and stores: + return stores + # Old format: top-level byok_rag + return config.get("byok_rag") or [] + + +def _extract_okp_dict(config: dict[str, Any]) -> dict[str, Any]: + """Extract OKP configuration from either new or old config format. + + Supports both the new ``rag.okp`` path and the deprecated top-level + ``okp`` key. + + Parameters: + config: Raw lightspeed-stack.yaml dict. + + Returns: + OKP configuration dict (may be empty). + """ + # New format: rag.okp + rag = config.get("rag") + if isinstance(rag, dict): + okp = rag.get("okp") + if isinstance(okp, dict) and okp: + return okp + # Old format: top-level okp + return config.get("okp") or {} + + +def _extract_rag_source_ids( + rag_config: dict[str, Any], +) -> tuple[list[str], list[str]]: + """Extract inline and tool RAG source IDs from either config format. + + Supports both the new ``retrieval.inline.sources`` / ``retrieval.tool.sources`` + path and the deprecated ``inline`` / ``tool`` keys. + + Parameters: + rag_config: RAG configuration dict. + + Returns: + Tuple of (inline_ids, tool_ids). + """ + retrieval = rag_config.get("retrieval") + if isinstance(retrieval, dict): + inline_cfg = retrieval.get("inline") or {} + tool_cfg = retrieval.get("tool") or {} + inline_ids = ( + inline_cfg.get("sources") or [] if isinstance(inline_cfg, dict) else [] + ) + tool_ids = tool_cfg.get("sources") or [] if isinstance(tool_cfg, dict) else [] + else: + inline_ids = rag_config.get("inline") or [] + tool_ids = rag_config.get("tool") or [] + return inline_ids, tool_ids + + def enrich_byok_rag(ls_config: dict[str, Any], byok_rag: list[dict[str, Any]]) -> None: """Enrich Llama Stack config with BYOK RAG settings. @@ -783,8 +857,7 @@ def enrich_solr( # pylint: disable=too-many-locals - chunk_filter_query (str): Solr filter query for chunk retrieval - rhokp_url (str): OKP/Solr base URL (e.g. from ${env.RH_SERVER_OKP}) """ - inline_ids = rag_config.get("inline") or [] - tool_ids = rag_config.get("tool") or [] + inline_ids, tool_ids = _extract_rag_source_ids(rag_config) okp_enabled = constants.OKP_RAG_ID in inline_ids or constants.OKP_RAG_ID in tool_ids if not okp_enabled: @@ -1157,9 +1230,12 @@ def synthesize_configuration( # 4. Existing enrichment — same calls as legacy generate_configuration so # unified output matches legacy output for equivalent inputs (R7). enrich_azure_entra_id_inference(ls_config, lcs_config.get("azure_entra_id")) - enrich_byok_rag(ls_config, lcs_config.get("byok_rag", [])) + byok_rag_list = _extract_byok_rag_list(lcs_config) + enrich_byok_rag(ls_config, byok_rag_list) enrich_vector_store(ls_config, lcs_config.get("vector_store")) - enrich_solr(ls_config, lcs_config.get("rag", {}), lcs_config.get("okp", {})) + rag_dict = lcs_config.get("rag", {}) + okp_dict = _extract_okp_dict(lcs_config) + enrich_solr(ls_config, rag_dict, okp_dict) # 5. High-level inference providers (Decision S5 — a root-level section). inference = lcs_config.get("inference") or {} @@ -1328,10 +1404,13 @@ def generate_configuration( enrich_azure_entra_id_inference(ls_config, config.get("azure_entra_id")) # Enrichment: BYOK RAG - enrich_byok_rag(ls_config, config.get("byok_rag", [])) + byok_rag_list = _extract_byok_rag_list(config) + enrich_byok_rag(ls_config, byok_rag_list) # Enrichment: Solr - enabled when "okp" appears in either inline or tool list - enrich_solr(ls_config, config.get("rag", {}), config.get("okp", {})) + rag_dict = config.get("rag", {}) + okp_dict = _extract_okp_dict(config) + enrich_solr(ls_config, rag_dict, okp_dict) dedupe_providers_vector_io(ls_config) diff --git a/src/models/config.py b/src/models/config.py index 462b038f5..cba9d8b60 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -3,6 +3,7 @@ # pylint: disable=too-many-lines import re +import warnings from enum import Enum from functools import cached_property from pathlib import Path @@ -2026,7 +2027,17 @@ def config( class ByokRag(ConfigurationBase): - """BYOK (Bring Your Own Knowledge) RAG configuration.""" + """BYOK (Bring Your Own Knowledge) RAG configuration. + + Attributes: + rag_id: Unique RAG identifier. + backend: Short backend name (e.g. ``"faiss"``, ``"pgvector"``). + Preferred over the deprecated ``rag_type`` field. + rag_type: Deprecated — use ``backend`` instead. Full Llama Stack + provider type string (e.g. ``"inline::faiss"``). + """ + + model_config = ConfigDict(extra="forbid", populate_by_name=True) rag_id: str = Field( ..., @@ -2035,11 +2046,20 @@ class ByokRag(ConfigurationBase): description="Unique RAG ID", ) - rag_type: str = Field( - constants.DEFAULT_RAG_TYPE, + backend: Optional[str] = Field( + default=None, + min_length=1, + title="Backend", + description="Short backend name (e.g. 'faiss', 'pgvector'). " + "Preferred over the deprecated 'rag_type' field.", + ) + + rag_type: Optional[str] = Field( + default=None, min_length=1, - title="RAG type", - description="Type of RAG database (e.g. 'inline::faiss', 'remote::pgvector').", + title="RAG type (deprecated)", + description="Deprecated — use 'backend' instead. " + "Type of RAG database (e.g. 'inline::faiss', 'remote::pgvector').", ) embedding_model: str = Field( @@ -2112,6 +2132,53 @@ class ByokRag(ConfigurationBase): "Defaults to ${env.POSTGRES_PASSWORD} when rag_type is remote::pgvector.", ) + @model_validator(mode="after") + def reconcile_backend_and_rag_type(self) -> Self: + """Reconcile ``backend`` and ``rag_type``, emitting a deprecation warning. + + When only ``rag_type`` is provided, ``backend`` is derived by stripping + the ``inline::`` or ``remote::`` prefix and a deprecation warning is + emitted. When only ``backend`` is provided, ``rag_type`` is synthesized + using the canonical prefix mapping. When neither is provided, the + default backend ``"faiss"`` is used. When both are provided, they must + be consistent. + + Returns: + Self: The validated model instance. + + Raises: + ValueError: If both ``backend`` and ``rag_type`` are provided and + they are inconsistent. + """ + if self.rag_type is not None and self.backend is None: + warnings.warn( + "ByokRag field 'rag_type' is deprecated; use 'backend' instead " + f"(e.g. backend='{_strip_rag_type_prefix(self.rag_type)}')", + DeprecationWarning, + stacklevel=2, + ) + self.backend = _strip_rag_type_prefix(self.rag_type) + elif self.backend is not None and self.rag_type is None: + self.rag_type = _synthesize_rag_type(self.backend) + elif self.backend is not None and self.rag_type is not None: + expected_backend = _strip_rag_type_prefix(self.rag_type) + if self.backend != expected_backend: + raise ValueError( + f"'backend' ({self.backend}) and 'rag_type' " + f"({self.rag_type}) are inconsistent; expected " + f"backend='{expected_backend}'" + ) + warnings.warn( + "ByokRag field 'rag_type' is deprecated; use 'backend' instead", + DeprecationWarning, + stacklevel=2, + ) + else: + # Neither provided — use default + self.backend = "faiss" + self.rag_type = constants.DEFAULT_RAG_TYPE + return self + @model_validator(mode="after") def validate_rag_type_fields(self) -> Self: """Validate and populate fields based on rag_type.""" @@ -2132,6 +2199,39 @@ def validate_rag_type_fields(self) -> Self: return self +def _strip_rag_type_prefix(rag_type: str) -> str: + """Strip the ``inline::`` or ``remote::`` prefix from a rag_type string. + + Parameters: + rag_type: Full Llama Stack provider type (e.g. ``"inline::faiss"``). + + Returns: + The short backend name (e.g. ``"faiss"``). + """ + if "::" in rag_type: + return rag_type.split("::", 1)[1] + return rag_type + + +_BACKEND_TO_PREFIX: dict[str, str] = { + "faiss": "inline", + "pgvector": "remote", +} + + +def _synthesize_rag_type(backend: str) -> str: + """Synthesize a full ``rag_type`` from a short backend name. + + Parameters: + backend: Short backend name (e.g. ``"faiss"``). + + Returns: + Full Llama Stack provider type (e.g. ``"inline::faiss"``). + """ + prefix = _BACKEND_TO_PREFIX.get(backend, "inline") + return f"{prefix}::{backend}" + + class FaissVectorStoreProviderConfig(ConfigurationBase): """Storage config for a FAISS dynamic vector-store provider.""" @@ -2497,39 +2597,118 @@ class QuotaHandlersConfiguration(ConfigurationBase): ) -class RagConfiguration(ConfigurationBase): - """RAG strategy configuration. - - Controls which RAG sources are used for inline and tool-based retrieval. +class RetrievalInlineConfiguration(ConfigurationBase): + """Inline retrieval strategy configuration. - Each strategy lists RAG IDs to include. The special ID ``"okp"`` defined in constants, - activates the OKP provider; all other IDs refer to entries in ``byok_rag``. + Controls which RAG sources are injected as context before the LLM call. - Both ``inline`` and ``tool`` default to ``[]`` (disabled). - Each must be explicitly configured to activate its respective RAG strategy. + Attributes: + sources: RAG IDs whose chunks are injected inline. + max_chunks: Hard cap on total inline RAG chunks delivered to the LLM. """ - inline: list[str] = Field( + sources: list[str] = Field( default_factory=list, - title="Inline RAG IDs", + title="Inline RAG source IDs", description="RAG IDs whose sources are injected as context before the LLM call. " - f"Use '{constants.OKP_RAG_ID}' to enable OKP inline RAG. Empty by default (no inline RAG).", + f"Use '{constants.OKP_RAG_ID}' to enable OKP inline RAG. " + "Empty by default (no inline RAG).", + ) + + max_chunks: PositiveInt = Field( + default=constants.INLINE_RAG_MAX_CHUNKS, + title="Inline max chunks", + description="Hard cap on total RAG chunks delivered to the LLM across all " + f"inline sources. Default: {constants.INLINE_RAG_MAX_CHUNKS}.", ) - tool: list[str] = Field( + +class RetrievalToolConfiguration(ConfigurationBase): + """Tool-based retrieval strategy configuration. + + Controls which RAG sources are made available to the LLM as a + ``file_search`` tool. + + Attributes: + sources: RAG IDs made available as tool RAG. + max_chunks: Maximum number of chunks returned by the tool. + """ + + sources: list[str] = Field( default_factory=list, - title="Tool RAG IDs", + title="Tool RAG source IDs", description="RAG IDs made available to the LLM as a file_search tool. " f"Use '{constants.OKP_RAG_ID}' to include the OKP vector store. " "When omitted, tool RAG is disabled.", ) + max_chunks: PositiveInt = Field( + default=constants.TOOL_RAG_MAX_CHUNKS, + title="Tool max chunks", + description="Maximum number of chunks returned by the file_search tool. " + f"Default: {constants.TOOL_RAG_MAX_CHUNKS}.", + ) + + +class RetrievalConfiguration(ConfigurationBase): + """Retrieval strategy configuration. + + Groups inline and tool-based retrieval settings. + + Attributes: + inline: Inline retrieval strategy configuration. + tool: Tool-based retrieval strategy configuration. + """ + + inline: RetrievalInlineConfiguration = Field( + default_factory=RetrievalInlineConfiguration, + title="Inline retrieval", + description="Configuration for inline RAG (context injection before LLM call).", + ) + + tool: RetrievalToolConfiguration = Field( + default_factory=RetrievalToolConfiguration, + title="Tool retrieval", + description="Configuration for tool-based RAG (file_search tool).", + ) + + +class ByokConfiguration(ConfigurationBase): + """BYOK (Bring Your Own Knowledge) section under ``rag``. + + Groups BYOK stores and the per-store fetch limit. + + Attributes: + max_chunks: Per-store fetch limit for BYOK RAG queries. + stores: List of BYOK RAG store configurations. + """ + + max_chunks: PositiveInt = Field( + default=constants.BYOK_RAG_MAX_CHUNKS, + title="BYOK max chunks", + description="Per-store fetch limit for BYOK RAG queries. " + f"Default: {constants.BYOK_RAG_MAX_CHUNKS}.", + ) + + stores: list[ByokRag] = Field( + default_factory=list, + title="BYOK RAG stores", + description="List of BYOK RAG store configurations.", + ) + class OkpConfiguration(ConfigurationBase): """OKP (Offline Knowledge Portal) provider configuration. Controls provider-specific behaviour for the OKP vector store. - Only relevant when ``"okp"`` is listed in ``rag.inline`` or ``rag.tool``. + Only relevant when ``"okp"`` is listed in ``rag.retrieval.inline.sources`` + or ``rag.retrieval.tool.sources``. + + Attributes: + rhokp_url: Base URL for the OKP server. + offline: When True, use parent_id for OKP chunk source URLs. + chunk_filter_query: Additional filter query for OKP search requests. + max_chunks: Maximum number of chunks fetched from OKP. """ rhokp_url: Optional[AnyHttpUrl] = Field( @@ -2554,6 +2733,104 @@ class OkpConfiguration(ConfigurationBase): "Use Solr boolean syntax, e.g. 'product:ansible AND product:*openshift*'.", ) + max_chunks: PositiveInt = Field( + default=constants.OKP_RAG_MAX_CHUNKS, + title="OKP max chunks", + description="Maximum number of chunks fetched from OKP. " + f"Default: {constants.OKP_RAG_MAX_CHUNKS}.", + ) + + +class RagConfiguration(ConfigurationBase): + """Unified RAG configuration. + + Groups all RAG-related settings: BYOK stores, OKP provider, and + retrieval strategies (inline and tool-based). + + Also supports the deprecated top-level ``inline`` and ``tool`` fields + for backward compatibility; these are migrated to + ``retrieval.inline.sources`` and ``retrieval.tool.sources`` with a + deprecation warning. + + Attributes: + byok: BYOK store configuration and per-store fetch limit. + okp: OKP provider configuration. + retrieval: Retrieval strategy configuration (inline + tool). + inline: Deprecated — migrated to ``retrieval.inline.sources``. + tool: Deprecated — migrated to ``retrieval.tool.sources``. + """ + + byok: ByokConfiguration = Field( + default_factory=ByokConfiguration, + title="BYOK configuration", + description="BYOK RAG stores and per-store fetch limit.", + ) + + okp: OkpConfiguration = Field( + default_factory=OkpConfiguration, + title="OKP configuration", + description=f"OKP provider settings. Only used when '{constants.OKP_RAG_ID}' " + "is listed in retrieval.inline.sources or retrieval.tool.sources.", + ) + + retrieval: RetrievalConfiguration = Field( + default_factory=RetrievalConfiguration, + title="Retrieval configuration", + description="Retrieval strategy configuration (inline + tool).", + ) + + # --- Deprecated fields for backward compatibility --- + # exclude=True is intentional: deprecated fields are accepted during + # parsing for backward compatibility but must not appear in + # model_dump() / config-dump output so consumers only see the new + # canonical structure. + inline: Optional[list[str]] = Field( + default=None, + title="Inline RAG IDs (deprecated)", + description="Deprecated — use 'retrieval.inline.sources' instead.", + exclude=True, + ) + + # exclude=True is intentional: same rationale as ``inline`` above. + tool: Optional[list[str]] = Field( + default=None, + title="Tool RAG IDs (deprecated)", + description="Deprecated — use 'retrieval.tool.sources' instead.", + exclude=True, + ) + + @model_validator(mode="after") + def migrate_deprecated_fields(self) -> Self: + """Migrate deprecated ``inline`` and ``tool`` fields to ``retrieval``. + + When the deprecated ``inline`` or ``tool`` fields are provided, + their values are copied into ``retrieval.inline.sources`` and + ``retrieval.tool.sources`` respectively, and a deprecation warning + is emitted. + + Returns: + Self: The validated model instance. + """ + if self.inline is not None: + warnings.warn( + "RagConfiguration field 'rag.inline' is deprecated; " + "use 'rag.retrieval.inline.sources' instead", + DeprecationWarning, + stacklevel=2, + ) + self.retrieval.inline.sources = self.inline # pylint: disable=no-member + self.inline = None + if self.tool is not None: + warnings.warn( + "RagConfiguration field 'rag.tool' is deprecated; " + "use 'rag.retrieval.tool.sources' instead", + DeprecationWarning, + stacklevel=2, + ) + self.retrieval.tool.sources = self.tool # pylint: disable=no-member + self.tool = None + return self + class RerankerConfiguration(ConfigurationBase): """Reranker configuration for RAG chunk reranking.""" @@ -2883,11 +3160,11 @@ class Configuration(ConfigurationBase): description="Settings for human-in-the-loop approval of MCP tool invocations", ) - byok_rag: list[ByokRag] = Field( - default_factory=list, - title="BYOK RAG configuration", - description="BYOK RAG configuration. This configuration can be used to " - "reconfigure Llama Stack through its run.yaml configuration file", + byok_rag: Optional[list[ByokRag]] = Field( + default=None, + title="BYOK RAG configuration (deprecated)", + description="Deprecated — use 'rag.byok.stores' instead. " + "BYOK RAG configuration for backward compatibility.", ) vector_store: VectorStoreConfiguration = Field( @@ -2943,14 +3220,16 @@ class Configuration(ConfigurationBase): rag: RagConfiguration = Field( default_factory=RagConfiguration, title="RAG configuration", - description="Configuration for all RAG strategies (inline and tool-based).", + description="Unified RAG configuration: BYOK stores, OKP provider, " + "and retrieval strategies (inline + tool).", ) - okp: OkpConfiguration = Field( - default_factory=OkpConfiguration, - title="OKP configuration", - description=f"OKP provider settings. Only used when '{constants.OKP_RAG_ID}' is listed " - "in rag.inline or rag.tool.", + okp: Optional[OkpConfiguration] = Field( + default=None, + title="OKP configuration (deprecated)", + description="Deprecated — use 'rag.okp' instead. " + f"OKP provider settings. Only used when '{constants.OKP_RAG_ID}' is listed " + "in rag.retrieval.inline.sources or rag.retrieval.tool.sources.", ) reranker: RerankerConfiguration = Field( @@ -2972,6 +3251,48 @@ class Configuration(ConfigurationBase): "maximum prompts per user, display name length, and content length.", ) + @model_validator(mode="after") + def migrate_deprecated_rag_fields(self) -> Self: + """Migrate deprecated top-level ``byok_rag`` and ``okp`` into ``rag``. + + When the deprecated ``byok_rag`` list is provided at the top level, + its entries are moved into ``rag.byok.stores`` and a deprecation + warning is emitted. Similarly, a top-level ``okp`` section is merged + into ``rag.okp``. + + Returns: + Self: The validated configuration instance. + """ + if self.byok_rag is not None and len(self.byok_rag) > 0: + if self.rag.byok.stores: # pylint: disable=no-member + raise ValueError( + "Both deprecated top-level 'byok_rag' and 'rag.byok.stores' " + "are populated. Remove the top-level 'byok_rag' and use only " + "'rag.byok.stores'." + ) + warnings.warn( + "Top-level 'byok_rag' is deprecated; use 'rag.byok.stores' instead", + DeprecationWarning, + stacklevel=2, + ) + self.rag.byok.stores = list(self.byok_rag) # pylint: disable=no-member + + if self.okp is not None: + warnings.warn( + "Top-level 'okp' is deprecated; use 'rag.okp' instead", + DeprecationWarning, + stacklevel=2, + ) + # Merge top-level okp into rag.okp only if rag.okp is at defaults. + # Using model_fields_set ensures this check stays correct if + # OkpConfiguration gains new fields in the future. + # pylint: disable=no-member + rag_okp = self.rag.okp + rag_okp_is_default = not rag_okp.model_fields_set + if rag_okp_is_default: + self.rag.okp = self.okp + return self + @model_validator(mode="after") def validate_mcp_auth_headers(self) -> Self: """ @@ -3078,7 +3399,7 @@ def validate_rlsapi_v1_quota_configuration(self) -> Self: def validate_reranker_auto_enable(self) -> Self: """Automatically enable reranker when both BYOK and OKP RAG are configured. - When users have both BYOK entries in byok_rag and OKP + When users have both BYOK entries in rag.byok.stores and OKP configured in the RAG strategies, automatically enable the reranker if it's not explicitly disabled. This improves result quality when multiple knowledge sources are available. @@ -3087,11 +3408,11 @@ def validate_reranker_auto_enable(self) -> Self: Self: The validated configuration instance with reranker potentially enabled. """ # Check if BYOK RAG entries are configured - has_byok = len(self.byok_rag) > 0 + has_byok = len(self.rag.byok.stores) > 0 # pylint: disable=no-member # Check if OKP is configured in either inline or tool RAG strategies # pylint: disable=no-member - has_okp = constants.OKP_RAG_ID in self.rag.inline + has_okp = constants.OKP_RAG_ID in self.rag.retrieval.inline.sources # If both BYOK and OKP are present and reranker is using default settings, # ensure it's enabled for optimal results @@ -3105,7 +3426,7 @@ def validate_reranker_auto_enable(self) -> Self: "Automatically enabling reranker: Both BYOK RAG (%d entries) or " "other inline RAG and OKP are configured. Reranking improves result " "quality when multiple knowledge sources are available.", - len(self.byok_rag), + len(self.rag.byok.stores), ) self.reranker.enabled = True diff --git a/src/utils/responses.py b/src/utils/responses.py index 2beff8990..7b712dc9a 100644 --- a/src/utils/responses.py +++ b/src/utils/responses.py @@ -217,17 +217,16 @@ async def prepare_tools( # pylint: disable=too-many-arguments,too-many-position # Vector store ID resolution priority: # 1. Per-request IDs: highest prio; customer-facing rag_ids are translated to vector_db_ids. - # 2. rag.tool config IDs: used when no per-request IDs provided, and rag.tool is configured. - byok_rags = configuration.configuration.byok_rag + # 2. rag.retrieval.tool.sources config IDs: used when no per-request IDs provided. + byok_rags = configuration.byok_rag - is_tool_rag_enabled = len(configuration.configuration.rag.tool) > 0 + tool_sources = configuration.configuration.rag.retrieval.tool.sources + is_tool_rag_enabled = len(tool_sources) > 0 if vector_store_ids is not None: effective_ids = resolve_vector_store_ids(vector_store_ids, byok_rags) elif is_tool_rag_enabled: - effective_ids = resolve_vector_store_ids( - configuration.configuration.rag.tool, byok_rags - ) + effective_ids = resolve_vector_store_ids(tool_sources, byok_rags) # Add RAG tools if vector stores are available rag_tools = get_rag_tools(effective_ids) @@ -665,7 +664,7 @@ def get_rag_tools(vector_store_ids: list[str]) -> Optional[list[InputToolFileSea InputToolFileSearch( type="file_search", vector_store_ids=vector_store_ids, - max_num_results=constants.TOOL_RAG_MAX_CHUNKS, + max_num_results=configuration.configuration.rag.retrieval.tool.max_chunks, ) ] @@ -1698,7 +1697,7 @@ async def _resolve_client_tools( # Per-request override of vector stores (user-facing rag_ids) vector_store_ids = extract_vector_store_ids_from_tools(tools) or None # Translate user-facing rag_ids to llama-stack vector_store_ids in each file_search tool - byok_rags = configuration.configuration.byok_rag + byok_rags = configuration.byok_rag prepared_tools = translate_tools_vector_store_ids(tools, byok_rags) prepared_tools = apply_mcp_headers_to_explicit_tools( prepared_tools, token, mcp_headers, request_headers @@ -1787,7 +1786,7 @@ async def resolve_tool_choice( ) else: # Pass tools explicitly configured for this request - byok_rags = configuration.configuration.byok_rag + byok_rags = configuration.byok_rag prepared_tools = translate_tools_vector_store_ids(tools, byok_rags) prepared_tools = apply_mcp_headers_to_explicit_tools( prepared_tools, token, mcp_headers, request_headers diff --git a/src/utils/vector_search.py b/src/utils/vector_search.py index 2e0fd3dea..675d2adba 100644 --- a/src/utils/vector_search.py +++ b/src/utils/vector_search.py @@ -454,33 +454,35 @@ async def _fetch_byok_rag( # pylint: disable=too-many-locals If provided, only these stores will be queried. If None, all stores (excluding Solr) will be queried. max_chunks: Maximum number of chunks to return. If None, uses - constants.BYOK_RAG_MAX_CHUNKS. + the configured rag.byok.max_chunks value. Returns: Tuple containing: - rag_chunks: RAG chunks from BYOK RAG - referenced_documents: Documents referenced in BYOK RAG results """ - limit = max_chunks if max_chunks is not None else constants.BYOK_RAG_MAX_CHUNKS + limit = ( + max_chunks + if max_chunks is not None + else configuration.configuration.rag.byok.max_chunks + ) rag_chunks: list[RAGChunk] = [] referenced_documents: list[ReferencedDocument] = [] # Determine which BYOK vector stores to query for inline RAG. - # Config is the source of truth: only rag_ids registered in rag.inline are eligible. + # Config is the source of truth: only rag_ids registered in + # rag.retrieval.inline.sources are eligible. # Per-request IDs are intersected with the config to prevent triggering inline RAG # for stores not explicitly configured for inline use. + inline_sources = configuration.configuration.rag.retrieval.inline.sources if vector_store_ids is None: - rag_ids_to_query = configuration.configuration.rag.inline + rag_ids_to_query = inline_sources else: - rag_ids_to_query = [ - v - for v in vector_store_ids - if v in set(configuration.configuration.rag.inline) - ] + rag_ids_to_query = [v for v in vector_store_ids if v in set(inline_sources)] # Translate user-facing rag_ids to llama-stack ids vector_store_ids_to_query: list[str] = resolve_vector_store_ids( - rag_ids_to_query, configuration.configuration.byok_rag + rag_ids_to_query, configuration.byok_rag ) # Request-level override: filter out Solr store, use the rest @@ -561,8 +563,6 @@ async def _fetch_solr_rag( # pylint: disable=too-many-locals client: The AsyncLlamaStackClient to use for the request query: The user's query solr: Structured Solr inline RAG request from the API (optional). - max_chunks: Maximum number of chunks to return. If None, uses - constants.OKP_RAG_MAX_CHUNKS. Returns: Tuple containing: @@ -571,7 +571,7 @@ async def _fetch_solr_rag( # pylint: disable=too-many-locals """ rag_chunks: list[RAGChunk] = [] referenced_documents: list[ReferencedDocument] = [] - limit = constants.OKP_RAG_MAX_CHUNKS + limit = configuration.configuration.rag.okp.max_chunks if not _is_solr_enabled(): logger.info("OKP vector IO is disabled, skipping OKP search") @@ -640,8 +640,8 @@ async def build_rag_context( # pylint: disable=too-many-locals,too-many-branche Each source fetches using its per-source limit to build the reranking pool. Results are merged, sorted by score, reranked with a cross-encoder if - enabled, then capped at INLINE_RAG_MAX_CHUNKS. Enabled sources can be BYOK - and/or Solr OKP. + enabled, then capped at the configured ``rag.retrieval.inline.max_chunks``. + Enabled sources can be BYOK and/or Solr OKP. Args: client: The AsyncLlamaStackClient to use for the request @@ -655,11 +655,14 @@ async def build_rag_context( # pylint: disable=too-many-locals,too-many-branche if moderation_decision == "blocked": return RAGContext() - top_k = constants.INLINE_RAG_MAX_CHUNKS + top_k = configuration.configuration.rag.retrieval.inline.max_chunks # Fetch from each source using per-source limits for the reranking pool byok_chunks_task = _fetch_byok_rag( - client, query, vector_store_ids, max_chunks=constants.BYOK_RAG_MAX_CHUNKS + client, + query, + vector_store_ids, + max_chunks=configuration.configuration.rag.byok.max_chunks, ) solr_chunks_task = _fetch_solr_rag(client, query, solr) diff --git a/tests/unit/models/config/test_byok_rag.py b/tests/unit/models/config/test_byok_rag.py index 1ac098299..2979763d8 100644 --- a/tests/unit/models/config/test_byok_rag.py +++ b/tests/unit/models/config/test_byok_rag.py @@ -1,5 +1,7 @@ """Unit tests for ByokRag model.""" +import warnings + import pytest from pydantic import ValidationError @@ -9,261 +11,351 @@ DEFAULT_RAG_TYPE, DEFAULT_SCORE_MULTIPLIER, ) -from models.config import ByokRag - - -def test_byok_rag_configuration_default_values() -> None: - """Test the ByokRag constructor. - - Verify that ByokRag initializes correctly when only required fields are provided. - - Asserts that the instance stores the given `rag_id`, `vector_db_id`, and - `db_path`, and that unspecified fields use the module's default values for - `rag_type`, `embedding_model`, `embedding_dimension`, and - `score_multiplier`. - """ - byok_rag = ByokRag( # pyright: ignore[reportCallIssue] - rag_id="rag_id", - vector_db_id="vector_db_id", - db_path="tests/configuration/rag.txt", - ) - assert byok_rag is not None - assert byok_rag.rag_id == "rag_id" - assert byok_rag.rag_type == DEFAULT_RAG_TYPE - assert byok_rag.embedding_model == DEFAULT_EMBEDDING_MODEL - assert byok_rag.embedding_dimension == DEFAULT_EMBEDDING_DIMENSION - assert byok_rag.vector_db_id == "vector_db_id" - assert byok_rag.db_path == "tests/configuration/rag.txt" - assert byok_rag.score_multiplier == DEFAULT_SCORE_MULTIPLIER - - -def test_byok_rag_configuration_nondefault_values() -> None: - """Test the ByokRag constructor. - - Verify that ByokRag class accepts and stores non-default configuration values. - - Asserts that rag_id, rag_type, embedding_model, embedding_dimension, and - vector_db_id match the provided inputs and that db_path is converted to a - Path. - """ - byok_rag = ByokRag( - rag_id="rag_id", - rag_type="rag_type", - embedding_model="embedding_model", - embedding_dimension=1024, - vector_db_id="vector_db_id", - db_path="tests/configuration/rag.txt", - score_multiplier=1.0, - ) - assert byok_rag is not None - assert byok_rag.rag_id == "rag_id" - assert byok_rag.rag_type == "rag_type" - assert byok_rag.embedding_model == "embedding_model" - assert byok_rag.embedding_dimension == 1024 - assert byok_rag.vector_db_id == "vector_db_id" - assert byok_rag.db_path == "tests/configuration/rag.txt" - - -def test_byok_rag_configuration_wrong_dimension() -> None: - """Test the ByokRag constructor. - - Verify constructing ByokRag with embedding_dimension less than or equal to - zero raises a ValidationError. - - The raised ValidationError's message must contain "should be greater than 0". - """ - with pytest.raises(ValidationError, match="should be greater than 0"): - _ = ByokRag( - rag_id="rag_id", - rag_type="rag_type", - embedding_model="embedding_model", - embedding_dimension=-1024, - vector_db_id="vector_db_id", - db_path="tests/configuration/rag.txt", - score_multiplier=1.0, - ) - +from models.config import ByokRag, _strip_rag_type_prefix, _synthesize_rag_type -def test_byok_rag_configuration_empty_rag_id() -> None: - """Test the ByokRag constructor. - Validate that constructing a ByokRag with an empty `rag_id` raises a validation error. +class TestByokRagBackendField: + """Tests for the new 'backend' field and rag_type deprecation.""" - Expects a `pydantic.ValidationError` whose message contains "String should - have at least 1 character". - """ - with pytest.raises( - ValidationError, match="String should have at least 1 character" - ): - _ = ByokRag( - rag_id="", - rag_type="rag_type", - embedding_model="embedding_model", - embedding_dimension=1024, - vector_db_id="vector_db_id", - db_path="tests/configuration/rag.txt", - score_multiplier=1.0, + def test_backend_only_faiss(self) -> None: + """Test that providing only 'backend' synthesizes rag_type.""" + store = ByokRag( + rag_id="test", + backend="faiss", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", ) - - -def test_byok_rag_configuration_empty_rag_type() -> None: - """Test the ByokRag constructor. - - Verify that constructing a ByokRag with an empty `rag_type` raises a validation error. - - Raises: - ValidationError: if `rag_type` is an empty string; error message - includes "String should have at least 1 character". - """ - with pytest.raises( - ValidationError, match="String should have at least 1 character" - ): - _ = ByokRag( - rag_id="rag_id", - rag_type="", - embedding_model="embedding_model", - embedding_dimension=1024, - vector_db_id="vector_db_id", - db_path="tests/configuration/rag.txt", - score_multiplier=1.0, + assert store.backend == "faiss" + assert store.rag_type == "inline::faiss" + + def test_backend_only_pgvector(self) -> None: + """Test that providing only 'backend=pgvector' synthesizes rag_type.""" + store = ByokRag( + rag_id="test", + backend="pgvector", + vector_db_id="vs_123", ) + assert store.backend == "pgvector" + assert store.rag_type == "remote::pgvector" + + def test_backend_unknown_type(self) -> None: + """Test that an unknown backend gets 'inline::' prefix by default.""" + store = ByokRag( + rag_id="test", + backend="chromadb", + vector_db_id="vs_123", + ) + assert store.backend == "chromadb" + assert store.rag_type == "inline::chromadb" + + def test_rag_type_only_emits_deprecation(self) -> None: + """Test that providing only 'rag_type' emits a deprecation warning.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + store = ByokRag( + rag_id="test", + rag_type="inline::faiss", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", + ) + assert store.backend == "faiss" + assert store.rag_type == "inline::faiss" + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) >= 1 + assert "deprecated" in str(deprecation_warnings[0].message).lower() + + def test_rag_type_only_pgvector(self) -> None: + """Test that providing only 'rag_type=remote::pgvector' derives backend.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + store = ByokRag( + rag_id="test", + rag_type="remote::pgvector", + vector_db_id="vs_123", + ) + assert store.backend == "pgvector" + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) >= 1 + + def test_both_backend_and_rag_type_consistent(self) -> None: + """Test that providing both consistent backend and rag_type works.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + store = ByokRag( + rag_id="test", + backend="faiss", + rag_type="inline::faiss", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", + ) + assert store.backend == "faiss" + assert store.rag_type == "inline::faiss" + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) >= 1 + + def test_both_backend_and_rag_type_inconsistent(self) -> None: + """Test that providing inconsistent backend and rag_type raises error.""" + with pytest.raises(ValidationError, match="inconsistent"): + ByokRag( + rag_id="test", + backend="pgvector", + rag_type="inline::faiss", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", + ) + + def test_neither_backend_nor_rag_type_uses_default(self) -> None: + """Test that providing neither backend nor rag_type uses default faiss.""" + store = ByokRag( + rag_id="test", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", + ) + assert store.backend == "faiss" + assert store.rag_type == DEFAULT_RAG_TYPE -def test_byok_rag_configuration_empty_embedding_model() -> None: - """Test the ByokRag constructor. - - Verify that constructing a ByokRag with an empty `embedding_model` raises a validation error. +class TestByokRagDefaults: + """Tests for ByokRag default values and basic construction.""" - Expects a pydantic.ValidationError whose message contains "String should - have at least 1 character". - """ - with pytest.raises( - ValidationError, match="String should have at least 1 character" - ): - _ = ByokRag( + def test_byok_rag_configuration_default_values(self) -> None: + """Test ByokRag initializes correctly with only required fields.""" + byok_rag = ByokRag( rag_id="rag_id", - rag_type="rag_type", - embedding_model="", - embedding_dimension=1024, vector_db_id="vector_db_id", db_path="tests/configuration/rag.txt", - score_multiplier=1.0, ) - - -def test_byok_rag_configuration_empty_vector_db_id() -> None: - """Test the ByokRag constructor. - - Ensure constructing a ByokRag with an empty `vector_db_id` raises a ValidationError. - - Asserts that Pydantic validation fails with a message containing "String - should have at least 1 character". - """ - with pytest.raises( - ValidationError, match="String should have at least 1 character" - ): - _ = ByokRag( + assert byok_rag is not None + assert byok_rag.rag_id == "rag_id" + assert byok_rag.rag_type == DEFAULT_RAG_TYPE + assert byok_rag.backend == "faiss" + assert byok_rag.embedding_model == DEFAULT_EMBEDDING_MODEL + assert byok_rag.embedding_dimension == DEFAULT_EMBEDDING_DIMENSION + assert byok_rag.vector_db_id == "vector_db_id" + assert byok_rag.db_path == "tests/configuration/rag.txt" + assert byok_rag.score_multiplier == DEFAULT_SCORE_MULTIPLIER + + def test_byok_rag_configuration_nondefault_values(self) -> None: + """Test ByokRag accepts and stores non-default configuration values.""" + byok_rag = ByokRag( rag_id="rag_id", - rag_type="rag_type", + backend="custom_type", embedding_model="embedding_model", embedding_dimension=1024, - vector_db_id="", + vector_db_id="vector_db_id", db_path="tests/configuration/rag.txt", score_multiplier=1.0, ) - - -def test_byok_rag_configuration_custom_score_multiplier() -> None: - """Test ByokRag with custom score_multiplier.""" - byok_rag = ByokRag( - rag_id="rag_id", - rag_type="rag_type", - vector_db_id="vector_db_id", - embedding_model="embedding_model", - embedding_dimension=1024, - db_path="tests/configuration/rag.txt", - score_multiplier=2.5, - ) - assert byok_rag.score_multiplier == 2.5 - - -def test_byok_rag_configuration_score_multiplier_must_be_positive() -> None: - """Test that score_multiplier must be greater than 0.""" - with pytest.raises(ValidationError, match="greater than 0"): - _ = ByokRag( + assert byok_rag is not None + assert byok_rag.rag_id == "rag_id" + assert byok_rag.backend == "custom_type" + assert byok_rag.rag_type == "inline::custom_type" + assert byok_rag.embedding_model == "embedding_model" + assert byok_rag.embedding_dimension == 1024 + assert byok_rag.vector_db_id == "vector_db_id" + assert byok_rag.db_path == "tests/configuration/rag.txt" + + +class TestByokRagValidation: + """Tests for ByokRag validation.""" + + def test_byok_rag_configuration_wrong_dimension(self) -> None: + """Test that embedding_dimension <= 0 raises ValidationError.""" + with pytest.raises(ValidationError, match="should be greater than 0"): + _ = ByokRag( + rag_id="rag_id", + backend="custom", + embedding_model="embedding_model", + embedding_dimension=-1024, + vector_db_id="vector_db_id", + db_path="tests/configuration/rag.txt", + score_multiplier=1.0, + ) + + def test_byok_rag_configuration_empty_rag_id(self) -> None: + """Test that empty rag_id raises ValidationError.""" + with pytest.raises( + ValidationError, match="String should have at least 1 character" + ): + _ = ByokRag( + rag_id="", + backend="custom", + embedding_model="embedding_model", + embedding_dimension=1024, + vector_db_id="vector_db_id", + db_path="tests/configuration/rag.txt", + score_multiplier=1.0, + ) + + def test_byok_rag_configuration_empty_embedding_model(self) -> None: + """Test that empty embedding_model raises ValidationError.""" + with pytest.raises( + ValidationError, match="String should have at least 1 character" + ): + _ = ByokRag( + rag_id="rag_id", + backend="custom", + embedding_model="", + embedding_dimension=1024, + vector_db_id="vector_db_id", + db_path="tests/configuration/rag.txt", + score_multiplier=1.0, + ) + + def test_byok_rag_configuration_empty_vector_db_id(self) -> None: + """Test that empty vector_db_id raises ValidationError.""" + with pytest.raises( + ValidationError, match="String should have at least 1 character" + ): + _ = ByokRag( + rag_id="rag_id", + backend="custom", + embedding_model="embedding_model", + embedding_dimension=1024, + vector_db_id="", + db_path="tests/configuration/rag.txt", + score_multiplier=1.0, + ) + + def test_byok_rag_configuration_custom_score_multiplier(self) -> None: + """Test ByokRag with custom score_multiplier.""" + byok_rag = ByokRag( rag_id="rag_id", - rag_type="rag_type", + backend="custom", vector_db_id="vector_db_id", embedding_model="embedding_model", embedding_dimension=1024, db_path="tests/configuration/rag.txt", - score_multiplier=0.0, + score_multiplier=2.5, ) - - -def test_byok_rag_faiss_requires_db_path() -> None: - """Test that inline::faiss requires db_path.""" - with pytest.raises(ValidationError, match="db_path is required"): - _ = ByokRag( - rag_id="rag_id", - rag_type="inline::faiss", - vector_db_id="vector_db_id", + assert byok_rag.score_multiplier == 2.5 + + def test_byok_rag_configuration_score_multiplier_must_be_positive(self) -> None: + """Test that score_multiplier must be greater than 0.""" + with pytest.raises(ValidationError, match="greater than 0"): + _ = ByokRag( + rag_id="rag_id", + backend="custom", + vector_db_id="vector_db_id", + embedding_model="embedding_model", + embedding_dimension=1024, + db_path="tests/configuration/rag.txt", + score_multiplier=0.0, + ) + + def test_byok_rag_faiss_requires_db_path(self) -> None: + """Test that inline::faiss requires db_path.""" + with pytest.raises(ValidationError, match="db_path is required"): + _ = ByokRag( + rag_id="rag_id", + backend="faiss", + vector_db_id="vector_db_id", + ) + + def test_byok_rag_pgvector_defaults(self) -> None: + """Test pgvector auto-populates connection fields with env var defaults.""" + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + store = ByokRag( + rag_id="pg_store", + backend="pgvector", + vector_db_id="vs_pg", + ) + assert store.rag_type == "remote::pgvector" + assert store.host == "${env.POSTGRES_HOST}" + assert store.port == "${env.POSTGRES_PORT}" + assert store.db == "${env.POSTGRES_DATABASE}" + assert store.user == "${env.POSTGRES_USER}" + password = store.password.get_secret_value() # pylint: disable=no-member + assert password == "${env.POSTGRES_PASSWORD}" + assert store.db_path is None + + def test_byok_rag_pgvector_custom_connection_fields(self) -> None: + """Test pgvector accepts custom connection field values.""" + store = ByokRag( + rag_id="pg_store", + backend="pgvector", + vector_db_id="vs_pg", + host="db.example.com", + port="5433", + db="my_knowledge", + user="admin", + password="secret", ) - - -def test_byok_rag_pgvector_defaults() -> None: - """Test pgvector auto-populates connection fields with env var defaults.""" - store = ByokRag( - rag_id="pg_store", - rag_type="remote::pgvector", - vector_db_id="vs_pg", - ) - assert store.rag_type == "remote::pgvector" - assert store.host == "${env.POSTGRES_HOST}" - assert store.port == "${env.POSTGRES_PORT}" - assert store.db == "${env.POSTGRES_DATABASE}" - assert store.user == "${env.POSTGRES_USER}" - password = store.password.get_secret_value() # pylint: disable=no-member - assert password == "${env.POSTGRES_PASSWORD}" - assert store.db_path is None - - -def test_byok_rag_pgvector_custom_connection_fields() -> None: - """Test pgvector accepts custom connection field values.""" - store = ByokRag( - rag_id="pg_store", - rag_type="remote::pgvector", - vector_db_id="vs_pg", - host="db.example.com", - port="5433", - db="my_knowledge", - user="admin", - password="secret", - ) - assert store.host == "db.example.com" - assert store.port == "5433" - assert store.db == "my_knowledge" - assert store.user == "admin" - assert store.password.get_secret_value() == "secret" # pylint: disable=no-member - - -def test_byok_rag_pgvector_partial_overrides() -> None: - """Test pgvector fills only missing connection fields with defaults.""" - store = ByokRag( - rag_id="pg_store", - rag_type="remote::pgvector", - vector_db_id="vs_pg", - host="custom-host", - ) - assert store.host == "custom-host" - assert store.port == "${env.POSTGRES_PORT}" - - -def test_byok_rag_pgvector_does_not_require_db_path() -> None: - """Test pgvector does not require db_path.""" - store = ByokRag( - rag_id="pg_store", - rag_type="remote::pgvector", - vector_db_id="vs_pg", - ) - assert store.db_path is None + assert store.host == "db.example.com" + assert store.port == "5433" + assert store.db == "my_knowledge" + assert store.user == "admin" + password = store.password.get_secret_value() # pylint: disable=no-member + assert password == "secret" + + def test_byok_rag_pgvector_partial_overrides(self) -> None: + """Test pgvector fills only missing connection fields with defaults.""" + store = ByokRag( + rag_id="pg_store", + backend="pgvector", + vector_db_id="vs_pg", + host="custom-host", + ) + assert store.host == "custom-host" + assert store.port == "${env.POSTGRES_PORT}" + + def test_byok_rag_pgvector_does_not_require_db_path(self) -> None: + """Test pgvector does not require db_path.""" + store = ByokRag( + rag_id="pg_store", + backend="pgvector", + vector_db_id="vs_pg", + ) + assert store.db_path is None + + def test_byok_rag_legacy_rag_type_faiss(self) -> None: + """Test backward compatibility: rag_type='inline::faiss' still works.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + store = ByokRag( # pyright: ignore[reportCallIssue] + rag_id="rag_id", + rag_type="inline::faiss", + vector_db_id="vector_db_id", + db_path="tests/configuration/rag.txt", + ) + assert store.rag_type == "inline::faiss" + assert store.backend == "faiss" + deprecation_warnings = [ + x for x in w if issubclass(x.category, DeprecationWarning) + ] + assert len(deprecation_warnings) >= 1 + + +class TestHelperFunctions: + """Tests for _strip_rag_type_prefix and _synthesize_rag_type.""" + + def test_strip_inline_prefix(self) -> None: + """Test stripping inline:: prefix.""" + assert _strip_rag_type_prefix("inline::faiss") == "faiss" + + def test_strip_remote_prefix(self) -> None: + """Test stripping remote:: prefix.""" + assert _strip_rag_type_prefix("remote::pgvector") == "pgvector" + + def test_strip_no_prefix(self) -> None: + """Test stripping when no prefix present.""" + assert _strip_rag_type_prefix("faiss") == "faiss" + + def test_synthesize_faiss(self) -> None: + """Test synthesizing rag_type for faiss backend.""" + assert _synthesize_rag_type("faiss") == "inline::faiss" + + def test_synthesize_pgvector(self) -> None: + """Test synthesizing rag_type for pgvector backend.""" + assert _synthesize_rag_type("pgvector") == "remote::pgvector" + + def test_synthesize_unknown(self) -> None: + """Test synthesizing rag_type for unknown backend defaults to inline.""" + assert _synthesize_rag_type("chromadb") == "inline::chromadb" diff --git a/tests/unit/models/config/test_dump_configuration.py b/tests/unit/models/config/test_dump_configuration.py index 5aae2db4b..399c653ab 100644 --- a/tests/unit/models/config/test_dump_configuration.py +++ b/tests/unit/models/config/test_dump_configuration.py @@ -38,6 +38,29 @@ "approval_retention_days": 30, } +_DEFAULT_RAG_DUMP: dict[str, Any] = { + "byok": { + "max_chunks": 10, + "stores": [], + }, + "okp": { + "rhokp_url": None, + "offline": True, + "chunk_filter_query": None, + "max_chunks": 5, + }, + "retrieval": { + "inline": { + "sources": [], + "max_chunks": 10, + }, + "tool": { + "sources": [], + "max_chunks": 10, + }, + }, +} + _DEFAULT_SAVED_PROMPTS_DUMP: dict[str, int] = { "max_prompts_per_user": constants.SAVED_PROMPTS_DEFAULT_MAX_PER_USER, "max_display_name_length": constants.SAVED_PROMPTS_DEFAULT_MAX_DISPLAY_NAME_LENGTH, @@ -102,7 +125,6 @@ def test_dump_configuration_minimal_cfg(tmp_path: Path) -> None: assert "customization" in content assert "inference" in content assert "database" in content - assert "byok_rag" in content assert "quota_handlers" in content assert "azure_entra_id" in content assert "reranker" in content @@ -197,7 +219,7 @@ def test_dump_configuration_minimal_cfg(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -218,15 +240,8 @@ def test_dump_configuration_minimal_cfg(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -424,7 +439,7 @@ def test_dump_configuration_valid_values(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -445,15 +460,8 @@ def test_dump_configuration_valid_values(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -787,7 +795,7 @@ def test_dump_configuration_with_quota_limiters(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -823,15 +831,8 @@ def test_dump_configuration_with_quota_limiters(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -1049,7 +1050,7 @@ def test_dump_configuration_with_quota_limiters_different_values( "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -1085,15 +1086,8 @@ def test_dump_configuration_with_quota_limiters_different_values( "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -1350,6 +1344,7 @@ def test_dump_configuration_byok(tmp_path: Path) -> None: "embedding_dimension": 768, "embedding_model": "sentence-transformers/all-mpnet-base-v2", "rag_id": "rag_id", + "backend": "faiss", "rag_type": "inline::faiss", "vector_db_id": "vector_db_id", "score_multiplier": 1.0, @@ -1381,14 +1376,44 @@ def test_dump_configuration_byok(tmp_path: Path) -> None: }, "azure_entra_id": None, "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, + "byok": { + "max_chunks": 10, + "stores": [ + { + "db_path": "tests/configuration/rag.txt", + "embedding_dimension": 768, + "embedding_model": "sentence-transformers/all-mpnet-base-v2", + "rag_id": "rag_id", + "backend": "faiss", + "rag_type": "inline::faiss", + "vector_db_id": "vector_db_id", + "score_multiplier": 1.0, + "host": None, + "port": None, + "db": None, + "user": None, + "password": None, + }, + ], + }, + "okp": { + "rhokp_url": None, + "offline": True, + "chunk_filter_query": None, + "max_chunks": 5, + }, + "retrieval": { + "inline": { + "sources": [], + "max_chunks": 10, + }, + "tool": { + "sources": [], + "max_chunks": 10, + }, + }, }, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -1581,7 +1606,7 @@ def test_dump_configuration_pg_namespace(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -1602,15 +1627,8 @@ def test_dump_configuration_pg_namespace(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -1963,7 +1981,7 @@ def test_dump_configuration_allow_degraded_mode(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -1984,15 +2002,8 @@ def test_dump_configuration_allow_degraded_mode(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -2191,7 +2202,7 @@ def test_dump_configuration_max_retries_settings(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -2212,15 +2223,8 @@ def test_dump_configuration_max_retries_settings(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -2419,7 +2423,7 @@ def test_dump_configuration_retry_count_settings(tmp_path: Path) -> None: "buffer_max_ratio": 0.3, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -2440,15 +2444,8 @@ def test_dump_configuration_retry_count_settings(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, @@ -2654,7 +2651,7 @@ def test_dump_configuration_specific_compaction_values(tmp_path: Path) -> None: "buffer_max_ratio": 0.5, }, "approvals": _DEFAULT_APPROVALS_DUMP, - "byok_rag": [], + "byok_rag": None, "vector_store": { "default_provider": None, "providers": [], @@ -2675,15 +2672,8 @@ def test_dump_configuration_specific_compaction_values(tmp_path: Path) -> None: "postgres": None, }, "azure_entra_id": None, - "rag": { - "inline": [], - "tool": [], - }, - "okp": { - "rhokp_url": None, - "offline": True, - "chunk_filter_query": None, - }, + "rag": _DEFAULT_RAG_DUMP, + "okp": None, "rlsapi_v1": { "allow_verbose_infer": False, "quota_subject": None, diff --git a/tests/unit/models/config/test_rag_config_refactoring.py b/tests/unit/models/config/test_rag_config_refactoring.py new file mode 100644 index 000000000..5b2c8a344 --- /dev/null +++ b/tests/unit/models/config/test_rag_config_refactoring.py @@ -0,0 +1,1213 @@ +"""Functional tests for LCORE-1426: BYOK Config Refactoring. + +Tests verify that all RAG-related configuration is unified under a single +``rag`` section, that old config formats are backward-compatible with +deprecation warnings, and that max-chunk constants are user-configurable. +""" + +# pylint: disable=no-member + +from collections.abc import Generator +from pathlib import Path + +import pytest + +from configuration import AppConfig + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _reset_app_config() -> Generator[None, None, None]: + """Reset AppConfig singleton between tests.""" + # pylint: disable=broad-exception-caught,protected-access + try: + AppConfig()._configuration = None # type: ignore[attr-defined] + AppConfig()._quota_limiters = [] # type: ignore[attr-defined] + except Exception: + pass + yield + try: + AppConfig()._configuration = None # type: ignore[attr-defined] + AppConfig()._quota_limiters = [] # type: ignore[attr-defined] + except Exception: + pass + + +def _base_config() -> dict: + """Return the minimal required config sections (non-RAG).""" + return { + "name": "test", + "service": {"host": "localhost", "port": 8080}, + "llama_stack": { + "api_key": "test-key", + "url": "http://test.com:1234", + "use_as_library_client": False, + }, + "user_data_collection": {}, + "authentication": {"module": "noop"}, + } + + +def _make_config(extra: dict) -> AppConfig: + """Create an AppConfig initialised with *extra* merged into the base.""" + cfg = AppConfig() + data = _base_config() + data.update(extra) + cfg.init_from_dict(data) + return cfg + + +# =================================================================== +# 1. New config structure — rag.byok.stores +# =================================================================== + + +class TestNewByokStoresSection: + """AC-1: byok_rag section is moved under rag.byok.stores.""" + + def test_single_store_under_rag_byok_stores(self, tmp_path: Path) -> None: + """A single BYOK store defined under rag.byok.stores is accepted.""" + db_file = tmp_path / "ocp.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "ocp-docs", + "backend": "faiss", + "embedding_model": "sentence-transformers/all-mpnet-base-v2", + "embedding_dimension": 768, + "vector_db_id": "vs_123", + "db_path": str(db_file), + "score_multiplier": 1.0, + }, + ], + }, + }, + } + ) + # The config should be loadable and the store accessible + assert cfg.configuration is not None + + def test_multiple_stores_under_rag_byok_stores(self, tmp_path: Path) -> None: + """Multiple BYOK stores under rag.byok.stores are accepted.""" + db1 = tmp_path / "ocp.faiss" + db1.touch() + db2 = tmp_path / "kb.faiss" + db2.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "ocp-docs", + "backend": "faiss", + "embedding_dimension": 768, + "vector_db_id": "vs_123", + "db_path": str(db1), + "score_multiplier": 1.0, + }, + { + "rag_id": "knowledge-base", + "backend": "faiss", + "embedding_dimension": 384, + "vector_db_id": "vs_456", + "db_path": str(db2), + "score_multiplier": 1.2, + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + def test_empty_stores_list_accepted(self) -> None: + """An empty rag.byok.stores list is valid (no BYOK stores).""" + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [], + }, + }, + } + ) + assert cfg.configuration is not None + + +# =================================================================== +# 2. New config structure — rag.retrieval.inline / rag.retrieval.tool +# =================================================================== + + +class TestNewRetrievalSection: + """AC-2: rag.inline and rag.tool moved under rag.retrieval.""" + + def test_retrieval_inline_sources(self) -> None: + """rag.retrieval.inline.sources accepts a list of source IDs.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": ["ocp-docs", "knowledge-base"], + }, + }, + }, + } + ) + assert cfg.configuration is not None + + def test_retrieval_tool_sources(self) -> None: + """rag.retrieval.tool.sources accepts a list of source IDs.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "tool": { + "sources": ["ocp-docs", "knowledge-base"], + }, + }, + }, + } + ) + assert cfg.configuration is not None + + def test_retrieval_inline_and_tool_together(self) -> None: + """Both inline and tool retrieval can be configured simultaneously.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": ["ocp-docs", "knowledge-base", "okp"], + }, + "tool": { + "sources": ["ocp-docs", "knowledge-base"], + }, + }, + }, + } + ) + assert cfg.configuration is not None + + def test_retrieval_empty_sources(self) -> None: + """Empty sources lists are valid (disables that retrieval mode).""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": [], + }, + "tool": { + "sources": [], + }, + }, + }, + } + ) + assert cfg.configuration is not None + + +# =================================================================== +# 3. Configurable max_chunks with correct defaults +# =================================================================== + + +class TestMaxChunksConfigurable: + """AC-3: RAG max chunk constants are user-configurable via config.""" + + def test_byok_max_chunks_default_is_10(self) -> None: + """rag.byok.max_chunks defaults to 10 when not specified.""" + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [], + }, + }, + } + ) + rag_config = cfg.configuration.rag + byok = rag_config.byok + assert byok.max_chunks == 10 + + def test_byok_max_chunks_custom_value(self, tmp_path: Path) -> None: + """rag.byok.max_chunks can be set to a custom value.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "max_chunks": 20, + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + assert cfg.configuration.rag.byok.max_chunks == 20 + + def test_okp_max_chunks_default_is_5(self) -> None: + """rag.okp.max_chunks defaults to 5 when not specified.""" + cfg = _make_config( + { + "rag": { + "okp": { + "offline": True, + }, + }, + } + ) + assert cfg.configuration.rag.okp.max_chunks == 5 + + def test_okp_max_chunks_custom_value(self) -> None: + """rag.okp.max_chunks can be set to a custom value.""" + cfg = _make_config( + { + "rag": { + "okp": { + "offline": True, + "max_chunks": 15, + }, + }, + } + ) + assert cfg.configuration.rag.okp.max_chunks == 15 + + def test_inline_max_chunks_default_is_10(self) -> None: + """rag.retrieval.inline.max_chunks defaults to 10.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": ["store-1"], + }, + }, + }, + } + ) + assert cfg.configuration.rag.retrieval.inline.max_chunks == 10 + + def test_inline_max_chunks_custom_value(self) -> None: + """rag.retrieval.inline.max_chunks can be set to a custom value.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": ["store-1"], + "max_chunks": 25, + }, + }, + }, + } + ) + assert cfg.configuration.rag.retrieval.inline.max_chunks == 25 + + def test_tool_max_chunks_default_is_10(self) -> None: + """rag.retrieval.tool.max_chunks defaults to 10.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "tool": { + "sources": ["store-1"], + }, + }, + }, + } + ) + assert cfg.configuration.rag.retrieval.tool.max_chunks == 10 + + def test_tool_max_chunks_custom_value(self) -> None: + """rag.retrieval.tool.max_chunks can be set to a custom value.""" + cfg = _make_config( + { + "rag": { + "retrieval": { + "tool": { + "sources": ["store-1"], + "max_chunks": 50, + }, + }, + }, + } + ) + assert cfg.configuration.rag.retrieval.tool.max_chunks == 50 + + +# =================================================================== +# 4. rag_type renamed to backend +# =================================================================== + + +class TestBackendFieldRename: + """AC-4: rag_type field renamed to backend.""" + + def test_backend_field_accepted(self, tmp_path: Path) -> None: + """The new 'backend' field is accepted in store config.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + def test_old_rag_type_still_accepted_with_deprecation( + self, + tmp_path: Path, + ) -> None: + """The old 'rag_type' field is still accepted for backward compat.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.warns(DeprecationWarning, match="rag_type"): + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "rag_type": "inline::faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + def test_backend_uses_short_name(self, tmp_path: Path) -> None: + """Backend uses short names like 'faiss' instead of 'inline::faiss'.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + +# =================================================================== +# 5. Backward compatibility — old config format +# =================================================================== + + +class TestBackwardCompatibility: + """AC-5: Old config format still works with deprecation warnings.""" + + def test_old_top_level_byok_rag_still_works( + self, + tmp_path: Path, + ) -> None: + """The old top-level 'byok_rag' key is still accepted.""" + import warnings + + db_file = tmp_path / "test.faiss" + db_file.touch() + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + cfg = _make_config( + { + "byok_rag": [ + { + "rag_id": "store-1", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + } + ) + assert cfg.configuration is not None + + def test_old_byok_rag_emits_deprecation_warning( + self, + tmp_path: Path, + ) -> None: + """Using old top-level 'byok_rag' emits a deprecation warning.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.warns(DeprecationWarning, match="byok_rag"): + _make_config( + { + "byok_rag": [ + { + "rag_id": "store-1", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + } + ) + + def test_both_byok_rag_and_rag_byok_stores_raises_error( + self, + tmp_path: Path, + ) -> None: + """Providing both deprecated byok_rag and rag.byok.stores raises ValueError.""" + from pydantic import ValidationError + + db1 = tmp_path / "old.faiss" + db1.touch() + db2 = tmp_path / "new.faiss" + db2.touch() + with pytest.raises((ValidationError, ValueError), match="byok_rag"): + _make_config( + { + "byok_rag": [ + { + "rag_id": "old-store", + "vector_db_id": "vs_old", + "db_path": str(db1), + }, + ], + "rag": { + "byok": { + "stores": [ + { + "rag_id": "new-store", + "backend": "faiss", + "vector_db_id": "vs_new", + "db_path": str(db2), + }, + ], + }, + }, + } + ) + + def test_old_rag_inline_list_still_works( + self, + ) -> None: + """The old rag.inline (flat list) format is still accepted.""" + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + cfg = _make_config( + { + "rag": { + "inline": ["store-1", "store-2"], + }, + } + ) + assert cfg.configuration is not None + + def test_old_rag_tool_list_still_works( + self, + ) -> None: + """The old rag.tool (flat list) format is still accepted.""" + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + cfg = _make_config( + { + "rag": { + "tool": ["store-1"], + }, + } + ) + assert cfg.configuration is not None + + def test_old_rag_inline_emits_deprecation( + self, + ) -> None: + """Using old rag.inline (flat list) emits a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="inline"): + _make_config( + { + "rag": { + "inline": ["store-1", "store-2"], + }, + } + ) + + def test_old_rag_tool_emits_deprecation( + self, + ) -> None: + """Using old rag.tool (flat list) emits a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="tool"): + _make_config( + { + "rag": { + "tool": ["store-1"], + }, + } + ) + + def test_old_top_level_okp_still_works( + self, + ) -> None: + """The old top-level 'okp' key is still accepted.""" + import warnings + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + cfg = _make_config( + { + "okp": { + "offline": True, + "chunk_filter_query": "is_chunk:true", + }, + } + ) + assert cfg.configuration is not None + + def test_old_top_level_okp_emits_deprecation( + self, + ) -> None: + """Using old top-level 'okp' emits a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="okp"): + _make_config( + { + "okp": { + "offline": True, + }, + } + ) + + +# =================================================================== +# 6. Full target config structure +# =================================================================== + + +class TestFullTargetConfigStructure: + """Test the complete target config structure from the spec.""" + + def test_full_target_config_accepted(self, tmp_path: Path) -> None: + """The full target config structure from the spec is accepted.""" + db1 = tmp_path / "ocp.faiss" + db1.touch() + db2 = tmp_path / "kb.faiss" + db2.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "max_chunks": 10, + "stores": [ + { + "rag_id": "ocp-docs", + "backend": "faiss", + "embedding_model": "sentence-transformers/all-mpnet-base-v2", + "embedding_dimension": 768, + "vector_db_id": "vs_123", + "db_path": str(db1), + "score_multiplier": 1.0, + }, + { + "rag_id": "knowledge-base", + "backend": "faiss", + "embedding_dimension": 384, + "vector_db_id": "vs_456", + "db_path": str(db2), + "score_multiplier": 1.2, + }, + ], + }, + "okp": { + "offline": True, + "chunk_filter_query": "is_chunk:true", + "max_chunks": 5, + }, + "retrieval": { + "inline": { + "sources": ["ocp-docs", "knowledge-base", "okp"], + "max_chunks": 10, + }, + "tool": { + "sources": ["ocp-docs", "knowledge-base"], + "max_chunks": 10, + }, + }, + }, + } + ) + assert cfg.configuration is not None + rag = cfg.configuration.rag + + # Verify byok section + assert rag.byok.max_chunks == 10 + assert len(rag.byok.stores) == 2 + assert rag.byok.stores[0].rag_id == "ocp-docs" + assert rag.byok.stores[1].rag_id == "knowledge-base" + + # Verify okp section + assert rag.okp.offline is True + assert rag.okp.chunk_filter_query == "is_chunk:true" + assert rag.okp.max_chunks == 5 + + # Verify retrieval section + assert rag.retrieval.inline.sources == [ + "ocp-docs", + "knowledge-base", + "okp", + ] + assert rag.retrieval.inline.max_chunks == 10 + assert rag.retrieval.tool.sources == [ + "ocp-docs", + "knowledge-base", + ] + assert rag.retrieval.tool.max_chunks == 10 + + +# =================================================================== +# 7. Edge cases +# =================================================================== + + +class TestEdgeCases: + """Edge cases: missing fields, invalid values, partial configs.""" + + def test_rag_section_with_no_subsections(self) -> None: + """An empty rag section is valid (all subsections optional).""" + cfg = _make_config( + { + "rag": {}, + } + ) + assert cfg.configuration is not None + + def test_no_rag_section_at_all(self) -> None: + """Config without any rag section is valid (RAG is optional).""" + cfg = _make_config({}) + assert cfg.configuration is not None + + def test_byok_without_stores_key(self) -> None: + """rag.byok without stores key defaults to empty stores.""" + cfg = _make_config( + { + "rag": { + "byok": {}, + }, + } + ) + assert cfg.configuration is not None + + def test_retrieval_without_inline_or_tool(self) -> None: + """rag.retrieval without inline or tool is valid.""" + cfg = _make_config( + { + "rag": { + "retrieval": {}, + }, + } + ) + assert cfg.configuration is not None + + def test_max_chunks_must_be_positive(self) -> None: + """max_chunks with zero or negative value should be rejected.""" + from pydantic import ValidationError + + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "max_chunks": 0, + "stores": [], + }, + }, + } + ) + + def test_max_chunks_negative_rejected(self) -> None: + """Negative max_chunks should be rejected.""" + from pydantic import ValidationError + + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "retrieval": { + "inline": { + "sources": [], + "max_chunks": -5, + }, + }, + }, + } + ) + + def test_byok_store_missing_required_rag_id(self, tmp_path: Path) -> None: + """A BYOK store without rag_id should be rejected.""" + from pydantic import ValidationError + + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + + def test_byok_store_missing_required_vector_db_id( + self, + tmp_path: Path, + ) -> None: + """A BYOK store without vector_db_id should be rejected.""" + from pydantic import ValidationError + + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + + def test_okp_max_chunks_zero_rejected(self) -> None: + """OKP max_chunks of zero should be rejected.""" + from pydantic import ValidationError + + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "okp": { + "offline": True, + "max_chunks": 0, + }, + }, + } + ) + + def test_tool_max_chunks_zero_rejected(self) -> None: + """Tool max_chunks of zero should be rejected.""" + from pydantic import ValidationError + + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "retrieval": { + "tool": { + "sources": [], + "max_chunks": 0, + }, + }, + }, + } + ) + + +# =================================================================== +# 8. OKP under rag section +# =================================================================== + + +class TestOkpUnderRag: + """Test OKP configuration under the rag section.""" + + def test_okp_under_rag_with_offline_true(self) -> None: + """rag.okp with offline=True is accepted.""" + cfg = _make_config( + { + "rag": { + "okp": { + "offline": True, + }, + }, + } + ) + assert cfg.configuration.rag.okp.offline is True + + def test_okp_under_rag_with_offline_false(self) -> None: + """rag.okp with offline=False is accepted.""" + cfg = _make_config( + { + "rag": { + "okp": { + "offline": False, + }, + }, + } + ) + assert cfg.configuration.rag.okp.offline is False + + def test_okp_under_rag_with_chunk_filter_query(self) -> None: + """rag.okp.chunk_filter_query is accepted.""" + cfg = _make_config( + { + "rag": { + "okp": { + "offline": True, + "chunk_filter_query": "is_chunk:true", + }, + }, + } + ) + assert cfg.configuration.rag.okp.chunk_filter_query == "is_chunk:true" + + def test_okp_under_rag_defaults(self) -> None: + """rag.okp with no fields uses defaults.""" + cfg = _make_config( + { + "rag": { + "okp": {}, + }, + } + ) + assert cfg.configuration.rag.okp.offline is True + assert cfg.configuration.rag.okp.max_chunks == 5 + + +# =================================================================== +# 9. Constant mapping verification +# =================================================================== + + +class TestConstantMappingDefaults: + """Verify the constant mapping table from the spec. + + | Old constant | New config field | Default | + |------------------------|-----------------------------------|---------| + | BYOK_RAG_MAX_CHUNKS | rag.byok.max_chunks | 10 | + | OKP_RAG_MAX_CHUNKS | rag.okp.max_chunks | 5 | + | INLINE_RAG_MAX_CHUNKS | rag.retrieval.inline.max_chunks | 10 | + | TOOL_RAG_MAX_CHUNKS | rag.retrieval.tool.max_chunks | 10 | + """ + + def test_all_defaults_match_spec(self, tmp_path: Path) -> None: + """All max_chunks defaults match the constant mapping table.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + "okp": { + "offline": True, + }, + "retrieval": { + "inline": { + "sources": ["store-1"], + }, + "tool": { + "sources": ["store-1"], + }, + }, + }, + } + ) + rag = cfg.configuration.rag + assert rag.byok.max_chunks == 10, "BYOK_RAG_MAX_CHUNKS default should be 10" + assert rag.okp.max_chunks == 5, "OKP_RAG_MAX_CHUNKS default should be 5" + assert ( + rag.retrieval.inline.max_chunks == 10 + ), "INLINE_RAG_MAX_CHUNKS default should be 10" + assert ( + rag.retrieval.tool.max_chunks == 10 + ), "TOOL_RAG_MAX_CHUNKS default should be 10" + + def test_all_max_chunks_overridable(self, tmp_path: Path) -> None: + """All max_chunks values can be overridden from defaults.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "max_chunks": 42, + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + "okp": { + "offline": True, + "max_chunks": 99, + }, + "retrieval": { + "inline": { + "sources": ["store-1"], + "max_chunks": 77, + }, + "tool": { + "sources": ["store-1"], + "max_chunks": 55, + }, + }, + }, + } + ) + rag = cfg.configuration.rag + assert rag.byok.max_chunks == 42 + assert rag.okp.max_chunks == 99 + assert rag.retrieval.inline.max_chunks == 77 + assert rag.retrieval.tool.max_chunks == 55 + + +# =================================================================== +# 10. No deprecation warnings for new format +# =================================================================== + + +class TestNoDeprecationForNewFormat: + """New config format should NOT emit deprecation warnings.""" + + def test_new_format_no_deprecation_warnings( + self, + tmp_path: Path, + ) -> None: + """Using the new config format should not emit deprecation warnings.""" + import warnings + + db_file = tmp_path / "test.faiss" + db_file.touch() + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + _make_config( + { + "rag": { + "byok": { + "max_chunks": 10, + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + "okp": { + "offline": True, + "max_chunks": 5, + }, + "retrieval": { + "inline": { + "sources": ["store-1"], + "max_chunks": 10, + }, + "tool": { + "sources": ["store-1"], + "max_chunks": 10, + }, + }, + }, + } + ) + deprecation_warnings = [ + w for w in caught if issubclass(w.category, DeprecationWarning) + ] + assert len(deprecation_warnings) == 0, ( + f"New config format should not emit deprecation warnings, " + f"but got: {[str(w.message) for w in deprecation_warnings]}" + ) + + +# =================================================================== +# 11. Store field validation in new structure +# =================================================================== + + +class TestStoreFieldValidation: + """Validate store fields within the new rag.byok.stores structure.""" + + def test_store_with_all_fields(self, tmp_path: Path) -> None: + """A store with all fields specified is accepted.""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "full-store", + "backend": "faiss", + "embedding_model": "sentence-transformers/all-mpnet-base-v2", + "embedding_dimension": 768, + "vector_db_id": "vs_full", + "db_path": str(db_file), + "score_multiplier": 1.5, + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + def test_store_with_minimal_fields(self, tmp_path: Path) -> None: + """A store with only required fields is accepted (defaults apply).""" + db_file = tmp_path / "test.faiss" + db_file.touch() + cfg = _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "minimal-store", + "vector_db_id": "vs_min", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + assert cfg.configuration is not None + + def test_store_empty_rag_id_rejected(self, tmp_path: Path) -> None: + """A store with empty rag_id is rejected.""" + from pydantic import ValidationError + + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + + def test_store_negative_embedding_dimension_rejected( + self, + tmp_path: Path, + ) -> None: + """A store with negative embedding_dimension is rejected.""" + from pydantic import ValidationError + + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "embedding_dimension": -768, + "vector_db_id": "vs_1", + "db_path": str(db_file), + }, + ], + }, + }, + } + ) + + def test_store_zero_score_multiplier_rejected( + self, + tmp_path: Path, + ) -> None: + """A store with score_multiplier of 0 is rejected.""" + from pydantic import ValidationError + + db_file = tmp_path / "test.faiss" + db_file.touch() + with pytest.raises((ValidationError, ValueError)): + _make_config( + { + "rag": { + "byok": { + "stores": [ + { + "rag_id": "store-1", + "backend": "faiss", + "vector_db_id": "vs_1", + "db_path": str(db_file), + "score_multiplier": 0.0, + }, + ], + }, + }, + } + ) diff --git a/tests/unit/models/config/test_rag_configuration.py b/tests/unit/models/config/test_rag_configuration.py index bc44ef154..c5993e675 100644 --- a/tests/unit/models/config/test_rag_configuration.py +++ b/tests/unit/models/config/test_rag_configuration.py @@ -1,13 +1,148 @@ -"""Unit tests for RAG and OKP configuration models.""" +"""Unit tests for RAG, OKP, retrieval, and BYOK configuration models.""" # pylint: disable=no-member # Pydantic Field(default_factory=...) pattern confuses pylint's static analysis +import warnings + import pytest from pydantic import ValidationError import constants -from models.config import OkpConfiguration, RagConfiguration +from models.config import ( + ByokConfiguration, + ByokRag, + OkpConfiguration, + RagConfiguration, + RetrievalConfiguration, + RetrievalInlineConfiguration, + RetrievalToolConfiguration, +) + + +class TestRetrievalInlineConfiguration: + """Tests for RetrievalInlineConfiguration model.""" + + def test_default_values(self) -> None: + """Test that RetrievalInlineConfiguration has correct default values.""" + config = RetrievalInlineConfiguration() + assert config.sources == [] + assert config.max_chunks == constants.INLINE_RAG_MAX_CHUNKS + + def test_custom_sources(self) -> None: + """Test inline sources with custom IDs.""" + config = RetrievalInlineConfiguration(sources=["store-1", "store-2"]) + assert config.sources == ["store-1", "store-2"] + + def test_custom_max_chunks(self) -> None: + """Test custom max_chunks value.""" + config = RetrievalInlineConfiguration(max_chunks=20) + assert config.max_chunks == 20 + + def test_max_chunks_must_be_positive(self) -> None: + """Test that max_chunks must be a positive integer.""" + with pytest.raises(ValidationError, match="greater than 0"): + RetrievalInlineConfiguration(max_chunks=0) + + def test_no_unknown_fields_allowed(self) -> None: + """Test that RetrievalInlineConfiguration rejects unknown fields.""" + with pytest.raises(ValidationError, match="Extra inputs are not permitted"): + RetrievalInlineConfiguration(unknown_field="value") # type: ignore[call-arg] + + +class TestRetrievalToolConfiguration: + """Tests for RetrievalToolConfiguration model.""" + + def test_default_values(self) -> None: + """Test that RetrievalToolConfiguration has correct default values.""" + config = RetrievalToolConfiguration() + assert config.sources == [] + assert config.max_chunks == constants.TOOL_RAG_MAX_CHUNKS + + def test_custom_sources(self) -> None: + """Test tool sources with custom IDs.""" + config = RetrievalToolConfiguration(sources=[constants.OKP_RAG_ID, "store-1"]) + assert config.sources == [constants.OKP_RAG_ID, "store-1"] + + def test_custom_max_chunks(self) -> None: + """Test custom max_chunks value.""" + config = RetrievalToolConfiguration(max_chunks=15) + assert config.max_chunks == 15 + + def test_max_chunks_must_be_positive(self) -> None: + """Test that max_chunks must be a positive integer.""" + with pytest.raises(ValidationError, match="greater than 0"): + RetrievalToolConfiguration(max_chunks=0) + + def test_no_unknown_fields_allowed(self) -> None: + """Test that RetrievalToolConfiguration rejects unknown fields.""" + with pytest.raises(ValidationError, match="Extra inputs are not permitted"): + RetrievalToolConfiguration(unknown_field="value") # type: ignore[call-arg] + + +class TestRetrievalConfiguration: + """Tests for RetrievalConfiguration model.""" + + def test_default_values(self) -> None: + """Test that RetrievalConfiguration has correct default values.""" + config = RetrievalConfiguration() + assert config.inline.sources == [] + assert config.inline.max_chunks == constants.INLINE_RAG_MAX_CHUNKS + assert config.tool.sources == [] + assert config.tool.max_chunks == constants.TOOL_RAG_MAX_CHUNKS + + def test_custom_inline_and_tool(self) -> None: + """Test RetrievalConfiguration with custom inline and tool settings.""" + config = RetrievalConfiguration( + inline=RetrievalInlineConfiguration(sources=["store-1"], max_chunks=20), + tool=RetrievalToolConfiguration(sources=["store-2"], max_chunks=15), + ) + assert config.inline.sources == ["store-1"] + assert config.inline.max_chunks == 20 + assert config.tool.sources == ["store-2"] + assert config.tool.max_chunks == 15 + + def test_no_unknown_fields_allowed(self) -> None: + """Test that RetrievalConfiguration rejects unknown fields.""" + with pytest.raises(ValidationError, match="Extra inputs are not permitted"): + RetrievalConfiguration(unknown_field="value") # type: ignore[call-arg] + + +class TestByokConfiguration: + """Tests for ByokConfiguration model.""" + + def test_default_values(self) -> None: + """Test that ByokConfiguration has correct default values.""" + config = ByokConfiguration() + assert config.max_chunks == constants.BYOK_RAG_MAX_CHUNKS + assert config.stores == [] + + def test_custom_max_chunks(self) -> None: + """Test custom max_chunks value.""" + config = ByokConfiguration(max_chunks=20) + assert config.max_chunks == 20 + + def test_max_chunks_must_be_positive(self) -> None: + """Test that max_chunks must be a positive integer.""" + with pytest.raises(ValidationError, match="greater than 0"): + ByokConfiguration(max_chunks=0) + + def test_with_stores(self) -> None: + """Test ByokConfiguration with BYOK stores.""" + store = ByokRag( + rag_id="test-store", + backend="faiss", + vector_db_id="vs_123", + db_path="/tmp/test.faiss", + ) + config = ByokConfiguration(stores=[store]) + assert len(config.stores) == 1 + assert config.stores[0].rag_id == "test-store" + + def test_no_unknown_fields_allowed(self) -> None: + """Test that ByokConfiguration rejects unknown fields.""" + with pytest.raises(ValidationError, match="Extra inputs are not permitted"): + ByokConfiguration(unknown_field="value") # type: ignore[call-arg] class TestRagConfiguration: @@ -16,39 +151,93 @@ class TestRagConfiguration: def test_default_values(self) -> None: """Test that RagConfiguration has correct default values.""" config = RagConfiguration() - assert config.inline == [] - assert config.tool == [] - - def test_inline_with_byok_ids(self) -> None: - """Test inline list with BYOK rag IDs.""" - config = RagConfiguration(inline=["store-1", "store-2"]) - assert config.inline == ["store-1", "store-2"] - assert config.tool == [] - - def test_inline_with_okp_rag(self) -> None: - """Test inline list including the special OKP ID.""" - config = RagConfiguration(inline=[constants.OKP_RAG_ID, "store-1"]) - assert constants.OKP_RAG_ID in config.inline - assert "store-1" in config.inline - - def test_tool_with_okp_rag_and_byok(self) -> None: - """Test tool list with OKP and BYOK IDs.""" + assert config.byok.stores == [] + assert config.byok.max_chunks == constants.BYOK_RAG_MAX_CHUNKS + assert config.okp.offline is True + assert config.okp.max_chunks == constants.OKP_RAG_MAX_CHUNKS + assert config.retrieval.inline.sources == [] + assert config.retrieval.inline.max_chunks == constants.INLINE_RAG_MAX_CHUNKS + assert config.retrieval.tool.sources == [] + assert config.retrieval.tool.max_chunks == constants.TOOL_RAG_MAX_CHUNKS + + def test_deprecated_inline_field_migrated(self) -> None: + """Test that deprecated 'inline' field is migrated to retrieval.inline.sources.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + config = RagConfiguration(inline=["store-1", "store-2"]) + assert config.retrieval.inline.sources == ["store-1", "store-2"] + assert config.inline is None + assert len(w) == 1 + assert "deprecated" in str(w[0].message).lower() + + def test_deprecated_tool_field_migrated(self) -> None: + """Test that deprecated 'tool' field is migrated to retrieval.tool.sources.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + config = RagConfiguration(tool=[constants.OKP_RAG_ID, "store-1"]) + assert config.retrieval.tool.sources == [constants.OKP_RAG_ID, "store-1"] + assert config.tool is None + assert len(w) == 1 + assert "deprecated" in str(w[0].message).lower() + + def test_deprecated_inline_and_tool_both_migrated(self) -> None: + """Test that both deprecated fields are migrated together.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + config = RagConfiguration( + inline=["store-1"], + tool=[constants.OKP_RAG_ID], + ) + assert config.retrieval.inline.sources == ["store-1"] + assert config.retrieval.tool.sources == [constants.OKP_RAG_ID] + assert len(w) == 2 + + def test_new_format_retrieval(self) -> None: + """Test RagConfiguration with new retrieval format.""" config = RagConfiguration( - inline=["store-1"], - tool=[constants.OKP_RAG_ID, "store-1"], + retrieval=RetrievalConfiguration( + inline=RetrievalInlineConfiguration( + sources=["store-1", constants.OKP_RAG_ID], + max_chunks=20, + ), + tool=RetrievalToolConfiguration( + sources=["store-1"], + max_chunks=15, + ), + ), ) - assert config.inline == ["store-1"] - assert config.tool == [constants.OKP_RAG_ID, "store-1"] + assert config.retrieval.inline.sources == ["store-1", constants.OKP_RAG_ID] + assert config.retrieval.inline.max_chunks == 20 + assert config.retrieval.tool.sources == ["store-1"] + assert config.retrieval.tool.max_chunks == 15 - def test_tool_empty_list(self) -> None: - """Test that an explicit empty tool list disables tool RAG.""" - config = RagConfiguration(tool=[]) - assert config.tool == [] + def test_new_format_byok(self) -> None: + """Test RagConfiguration with new byok format.""" + store = ByokRag( + rag_id="ocp-docs", + backend="faiss", + vector_db_id="vs_123", + db_path="/tmp/ocp.faiss", + ) + config = RagConfiguration( + byok=ByokConfiguration(max_chunks=15, stores=[store]), + ) + assert config.byok.max_chunks == 15 + assert len(config.byok.stores) == 1 + assert config.byok.stores[0].rag_id == "ocp-docs" - def test_tool_default_is_empty_list(self) -> None: - """Test that tool defaults to an empty list.""" - config = RagConfiguration() - assert config.tool == [] + def test_new_format_okp(self) -> None: + """Test RagConfiguration with new okp format.""" + config = RagConfiguration( + okp=OkpConfiguration( + offline=False, + chunk_filter_query="product:*openshift*", + max_chunks=8, + ), + ) + assert config.okp.offline is False + assert config.okp.chunk_filter_query == "product:*openshift*" + assert config.okp.max_chunks == 8 def test_no_unknown_fields_allowed(self) -> None: """Test that RagConfiguration rejects unknown fields.""" @@ -56,14 +245,32 @@ def test_no_unknown_fields_allowed(self) -> None: RagConfiguration(unknown_field="value") # type: ignore[call-arg] def test_fully_custom_config(self) -> None: - """Test RagConfiguration with all fields set.""" + """Test RagConfiguration with all sections set.""" + store = ByokRag( + rag_id="kb", + backend="faiss", + vector_db_id="vs_456", + db_path="/tmp/kb.faiss", + ) config = RagConfiguration( - inline=[constants.OKP_RAG_ID, "store-1"], - tool=["store-1"], + byok=ByokConfiguration(max_chunks=12, stores=[store]), + okp=OkpConfiguration(offline=True, max_chunks=7), + retrieval=RetrievalConfiguration( + inline=RetrievalInlineConfiguration( + sources=[constants.OKP_RAG_ID, "kb"], + max_chunks=15, + ), + tool=RetrievalToolConfiguration( + sources=["kb"], + max_chunks=8, + ), + ), ) - assert constants.OKP_RAG_ID in config.inline - assert "store-1" in config.inline - assert config.tool == ["store-1"] + assert config.byok.max_chunks == 12 + assert len(config.byok.stores) == 1 + assert config.okp.max_chunks == 7 + assert config.retrieval.inline.max_chunks == 15 + assert config.retrieval.tool.max_chunks == 8 class TestOkpConfiguration: @@ -74,6 +281,7 @@ def test_default_values(self) -> None: config = OkpConfiguration() assert config.offline is True assert config.chunk_filter_query is None + assert config.max_chunks == constants.OKP_RAG_MAX_CHUNKS def test_offline_false(self) -> None: """Test offline can be set to False (online mode).""" @@ -85,6 +293,16 @@ def test_custom_chunk_filter_query(self) -> None: config = OkpConfiguration(chunk_filter_query="product:*openshift*") assert config.chunk_filter_query == "product:*openshift*" + def test_custom_max_chunks(self) -> None: + """Test that max_chunks can be customised.""" + config = OkpConfiguration(max_chunks=8) + assert config.max_chunks == 8 + + def test_max_chunks_must_be_positive(self) -> None: + """Test that max_chunks must be a positive integer.""" + with pytest.raises(ValidationError, match="greater than 0"): + OkpConfiguration(max_chunks=0) + def test_no_unknown_fields_allowed(self) -> None: """Test that OkpConfiguration rejects unknown fields.""" with pytest.raises(ValidationError, match="Extra inputs are not permitted"): diff --git a/tests/unit/utils/test_responses.py b/tests/unit/utils/test_responses.py index d8530cec1..0286cfdb6 100644 --- a/tests/unit/utils/test_responses.py +++ b/tests/unit/utils/test_responses.py @@ -1711,9 +1711,12 @@ async def test_translates_byok_ids_in_prepare_tools( mock_byok_rag.rag_id = "ocp_docs" mock_byok_rag.vector_db_id = "vs-001" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [mock_byok_rag] - mock_config.configuration.rag.tool = [] - mock_config.configuration.rag.inline = [] + mock_config.byok_rag = [mock_byok_rag] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = [] + mock_config.configuration.rag.retrieval.inline.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(["ocp_docs"], False, "token") @@ -1731,9 +1734,12 @@ async def test_passes_through_unknown_ids_in_prepare_tools( # Configure empty BYOK RAG mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] - mock_config.configuration.rag.tool = [] - mock_config.configuration.rag.inline = [] + mock_config.byok_rag = [] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = [] + mock_config.configuration.rag.retrieval.inline.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(["raw-internal-id"], False, "token") @@ -1753,9 +1759,15 @@ async def test_uses_rag_tool_config_when_no_per_request_ids( mocker.patch("utils.responses.get_mcp_tools", return_value=None) mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] - mock_config.configuration.rag.tool = ["rag-tool-id-1", "rag-tool-id-2"] - mock_config.configuration.rag.inline = [] + mock_config.byok_rag = [] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = [ + "rag-tool-id-1", + "rag-tool-id-2", + ] + mock_config.configuration.rag.retrieval.inline.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(None, False, "token") @@ -1776,9 +1788,12 @@ async def test_rag_tool_config_ids_are_translated( mock_byok_rag.rag_id = "ocp_docs" mock_byok_rag.vector_db_id = "vs-001" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [mock_byok_rag] - mock_config.configuration.rag.tool = ["ocp_docs"] - mock_config.configuration.rag.inline = [] + mock_config.byok_rag = [mock_byok_rag] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = ["ocp_docs"] + mock_config.configuration.rag.retrieval.inline.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(None, False, "token") @@ -1795,9 +1810,12 @@ async def test_inline_rag_config_does_not_affect_tool_rag( mocker.patch("utils.responses.get_mcp_tools", return_value=None) mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] - mock_config.configuration.rag.tool = [] - mock_config.configuration.rag.inline = [ + mock_config.byok_rag = [] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = [] + mock_config.configuration.rag.retrieval.inline.sources = [ "inline-store-id" ] # inline is configured mocker.patch("utils.responses.configuration", mock_config) @@ -1814,9 +1832,12 @@ async def test_per_request_ids_override_rag_tool_config( mocker.patch("utils.responses.get_mcp_tools", return_value=None) mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] - mock_config.configuration.rag.tool = ["config-id-1"] - mock_config.configuration.rag.inline = [] + mock_config.byok_rag = [] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = ["config-id-1"] + mock_config.configuration.rag.retrieval.inline.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(["request-id-1"], False, "token") @@ -1833,8 +1854,11 @@ async def test_tool_rag_disabled_when_tool_not_configured( mocker.patch("utils.responses.get_mcp_tools", return_value=None) mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] - mock_config.configuration.rag.tool = [] + mock_config.byok_rag = [] + mock_config.configuration.rag.retrieval.tool.max_chunks = ( + constants.TOOL_RAG_MAX_CHUNKS + ) + mock_config.configuration.rag.retrieval.tool.sources = [] mocker.patch("utils.responses.configuration", mock_config) result = await prepare_tools(None, False, "token") @@ -3263,7 +3287,7 @@ async def test_client_tools_without_merge_header( ) -> None: """Test client tools used as-is without merge header.""" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] + mock_config.byok_rag = [] mock_config.mcp_servers = [] mocker.patch("utils.responses.configuration", mock_config) @@ -3281,7 +3305,7 @@ async def test_client_tools_without_merge_header( async def test_client_tools_with_merge_header(self, mocker: MockerFixture) -> None: """Test client tools merged with server tools when header is set.""" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] + mock_config.byok_rag = [] mock_config.mcp_servers = [] mocker.patch("utils.responses.configuration", mock_config) @@ -3311,7 +3335,7 @@ async def test_merge_header_conflict_raises_409( ) -> None: """Test 409 when merge header is set and tools conflict.""" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] + mock_config.byok_rag = [] mock_config.mcp_servers = [] mocker.patch("utils.responses.configuration", mock_config) @@ -3356,7 +3380,7 @@ async def test_merge_header_no_server_tools_returns_client_only( ) -> None: """Test merge header with no server tools returns client tools unchanged.""" mock_config = mocker.Mock() - mock_config.configuration.byok_rag = [] + mock_config.byok_rag = [] mock_config.mcp_servers = [] mocker.patch("utils.responses.configuration", mock_config) mocker.patch( diff --git a/tests/unit/utils/test_vector_search.py b/tests/unit/utils/test_vector_search.py index e53be0148..9412f3076 100644 --- a/tests/unit/utils/test_vector_search.py +++ b/tests/unit/utils/test_vector_search.py @@ -518,8 +518,8 @@ class TestFetchByokRag: async def test_byok_no_inline_ids(self, mocker: MockerFixture) -> None: """Test when no inline BYOK sources are configured.""" config_mock = mocker.Mock(spec=AppConfig) - config_mock.configuration.rag.inline = [] - config_mock.configuration.byok_rag = [] + config_mock.configuration.rag.retrieval.inline.sources = [] + config_mock.byok_rag = [] mocker.patch("utils.vector_search.configuration", config_mock) client_mock = mocker.AsyncMock() @@ -537,8 +537,9 @@ async def test_byok_enabled_success(self, mocker: MockerFixture) -> None: byok_rag_mock = mocker.Mock() byok_rag_mock.rag_id = "rag_1" byok_rag_mock.vector_db_id = "vs_1" - config_mock.configuration.rag.inline = ["rag_1"] - config_mock.configuration.byok_rag = [byok_rag_mock] + config_mock.configuration.rag.retrieval.inline.sources = ["rag_1"] + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.byok_rag = [byok_rag_mock] config_mock.score_multiplier_mapping = {"vs_1": 1.5} config_mock.rag_id_mapping = {"vs_1": "rag_1"} mocker.patch("utils.vector_search.configuration", config_mock) @@ -576,8 +577,9 @@ async def test_user_facing_ids_translated_to_internal_ids( byok_rag_mock = mocker.Mock() byok_rag_mock.rag_id = "my-kb" byok_rag_mock.vector_db_id = "vs-internal-001" - config_mock.configuration.byok_rag = [byok_rag_mock] - config_mock.configuration.rag.inline = ["my-kb"] + config_mock.byok_rag = [byok_rag_mock] + config_mock.configuration.rag.retrieval.inline.sources = ["my-kb"] + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS config_mock.score_multiplier_mapping = {"vs-internal-001": 1.0} config_mock.rag_id_mapping = {"vs-internal-001": "my-kb"} mocker.patch("utils.vector_search.configuration", config_mock) @@ -616,8 +618,12 @@ async def test_multiple_user_facing_ids_each_translated( byok_rag_2 = mocker.Mock() byok_rag_2.rag_id = "kb-part2" byok_rag_2.vector_db_id = "vs-bbb-222" - config_mock.configuration.byok_rag = [byok_rag_1, byok_rag_2] - config_mock.configuration.rag.inline = ["kb-part1", "kb-part2"] + config_mock.byok_rag = [byok_rag_1, byok_rag_2] + config_mock.configuration.rag.retrieval.inline.sources = [ + "kb-part1", + "kb-part2", + ] + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS config_mock.score_multiplier_mapping = {"vs-aaa-111": 1.0, "vs-bbb-222": 1.0} config_mock.rag_id_mapping = { "vs-aaa-111": "kb-part1", @@ -658,8 +664,8 @@ async def test_no_inline_rag_configured_skips_byok( ) -> None: """Test that BYOK inline RAG is skipped when rag.inline is empty.""" config_mock = mocker.Mock(spec=AppConfig) - config_mock.configuration.rag.inline = [] - config_mock.configuration.byok_rag = [] + config_mock.configuration.rag.retrieval.inline.sources = [] + config_mock.byok_rag = [] mocker.patch("utils.vector_search.configuration", config_mock) client_mock = mocker.AsyncMock() @@ -678,8 +684,9 @@ async def test_request_id_not_in_inline_config_skips_byok( ) -> None: """Test that a request vector_store_id not registered in rag.inline is filtered out.""" config_mock = mocker.Mock(spec=AppConfig) - config_mock.configuration.rag.inline = ["registered-id"] - config_mock.configuration.byok_rag = [] + config_mock.configuration.rag.retrieval.inline.sources = [] + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.byok_rag = [] mocker.patch("utils.vector_search.configuration", config_mock) client_mock = mocker.AsyncMock() @@ -718,6 +725,7 @@ async def test_solr_enabled_success(self, mocker: MockerFixture) -> None: config_mock.inline_solr_enabled = True config_mock.okp.offline = True config_mock.okp.rhokp_url = "https://okp.test" + config_mock.configuration.rag.okp.max_chunks = constants.OKP_RAG_MAX_CHUNKS mocker.patch("utils.vector_search.configuration", config_mock) # Mock chunk @@ -783,8 +791,13 @@ class TestBuildRagContext: async def test_both_sources_disabled(self, mocker: MockerFixture) -> None: """Test when both BYOK inline and Solr inline are not configured.""" config_mock = mocker.Mock(spec=AppConfig) - config_mock.configuration.rag.inline = [] - config_mock.configuration.byok_rag = [] + config_mock.configuration.rag.retrieval.inline.sources = [] + config_mock.configuration.rag.retrieval.inline.max_chunks = ( + constants.INLINE_RAG_MAX_CHUNKS + ) + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.configuration.rag.okp.max_chunks = constants.OKP_RAG_MAX_CHUNKS + config_mock.byok_rag = [] config_mock.inline_solr_enabled = False mocker.patch("utils.vector_search.configuration", config_mock) @@ -803,8 +816,13 @@ async def test_byok_enabled_only(self, mocker: MockerFixture) -> None: byok_rag_mock = mocker.Mock() byok_rag_mock.rag_id = "rag_1" byok_rag_mock.vector_db_id = "vs_1" - config_mock.configuration.rag.inline = ["rag_1"] - config_mock.configuration.byok_rag = [byok_rag_mock] + config_mock.configuration.rag.retrieval.inline.sources = ["rag_1"] + config_mock.configuration.rag.retrieval.inline.max_chunks = ( + constants.INLINE_RAG_MAX_CHUNKS + ) + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.configuration.rag.okp.max_chunks = constants.OKP_RAG_MAX_CHUNKS + config_mock.byok_rag = [byok_rag_mock] config_mock.inline_solr_enabled = False config_mock.score_multiplier_mapping = {"vs_1": 1.0} config_mock.rag_id_mapping = {"vs_1": "rag_1"} @@ -840,8 +858,13 @@ async def test_reranker_enabled_calls_cross_encoder( byok_rag_mock = mocker.Mock() byok_rag_mock.rag_id = "rag_1" byok_rag_mock.vector_db_id = "vs_1" - config_mock.configuration.rag.inline = ["rag_1"] - config_mock.configuration.byok_rag = [byok_rag_mock] + config_mock.configuration.rag.retrieval.inline.sources = ["rag_1"] + config_mock.configuration.rag.retrieval.inline.max_chunks = ( + constants.INLINE_RAG_MAX_CHUNKS + ) + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.configuration.rag.okp.max_chunks = constants.OKP_RAG_MAX_CHUNKS + config_mock.byok_rag = [byok_rag_mock] config_mock.inline_solr_enabled = False config_mock.score_multiplier_mapping = {"vs_1": 1.0} config_mock.rag_id_mapping = {"vs_1": "rag_1"} @@ -891,8 +914,13 @@ async def test_reranker_disabled_skips_cross_encoder( byok_rag_mock = mocker.Mock() byok_rag_mock.rag_id = "rag_1" byok_rag_mock.vector_db_id = "vs_1" - config_mock.configuration.rag.inline = ["rag_1"] - config_mock.configuration.byok_rag = [byok_rag_mock] + config_mock.configuration.rag.retrieval.inline.sources = ["rag_1"] + config_mock.configuration.rag.retrieval.inline.max_chunks = ( + constants.INLINE_RAG_MAX_CHUNKS + ) + config_mock.configuration.rag.byok.max_chunks = constants.BYOK_RAG_MAX_CHUNKS + config_mock.configuration.rag.okp.max_chunks = constants.OKP_RAG_MAX_CHUNKS + config_mock.byok_rag = [byok_rag_mock] config_mock.inline_solr_enabled = False config_mock.score_multiplier_mapping = {"vs_1": 1.0} config_mock.rag_id_mapping = {"vs_1": "rag_1"}