cleanup

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>

Author: franciscojavierarceo

src/llama_stack/core/routers/vector_io.py

@@ -135,6 +135,67 @@ async def insert_chunks(
         )
         return await self.routing_table.insert_chunks(vector_store_id, chunks, ttl_seconds)
 
+    def _parse_filter(self, filter_data: Any) -> Any:
+        """Recursively parse filter data into typed Filter objects.
+
+        This method handles the conversion of dictionary-based filter data into proper
+        typed Filter objects (ComparisonFilter or CompoundFilter), supporting arbitrary
+        nesting levels and pre-typed filter objects.
+
+        Args:
+            filter_data: Filter data as dict, ComparisonFilter, or CompoundFilter
+
+        Returns:
+            Typed filter object (ComparisonFilter or CompoundFilter)
+
+        Raises:
+            ValueError: If filter data is invalid or has unsupported structure
+        """
+        from llama_stack_api.filters import (
+            ALL_FILTER_TYPES,
+            COMPARISON_FILTER_TYPES,
+            COMPOUND_FILTER_TYPES,
+            ComparisonFilter,
+            CompoundFilter,
+        )
+
+        # Handle pre-typed filter objects - pass through unchanged
+        if isinstance(filter_data, ComparisonFilter | CompoundFilter):
+            return filter_data
+
+        # Validate that filter_data is a dictionary
+        if not isinstance(filter_data, dict):
+            raise ValueError("Filter must be a dict or typed Filter object")
+
+        filter_type = filter_data.get("type")
+        if not filter_type:
+            raise ValueError("Filter must have a 'type' field")
+
+        # Handle comparison filters
+        if filter_type in COMPARISON_FILTER_TYPES:
+            # Validate required fields for comparison filters
+            if "key" not in filter_data or "value" not in filter_data:
+                raise ValueError(f"Comparison filter '{filter_type}' must have 'key' and 'value' fields")
+            return ComparisonFilter(**filter_data)
+
+        # Handle compound filters
+        elif filter_type in COMPOUND_FILTER_TYPES:
+            sub_filters_data = filter_data.get("filters", [])
+            if not isinstance(sub_filters_data, list):
+                raise ValueError(f"Compound filter '{filter_type}' must have a 'filters' list")
+
+            # Recursively parse all sub-filters
+            parsed_sub_filters = []
+            for sub_filter in sub_filters_data:
+                parsed_sub_filters.append(self._parse_filter(sub_filter))
+
+            return CompoundFilter(type=filter_type, filters=parsed_sub_filters)
+
+        else:
+            # Use the constants in error message for consistency
+            supported_types = ", ".join(sorted(ALL_FILTER_TYPES))
+            raise ValueError(f"Invalid filter type: '{filter_type}'. Supported types: {supported_types}")
+
     async def query_chunks(
         self,
         vector_store_id: str,
@@ -143,26 +204,23 @@ async def query_chunks(
     ) -> QueryChunksResponse:
         logger.debug(f"VectorIORouter.query_chunks: {vector_store_id}")
 
-        # Extract filters from params for internal processing
-        filters = None
-        if params and "filters" in params:
-            from llama_stack_api.filters import ComparisonFilter, CompoundFilter
-
-            filter_data = params.pop("filters")  # Remove from params to avoid duplication
-            if isinstance(filter_data, dict):
-                # Convert dict to typed filter
-                if filter_data.get("type") in ["eq", "ne", "gt", "gte", "lt", "lte", "in", "nin"]:
-                    filters = ComparisonFilter(**filter_data)
-                elif filter_data.get("type") in ["and", "or"]:
-                    # Handle nested filters recursively
-                    converted_sub_filters = []
-                    for sub_filter in filter_data.get("filters", []):
-                        if sub_filter.get("type") in ["eq", "ne", "gt", "gte", "lt", "lte", "in", "nin"]:
-                            converted_sub_filters.append(ComparisonFilter(**sub_filter))
-                        # Add more nesting support if needed
-                    filters = CompoundFilter(type=filter_data["type"], filters=converted_sub_filters)
-
-        return await self.routing_table.query_chunks(vector_store_id, query, params, filters)
+        # Handle the no-filters case early
+        if not params or "filters" not in params:
+            return await self.routing_table.query_chunks(vector_store_id, query, params, None)
+
+        # Extract and parse filters from params
+        # Create a shallow copy to avoid mutating the caller's dictionary
+        params_copy = dict(params)
+        filter_data = params_copy.pop("filters")
+
+        try:
+            # Parse filter data with full recursive support
+            parsed_filters = self._parse_filter(filter_data)
+        except ValueError as e:
+            logger.error(f"Invalid filter data: {e}")
+            raise ValueError(f"Invalid filter: {e}") from e
+
+        return await self.routing_table.query_chunks(vector_store_id, query, params_copy, parsed_filters)
 
     # OpenAI Vector Stores API endpoints
     async def openai_create_vector_store(

src/llama_stack/providers/utils/memory/openai_vector_store_mixin.py

@@ -10,6 +10,7 @@
 import time
 import uuid
 from abc import ABC, abstractmethod
+from collections.abc import Callable
 from typing import Annotated, Any
 
 from fastapi import Body
@@ -63,12 +64,28 @@
     RetrieveFileContentRequest,
     RetrieveFileRequest,
 )
+from llama_stack_api.filters import (
+    COMPARISON_FILTER_TYPES,
+    COMPOUND_FILTER_TYPES,
+)
 from llama_stack_api.internal.kvstore import KVStore
 
 EMBEDDING_DIMENSION = 768
 
 logger = get_logger(name=__name__, category="providers::utils")
 
+# Comparison operators for filter matching (dispatch table)
+COMPARISON_OPERATORS: dict[str, Callable[[Any, Any], bool]] = {
+    "eq": lambda mv, fv: bool(mv == fv),
+    "ne": lambda mv, fv: bool(mv != fv),
+    "gt": lambda mv, fv: bool(mv > fv),
+    "gte": lambda mv, fv: bool(mv >= fv),
+    "lt": lambda mv, fv: bool(mv < fv),
+    "lte": lambda mv, fv: bool(mv <= fv),
+    "in": lambda mv, fv: bool(isinstance(fv, list) and mv in fv),
+    "nin": lambda mv, fv: bool(isinstance(fv, list) and mv not in fv),
+}
+
 # Constants for OpenAI vector stores
 
 VERSION = "v3"
@@ -803,64 +820,102 @@ def _build_reranker_params(
 
         return params
 
+    def _matches_comparison_filter(self, metadata_value: Any, filter_type: str, filter_value: Any) -> bool:
+        """Check if a metadata value matches a comparison filter.
+
+        Args:
+            metadata_value: The value from the metadata to test
+            filter_type: The comparison operator (eq, ne, gt, etc.)
+            filter_value: The value to compare against
+
+        Returns:
+            bool: True if the comparison matches, False otherwise
+        """
+        if filter_type not in COMPARISON_OPERATORS:
+            raise ValueError(f"Unsupported comparison filter type: {filter_type}")
+
+        return COMPARISON_OPERATORS[filter_type](metadata_value, filter_value)
+
+    def _matches_legacy_filter(self, metadata: dict[str, Any], filters: dict[str, Any]) -> bool:
+        """Handle legacy filter format (direct key-value pairs without type field).
+
+        Args:
+            metadata: The metadata to test against
+            filters: Dict of key-value pairs to match
+
+        Returns:
+            bool: True if all key-value pairs match the metadata
+        """
+        for key, value in filters.items():
+            if key not in metadata or metadata[key] != value:
+                return False
+        return True
+
+    def _matches_compound_filter(
+        self, metadata: dict[str, Any], filter_type: str, sub_filters: list[dict[str, Any]]
+    ) -> bool:
+        """Handle compound filters (and/or logic).
+
+        Args:
+            metadata: The metadata to test against
+            filter_type: Either "and" or "or"
+            sub_filters: List of filters to combine
+
+        Returns:
+            bool: True if the compound filter matches
+        """
+        if filter_type == "and":
+            return all(self._matches_filters(metadata, f) for f in sub_filters)
+        elif filter_type == "or":
+            return any(self._matches_filters(metadata, f) for f in sub_filters)
+        else:
+            raise ValueError(f"Unsupported compound filter type: {filter_type}")
+
     def _matches_filters(self, metadata: dict[str, Any], filters: dict[str, Any]) -> bool:
-        """Check if metadata matches the provided filters."""
+        """Check if metadata matches the provided filters.
+
+        This method supports multiple filter formats:
+        - OpenAI-style typed filters with "type" field
+        - Legacy direct key-value filters
+        - Compound filters with nested logic
+
+        Args:
+            metadata: The metadata to test against
+            filters: The filter specification
+
+        Returns:
+            bool: True if the metadata matches the filter criteria
+        """
         if not filters:
             return True
 
         filter_type = filters.get("type")
 
+        # Handle legacy format (no type field)
         if filter_type is None:
             if "key" not in filters and "value" not in filters and "filters" not in filters:
-                for key, value in filters.items():
-                    if key not in metadata:
-                        return False
-                    if metadata[key] != value:
-                        return False
-                return True
+                return self._matches_legacy_filter(metadata, filters)
             else:
                 raise ValueError("Unsupported filter structure: missing 'type' field")
 
-        if filter_type in ["eq", "ne", "gt", "gte", "lt", "lte"]:
-            # Comparison filter
+        # Handle comparison filters
+        if filter_type in COMPARISON_FILTER_TYPES:
             filter_key = filters.get("key")
-            value = filters.get("value")
+            filter_value = filters.get("value")
 
-            if filter_key is None or not isinstance(filter_key, str):
-                return False
-
-            if filter_key not in metadata:
+            # Validate filter structure
+            if not isinstance(filter_key, str) or filter_key not in metadata:
                 return False
 
             metadata_value = metadata[filter_key]
+            return self._matches_comparison_filter(metadata_value, filter_type, filter_value)
 
-            if filter_type == "eq":
-                return bool(metadata_value == value)
-            elif filter_type == "ne":
-                return bool(metadata_value != value)
-            elif filter_type == "gt":
-                return bool(metadata_value > value)
-            elif filter_type == "gte":
-                return bool(metadata_value >= value)
-            elif filter_type == "lt":
-                return bool(metadata_value < value)
-            elif filter_type == "lte":
-                return bool(metadata_value <= value)
-            else:
-                raise ValueError(f"Unsupported filter type: {filter_type}")
-
-        elif filter_type == "and":
-            # All filters must match
+        # Handle compound filters
+        elif filter_type in COMPOUND_FILTER_TYPES:
             sub_filters = filters.get("filters", [])
-            return all(self._matches_filters(metadata, f) for f in sub_filters)
-
-        elif filter_type == "or":
-            # At least one filter must match
-            sub_filters = filters.get("filters", [])
-            return any(self._matches_filters(metadata, f) for f in sub_filters)
+            return self._matches_compound_filter(metadata, filter_type, sub_filters)
 
         else:
-            # Unknown filter type, default to no match
             raise ValueError(f"Unsupported filter type: {filter_type}")
 
     def _chunk_to_vector_store_content(

src/llama_stack_api/filters.py

@@ -20,6 +20,11 @@
 
 from .schema_utils import json_schema_type
 
+# Constants for filter type validation (using sets for O(1) membership testing)
+COMPARISON_FILTER_TYPES = frozenset(["eq", "ne", "gt", "gte", "lt", "lte", "in", "nin"])
+COMPOUND_FILTER_TYPES = frozenset(["and", "or"])
+ALL_FILTER_TYPES = COMPARISON_FILTER_TYPES | COMPOUND_FILTER_TYPES
+
 
 @json_schema_type
 class ComparisonFilter(BaseModel):