fix(chat): a question the planner splits keeps the chat's restrictions

.claude/rules/file-map.md

@@ -219,6 +219,7 @@ Full module index for LocalChat. **Keep this current** — update in the same co
 | `tests/unit/test_upload_isolation_and_size.py` | P1-1/P1-5 — each upload stages into its own directory so two of the same filename cannot collide or delete each other (H4), `MAX_CONTENT_LENGTH` is enforced while the body streams (M2), and more than one uvicorn worker aborts the boot (M7) |
 | `tests/unit/test_aggregator_carries_request_scope.py` | With the aggregator on, retrieval runs in the caller's workspace: the chat route binds the scope before retrieval, not only for the stream, and each `ThreadPoolExecutor` job runs in a copy of the caller's context. Both were missing, and every `local_docs` job came back empty |
 | `tests/unit/test_chat_additional_workspaces_authorised.py` | Chat's `additional_workspace_ids` are authorised like the primary workspace — a member may name their own, an admin any, a workspace API key none — and one foreign id refuses the request before any retrieval; the guard used to authorise the primary workspace only |
+| `tests/unit/test_multi_hop_keeps_the_request_scope.py` | ADR-6 — a question the planner splits passes the chat's `source_ids` and `additional_workspace_ids` to every sub-question, and its citations name their tier and weak relevance like the single-query path's |
 | `tests/unit/test_chat_conversation_is_authorised.py` | R-3 (QP-9 review) — a chat naming a conversation outside the authorised scope is a 404 before any retrieval or write, the lookup uses that scope, and the caller's own conversation is still answered |
 | `tests/integration/test_rating_ids_are_in_scope.py` | R-8 (QP-9 review) — against Postgres: the caller's own and the global tier's chunks are citable, a foreign, retired or archived one is not, nor a global one where the tier is closed; a message is found only in a live conversation of the scope |
 | `tests/unit/test_frontend_feedback_workspace.py` | R-8 — a rating sends the active workspace, so the server checks its ids there rather than in the default one |

CHANGELOG.md

