From 307e250f290816c31cccbbcb8e5afa12310ec077 Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Mon, 24 Aug 2026 09:13:20 +0300 Subject: [PATCH] Select documents to filter by id, not by displayed name A title repeats within a corpus and across databases, so a substring match on the displayed name widened the filter to documents the user did not pick. The label names the database. --- CHANGELOG.md | 1 + haiku_rag_slim/haiku/rag/chat/app.py | 4 +- .../rag/chat/widgets/document_filter_modal.py | 36 +++++++++---- haiku_rag_slim/haiku/rag/tools/__init__.py | 6 ++- haiku_rag_slim/haiku/rag/tools/filters.py | 16 ++++++ tests/chat/test_chat_app.py | 51 +++++++++++++++++-- tests/tools/test_filters.py | 22 +++++++- 7 files changed, 119 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc682f27..a1ddf44f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Fixed +- The chat TUI's document filter selects documents by id and names each document's database, instead of matching the displayed title or URI as a substring across every database. - `doctor`'s docling-serve probe sends `X-Api-Key`, so an instance requiring a key is reported reachable rather than unreachable. - The picture-description request to the public OpenAI endpoint sends `OPENAI_API_KEY`; it carried no authorization header. - `haiku-rag` prints the message and exits when the configured embedder does not match the database, instead of raising a traceback. diff --git a/haiku_rag_slim/haiku/rag/chat/app.py b/haiku_rag_slim/haiku/rag/chat/app.py index 77650495..a0d78513 100644 --- a/haiku_rag_slim/haiku/rag/chat/app.py +++ b/haiku_rag_slim/haiku/rag/chat/app.py @@ -416,11 +416,11 @@ class ChatApp(App): def on_document_filter_modal_filter_changed(self, event: Any) -> None: """Handle document filter changes from modal.""" - from haiku.rag.tools.filters import build_multi_document_filter + from haiku.rag.tools.filters import build_document_id_filter self._document_filter = event.selected - doc_filter = build_multi_document_filter(self._document_filter) + doc_filter = build_document_id_filter(self._document_filter) for namespace, state_type in ( (RAG_STATE_NAMESPACE, RAGState), (ANALYSIS_STATE_NAMESPACE, AnalysisState), diff --git a/haiku_rag_slim/haiku/rag/chat/widgets/document_filter_modal.py b/haiku_rag_slim/haiku/rag/chat/widgets/document_filter_modal.py index 597dd1ac..ef556a54 100644 --- a/haiku_rag_slim/haiku/rag/chat/widgets/document_filter_modal.py +++ b/haiku_rag_slim/haiku/rag/chat/widgets/document_filter_modal.py @@ -148,19 +148,37 @@ class DocumentFilterModal(ModalScreen): await filter_list.remove_children() # The page is picked to represent every database; sorting is so it reads - # like a list rather than in whatever order the tables returned. - names = sorted(doc.title or doc.uri or str(doc.id) for doc in docs) + # like a list rather than in whatever order the tables returned. The label + # names the database, since a title alone does not say which one it is in. + labelled = sorted( + ( + ( + f"{doc.title or doc.uri or doc.id}" + + (f" ({doc.source})" if doc.source else ""), + doc.id, + ) + for doc in docs + if doc.id is not None + ), + key=lambda pair: pair[0], + ) boxes = [] - for position, display_name in enumerate(names): + for position, (label, doc_id) in enumerate(labelled): checkbox = Checkbox( - display_name, - value=display_name in self._selected, - # Positional, because titles repeat and a repeated id is an error. + label, + value=doc_id in self._selected, + # Positional, because a label is not unique and a repeated + # widget id is an error. id=f"doc-{position}", classes="doc-checkbox", ) - checkbox._doc_id = display_name # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + # The selection is the document id: a title repeats within a corpus + # and across databases, so selecting by name widens to documents the + # user did not pick. + checkbox._doc_id = doc_id # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] + # Not `_label`: Textual's ToggleButton owns that name. + checkbox._search_text = label # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] boxes.append(checkbox) if boxes: await filter_list.mount_all(boxes) @@ -207,8 +225,8 @@ class DocumentFilterModal(ModalScreen): filter_list = self.query_one("#filter-list", VerticalScroll) for checkbox in filter_list.query(Checkbox): - doc_id = getattr(checkbox, "_doc_id", "") - if search_term == "" or search_term in doc_id.lower(): + label = getattr(checkbox, "_search_text", "") + if search_term == "" or search_term in label.lower(): checkbox.display = True else: checkbox.display = False diff --git a/haiku_rag_slim/haiku/rag/tools/__init__.py b/haiku_rag_slim/haiku/rag/tools/__init__.py index 156a003e..d6fa30c8 100644 --- a/haiku_rag_slim/haiku/rag/tools/__init__.py +++ b/haiku_rag_slim/haiku/rag/tools/__init__.py @@ -1,10 +1,14 @@ from haiku.rag.tools.context import RAGDeps from haiku.rag.tools.document import create_document_toolset -from haiku.rag.tools.filters import build_multi_document_filter +from haiku.rag.tools.filters import ( + build_document_id_filter, + build_multi_document_filter, +) from haiku.rag.tools.search import create_search_toolset __all__ = [ "RAGDeps", + "build_document_id_filter", "build_multi_document_filter", "create_document_toolset", "create_search_toolset", diff --git a/haiku_rag_slim/haiku/rag/tools/filters.py b/haiku_rag_slim/haiku/rag/tools/filters.py index 66d7f5b9..164eb78e 100644 --- a/haiku_rag_slim/haiku/rag/tools/filters.py +++ b/haiku_rag_slim/haiku/rag/tools/filters.py @@ -1,3 +1,6 @@ +from haiku.rag.utils import escape_sql_string + + def _build_document_filter(document_name: str) -> str: """Build SQL filter for document name matching. @@ -12,6 +15,19 @@ def _build_document_filter(document_name: str) -> str: ) +def build_document_id_filter(document_ids: list[str]) -> str | None: + """SQL filter matching exactly these documents, or None for an empty list. + + Unlike name matching, an id identifies one document: names repeat within a + corpus and across databases, so a name filter can widen to documents the + caller did not pick. + """ + if not document_ids: + return None + ids = ", ".join(f"'{escape_sql_string(i)}'" for i in document_ids) + return f"id IN ({ids})" + + def build_multi_document_filter(document_names: list[str]) -> str | None: """Build SQL filter for multiple document names (OR combined). diff --git a/tests/chat/test_chat_app.py b/tests/chat/test_chat_app.py index 02123d04..503a56d5 100644 --- a/tests/chat/test_chat_app.py +++ b/tests/chat/test_chat_app.py @@ -386,22 +386,27 @@ async def test_document_filter_updates_rag_state(temp_db_path: Path): """Test that selecting document filters updates RAGState.document_filter.""" from haiku.rag.chat.app import RAG_STATE_NAMESPACE from haiku.rag.chat.widgets.document_filter_modal import DocumentFilterModal - from haiku.rag.tools.filters import build_multi_document_filter + from haiku.rag.tools.filters import build_document_id_filter app, mock_client = _make_app_with_state(temp_db_path) with patch("haiku.rag.chat.app.HaikuRAG", return_value=mock_client): async with app.run_test(): - # Simulate the FilterChanged message - selected = ["AI Overview", "ML Basics"] + # The selection is document ids, so a repeated title cannot widen it. + selected = [ + "6f1c2d4e-0000-4000-8000-000000000001", + "6f1c2d4e-0000-4000-8000-000000000002", + ] app.on_document_filter_modal_filter_changed( DocumentFilterModal.FilterChanged(selected) ) # RAGState.document_filter should be set rag_state = RAGState.model_validate(app._state[RAG_STATE_NAMESPACE]) - expected_filter = build_multi_document_filter(selected) + expected_filter = build_document_id_filter(selected) assert rag_state.document_filter == expected_filter + assert rag_state.document_filter is not None + assert "LIKE" not in rag_state.document_filter # The state snapshot should also reflect the change assert app._state["rag"]["document_filter"] == expected_filter @@ -541,6 +546,44 @@ async def test_visual_grounding_uses_the_database_holding_the_citation(tmp_path) assert push.await_args.args[0].client is owner +class TestDocumentSelectionIdentity: + """Two documents can share a title, within a corpus and across databases, so + the selection is by id and the label says which database.""" + + @pytest.mark.asyncio + async def test_a_repeated_title_selects_one_document(self, temp_db_path: Path): + from textual.widgets import Checkbox + + from haiku.rag.chat.widgets.document_filter_modal import DocumentFilterModal + from haiku.rag.store.models.document import Document + + client = AsyncMock() + client._federated = {"arxiv": "a", "wiki": "b"} + client.list_documents.return_value = [ + Document(id="id-one", content="", title="Capital region", source="arxiv"), + Document(id="id-two", content="", title="Capital region", source="wiki"), + ] + client.count_documents.return_value = 2 + + modal = DocumentFilterModal(client=client) + app, _ = _make_app(temp_db_path, client) + with patch("haiku.rag.chat.app.HaikuRAG", return_value=client): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + + boxes = list(modal.query(Checkbox)) + selected_ids = [getattr(box, "_doc_id", None) for box in boxes] + assert selected_ids == ["id-one", "id-two"] + labels = [str(b.label) for b in boxes] + assert "Capital region (arxiv)" in labels + assert "Capital region (wiki)" in labels + + boxes[0].value = True + await pilot.pause() + assert modal._selected == {"id-one"} + + class TestDocumentSearchFilter: """The filter modal shows one page and asks the database for the rest, so the typed term reaches SQL.""" diff --git a/tests/tools/test_filters.py b/tests/tools/test_filters.py index 85d02290..5d8bfe9b 100644 --- a/tests/tools/test_filters.py +++ b/tests/tools/test_filters.py @@ -1,4 +1,8 @@ -from haiku.rag.tools.filters import _build_document_filter, build_multi_document_filter +from haiku.rag.tools.filters import ( + _build_document_filter, + build_document_id_filter, + build_multi_document_filter, +) def test_build_document_filter_simple(): @@ -45,3 +49,19 @@ def test_build_multi_document_filter_multiple(): assert "doc1" in result assert "doc2" in result assert " OR (" in result + + +def test_build_document_id_filter_empty(): + """No selection means no filter.""" + assert build_document_id_filter([]) is None + + +def test_build_document_id_filter_matches_exactly(): + """An id filter never widens: names repeat, ids do not.""" + result = build_document_id_filter(["id-one", "id-two"]) + + assert result == "id IN ('id-one', 'id-two')" + + +def test_build_document_id_filter_escapes_quotes(): + assert build_document_id_filter(["O'Reilly"]) == "id IN ('O''Reilly')"