From 957e954f1e01d4ffc1459551fafae7d1b41efcb4 Mon Sep 17 00:00:00 2001 From: ewan102 Date: Thu, 23 Jul 2026 16:20:41 +0200 Subject: [PATCH 01/14] allowing scope retrieval via a list of file_ids sent by a POST /v1/chat/completions, used a lot of the workspace filter mechanism, adding a lists of the file_id effectively indexed on the extra.attachments, in order to know which one of the file_ids' lists were not indexed in RAG --- openrag/core/utils/source_filtering.py | 3 + .../services/orchestrators/query_service.py | 78 ++++++++- .../orchestrators/test_query_service.py | 152 +++++++++++++++++- 3 files changed, 223 insertions(+), 10 deletions(-) diff --git a/openrag/core/utils/source_filtering.py b/openrag/core/utils/source_filtering.py index e64418bda..270aea2a7 100644 --- a/openrag/core/utils/source_filtering.py +++ b/openrag/core/utils/source_filtering.py @@ -83,6 +83,7 @@ async def stream_with_source_filtering( sources: list, model_name: str, buffer_size: int | None = None, + extra_fields: dict | None = None, ): """Process an LLM SSE stream, stripping line-terminal source tags. @@ -221,6 +222,8 @@ async def stream_with_source_filtering( filtered = filter_sources_by_citations(sources, citations) extra_payload = {"sources": filtered} + if extra_fields: + extra_payload.update(extra_fields) if not saw_done: extra_payload["truncated"] = True logger.warning( diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 17054fc52..59f8a2ac5 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -397,10 +397,12 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L spoken_style = metadata.get("spoken_style_answer", False) use_websearch = metadata.get("websearch", False) workspace = metadata.get("workspace") + attachment_ids = _extract_attachment_ids(metadata) top_k = self._mr_max if use_map_reduce else None filter_params = None + indexed_attachment_ids: list[str] = [] if workspace and partition: scope = await self._workspace.resolve_scope(workspace, partition) if scope is None: @@ -411,6 +413,16 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L # partition the caller also has access to (#706). partition = [scope.partition] filter_params = {"file_id": scope.file_ids} + elif attachment_ids and partition: + # Client-scoped chat on specific files (Cozy attachments). Mirror the + # workspace flow: a Postgres catalog lookup resolves the real indexed + # subset, which both drives the filter and is reported back to the + # client in `extra` (see chat/chat_stream). A non-indexed id is simply + # left out. The file_id filter is ANDed with the partition filter + # downstream and the partition is fixed server-side, so no DB-side + # security check is needed — this lookup is purely resolution/reporting. + indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) + filter_params = {"file_id": indexed_attachment_ids} web_results: list = [] if partition is not None and use_websearch: @@ -424,8 +436,19 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L web_results = _dedupe_web(await asyncio.gather(*[self._web.search(q.query) for q in queries.query_list])) chunks = [] + if attachment_ids and not chunks: + # Diagnostic: attachments were requested but the file_id filter matched + # nothing. Usually a file_id/partition mismatch or an eventual-consistency + # race (file attached before its indexing reached COMPLETED). The chat + # still proceeds without context. + logger.warning( + "Attachments specified but no chunks matched — check file_id/partition mapping", + file_ids=attachment_ids, + partition=partition, + ) + if not chunks and not web_results and partition is None: - return payload, [], [] + return payload, [], [], [] docs = [c.to_langchain() for c in chunks] if use_map_reduce and docs: @@ -470,7 +493,24 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L }, ) payload["messages"] = new_messages - return payload, docs, web_results + return payload, docs, web_results, indexed_attachment_ids + + async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: + """The order-preserving subset of ``file_ids`` actually indexed in ``partitions``. + + Mirrors the workspace ``resolve_scope`` flow for a client-supplied + attachment list: one batched Postgres catalog lookup per concrete + partition, unioned. The ``"all"`` sentinel (super-admin cross-partition) + can't be validated against a real partition, so the list passes through + unchanged. + """ + concrete = [p for p in partitions if p != "all"] + if not concrete: + return list(file_ids) + found: set[str] = set() + for p in concrete: + found.update(await self._workspace.get_existing_file_ids(p, file_ids)) + return [fid for fid in file_ids if fid in found] async def _gather_rag_and_web(self, queries, partition, top_k, filter_params): # Fuse the doc branch through retrieve_multi so a partition's rrf_k drives @@ -569,9 +609,9 @@ async def chat( metadata = payload.get("metadata") or {} llm = self._resolve_llm(partitions) if partitions is None and not metadata.get("websearch", False): - docs, web_results = [], [] + docs, web_results, attachments = [], [], [] else: - payload, docs, web_results = await self._prepare_chat(partitions, payload, llm) + payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) payload["messages"] = self._sanitize_messages(payload["messages"]) @@ -580,7 +620,12 @@ async def chat( content = chunk.get("choices", [{}])[0].get("message", {}).get("content", "") or "" clean, citations = extract_and_strip_sources_block(content) chunk["choices"][0]["message"]["content"] = clean - chunk["extra"] = json.dumps({"sources": filter_sources_by_citations(sources, citations)}) + extra = {"sources": filter_sources_by_citations(sources, citations)} + if metadata.get("attachments"): + # Report the attachment file_ids actually indexed/used so the client + # can tell which of its attachments were leveraged (vs not yet indexed). + extra["attachments"] = attachments + chunk["extra"] = json.dumps(extra) return chunk async def chat_stream( @@ -595,14 +640,17 @@ async def chat_stream( metadata = payload.get("metadata") or {} llm = self._resolve_llm(partitions) if partitions is None and not metadata.get("websearch", False): - docs, web_results = [], [] + docs, web_results, attachments = [], [], [] else: - payload, docs, web_results = await self._prepare_chat(partitions, payload, llm) + payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) + # Report indexed attachment file_ids in the streamed `extra` (same as chat()). + extra_fields = {"attachments": attachments} if metadata.get("attachments") else None + payload["messages"] = self._sanitize_messages(payload["messages"]) llm_stream = llm.stream_chat(payload["messages"], **_sampling(payload)) - async for sse_line in stream_with_source_filtering(llm_stream, sources, model_name): + async for sse_line in stream_with_source_filtering(llm_stream, sources, model_name, extra_fields=extra_fields): yield sse_line async def complete( @@ -645,6 +693,20 @@ def _summary_doc(chunk, summary: str): return chunk.__class__(page_content=summary, metadata=chunk.metadata) +def _extract_attachment_ids(metadata: dict) -> list[str]: + """Extract the file_id allowlist from a chat request's attachments. + + Cozy (via cozy-stack) sends ``metadata.attachments = [{"id": ""}, + ...]``. Parse defensively — an item without a usable ``id`` (or not a dict) + is skipped rather than raising, so a malformed attachments blob degrades to + a normal unscoped chat instead of failing the request. + """ + raw = metadata.get("attachments") + if not raw: + return [] + return [a["id"] for a in raw if isinstance(a, dict) and a.get("id")] + + def _dedupe_web(web_lists: list[list]) -> list: seen: set[str] = set() out: list = [] diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 62fa1b27d..2fb16bb3e 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -89,8 +89,11 @@ async def search(self, query): class FakeWorkspace: - def __init__(self, scope=None): + def __init__(self, scope=None, existing=None): self._scope = scope + # None => every requested file_id is treated as indexed (default); + # a set/list => only those ids exist in the partition. + self._existing = existing async def get_workspace(self, wid): return None @@ -98,6 +101,11 @@ async def get_workspace(self, wid): async def resolve_scope(self, workspace_id, allowed_partitions): return self._scope + async def get_existing_file_ids(self, partition, file_ids): + if self._existing is None: + return list(file_ids) + return [fid for fid in file_ids if fid in self._existing] + def _config(mode="SimpleRag"): return SimpleNamespace( @@ -406,7 +414,7 @@ async def test_websearch_with_partition_fuses_docs_via_retrieve_multi(): retrieval = FakeRetrieval() web_result = SimpleNamespace(url="https://ex.com", title="T", content="web body", snippet="") svc = _svc(retrieval=retrieval, web=FakeWeb(results=[web_result])) - _payload, _docs, web = await svc._prepare_chat( + _payload, _docs, web, _attachments = await svc._prepare_chat( ["p"], {"messages": [{"role": "user", "content": "q"}], "metadata": {"websearch": True}} ) assert len(retrieval.retrieve_multi_calls) == 1 # doc branch fused via the rrf_k-aware retrieve_multi @@ -512,6 +520,146 @@ async def test_chat_without_workspace_unaffected(): assert call["filter_params"] is None +# --------------------------------------------------------------------------- # +# attachment scoping (Cozy attachments: metadata.attachments = [{"id": ...}]) +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_chat_with_valid_attachments_scopes_search_to_file_ids(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["partitions"] == ["p1"] # attachments do NOT narrow the partition + assert call["filter_params"] == {"file_id": ["fa", "fb"]} + + +@pytest.mark.asyncio +async def test_chat_attachments_malformed_ignored(): + # Items without a usable id (or not dicts) are skipped; an all-malformed + # attachments blob degrades to a normal unscoped chat, never raises. + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"nope": "x"}, "raw-string", 123, {"id": ""}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_empty_attachments_unaffected(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={"messages": [{"role": "user", "content": "q"}], "metadata": {"attachments": []}}, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_workspace_and_attachments_both_present_workspace_wins(): + # workspace is checked first (elif) — when both are present the workspace + # scope wins and the attachments are ignored. + scope = WorkspaceScope(workspace_id="w1", partition="p1", file_ids=["wsa", "wsb"]) + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"]), workspace=FakeWorkspace(scope) + ) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"workspace": "w1", "attachments": [{"id": "att"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] == {"file_id": ["wsa", "wsb"]} + + +@pytest.mark.asyncio +async def test_chat_attachments_zero_matches_logs_warning(monkeypatch): + warnings: list[str] = [] + monkeypatch.setattr(qs.logger, "warning", lambda msg, *a, **kw: warnings.append(msg)) + retrieval = FakeRetrieval(chunks=[]) # filter matches nothing + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert any("no chunks matched" in msg for msg in warnings) + + +@pytest.mark.asyncio +async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"fa"}), # only fa is indexed; fb is not + ) + chunk = await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + # Only the indexed id drives the filter... + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + # ...and the same validated list is reported back to the client in extra. + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + +@pytest.mark.asyncio +async def test_chat_stream_reports_indexed_attachments_in_extra(): + svc = _svc(workspace=FakeWorkspace(existing={"fa"})) # only fa indexed + out = "".join( + [ + line + async for line in svc.chat_stream( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + ] + ) + assert "attachments" in out and "fa" in out + assert "fb" not in out # unindexed id dropped, never reported + + @pytest.mark.asyncio async def test_complete_strips_and_filters(): svc = _svc(llm=FakeLLM(gen_text="text body [Sources: none]")) From 867105b8e297b7b2fc886c1a9b3826e3e7fced4b Mon Sep 17 00:00:00 2001 From: ewan102 Date: Thu, 23 Jul 2026 17:29:48 +0200 Subject: [PATCH 02/14] scraping useless comments and hardening the file search in POSTGRE --- .../services/orchestrators/query_service.py | 51 +++-------- .../orchestrators/test_query_service.py | 86 +++++++++++++------ 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 59f8a2ac5..555d685ce 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -414,13 +414,8 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L partition = [scope.partition] filter_params = {"file_id": scope.file_ids} elif attachment_ids and partition: - # Client-scoped chat on specific files (Cozy attachments). Mirror the - # workspace flow: a Postgres catalog lookup resolves the real indexed - # subset, which both drives the filter and is reported back to the - # client in `extra` (see chat/chat_stream). A non-indexed id is simply - # left out. The file_id filter is ANDed with the partition filter - # downstream and the partition is fixed server-side, so no DB-side - # security check is needed — this lookup is purely resolution/reporting. + # No security check on the ids: file_id is ANDed with the + # server-fixed partition, so a foreign id can never match. indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) filter_params = {"file_id": indexed_attachment_ids} @@ -436,17 +431,6 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L web_results = _dedupe_web(await asyncio.gather(*[self._web.search(q.query) for q in queries.query_list])) chunks = [] - if attachment_ids and not chunks: - # Diagnostic: attachments were requested but the file_id filter matched - # nothing. Usually a file_id/partition mismatch or an eventual-consistency - # race (file attached before its indexing reached COMPLETED). The chat - # still proceeds without context. - logger.warning( - "Attachments specified but no chunks matched — check file_id/partition mapping", - file_ids=attachment_ids, - partition=partition, - ) - if not chunks and not web_results and partition is None: return payload, [], [], [] @@ -496,19 +480,14 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L return payload, docs, web_results, indexed_attachment_ids async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: - """The order-preserving subset of ``file_ids`` actually indexed in ``partitions``. + """Order-preserving subset of ``file_ids`` indexed in ``partitions``. - Mirrors the workspace ``resolve_scope`` flow for a client-supplied - attachment list: one batched Postgres catalog lookup per concrete - partition, unioned. The ``"all"`` sentinel (super-admin cross-partition) - can't be validated against a real partition, so the list passes through - unchanged. + Each lookup is partition-scoped: ``(file_id, partition)`` is the catalog's + unique key, so file_id alone is not. A wildcard like ``"all"`` matches no + real ``partition_name`` and so contributes nothing (fail-closed). """ - concrete = [p for p in partitions if p != "all"] - if not concrete: - return list(file_ids) found: set[str] = set() - for p in concrete: + for p in partitions: found.update(await self._workspace.get_existing_file_ids(p, file_ids)) return [fid for fid in file_ids if fid in found] @@ -622,8 +601,7 @@ async def chat( chunk["choices"][0]["message"]["content"] = clean extra = {"sources": filter_sources_by_citations(sources, citations)} if metadata.get("attachments"): - # Report the attachment file_ids actually indexed/used so the client - # can tell which of its attachments were leveraged (vs not yet indexed). + # Indicate which attachments were actually leveraged to generate the answer. extra["attachments"] = attachments chunk["extra"] = json.dumps(extra) return chunk @@ -645,7 +623,6 @@ async def chat_stream( payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) - # Report indexed attachment file_ids in the streamed `extra` (same as chat()). extra_fields = {"attachments": attachments} if metadata.get("attachments") else None payload["messages"] = self._sanitize_messages(payload["messages"]) @@ -694,17 +671,11 @@ def _summary_doc(chunk, summary: str): def _extract_attachment_ids(metadata: dict) -> list[str]: - """Extract the file_id allowlist from a chat request's attachments. - - Cozy (via cozy-stack) sends ``metadata.attachments = [{"id": ""}, - ...]``. Parse defensively — an item without a usable ``id`` (or not a dict) - is skipped rather than raising, so a malformed attachments blob degrades to - a normal unscoped chat instead of failing the request. - """ + """file_ids from ``metadata.attachments = [{"id": ...}, ...]``; malformed payloads dropped.""" raw = metadata.get("attachments") - if not raw: + if not isinstance(raw, list): return [] - return [a["id"] for a in raw if isinstance(a, dict) and a.get("id")] + return [a["id"] for a in raw if isinstance(a, dict) and isinstance(a.get("id"), str) and a["id"]] def _dedupe_web(web_lists: list[list]) -> list: diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 2fb16bb3e..86a6b271e 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -91,8 +91,8 @@ async def search(self, query): class FakeWorkspace: def __init__(self, scope=None, existing=None): self._scope = scope - # None => every requested file_id is treated as indexed (default); - # a set/list => only those ids exist in the partition. + # None => every requested file_id is indexed in any partition (default); + # dict {partition: set(file_ids)} => partition-scoped existence. self._existing = existing async def get_workspace(self, wid): @@ -104,7 +104,8 @@ async def resolve_scope(self, workspace_id, allowed_partitions): async def get_existing_file_ids(self, partition, file_ids): if self._existing is None: return list(file_ids) - return [fid for fid in file_ids if fid in self._existing] + allowed = self._existing.get(partition, set()) + return [fid for fid in file_ids if fid in allowed] def _config(mode="SimpleRag"): @@ -521,7 +522,7 @@ async def test_chat_without_workspace_unaffected(): # --------------------------------------------------------------------------- # -# attachment scoping (Cozy attachments: metadata.attachments = [{"id": ...}]) +# attachment scoping (metadata.attachments = [{"id": ...}]) # --------------------------------------------------------------------------- # @@ -562,6 +563,38 @@ async def test_chat_attachments_malformed_ignored(): assert call["filter_params"] is None +@pytest.mark.asyncio +@pytest.mark.parametrize("bad", [123, True, "abc", {"id": "x"}]) +async def test_chat_attachments_scalar_payload_is_unscoped(bad): + # A non-list attachments payload must degrade to a normal unscoped chat, + # never raise (a scalar like 123 is not iterable). + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={"messages": [{"role": "user", "content": "q"}], "metadata": {"attachments": bad}}, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_attachments_non_string_ids_ignored(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": 123}, {"id": None}, {"id": ["x"]}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] is None + + @pytest.mark.asyncio async def test_chat_empty_attachments_unaffected(): retrieval = FakeRetrieval() @@ -598,31 +631,13 @@ async def test_chat_workspace_and_attachments_both_present_workspace_wins(): assert call["filter_params"] == {"file_id": ["wsa", "wsb"]} -@pytest.mark.asyncio -async def test_chat_attachments_zero_matches_logs_warning(monkeypatch): - warnings: list[str] = [] - monkeypatch.setattr(qs.logger, "warning", lambda msg, *a, **kw: warnings.append(msg)) - retrieval = FakeRetrieval(chunks=[]) # filter matches nothing - svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) - await svc.chat( - partitions=["p1"], - payload={ - "messages": [{"role": "user", "content": "q"}], - "metadata": {"attachments": [{"id": "fa"}]}, - }, - prepare_sources=lambda d, w: [], - model_name="m", - ) - assert any("no chunks matched" in msg for msg in warnings) - - @pytest.mark.asyncio async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): retrieval = FakeRetrieval() svc = _svc( retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"]), - workspace=FakeWorkspace(existing={"fa"}), # only fa is indexed; fb is not + workspace=FakeWorkspace(existing={"p1": {"fa"}}), # only fa is indexed; fb is not ) chunk = await svc.chat( partitions=["p1"], @@ -641,7 +656,7 @@ async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): @pytest.mark.asyncio async def test_chat_stream_reports_indexed_attachments_in_extra(): - svc = _svc(workspace=FakeWorkspace(existing={"fa"})) # only fa indexed + svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) # only fa indexed in p1 out = "".join( [ line @@ -660,6 +675,29 @@ async def test_chat_stream_reports_indexed_attachments_in_extra(): assert "fb" not in out # unindexed id dropped, never reported +@pytest.mark.asyncio +async def test_chat_attachments_all_partition_is_fail_closed(): + # "all" is a search wildcard, not a real partition_name, so nothing validates + # against it — attachments scope to zero rather than passing through unchecked. + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"p1": {"fa"}}), + ) + chunk = await svc.chat( + partitions=["all"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": []} + assert json.loads(chunk["extra"])["attachments"] == [] + + @pytest.mark.asyncio async def test_complete_strips_and_filters(): svc = _svc(llm=FakeLLM(gen_text="text body [Sources: none]")) From 0dc248a51c1f88c61dba5d11ef31ca74d36004fc Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Mon, 27 Jul 2026 15:21:12 +0200 Subject: [PATCH 03/14] fix(chat): scope attachment lookup for openrag-all admin wildcard --- .../services/orchestrators/query_service.py | 17 ++++--- .../orchestrators/workspace_service.py | 3 ++ .../services/persistence/workspace_repo.py | 16 ++++++ .../orchestrators/test_query_service.py | 50 +++++++++++++++++-- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 555d685ce..30e26c36a 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -482,14 +482,17 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: """Order-preserving subset of ``file_ids`` indexed in ``partitions``. - Each lookup is partition-scoped: ``(file_id, partition)`` is the catalog's - unique key, so file_id alone is not. A wildcard like ``"all"`` matches no - real ``partition_name`` and so contributes nothing (fail-closed). + ``"all"`` (``SUPER_ADMIN_MODE`` wildcard) takes an unscoped lookup instead + of a per-partition one. """ - found: set[str] = set() - for p in partitions: - found.update(await self._workspace.get_existing_file_ids(p, file_ids)) - return [fid for fid in file_ids if fid in found] + if "all" in partitions: + if len(partitions) > 1: + raise ValueError("`partitions` cannot mix the wildcard with explicit values.") + found = set(await self._workspace.get_existing_file_ids_any_partition(file_ids)) + else: + results = await asyncio.gather(*(self._workspace.get_existing_file_ids(p, file_ids) for p in partitions)) + found = {fid for r in results for fid in r} + return [fid for fid in dict.fromkeys(file_ids) if fid in found] async def _gather_rag_and_web(self, queries, partition, top_k, filter_params): # Fuse the doc branch through retrieve_multi so a partition's rrf_k drives diff --git a/openrag/services/orchestrators/workspace_service.py b/openrag/services/orchestrators/workspace_service.py index 78431d92f..8eda48165 100644 --- a/openrag/services/orchestrators/workspace_service.py +++ b/openrag/services/orchestrators/workspace_service.py @@ -82,6 +82,9 @@ async def create_workspace( async def get_existing_file_ids(self, partition: str, file_ids: list[str]) -> list[str]: return list(await self._workspace_repo.get_existing_file_ids(partition, file_ids)) + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> list[str]: + return list(await self._workspace_repo.get_existing_file_ids_any_partition(file_ids)) + async def add_files(self, workspace_id: str, file_ids: list[str]) -> list[str]: """Associate files; returns any file_ids that were not found.""" return await self._workspace_repo.add_files_to_workspace(workspace_id, file_ids) diff --git a/openrag/services/persistence/workspace_repo.py b/openrag/services/persistence/workspace_repo.py index 28fefb742..ea8e01af2 100644 --- a/openrag/services/persistence/workspace_repo.py +++ b/openrag/services/persistence/workspace_repo.py @@ -250,6 +250,22 @@ async def get_existing_file_ids( ) return {r["file_id"] for r in rows} + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> set[str]: + """Return the subset of ``file_ids`` that exist in *any* partition. + + Unscoped by design — only for the ``SUPER_ADMIN_MODE`` ``"all"`` wildcard. + """ + if not file_ids: + return set() + rows = await self.pool.fetch( + """ + SELECT DISTINCT file_id FROM files + WHERE file_id = ANY($1::text[]) + """, + file_ids, + ) + return {r["file_id"] for r in rows} + async def remove_file_from_all_workspaces( self, file_id: str, diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 86a6b271e..8663b932b 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -107,6 +107,12 @@ async def get_existing_file_ids(self, partition, file_ids): allowed = self._existing.get(partition, set()) return [fid for fid in file_ids if fid in allowed] + async def get_existing_file_ids_any_partition(self, file_ids): + if self._existing is None: + return list(file_ids) + allowed = {fid for partition_ids in self._existing.values() for fid in partition_ids} + return [fid for fid in file_ids if fid in allowed] + def _config(mode="SimpleRag"): return SimpleNamespace( @@ -654,6 +660,27 @@ async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): assert json.loads(chunk["extra"])["attachments"] == ["fa"] +@pytest.mark.asyncio +async def test_chat_attachments_duplicate_ids_deduped(): + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"p1": {"fa"}}), + ) + chunk = await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + @pytest.mark.asyncio async def test_chat_stream_reports_indexed_attachments_in_extra(): svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) # only fa indexed in p1 @@ -676,9 +703,7 @@ async def test_chat_stream_reports_indexed_attachments_in_extra(): @pytest.mark.asyncio -async def test_chat_attachments_all_partition_is_fail_closed(): - # "all" is a search wildcard, not a real partition_name, so nothing validates - # against it — attachments scope to zero rather than passing through unchecked. +async def test_chat_attachments_all_partition_looks_up_any_partition(): retrieval = FakeRetrieval() svc = _svc( retrieval=retrieval, @@ -694,8 +719,23 @@ async def test_chat_attachments_all_partition_is_fail_closed(): prepare_sources=lambda d, w: [], model_name="m", ) - assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": []} - assert json.loads(chunk["extra"])["attachments"] == [] + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + +@pytest.mark.asyncio +async def test_chat_attachments_all_mixed_with_explicit_partition_raises(): + svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) + with pytest.raises(ValueError): + await svc.chat( + partitions=["all", "p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) @pytest.mark.asyncio From e7a08759cf84c64b3e7362fae4275cb4cc2f8f26 Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Tue, 28 Jul 2026 12:11:32 +0200 Subject: [PATCH 04/14] fix(workspace): declare get_existing_file_ids_any_partition on the port PgWorkspaceRepository implemented it and WorkspaceService (typed against WorkspaceRepository) already called it, but it was missing from the abstract interface. --- openrag/core/ports/workspace_repo.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openrag/core/ports/workspace_repo.py b/openrag/core/ports/workspace_repo.py index 7d3361029..748cf9a1e 100644 --- a/openrag/core/ports/workspace_repo.py +++ b/openrag/core/ports/workspace_repo.py @@ -53,6 +53,14 @@ async def get_existing_file_ids(self, partition: str, file_ids: list[str]) -> se """Return the subset of ``file_ids`` that actually exist in ``partition``.""" ... + @abstractmethod + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> set[str]: + """Return the subset of ``file_ids`` that exist in *any* partition. + + Unscoped by design — only for the ``SUPER_ADMIN_MODE`` ``"all"`` wildcard. + """ + ... + @abstractmethod async def remove_file_from_all_workspaces(self, file_id: str, partition: str) -> None: """Detach ``file_id`` from every workspace in ``partition``.""" From bf0b7708cdbba277e2bd0249873dd46218b5ebb7 Mon Sep 17 00:00:00 2001 From: ewan102 Date: Thu, 23 Jul 2026 16:20:41 +0200 Subject: [PATCH 05/14] allowing scope retrieval via a list of file_ids sent by a POST /v1/chat/completions, used a lot of the workspace filter mechanism, adding a lists of the file_id effectively indexed on the extra.attachments, in order to know which one of the file_ids' lists were not indexed in RAG --- openrag/core/utils/source_filtering.py | 3 + .../services/orchestrators/query_service.py | 78 ++++++++- .../orchestrators/test_query_service.py | 152 +++++++++++++++++- 3 files changed, 223 insertions(+), 10 deletions(-) diff --git a/openrag/core/utils/source_filtering.py b/openrag/core/utils/source_filtering.py index e64418bda..270aea2a7 100644 --- a/openrag/core/utils/source_filtering.py +++ b/openrag/core/utils/source_filtering.py @@ -83,6 +83,7 @@ async def stream_with_source_filtering( sources: list, model_name: str, buffer_size: int | None = None, + extra_fields: dict | None = None, ): """Process an LLM SSE stream, stripping line-terminal source tags. @@ -221,6 +222,8 @@ async def stream_with_source_filtering( filtered = filter_sources_by_citations(sources, citations) extra_payload = {"sources": filtered} + if extra_fields: + extra_payload.update(extra_fields) if not saw_done: extra_payload["truncated"] = True logger.warning( diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 17054fc52..59f8a2ac5 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -397,10 +397,12 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L spoken_style = metadata.get("spoken_style_answer", False) use_websearch = metadata.get("websearch", False) workspace = metadata.get("workspace") + attachment_ids = _extract_attachment_ids(metadata) top_k = self._mr_max if use_map_reduce else None filter_params = None + indexed_attachment_ids: list[str] = [] if workspace and partition: scope = await self._workspace.resolve_scope(workspace, partition) if scope is None: @@ -411,6 +413,16 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L # partition the caller also has access to (#706). partition = [scope.partition] filter_params = {"file_id": scope.file_ids} + elif attachment_ids and partition: + # Client-scoped chat on specific files (Cozy attachments). Mirror the + # workspace flow: a Postgres catalog lookup resolves the real indexed + # subset, which both drives the filter and is reported back to the + # client in `extra` (see chat/chat_stream). A non-indexed id is simply + # left out. The file_id filter is ANDed with the partition filter + # downstream and the partition is fixed server-side, so no DB-side + # security check is needed — this lookup is purely resolution/reporting. + indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) + filter_params = {"file_id": indexed_attachment_ids} web_results: list = [] if partition is not None and use_websearch: @@ -424,8 +436,19 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L web_results = _dedupe_web(await asyncio.gather(*[self._web.search(q.query) for q in queries.query_list])) chunks = [] + if attachment_ids and not chunks: + # Diagnostic: attachments were requested but the file_id filter matched + # nothing. Usually a file_id/partition mismatch or an eventual-consistency + # race (file attached before its indexing reached COMPLETED). The chat + # still proceeds without context. + logger.warning( + "Attachments specified but no chunks matched — check file_id/partition mapping", + file_ids=attachment_ids, + partition=partition, + ) + if not chunks and not web_results and partition is None: - return payload, [], [] + return payload, [], [], [] docs = [c.to_langchain() for c in chunks] if use_map_reduce and docs: @@ -470,7 +493,24 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L }, ) payload["messages"] = new_messages - return payload, docs, web_results + return payload, docs, web_results, indexed_attachment_ids + + async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: + """The order-preserving subset of ``file_ids`` actually indexed in ``partitions``. + + Mirrors the workspace ``resolve_scope`` flow for a client-supplied + attachment list: one batched Postgres catalog lookup per concrete + partition, unioned. The ``"all"`` sentinel (super-admin cross-partition) + can't be validated against a real partition, so the list passes through + unchanged. + """ + concrete = [p for p in partitions if p != "all"] + if not concrete: + return list(file_ids) + found: set[str] = set() + for p in concrete: + found.update(await self._workspace.get_existing_file_ids(p, file_ids)) + return [fid for fid in file_ids if fid in found] async def _gather_rag_and_web(self, queries, partition, top_k, filter_params): # Fuse the doc branch through retrieve_multi so a partition's rrf_k drives @@ -569,9 +609,9 @@ async def chat( metadata = payload.get("metadata") or {} llm = self._resolve_llm(partitions) if partitions is None and not metadata.get("websearch", False): - docs, web_results = [], [] + docs, web_results, attachments = [], [], [] else: - payload, docs, web_results = await self._prepare_chat(partitions, payload, llm) + payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) payload["messages"] = self._sanitize_messages(payload["messages"]) @@ -580,7 +620,12 @@ async def chat( content = chunk.get("choices", [{}])[0].get("message", {}).get("content", "") or "" clean, citations = extract_and_strip_sources_block(content) chunk["choices"][0]["message"]["content"] = clean - chunk["extra"] = json.dumps({"sources": filter_sources_by_citations(sources, citations)}) + extra = {"sources": filter_sources_by_citations(sources, citations)} + if metadata.get("attachments"): + # Report the attachment file_ids actually indexed/used so the client + # can tell which of its attachments were leveraged (vs not yet indexed). + extra["attachments"] = attachments + chunk["extra"] = json.dumps(extra) return chunk async def chat_stream( @@ -595,14 +640,17 @@ async def chat_stream( metadata = payload.get("metadata") or {} llm = self._resolve_llm(partitions) if partitions is None and not metadata.get("websearch", False): - docs, web_results = [], [] + docs, web_results, attachments = [], [], [] else: - payload, docs, web_results = await self._prepare_chat(partitions, payload, llm) + payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) + # Report indexed attachment file_ids in the streamed `extra` (same as chat()). + extra_fields = {"attachments": attachments} if metadata.get("attachments") else None + payload["messages"] = self._sanitize_messages(payload["messages"]) llm_stream = llm.stream_chat(payload["messages"], **_sampling(payload)) - async for sse_line in stream_with_source_filtering(llm_stream, sources, model_name): + async for sse_line in stream_with_source_filtering(llm_stream, sources, model_name, extra_fields=extra_fields): yield sse_line async def complete( @@ -645,6 +693,20 @@ def _summary_doc(chunk, summary: str): return chunk.__class__(page_content=summary, metadata=chunk.metadata) +def _extract_attachment_ids(metadata: dict) -> list[str]: + """Extract the file_id allowlist from a chat request's attachments. + + Cozy (via cozy-stack) sends ``metadata.attachments = [{"id": ""}, + ...]``. Parse defensively — an item without a usable ``id`` (or not a dict) + is skipped rather than raising, so a malformed attachments blob degrades to + a normal unscoped chat instead of failing the request. + """ + raw = metadata.get("attachments") + if not raw: + return [] + return [a["id"] for a in raw if isinstance(a, dict) and a.get("id")] + + def _dedupe_web(web_lists: list[list]) -> list: seen: set[str] = set() out: list = [] diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 62fa1b27d..2fb16bb3e 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -89,8 +89,11 @@ async def search(self, query): class FakeWorkspace: - def __init__(self, scope=None): + def __init__(self, scope=None, existing=None): self._scope = scope + # None => every requested file_id is treated as indexed (default); + # a set/list => only those ids exist in the partition. + self._existing = existing async def get_workspace(self, wid): return None @@ -98,6 +101,11 @@ async def get_workspace(self, wid): async def resolve_scope(self, workspace_id, allowed_partitions): return self._scope + async def get_existing_file_ids(self, partition, file_ids): + if self._existing is None: + return list(file_ids) + return [fid for fid in file_ids if fid in self._existing] + def _config(mode="SimpleRag"): return SimpleNamespace( @@ -406,7 +414,7 @@ async def test_websearch_with_partition_fuses_docs_via_retrieve_multi(): retrieval = FakeRetrieval() web_result = SimpleNamespace(url="https://ex.com", title="T", content="web body", snippet="") svc = _svc(retrieval=retrieval, web=FakeWeb(results=[web_result])) - _payload, _docs, web = await svc._prepare_chat( + _payload, _docs, web, _attachments = await svc._prepare_chat( ["p"], {"messages": [{"role": "user", "content": "q"}], "metadata": {"websearch": True}} ) assert len(retrieval.retrieve_multi_calls) == 1 # doc branch fused via the rrf_k-aware retrieve_multi @@ -512,6 +520,146 @@ async def test_chat_without_workspace_unaffected(): assert call["filter_params"] is None +# --------------------------------------------------------------------------- # +# attachment scoping (Cozy attachments: metadata.attachments = [{"id": ...}]) +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_chat_with_valid_attachments_scopes_search_to_file_ids(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["partitions"] == ["p1"] # attachments do NOT narrow the partition + assert call["filter_params"] == {"file_id": ["fa", "fb"]} + + +@pytest.mark.asyncio +async def test_chat_attachments_malformed_ignored(): + # Items without a usable id (or not dicts) are skipped; an all-malformed + # attachments blob degrades to a normal unscoped chat, never raises. + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"nope": "x"}, "raw-string", 123, {"id": ""}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_empty_attachments_unaffected(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={"messages": [{"role": "user", "content": "q"}], "metadata": {"attachments": []}}, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_workspace_and_attachments_both_present_workspace_wins(): + # workspace is checked first (elif) — when both are present the workspace + # scope wins and the attachments are ignored. + scope = WorkspaceScope(workspace_id="w1", partition="p1", file_ids=["wsa", "wsb"]) + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"]), workspace=FakeWorkspace(scope) + ) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"workspace": "w1", "attachments": [{"id": "att"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + call = retrieval.retrieve_multi_calls[0] + assert call["filter_params"] == {"file_id": ["wsa", "wsb"]} + + +@pytest.mark.asyncio +async def test_chat_attachments_zero_matches_logs_warning(monkeypatch): + warnings: list[str] = [] + monkeypatch.setattr(qs.logger, "warning", lambda msg, *a, **kw: warnings.append(msg)) + retrieval = FakeRetrieval(chunks=[]) # filter matches nothing + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert any("no chunks matched" in msg for msg in warnings) + + +@pytest.mark.asyncio +async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"fa"}), # only fa is indexed; fb is not + ) + chunk = await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + # Only the indexed id drives the filter... + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + # ...and the same validated list is reported back to the client in extra. + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + +@pytest.mark.asyncio +async def test_chat_stream_reports_indexed_attachments_in_extra(): + svc = _svc(workspace=FakeWorkspace(existing={"fa"})) # only fa indexed + out = "".join( + [ + line + async for line in svc.chat_stream( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + ] + ) + assert "attachments" in out and "fa" in out + assert "fb" not in out # unindexed id dropped, never reported + + @pytest.mark.asyncio async def test_complete_strips_and_filters(): svc = _svc(llm=FakeLLM(gen_text="text body [Sources: none]")) From b7ed89e8ad36a081adbd6764e6465ea49bdb991d Mon Sep 17 00:00:00 2001 From: ewan102 Date: Thu, 23 Jul 2026 17:29:48 +0200 Subject: [PATCH 06/14] scraping useless comments and hardening the file search in POSTGRE --- .../services/orchestrators/query_service.py | 51 +++-------- .../orchestrators/test_query_service.py | 86 +++++++++++++------ 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 59f8a2ac5..555d685ce 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -414,13 +414,8 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L partition = [scope.partition] filter_params = {"file_id": scope.file_ids} elif attachment_ids and partition: - # Client-scoped chat on specific files (Cozy attachments). Mirror the - # workspace flow: a Postgres catalog lookup resolves the real indexed - # subset, which both drives the filter and is reported back to the - # client in `extra` (see chat/chat_stream). A non-indexed id is simply - # left out. The file_id filter is ANDed with the partition filter - # downstream and the partition is fixed server-side, so no DB-side - # security check is needed — this lookup is purely resolution/reporting. + # No security check on the ids: file_id is ANDed with the + # server-fixed partition, so a foreign id can never match. indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) filter_params = {"file_id": indexed_attachment_ids} @@ -436,17 +431,6 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L web_results = _dedupe_web(await asyncio.gather(*[self._web.search(q.query) for q in queries.query_list])) chunks = [] - if attachment_ids and not chunks: - # Diagnostic: attachments were requested but the file_id filter matched - # nothing. Usually a file_id/partition mismatch or an eventual-consistency - # race (file attached before its indexing reached COMPLETED). The chat - # still proceeds without context. - logger.warning( - "Attachments specified but no chunks matched — check file_id/partition mapping", - file_ids=attachment_ids, - partition=partition, - ) - if not chunks and not web_results and partition is None: return payload, [], [], [] @@ -496,19 +480,14 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L return payload, docs, web_results, indexed_attachment_ids async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: - """The order-preserving subset of ``file_ids`` actually indexed in ``partitions``. + """Order-preserving subset of ``file_ids`` indexed in ``partitions``. - Mirrors the workspace ``resolve_scope`` flow for a client-supplied - attachment list: one batched Postgres catalog lookup per concrete - partition, unioned. The ``"all"`` sentinel (super-admin cross-partition) - can't be validated against a real partition, so the list passes through - unchanged. + Each lookup is partition-scoped: ``(file_id, partition)`` is the catalog's + unique key, so file_id alone is not. A wildcard like ``"all"`` matches no + real ``partition_name`` and so contributes nothing (fail-closed). """ - concrete = [p for p in partitions if p != "all"] - if not concrete: - return list(file_ids) found: set[str] = set() - for p in concrete: + for p in partitions: found.update(await self._workspace.get_existing_file_ids(p, file_ids)) return [fid for fid in file_ids if fid in found] @@ -622,8 +601,7 @@ async def chat( chunk["choices"][0]["message"]["content"] = clean extra = {"sources": filter_sources_by_citations(sources, citations)} if metadata.get("attachments"): - # Report the attachment file_ids actually indexed/used so the client - # can tell which of its attachments were leveraged (vs not yet indexed). + # Indicate which attachments were actually leveraged to generate the answer. extra["attachments"] = attachments chunk["extra"] = json.dumps(extra) return chunk @@ -645,7 +623,6 @@ async def chat_stream( payload, docs, web_results, attachments = await self._prepare_chat(partitions, payload, llm) sources = prepare_sources(docs, web_results) - # Report indexed attachment file_ids in the streamed `extra` (same as chat()). extra_fields = {"attachments": attachments} if metadata.get("attachments") else None payload["messages"] = self._sanitize_messages(payload["messages"]) @@ -694,17 +671,11 @@ def _summary_doc(chunk, summary: str): def _extract_attachment_ids(metadata: dict) -> list[str]: - """Extract the file_id allowlist from a chat request's attachments. - - Cozy (via cozy-stack) sends ``metadata.attachments = [{"id": ""}, - ...]``. Parse defensively — an item without a usable ``id`` (or not a dict) - is skipped rather than raising, so a malformed attachments blob degrades to - a normal unscoped chat instead of failing the request. - """ + """file_ids from ``metadata.attachments = [{"id": ...}, ...]``; malformed payloads dropped.""" raw = metadata.get("attachments") - if not raw: + if not isinstance(raw, list): return [] - return [a["id"] for a in raw if isinstance(a, dict) and a.get("id")] + return [a["id"] for a in raw if isinstance(a, dict) and isinstance(a.get("id"), str) and a["id"]] def _dedupe_web(web_lists: list[list]) -> list: diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 2fb16bb3e..86a6b271e 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -91,8 +91,8 @@ async def search(self, query): class FakeWorkspace: def __init__(self, scope=None, existing=None): self._scope = scope - # None => every requested file_id is treated as indexed (default); - # a set/list => only those ids exist in the partition. + # None => every requested file_id is indexed in any partition (default); + # dict {partition: set(file_ids)} => partition-scoped existence. self._existing = existing async def get_workspace(self, wid): @@ -104,7 +104,8 @@ async def resolve_scope(self, workspace_id, allowed_partitions): async def get_existing_file_ids(self, partition, file_ids): if self._existing is None: return list(file_ids) - return [fid for fid in file_ids if fid in self._existing] + allowed = self._existing.get(partition, set()) + return [fid for fid in file_ids if fid in allowed] def _config(mode="SimpleRag"): @@ -521,7 +522,7 @@ async def test_chat_without_workspace_unaffected(): # --------------------------------------------------------------------------- # -# attachment scoping (Cozy attachments: metadata.attachments = [{"id": ...}]) +# attachment scoping (metadata.attachments = [{"id": ...}]) # --------------------------------------------------------------------------- # @@ -562,6 +563,38 @@ async def test_chat_attachments_malformed_ignored(): assert call["filter_params"] is None +@pytest.mark.asyncio +@pytest.mark.parametrize("bad", [123, True, "abc", {"id": "x"}]) +async def test_chat_attachments_scalar_payload_is_unscoped(bad): + # A non-list attachments payload must degrade to a normal unscoped chat, + # never raise (a scalar like 123 is not iterable). + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={"messages": [{"role": "user", "content": "q"}], "metadata": {"attachments": bad}}, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] is None + + +@pytest.mark.asyncio +async def test_chat_attachments_non_string_ids_ignored(): + retrieval = FakeRetrieval() + svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) + await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": 123}, {"id": None}, {"id": ["x"]}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] is None + + @pytest.mark.asyncio async def test_chat_empty_attachments_unaffected(): retrieval = FakeRetrieval() @@ -598,31 +631,13 @@ async def test_chat_workspace_and_attachments_both_present_workspace_wins(): assert call["filter_params"] == {"file_id": ["wsa", "wsb"]} -@pytest.mark.asyncio -async def test_chat_attachments_zero_matches_logs_warning(monkeypatch): - warnings: list[str] = [] - monkeypatch.setattr(qs.logger, "warning", lambda msg, *a, **kw: warnings.append(msg)) - retrieval = FakeRetrieval(chunks=[]) # filter matches nothing - svc = _svc(retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"])) - await svc.chat( - partitions=["p1"], - payload={ - "messages": [{"role": "user", "content": "q"}], - "metadata": {"attachments": [{"id": "fa"}]}, - }, - prepare_sources=lambda d, w: [], - model_name="m", - ) - assert any("no chunks matched" in msg for msg in warnings) - - @pytest.mark.asyncio async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): retrieval = FakeRetrieval() svc = _svc( retrieval=retrieval, llm=FakeLLM(chat_responses=["answer [Sources: none]"]), - workspace=FakeWorkspace(existing={"fa"}), # only fa is indexed; fb is not + workspace=FakeWorkspace(existing={"p1": {"fa"}}), # only fa is indexed; fb is not ) chunk = await svc.chat( partitions=["p1"], @@ -641,7 +656,7 @@ async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): @pytest.mark.asyncio async def test_chat_stream_reports_indexed_attachments_in_extra(): - svc = _svc(workspace=FakeWorkspace(existing={"fa"})) # only fa indexed + svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) # only fa indexed in p1 out = "".join( [ line @@ -660,6 +675,29 @@ async def test_chat_stream_reports_indexed_attachments_in_extra(): assert "fb" not in out # unindexed id dropped, never reported +@pytest.mark.asyncio +async def test_chat_attachments_all_partition_is_fail_closed(): + # "all" is a search wildcard, not a real partition_name, so nothing validates + # against it — attachments scope to zero rather than passing through unchecked. + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"p1": {"fa"}}), + ) + chunk = await svc.chat( + partitions=["all"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fb"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": []} + assert json.loads(chunk["extra"])["attachments"] == [] + + @pytest.mark.asyncio async def test_complete_strips_and_filters(): svc = _svc(llm=FakeLLM(gen_text="text body [Sources: none]")) From 0ed5ee2738ba97489a6941106b0c383b4d1945a1 Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Mon, 27 Jul 2026 15:21:12 +0200 Subject: [PATCH 07/14] fix(chat): scope attachment lookup for openrag-all admin wildcard --- .../services/orchestrators/query_service.py | 17 ++++--- .../orchestrators/workspace_service.py | 3 ++ .../services/persistence/workspace_repo.py | 16 ++++++ .../orchestrators/test_query_service.py | 50 +++++++++++++++++-- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 555d685ce..30e26c36a 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -482,14 +482,17 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: """Order-preserving subset of ``file_ids`` indexed in ``partitions``. - Each lookup is partition-scoped: ``(file_id, partition)`` is the catalog's - unique key, so file_id alone is not. A wildcard like ``"all"`` matches no - real ``partition_name`` and so contributes nothing (fail-closed). + ``"all"`` (``SUPER_ADMIN_MODE`` wildcard) takes an unscoped lookup instead + of a per-partition one. """ - found: set[str] = set() - for p in partitions: - found.update(await self._workspace.get_existing_file_ids(p, file_ids)) - return [fid for fid in file_ids if fid in found] + if "all" in partitions: + if len(partitions) > 1: + raise ValueError("`partitions` cannot mix the wildcard with explicit values.") + found = set(await self._workspace.get_existing_file_ids_any_partition(file_ids)) + else: + results = await asyncio.gather(*(self._workspace.get_existing_file_ids(p, file_ids) for p in partitions)) + found = {fid for r in results for fid in r} + return [fid for fid in dict.fromkeys(file_ids) if fid in found] async def _gather_rag_and_web(self, queries, partition, top_k, filter_params): # Fuse the doc branch through retrieve_multi so a partition's rrf_k drives diff --git a/openrag/services/orchestrators/workspace_service.py b/openrag/services/orchestrators/workspace_service.py index 78431d92f..8eda48165 100644 --- a/openrag/services/orchestrators/workspace_service.py +++ b/openrag/services/orchestrators/workspace_service.py @@ -82,6 +82,9 @@ async def create_workspace( async def get_existing_file_ids(self, partition: str, file_ids: list[str]) -> list[str]: return list(await self._workspace_repo.get_existing_file_ids(partition, file_ids)) + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> list[str]: + return list(await self._workspace_repo.get_existing_file_ids_any_partition(file_ids)) + async def add_files(self, workspace_id: str, file_ids: list[str]) -> list[str]: """Associate files; returns any file_ids that were not found.""" return await self._workspace_repo.add_files_to_workspace(workspace_id, file_ids) diff --git a/openrag/services/persistence/workspace_repo.py b/openrag/services/persistence/workspace_repo.py index 28fefb742..ea8e01af2 100644 --- a/openrag/services/persistence/workspace_repo.py +++ b/openrag/services/persistence/workspace_repo.py @@ -250,6 +250,22 @@ async def get_existing_file_ids( ) return {r["file_id"] for r in rows} + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> set[str]: + """Return the subset of ``file_ids`` that exist in *any* partition. + + Unscoped by design — only for the ``SUPER_ADMIN_MODE`` ``"all"`` wildcard. + """ + if not file_ids: + return set() + rows = await self.pool.fetch( + """ + SELECT DISTINCT file_id FROM files + WHERE file_id = ANY($1::text[]) + """, + file_ids, + ) + return {r["file_id"] for r in rows} + async def remove_file_from_all_workspaces( self, file_id: str, diff --git a/tests/unit/services/orchestrators/test_query_service.py b/tests/unit/services/orchestrators/test_query_service.py index 86a6b271e..8663b932b 100644 --- a/tests/unit/services/orchestrators/test_query_service.py +++ b/tests/unit/services/orchestrators/test_query_service.py @@ -107,6 +107,12 @@ async def get_existing_file_ids(self, partition, file_ids): allowed = self._existing.get(partition, set()) return [fid for fid in file_ids if fid in allowed] + async def get_existing_file_ids_any_partition(self, file_ids): + if self._existing is None: + return list(file_ids) + allowed = {fid for partition_ids in self._existing.values() for fid in partition_ids} + return [fid for fid in file_ids if fid in allowed] + def _config(mode="SimpleRag"): return SimpleNamespace( @@ -654,6 +660,27 @@ async def test_chat_attachments_drops_unindexed_and_reports_in_extra(): assert json.loads(chunk["extra"])["attachments"] == ["fa"] +@pytest.mark.asyncio +async def test_chat_attachments_duplicate_ids_deduped(): + retrieval = FakeRetrieval() + svc = _svc( + retrieval=retrieval, + llm=FakeLLM(chat_responses=["answer [Sources: none]"]), + workspace=FakeWorkspace(existing={"p1": {"fa"}}), + ) + chunk = await svc.chat( + partitions=["p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}, {"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + @pytest.mark.asyncio async def test_chat_stream_reports_indexed_attachments_in_extra(): svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) # only fa indexed in p1 @@ -676,9 +703,7 @@ async def test_chat_stream_reports_indexed_attachments_in_extra(): @pytest.mark.asyncio -async def test_chat_attachments_all_partition_is_fail_closed(): - # "all" is a search wildcard, not a real partition_name, so nothing validates - # against it — attachments scope to zero rather than passing through unchecked. +async def test_chat_attachments_all_partition_looks_up_any_partition(): retrieval = FakeRetrieval() svc = _svc( retrieval=retrieval, @@ -694,8 +719,23 @@ async def test_chat_attachments_all_partition_is_fail_closed(): prepare_sources=lambda d, w: [], model_name="m", ) - assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": []} - assert json.loads(chunk["extra"])["attachments"] == [] + assert retrieval.retrieve_multi_calls[0]["filter_params"] == {"file_id": ["fa"]} + assert json.loads(chunk["extra"])["attachments"] == ["fa"] + + +@pytest.mark.asyncio +async def test_chat_attachments_all_mixed_with_explicit_partition_raises(): + svc = _svc(workspace=FakeWorkspace(existing={"p1": {"fa"}})) + with pytest.raises(ValueError): + await svc.chat( + partitions=["all", "p1"], + payload={ + "messages": [{"role": "user", "content": "q"}], + "metadata": {"attachments": [{"id": "fa"}]}, + }, + prepare_sources=lambda d, w: [], + model_name="m", + ) @pytest.mark.asyncio From 3b5cf4ff88bdbf4dd93ab729932c1e70e8293634 Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Tue, 28 Jul 2026 12:11:32 +0200 Subject: [PATCH 08/14] fix(workspace): declare get_existing_file_ids_any_partition on the port PgWorkspaceRepository implemented it and WorkspaceService (typed against WorkspaceRepository) already called it, but it was missing from the abstract interface. --- openrag/core/ports/workspace_repo.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/openrag/core/ports/workspace_repo.py b/openrag/core/ports/workspace_repo.py index 7d3361029..748cf9a1e 100644 --- a/openrag/core/ports/workspace_repo.py +++ b/openrag/core/ports/workspace_repo.py @@ -53,6 +53,14 @@ async def get_existing_file_ids(self, partition: str, file_ids: list[str]) -> se """Return the subset of ``file_ids`` that actually exist in ``partition``.""" ... + @abstractmethod + async def get_existing_file_ids_any_partition(self, file_ids: list[str]) -> set[str]: + """Return the subset of ``file_ids`` that exist in *any* partition. + + Unscoped by design — only for the ``SUPER_ADMIN_MODE`` ``"all"`` wildcard. + """ + ... + @abstractmethod async def remove_file_from_all_workspaces(self, file_id: str, partition: str) -> None: """Detach ``file_id`` from every workspace in ``partition``.""" From 44d78777c11382d5215e4893acd74d9215f19786 Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Wed, 29 Jul 2026 09:57:52 +0000 Subject: [PATCH 09/14] docs(chat): correct the attachment-scoping security note The comment justified skipping an ownership check on the attachment ids by saying file_id is ANDed with the server-fixed partition. That stopped being the whole story when the "all" wildcard switched to an unscoped catalog lookup: on that path there is no partition to AND against, and the safety argument is the SUPER_ADMIN_MODE invariant instead. Spell both out, since the comment exists to justify the absence of a check. --- openrag/services/orchestrators/query_service.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 30e26c36a..04aa10f71 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -414,8 +414,11 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L partition = [scope.partition] filter_params = {"file_id": scope.file_ids} elif attachment_ids and partition: - # No security check on the ids: file_id is ANDed with the - # server-fixed partition, so a foreign id can never match. + # The ids need no ownership check of their own: file_id is ANDed with + # the server-fixed partition, so a foreign id can never match. The one + # exception is the "all" wildcard, whose lookup is deliberately + # unscoped — safe only because "all" reaches this layer solely for a + # SUPER_ADMIN_MODE admin (see _existing_file_ids). indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) filter_params = {"file_id": indexed_attachment_ids} From 8d65874a8a7734357de0e0bad05b4c111b2bad8e Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Wed, 29 Jul 2026 09:58:09 +0000 Subject: [PATCH 10/14] docs(chat): say attachments were 'searched', not 'leveraged' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wording requested in review. It is also the accurate word: the list reports what retrieval was scoped to, not what the answer drew on — that is extra.sources. --- openrag/services/orchestrators/query_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 04aa10f71..7a0128eac 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -607,7 +607,7 @@ async def chat( chunk["choices"][0]["message"]["content"] = clean extra = {"sources": filter_sources_by_citations(sources, citations)} if metadata.get("attachments"): - # Indicate which attachments were actually leveraged to generate the answer. + # Indicate which attachments were actually searched to generate the answer. extra["attachments"] = attachments chunk["extra"] = json.dumps(extra) return chunk From 1577b4e5d2fa5d0b5c1b410bb2e9dcf31691bd51 Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Wed, 29 Jul 2026 10:02:56 +0000 Subject: [PATCH 11/14] test(workspace): cover the catalog existence lookups against real Postgres get_existing_file_ids_any_partition backs attachment scoping for the openrag-all wildcard, but every test reaching it went through a fake, so its SQL was never executed anywhere in CI. Same for the partition-scoped sibling. Both are now exercised by the repos suite, which runs against a real database on every PR. The two scoping cases are mutation-checked: dropping the partition clause from the scoped query, or adding one to the unscoped query, each fails a test rather than passing quietly. --- .../integration/repos/test_workspace_repo.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/integration/repos/test_workspace_repo.py b/tests/integration/repos/test_workspace_repo.py index d52507365..cf7296938 100644 --- a/tests/integration/repos/test_workspace_repo.py +++ b/tests/integration/repos/test_workspace_repo.py @@ -99,6 +99,46 @@ async def test_get_file_workspaces_is_partition_scoped( assert in_a == ["ws-a"] +class TestExistingFileIds: + """The two catalog-existence lookups behind attachment scoping. + + Exercised against real SQL rather than a fake, so the ``ANY($1::text[])`` + predicate and the presence/absence of the partition clause are actually + executed — the scoping assertions below both fail if either clause is + changed. + """ + + async def test_partition_scoped_ignores_other_partitions(self, postgres_store: PostgresStore): + await _seed_partition_and_files(postgres_store, partition="a", file_ids=("f1", "f2")) + await _seed_partition_and_files(postgres_store, partition="b", file_ids=("f3",)) + found = await postgres_store.workspace_repo.get_existing_file_ids("a", ["f1", "f3", "ghost"]) + # f3 exists, but in partition "b" — a partition-scoped lookup must not see it. + assert found == {"f1"} + + async def test_any_partition_spans_partitions(self, postgres_store: PostgresStore): + await _seed_partition_and_files(postgres_store, partition="a", file_ids=("f1",)) + await _seed_partition_and_files(postgres_store, partition="b", file_ids=("f3",)) + found = await postgres_store.workspace_repo.get_existing_file_ids_any_partition( + ["f1", "f3", "ghost"], + ) + assert found == {"f1", "f3"} + + async def test_any_partition_reports_a_shared_file_id_once(self, postgres_store: PostgresStore): + # The same file_id in two partitions is two ``files`` rows. Callers treat + # the result as a membership set, so it must come back as a single entry. + # (Both the query's DISTINCT and the set-building enforce this; the test + # pins the contract, not either mechanism.) + await _seed_partition_and_files(postgres_store, partition="a", file_ids=("shared",)) + await _seed_partition_and_files(postgres_store, partition="b", file_ids=("shared",)) + found = await postgres_store.workspace_repo.get_existing_file_ids_any_partition(["shared"]) + assert found == {"shared"} + + async def test_empty_input_short_circuits(self, postgres_store: PostgresStore): + repo = postgres_store.workspace_repo + assert await repo.get_existing_file_ids("a", []) == set() + assert await repo.get_existing_file_ids_any_partition([]) == set() + + class TestDeleteWorkspace: async def test_returns_orphan_file_ids(self, postgres_store: PostgresStore): await _seed_partition_and_files(postgres_store) From 87bb7b02c1909f5346953cd3d609645b2ea63c15 Mon Sep 17 00:00:00 2001 From: Ahmath-Gadji Date: Wed, 29 Jul 2026 10:02:56 +0000 Subject: [PATCH 12/14] docs(chat): note that _existing_file_ids deduplicates The list is client-supplied, so repeats would otherwise be echoed into the retrieval filter and back out through extra.attachments. The dedupe is in the code; the docstring still described a plain subset. --- openrag/services/orchestrators/query_service.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 7a0128eac..243365aaa 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -483,7 +483,10 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L return payload, docs, web_results, indexed_attachment_ids async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: - """Order-preserving subset of ``file_ids`` indexed in ``partitions``. + """Order-preserving, deduplicated subset of ``file_ids`` indexed in ``partitions``. + + ``file_ids`` is client-supplied, so repeats are dropped rather than + echoed back into the filter and into ``extra.attachments``. ``"all"`` (``SUPER_ADMIN_MODE`` wildcard) takes an unscoped lookup instead of a per-partition one. From 6d5f4154a9db3e3aa258cd1e72505ee616fc600a Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Mon, 24 Aug 2026 16:59:35 +0200 Subject: [PATCH 13/14] fix: comments --- openrag/services/orchestrators/query_service.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index b712ecdaa..073d5c0ac 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -86,6 +86,7 @@ class _PrepareChatResult(NamedTuple): citation_protocol_active: bool indexed_attachment_ids: list[str] + _MAP_SYSTEM_PROMPT = """You are an AI assistant specialized in extracting and synthesizing relevant information from text. Your task: @@ -581,7 +582,9 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L }, ) payload["messages"] = new_messages - return _PrepareChatResult(payload, docs, web_results, retrieved_docs, retrieved_web_results, True, indexed_attachment_ids) + return _PrepareChatResult( + payload, docs, web_results, retrieved_docs, retrieved_web_results, True, indexed_attachment_ids + ) async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: """Order-preserving, deduplicated subset of ``file_ids`` indexed in ``partitions``. From c7d528e083dc6ea8dbb34ee13a3b3fc5094c56c0 Mon Sep 17 00:00:00 2001 From: Ewan Magdelaine Date: Tue, 25 Aug 2026 10:16:15 +0200 Subject: [PATCH 14/14] docs(api): document the attachments metadata field; trim verbose comments --- docs/content/docs/documentation/API.mdx | 25 +++++++++++++++++++ .../services/orchestrators/query_service.py | 10 ++------ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/docs/content/docs/documentation/API.mdx b/docs/content/docs/documentation/API.mdx index e800943bb..9d93a58f8 100644 --- a/docs/content/docs/documentation/API.mdx +++ b/docs/content/docs/documentation/API.mdx @@ -716,6 +716,7 @@ OpenAI-compatible text completion endpoint. | `spoken_style_answer` | `bool` | `false` | Generates a succinct spoken-style conversational answer based on the retrieved documents. | | `use_map_reduce` | `bool` | `false` | Uses a map-reduce strategy to aggregate information from multiple documents. See [map-reduce configuration](/openrag/documentation/env_vars/#map--reduce-configuration). | | `llm_override` | `object` | `null` | Overrides only the downstream LLM model name while still using OpenRAG's configured LLM endpoint and credentials. Accepts: `model` (string). Endpoint URL and API key are server-side configuration and cannot be changed by a client request. | +| `attachments` | `list[{"id": string}]` | `null` | Scopes RAG retrieval to a specific list of file IDs within the target partition, instead of searching the whole partition. Unknown or unindexed IDs are silently dropped; duplicates are deduplicated. The response's `extra.attachments` reports which IDs were actually used. | Examples: @@ -802,6 +803,30 @@ curl -X 'POST' 'http://localhost:8080/v1/chat/completions' \ }' ``` +```bash title="Scoping retrieval to specific attachments" +curl -X 'POST' 'http://localhost:8080/v1/chat/completions' \ + -H 'accept: application/json' \ + -H 'Authorization: Bearer YOUR_AUTH_TOKEN' \ + -H 'Content-Type: application/json' \ + -d '{ + "model": "openrag-{partition_name}", + "messages": [ + { + "role": "user", + "content": "summarize these files" + } + ], + "stream": false, + "metadata": { + "attachments": [ + {"id": "file-001"}, + {"id": "file-002"}, + {"id": "file-003"} + ] + } +}' +``` + ### 🔧 Tools diff --git a/openrag/services/orchestrators/query_service.py b/openrag/services/orchestrators/query_service.py index 073d5c0ac..393e0be93 100644 --- a/openrag/services/orchestrators/query_service.py +++ b/openrag/services/orchestrators/query_service.py @@ -477,11 +477,8 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L partition = [scope.partition] filter_params = {"file_id": scope.file_ids} elif attachment_ids and partition: - # The ids need no ownership check of their own: file_id is ANDed with - # the server-fixed partition, so a foreign id can never match. The one - # exception is the "all" wildcard, whose lookup is deliberately - # unscoped — safe only because "all" reaches this layer solely for a - # SUPER_ADMIN_MODE admin (see _existing_file_ids). + # No ownership check needed: file_id is ANDed with the server-fixed + # partition (or, for the "all" wildcard, SUPER_ADMIN_MODE-only). indexed_attachment_ids = await self._existing_file_ids(attachment_ids, partition) filter_params = {"file_id": indexed_attachment_ids} @@ -589,9 +586,6 @@ async def _prepare_chat(self, partition: list[str] | None, payload: dict, llm: L async def _existing_file_ids(self, file_ids: list[str], partitions: list[str]) -> list[str]: """Order-preserving, deduplicated subset of ``file_ids`` indexed in ``partitions``. - ``file_ids`` is client-supplied, so repeats are dropped rather than - echoed back into the filter and into ``extra.attachments``. - ``"all"`` (``SUPER_ADMIN_MODE`` wildcard) takes an unscoped lookup instead of a per-partition one. """