fix: more filter_id issues (#1730)

* missed filter_id fixes

* unit test

* clarify filter comments

* Use bracket notation for document body keys (#1734)

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

* fix: Update integration.test.ts (#1737)

---------

Co-authored-by: Edwin Jose <edwin.jose@datastax.com>

Author: phact

sdks/python/openrag_sdk/documents.py

@@ -163,7 +163,7 @@ async def delete(
         except NotFoundError as e:
             # Keep delete idempotent for SDK callers: a missing document is not
             # an exception.
-            # (Filter-not-found 404s do raise — that's a caller error, not idempotency.)
+            # Filter-not-found 404s still raise because the filter_id is caller input.
             if filename is not None and getattr(e, "status_code", None) == 404:
                 return DeleteDocumentResponse(
                     success=False,

src/api/v1/_filter_resolution.py

@@ -4,8 +4,9 @@
 without first GETting the filter, parsing its `query_data`, and resending the parts as
 inline `filters`. This helper performs that lookup + normalization server-side.
 
-Wildcard handling mirrors `frontend/lib/filter-normalization.ts::buildSearchPayloadFilters`:
-a dimension like `data_sources: ["*"]` collapses to `[]` (i.e. "no filter on this field").
+Wildcard handling mirrors `frontend/lib/filter-normalization.ts::buildSearchPayloadFilters`.
+Each filter dimension is a list; if that list contains the wildcard value `"*"`
+(for example, `data_sources: ["*"]`), the dimension is treated as unscoped.
 """
 
 import json
@@ -17,7 +18,7 @@
 
 
 def _strip_wildcards(filters: dict[str, Any] | None) -> dict[str, list[str]]:
-    """Drop `["*"]` and empty lists from each filter dimension."""
+    """Keep only filter dimensions that contain concrete values."""
     if not filters:
         return {}
     cleaned: dict[str, list[str]] = {}
@@ -66,3 +67,34 @@ async def resolve_filter_id(
         "limit": query_data.get("limit", 10),
         "score_threshold": query_data.get("scoreThreshold", 0),
     }
+
+
+def merge_filter_overrides(
+    resolved: dict[str, Any],
+    request_body: Any,
+) -> tuple[dict[str, Any] | None, int, float]:
+    """Merge resolved filter values with explicitly provided request fields.
+
+    Inline request fields override saved filter values by field presence, not by
+    truthiness. This lets callers intentionally set values such as `limit=10`,
+    `score_threshold=0`, or `filters={}`.
+    """
+    provided_fields: set[str] = getattr(request_body, "model_fields_set", set())
+
+    filters: dict[str, Any] | None = resolved["filters"]
+    if "filters" in provided_fields:
+        inline_filters = request_body.filters
+        if inline_filters:
+            filters = {**resolved["filters"], **inline_filters}
+        else:
+            filters = inline_filters
+
+    limit = request_body.limit
+    if "limit" not in provided_fields:
+        limit = resolved["limit"]
+
+    score_threshold = request_body.score_threshold
+    if "score_threshold" not in provided_fields:
+        score_threshold = resolved["score_threshold"]
+
+    return filters, limit, score_threshold

src/api/v1/chat.py

@@ -13,7 +13,7 @@
 from fastapi.responses import JSONResponse, StreamingResponse
 from pydantic import BaseModel
 
-from api.v1._filter_resolution import resolve_filter_id
+from api.v1._filter_resolution import merge_filter_overrides, resolve_filter_id
 from auth_context import set_auth_context, set_score_threshold, set_search_filters, set_search_limit
 from dependencies import (
     get_chat_service,
@@ -147,13 +147,9 @@ async def chat_create_endpoint(
             user_id=user.user_id,
             jwt_token=jwt_token,
         )
-        # Inline values override per-field; defaults (10 / 0) fall back to the filter.
-        if not body.filters:
-            resolved_filters = resolved["filters"]
-        if body.limit == 10:
-            resolved_limit = resolved["limit"]
-        if body.score_threshold == 0:
-            resolved_score_threshold = resolved["score_threshold"]
+        resolved_filters, resolved_limit, resolved_score_threshold = merge_filter_overrides(
+            resolved, body
+        )
 
     if resolved_filters:
         set_search_filters(resolved_filters)

src/api/v1/documents.py

@@ -50,12 +50,10 @@ async def ingest_endpoint(
     POST /v1/documents/ingest
     Request: multipart/form-data with "file" field
 
-    NOTE: `create_filter` is kept here for response-shape compatibility — the
-    non-v1 onboarding flow consumes the `create_filter` field echoed back in
-    the response. v1 SDK consumers do not currently have a workflow that uses
-    it, and the field is never forwarded to the actual ingest task. It should
-    be removed in a future major version of the v1 API once we are willing to
-    take the breaking change (response no longer contains `create_filter`).
+    NOTE: `create_filter` is upload-router/onboarding metadata. The router
+    echoes it in the upload response so onboarding can create a saved knowledge
+    filter after ingestion completes. It does not affect task creation or
+    ingestion behavior, and the v1 SDK does not expose this option.
     """
     return await upload_ingest_router(
         file=file,
@@ -139,8 +137,8 @@ async def delete_document_endpoint(
     Provide exactly one of:
       - `filename`: delete all chunks for that filename.
       - `filter_id`: resolve the filter's `data_sources` and delete chunks for
-        each of those filenames. Wildcard (`["*"]`) or empty `data_sources`
-        is rejected to prevent mass deletion.
+        each concrete filename. Empty `data_sources` or a list containing the
+        wildcard value `"*"` is rejected to prevent mass deletion.
     """
     if bool(body.filename) == bool(body.filter_id):
         return JSONResponse(
@@ -153,7 +151,7 @@ async def delete_document_endpoint(
             body.filter_id,
             knowledge_filter_service,
             user_id=user.user_id,
-            jwt_token=None,
+            jwt_token=user.jwt_token,
         )
         filenames = resolved["filters"].get("data_sources") or []
         if not filenames:
@@ -164,19 +162,21 @@ async def delete_document_endpoint(
 
         results = []
         total_deleted = 0
+        statuses = []
         for fname in filenames:
             payload, _status = await delete_documents_by_filename_core(
                 filename=fname,
                 session_manager=session_manager,
                 user_id=user.user_id,
-                jwt_token=None,
+                jwt_token=user.jwt_token,
             )
             results.append(payload)
+            statuses.append(_status)
             total_deleted += payload.get("deleted_chunks", 0) or 0
 
         return JSONResponse(
             {
-                "success": True,
+                "success": all(200 <= status < 300 for status in statuses),
                 "deleted_chunks": total_deleted,
                 "filenames": filenames,
                 "filter_id": body.filter_id,
@@ -188,6 +188,6 @@ async def delete_document_endpoint(
         filename=body.filename,
         session_manager=session_manager,
         user_id=user.user_id,
-        jwt_token=None,
+        jwt_token=user.jwt_token,
     )
     return JSONResponse(payload, status_code=status_code)

src/api/v1/search.py

@@ -11,7 +11,7 @@
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel
 
-from api.v1._filter_resolution import resolve_filter_id
+from api.v1._filter_resolution import merge_filter_overrides, resolve_filter_id
 from auth_context import set_auth_context
 from dependencies import (
     get_knowledge_filter_service,
@@ -44,9 +44,8 @@ async def search_endpoint(
     if not query:
         return JSONResponse({"error": "Query is required"}, status_code=400)
 
-    # API-key auth has no JWT; the gate inside search_service.search()
-    # skips set_auth_context() in that case, leaving search_tool() unable to
-    # resolve the user. Set it explicitly here, mirroring v1 chat.
+    # API-key requests can arrive without a JWT. Set the auth context before
+    # resolving filters so search_tool() can still identify the caller.
     set_auth_context(user.user_id, user.jwt_token)
 
     resolved_filters = body.filters
@@ -57,14 +56,11 @@ async def search_endpoint(
             body.filter_id,
             knowledge_filter_service,
             user_id=user.user_id,
-            jwt_token=None,
+            jwt_token=user.jwt_token,
+        )
+        resolved_filters, resolved_limit, resolved_score_threshold = merge_filter_overrides(
+            resolved, body
         )
-        if not body.filters:
-            resolved_filters = resolved["filters"]
-        if body.limit == 10:
-            resolved_limit = resolved["limit"]
-        if body.score_threshold == 0:
-            resolved_score_threshold = resolved["score_threshold"]
 
     logger.debug(
         "Public API search request",

tests/integration/sdk/test_documents.py

@@ -155,40 +155,50 @@ async def _create_filter(self, client, data_sources: list[str]) -> str:
             }
         )
         assert result.success is True, f"Failed to create filter: {result.error}"
+        assert isinstance(result.id, str) and result.id, (
+            f"Filter creation returned no id: {result.error}"
+        )
         return result.id
 
     @pytest.mark.asyncio
     async def test_delete_documents_by_filter_id(self, client, tmp_path):
         """Deleting by filter_id removes only the filenames in the filter's data_sources."""
         alpha, beta = await self._ingest_two(client, tmp_path)
-        filter_id = await self._create_filter(client, [alpha.name])
+        filter_id = None
 
         try:
+            filter_id = await self._create_filter(client, [alpha.name])
             result = await client.documents.delete(filter_id=filter_id)
             assert result.success is True
             assert result.filter_id == filter_id
             assert alpha.name in (result.filenames or [])
             assert beta.name not in (result.filenames or [])
-            # Beta still searchable
-            still_there = await client.search.query("tigers")
+            # Beta still exists; scope the verification to beta's filename so
+            # the assertion does not depend on semantic ranking.
+            still_there = await client.search.query(
+                "*", filters={"data_sources": [beta.name]}, limit=5
+            )
             assert any(r.filename == beta.name for r in still_there.results), (
                 "Beta should still be present after filter-id delete of alpha"
             )
         finally:
-            await client.knowledge_filters.delete(filter_id)
+            if filter_id is not None:
+                await client.knowledge_filters.delete(filter_id)
             # Best-effort cleanup
             await client.documents.delete(alpha.name)
             await client.documents.delete(beta.name)
 
     @pytest.mark.asyncio
     async def test_delete_by_filter_id_with_wildcard_rejects(self, client):
-        """A filter with `["*"]` data_sources must NOT be allowed to mass-delete."""
-        filter_id = await self._create_filter(client, ["*"])
+        """A filter whose data_sources list contains `"*"` is rejected."""
+        filter_id = None
         try:
+            filter_id = await self._create_filter(client, ["*"])
             with pytest.raises(OpenRAGError):
                 await client.documents.delete(filter_id=filter_id)
         finally:
-            await client.knowledge_filters.delete(filter_id)
+            if filter_id is not None:
+                await client.knowledge_filters.delete(filter_id)
 
     @pytest.mark.asyncio
     async def test_delete_with_both_filename_and_filter_id_rejects(self, client):

tests/integration/sdk/test_filters.py

@@ -59,6 +59,9 @@ async def _create_filter_for(client, name: str, data_sources: list[str]) -> str:
         }
     )
     assert result.success is True, f"Failed to create filter: {result.error}"
+    assert isinstance(result.id, str) and result.id, (
+        f"Filter creation returned no id: {result.error}"
+    )
     return result.id
 
 
@@ -138,7 +141,7 @@ async def test_filter_id_in_chat_actually_filters(self, client, tmp_path):
 
     @pytest.mark.asyncio
     async def test_filter_id_in_chat_inline_overrides(self, client, tmp_path):
-        """Inline `filters` win over filter_id per the v1 override contract."""
+        """Inline filters override the resolved filter_id per field."""
         alpha, beta = await _ingest_pair(client, tmp_path)
         filter_id = await _create_filter_for(client, "SDK chat inline-override", [alpha.name])
 
@@ -215,7 +218,7 @@ async def test_filter_id_in_search_actually_filters(self, client, tmp_path):
 
     @pytest.mark.asyncio
     async def test_filter_id_in_search_inline_overrides(self, client, tmp_path):
-        """Inline filters override the resolved filter_id per-field."""
+        """Inline filters override the resolved filter_id per field."""
         alpha, beta = await _ingest_pair(client, tmp_path)
         filter_id = await _create_filter_for(client, "SDK search inline-override", [alpha.name])
 

tests/unit/api/test_v1_filter_resolution.py

@@ -0,0 +1,55 @@
+from typing import Any
+
+from pydantic import BaseModel
+
+from api.v1._filter_resolution import merge_filter_overrides
+
+
+class _RequestBody(BaseModel):
+    filters: dict[str, Any] | None = None
+    limit: int = 10
+    score_threshold: float = 0
+
+
+def test_merge_filter_overrides_uses_resolved_values_when_fields_absent():
+    resolved = {
+        "filters": {"data_sources": ["alpha.md"], "owners": ["alice"]},
+        "limit": 5,
+        "score_threshold": 0.4,
+    }
+
+    filters, limit, score_threshold = merge_filter_overrides(resolved, _RequestBody())
+
+    assert filters == resolved["filters"]
+    assert limit == 5
+    assert score_threshold == 0.4
+
+
+def test_merge_filter_overrides_respects_explicit_defaults_and_empty_filters():
+    resolved = {
+        "filters": {"data_sources": ["alpha.md"]},
+        "limit": 5,
+        "score_threshold": 0.4,
+    }
+    body = _RequestBody(filters={}, limit=10, score_threshold=0)
+
+    filters, limit, score_threshold = merge_filter_overrides(resolved, body)
+
+    assert filters == {}
+    assert limit == 10
+    assert score_threshold == 0
+
+
+def test_merge_filter_overrides_merges_partial_inline_filters_per_field():
+    resolved = {
+        "filters": {"data_sources": ["alpha.md"], "owners": ["alice"]},
+        "limit": 5,
+        "score_threshold": 0.4,
+    }
+    body = _RequestBody(filters={"data_sources": ["beta.md"]})
+
+    filters, limit, score_threshold = merge_filter_overrides(resolved, body)
+
+    assert filters == {"data_sources": ["beta.md"], "owners": ["alice"]}
+    assert limit == 5
+    assert score_threshold == 0.4