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.
This commit is contained in:
Yiorgis Gozadinos
parent
2fef67d9fd
commit
105b2628de
2 changed files with 360 additions and 30 deletions
|
|
@ -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":
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue