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."""