@@ -19,6 +19,14 @@ OpenAI, `https://api.openai.com/v1`) and drop any `provider/` prefix from `CLOUD
   proxy server LocalChat never ran, and every fixed version pulls `boto3` into the image.
   The `pip-audit` suppression for it is removed; `SECURITY.md` item 12 is resolved.
 
+### Fixed
+
+- **A question the planner splits keeps the chat's restrictions.** Multi-hop retrieval
+  passed neither the chosen sources nor the additional workspaces to its sub-questions, so
+  a chat narrowed to some documents searched the whole workspace whenever the planner
+  judged a question multi-hop (ADR-6 recorded it). Its citations also lacked the tier, so
+  a global contribution was cited as a workspace document. Workspace isolation itself held.
+
 ### Changed
 
 - **The cloud fallback talks to an OpenAI-compatible endpoint directly.**

docs/ADR.md

@@ -443,13 +443,14 @@ settles it.
 - Merging an orchestration experiment to the default path on a qualitative impression.
 
 **Consequences to execute.** These make the code agree with the decision; none needs P2-3.
-- ⬜ **Multi-hop retrieval drops the caller's scoping** (a live defect, found while writing
-  this record). `get_rag_context_multi_hop` takes neither `source_ids` nor
-  `additional_workspace_ids`, so a user who narrowed the chat to specific sources and asks a
-  ≥ 7-word question the planner judges multi-hop gets retrieval across the whole workspace,
-  and a request spanning additional workspaces silently loses them. Workspace isolation
-  itself holds; `workspace_id` is still passed. Fix first: it is a correctness bug on the
-  default path, independent of everything else here.
+- ✅ (2026-11-11) **Multi-hop retrieval dropped the caller's scoping** (a live defect, found
+  while writing this record). `get_rag_context_multi_hop` took neither `source_ids` nor
+  `additional_workspace_ids`, so a user who narrowed the chat to specific sources and asked
+  a ≥ 7-word question the planner judged multi-hop got retrieval across the whole
+  workspace, and a request spanning additional workspaces silently lost them. Workspace
+  isolation itself held; `workspace_id` was still passed. Fixed: every sub-question now gets
+  both, and its citations carry `knowledge_tier` and `low_relevance` like the single-query
+  path's (`tests/unit/test_multi_hop_keeps_the_request_scope.py`).
 - ⬜ The aggregator path passes `filename_filter=[]` and receives no `source_ids` or
   `additional_workspace_ids` (`services/chat.py`, `retrieve_via_aggregator`). Latent while
   the flag is off; enabling it would change what gets searched. Wire them through or delete

src/services/chat.py

@@ -136,26 +136,27 @@ def get_rag_context(
         logger.warning("[RAG] No chunks retrieved from documents")
         return "", []
 
-    sources = [
-        {
-            "filename": r.filename,
-            "chunk_index": r.chunk_index,
-            "page_number": r.metadata.get("page_number"),
-            "section_title": r.metadata.get("section_title"),
-            "chunk_id": r.chunk_id,
-            # Carried so the interface can say the sources are weak rather than
-            # presenting them as if they were not. This is what a citation to an
-            # unrelated document looked like from the outside: indistinguishable.
-            "low_relevance": bool(r.metadata.get("low_relevance")),
-            # "global" when the chunk came from a contribution, so a citation says which
-            # tier it is (GKB-3); a workspace document is "local".
-            "knowledge_tier": r.metadata.get("knowledge_tier", "local"),
-        }
-        for r in results
-    ]
+    sources = [_citation(r) for r in results]
     return doc_processor.format_context_for_llm(results, max_length=config.MAX_CONTEXT_LENGTH), sources
 
 
+def _citation(r: RetrievalResult) -> dict[str, Any]:
+    return {
+        "filename": r.filename,
+        "chunk_index": r.chunk_index,
+        "page_number": r.metadata.get("page_number"),
+        "section_title": r.metadata.get("section_title"),
+        "chunk_id": r.chunk_id,
+        # Carried so the interface can say the sources are weak rather than
+        # presenting them as if they were not. This is what a citation to an
+        # unrelated document looked like from the outside: indistinguishable.
+        "low_relevance": bool(r.metadata.get("low_relevance")),
+        # "global" when the chunk came from a contribution, so a citation says which
+        # tier it is (GKB-3); a workspace document is "local".
+        "knowledge_tier": r.metadata.get("knowledge_tier", "local"),
+    }
+
+
 def get_web_context(message: str) -> tuple[str, list[dict[str, Any]]]:
     """Return (context_for_prompt, source_entries) for a web-enhanced query."""
     if config.MCP_ENABLED:
@@ -341,7 +342,10 @@ def retrieve_contexts(
             if plan is not None and plan.is_multi_hop:
                 local_context, sources = get_rag_context_multi_hop(
                     plan.sub_questions, doc_processor, filename_filter,
-                    chunks_retrieved_ref=chunks_retrieved_ref, scope=scope,
+                    chunks_retrieved_ref=chunks_retrieved_ref,
+                    additional_workspace_ids=additional_workspace_ids,
+                    source_ids=source_ids,
+                    scope=scope,
                     knowledge_scope=knowledge_scope,
                 )
             else:
@@ -373,6 +377,8 @@ def get_rag_context_multi_hop(
     doc_processor: Any,
     filename_filter: list[str],
     chunks_retrieved_ref: list[int] | None = None,
+    additional_workspace_ids: list[str] | None = None,
+    source_ids: list[str] | None = None,
     *,
     scope: Scope,
     knowledge_scope: KnowledgeScope = "local",
@@ -384,7 +390,10 @@ def get_rag_context_multi_hop(
         futures = {
             executor.submit(
                 doc_processor.retrieve_context, q,
-                filename_filter=filename_filter, scope=scope,
+                filename_filter=filename_filter,
+                additional_workspace_ids=additional_workspace_ids,
+                source_ids=source_ids or [],
+                scope=scope,
                 knowledge_scope=knowledge_scope,
             ): q
             for q in sub_questions
@@ -410,14 +419,7 @@ def get_rag_context_multi_hop(
 
     local_context = doc_processor.format_context_for_llm(merged, max_length=config.MAX_CONTEXT_LENGTH)
     sources = [
-        {
-            "filename": r.filename,
-            "chunk_index": r.chunk_index,
-            "page_number": r.metadata.get("page_number"),
-            "section_title": r.metadata.get("section_title"),
-            "chunk_id": r.chunk_id,
-            "combined_score": r.metadata.get("combined_score"),
-        }
+        {**_citation(r), "combined_score": r.metadata.get("combined_score")}
         for r in merged
     ]
     return local_context, sources

tests/unit/test_multi_hop_keeps_the_request_scope.py

@@ -0,0 +1,123 @@
+"""A question the planner splits keeps every restriction the single-query path keeps.
+
+ADR-6 recorded it as a live defect: `get_rag_context_multi_hop` took neither
+`source_ids` nor `additional_workspace_ids`, so a chat narrowed to chosen sources searched
+the whole workspace whenever the planner judged the question multi-hop, and a chat spanning
+extra workspaces silently lost them. Its citations also lacked `knowledge_tier`, so a
+global contribution in a multi-hop answer was cited as a workspace document (GKB-3).
+"""
+
+from __future__ import annotations
+
+from typing import Any
+from unittest.mock import MagicMock
+
+import pytest
+
+from src import config
+from src.rag.planner import QueryPlan
+from src.rag.retrieval import RetrievalResult
+from src.services import chat
+
+pytestmark = pytest.mark.unit
+
+
+@pytest.fixture(autouse=True)
+def _direct_retrieval(monkeypatch):
+    # Either flag sends retrieval down another path before multi-hop is reached, and the
+    # single-query control would then pass for the wrong reason.
+    monkeypatch.setattr(config, "AGGREGATOR_AGENT_ENABLED", False)
+    monkeypatch.setattr(config, "MCP_ENABLED", False)
+
+WS = "11111111-1111-1111-1111-111111111111"
+OTHER_WS = "22222222-2222-2222-2222-222222222222"
+
+
+def _processor(results: list[RetrievalResult]) -> MagicMock:
+    processor = MagicMock()
+    processor.retrieve_context.return_value = results
+    processor.format_context_for_llm.return_value = "context"
+    return processor
+
+
+def _result(chunk_id: int, tier: str = "local", low: bool = False) -> RetrievalResult:
+    return RetrievalResult(
+        chunk_text="text", filename=f"doc{chunk_id}.md", chunk_index=0,
+        similarity=0.9 - chunk_id / 100,
+        metadata={"knowledge_tier": tier, "low_relevance": low}, chunk_id=chunk_id,
+    )
+
+
+def _retrieve(processor: MagicMock, plan: QueryPlan, **kwargs: Any) -> list[dict[str, Any]]:
+    fields = {"message": "a question", "use_rag": True, "enhance": False,
+              "conversation_id": None, "knowledge_scope": kwargs.pop("knowledge_scope", "local")}
+    db = MagicMock()
+    db.conversations.document_filter.return_value = []
+    _, _, sources, _ = chat.retrieve_contexts(
+        fields, processor, db, [0], plan=plan, scope=WS, **kwargs,
+    )
+    return sources
+
+
+MULTI_HOP = QueryPlan(sub_questions=["first part?", "second part?"], estimated_hops=2)
+
+
+class TestEverySubQuestionKeepsTheRequestScope:
+    def test_chosen_sources_reach_every_sub_question(self):
+        processor = _processor([])
+        _retrieve(processor, MULTI_HOP, source_ids=["src-1", "src-2"])
+        assert [c.kwargs["source_ids"] for c in processor.retrieve_context.call_args_list] == [
+            ["src-1", "src-2"], ["src-1", "src-2"],
+        ]
+
+    def test_additional_workspaces_reach_every_sub_question(self):
+        processor = _processor([])
+        _retrieve(processor, MULTI_HOP, additional_workspace_ids=[OTHER_WS])
+        assert [c.kwargs["additional_workspace_ids"] for c in processor.retrieve_context.call_args_list] == [
+            [OTHER_WS], [OTHER_WS],
+        ]
+
+    def test_without_either_the_sub_questions_search_the_workspace_alone(self):
+        processor = _processor([])
+        _retrieve(processor, MULTI_HOP)
+        assert [
+            (c.kwargs["source_ids"], c.kwargs["additional_workspace_ids"])
+            for c in processor.retrieve_context.call_args_list
+        ] == [([], None), ([], None)]
+
+    def test_every_sub_question_gets_all_five_restrictions_of_the_request(self):
+        processor = _processor([])
+        db_filter = ["chosen.md"]
+        fields = {"message": "a question", "use_rag": True, "enhance": False,
+                  "conversation_id": "c1", "knowledge_scope": "hybrid"}
+        db = MagicMock()
+        db.conversations.document_filter.return_value = db_filter
+        chat.retrieve_contexts(fields, processor, db, [0], plan=MULTI_HOP, scope=WS,
+                               source_ids=["src-1"], additional_workspace_ids=[OTHER_WS])
+        assert [
+            {k: c.kwargs[k] for k in ("scope", "knowledge_scope", "filename_filter",
+                                      "source_ids", "additional_workspace_ids")}
+            for c in processor.retrieve_context.call_args_list
+        ] == [{"scope": WS, "knowledge_scope": "hybrid", "filename_filter": db_filter,
+               "source_ids": ["src-1"], "additional_workspace_ids": [OTHER_WS]}] * 2
+
+    def test_the_single_query_path_still_passes_the_same_restriction(self):
+        processor = _processor([])
+        _retrieve(processor, QueryPlan(sub_questions=["one?"], estimated_hops=1),
+                  source_ids=["src-1"], additional_workspace_ids=[OTHER_WS])
+        call = processor.retrieve_context.call_args
+        assert (call.kwargs["source_ids"], call.kwargs["additional_workspace_ids"]) == (
+            ["src-1"], [OTHER_WS],
+        )
+
+
+class TestAMultiHopCitationSaysWhatItIs:
+    def test_a_global_chunk_is_cited_as_global_and_a_workspace_one_as_local(self):
+        processor = _processor([_result(1, tier="global"), _result(2, tier="local")])
+        sources = _retrieve(processor, MULTI_HOP, knowledge_scope="hybrid")
+        assert {s["chunk_id"]: s["knowledge_tier"] for s in sources} == {1: "global", 2: "local"}
+
+    def test_a_weak_chunk_is_marked_low_relevance(self):
+        processor = _processor([_result(1, low=True), _result(2, low=False)])
+        sources = _retrieve(processor, MULTI_HOP)
+        assert {s["chunk_id"]: s["low_relevance"] for s in sources} == {1: True, 2: False}