diff --git a/Containerfile b/Containerfile index 16f957389..33af34380 100644 --- a/Containerfile +++ b/Containerfile @@ -83,9 +83,11 @@ ENV PYTHONDONTWRITEBYTECODE=1 \ PYTHONUTF8=1 \ PYTHONIOENCODING=UTF-8 \ LANG=en_US.UTF-8 \ - LLAMA_INDEX_CACHE_DIR=/tmp/llama_index \ + LLAMA_INDEX_CACHE_DIR=/app-root/.cache/llama_index \ TIKTOKEN_CACHE_DIR=/app-root/.tiktoken_cache \ - HF_HOME=/app-root/.cache/huggingface + HF_HOME=/app-root/.cache/huggingface \ + HF_HUB_OFFLINE=1 \ + TRANSFORMERS_OFFLINE=1 COPY --from=builder /app-root/.venv .venv COPY --from=builder /app-root/.tiktoken_cache .tiktoken_cache @@ -95,9 +97,11 @@ COPY --from=lightspeed-rag-content /rag/vector_db/ocp_product_docs ./vector_db/o COPY --chmod=775 embeddings_model ./embeddings_model RUN if [ -d /cachi2/output/deps/generic ]; then \ cp /cachi2/output/deps/generic/all-mpnet-base-v2-model.safetensors embeddings_model/all-mpnet-base-v2/model.safetensors ; \ + cp /cachi2/output/deps/generic/granite-embedding-30m-english-model.safetensors embeddings_model/granite-embedding-30m-english/model.safetensors ; \ fi && \ for f in \ "embeddings_model/all-mpnet-base-v2/model.safetensors|https://huggingface.co/sentence-transformers/all-mpnet-base-v2/resolve/main/model.safetensors" \ + "embeddings_model/granite-embedding-30m-english/model.safetensors|https://huggingface.co/ibm-granite/granite-embedding-30m-english/resolve/main/model.safetensors" \ ; do \ path="${f%%|*}" && url="${f##*|}" && \ if [ ! -f "$path" ]; then \ @@ -107,16 +111,22 @@ RUN if [ -d /cachi2/output/deps/generic ]; then \ { echo "ERROR: corrupt safetensors file: $path" ; exit 1 ; } ; \ done -# Pre-populate HuggingFace cache so models can be loaded by ID with TRANSFORMERS_OFFLINE=1 +# Pre-populate HuggingFace cache so models can be loaded by ID with TRANSFORMERS_OFFLINE=1. +# Two cache trees are needed: the standard HF hub cache (used by huggingface_hub and +# transformers when no cache_dir override is given) and the llama_index cache (passed as +# cache_folder to SentenceTransformer by llama_index's HuggingFaceEmbedding wrapper). USER root RUN for model_dir in all-mpnet-base-v2 granite-embedding-30m-english; do \ case "$model_dir" in \ all-mpnet-base-v2) hf_id="sentence-transformers--all-mpnet-base-v2" ;; \ + granite-embedding-30m-english) hf_id="ibm-granite--granite-embedding-30m-english" ;; \ esac && \ - repo_dir="/app-root/.cache/huggingface/hub/models--${hf_id}" && \ - mkdir -p "$repo_dir/snapshots/local" "$repo_dir/refs" && \ - echo "local" > "$repo_dir/refs/main" && \ - ln -sf "/app-root/embeddings_model/${model_dir}"/* "$repo_dir/snapshots/local/" ; \ + for cache_root in /app-root/.cache/huggingface/hub /app-root/.cache/llama_index; do \ + repo_dir="${cache_root}/models--${hf_id}" && \ + mkdir -p "$repo_dir/snapshots/local" "$repo_dir/refs" && \ + printf '%s' "local" > "$repo_dir/refs/main" && \ + ln -sf "/app-root/embeddings_model/${model_dir}"/* "$repo_dir/snapshots/local/" ; \ + done ; \ done && \ chown -R 1001:0 /app-root/.cache diff --git a/artifacts.lock.yaml b/artifacts.lock.yaml index abbeff262..d1a6c1289 100644 --- a/artifacts.lock.yaml +++ b/artifacts.lock.yaml @@ -5,6 +5,6 @@ artifacts: - download_url: "https://huggingface.co/sentence-transformers/all-mpnet-base-v2/resolve/main/model.safetensors" checksum: "sha256:78c0197b6159d92658e319bc1d72e4c73a9a03dd03815e70e555c5ef05615658" filename: "all-mpnet-base-v2-model.safetensors" - # - download_url: "https://huggingface.co/ibm-granite/granite-embedding-30m-english/resolve/main/model.safetensors" - # checksum: "sha256:3b1fcdc9c5eb954f603bc386474e321505ff29c6c67f21e3aa8db3d2d1a533cf" - # filename: "granite-embedding-30m-english-model.safetensors" + - download_url: "https://huggingface.co/ibm-granite/granite-embedding-30m-english/resolve/main/model.safetensors" + checksum: "sha256:3b1fcdc9c5eb954f603bc386474e321505ff29c6c67f21e3aa8db3d2d1a533cf" + filename: "granite-embedding-30m-english-model.safetensors" diff --git a/docs/ai/config.md b/docs/ai/config.md index a2ad2fc48..9d8cd4584 100644 --- a/docs/ai/config.md +++ b/docs/ai/config.md @@ -103,6 +103,89 @@ Config classes have two validation phases: If you add a new config class that needs file existence checks or cross-section validation, add a `validate_yaml()` method and call it from the appropriate parent config's `validate_yaml()`. +### Optional Solr hybrid RAG (`ols_config.solr_hybrid`) + +Omit the key to leave Solr hybrid RAG off (no client, no tool). When present, values map to `SolrHybridSettings` and the feature is active. `solr_http_base` must be a valid `http` or `https` URL with a host (checked in `validate_yaml()`). + +When ``solr_hybrid`` is defined, every entry under ``reference_content.indexes`` +must include ``byok_index: true`` if any local indexes are configured. That rejects +Solr together with an unmarked local vector index (duplicate product RAG); omit +``indexes`` for Solr-only, or mark BYOK indexes explicitly. + +```yaml +ols_config: + solr_hybrid: + solr_http_base: "https://solr.example.com:8983" + # max_results, hybrid_*, max_expansion_neighbors: optional overrides +``` + +Do not duplicate the same product documentation in Solr and a local product index. +You may use ``solr_hybrid`` for product docs and ``reference_content`` for separate +BYOK indexes; each BYOK row must set ``byok_index: true`` when ``solr_hybrid`` is present. + +### Hybrid search and chunk expansion + +OLS sends a hybrid query to OKP Solr's ``/hybrid-search`` endpoint. The query +performs keyword retrieval first, then reranks candidates by KNN vector +similarity. See the +[OKP RAG Chunk Retrieval Strategy](https://docs.google.com/document/d/1W7G3Tbz5peMAh8cnGcpKvayQAciAyDXzXY_5KXdkY_0/edit?tab=t.0) +for the full specification. + +Solr returns raw matched chunks. OLS implements chunk expansion client-side +(all logic in ``SolrHybridSearch`` in ``ols/src/rag_index/solr_support.py``): + +1. **Deduplicate by parent** — keep only the first (highest-scored) chunk per + ``parent_id``. Chunks without a ``parent_id`` are kept as-is. +2. **Cap results** — slice the deduped list to ``max_results``. +3. **Compute per-chunk budget** — divide the tool's ``tools_token_budget`` + (set by the execution framework via ``tool.metadata``) equally across the + deduped chunks. When the budget is 0, skip expansion entirely and return + the matched chunk as-is. +4. **Fetch family** — for each matched chunk, query Solr ``/select`` for all + chunks sharing the same ``parent_id`` AND ``heading_id`` (ordered by + ``chunk_index``). Orphan chunks (missing ``heading_id``) skip this step. +5. **Expand around match** — starting from the matched chunk, alternate + between previous and next siblings until one of these limits is hit: + - per-chunk token budget exhausted (tracked via ``num_tokens``) + - ``max_expansion_neighbors`` reached on each side (default 2, configurable + 0–10 in ``SolrHybridSettings``; 0 disables expansion) +6. **Concatenate** — assemble the expanded chunks in ``chunk_index`` order, + strip HTML, and return as the tool result (one content block per deduped + match). + +Relevant Solr fields for expansion: + +| Field | Purpose | +|---|---| +| ``parent_id`` | Groups chunks by source document | +| ``chunk_index`` | Sequential ordering for neighbor expansion | +| ``heading_id`` | Family grouping (chunks under the same heading) | +| ``num_tokens`` | Token count per chunk for budget tracking | + +Relevant config fields on ``SolrHybridSettings``: + +| Field | Default | Purpose | +|---|---|---| +| ``max_results`` | 5 | Max deduped chunks returned | +| ``max_expansion_neighbors`` | 2 | Max siblings per side during expansion (0 disables) | + +### OCP version resolution at startup + +When ``solr_hybrid`` is configured, ``SolrHybridSearch.__init__`` resolves the +OCP product version for ``chunk_filter_query``: + +1. Read the ``OCP_CLUSTER_VERSION`` environment variable (set by the operator). + If not set, raise ``InvalidConfigurationError`` and stop. +2. Query Solr for available versions of ``openshift_container_platform`` + (facet on ``product_version`` with ``fq=product:openshift_container_platform``). +3. Clamp the requested version to the nearest available: + - env version < lowest available → use lowest available + - env version > highest available → use highest available + - otherwise → use the closest available version ≤ requested +4. Build ``chunk_filter_query``: + ``is_chunk:true AND product:openshift_container_platform AND product_version:`` + + ## Important Constants Config-related constants live in `ols/constants.py`: diff --git a/docs/config.puml b/docs/config.puml index 3da81d913..e03ea3944 100644 --- a/docs/config.puml +++ b/docs/config.puml @@ -180,13 +180,14 @@ class "RHOAIVLLMConfig" as ols.app.models.config.RHOAIVLLMConfig { credentials_path : str } class "ReferenceContent" as ols.app.models.config.ReferenceContent { - embeddings_model_path : Optional[FilePath] indexes : Optional[list[ReferenceContentIndex]] validate_yaml() -> None } class "ReferenceContentIndex" as ols.app.models.config.ReferenceContentIndex { product_docs_index_id : Optional[str] product_docs_index_path : Optional[FilePath] + product_docs_origin : Optional[str] + byok_index : bool validate_yaml() -> None } class "SchedulerConfig" as ols.app.models.config.SchedulerConfig { diff --git a/examples/olsconfig-local-ollama.yaml b/examples/olsconfig-local-ollama.yaml index 90679c833..281f75328 100644 --- a/examples/olsconfig-local-ollama.yaml +++ b/examples/olsconfig-local-ollama.yaml @@ -19,9 +19,9 @@ llm_providers: ols_config: # max_workers: 1 reference_content: -# product_docs_index_path: "./vector_db/ocp_product_docs/4.15" -# product_docs_index_id: ocp-product-docs-4_15 -# embeddings_model_path: "./embeddings_model" +# indexes: +# - product_docs_index_path: "./vector_db/ocp_product_docs/4.15" +# product_docs_index_id: ocp-product-docs-4_15 conversation_cache: type: memory memory: diff --git a/examples/olsconfig.yaml b/examples/olsconfig.yaml index 713c1cef1..4c66604c9 100644 --- a/examples/olsconfig.yaml +++ b/examples/olsconfig.yaml @@ -75,13 +75,16 @@ llm_providers: - name: model-name ols_config: # max_workers: 1 + # Product docs via Solr hybrid (OKP). Omit solr_hybrid to disable. + solr_hybrid: + solr_http_base: "https://solr.example.com:8983" + # BYOK / local FAISS indexes. + # With solr_hybrid present, every index must set byok_index: true. reference_content: indexes: - - product_docs_index_path: "./vector_db/ocp_product_docs/4.17" - product_docs_index_id: ocp-product-docs-4_17 - product_docs_index_path: "./vector_db/user_application_docs/version_1" product_docs_index_id: user-application-docs-version_1 - embeddings_model_path: "./embeddings_model/all-mpnet-base-v2" + byok_index: true conversation_cache: type: memory memory: diff --git a/examples/openshift-lightspeed-tls.yaml b/examples/openshift-lightspeed-tls.yaml index 994d3fffc..2266f4fb7 100644 --- a/examples/openshift-lightspeed-tls.yaml +++ b/examples/openshift-lightspeed-tls.yaml @@ -213,9 +213,9 @@ data: ols_config: max_workers: 1 reference_content: - product_docs_index_path: "./vector_db/ocp_product_docs/4.15" - product_docs_index_id: ocp-product-docs-4_15 - embeddings_model_path: "./embeddings_model" + indexes: + - product_docs_index_path: "./vector_db/ocp_product_docs/4.15" + product_docs_index_id: ocp-product-docs-4_15 conversation_cache: type: memory memory: diff --git a/ols/app/endpoints/ols.py b/ols/app/endpoints/ols.py index fe61eeec8..4ba9c2d78 100644 --- a/ols/app/endpoints/ols.py +++ b/ols/app/endpoints/ols.py @@ -541,17 +541,22 @@ def generate_response( streaming=streaming, audit_ctx=audit_ctx, ) + rag_retriever = ( + config.rag_index_loader.get_retriever() + if config.ols_config.reference_content is not None + else None + ) if streaming: return docs_summarizer.generate_response( llm_request.query, - config.rag_index_loader.get_retriever(), + rag_retriever, user_id=user_id, conversation_id=conversation_id, skip_user_id_check=skip_user_id_check, ) response = docs_summarizer.create_response( llm_request.query, - config.rag_index_loader.get_retriever(), + rag_retriever, user_id=user_id, conversation_id=conversation_id, skip_user_id_check=skip_user_id_check, diff --git a/ols/app/models/config.py b/ols/app/models/config.py index 39ff948ce..d5f75cd7c 100644 --- a/ols/app/models/config.py +++ b/ols/app/models/config.py @@ -9,6 +9,7 @@ from pydantic import ( AnyHttpUrl, BaseModel, + ConfigDict, Field, FilePath, PositiveInt, @@ -713,11 +714,6 @@ class ToolFilteringConfig(BaseModel): If this config is present, tool filtering is enabled. If absent, all tools are used. """ - embed_model_path: Optional[str] = Field( - default=None, - description="Path to sentence transformer model for embeddings", - ) - alpha: float = Field( default=0.8, ge=0.0, @@ -748,11 +744,6 @@ class SkillsConfig(BaseModel): description="Path to directory containing skill subdirectories", ) - embed_model_path: Optional[str] = Field( - default=None, - description="Path to sentence transformer model for embeddings", - ) - alpha: float = Field( default=0.8, ge=0.0, @@ -984,11 +975,19 @@ def __init__(self, **data: Any) -> None: class ReferenceContentIndex(BaseModel): - """Reference content index configuration.""" + """One on-disk FAISS / vector index (e.g. BYOK or other local RAG stores). + + Field names are historical: ``product_docs_*`` is used for any persisted + LlamaIndex vector store path, not only shipped product documentation. Managed + OpenShift product docs are typically retrieved via ``ols_config.solr_hybrid`` + instead of a second index here. When ``solr_hybrid`` is enabled, any local index + listed alongside it must set ``byok_index: true`` (see ``OLSConfig.validate_yaml``). + """ product_docs_index_path: Optional[FilePath] = None product_docs_index_id: Optional[str] = None product_docs_origin: Optional[str] = None + byok_index: bool = False def __init__(self, data: Optional[dict] = None) -> None: """Initialize configuration and perform basic validation.""" @@ -998,6 +997,7 @@ def __init__(self, data: Optional[dict] = None) -> None: self.product_docs_index_path = data.get("product_docs_index_path", None) self.product_docs_index_id = data.get("product_docs_index_id", None) self.product_docs_origin = data.get("product_docs_origin", None) + self.byok_index = bool(data.get("byok_index", False)) def validate_yaml(self) -> None: """Validate reference content index config.""" @@ -1028,9 +1028,12 @@ def validate_yaml(self) -> None: class ReferenceContent(BaseModel): - """Reference content configuration.""" + """Local vector indexes (FAISS on disk) for BYOK. + + Omit or leave ``indexes`` empty when no on-disk indexes are needed. + Product documentation is served by Solr (see ``solr_hybrid``). + """ - embeddings_model_path: Optional[FilePath] = None indexes: Optional[list[ReferenceContentIndex]] = None def __init__(self, data: Optional[dict] = None) -> None: @@ -1039,7 +1042,6 @@ def __init__(self, data: Optional[dict] = None) -> None: if data is None: return - self.embeddings_model_path = data.get("embeddings_model_path", None) if "indexes" in data: self.indexes = [ReferenceContentIndex(i) for i in data["indexes"]] else: @@ -1047,13 +1049,87 @@ def __init__(self, data: Optional[dict] = None) -> None: def validate_yaml(self) -> None: """Validate reference content config.""" - if self.embeddings_model_path is not None: - checks.dir_check(self.embeddings_model_path, "Embeddings model path") if self.indexes is not None: for index in self.indexes: index.validate_yaml() +class SolrHybridSettings(BaseModel): + """Pydantic container for Solr hybrid RAG (portal-rag ``/hybrid-search``). + + Holds the Solr HTTP base URL, ranked hit count, optional ``fq`` filter, hybrid + rerank pool and vector weight, optional post-score cutoff, and HTTP client timeout. + + Presence of the ``solr_hybrid`` section in the config enables the feature; + omit the section entirely to disable it. + """ + + model_config = ConfigDict(extra="forbid") + + solr_http_base: str = Field( + default="http://localhost:8080", + description="Solr base URL without trailing /solr.", + ) + max_results: int = Field( + default=constants.RAG_CONTENT_LIMIT, + ge=1, + le=50, + description=( + "Target number of passages to return after parent dedupe; matches the " + "default index retriever chunk cap (``ols.constants.RAG_CONTENT_LIMIT`` / " + "``similarity_top_k``). Also scales how many rows Solr fetches for the hybrid " + "request pool before reranking." + ), + ) + hybrid_vector_boost: float = Field( + default=8.0, + ge=0.0, + description=( + "Solr rerank vector weight (``reRankWeight``): relative emphasis of dense " + "vector score versus lexical relevance in the hybrid reranker." + ), + ) + hybrid_pool_docs: int = Field( + default=100, + ge=1, + le=500, + description=( + "Candidate document pool size (``reRankDocs``) passed to Solr's " + "``{!rerank}`` query for hybrid reranking." + ), + ) + hybrid_score_threshold: float = Field( + default=0.0, + ge=0.0, + description=( + "Drop hits whose hybrid ``score`` is below this value after retrieval; " + "use ``0`` to keep all Solr-ranked docs." + ), + ) + hybrid_solr_timeout_s: float = Field( + default=60.0, + ge=5.0, + description="Total HTTP timeout in seconds for each hybrid-search request.", + ) + max_expansion_neighbors: int = Field( + default=2, + ge=0, + le=10, + description=( + "Maximum number of sibling chunks to include on each side of the " + "matched chunk during chunk expansion. ``0`` disables expansion." + ), + ) + + def validate_yaml(self) -> None: + """Validate Solr hybrid settings.""" + if not checks.is_valid_http_url(self.solr_http_base): + raise checks.InvalidConfigurationError( + "solr_hybrid.solr_http_base must be a valid http or https URL with a " + f"host; got {self.solr_http_base!r}" + ) + + class UserDataCollection(BaseModel): """User data collection configuration.""" @@ -1221,11 +1297,13 @@ class OLSConfig(BaseModel): audit: AuditConfig = AuditConfig() + solr_hybrid: Optional[SolrHybridSettings] = None + tool_round_cap_fraction: float = constants.DEFAULT_TOOL_ROUND_CAP_FRACTION offload_storage_path: str = constants.DEFAULT_OFFLOAD_STORAGE_PATH - def __init__( + def __init__( # noqa: C901 self, data: Optional[dict] = None, ignore_missing_certs: bool = False ) -> None: """Initialize configuration and perform basic validation.""" @@ -1283,6 +1361,8 @@ def __init__( self.tools_approval = ToolsApprovalConfig(**data.get("tools_approval")) if data.get("skills", None) is not None: self.skills = SkillsConfig(**data.get("skills")) + if data.get("solr_hybrid", None) is not None: + self.solr_hybrid = SolrHybridSettings(**data.get("solr_hybrid")) self.audit = AuditConfig(**data.get("audit", {})) @@ -1332,6 +1412,26 @@ def validate_yaml(self, disable_tls: bool = False) -> None: self.authentication_config.validate_yaml() if self.proxy_config is not None: self.proxy_config.validate_yaml() + if self.solr_hybrid is not None: + self.solr_hybrid.validate_yaml() + self._validate_solr_hybrid_vs_reference_indexes() + + def _validate_solr_hybrid_vs_reference_indexes(self) -> None: + """Reject Solr hybrid together with local indexes that are not BYOK-only.""" + solr = self.solr_hybrid + if solr is None: + return + rc = self.reference_content + if rc is None or not rc.indexes: + return + if any(not idx.byok_index for idx in rc.indexes): + raise checks.InvalidConfigurationError( + "ols_config.solr_hybrid is enabled together with " + "ols_config.reference_content.indexes entries that are not marked " + "BYOK-only. Remove local product vector indexes when using Solr, or set " + "byok_index: true on each index that is intentionally kept with Solr " + "(e.g. BYOK)." + ) class DevConfig(BaseModel): @@ -1432,10 +1532,12 @@ def _validate_mcp_servers(self) -> None: def _compute_tool_budgets(self) -> None: """Set tool token budget per model and ensure the context window fits reserved tokens.""" - has_mcp = bool(self.mcp_servers.servers) + reserve_tool_budget = ( + bool(self.mcp_servers.servers) or self.ols_config.solr_hybrid is not None + ) for provider in self.llm_providers.providers.values(): for model in provider.models.values(): - if has_mcp: + if reserve_tool_budget: model.max_tokens_for_tools = int( model.context_window_size * model.parameters.tool_budget_ratio ) diff --git a/ols/constants.py b/ols/constants.py index 724c3afae..89a8611a3 100644 --- a/ols/constants.py +++ b/ols/constants.py @@ -87,8 +87,8 @@ class GenericLLMParameters: # Example: 1.05 means we increase by 5%. TOKEN_BUFFER_WEIGHT = 1.1 -# Fraction of context window reserved for tool outputs (only when MCP servers configured). -# Computed as int(context_window_size * ratio) at startup. +# Fraction of context window reserved for tool outputs when MCP servers or +# Solr hybrid search is configured. Computed as int(context_window_size * ratio) at startup. DEFAULT_TOOL_BUDGET_RATIO = 0.5 TOOL_BUDGET_RATIO_MIN = 0.1 TOOL_BUDGET_RATIO_MAX = 0.6 @@ -123,6 +123,10 @@ class GenericLLMParameters: # Range: 0 to 1 RAG_SIMILARITY_CUTOFF = 0.3 +# Solr hybrid (OKP ``portal-rag`` / ``hybrid-search``) query embeddings must match the +# vectors stored in the index (same default as solr-experiment / solr_vector_io). +SOLR_HYBRID_EMBEDDING_MODEL_ID = "ibm-granite/granite-embedding-30m-english" + # cache constants CACHE_TYPE_MEMORY = "memory" diff --git a/ols/src/prompts/prompt_generator.py b/ols/src/prompts/prompt_generator.py index 94b113dfe..0f6600124 100644 --- a/ols/src/prompts/prompt_generator.py +++ b/ols/src/prompts/prompt_generator.py @@ -32,6 +32,7 @@ def __init__( mode: QueryMode = QueryMode.ASK, cluster_version: str = "unknown", skill_content: Optional[str] = None, + solr_docs_tool_guidance: bool = False, ) -> None: """Initialize prompt generator.""" self._query = query @@ -42,6 +43,7 @@ def __init__( self._mode = mode self._cluster_version = cluster_version self._skill_content = skill_content + self._solr_docs_tool_guidance = solr_docs_tool_guidance def _get_agent_instructions(self, model: str) -> str: """Return agent instructions based on mode and model family.""" @@ -68,6 +70,12 @@ def generate_prompt(self, model: str) -> tuple[ChatPromptTemplate, dict]: if self._tool_call: agent_instructions = self._get_agent_instructions(model) + if self._solr_docs_tool_guidance and self._mode == QueryMode.ASK: + agent_instructions = ( + agent_instructions + + "\n" + + prompts.SOLR_DOCS_TOOL_SUPPLEMENT.strip() + ) sys_intruction = sys_intruction + "\n" + agent_instructions if len(self._rag_context) > 0: diff --git a/ols/src/prompts/prompts.py b/ols/src/prompts/prompts.py index e0ac1f7a5..458e0deaf 100644 --- a/ols/src/prompts/prompts.py +++ b/ols/src/prompts/prompts.py @@ -85,6 +85,18 @@ * Terseness must not omit critical info. """ +SOLR_DOCS_TOOL_SUPPLEMENT = """ +Solr docs tool: +* ``search_openshift_documentation`` searches published Red Hat product documentation (not live cluster resources). Prefer one short query before answering from memory on product behavior, APIs, operators, upgrades, or whether docs exist. +* Ground factual claims on returned passages when they help; if results are empty or off-topic, say so briefly, then you may use general knowledge. +* When you use a passage, cite its ``title`` and ``docs_url`` from the tool JSON. Never invent documentation URLs. + +Grounded answers (passages from ``search_openshift_documentation``): +* Bullets or numbered steps are allowed so passage detail is not dropped for brevity. +* Keep concrete details from passages (manifests, ``oc`` commands, console paths). +* If the answer is not based on such passages, stay concise per the general style rules above. +""" + TROUBLESHOOTING_SYSTEM_INSTRUCTION = """# ROLE You are "OpenShift Lightspeed", an AI assistant specializing in OpenShift troubleshooting and diagnostics. diff --git a/ols/src/query_helpers/docs_summarizer.py b/ols/src/query_helpers/docs_summarizer.py index b0365d037..94866e55b 100644 --- a/ols/src/query_helpers/docs_summarizer.py +++ b/ols/src/query_helpers/docs_summarizer.py @@ -28,6 +28,7 @@ log_tool_loop_iteration, ) from ols.src.query_helpers.query_helper import QueryHelper +from ols.src.rag_index.solr_support import get_openshift_docs_tool from ols.src.skills.skills_rag import create_skill_support_tool from ols.src.tools.offloaded_content import OffloadManager from ols.utils.audit_logger import AuditContext @@ -91,12 +92,21 @@ def __init__( self.mcp_servers = build_mcp_config( config.mcp_servers.servers, self.user_token, self.client_headers ) + self._solr_hybrid = config.ols_config.solr_hybrid + self._solr_client = config.solr_hybrid_search + solr_docs_tool_active = ( + self._solr_hybrid is not None and self._solr_client is not None + ) + self._solr_docs_tool_prompt_guidance = solr_docs_tool_active + self._tool_calling_enabled = bool(self.mcp_servers) or solr_docs_tool_active if self.mcp_servers: logger.info("MCP servers provided: %s", list(self.mcp_servers.keys())) - self._tool_calling_enabled = True + elif self._tool_calling_enabled: + logger.info( + "Tool calling enabled for Solr hybrid OpenShift documentation search" + ) else: logger.debug("No MCP servers provided, tool calling is disabled") - self._tool_calling_enabled = False self._tracker = TokenBudgetTracker( token_handler=TokenHandler(), @@ -120,6 +130,17 @@ def __init__( audit_ctx=self._audit_ctx, ) + async def _resolve_tools_for_request( + self, mcp_tools_query: str + ) -> list[StructuredTool]: + """Load MCP tools and append built-in Solr docs search when configured for tool-only.""" + tools = await get_mcp_tools( + mcp_tools_query, self.user_token, self.client_headers + ) + if self._solr_hybrid is not None and self._solr_client is not None: + tools.append(get_openshift_docs_tool(self._solr_client)) + return tools + def _prepare_llm(self) -> None: """Prepare the LLM configuration.""" self.provider_config = config.llm_config.providers.get(self.provider) @@ -155,6 +176,7 @@ async def _prepare_prompt_context( self._tool_calling_enabled, self._mode, self._cluster_version, + solr_docs_tool_guidance=self._solr_docs_tool_prompt_guidance, ).generate_prompt(self.model) prompt_tokens = self._tracker.count_tokens( temp_prompt.format(**temp_prompt_input) @@ -175,7 +197,10 @@ async def _prepare_prompt_context( ) with rag_span as span: retrieved_nodes = rag_retriever.retrieve(query) - logger.info("Retrieved %d documents from indexes", len(retrieved_nodes)) + logger.info( + "Retrieved %d document nodes for RAG context", + len(retrieved_nodes), + ) for i, node in enumerate(retrieved_nodes[:5]): logger.info( @@ -263,6 +288,7 @@ def _build_final_prompt( self._mode, self._cluster_version, skill_content=skill_content, + solr_docs_tool_guidance=self._solr_docs_tool_prompt_guidance, ).generate_prompt(self.model) log_tool_loop_iteration( @@ -420,9 +446,7 @@ async def generate_response( # noqa: C901 # pylint: disable=too-many-branches, messages = final_prompt.model_copy() mcp_tools_query = f"{skill_content}\n\n{query}" if skill_content else query - all_mcp_tools = await get_mcp_tools( - mcp_tools_query, self.user_token, self.client_headers - ) + all_mcp_tools = await self._resolve_tools_for_request(mcp_tools_query) if skill is not None and skill_content is not None and has_support_files: all_mcp_tools.append(create_skill_support_tool(skill)) tool_definitions_text = self._serialized_tool_definitions_text(all_mcp_tools) diff --git a/ols/src/rag/hybrid_rag.py b/ols/src/rag/hybrid_rag.py index c7a2186ff..3698ef49d 100644 --- a/ols/src/rag/hybrid_rag.py +++ b/ols/src/rag/hybrid_rag.py @@ -17,111 +17,15 @@ ) from rank_bm25 import BM25Okapi -_NON_ALPHA = re.compile(r"[^a-z0-9\s]") +from ols.src.rag.stop_words import ENGLISH_STOP_WORDS -# Subset of NLTK's English stop-word list, inlined to avoid the dependency. -_STOP_WORDS = frozenset( - { - "a", - "an", - "the", - "and", - "or", - "but", - "in", - "on", - "at", - "to", - "for", - "of", - "with", - "by", - "from", - "is", - "are", - "was", - "were", - "be", - "been", - "being", - "have", - "has", - "had", - "do", - "does", - "did", - "will", - "would", - "could", - "should", - "may", - "might", - "shall", - "can", - "need", - "dare", - "it", - "its", - "this", - "that", - "these", - "those", - "i", - "we", - "you", - "he", - "she", - "they", - "me", - "him", - "her", - "us", - "them", - "my", - "our", - "your", - "his", - "their", - "what", - "which", - "who", - "whom", - "how", - "when", - "where", - "why", - "not", - "no", - "nor", - "so", - "if", - "then", - "than", - "too", - "very", - "just", - "about", - "above", - "after", - "before", - "between", - "into", - "out", - "up", - "down", - "over", - "under", - "again", - "further", - "once", - } -) +_NON_ALPHA = re.compile(r"[^a-z0-9\s]") def _tokenize(text: str) -> list[str]: """Lowercase, strip punctuation, remove stop words, and split.""" tokens = _NON_ALPHA.sub("", text.lower()).split() - return [t for t in tokens if t not in _STOP_WORDS] + return [t for t in tokens if t not in ENGLISH_STOP_WORDS] class QdrantStore: diff --git a/ols/src/rag/stop_words.py b/ols/src/rag/stop_words.py new file mode 100644 index 000000000..925074740 --- /dev/null +++ b/ols/src/rag/stop_words.py @@ -0,0 +1,99 @@ +"""English stop words for hybrid RAG tokenization and Solr query normalization.""" + +# Subset of NLTK's English stop-word list, inlined to avoid the dependency. +ENGLISH_STOP_WORDS: frozenset[str] = frozenset( + { + "a", + "an", + "the", + "and", + "or", + "but", + "in", + "on", + "at", + "to", + "for", + "of", + "with", + "by", + "from", + "is", + "are", + "was", + "were", + "be", + "been", + "being", + "have", + "has", + "had", + "do", + "does", + "did", + "will", + "would", + "could", + "should", + "may", + "might", + "shall", + "can", + "need", + "dare", + "it", + "its", + "this", + "that", + "these", + "those", + "i", + "we", + "you", + "he", + "she", + "they", + "me", + "him", + "her", + "us", + "them", + "my", + "our", + "your", + "his", + "their", + "what", + "which", + "who", + "whom", + "how", + "when", + "where", + "why", + "not", + "no", + "nor", + "so", + "if", + "then", + "than", + "too", + "very", + "just", + "about", + "above", + "after", + "before", + "between", + "into", + "out", + "up", + "down", + "over", + "under", + "again", + "further", + "once", + } +) diff --git a/ols/src/rag_index/index_loader.py b/ols/src/rag_index/index_loader.py index d4f7aa757..609750f5e 100644 --- a/ols/src/rag_index/index_loader.py +++ b/ols/src/rag_index/index_loader.py @@ -2,9 +2,10 @@ """Module for loading index.""" import logging -from pathlib import Path from typing import Any, Optional +from llama_index.embeddings.huggingface import HuggingFaceEmbedding + from ols.app.models.config import ReferenceContent from ols.constants import EMBEDDINGS_MODEL_BYOK_SUBDIR, RAG_CONTENT_LIMIT @@ -133,51 +134,14 @@ def __init__(self, index_config: Optional[ReferenceContent]) -> None: elif self._index_config.indexes is None or len(self._index_config.indexes) == 0: logger.warning("Indexes are not set in the config for reference content.") else: - - self._embed_model_path = self._resolve_embeddings_path( - self._index_config.embeddings_model_path - ) self._embed_model = self._get_embed_model() self._load_index() @staticmethod - def _resolve_embeddings_path(path: object) -> Optional[str]: - """Resolve embeddings model path to an actual model directory. - - If the given path does not contain a config.json (i.e. it is a parent - directory holding model subdirectories), look for the default model - subdirectory instead. - """ - if path is None: - return None - path = Path(str(path)) - config_file = path / "config.json" - if config_file.is_file(): - return str(path) - candidate = path / EMBEDDINGS_MODEL_BYOK_SUBDIR - if (candidate / "config.json").is_file(): - logger.info( - "Resolved embeddings path %s -> %s", - path, - candidate, - ) - return str(candidate) - return str(path) - - def _get_embed_model(self) -> Any: - """Get embed model according to configuration.""" - if self._embed_model_path is not None: - # pylint: disable=C0415 - from llama_index.embeddings.huggingface import HuggingFaceEmbedding - - logger.debug( - "Loading embedding model info from path %s", self._embed_model_path - ) - return HuggingFaceEmbedding(model_name=self._embed_model_path) - - logger.warning("Embedding model path is not set.") - logger.warning("Embedding model is set to default") - return "local:sentence-transformers/all-mpnet-base-v2" + def _get_embed_model() -> HuggingFaceEmbedding: + """Load the bundled BYOK embedding model.""" + logger.debug("Loading embedding model: %s", EMBEDDINGS_MODEL_BYOK_SUBDIR) + return HuggingFaceEmbedding(model_name=EMBEDDINGS_MODEL_BYOK_SUBDIR) def _load_index(self) -> None: """Load vector index.""" diff --git a/ols/src/rag_index/solr_support.py b/ols/src/rag_index/solr_support.py new file mode 100644 index 000000000..6584b59f4 --- /dev/null +++ b/ols/src/rag_index/solr_support.py @@ -0,0 +1,751 @@ +"""Solr hybrid RAG client for OLS (portal-rag / hybrid-search). + +Runs ``POST /hybrid-search`` with **lexical-primary** edismax ``q`` and ``{!rerank}`` +where ``rqq`` is ``{!knn f=chunk_vector …}[vector]`` (SolrVectorIO / OKP shape). A +KNN-first ``q`` can return empty ``response.docs`` when the vector leg finds no +neighbors even though lexical would match. + +Results are **deduped by parent** (first hit per ``parent_id`` / ``id``), then +truncated to ``SolrHybridSettings.max_results``. +""" + +from __future__ import annotations + +import asyncio +import html as html_mod +import json +import logging +import os +import re +import time +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any + +import httpx +from langchain_core.tools.structured import StructuredTool +from packaging.version import InvalidVersion, Version + +from ols.src.rag.stop_words import ENGLISH_STOP_WORDS +from ols.utils.checks import InvalidConfigurationError + +if TYPE_CHECKING: + from collections.abc import Callable + + from ols.app.models.config import SolrHybridSettings + +logger = logging.getLogger(__name__) + +_ACCESS_BASE = "https://access.redhat.com" + +_SOLR_COLLECTION = "portal-rag" +_SOLR_VECTOR_FIELD = "chunk_vector" +_SOLR_CHUNK_TEXT_FIELD = "chunk" + +_RAG_CHUNK_EDISMAX_QF = "chunk^3 title^4 headings^2 resourceName^1 product^0.5" +_RAG_CHUNK_EDISMAX_MM = "2<-1 5<75% 10<50%" + +_RAG_CHUNK_LEXICAL_HEADROOM_MULT = 5 +_RAG_CHUNK_LEXICAL_MIN_ROWS = 15 +_RAG_CHUNK_LEXICAL_MAX_ROWS = 80 + +_MAX_FAMILY_CHUNKS = 50 + +_TERM_TRIM_CHARS = "?.,!" +_IP_CIDR_RE = re.compile(r"^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}(?:/\d{1,2})?$") +_NIC_NAME_RE = re.compile(r"^(?:ens|enp|eth|em)\d", re.IGNORECASE) + + +def _safe_solr_score(value: Any) -> float: + """Coerce Solr ``score`` field to float; missing or invalid values become ``0.0``.""" + if value is None: + return 0.0 + try: + return float(value) + except (TypeError, ValueError): + return 0.0 + + +def _solr_response_json(response: httpx.Response, *, log_url: str) -> dict[str, Any]: + """Parse Solr JSON body; return empty dict on decode failure or non-object root.""" + try: + data = response.json() + except ValueError: + logger.exception("Solr response JSON decode failed for %s", log_url) + return {} + return data if isinstance(data, dict) else {} + + +def _split_quoted_and_plain(text: str) -> list[str]: + """Split text on whitespace while keeping double-quoted spans as single tokens. + + Args: + text: Raw user input. + + Returns: + Ordered tokens; quoted phrases appear as ``"..."`` elements. + """ + tokens: list[str] = [] + remainder = text + while '"' in remainder: + before, _, rest = remainder.partition('"') + tokens.extend(before.split()) + if '"' not in rest: + tokens.extend(rest.split()) + remainder = "" + break + phrase, _, remainder = rest.partition('"') + if phrase: + tokens.append(f'"{phrase}"') + tokens.extend(remainder.split()) + return tokens + + +def normalize_solr_hybrid_query(query: str) -> str: + """Normalize user text for Solr hybrid lexical and embedding paths. + + Strip stopwords and quote hyphenated compounds for stable Solr matching. + """ + tokens = _split_quoted_and_plain(query) + parts: list[str] = [] + for t in tokens: + if t.startswith('"'): + parts.append(t) + continue + stripped = t.rstrip(_TERM_TRIM_CHARS) + if not stripped: + continue + if _IP_CIDR_RE.match(stripped) or _NIC_NAME_RE.match(stripped): + continue + norm = stripped.lower().rstrip(_TERM_TRIM_CHARS) + if re.fullmatch(r"\d+(?:\.\d+)*", norm) or norm not in ENGLISH_STOP_WORDS: + parts.append(stripped) + # Quote hyphenated compounds so Solr treats them as phrases, not subtraction + parts = [ + f'"{t}"' if "-" in t and not t.startswith('"') and len(t) > 3 else t + for t in parts + ] + return " ".join(parts) if parts else query + + +def _canonical_url_from_solr_doc(doc: dict[str, Any]) -> str: + """Resolve a citation URL from portal-rag chunk field values. + + Args: + doc: One Solr document dict. + + Returns: + Absolute ``https://`` URL when possible; empty string if none found. + """ + for key in ("resourceName", "view_uri"): + v = doc.get(key) + if v: + s = str(v).strip() + if s.startswith("http"): + return s + if s.startswith("/"): + return f"{_ACCESS_BASE}{s}" + vid = doc.get("id") + if vid: + s = str(vid).strip() + if s.startswith("http"): + return s + if s.startswith("/"): + return f"{_ACCESS_BASE}{s}" + return "" + + +def _strip_html(text: str) -> str: + """Turn HTML-ish chunk text into plain space-normalized text for the LLM. + + Args: + text: Raw string that may contain entities and tags. + + Returns: + Decoded, tag-stripped, whitespace-collapsed text. + """ + t = html_mod.unescape(text) + t = re.sub(r"<[^>]+>", " ", t) + return re.sub(r"\s+", " ", t).strip() + + +def _solr_chunk_text_for_rag(doc: dict[str, Any], *, content_field: str) -> str: + """Extract chunk body text from a Solr document and strip HTML for RAG. + + Args: + doc: One Solr document dict. + content_field: Primary field name for chunk text (e.g. ``chunk``). + + Returns: + Plain text suitable for prompt context, or empty string if missing. + """ + for key in (content_field, "chunk", "main_content"): + raw = doc.get(key) + if raw: + return _strip_html(str(raw).strip()) + return "" + + +@dataclass(frozen=True) +class RetrievedChunk: + """One ranked passage from Solr for downstream RAG.""" + + text: str + score: float | None = None + metadata: dict[str, Any] = field(default_factory=dict) + + +class SolrHybridSearch: + """Solr hybrid retrieval via ``POST …/hybrid-search`` (lexical-primary + KNN rerank).""" + + def __init__( + self, + settings: SolrHybridSettings, + encode_fn: Callable[[str], Any], + ) -> None: + """Store Solr HTTP settings and the embedding function used for ``rqq`` KNN. + + Resolves the OCP product version at construction time by reading the + ``OCP_CLUSTER_VERSION`` environment variable and querying Solr for + available versions. The resolved version is used to build + ``chunk_filter_query``. + + Args: + settings: Solr base URL, hybrid weights, timeouts, and row limits. + encode_fn: Maps query text to a dense vector (e.g. LlamaIndex + ``HuggingFaceEmbedding.get_text_embedding``). + + Raises: + InvalidConfigurationError: If ``OCP_CLUSTER_VERSION`` is not set. + """ + self._settings = settings + self._encode_fn = encode_fn + self._http_client: httpx.AsyncClient | None = None + self.chunk_filter_query: str = self._resolve_chunk_filter_query( + settings.solr_http_base, settings.hybrid_solr_timeout_s + ) + + def _get_http_client(self) -> httpx.AsyncClient: + """Return a persistent async HTTP client for Solr (connection pooling).""" + if self._http_client is None or self._http_client.is_closed: + self._http_client = httpx.AsyncClient( + timeout=self._settings.hybrid_solr_timeout_s + ) + return self._http_client + + async def aclose(self) -> None: + """Close the persistent HTTP client (e.g. on shutdown or config reload).""" + if self._http_client is not None and not self._http_client.is_closed: + await self._http_client.aclose() + self._http_client = None + + def close_http_client_sync(self) -> None: + """Best-effort sync cleanup when tearing down or reloading config.""" + client = self._http_client + if client is None or client.is_closed: + self._http_client = None + return + try: + loop = asyncio.get_running_loop() + except RuntimeError: + asyncio.run(self.aclose()) + else: + loop.create_task(self.aclose()) # noqa: RUF006 + + async def search(self, query: str, token_budget: int = 0) -> list[RetrievedChunk]: + """Run hybrid-search; return expanded passages capped at ``max_results``, or ``[]``. + + Args: + query: User query text. + token_budget: Remaining token budget for tool output. Controls how + much chunk expansion is performed per matched chunk. When 0, + chunks are returned without expansion. + + On embedding failures, HTTP errors, JSON decode errors, or malformed Solr payloads, + logs and returns an empty list so callers can continue without passages. + """ + try: + return await self._search_impl(query, token_budget) + except Exception: + logger.exception( + "Solr hybrid search failed for query: %.200s", + query, + ) + return [] + + async def _search_impl(self, query: str, token_budget: int) -> list[RetrievedChunk]: + """Execute the hybrid search, dedupe, expand chunks, and build results.""" + cfg = self._settings + cleaned = normalize_solr_hybrid_query(query) + base = cfg.solr_http_base.rstrip("/") + hybrid_url = f"{base}/solr/{_SOLR_COLLECTION}/hybrid-search" + + def _encode() -> list[float]: + vec = self._encode_fn(cleaned) + return [float(x) for x in (vec.tolist() if hasattr(vec, "tolist") else vec)] + + query_embedding = await asyncio.to_thread(_encode) + vector_str = "[" + ",".join(str(v) for v in query_embedding) + "]" + form = self._build_hybrid_form(cleaned=cleaned, vector_str=vector_str) + client = self._get_http_client() + response = await client.post( + hybrid_url, + data=form, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + response.raise_for_status() + payload = _solr_response_json(response, log_url=hybrid_url) + hybrid_docs = list(payload.get("response", {}).get("docs", [])) + + if not hybrid_docs: + logger.warning("No results (hybrid-search) for: %s", query) + return [] + + if cfg.hybrid_score_threshold > 0: + hybrid_docs = [ + d + for d in hybrid_docs + if _safe_solr_score(d.get("score", 0.0)) >= cfg.hybrid_score_threshold + ] + if not hybrid_docs: + logger.warning( + "No docs above hybrid_score_threshold=%s for: %s", + cfg.hybrid_score_threshold, + query, + ) + return [] + + deduped = self._dedupe_by_parent(hybrid_docs)[: cfg.max_results] + + per_chunk_budget = token_budget // len(deduped) if token_budget > 0 else 0 + + expanded: list[RetrievedChunk] = [] + for doc in deduped: + if per_chunk_budget > 0 and cfg.max_expansion_neighbors > 0: + family = await self._fetch_family(client, base, doc) + ordered = self._expand_around_match( + family, + doc.get("chunk_index", -1), + per_chunk_budget, + max_neighbors=cfg.max_expansion_neighbors, + ) + else: + ordered = [doc] + chunk = self._assemble_chunk(doc, ordered) + logger.debug( + "Chunk %s: expanded %d→%d siblings (family=%d, budget=%d)", + doc.get("chunk_index", "?"), + 1, + len(ordered), + ( + len(family) + if per_chunk_budget > 0 and cfg.max_expansion_neighbors > 0 + else 0 + ), + per_chunk_budget, + ) + expanded.append(chunk) + return expanded + + # ------------------------------------------------------------------ + # Startup: OCP version resolution and chunk_filter_query + # ------------------------------------------------------------------ + + _OCP_CLUSTER_VERSION_ENV = "OCP_CLUSTER_VERSION" + _OCP_PRODUCT = "openshift_container_platform" + + @staticmethod + def _resolve_chunk_filter_query(solr_http_base: str, timeout_s: float) -> str: + """Build ``chunk_filter_query`` from the cluster's OCP version. + + Raises: + InvalidConfigurationError: If ``OCP_CLUSTER_VERSION`` is not set + or Solr is unreachable. + """ + env_version = os.environ.get(SolrHybridSearch._OCP_CLUSTER_VERSION_ENV) + if not env_version: + raise InvalidConfigurationError( + f"{SolrHybridSearch._OCP_CLUSTER_VERSION_ENV} environment variable " + "must be set when solr_hybrid is configured" + ) + + env_version = env_version.strip() + available = SolrHybridSearch._fetch_available_ocp_versions( + solr_http_base, timeout_s + ) + if not available: + raise InvalidConfigurationError( + "Cannot fetch available OCP versions from Solr at " + f"{solr_http_base} — service cannot start" + ) + resolved = SolrHybridSearch._clamp_version(env_version, available) + logger.info( + "OCP version resolved: env=%s, available=%s, resolved=%s", + env_version, + available, + resolved, + ) + return ( + f"is_chunk:true AND product:{SolrHybridSearch._OCP_PRODUCT}" + f" AND product_version:{resolved}" + ) + + _SOLR_STARTUP_RETRIES = 24 + _SOLR_STARTUP_BACKOFF_S = 5 + + @staticmethod + def _fetch_available_ocp_versions( + solr_http_base: str, timeout_s: float + ) -> list[str]: + """Query Solr for available ``product_version`` values for OCP. + + Retries up to ``_SOLR_STARTUP_RETRIES`` times with + ``_SOLR_STARTUP_BACKOFF_S`` seconds between attempts to tolerate + Solr starting up concurrently with OLS. + """ + base = solr_http_base.rstrip("/") + select_url = f"{base}/solr/{_SOLR_COLLECTION}/select" + params = { + "q": "*:*", + "fq": f"product:{SolrHybridSearch._OCP_PRODUCT}", + "rows": "0", + "facet": "true", + "facet.field": "product_version", + "facet.mincount": "1", + "wt": "json", + } + last_error: Exception | None = None + for attempt in range(SolrHybridSearch._SOLR_STARTUP_RETRIES): + try: + response = httpx.get(select_url, params=params, timeout=timeout_s) + response.raise_for_status() + data = response.json() + facet_fields = data.get("facet_counts", {}).get("facet_fields", {}) + raw = facet_fields.get("product_version", []) + return [ + raw[i] for i in range(0, len(raw), 2) if isinstance(raw[i], str) + ] + except Exception as exc: + last_error = exc + logger.warning( + "Solr not ready (attempt %d/%d): %s", + attempt + 1, + SolrHybridSearch._SOLR_STARTUP_RETRIES, + exc, + ) + time.sleep(SolrHybridSearch._SOLR_STARTUP_BACKOFF_S) + logger.error( + "Failed to fetch OCP versions from Solr after all retries: %s", last_error + ) + return [] + + @staticmethod + def _to_major_minor(version_str: str) -> Version: + """Parse a version string and return only major.minor. + + ``4.19.26`` → ``Version("4.19")``, ``4.18`` → ``Version("4.18")``. + """ + v = Version(version_str) + return Version(f"{v.major}.{v.minor}") + + @staticmethod + def _clamp_version(requested: str, available: list[str]) -> str: + """Clamp *requested* to the nearest available major.minor version. + + The env var may contain a full ``major.minor.patch`` version (e.g. + ``4.19.26``) while Solr only indexes ``major.minor`` (e.g. ``4.19``). + Comparison is done on major.minor only. When the exact minor version + is not available, the highest available version ≤ requested is chosen. + """ + try: + req = SolrHybridSearch._to_major_minor(requested) + except InvalidVersion: + logger.warning("Cannot parse OCP_CLUSTER_VERSION '%s'", requested) + return requested + parsed: list[tuple[Version, str]] = [] + for v in available: + try: + parsed.append((SolrHybridSearch._to_major_minor(v), v)) + except InvalidVersion: + continue + if not parsed: + return requested + parsed.sort(key=lambda t: t[0]) + lowest, lowest_str = parsed[0] + highest, highest_str = parsed[-1] + if req < lowest: + return lowest_str + if req > highest: + return highest_str + best_str = lowest_str + for pv, raw in parsed: + if pv == req: + return raw + if pv < req: + best_str = raw + return best_str + + # ------------------------------------------------------------------ + # Private helpers called only from _search_impl + # ------------------------------------------------------------------ + + @staticmethod + def _assemble_chunk( + doc: dict[str, Any], ordered: list[dict[str, Any]] + ) -> RetrievedChunk: + """Merge expanded chunk family into a single ``RetrievedChunk``.""" + texts = [ + _solr_chunk_text_for_rag(f, content_field=_SOLR_CHUNK_TEXT_FIELD) + for f in ordered + ] + merged_text = "\n".join(t for t in texts if t) + raw_score = doc.get("score") + try: + score_f = float(raw_score) if raw_score is not None else None + except (TypeError, ValueError): + score_f = None + return RetrievedChunk( + text=merged_text, + score=score_f, + metadata={ + "title": str(doc.get("title") or "").strip(), + "docs_url": _canonical_url_from_solr_doc(doc), + "index_origin": "solr_hybrid", + "chunks_expanded": len(ordered), + }, + ) + + def _build_hybrid_form( + self, + *, + cleaned: str, + vector_str: str, + ) -> dict[str, str]: + """POST body for hybrid-search: lexical edismax ``q`` + ``{!rerank}`` with KNN.""" + cfg = self._settings + lexical_fetch_rows = min( + max( + cfg.max_results * _RAG_CHUNK_LEXICAL_HEADROOM_MULT, + _RAG_CHUNK_LEXICAL_MIN_ROWS, + ), + _RAG_CHUNK_LEXICAL_MAX_ROWS, + ) + lexical_rows = max(cfg.hybrid_pool_docs, lexical_fetch_rows) + q_text = cleaned.replace("?", "").replace("*", "") + rq = ( + f"{{!rerank reRankQuery=$rqq reRankDocs={cfg.hybrid_pool_docs} " + f"reRankWeight={cfg.hybrid_vector_boost}}}" + ) + rqq = f"{{!knn f={_SOLR_VECTOR_FIELD} topK={lexical_rows}}}{vector_str}" + return { + "q": q_text, + "qf": _RAG_CHUNK_EDISMAX_QF, + "pf": "chunk^4 title^6", + "pf2": "chunk^2 title^4", + "pf3": "chunk^1 title^2", + "mm": _RAG_CHUNK_EDISMAX_MM, + "hl": "on", + "hl.fl": _SOLR_CHUNK_TEXT_FIELD, + "hl.snippets": "4", + "hl.fragsize": "480", + "fq": self.chunk_filter_query, + "defType": "edismax", + "hl.q": q_text, + "rq": rq, + "rqq": rqq, + "rows": str(lexical_rows), + "fl": "*,score,originalScore()", + "wt": "json", + } + + @staticmethod + def _dedupe_by_parent(docs: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Keep the first (highest-scored) raw Solr doc per ``parent_id``.""" + seen: set[str] = set() + out: list[dict[str, Any]] = [] + for doc in docs: + pid = str(doc.get("parent_id") or doc.get("id") or "").strip() + if not pid: + out.append(doc) + continue + if pid in seen: + continue + seen.add(pid) + out.append(doc) + return out + + @staticmethod + async def _fetch_family( + client: httpx.AsyncClient, + base_url: str, + doc: dict[str, Any], + ) -> list[dict[str, Any]]: + """Fetch sibling chunks that share ``parent_id`` and ``heading_id``. + + Returns family members ordered by ``chunk_index``, or just the original + doc wrapped in a list when ``heading_id`` is missing (orphan). + """ + parent_id = doc.get("parent_id") + heading_id = doc.get("heading_id") + if not parent_id or not heading_id: + return [doc] + + select_url = f"{base_url}/solr/{_SOLR_COLLECTION}/select" + params = { + "q": "*:*", + "fq": ( + f"parent_id:{SolrHybridSearch._solr_escape(parent_id)}" + f" AND heading_id:{SolrHybridSearch._solr_escape(heading_id)}" + " AND is_chunk:true" + ), + "sort": "chunk_index asc", + "rows": str(_MAX_FAMILY_CHUNKS), + "fl": f"id,chunk_index,num_tokens,{_SOLR_CHUNK_TEXT_FIELD},title," + "parent_id,heading_id,resourceName,score", + "wt": "json", + } + response = await client.get(select_url, params=params) + response.raise_for_status() + payload = _solr_response_json(response, log_url=select_url) + family = list(payload.get("response", {}).get("docs", [])) + return family or [doc] + + @staticmethod + def _solr_escape(value: str) -> str: + """Escape special Solr query characters in a field value.""" + special = r'+-&|!(){}[]^"~*?:\/' + escaped = [] + for ch in value: + if ch in special: + escaped.append("\\") + escaped.append(ch) + return "".join(escaped) + + @staticmethod + def _expand_around_match( + family: list[dict[str, Any]], + matched_chunk_index: int, + token_budget: int, + max_neighbors: int = 2, + ) -> list[dict[str, Any]]: + """Expand bidirectionally from matched chunk within a token budget. + + Starts from the matched chunk and alternates between previous and next + neighbors, accumulating ``num_tokens`` until *token_budget* is exhausted, + *max_neighbors* is reached on each side, or all family members are + included. The result is sorted by ``chunk_index`` for coherent reading + order. + """ + match_pos = None + for i, doc in enumerate(family): + if doc.get("chunk_index") == matched_chunk_index: + match_pos = i + break + if match_pos is None: + return family + + budget = token_budget + selected = [family[match_pos]] + budget -= family[match_pos].get("num_tokens", 0) + + lo = match_pos - 1 + hi = match_pos + 1 + added_lo = 0 + added_hi = 0 + lo_done = False + hi_done = False + while budget > 0 and not (lo_done and hi_done): + if lo >= 0 and added_lo < max_neighbors and not lo_done: + cost = family[lo].get("num_tokens", 0) + if cost <= budget: + selected.append(family[lo]) + budget -= cost + lo -= 1 + added_lo += 1 + else: + lo_done = True + else: + lo_done = True + if hi < len(family) and added_hi < max_neighbors and not hi_done: + cost = family[hi].get("num_tokens", 0) + if cost <= budget: + selected.append(family[hi]) + budget -= cost + hi += 1 + added_hi += 1 + else: + hi_done = True + else: + hi_done = True + selected.sort(key=lambda d: d.get("chunk_index", 0)) + return selected + + +def get_openshift_docs_tool(client: SolrHybridSearch) -> StructuredTool: + """Return a LangChain tool that searches product docs via Solr hybrid RAG. + + The returned tool reads ``tools_token_budget`` from its own ``metadata`` + dict (set by the execution framework before each invocation) and passes it + to :pymethod:`SolrHybridSearch.search` so chunk expansion respects the + remaining token budget for the round. + + Args: + client: Configured ``SolrHybridSearch`` (OKP portal-rag query embeddings). + + Returns: + Async ``StructuredTool`` the model can invoke with a search query string. + """ + tool_ref: list[StructuredTool] = [] + + async def _search_openshift_documentation( + search_query: str, + ) -> list[dict[str, str]]: + """Run hybrid Solr search and return content blocks for the model. + + Returns a list of ``{"text": ...}`` dicts so the tool output framework + can accumulate blocks until the per-tool token budget is reached. + """ + token_budget = ( + (tool_ref[0].metadata or {}).get("tools_token_budget", 0) if tool_ref else 0 + ) + try: + chunks = await client.search(search_query, token_budget=token_budget) + except Exception: + logger.exception("OpenShift docs tool: Solr search failed") + return [ + { + "text": json.dumps( + { + "error": "documentation_search_failed", + "detail": "search_unexpected_error", + } + ) + } + ] + return [ + { + "text": json.dumps( + { + "text": c.text, + "score": c.score, + "title": c.metadata.get("title"), + "docs_url": c.metadata.get("docs_url"), + }, + ensure_ascii=False, + ) + } + for c in chunks + ] + + tool = StructuredTool.from_function( + coroutine=_search_openshift_documentation, + name="search_openshift_documentation", + description=( + "Search published Red Hat OpenShift and related product documentation " + "(not the live cluster). Returns JSON: an array of " + "{text, score, title, docs_url}, or [] if no hits, or an object with " + "error on failure. Cite docs_url when using a passage." + ), + metadata={"tools_token_budget": 0}, + ) + tool_ref.append(tool) + return tool diff --git a/ols/src/tools/tools.py b/ols/src/tools/tools.py index d2714c450..681d223eb 100644 --- a/ols/src/tools/tools.py +++ b/ols/src/tools/tools.py @@ -241,6 +241,8 @@ async def execute_tool_call( """ structured_content: dict | None = None tool_name = tool.name + if tool.metadata is not None: + tool.metadata["tools_token_budget"] = tools_token_budget result = await tool.coroutine(**tool_args) # type: ignore[misc] raw_output = result[0] if isinstance(result, tuple) and len(result) == 2 else result diff --git a/ols/utils/config.py b/ols/utils/config.py index 159c655f3..171c38327 100644 --- a/ols/utils/config.py +++ b/ols/utils/config.py @@ -11,6 +11,7 @@ import yaml import ols.app.models.config as config_model +from ols import constants from ols.src.cache.cache_factory import CacheFactory from ols.src.quota.quota_limiter_factory import QuotaLimiterFactory from ols.src.quota.token_usage_history import TokenUsageHistory @@ -18,6 +19,7 @@ # as the index_loader.py is excluded from type checks, it confuses # mypy a bit, hence the [attr-defined] bellow from ols.src.rag_index.index_loader import IndexLoader # type: ignore [attr-defined] +from ols.src.rag_index.solr_support import SolrHybridSearch from ols.src.skills.skills_rag import SkillsRAG, load_skills_from_directory from ols.src.tools.tools_rag.hybrid_tools_rag import ToolsRAG from ols.utils.redactor import Redactor @@ -54,6 +56,8 @@ def __init__(self) -> None: self.k8s_tools_resolved = False self._tools_approval: Optional[config_model.ToolsApprovalConfig] = None self._pending_approval_store: Optional["PendingApprovalStoreBase"] = None + self._cached_solr_embed_model: Any = None + self._cached_byok_embed_model: Any = None @property def llm_config(self) -> config_model.LLMProviders: @@ -147,48 +151,35 @@ def rag_index(self) -> Optional[list[Any]]: Returns a list of LlamaIndex BaseIndex objects, but we use Any because the index_loader module is excluded from type checking. """ - # TODO: OLS-380 Config object mirrors configuration - return self.rag_index_loader.vector_indexes + loader = self.rag_index_loader + if loader is None: + return None + return loader.vector_indexes @property - def rag_index_loader(self) -> IndexLoader: - """Return the RAG index loader.""" + def rag_index_loader(self) -> Optional[IndexLoader]: + """Return the RAG index loader, or ``None`` when no reference content is configured.""" + if self.config.ols_config.reference_content is None: # type: ignore[attr-defined] + return None if self._rag_index_loader is None: self._rag_index_loader = IndexLoader(self.ols_config.reference_content) return self._rag_index_loader - def _resolve_embed_model(self, embed_model_path: Optional[str] = None) -> Any: - """Resolve the embedding model for hybrid RAG (tools and skills). + def _byok_embed_model(self) -> Any: + """Load and cache the BYOK HuggingFace embedding model. - Uses the RAG index loader's model when available (production/operator path). - Falls back to creating a HuggingFaceEmbedding from embed_model_path or the - default sentence-transformers model (local testing only). + Shared by tools_rag and skills_rag so the model weights are loaded once. """ + if self._cached_byok_embed_model is not None: + return self._cached_byok_embed_model from llama_index.embeddings.huggingface import ( # pylint: disable=import-outside-toplevel HuggingFaceEmbedding, ) - # Suppress noisy progress bars and model load reports from HuggingFace. - for name in ("sentence_transformers", "transformers"): - logging.getLogger(name).setLevel(logging.ERROR) - - # Local testing override -- not exposed by the operator. - if embed_model_path: - return HuggingFaceEmbedding(model_name=embed_model_path) - - # Production path -- reuse the model from the RAG index loader. - embed_model = self.rag_index_loader.embed_model - if embed_model is not None and not isinstance(embed_model, str): - return embed_model - - # Fallback when no RAG index is configured. - fallback_model = "sentence-transformers/all-mpnet-base-v2" - logger.warning( - "No embedding model from RAG index or config; " - "downloading '%s' from HuggingFace Hub", - fallback_model, + self._cached_byok_embed_model = HuggingFaceEmbedding( + model_name=constants.EMBEDDINGS_MODEL_BYOK_SUBDIR ) - return HuggingFaceEmbedding(model_name=fallback_model) + return self._cached_byok_embed_model @cached_property def tools_rag(self) -> Optional[ToolsRAG]: @@ -204,7 +195,7 @@ def tools_rag(self) -> Optional[ToolsRAG]: ): tool_config = self.config.ols_config.tool_filtering try: - embed_model = self._resolve_embed_model(tool_config.embed_model_path) + embed_model = self._byok_embed_model() except Exception: logger.exception( "Failed to load embedding model for tool filtering; " @@ -241,7 +232,7 @@ def skills_rag(self) -> Optional[SkillsRAG]: return None try: - embed_model = self._resolve_embed_model(skills_config.embed_model_path) + embed_model = self._byok_embed_model() except Exception: logger.exception( "Failed to load embedding model for skills; skills disabled" @@ -257,6 +248,35 @@ def skills_rag(self) -> Optional[SkillsRAG]: return rag + def _solr_hybrid_embed_model(self) -> Any: + """Load and cache the HuggingFace embedding model for OKP ``portal-rag`` hybrid search.""" + if self._cached_solr_embed_model is not None: + return self._cached_solr_embed_model + from llama_index.embeddings.huggingface import ( # pylint: disable=import-outside-toplevel + HuggingFaceEmbedding, + ) + + for name in ("sentence_transformers", "transformers"): + logging.getLogger(name).setLevel(logging.ERROR) + self._cached_solr_embed_model = HuggingFaceEmbedding( + model_name=constants.SOLR_HYBRID_EMBEDDING_MODEL_ID + ) + return self._cached_solr_embed_model + + @cached_property + def solr_hybrid_search(self) -> SolrHybridSearch | None: + """Return Solr hybrid RAG client when ``ols_config.solr_hybrid`` is present.""" + settings = self.config.ols_config.solr_hybrid # type: ignore[attr-defined] + if settings is None: + return None + try: + embed_model = self._solr_hybrid_embed_model() + encode_fn = embed_model.get_text_embedding + except Exception: + logger.exception("Failed to resolve embedding model for Solr hybrid RAG") + return None + return SolrHybridSearch(settings, encode_fn) + @property def proxy_config(self) -> Optional[config_model.ProxyConfig]: """Return the proxy configuration.""" @@ -303,6 +323,11 @@ def reload_from_yaml_file( del self.__dict__["tools_rag"] if "skills_rag" in self.__dict__: del self.__dict__["skills_rag"] + if "solr_hybrid_search" in self.__dict__: + old = self.__dict__["solr_hybrid_search"] + del self.__dict__["solr_hybrid_search"] + if old is not None: + old.close_http_client_sync() except Exception as e: print(f"Failed to load config file {config_file}: {e!s}") print(traceback.format_exc()) diff --git a/ols/utils/environments.py b/ols/utils/environments.py index cdac4490d..d41bca410 100644 --- a/ols/utils/environments.py +++ b/ols/utils/environments.py @@ -2,10 +2,6 @@ import os import tempfile -from pathlib import Path - -import ols.app.models.config as config_model -from ols.constants import EMBEDDINGS_MODEL_BYOK_SUBDIR def configure_gradio_ui_envs() -> None: @@ -21,26 +17,6 @@ def configure_gradio_ui_envs() -> None: os.environ["MPLCONFIGDIR"] = tempdir -def _resolve_embeddings_path(path: Path) -> Path: - """Resolve embeddings path to model subdirectory when needed.""" - if (path / "config.json").is_file(): - return path - candidate = path / EMBEDDINGS_MODEL_BYOK_SUBDIR - if (candidate / "config.json").is_file(): - return candidate - return path - - -def configure_hugging_face_envs(ols_config: config_model.OLSConfig) -> None: +def configure_hugging_face_envs() -> None: """Configure HuggingFace library environment variables.""" - if ( - ols_config - and hasattr(ols_config, "reference_content") - and hasattr(ols_config.reference_content, "embeddings_model_path") - and ols_config.reference_content.embeddings_model_path - ): - resolved = _resolve_embeddings_path( - Path(ols_config.reference_content.embeddings_model_path) - ) - os.environ["TRANSFORMERS_CACHE"] = str(resolved) - os.environ["TRANSFORMERS_OFFLINE"] = "1" + os.environ["TRANSFORMERS_OFFLINE"] = "1" diff --git a/pyproject.toml b/pyproject.toml index ebbf85a19..c683372b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -141,7 +141,7 @@ dependencies = [ "qdrant-client>=1.13.3", # For ToolsRAG vector storage (pure Python, no Rust/Cargo) "rank-bm25>=0.2.2", # For ToolsRAG sparse retrieval "python-frontmatter>=1.1.0", # For parsing skill YAML frontmatter - "opentelemetry-api>=1.20.0", # For audit tracing + "opentelemetry-api>=1.20.0", "opentelemetry-sdk>=1.20.0", # For audit tracing "opentelemetry-exporter-otlp-proto-grpc>=1.20.0", # For OTLP span export "torch>=2.10.0", diff --git a/runner.py b/runner.py index b1fdba47e..45ebda501 100644 --- a/runner.py +++ b/runner.py @@ -58,7 +58,7 @@ def load_index(): configure_logging(config.ols_config.logging_config) logger.info("Config loaded from %s", Path(cfg_file).resolve()) logger.info("Running on Python version %s", sys.version) - configure_hugging_face_envs(config.ols_config) + configure_hugging_face_envs() # generate certificates file from all certificates from certifi package # merged with explicitly specified certificates @@ -91,6 +91,10 @@ def load_index(): "Pyroscope url is not specified. To enable profiling please set `pyroscope_url` " "in the `dev_config` section of the configuration file." ) + # Eagerly initialize Solr hybrid search so OCP version resolution + # (requires OCP_CLUSTER_VERSION and a reachable Solr) fails fast. + config.solr_hybrid_search # pylint: disable=W0104 + # create and start the rag_index_thread - allows loading index in # parallel with starting the Uvicorn server rag_index_thread = threading.Thread(target=load_index) diff --git a/tests/e2e/utils/ols_installer.py b/tests/e2e/utils/ols_installer.py index e50b97ffb..8d6f64d5b 100644 --- a/tests/e2e/utils/ols_installer.py +++ b/tests/e2e/utils/ols_installer.py @@ -151,15 +151,11 @@ def update_ols_config() -> None: old_rag_config = olsconfig["ols_config"]["reference_content"] product_docs_index_id = "" product_docs_index_path = "" - embeddings_model_path = "" - if "embeddings_model_path" in old_rag_config: - embeddings_model_path = old_rag_config["embeddings_model_path"] if "product_docs_index_id" in old_rag_config: product_docs_index_id = old_rag_config["product_docs_index_id"] if "product_docs_index_path" in old_rag_config: product_docs_index_path = old_rag_config["product_docs_index_path"] olsconfig["ols_config"]["reference_content"] = { - "embeddings_model_path": embeddings_model_path, "indexes": [ { "product_docs_index_id": product_docs_index_id, diff --git a/tests/unit/app/endpoints/test_health.py b/tests/unit/app/endpoints/test_health.py index f5278c659..d8347bf8b 100644 --- a/tests/unit/app/endpoints/test_health.py +++ b/tests/unit/app/endpoints/test_health.py @@ -171,14 +171,15 @@ def test_readiness_probe_get_method_index_is_ready(): patch("ols.app.endpoints.health.llm_is_ready_persistent_state", new=True), ): # simulate that the index is loaded - config._rag_index = True + config._rag_index_loader = Mock(vector_indexes=["idx"]) + config.ols_config.reference_content = "some content" assert index_is_ready() response = readiness_probe_get_method() assert response == ReadinessResponse(ready=True, reason="service is ready") # simulate that the index is not loaded, but it shouldn't as there # is no reference content in config - config._rag_index = None + config._rag_index_loader = None config.ols_config.reference_content = None assert index_is_ready() response = readiness_probe_get_method() @@ -188,8 +189,7 @@ def test_readiness_probe_get_method_index_is_ready(): def test_readiness_probe_get_method_index_not_ready(): """Test the readiness_probe function when index is not loaded.""" with patch("ols.app.endpoints.health.llm_is_ready_persistent_state", new=True): - # simulate that the index is not loaded - config._rag_index = None + config._rag_index_loader = Mock(vector_indexes=None) config.ols_config.reference_content = "something else than None" assert not index_is_ready() diff --git a/tests/unit/app/models/test_config.py b/tests/unit/app/models/test_config.py index 46416290f..5a80b352a 100644 --- a/tests/unit/app/models/test_config.py +++ b/tests/unit/app/models/test_config.py @@ -34,6 +34,7 @@ ReferenceContent, ReferenceContentIndex, SkillsConfig, + SolrHybridSettings, TLSConfig, TLSSecurityProfile, UserDataCollection, @@ -2242,6 +2243,7 @@ def test_ols_config(tmpdir): ols_config.tool_round_cap_fraction == constants.DEFAULT_TOOL_ROUND_CAP_FRACTION ) assert ols_config.offload_storage_path == constants.DEFAULT_OFFLOAD_STORAGE_PATH + assert ols_config.solr_hybrid is None def test_ols_config_with_custom_offload_storage_path(): @@ -2257,6 +2259,137 @@ def test_ols_config_with_custom_offload_storage_path(): assert ols_config.offload_storage_path == "/custom/offload/path" +def test_ols_config_solr_hybrid_parses_from_yaml_dict(): + """Parse optional ``solr_hybrid`` under ``ols_config`` into ``SolrHybridSettings``.""" + base = { + "default_provider": "p", + "default_model": "m", + "conversation_cache": {"type": "memory", "memory": {"max_entries": 10}}, + } + ols_config = OLSConfig( + { + **base, + "solr_hybrid": { + "solr_http_base": "https://solr.example.com:8983", + "max_results": 7, + }, + } + ) + assert ols_config.solr_hybrid is not None + assert ols_config.solr_hybrid.solr_http_base == "https://solr.example.com:8983" + assert ols_config.solr_hybrid.max_results == 7 + + +def test_config_reserves_tool_budget_for_solr_hybrid_without_mcp(): + """Tool budget is non-zero when Solr hybrid is configured, even without MCP servers.""" + data = { + "llm_providers": [ + { + "name": "openai", + "type": "openai", + "url": "http://localhost", + "credentials_path": "tests/config/secret/apitoken", + "models": [ + { + "name": "m1", + "context_window_size": 10000, + "parameters": {"tool_budget_ratio": 0.5}, + } + ], + } + ], + "ols_config": { + "default_provider": "openai", + "default_model": "m1", + "conversation_cache": {"type": "memory", "memory": {"max_entries": 10}}, + "authentication_config": {"module": "foo"}, + "solr_hybrid": { + "solr_http_base": "https://solr.example.com:8983", + }, + }, + "dev_config": {"disable_tls": True}, + } + cfg = Config(data) + model = cfg.llm_providers.providers["openai"].models["m1"] + assert model.max_tokens_for_tools == 5000 + + +def test_ols_config_solr_hybrid_validate_yaml_rejects_bad_url(): + """``validate_yaml`` rejects non-http(s) Solr base URLs.""" + settings = SolrHybridSettings(solr_http_base="not-a-url") + with pytest.raises(InvalidConfigurationError, match=r"solr_hybrid\.solr_http_base"): + settings.validate_yaml() + + +def test_ols_config_validate_yaml_solr_hybrid_rejects_invalid_base_url(): + """``OLSConfig.validate_yaml`` validates ``solr_hybrid`` when present.""" + ols_config = OLSConfig( + { + "default_provider": "p", + "default_model": "m", + "conversation_cache": {"type": "memory", "memory": {"max_entries": 10}}, + "logging_config": {"logging_level": "INFO"}, + "solr_hybrid": {"solr_http_base": "ftp://example.com"}, + } + ) + with pytest.raises(InvalidConfigurationError, match=r"solr_hybrid\.solr_http_base"): + ols_config.validate_yaml(disable_tls=True) + + +def test_ols_config_validate_yaml_rejects_solr_with_non_byok_indexes(tmp_path): + """Solr enabled with local indexes requires byok_index on each index row.""" + idx_dir = tmp_path / "idx" + idx_dir.mkdir() + ols_config = OLSConfig( + { + "default_provider": "p", + "default_model": "m", + "conversation_cache": {"type": "memory", "memory": {"max_entries": 10}}, + "logging_config": {"logging_level": "INFO"}, + "solr_hybrid": { + "solr_http_base": "https://solr.example.com", + }, + "reference_content": { + "indexes": [ + { + "product_docs_index_path": str(idx_dir), + "product_docs_index_id": "idx1", + } + ], + }, + } + ) + with pytest.raises(InvalidConfigurationError, match="byok_index"): + ols_config.validate_yaml(disable_tls=True) + + +def test_ols_config_validate_yaml_allows_solr_with_byok_indexes(tmp_path): + """Solr plus BYOK-marked local indexes passes validation.""" + idx_dir = tmp_path / "idx" + idx_dir.mkdir() + ols_config = OLSConfig( + { + "default_provider": "p", + "default_model": "m", + "conversation_cache": {"type": "memory", "memory": {"max_entries": 10}}, + "logging_config": {"logging_level": "INFO"}, + "solr_hybrid": { + "solr_http_base": "https://solr.example.com", + }, + "reference_content": { + "indexes": [ + { + "product_docs_index_path": str(idx_dir), + "product_docs_index_id": "idx1", + "byok_index": True, + } + ], + }, + } + ) + ols_config.validate_yaml(disable_tls=True) + + def test_ols_config_tool_round_cap_fraction(): """tool_round_cap_fraction is on OLSConfig and uses TOOL_ROUND_CAP_FRACTION bounds.""" base = { @@ -3075,6 +3208,16 @@ def test_reference_content_index_constructor(): ) assert reference_content_index.product_docs_index_id == "id" assert reference_content_index.product_docs_index_path == "/path/" + assert reference_content_index.byok_index is False + + byok = ReferenceContentIndex( + { + "product_docs_index_id": "id2", + "product_docs_index_path": "/path2/", + "byok_index": True, + } + ) + assert byok.byok_index is True def test_reference_content_index_equality(): @@ -3145,7 +3288,6 @@ def test_reference_content_constructor(): """Test the ReferenceContent constructor.""" reference_content = ReferenceContent( { - "embeddings_model_path": "/path/2/", "indexes": [ { "product_docs_index_id": "id", @@ -3157,7 +3299,6 @@ def test_reference_content_constructor(): assert reference_content.indexes[0] == ReferenceContentIndex( {"product_docs_index_id": "id", "product_docs_index_path": "/path/1/"} ) - assert reference_content.embeddings_model_path == "/path/2/" def test_reference_content_equality(): @@ -3169,10 +3310,6 @@ def test_reference_content_equality(): assert reference_content_1 == reference_content_2 # compare different configs - reference_content_2.embeddings_model_path = "foo" - assert reference_content_1 != reference_content_2 - - reference_content_2 = ReferenceContent() reference_content_2.indexes = [ ReferenceContentIndex( {"product_docs_index_id": "foo", "product_docs_index_path": "."}, diff --git a/tests/unit/config_status/test_config_status.py b/tests/unit/config_status/test_config_status.py index 4f47003b0..94ad7dd40 100644 --- a/tests/unit/config_status/test_config_status.py +++ b/tests/unit/config_status/test_config_status.py @@ -102,9 +102,6 @@ def test_extract_config_status_with_rag_enabled(self): index = ReferenceContentIndex() index.product_docs_index_id = "ocp-product-docs-4_17" config.ols_config.reference_content.indexes = [index] - config.ols_config.reference_content.embeddings_model_path = Path( - "/models/embeddings" - ) status = extract_config_status(config) diff --git a/tests/unit/prompts/test_prompt_generator.py b/tests/unit/prompts/test_prompt_generator.py index f588a7b07..2146a9cbd 100644 --- a/tests/unit/prompts/test_prompt_generator.py +++ b/tests/unit/prompts/test_prompt_generator.py @@ -389,3 +389,49 @@ def test_generate_prompt_skill_content_none_has_no_effect(model): prompt_with_none.messages[0].prompt.template == prompt_without.messages[0].prompt.template ) + + +def test_solr_docs_tool_guidance_appended_for_ask_tool_mode(): + """Solr docs tool prompt supplements attach in ask mode when enabled.""" + prompt, llm_input_values = GeneratePrompt( + query, + [], + [], + "SYS", + tool_call=True, + mode=QueryMode.ASK, + solr_docs_tool_guidance=True, + ).generate_prompt("gpt-4o-mini") + template = prompt.messages[0].prompt.template + assert "Solr docs tool:" in template + assert "Grounded answers (passages from" in template + assert "search_openshift_documentation" in template + assert prompt.format(**llm_input_values).startswith("System: SYS") + + +def test_solr_docs_tool_guidance_not_appended_for_troubleshooting(): + """Solr docs supplements are ask-only; troubleshooting keeps cluster agent text.""" + prompt, _ = GeneratePrompt( + query, + [], + [], + "SYS", + tool_call=True, + mode=QueryMode.TROUBLESHOOTING, + solr_docs_tool_guidance=True, + ).generate_prompt("gpt-4o-mini") + assert "Solr docs tool:" not in prompt.messages[0].prompt.template + + +def test_solr_docs_tool_guidance_false_omits_supplement(): + """When guidance flag is off, supplements are not added.""" + prompt, _ = GeneratePrompt( + query, + [], + [], + "SYS", + tool_call=True, + mode=QueryMode.ASK, + solr_docs_tool_guidance=False, + ).generate_prompt("gpt-4o-mini") + assert "Grounded answers (passages from" not in prompt.messages[0].prompt.template diff --git a/tests/unit/query_helpers/test_docs_summarizer.py b/tests/unit/query_helpers/test_docs_summarizer.py index 6af93df7f..c2fc2ebe8 100644 --- a/tests/unit/query_helpers/test_docs_summarizer.py +++ b/tests/unit/query_helpers/test_docs_summarizer.py @@ -9,13 +9,14 @@ from langchain_core.messages.ai import AIMessageChunk from ols import config -from ols.app.models.config import LoggingConfig, MCPServerConfig +from ols.app.models.config import LoggingConfig, MCPServerConfig, SolrHybridSettings from ols.app.models.models import StreamChunkType, StreamedChunk from ols.constants import ( DEFAULT_MAX_ITERATIONS, DEFAULT_MAX_ITERATIONS_TROUBLESHOOTING, QueryMode, ) +from ols.utils.config import AppConfig # needs to be setup before importing docs_summarizer config.ols_config.authentication_config.module = "k8s" @@ -73,6 +74,28 @@ def test_if_system_prompt_was_updated(): assert summarizer._system_prompt == config.ols_config.system_prompt +def test_tool_calling_enabled_when_solr_docs_tool_active_without_mcp(): + """Enable tool loop when Solr hybrid docs tool is active, without MCP servers.""" + hybrid = SolrHybridSettings() + mock_client = MagicMock() + with ( + patch.object(config.ols_config, "solr_hybrid", hybrid), + patch.object( + AppConfig, + "solr_hybrid_search", + PropertyMock(return_value=mock_client), + ), + ): + summarizer = DocsSummarizer(llm_loader=mock_llm_loader(None)) + assert summarizer._tool_calling_enabled is True + + +def test_tool_calling_disabled_without_mcp_and_without_solr_docs_tool(): + """Tool calling stays off when neither MCP nor Solr docs tool is active.""" + summarizer = DocsSummarizer(llm_loader=mock_llm_loader(None)) + assert summarizer._tool_calling_enabled is False + + def test_summarize_empty_history(): """Basic test for DocsSummarizer using mocked retriever and empty history.""" with ( @@ -188,7 +211,28 @@ def test_summarize_retrieval_logging(caplog): question = "What's the ultimate question with answer 42?" summary = summarizer.create_response(question, MockRetriever()) check_summary_result(summary, question) - assert "Retrieved 1 documents from indexes" in caplog.text + assert "Retrieved 1 document nodes for RAG context" in caplog.text + + +@pytest.mark.asyncio +async def test_resolve_tools_appends_openshift_docs_tool_when_solr_configured_no_mcp(): + """Append docs tool for Solr hybrid when MCP returns no tools.""" + with patch( + "ols.src.query_helpers.docs_summarizer.get_mcp_tools", + new_callable=AsyncMock, + ) as m_get: + m_get.return_value = [] + prev = config.ols_config.solr_hybrid + config.ols_config.solr_hybrid = SolrHybridSettings() + config.__dict__["solr_hybrid_search"] = MagicMock() + try: + summarizer = DocsSummarizer(llm_loader=mock_llm_loader(None)) + tools = await summarizer._resolve_tools_for_request("query") + finally: + config.ols_config.solr_hybrid = prev + config.__dict__.pop("solr_hybrid_search", None) + assert [t.name for t in tools] == ["search_openshift_documentation"] + m_get.assert_awaited_once() @pytest.mark.asyncio @@ -700,10 +744,9 @@ def _tracked_cleanup(): f"Expected 1 OffloadManager created (streaming={streaming}), " f"got {len(created_managers)}" ) - assert len(cleanup_calls) == 1, ( - f"Expected cleanup called once (streaming={streaming}), " - f"got {len(cleanup_calls)}" - ) + assert ( + len(cleanup_calls) == 1 + ), f"Expected cleanup called once (streaming={streaming}), got {len(cleanup_calls)}" assert any( c.type == StreamChunkType.END for c in chunks ), f"Expected END chunk (streaming={streaming})" diff --git a/tests/unit/rag_index/test_index_loader.py b/tests/unit/rag_index/test_index_loader.py index 8d6affa65..6d220ac8e 100644 --- a/tests/unit/rag_index/test_index_loader.py +++ b/tests/unit/rag_index/test_index_loader.py @@ -1,7 +1,6 @@ """Unit test for the index loader module.""" -import os -from unittest.mock import patch +from unittest.mock import MagicMock, patch import ols.src.rag_index.index_loader as il from ols import config @@ -29,17 +28,20 @@ def test_index_loader_no_id(): ) ] + mock_embed = MagicMock() + mock_embed.model_name = "all-mpnet-base-v2" + with ( - patch("llama_index.core.StorageContext.from_defaults"), - patch.dict(os.environ, {"TRANSFORMERS_CACHE": "", "TRANSFORMERS_OFFLINE": ""}), + patch( + "ols.src.rag_index.index_loader.HuggingFaceEmbedding", + return_value=mock_embed, + ), + patch.object(il.IndexLoader, "_load_index"), ): index_loader_obj = il.IndexLoader(config.ols_config.reference_content) indexes = index_loader_obj.vector_indexes - assert ( - index_loader_obj._embed_model - == "local:sentence-transformers/all-mpnet-base-v2" - ) + assert index_loader_obj._embed_model is mock_embed assert indexes is None @@ -53,7 +55,11 @@ def test_index_loader(): "llama_index.vector_stores.faiss.FaissVectorStore.from_persist_dir" ) as from_persist_dir, patch("llama_index.core.load_index_from_storage", new=MockLlamaIndex), - patch.dict(os.environ, {"TRANSFORMERS_CACHE": "", "TRANSFORMERS_OFFLINE": ""}), + patch("ols.src.rag_index.index_loader.HuggingFaceEmbedding"), + patch( + "llama_index.core.settings.resolve_embed_model", + side_effect=lambda m, **kw: m, + ), ): config.ols_config.reference_content.indexes = [ ReferenceContentIndex( diff --git a/tests/unit/rag_index/test_solr_support.py b/tests/unit/rag_index/test_solr_support.py new file mode 100644 index 000000000..5f86c142e --- /dev/null +++ b/tests/unit/rag_index/test_solr_support.py @@ -0,0 +1,564 @@ +"""Unit tests for Solr hybrid RAG support helpers.""" + +import json +from typing import Any +from unittest.mock import AsyncMock, MagicMock, patch + +import httpx +import pytest +from pydantic import ValidationError + +from ols.app.models.config import SolrHybridSettings +from ols.src.rag_index.solr_support import ( + RetrievedChunk, + SolrHybridSearch, + _safe_solr_score, + get_openshift_docs_tool, + normalize_solr_hybrid_query, +) +from ols.utils.checks import InvalidConfigurationError + +_FAKE_REQUEST = httpx.Request("POST", "http://solr/hybrid-search") + +_PATCH_RESOLVE = patch.object( + SolrHybridSearch, + "_resolve_chunk_filter_query", + return_value="is_chunk:true AND product:*openshift*", +) + + +def _ok_response(payload: dict[str, Any]) -> httpx.Response: + """Build an ``httpx.Response`` that supports ``raise_for_status()``.""" + return httpx.Response(200, json=payload, request=_FAKE_REQUEST) + + +def _patch_httpx_client(fake_post, fake_get=None): + """Patch Solr hybrid search to use a mock persistent ``httpx.AsyncClient``.""" + mock_client = AsyncMock() + mock_client.post = fake_post + mock_client.get = fake_get if fake_get is not None else AsyncMock() + mock_client.is_closed = False + return patch.object(SolrHybridSearch, "_get_http_client", return_value=mock_client) + + +def test_normalize_solr_hybrid_query_strips_common_stopwords() -> None: + """Remove common English stopwords from the query string.""" + out = normalize_solr_hybrid_query("what is the OpenShift route") + assert "what" not in out.split() + assert "the" not in out.split() + assert "OpenShift" in out or "openshift" in out.lower() + + +def test_normalize_solr_hybrid_query_empty_fallback() -> None: + """Return the original query when every token is a stopword.""" + out = normalize_solr_hybrid_query("the a an") + assert out == "the a an" + + +def test_get_openshift_docs_tool_has_expected_name() -> None: + """Factory returns a tool named ``search_openshift_documentation``.""" + tool = get_openshift_docs_tool(MagicMock()) + assert tool.name == "search_openshift_documentation" + + +def test_solr_hybrid_settings_rejects_unknown_field() -> None: + """Reject unexpected fields on SolrHybridSettings.""" + with pytest.raises(ValidationError): + SolrHybridSettings(unknown_field=True) # type: ignore[call-arg] + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_invokes_encode_fn() -> None: + """Pass normalized query text through the injected embedding callable.""" + seen: list[str] = [] + + def encode_fn(text: str) -> list[float]: + seen.append(text) + return [0.25, 0.5, 0.75] + + posted: list[dict[str, str]] = [] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + assert "hybrid-search" in url + assert data.get("defType") == "edismax" + assert "chunk_vector" in data.get("rqq", "") + assert data.get("q") == "pod" + posted.append(data) + resp = _ok_response({"response": {"docs": []}}) + return resp + + with _PATCH_RESOLVE: + client = SolrHybridSearch(SolrHybridSettings(), encode_fn) + with _patch_httpx_client(fake_post): + await client.search("what is the pod") + assert seen == [normalize_solr_hybrid_query("what is the pod")] + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_returns_empty_when_hybrid_empty() -> None: + """Hybrid-search with no docs returns ``[]`` (no lexical fallback).""" + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response({"response": {"docs": []}}) + + with _PATCH_RESOLVE: + client = SolrHybridSearch(SolrHybridSettings(max_results=3), encode_fn) + with _patch_httpx_client(fake_post): + chunks = await client.search("openshift routes") + assert chunks == [] + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_prefers_hybrid_when_docs_present() -> None: + """Return hybrid chunks when hybrid-search returns docs.""" + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response( + { + "response": { + "docs": [ + { + "id": "h1", + "title": "Hybrid Title", + "chunk": "

