Fix document-filter searches after the initial load
Every load looked up the loading indicator, which only the first page has, so pressing enter in the search box raised `NoMatches`. The indicator is a child of the list `remove_children()` already clears. A document's id and search text travel on a `DocumentCheckbox` instead of being assigned onto a `Checkbox` behind type suppressions. The positional widget ids are gone; nothing queried them.
This commit is contained in:
parent
ea6b864f6e
commit
0e0ce8d3f0
2 changed files with 68 additions and 36 deletions
|
|
@ -13,6 +13,14 @@ from haiku.rag.utils import escape_sql_string
|
|||
DOCUMENT_PAGE = 200
|
||||
|
||||
|
||||
class DocumentCheckbox(Checkbox):
|
||||
def __init__(self, label: str, doc_id: str, *, value: bool) -> None:
|
||||
super().__init__(label, value=value, classes="doc-checkbox")
|
||||
self.doc_id = doc_id
|
||||
# `label` is a reactive Text; narrowing the page wants the plain string.
|
||||
self.search_text = label
|
||||
|
||||
|
||||
def search_filter(term: str) -> str | None:
|
||||
"""A document filter matching `term` in a title or URI, or None for no term.
|
||||
|
||||
|
|
@ -139,10 +147,6 @@ class DocumentFilterModal(ModalScreen):
|
|||
)
|
||||
self._matching = await self.client.count_documents(filter=document_filter)
|
||||
|
||||
loading = self.query_one("#loading-indicator", Static)
|
||||
if loading.parent is not None:
|
||||
loading.remove()
|
||||
|
||||
filter_list = self.query_one("#filter-list", VerticalScroll)
|
||||
await filter_list.remove_children()
|
||||
|
||||
|
|
@ -161,23 +165,10 @@ class DocumentFilterModal(ModalScreen):
|
|||
key=lambda pair: pair[0],
|
||||
)
|
||||
|
||||
boxes = []
|
||||
for position, (label, doc_id) in enumerate(labelled):
|
||||
checkbox = Checkbox(
|
||||
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",
|
||||
)
|
||||
# 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)
|
||||
boxes = [
|
||||
DocumentCheckbox(label, doc_id, value=doc_id in self._selected)
|
||||
for label, doc_id in labelled
|
||||
]
|
||||
if boxes:
|
||||
await filter_list.mount_all(boxes)
|
||||
|
||||
|
|
@ -202,14 +193,13 @@ class DocumentFilterModal(ModalScreen):
|
|||
def on_checkbox_changed(self, event: Checkbox.Changed) -> None:
|
||||
"""Handle checkbox state changes."""
|
||||
checkbox = event.checkbox
|
||||
doc_id = getattr(checkbox, "_doc_id", None)
|
||||
if doc_id is None:
|
||||
if not isinstance(checkbox, DocumentCheckbox):
|
||||
return
|
||||
|
||||
if event.value:
|
||||
self._selected.add(doc_id)
|
||||
self._selected.add(checkbox.doc_id)
|
||||
else:
|
||||
self._selected.discard(doc_id)
|
||||
self._selected.discard(checkbox.doc_id)
|
||||
|
||||
self._update_footer()
|
||||
|
||||
|
|
@ -222,12 +212,10 @@ class DocumentFilterModal(ModalScreen):
|
|||
search_term = event.value.lower().strip()
|
||||
filter_list = self.query_one("#filter-list", VerticalScroll)
|
||||
|
||||
for checkbox in filter_list.query(Checkbox):
|
||||
label = getattr(checkbox, "_search_text", "")
|
||||
if search_term == "" or search_term in label.lower():
|
||||
checkbox.display = True
|
||||
else:
|
||||
checkbox.display = False
|
||||
for checkbox in filter_list.query(DocumentCheckbox):
|
||||
checkbox.display = (
|
||||
search_term == "" or search_term in checkbox.search_text.lower()
|
||||
)
|
||||
|
||||
def on_button_pressed(self, event: Button.Pressed) -> None:
|
||||
"""Handle button presses."""
|
||||
|
|
|
|||
|
|
@ -631,9 +631,10 @@ class TestDocumentSelectionIdentity:
|
|||
|
||||
@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.chat.widgets.document_filter_modal import (
|
||||
DocumentCheckbox,
|
||||
DocumentFilterModal,
|
||||
)
|
||||
from haiku.rag.store.models.document import Document
|
||||
|
||||
client = AsyncMock()
|
||||
|
|
@ -655,8 +656,8 @@ class TestDocumentSelectionIdentity:
|
|||
await app.push_screen(modal)
|
||||
await pilot.pause()
|
||||
|
||||
boxes = list(modal.query(Checkbox))
|
||||
selected_ids = [getattr(box, "_doc_id", None) for box in boxes]
|
||||
boxes = list(modal.query(DocumentCheckbox))
|
||||
selected_ids = [box.doc_id 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
|
||||
|
|
@ -714,3 +715,46 @@ class TestDocumentSearchFilter:
|
|||
|
||||
assert built is not None
|
||||
assert "o''brien" in built
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_submitting_a_search_reloads_the_page(self, temp_db_path: Path):
|
||||
"""The typed term reaches the database and replaces what is shown."""
|
||||
from textual.widgets import Input
|
||||
|
||||
from haiku.rag.chat.widgets.document_filter_modal import (
|
||||
DocumentCheckbox,
|
||||
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 = 2
|
||||
|
||||
modal = DocumentFilterModal(client=client)
|
||||
app, _ = _make_app(temp_db_path, client)
|
||||
with (
|
||||
patch("haiku.rag.chat.app.HaikuRAG") as _stub_rag,
|
||||
_covering_returns(_stub_rag, client),
|
||||
):
|
||||
async with app.run_test() as pilot:
|
||||
await app.push_screen(modal)
|
||||
await pilot.pause()
|
||||
assert len(list(modal.query(DocumentCheckbox))) == 2
|
||||
|
||||
client.list_documents.return_value = [
|
||||
Document(id="id-two", content="", title="Nobel laureates"),
|
||||
]
|
||||
client.count_documents.return_value = 1
|
||||
await modal.on_input_submitted(Input.Submitted(Input(), "Nobel"))
|
||||
await pilot.pause()
|
||||
|
||||
assert client.list_documents.await_args is not None
|
||||
assert "nobel" in client.list_documents.await_args.kwargs["filter"]
|
||||
labels = [str(b.label) for b in modal.query(DocumentCheckbox)]
|
||||
assert labels == ["Nobel laureates"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue