Trim the comments to what a reader needs

One sentence for the contract, one or two more only where an invariant is not
obvious. The reasons that stay are about correctness and ownership: which
database a result belongs to, who closes what, why assembly order is the
tiebreak. The ones that go narrated how the code got here.
This commit is contained in:
Yiorgis Gozadinos 2026-08-26 13:29:00 +03:00
parent 503db3271c
commit 1b3334b5af
No known key found for this signature in database
7 changed files with 31 additions and 68 deletions

View file

@ -39,12 +39,7 @@ class HaikuRAGApp:
config: AppConfig | None = None, config: AppConfig | None = None,
read_only: bool = False, read_only: bool = False,
): ):
"""The databases this command works on, resolved by whoever built it. """The databases this command works on, resolved by whoever built it."""
One selector, not three: a command that took a path and a name and a
scope would have to decide between them, which is the deciding this
layer exists to have already done.
"""
self.scope = scope self.scope = scope
self.config = config if config is not None else get_config() self.config = config if config is not None else get_config()
self.read_only = read_only self.read_only = read_only

View file

@ -8,9 +8,8 @@ from textual.widgets import Button, Checkbox, Input, Static
from haiku.rag.client import HaikuRAG from haiku.rag.client import HaikuRAG
from haiku.rag.utils import escape_sql_string from haiku.rag.utils import escape_sql_string
# One page of documents. A corpus is not a list to scroll: mounting a checkbox # Documents listed at once. Mounting a checkbox per document wedges the modal on
# per document wedges the modal at tens of thousands, so the list is a page and # a large corpus, so the rest is reached through the search box.
# the search box asks the database for the rest.
DOCUMENT_PAGE = 200 DOCUMENT_PAGE = 200

View file

@ -360,12 +360,7 @@ class HaikuRAG:
] ]
def _facade_for(self, name: str, session: SingleDatabaseSession) -> "HaikuRAG": def _facade_for(self, name: str, session: SingleDatabaseSession) -> "HaikuRAG":
"""The client for one covered database, made once and kept. """The cached client borrowing this session, made once and kept."""
The facade is public and the session is not, so the wrapper lives here
while the federated session keeps the database it wraps. A borrowed facade
never closes what it did not open.
"""
facade = self._clients.get(name) facade = self._clients.get(name)
if facade is None: if facade is None:
facade = HaikuRAG._from_session(session) facade = HaikuRAG._from_session(session)

View file

@ -8,20 +8,12 @@ from haiku.rag.utils import locate_database
@dataclass(frozen=True) @dataclass(frozen=True)
class DatabaseRef: class DatabaseRef:
"""One database, and the name it answers to. """A resolved database location, and the configured name it answers to.
``name`` is the key from ``lancedb.databases``, and the only identity that Exactly one of ``uri`` and ``db_path`` is set. ``name`` is the key from
leaves the configuration: it travels in results, citations and the errors an ``lancedb.databases``, and the only identity that leaves the configuration:
operator or a model sees, where a location must not. The invariant below is it travels in results, citations and errors, where a location must not.
the exception, and deliberately so a malformed ref is a programming error None where nothing names the database.
raised in the caller's own process, where naming what it was given is what
makes it fixable. None where nothing names the database a path given
directly, or the legacy single ``uri``.
Location is resolved once, on construction, into exactly one of ``uri`` and
``db_path``. Keeping the configured string and re-reading it later would let a
path the caller gave be reinterpreted as a URI because it happens to carry a
scheme.
""" """
name: str | None name: str | None
@ -42,18 +34,16 @@ class DatabaseRef:
@classmethod @classmethod
def configured(cls, name: str | None, location: str) -> "DatabaseRef": def configured(cls, name: str | None, location: str) -> "DatabaseRef":
"""A database the configuration placed, by ``lancedb.uri`` or by an entry """A database the configuration placed, by ``lancedb.uri`` or an entry in
in ``lancedb.databases``. A value carrying a scheme is a URI and anything ``lancedb.databases``. A location carrying a scheme is a URI, anything
else is a local path, so the two settings place a database alike.""" else a local path."""
uri, db_path = locate_database(location) uri, db_path = locate_database(location)
return cls(name=name, uri=uri, db_path=db_path) return cls(name=name, uri=uri, db_path=db_path)
def connection(self, config: AppConfig) -> tuple[AppConfig, Path | None]: def connection(self, config: AppConfig) -> tuple[AppConfig, Path | None]:
"""The configuration and path to open this one database with. """The configuration and path to open this one database with.
A copy. The scope is resolved once from the caller's configuration, and A copy: the caller's configuration still names whatever set it named.
rewriting that configuration in place is what left downstream code unable
to tell that a set had been named.
""" """
one = config.model_copy(deep=True) one = config.model_copy(deep=True)
one.lancedb.databases = {} one.lancedb.databases = {}
@ -66,12 +56,10 @@ class DatabaseScope:
"""The databases an operation covers. """The databases an operation covers.
Resolved once, from configuration plus at most one selector, and passed down Resolved once, from configuration plus at most one selector, and passed down
rather than re-derived. Never empty: every resolution reaches a database, and rather than re-derived. Never empty.
the sessions built from a scope have no meaning without one.
Nothing here reads the environment. ``HAIKU_RAG_DB`` is honoured by the Nothing here reads the environment: ``HAIKU_RAG_DB`` is the capability entry
capability entry point alone, which passes it as ``database_path``, so point's to honour, and reading it here would extend it to every caller.
resolving a scope cannot quietly change what any other caller opens.
""" """
databases: tuple[DatabaseRef, ...] databases: tuple[DatabaseRef, ...]

