Finish the comment pass, and escape document fields everywhere Rich renders

`_rich_print_document` escapes uri, title and metadata, the sibling of
the escaped search-result renderer. The remaining comments and
docstrings that narrated rejected alternatives, consequences or history
now state the current invariant. The Sandbox class docstring names the
held connection close() releases, and wrapped docs paragraphs join to
one line.
This commit is contained in:
Yiorgis Gozadinos 2026-08-28 15:34:36 +03:00
parent 09a7076b7e
commit afdef92b5b
No known key found for this signature in database
28 changed files with 102 additions and 124 deletions

View file

@ -149,11 +149,8 @@ RAG and analysis capabilities select databases in this order:
1. The `db_path` argument.
2. `HAIKU_RAG_DB`.
3. [`lancedb.databases`](../configuration/storage.md#multiple-databases), which
selects the full configured set.
4. [`lancedb.uri`](../configuration/storage.md#changing-the-default-database-path),
which selects one database.
3. [`lancedb.databases`](../configuration/storage.md#multiple-databases), which selects the full configured set.
4. [`lancedb.uri`](../configuration/storage.md#changing-the-default-database-path), which selects one database.
5. `config.storage.data_dir / "haiku.rag.lancedb"`.
Passing a client through `rag=` bypasses this selection. The capability uses the
databases covered by that client and does not close it.
Passing a client through `rag=` bypasses this selection. The capability uses the databases covered by that client and does not close it.

View file

@ -88,7 +88,7 @@ lancedb:
uri: "" # Empty for local, or db://, s3://, az://, gs://
api_key: ""
region: ""
databases: {} # Name-to-location map to search multiple at once, instead of uri
databases: {} # Name-to-location map to search multiple at once; excludes uri
embeddings:
model:

View file

@ -92,10 +92,7 @@ You can override this with the `--db` option.
### Evaluating over Multiple Databases
With [`lancedb.databases`](https://ggozad.github.io/haiku.rag/configuration/storage/#multiple-databases)
configured, `evaluations run <dataset> --skip-db` benchmarks the full set.
Retrieval, QA, and live conversations preserve the database name on results and
citations. A configured set of one follows the same path and retains its name.
With [`lancedb.databases`](https://ggozad.github.io/haiku.rag/configuration/storage/#multiple-databases) configured, `evaluations run <dataset> --skip-db` benchmarks the full set. Retrieval, QA, and live conversations preserve the database name on results and citations. A configured set of one follows the same path and retains its name.
Population writes one database and therefore requires `--db`:

View file

@ -454,8 +454,7 @@ async def test_gold_prefix_run_answers_with_history(tmp_path):
def test_records_the_database_each_citation_came_from():
"""A run over several databases has to record which one grounded the answer:
the distribution cannot be recovered from the report afterwards."""
"""A run over several databases records which one grounded the answer."""
from haiku.rag.capabilities._base import EvidenceState
from haiku.rag.capabilities.ledger import CapabilityEvidenceRecord
from haiku.rag.store.models.citation import Citation

View file

@ -148,8 +148,8 @@ class TestRetrievalSample:
class TestCoversASet:
"""A run over `lancedb.databases` must pass no path, since a path names one
database and wins over the configured set."""
"""A run over `lancedb.databases` passes no path: a path names one
database."""
def test_a_configured_set_is_covered(self):
from haiku.rag.config.models import AppConfig, LanceDBConfig
@ -164,8 +164,8 @@ class TestCoversASet:
assert spec.uses_configured_databases(config) is True
def test_a_named_path_overrides_the_set(self):
"""`--db` is documented as an override, so it names the one database to
evaluate even when the configuration names several."""
"""`--db` names the one database to evaluate, whatever the
configuration names."""
from pathlib import Path as _Path
from haiku.rag.config.models import AppConfig, LanceDBConfig

View file

@ -68,8 +68,7 @@ class HaikuRAGApp:
def _connection(self) -> "tuple[AppConfig, Path]":
"""How to open the one database this command works on, directly.
Derived per database: opening one of a set against the set's own
configuration would reach the local path that stands in for it.
Derived per database from its configured location.
"""
from haiku.rag.client.session import default_db_path
@ -85,9 +84,8 @@ class HaikuRAGApp:
def _is_local(self) -> bool:
"""True when the database is a local path, False for a URI.
Read from the database this command resolved to, not from the
configuration: a database named in `lancedb.databases` can sit behind a
URI while the configuration's own `uri` is empty.
Read from the resolved database: one named in `lancedb.databases` can
sit behind a URI while the configuration's own `uri` is empty.
"""
return self._one.db_path is not None
@ -853,9 +851,8 @@ class HaikuRAGApp:
def show_settings(self):
"""Display current configuration settings.
As YAML, which is the shape `haiku.rag.yaml` is written in, so what is
read here is what would be written there. `mode="json"` keeps paths and
enums out of their Python reprs.
As YAML, the shape `haiku.rag.yaml` is written in. `mode="json"` keeps
paths and enums out of their Python reprs.
"""
import yaml
@ -884,11 +881,15 @@ class HaikuRAGApp:
content = Markdown(doc.content)
parts = [f"[repr.attrib_name]id[/repr.attrib_name]: {doc.id}"]
if doc.uri:
parts.append(f"[repr.attrib_name]uri[/repr.attrib_name]: {doc.uri}")
parts.append(f"[repr.attrib_name]uri[/repr.attrib_name]: {escape(doc.uri)}")
if doc.title:
parts.append(f"[repr.attrib_name]title[/repr.attrib_name]: {doc.title}")
parts.append(
f"[repr.attrib_name]title[/repr.attrib_name]: {escape(doc.title)}"
)
if doc.metadata:
parts.append(f"[repr.attrib_name]meta[/repr.attrib_name]: {doc.metadata}")
parts.append(
f"[repr.attrib_name]meta[/repr.attrib_name]: {escape(str(doc.metadata))}"
)
self.console.print(" ".join(parts))
self.console.print(
f"[repr.attrib_name]created at[/repr.attrib_name]: {doc.created_at} [repr.attrib_name]updated at[/repr.attrib_name]: {doc.updated_at}"

View file

@ -90,9 +90,7 @@ def _nearest_known_id(chunk_id: str, known_ids: list[str]) -> str:
def resolve_scope(db_path: Path | str | None, config: AppConfig) -> DatabaseScope:
"""The databases a capability covers, resolved once at its entry point.
``HAIKU_RAG_DB`` is read here and nowhere else. ``DatabaseScope`` is
environment-agnostic on purpose, so honouring the variable inside it would
silently extend it to every other caller.
``HAIKU_RAG_DB`` is read here and nowhere else.
"""
if db_path is None and (env_db := os.environ.get("HAIKU_RAG_DB")):
db_path = Path(env_db).expanduser()
@ -559,8 +557,8 @@ class RAGCapabilityBase[StateT: EvidenceState](AbstractCapability[Any]):
synthetic: list[SearchResult] = []
documents: dict[tuple[str | None, str], Any] = {}
for chunk_id in missing:
# Every database that has it, not the first: the first
# alone cannot see that another had it too.
# All holders are asked: a collision shows only across
# every database's answer.
holders = await all_found(
lookups, lambda owner: owner.get_chunk_by_id(chunk_id)
)

View file

@ -300,9 +300,8 @@ class ChatApp(App):
refs = list(citation.picture_refs or [])
if not refs:
continue
# Evidence recorded before databases could be named carries no
# source, and across databases nothing places its pictures. The
# citation still renders, with its figure markers.
# A sourceless citation across a set has no owner to fetch
# pictures from: it renders with its figure markers alone.
owner = await self.client.reader_for(citation.source)
if owner is None:
continue

View file

@ -226,8 +226,8 @@ class DocumentFilterModal(ModalScreen):
if boxes:
await filter_list.mount_all(boxes)
else:
# Otherwise the list is an empty box, indistinguishable from one
# still loading.
# An explicit message tells an empty listing from one still
# loading.
empty = (
"Nothing selected." if self._listing_selected else "No documents match."
)

View file

@ -92,7 +92,7 @@ def resolve_scope(
The CLI decides only what it alone knows: that `--db` and `--db-name` are
the same thing said twice, and whether this command can read more than one.
Everything else an unknown name, a `lancedb.uri`, the default location
is `DatabaseScope.resolve`'s to answer, so there is one table and not two.
is `DatabaseScope.resolve`'s to answer.
"""
from haiku.rag.client.scope import DatabaseScope

View file

@ -66,8 +66,7 @@ async def all_found(
"""Every client for which `lookup` finds something, and what each found.
An id or a URI says nothing about which database holds it, so every one is
asked at once. Asking in turn would cost a round trip per database for an
identifier that is missing or held by the last of them.
asked at once.
"""
found_by_client = await gather_all(*(lookup(client) for client in clients))
return [
@ -98,11 +97,7 @@ def _spell(embedding: tuple[str | None, str | None, int | None]) -> str:
def _without_repeats(names: list[str]) -> list[str]:
"""`names` in order, without repeats.
A database named twice would be searched twice and fused as two rank lists,
which counts it double.
"""
"""`names` in order, without repeats: a name selects its database once."""
return list(dict.fromkeys(names))
@ -207,10 +202,10 @@ class HaikuRAG:
"""The client that can read `source` — itself, where it reads one
database.
None only when a client covering a set is given no name, as for evidence
recorded before databases could be named. A name this client does not
cover raises `UnknownDatabaseError`, decided by `clients_covering`: one
database answers a wrong name the same way a set does.
None only when a client covering a set is given no name. A name this
client does not cover raises `UnknownDatabaseError`, decided by
`clients_covering`: one database answers a wrong name the same way a
set does.
"""
if source is None:
return None if self.covers_multiple else self
@ -287,10 +282,9 @@ class HaikuRAG:
def reranker(self) -> "RerankerBase | None":
"""The configured reranker, built once and reused across searches.
None when reranking is disabled. Local rerankers load model weights on
construction, so one per database in a set would load the same weights
that many times over: a client covering a database for another borrows
that one's, built on the first query to reach any of them.
None when reranking is disabled. A client covering a database for
another borrows that one's, built on the first query to reach any of
them.
"""
if self._lender is not None:
return self._lender.reranker
@ -814,8 +808,7 @@ class HaikuRAG:
for owner in await self.clients_covering()
)
)
# Round-robin, so a window shows every database. Concatenating lets
# the first one fill the whole page and hide the rest.
# Round-robin: a window shows every database.
merged = [
doc for row in zip_longest(*groups) for doc in row if doc is not None
]

View file

@ -62,7 +62,7 @@ class DatabaseScope:
down. Never empty.
Nothing here reads the environment: ``HAIKU_RAG_DB`` is the capability entry
point's to honour, and reading it here would extend it to every caller.
point's to honour.
"""
databases: tuple[DatabaseRef, ...]
@ -122,8 +122,7 @@ class DatabaseScope:
def select(self, names: list[str]) -> "DatabaseScope":
"""The databases in this scope named by `names`, in the order given.
Repeats collapse: a database named twice would be searched twice and
fused as two rank lists, which counts it double.
Repeats collapse: a name selects its database once.
"""
if not names:
raise ValueError(

View file

@ -91,8 +91,8 @@ async def search_sources(
return []
selected = await client.clients_for(names)
if len(selected) == 1:
# One database is an ordinary search: fusion would replace its hybrid
# scores with ranks, and embedding up front would defeat the late embed.
# One database is an ordinary search: it keeps the database's own
# hybrid scores and embeds late.
return await selected[0].search(
query, limit, search_type, filter, include_images
)
@ -101,8 +101,7 @@ async def search_sources(
client._require_one_embedder(selected)
# One over-fetch decision, one query vector and one reranker for the whole
# set: the databases share an embedder, and deciding per database would have
# each consult its own reranker.
# set: the databases share an embedder.
fetch_limit = _fetch_limit(client, query, limit)
query_vector = await _embed_query(selected[0], query, resolved)
text = query if isinstance(query, str) else ""
@ -316,8 +315,8 @@ async def _attach_picture_data(client: "HaikuRAG", chunks: list[Chunk]) -> None:
def _dedup_picture_chunks(results: list[SearchResult]) -> list[SearchResult]:
"""Collapse duplicate picture-only chunks to one result per ``self_ref``.
Keyed by database as well, since a database copied from another holds the same
document id: collapsing across them would drop one of two real results.
Keyed by database as well: copies of a database hold the same document id,
and each copy's picture is its own result.
A single picture can produce two chunks for the same self_ref: one whose
vector is the text embedding of the picture's description, and one whose

View file

@ -116,8 +116,8 @@ class SingleDatabaseSession:
raise
failure = type(error).__name__
if failure is not None:
# Raised outside the handler to discard the location-bearing
# context, which `from None` would only stop printing.
# Raised outside the handler: the exception carries neither a cause
# nor a location-bearing context.
raise SourceUnavailableError(
f"database {self.source!r} could not be opened: {failure}"
)

View file

@ -161,8 +161,8 @@ class InfoModal(ModalScreen):
location = self.client.location
if location is None:
# Report each database independently so one failure does not hide
# the rest. Names only: a location belongs in the configuration.
# Each database reports independently, failures included. Names
# only: a location belongs in the configuration.
blocks = await asyncio.gather(
*(self._report(name) for name in sorted(self.client.source_names))
)

View file

@ -124,7 +124,7 @@ class Sandbox:
The session persists across ``execute()`` calls within the same Sandbox
instance variables carry over. Call ``close()`` to return the worker to
the pool and shut the pool down.
the pool, shut the pool down and release any held connection.
sandbox = Sandbox(db_path, config, context)
result = await sandbox.execute("x = await search('query')")
@ -285,9 +285,7 @@ class Sandbox:
groups = await gather_all(
*(owner.list_documents(filter=self._context.filter) for owner in owners)
)
# Interleaved, not concatenated: code that prints the listing is read
# through a truncated output, and concatenating shows one database's
# documents until the truncation, hiding that there are others.
# Interleaved: a truncated listing still shows every database.
docs = [doc for row in zip_longest(*groups) for doc in row if doc is not None]
return docs, self._holders(owners, groups)
@ -296,10 +294,7 @@ class Sandbox:
owners: "list[HaikuRAG]", groups: "list[list[Any]]"
) -> "dict[str, HaikuRAG]":
"""Map each document id to the database holding it, rejecting an id two
databases claim.
The mount has one `/documents/{id}/` path per id, so a copied database
would put two documents on one path and the last would answer for both.
databases claim. The mount has one `/documents/{id}/` path per id.
"""
holders: dict[str, HaikuRAG] = {}
held_by: dict[str, str | None] = {}

View file

@ -76,9 +76,8 @@ def resolve_citations(
A chunk returned by more than one search resolves to its last occurrence.
Raises ``AmbiguousCitationError`` instead when a cited id names a chunk in
more than one of the databases searched, as after copying a database: a
citation records the id alone, so resolving one would attribute the answer
to a database it may not have come from.
more than one of the databases searched: a citation records the id alone
and cannot say which.
"""
by_id: dict[str, SearchResult] = {}
ambiguous: dict[str, set[str | None]] = {}

View file

@ -18,10 +18,8 @@ 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. Ids repeat only between copies of a database, where the
same id names the same document in each.
Ids repeat only between copies of a database, where the same id names the
same document in each.
"""
if not document_ids:
return None

View file

@ -83,8 +83,8 @@ def discovered(
def test_a_retained_picture_carries_the_source_it_came_from():
"""Compaction re-fetches cited pictures later, so the capsule has to remember
which database each came from."""
"""A retained picture carries its database: compaction re-fetches cited
pictures through it."""
found = discovered(cited={"c1": [2]}, pictures={"c1": ["#/pictures/0"]})
found = replace(
found,

View file

@ -63,8 +63,7 @@ def test_run_chat_covers_a_configured_set(tmp_path, monkeypatch):
def test_chat_capabilities_read_the_named_database(tmp_path, monkeypatch):
"""`--db PATH` and `--db-name NAME` have to reach the capabilities too, or
they answer from the default database or the whole set."""
"""`--db PATH` and `--db-name NAME` reach the capabilities too."""
import haiku.rag.config as config_module
from haiku.rag.client.scope import DatabaseScope
from haiku.rag.config import set_config
@ -821,9 +820,8 @@ class TestRenderingUnattributedPictures:
async def test_a_sourceless_picture_citation_does_not_fail_the_answer(
self, temp_db_path: Path
):
"""Evidence recorded before databases could be named carries no source,
and across databases nothing places its pictures. The citation still
renders, with its figure markers."""
"""A citation without a source has no picture owner across databases.
It renders with its figure markers."""
from haiku.rag.chat.app import RAG_STATE_NAMESPACE
from haiku.rag.store.models.citation import Citation
@ -1004,8 +1002,7 @@ class TestKeepingSelectionsReachable:
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,
"""Selections accumulate across searches and get their own listing,
paged the same way."""
from textual.widgets import Button, Static

View file

@ -14,11 +14,8 @@ def vcr_cassette_dir(request):
@pytest.fixture
def query_embedding(monkeypatch):
"""Vector search with no embedder behind it, recording the queries embedded.
These tests are about which databases are asked and how often, not about
retrieval quality, and CI has no embedding endpoint.
"""
"""Vector search with no embedder behind it, recording the queries
embedded."""
from haiku.rag.embeddings import EmbedderWrapper
embedded: list[str] = []

View file

@ -37,8 +37,7 @@ class TestOpeningDatabases:
open_one = rag._session._open
async def gated(name):
# Every open has to be in flight before any of them finishes, so
# a serial loop cannot get past this and the wait times out.
# Every open is in flight before any finishes.
await barrier.wait()
await open_one(name)

View file

@ -288,7 +288,7 @@ class TestPlacingADatabase:
@pytest.mark.asyncio
async def test_a_set_cannot_place_evidence_that_names_no_database(self, tmp_path):
"""Evidence recorded before databases could be named carries no source."""
"""A sourceless citation names no database a set could place."""
config = _config(tmp_path, ["alpha", "beta"])
await _seed(config, "alpha", ["alpha one"])

View file

@ -70,9 +70,9 @@ class TestSerializingTheConnection:
class TestStandaloneAcrossDatabases:
"""Without a lent client the sandbox opens its own. The owners it hands out
are stored for later file reads, so that connection has to outlive the call
that produced them."""
"""Without a lent client the sandbox opens its own. The owners it hands
out are stored for later file reads and outlive the call that produced
them."""
@pytest.mark.asyncio
async def test_owners_stay_open_for_later_reads(self, tmp_path):
@ -181,8 +181,7 @@ class TestTheSandboxConstructors:
@pytest.mark.asyncio
async def test_covering_resolves_nothing_of_its_own(self, tmp_path, monkeypatch):
"""Handed a scope, it must not reach resolution again: resolving twice
is what let a capability's databases and its sandbox's disagree."""
"""Handed a scope, it resolves nothing of its own."""
config = _config(tmp_path, ["alpha", "beta"])
await _seed(config, "alpha", ["alpha document about cats"])
scope = DatabaseScope.resolve(config, database_name="alpha")
@ -200,9 +199,8 @@ class TestTheSandboxConstructors:
class TestTheSandboxCoversWhatTheCapabilityCovers:
@pytest.mark.asyncio
async def test_the_capability_hands_over_the_scope_it_resolved(self, tmp_path):
"""The capability resolved its databases once. Letting the sandbox
resolve them again from the same configuration reaches a different
answer wherever a path or the environment named one of a set."""
"""The sandbox covers the scope the capability resolved, as handed
over."""
from haiku.rag.capabilities.analysis import AnalysisState, create_capability
config = _config(tmp_path, ["alpha", "beta"])
@ -348,8 +346,7 @@ class TestCopiedDatabases:
class TestListingOrder:
@pytest.mark.asyncio
async def test_the_listing_interleaves_the_databases(self, tmp_path):
"""Code reads the listing through a truncated output, so concatenating
shows one database's documents until the truncation and hides the rest."""
"""A truncated listing still shows documents from every database."""
config = _config(tmp_path, ["alpha", "beta"])
await _seed(config, "alpha", [f"alpha {i}" for i in range(5)])
await _seed(config, "beta", ["beta one"])

View file

@ -95,6 +95,23 @@ async def test_a_listing_prints_the_fields_it_has(app, client):
assert "content:" in printed
async def test_a_document_renders_fields_that_look_like_markup(app):
"""Uris, titles and metadata render as text, not markup."""
doc = _doc(
"body",
uri="test://doc [/blue]",
title="The [/bold] Title",
metadata={"k": "[/red]"},
)
app._rich_print_document(doc)
printed = out(app)
assert "test://doc [/blue]" in printed
assert "The [/bold] Title" in printed
assert "[/red]" in printed
async def test_list_documents_prints_each_document(app, client):
client.list_documents.return_value = [_doc("first"), _doc("second")]
@ -443,8 +460,8 @@ def test_show_settings_renders_the_shape_a_config_file_has(tmp_path):
printed = app.console.export_text()
assert "PosixPath" not in printed
# A Python dict repr parses as YAML flow style, so parsing alone does not
# distinguish a block per field from one flattened repr per field.
# A dict repr parses as YAML flow style; the brace check tells the shapes
# apart.
assert "{'" not in printed
assert "\n alpha: /tmp/a.lancedb" in printed
body = printed.split("haiku.rag configuration", 1)[1]
@ -454,9 +471,8 @@ def test_show_settings_renders_the_shape_a_config_file_has(tmp_path):
def test_show_settings_survives_a_narrow_console_and_bracketed_values(tmp_path):
"""Rich reads `[...]` as markup and wraps to the terminal, so a prompt
carrying an instruction tag rendered away or raised, and a long path was
broken across lines into something that no longer parsed."""
"""Values in `[...]` render verbatim and a long path stays on one
parseable line, however narrow the console."""
import yaml
long_dir = tmp_path / "Application Support" / "haiku.rag" / "collections" / "one"

View file

@ -125,8 +125,8 @@ class TestOneDatabaseCommands:
assert "--db PATH after it" in str(raised.value)
def test_the_refusal_names_the_databases_and_not_their_locations(self, monkeypatch):
"""A location in an error message travels into logs and terminals; the
names exist so it does not have to."""
"""The refusal carries names only; a location in an error message
travels into logs and terminals."""
self._install(
monkeypatch,
alpha="s3://bucket/prefix/a.lancedb",
@ -385,7 +385,7 @@ class TestResolvingTheDatabasePath:
class TestConfiguredLocalUri:
"""`lancedb.uri` with a local path, the surface issue #582 reports."""
"""`lancedb.uri` with a local path."""
def _config_file(self, tmp_path, located: Path) -> Path:
config_file = tmp_path / "haiku.rag.yaml"

View file

@ -230,8 +230,7 @@ async def test_app_info_uses_connect_lancedb_for_remote(tmp_path):
mock_db.list_tables = AsyncMock(return_value=mock_list_result)
await app.info()
# The uri decides where it connects; the path argument is not read, so the
# claim here is the remote branch, not which placeholder path it carried.
# The uri decides where it connects; the path argument is not read.
mock_connect.assert_called_once()
assert mock_connect.call_args.args[0].lancedb.uri == "s3://bucket/path"

View file

@ -797,9 +797,8 @@ async def test_format_citations_rich_names_the_database_when_federating():
async def test_an_unattributable_picture_renders_its_marker(tmp_path):
"""Evidence recorded before databases could be named carries no source, so
across databases nothing says which holds the picture. One unrenderable
figure must not cost the answer."""
"""A citation without a source has no picture owner across databases. One
unrenderable figure must not cost the answer."""
from rich.console import Console
from haiku.rag.store.models.citation import Citation