hybrid only

", + "score": 1.0, + "resourceName": "/docs/hybrid", + } + ] + } + } + ) + + with _PATCH_RESOLVE: + client = SolrHybridSearch(SolrHybridSettings(max_results=2), encode_fn) + with _patch_httpx_client(fake_post): + chunks = await client.search("routes") + assert len(chunks) == 1 + assert "hybrid only" in chunks[0].text + assert chunks[0].metadata["index_origin"] == "solr_hybrid" + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_dedupes_and_caps_max_results() -> None: + """Parent dedupe then ``max_results`` slice across distinct parents.""" + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response( + { + "response": { + "docs": [ + { + "id": "a1", + "parent_id": "p1", + "title": "T1", + "chunk": "

first p1

", + "score": 10.0, + "resourceName": "/r1", + }, + { + "id": "a2", + "parent_id": "p1", + "title": "T1b", + "chunk": "

dup p1

", + "score": 9.0, + "resourceName": "/r1b", + }, + { + "id": "b1", + "parent_id": "p2", + "title": "T2", + "chunk": "

p2 only

", + "score": 8.0, + "resourceName": "/r2", + }, + { + "id": "c1", + "parent_id": "p3", + "title": "T3", + "chunk": "

p3 dropped by cap

", + "score": 7.0, + "resourceName": "/r3", + }, + ] + } + }, + ) + + with _PATCH_RESOLVE: + client = SolrHybridSearch(SolrHybridSettings(max_results=2), encode_fn) + with _patch_httpx_client(fake_post): + chunks = await client.search("q") + assert len(chunks) == 2 + assert "first p1" in chunks[0].text + assert "p2 only" in chunks[1].text + + +@pytest.mark.asyncio +async def test_get_openshift_docs_tool_returns_content_blocks() -> None: + """Tool coroutine returns a list of content blocks from the bound client.""" + + class _StubSolrClient: + """Minimal async client stub matching ``SolrHybridSearch.search`` for tool tests.""" + + async def search( + self, query: str, token_budget: int = 0 + ) -> list[RetrievedChunk]: + assert query == "routes" + return [ + RetrievedChunk( + text="body", + score=0.5, + metadata={"title": "T", "docs_url": "https://access.redhat.com/x"}, + ) + ] + + tool = get_openshift_docs_tool(_StubSolrClient()) # type: ignore[arg-type] + out = await tool.coroutine(search_query="routes") # type: ignore[misc] + assert isinstance(out, list) + assert len(out) == 1 + data = json.loads(out[0]["text"]) + assert data["text"] == "body" + assert data["score"] == 0.5 + assert data["title"] == "T" + assert "access.redhat.com" in data["docs_url"] + + +def test_safe_solr_score_coerces_none_and_invalid() -> None: + """Hybrid score field tolerates null and non-numeric Solr values.""" + assert _safe_solr_score(None) == 0.0 + assert _safe_solr_score("not-a-number") == 0.0 + assert _safe_solr_score(2.5) == 2.5 + + +@pytest.mark.asyncio +async def test_solr_search_returns_empty_when_embedding_raises() -> None: + """``search`` swallows embedding failures and returns no passages.""" + + def bad_encode(_text: str) -> list[float]: + raise RuntimeError("embedding unavailable") + + with _PATCH_RESOLVE: + client = SolrHybridSearch(SolrHybridSettings(), bad_encode) + chunks = await client.search("any query") + assert chunks == [] + + +@pytest.mark.asyncio +async def test_get_openshift_docs_tool_returns_error_json_when_search_raises() -> None: + """Tool maps unexpected ``search`` raises to a structured error block.""" + + class _BrokenClient: + async def search( + self, _query: str, token_budget: int = 0 + ) -> list[RetrievedChunk]: + raise RuntimeError("unexpected") + + tool = get_openshift_docs_tool(_BrokenClient()) # type: ignore[arg-type] + out = await tool.coroutine(search_query="x") # type: ignore[misc] + assert isinstance(out, list) + data = json.loads(out[0]["text"]) + assert data.get("error") == "documentation_search_failed" + + +def test_expand_around_match_respects_token_budget() -> None: + """Only include neighbors whose cumulative tokens fit within the budget.""" + family = [ + {"chunk_index": 0, "num_tokens": 100, "chunk": "c0"}, + {"chunk_index": 1, "num_tokens": 100, "chunk": "c1"}, + {"chunk_index": 2, "num_tokens": 100, "chunk": "c2"}, + {"chunk_index": 3, "num_tokens": 100, "chunk": "c3"}, + {"chunk_index": 4, "num_tokens": 100, "chunk": "c4"}, + ] + result = SolrHybridSearch._expand_around_match( + family, matched_chunk_index=2, token_budget=250 + ) + indices = [d["chunk_index"] for d in result] + assert 2 in indices + assert len(result) <= 3 + assert all(d["chunk_index"] in (1, 2, 3) for d in result) + + +def test_expand_around_match_includes_all_when_budget_is_large() -> None: + """Include every family member when the budget exceeds the total.""" + family = [ + {"chunk_index": 0, "num_tokens": 50, "chunk": "c0"}, + {"chunk_index": 1, "num_tokens": 50, "chunk": "c1"}, + {"chunk_index": 2, "num_tokens": 50, "chunk": "c2"}, + ] + result = SolrHybridSearch._expand_around_match( + family, matched_chunk_index=1, token_budget=10000 + ) + assert len(result) == 3 + + +def test_expand_around_match_zero_budget_returns_only_match() -> None: + """Zero budget returns only the matched chunk itself.""" + family = [ + {"chunk_index": 0, "num_tokens": 50, "chunk": "c0"}, + {"chunk_index": 1, "num_tokens": 0, "chunk": "c1"}, + {"chunk_index": 2, "num_tokens": 50, "chunk": "c2"}, + ] + result = SolrHybridSearch._expand_around_match( + family, matched_chunk_index=1, token_budget=0 + ) + assert len(result) == 1 + assert result[0]["chunk_index"] == 1 + + +def test_expand_around_match_caps_at_max_neighbors() -> None: + """Even with a huge budget, expansion stops at _MAX_EXPANSION_NEIGHBORS per side.""" + family = [{"chunk_index": i, "num_tokens": 10, "chunk": f"c{i}"} for i in range(10)] + result = SolrHybridSearch._expand_around_match( + family, matched_chunk_index=5, token_budget=100000 + ) + assert len(result) == 5 + indices = [d["chunk_index"] for d in result] + assert indices == [3, 4, 5, 6, 7] + + +@pytest.mark.asyncio +async def test_solr_search_expands_chunks_when_budget_provided() -> None: + """With a token budget, search fetches family chunks and expands around match.""" + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + family_docs = { + "response": { + "docs": [ + { + "id": "f0", + "parent_id": "parent1", + "heading_id": "h1", + "chunk_index": 0, + "num_tokens": 40, + "chunk": "

before

", + }, + { + "id": "f1", + "parent_id": "parent1", + "heading_id": "h1", + "chunk_index": 1, + "num_tokens": 50, + "chunk": "

matched chunk

", + }, + { + "id": "f2", + "parent_id": "parent1", + "heading_id": "h1", + "chunk_index": 2, + "num_tokens": 40, + "chunk": "

after

", + }, + ] + } + } + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response( + { + "response": { + "docs": [ + { + "id": "m1", + "parent_id": "parent1", + "heading_id": "h1", + "chunk_index": 1, + "num_tokens": 50, + "title": "T1", + "chunk": "

matched chunk

", + "score": 5.0, + "resourceName": "/docs/m", + } + ] + } + } + ) + + async def fake_get(url: str, *, params: Any) -> Any: + return _ok_response(family_docs) + + mock_client = AsyncMock() + mock_client.post = fake_post + mock_client.get = fake_get + mock_client.is_closed = False + + with ( + patch.object(SolrHybridSearch, "_get_http_client", return_value=mock_client), + _PATCH_RESOLVE, + ): + client = SolrHybridSearch(SolrHybridSettings(max_results=3), encode_fn) + chunks = await client.search("query", token_budget=500) + assert len(chunks) == 1 + assert "before" in chunks[0].text + assert "matched chunk" in chunks[0].text + assert "after" in chunks[0].text + assert chunks[0].metadata["chunks_expanded"] == 3 + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_max_expansion_neighbors_zero_with_budget() -> None: + """``max_expansion_neighbors=0`` with a token budget must not raise ``NameError``.""" + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response( + { + "response": { + "docs": [ + { + "id": "h1", + "title": "Hybrid Title", + "chunk": "

no expansion

", + "score": 1.0, + "resourceName": "/docs/hybrid", + } + ] + } + } + ) + + with _PATCH_RESOLVE: + client = SolrHybridSearch( + SolrHybridSettings(max_expansion_neighbors=0), encode_fn + ) + with _patch_httpx_client(fake_post): + chunks = await client.search("routes", token_budget=500) + assert len(chunks) == 1 + assert "no expansion" in chunks[0].text + + +@pytest.mark.asyncio +async def test_solr_hybrid_search_reuses_async_http_client() -> None: + """Repeated searches reuse one ``httpx.AsyncClient`` instance.""" + clients_created: list[AsyncMock] = [] + + def encode_fn(text: str) -> list[float]: + return [0.1, 0.2, 0.3] + + async def fake_post(url: str, *, data: Any, headers: Any) -> Any: + return _ok_response( + { + "response": { + "docs": [ + { + "id": "h1", + "chunk": "

hit

", + "score": 1.0, + "resourceName": "/docs/hybrid", + } + ] + } + } + ) + + def client_factory(*args: Any, **kwargs: Any) -> AsyncMock: + mock = AsyncMock() + mock.post = fake_post + mock.get = AsyncMock() + mock.is_closed = False + clients_created.append(mock) + return mock + + with ( + _PATCH_RESOLVE, + patch( + "ols.src.rag_index.solr_support.httpx.AsyncClient", + side_effect=client_factory, + ), + ): + client = SolrHybridSearch(SolrHybridSettings(), encode_fn) + await client.search("q1") + await client.search("q2") + await client.aclose() + assert len(clients_created) == 1 + + +@pytest.mark.asyncio +async def test_tool_passes_metadata_budget_to_search() -> None: + """Tool reads ``tools_token_budget`` from its metadata and passes it to search.""" + received_budgets: list[int] = [] + + class _BudgetCapture: + async def search( + self, query: str, token_budget: int = 0 + ) -> list[RetrievedChunk]: + received_budgets.append(token_budget) + return [] + + tool = get_openshift_docs_tool(_BudgetCapture()) # type: ignore[arg-type] + tool.metadata["tools_token_budget"] = 4096 + await tool.coroutine(search_query="test") # type: ignore[misc] + assert received_budgets == [4096] + + +def test_clamp_version_exact_match() -> None: + """Return the exact version when it exists in available.""" + assert SolrHybridSearch._clamp_version("4.18", ["4.16", "4.18", "4.20"]) == "4.18" + + +def test_clamp_version_full_version_matches_major_minor() -> None: + """Full version like 4.19.26 matches available 4.19.""" + assert ( + SolrHybridSearch._clamp_version("4.19.26", ["4.18", "4.19", "4.20"]) == "4.19" + ) + + +def test_clamp_version_below_range() -> None: + """Version below all available clamps to lowest.""" + assert SolrHybridSearch._clamp_version("4.10", ["4.18", "4.19", "4.20"]) == "4.18" + + +def test_clamp_version_above_range() -> None: + """Version above all available clamps to highest.""" + assert SolrHybridSearch._clamp_version("4.99", ["4.18", "4.19", "4.20"]) == "4.20" + + +def test_clamp_version_between_available_picks_lower() -> None: + """Version between available picks highest available <= requested.""" + assert SolrHybridSearch._clamp_version("4.19", ["4.18", "4.20", "4.22"]) == "4.18" + + +def test_clamp_version_unparseable_returns_as_is() -> None: + """Unparseable version string is returned unchanged.""" + assert SolrHybridSearch._clamp_version("not-a-version", ["4.18"]) == "not-a-version" + + +def test_resolve_raises_when_env_not_set() -> None: + """Missing OCP_CLUSTER_VERSION raises InvalidConfigurationError.""" + with ( + patch.dict("os.environ", {}, clear=True), + pytest.raises(InvalidConfigurationError), + ): + SolrHybridSearch._resolve_chunk_filter_query("http://solr:8080", 10.0) + + +def test_resolve_builds_filter_with_clamped_version() -> None: + """Env var + Solr facet response produces a version-specific filter query.""" + facet_response = httpx.Response( + 200, + json={ + "facet_counts": { + "facet_fields": {"product_version": ["4.18", 10, "4.19", 5, "4.20", 3]} + } + }, + request=httpx.Request("GET", "http://solr:8080/solr/portal-rag/select"), + ) + with ( + patch.dict("os.environ", {"OCP_CLUSTER_VERSION": "4.19.7"}), + patch("ols.src.rag_index.solr_support.httpx.get", return_value=facet_response), + ): + result = SolrHybridSearch._resolve_chunk_filter_query("http://solr:8080", 10.0) + assert "product_version:4.19" in result + assert "openshift_container_platform" in result diff --git a/tests/unit/utils/test_config.py b/tests/unit/utils/test_config.py index 283d4d406..ab53e34e7 100644 --- a/tests/unit/utils/test_config.py +++ b/tests/unit/utils/test_config.py @@ -467,34 +467,6 @@ def test_invalid_config_improper_reference_content(): check_expected_exception( """ --- -llm_providers: - - name: p1 - type: openai - credentials_path: tests/config/secret/apitoken - models: - - name: m1 - credentials_path: tests/config/secret/apitoken -ols_config: - reference_content: - embeddings_model_path: ./invalid_dir - conversation_cache: - type: memory - memory: - max_entries: 1000 -dev_config: - llm_temperature_override: 0.1 - enable_dev_ui: true - disable_auth: false - pyroscope_url: https://pyroscope.pyroscope.svc.cluster.local:4040 - -""", - InvalidConfigurationError, - "Embeddings model path './invalid_dir' does not exist", - ) - - check_expected_exception( - """ ---- llm_providers: - name: p1 type: openai @@ -547,38 +519,6 @@ def test_invalid_config_improper_reference_content(): ) -def test_unreadable_directory(): - """Check if an unredable directory is reported correctly.""" - with patch("os.access", return_value=False): - check_expected_exception( - """ ---- -llm_providers: - - name: p1 - type: openai - credentials_path: tests/config/secret/apitoken - models: - - name: m1 - credentials_path: tests/config/secret/apitoken -ols_config: - reference_content: - embeddings_model_path: tests/config - conversation_cache: - type: memory - memory: - max_entries: 1000 -dev_config: - llm_temperature_override: 0.1 - enable_dev_ui: true - disable_auth: false - disable_tls: true - -""", - InvalidConfigurationError, - "Embeddings model path 'tests/config' is not readable", - ) - - def test_valid_config_stream(): """Check if a valid configuration stream is handled correctly.""" try: @@ -1213,3 +1153,83 @@ def test_quota_limiters_property(): config.reload_from_yaml_file("tests/config/valid_config_without_query_filter.yaml") # force reinitialization assert config.quota_limiters is not None + + +def test_solr_hybrid_search_uses_fixed_portal_rag_embedding_model(): + """Solr hybrid loads the OKP portal-rag HuggingFace embedding id, not the RAG index model.""" + yaml_stream = """ +--- +llm_providers: + - name: p1 + type: openai + url: "https://url1" + credentials_path: tests/config/secret/apitoken + models: + - name: m1 + url: "https://murl1" + credentials_path: tests/config/secret/apitoken + context_window_size: 450 + parameters: + max_tokens_for_response: 100 +ols_config: + solr_hybrid: + solr_http_base: "http://localhost:8080" + conversation_cache: + type: memory + memory: + max_entries: 1000 + logging_config: + app_log_level: info + lib_log_level: warning + default_provider: p1 + default_model: m1 + user_data_collection: + transcripts_disabled: true + tls_config: + tls_certificate_path: tests/config/server.crt + tls_key_path: tests/config/server.key + tls_key_password_path: tests/config/password + tlsSecurityProfile: + type: Custom + ciphers: + - TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 + - TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 + minTLSVersion: VersionTLS13 +dev_config: + disable_tls: true + disable_auth: true +""" + + class _StubEmbed: + def get_text_embedding(self, text: str) -> list[float]: + return [0.0] + + with ( + patch( + "llama_index.embeddings.huggingface.HuggingFaceEmbedding", + autospec=True, + ) as mock_hf, + patch( + "ols.src.rag_index.solr_support.SolrHybridSearch._resolve_chunk_filter_query", + return_value="is_chunk:true AND product:openshift_container_platform", + ), + ): + mock_hf.return_value = _StubEmbed() + config.reload_empty() + config.config = config._load_config_from_yaml_stream( + io.StringIO(yaml_stream), ignore_missing_certs=True + ) + for key in ( + "solr_hybrid_search", + "rag_index_loader", + "mcp_servers_dict", + "tools_rag", + "skills_rag", + ): + config.__dict__.pop(key, None) + client = config.solr_hybrid_search + assert client is not None + mock_hf.assert_called_once_with( + model_name=constants.SOLR_HYBRID_EMBEDDING_MODEL_ID + ) + config.reload_empty() diff --git a/tests/unit/utils/test_environments.py b/tests/unit/utils/test_environments.py index 3ad84d6ee..099317c4a 100644 --- a/tests/unit/utils/test_environments.py +++ b/tests/unit/utils/test_environments.py @@ -3,7 +3,6 @@ import os from unittest.mock import patch -from ols.app.models.config import OLSConfig, ReferenceContent from ols.utils.environments import configure_gradio_ui_envs, configure_hugging_face_envs @@ -22,38 +21,11 @@ def test_configure_gradio_ui_envs(): assert os.environ.get("MPLCONFIGDIR", None) != "" -def test_configure_hugging_face_env_no_reference_content_set(): - """Test the function configure_hugging_face_envs.""" - with patch.dict(os.environ, {"TRANSFORMERS_CACHE": "", "TRANSFORMERS_OFFLINE": ""}): - # setup before tested function is called - assert os.environ.get("TRANSFORMERS_CACHE", None) == "" - assert os.environ.get("TRANSFORMERS_OFFLINE", None) == "" - - ols_config = OLSConfig() - ols_config.reference_content = None - - # call the tested function - configure_hugging_face_envs(ols_config) - - # expected environment variables - assert os.environ.get("TRANSFORMERS_CACHE", None) == "" +def test_configure_hugging_face_envs_sets_offline(): + """Verify configure_hugging_face_envs sets TRANSFORMERS_OFFLINE=1.""" + with patch.dict(os.environ, {"TRANSFORMERS_OFFLINE": ""}): assert os.environ.get("TRANSFORMERS_OFFLINE", None) == "" + configure_hugging_face_envs() -def test_configure_hugging_face_env_reference_content_set(): - """Test the function configure_hugging_face_envs.""" - with patch.dict(os.environ, {"TRANSFORMERS_CACHE": "", "TRANSFORMERS_OFFLINE": ""}): - # setup before tested function is called - assert os.environ.get("TRANSFORMERS_CACHE", None) == "" - assert os.environ.get("TRANSFORMERS_OFFLINE", None) == "" - - ols_config = OLSConfig() - ols_config.reference_content = ReferenceContent() - ols_config.reference_content.embeddings_model_path = "foo" - - # call the tested function - configure_hugging_face_envs(ols_config) - - # expected environment variables - assert os.environ.get("TRANSFORMERS_CACHE", None) == "foo" assert os.environ.get("TRANSFORMERS_OFFLINE", None) == "1"