View file

@ -58,9 +58,8 @@ class SingleDatabaseSession:
it has one. ``source`` is the configured name this database answers to, or it has one. ``source`` is the configured name this database answers to, or
None where nothing names it. None where nothing names it.
``db_path``, ``config``, ``read_only`` and ``source`` are readable because a ``db_path``, ``config``, ``read_only`` and ``source`` are readable so that a
client facade is built over a session it does not own, and needs to report client borrowing this session can report them as its own.
the same things it would have reported had it opened the database itself.
""" """
def __init__( def __init__(
@ -131,12 +130,9 @@ class SingleDatabaseSession:
async def drain_vacuum(self) -> None: async def drain_vacuum(self) -> None:
"""Drain background vacuum work and run a final collapse before teardown. """Drain background vacuum work and run a final collapse before teardown.
Writes schedule a throttled background vacuum; many are debounced or skip The final pass runs whenever writes happened, not when tasks remain: a
because another vacuum holds the lock. The final pass collapses the debounced run may have scheduled none and still left versions behind. It
versions those left behind. It runs whenever writes happened never runs without writes, so opening and closing a store writes nothing.
(``_vacuum_dirty``) not gated on in-flight tasks remaining, since a
debounced run may have scheduled none but never when nothing was
written (so opening + closing a store still never writes).
""" """
if self._vacuum_tasks: if self._vacuum_tasks:
await asyncio.gather(*self._vacuum_tasks, return_exceptions=True) await asyncio.gather(*self._vacuum_tasks, return_exceptions=True)
@ -251,15 +247,12 @@ class SingleDatabaseSession:
class FederatedSession: class FederatedSession:
"""Several databases, read as one. """Several databases, read as one. Reads only.
Composes single-database sessions and owns their teardown. They open on first Composes single-database sessions and owns their teardown. They open on first
use rather than at entry: which databases a query covers is a per-query use rather than at entry: which databases a query covers is a per-query
choice, so a database nobody asked for must neither be opened for nothing nor choice, so a database nobody asked for must neither be opened for nothing nor
be able to fail a query. be able to fail a query.
Reads only. Writing names a database, and naming one is what
``SingleDatabaseSession`` is.
""" """
def __init__( def __init__(

View file

@ -272,14 +272,11 @@ class Sandbox:
def _holders( def _holders(
owners: "list[HaikuRAG]", groups: "list[list[Any]]" owners: "list[HaikuRAG]", groups: "list[list[Any]]"
) -> "dict[str, HaikuRAG]": ) -> "dict[str, HaikuRAG]":
"""Map each document id to the database holding it. """Map each document id to the database holding it, rejecting an id two
databases claim.
Document ids are UUID4, so the flat `/documents/{id}/` namespace is The mount has one `/documents/{id}/` path per id, so a copied database
unambiguous for databases that were filled independently but not for one would put two documents on one path and the last would answer for both.
copied from another, where the same id is in both. Duplicate results are
merely redundant in a search; here they would be two documents claiming one
path, and whichever arrived last would answer for both. Rejected rather
than resolved, since either answer would be wrong half the time.
""" """
holders: dict[str, HaikuRAG] = {} holders: dict[str, HaikuRAG] = {}
held_by: dict[str, str | None] = {} held_by: dict[str, str | None] = {}

View file

@ -117,16 +117,12 @@ SearchType = Literal["vector", "fts", "hybrid"]
def qualified_id(source: str | None, id: str | None) -> tuple[str | None, str | None]: def qualified_id(source: str | None, id: str | None) -> tuple[str | None, str | None]:
"""What tells one chunk from another. """What tells one chunk from another: a chunk id is unique within a database
and says nothing across them.
A chunk id is unique within a database and says nothing across them: a For in-memory structures only. Everything serialized records the id alone
database copied from another holds the same ids, so the database is part of and rejects ambiguity instead. `id` is optional, and results built by hand
the identity. `id` is optional because a result built by hand carries none, carry none and cannot be told apart.
and those cannot be told apart.
Only for structures held in memory. Everything serialized citations,
evidence refs, compaction records the id alone, so ambiguity there is
rejected rather than qualified.
""" """
return (source, id) return (source, id)