From 105b2628de723046882b1eaf29027e9b683387c8 Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Thu, 27 Aug 2026 16:09:08 +0300 Subject: [PATCH] Page the filter modal, and list the selected A selection outside the page stayed applied while its checkbox was gone, so there was no way to remove it. Appending those documents to the page instead loses the bound the page exists to keep, since selections accumulate across searches. Both listings page at `DOCUMENT_PAGE`, and `Selected` switches between them, so the mounted widgets stay bounded whichever is showing and every selection is a page away rather than unreachable. The count reads the checkboxes on screen, so narrowing as the user types reports what is visible. A listing with nothing in it says so, instead of leaving an empty box that reads as still loading. --- .../rag/chat/widgets/document_filter_modal.py | 145 ++++++++--- tests/chat/test_chat_app.py | 245 ++++++++++++++++++ 2 files changed, 360 insertions(+), 30 deletions(-) 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 84f5a47b..372e92e3 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 @@ -6,6 +6,7 @@ from textual.screen import ModalScreen from textual.widgets import Button, Checkbox, Input, Static from haiku.rag.client import HaikuRAG +from haiku.rag.tools.filters import build_document_id_filter from haiku.rag.utils import escape_sql_string # Documents listed at once. Mounting a checkbox per document wedges the modal on @@ -21,6 +22,20 @@ class DocumentCheckbox(Checkbox): self.search_text = label +def _labelled(docs) -> list[tuple[str, str]]: + """Each document's label and id, sorted. The database is named alongside the + title, which a title alone does not say.""" + return 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 + ) + + def search_filter(term: str) -> str | None: """A document filter matching `term` in a title or URI, or None for no term. @@ -121,8 +136,13 @@ class DocumentFilterModal(ModalScreen): self.client = client self.initial_selected = selected or [] self._selected: set[str] = set(self.initial_selected) - self._shown = 0 self._matching = 0 + self._search = "" + self._page = 0 + # Selections outside the page stay applied, and a checkbox is the only + # way to remove one, so they are reachable through their own listing + # rather than appended to this one, which the page bound has to hold. + self._listing_selected = False def compose(self) -> ComposeResult: with Vertical(id="filter-container"): @@ -132,6 +152,9 @@ class DocumentFilterModal(ModalScreen): yield Static("Loading...", id="loading-indicator") yield Static("", id="filter-footer") with Horizontal(id="button-row"): + yield Button("Selected", id="selected-btn", variant="default") + yield Button("Prev", id="prev-btn", variant="default") + yield Button("Next", id="next-btn", variant="default") yield Button("Cancel", id="cancel-btn", variant="default") yield Button("Apply", id="apply-btn", variant="primary") @@ -139,58 +162,94 @@ class DocumentFilterModal(ModalScreen): """Load the first page of documents when mounted.""" await self._load_documents() - async def _load_documents(self, search: str = "") -> None: - """Show one page of documents, narrowed by `search` when given.""" - document_filter = search_filter(search) - docs = await self.client.list_documents( - limit=DOCUMENT_PAGE, filter=document_filter - ) - self._matching = await self.client.count_documents(filter=document_filter) + async def _load_documents(self, search: str | None = None) -> None: + """Show one page of whichever listing is on screen. + + Either the documents matching the search, or the selected ones. Both + page at `DOCUMENT_PAGE`, so the mounted widgets stay bounded whichever + is showing and every selection stays reachable. + """ + if search is not None: + self._search = search + self._page = 0 + self._listing_selected = False + + if self._listing_selected: + ids = sorted(self._selected) + self._matching = len(ids) + page = ids[self._page * DOCUMENT_PAGE : (self._page + 1) * DOCUMENT_PAGE] + docs = ( + list( + await self.client.list_documents( + filter=build_document_id_filter(page) + ) + ) + if page + else [] + ) + else: + document_filter = search_filter(self._search) + docs = list( + await self.client.list_documents( + limit=DOCUMENT_PAGE, + offset=self._page * DOCUMENT_PAGE, + filter=document_filter, + ) + ) + self._matching = await self.client.count_documents(filter=document_filter) filter_list = self.query_one("#filter-list", VerticalScroll) await filter_list.remove_children() - # Sort the interleaved page and label each document's database, which - # a title alone does not say. - 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 = [ DocumentCheckbox(label, doc_id, value=doc_id in self._selected) - for label, doc_id in labelled + for label, doc_id in _labelled(docs) ] if boxes: await filter_list.mount_all(boxes) + else: + # Otherwise the list is an empty box, indistinguishable from one + # still loading. + empty = ( + "Nothing selected." if self._listing_selected else "No documents match." + ) + await filter_list.mount(Static(empty, id="filter-empty")) - self._shown = len(boxes) self._update_footer() + @property + def _pages(self) -> int: + """Pages the current listing spans, at least one.""" + return max(1, -(-self._matching // DOCUMENT_PAGE)) + def _update_footer(self) -> None: - """Update the footer with the selection and how much of the corpus is shown.""" + """Report the selection, and where in the listing this page sits. + + Counted from the checkboxes on screen, so narrowing the page as the user + types reports what they can actually see. + """ footer = self.query_one("#filter-footer", Static) count = len(self._selected) if count == 0: state = "[dim]No filter (all documents)[/dim]" else: state = f"[bold]{count}[/bold] document(s) selected" - if self._matching > self._shown: + if self._listing_selected: + state += " [dim]— listing the selected[/dim]" + visible = sum(1 for box in self.query(DocumentCheckbox) if box.display) + if self._pages > 1: state += ( - f" [dim]— showing {self._shown} of {self._matching};" + f" [dim]— page {self._page + 1} of {self._pages}" + f" ({self._matching} total)[/dim]" + ) + elif self._matching > visible: + state += ( + f" [dim]— showing {visible} of {self._matching};" " type and press enter to search[/dim]" ) footer.update(state) - def on_checkbox_changed(self, event: Checkbox.Changed) -> None: + async def on_checkbox_changed(self, event: Checkbox.Changed) -> None: """Handle checkbox state changes.""" checkbox = event.checkbox if not isinstance(checkbox, DocumentCheckbox): @@ -201,6 +260,14 @@ class DocumentFilterModal(ModalScreen): else: self._selected.discard(checkbox.doc_id) + if self._listing_selected: + # This listing is the selection, so removing one changes both what + # it holds and how far it runs. The last page can stop existing. + pages = max(1, -(-len(self._selected) // DOCUMENT_PAGE)) + self._page = min(self._page, pages - 1) + await self._load_documents() + return + self._update_footer() async def on_input_submitted(self, event: Input.Submitted) -> None: @@ -216,9 +283,27 @@ class DocumentFilterModal(ModalScreen): checkbox.display = ( search_term == "" or search_term in checkbox.search_text.lower() ) + self._update_footer() - def on_button_pressed(self, event: Button.Pressed) -> None: + async def _turn_to(self, page: int) -> None: + """Show `page` of the current listing, if there is one.""" + if 0 <= page < self._pages: + self._page = page + await self._load_documents() + + async def on_button_pressed(self, event: Button.Pressed) -> None: """Handle button presses.""" + if event.button.id == "selected-btn": + self._listing_selected = not self._listing_selected + self._page = 0 + await self._load_documents() + return + if event.button.id == "prev-btn": + await self._turn_to(self._page - 1) + return + if event.button.id == "next-btn": + await self._turn_to(self._page + 1) + return if event.button.id == "cancel-btn": self.action_cancel() elif event.button.id == "apply-btn": diff --git a/tests/chat/test_chat_app.py b/tests/chat/test_chat_app.py index 2da58fca..28b82427 100644 --- a/tests/chat/test_chat_app.py +++ b/tests/chat/test_chat_app.py @@ -687,6 +687,251 @@ class TestDocumentSelectionIdentity: assert modal._selected == {"id-one"} +class TestKeepingSelectionsReachable: + """A selection applies whether or not the page shows it, and a checkbox is + the only way to remove one.""" + + @pytest.mark.asyncio + async def test_every_selection_is_reachable_and_the_page_stays_bounded( + self, temp_db_path: Path + ): + """Selections accumulate across searches. Appending them to the results + loses the bound the page exists to keep, so they get their own listing, + paged the same way.""" + from textual.widgets import Button, Static + + from haiku.rag.chat.widgets.document_filter_modal import ( + DOCUMENT_PAGE, + DocumentCheckbox, + DocumentFilterModal, + ) + from haiku.rag.store.models.document import Document + + picked = [ + Document(id=f"sel-{i:04d}", content="", title=f"Selected {i:04d}") + for i in range(DOCUMENT_PAGE + 20) + ] + by_id = {d.id: d for d in picked} + matched = [ + Document(id=f"hit-{i}", content="", title=f"Hit {i}") for i in range(5) + ] + + client = AsyncMock() + client.covers_multiple = False + client.source_names = () + client.count_documents.return_value = 5 + + async def listing(limit=None, offset=0, filter=None): + if filter and filter.startswith("id IN"): + ids = filter[len("id IN (") : -1].replace("'", "").split(", ") + return [by_id[i] for i in ids] + return matched + + client.list_documents.side_effect = listing + + modal = DocumentFilterModal( + client=client, selected=[d.id or "" for d in picked] + ) + app, _ = _make_app(temp_db_path, client) + with ( + patch("haiku.rag.chat.app.HaikuRAG") as stub, + _covering_returns(stub, client), + ): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + + # Results are the results: selections are not appended to them. + assert len(list(modal.query(DocumentCheckbox))) == len(matched) + + await modal.on_button_pressed( + Button.Pressed(modal.query_one("#selected-btn", Button)) + ) + await pilot.pause() + first = [b.doc_id for b in modal.query(DocumentCheckbox)] + footer = str(modal.query_one("#filter-footer", Static).content) + + await modal.on_button_pressed( + Button.Pressed(modal.query_one("#next-btn", Button)) + ) + await pilot.pause() + second = [b.doc_id for b in modal.query(DocumentCheckbox)] + + assert len(first) == DOCUMENT_PAGE + assert "page 1 of 2" in footer + # The rest are on the next page, so every selection can be removed. + assert len(second) == 20 + assert set(first) | set(second) == {d.id for d in picked} + + @pytest.mark.asyncio + async def test_deselecting_updates_the_selected_listing(self, temp_db_path: Path): + """The listing is the selection, so removing one changes what it holds + and how far it runs. Its last page can stop existing.""" + from textual.widgets import Button, Checkbox, Static + + from haiku.rag.chat.widgets.document_filter_modal import ( + DOCUMENT_PAGE, + DocumentCheckbox, + DocumentFilterModal, + ) + from haiku.rag.store.models.document import Document + + picked = [ + Document(id=f"sel-{i:04d}", content="", title=f"Selected {i:04d}") + for i in range(DOCUMENT_PAGE + 1) + ] + by_id = {d.id: d for d in picked} + + client = AsyncMock() + client.covers_multiple = False + client.source_names = () + client.count_documents.return_value = 0 + + async def listing(limit=None, offset=0, filter=None): + if filter and filter.startswith("id IN"): + ids = filter[len("id IN (") : -1].replace("'", "").split(", ") + return [by_id[i] for i in ids] + return [] + + client.list_documents.side_effect = listing + + modal = DocumentFilterModal( + client=client, selected=[d.id or "" for d in picked] + ) + app, _ = _make_app(temp_db_path, client) + with ( + patch("haiku.rag.chat.app.HaikuRAG") as stub, + _covering_returns(stub, client), + ): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + await modal.on_button_pressed( + Button.Pressed(modal.query_one("#selected-btn", Button)) + ) + await pilot.pause() + await modal.on_button_pressed( + Button.Pressed(modal.query_one("#next-btn", Button)) + ) + await pilot.pause() + + [only] = list(modal.query(DocumentCheckbox)) + assert only.doc_id == "sel-0200" + # Awaited rather than posted: the handler reads the database, so + # a single pause need not have flushed it. + await modal.on_checkbox_changed(Checkbox.Changed(only, False)) + await pilot.pause() + + remaining = [b.doc_id for b in modal.query(DocumentCheckbox)] + footer = str(modal.query_one("#filter-footer", Static).content) + + # The row is gone from the listing, not merely unchecked. + assert "sel-0200" not in remaining + assert len(remaining) == DOCUMENT_PAGE + assert modal._selected == {d.id for d in picked} - {"sel-0200"} + # The page it was on no longer exists, so the modal does not report it. + assert modal._page == 0 + assert "page" not in footer + assert f"[bold]{DOCUMENT_PAGE}[/bold] document(s) selected" in footer + + @pytest.mark.asyncio + async def test_the_results_listing_pages_too(self, temp_db_path: Path): + """More documents match than one page holds, so the rest are a page + away rather than unreachable.""" + from textual.widgets import Button + + from haiku.rag.chat.widgets.document_filter_modal import ( + DOCUMENT_PAGE, + DocumentFilterModal, + ) + from haiku.rag.store.models.document import Document + + client = AsyncMock() + client.covers_multiple = False + client.source_names = () + client.count_documents.return_value = DOCUMENT_PAGE * 2 + client.list_documents.return_value = [ + Document(id="d1", content="", title="One") + ] + + modal = DocumentFilterModal(client=client) + app, _ = _make_app(temp_db_path, client) + with ( + patch("haiku.rag.chat.app.HaikuRAG") as stub, + _covering_returns(stub, client), + ): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + assert client.list_documents.await_args.kwargs["offset"] == 0 + + await modal.on_button_pressed( + Button.Pressed(modal.query_one("#next-btn", Button)) + ) + await pilot.pause() + + assert client.list_documents.await_args.kwargs["offset"] == DOCUMENT_PAGE + + @pytest.mark.asyncio + async def test_a_search_matching_nothing_says_so(self, temp_db_path: Path): + """An empty list is indistinguishable from one still loading.""" + from textual.widgets import Static + + from haiku.rag.chat.widgets.document_filter_modal import DocumentFilterModal + + client = AsyncMock() + client.covers_multiple = False + client.source_names = () + client.list_documents.return_value = [] + client.count_documents.return_value = 0 + + modal = DocumentFilterModal(client=client) + app, _ = _make_app(temp_db_path, client) + with ( + patch("haiku.rag.chat.app.HaikuRAG") as stub, + _covering_returns(stub, client), + ): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + + empty = modal.query_one("#filter-empty", Static) + assert "No documents match" in str(empty.content) + + @pytest.mark.asyncio + async def test_narrowing_as_you_type_updates_the_count(self, temp_db_path: Path): + from textual.widgets import Input, Static + + from haiku.rag.chat.widgets.document_filter_modal import DocumentFilterModal + from haiku.rag.store.models.document import Document + + client = AsyncMock() + client.covers_multiple = False + client.source_names = () + client.list_documents.return_value = [ + Document(id="id-one", content="", title="Capital region"), + Document(id="id-two", content="", title="Nobel laureates"), + ] + client.count_documents.return_value = 9 + + modal = DocumentFilterModal(client=client) + app, _ = _make_app(temp_db_path, client) + with ( + patch("haiku.rag.chat.app.HaikuRAG") as stub, + _covering_returns(stub, client), + ): + async with app.run_test() as pilot: + await app.push_screen(modal) + await pilot.pause() + footer = modal.query_one("#filter-footer", Static) + assert "showing 2 of 9" in str(footer.content) + + modal.on_input_changed(Input.Changed(Input(), "nobel")) + await pilot.pause() + + assert "showing 1 of 9" in str(footer.content) + + class TestDocumentSearchFilter: """The filter modal shows one page and asks the database for the rest, so the typed term reaches SQL."""