State what comments guarantee, not what would go wrong without them
Comments that narrated a failure mode across three or four lines say the invariant they protect instead: the repository's late embedding, one database keeping its hybrid scores, one over-fetch decision for a selection, and a cite fallback that covers exactly what the question covers.
This commit is contained in:
parent
81e05cba7a
commit
688d7e708e
6 changed files with 20 additions and 26 deletions
|
|
@ -554,16 +554,14 @@ class RAGCapabilityBase[StateT: EvidenceState](AbstractCapability[Any]):
|
||||||
if missing:
|
if missing:
|
||||||
async with self.rag_lock:
|
async with self.rag_lock:
|
||||||
rag = await self._ensure_rag()
|
rag = await self._ensure_rag()
|
||||||
# A chunk id says nothing about which database holds it, so the
|
# A chunk id names no database, so the fallback covers exactly
|
||||||
# fallback looks through everything the question covers — and
|
# what the question covers, and nothing it does not.
|
||||||
# nothing it does not.
|
|
||||||
lookups = await rag.clients_covering(self.state.sources)
|
lookups = await rag.clients_covering(self.state.sources)
|
||||||
synthetic: list[SearchResult] = []
|
synthetic: list[SearchResult] = []
|
||||||
documents: dict[tuple[str | None, str], Any] = {}
|
documents: dict[tuple[str | None, str], Any] = {}
|
||||||
for chunk_id in missing:
|
for chunk_id in missing:
|
||||||
# Every database that has it, not the first: taking the first
|
# Every database that has it, not the first: the first
|
||||||
# would attribute the answer to one of them without ever
|
# alone cannot see that another had it too.
|
||||||
# seeing that another had it too.
|
|
||||||
holders = await all_found(
|
holders = await all_found(
|
||||||
lookups, lambda owner: owner.get_chunk_by_id(chunk_id)
|
lookups, lambda owner: owner.get_chunk_by_id(chunk_id)
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -42,9 +42,9 @@ def run_chat(
|
||||||
config.qa.model = model_config
|
config.qa.model = model_config
|
||||||
config.analysis.model = model_config
|
config.analysis.model = model_config
|
||||||
|
|
||||||
# The capabilities read the databases the scope covers, not whatever the
|
# The capabilities read the databases the scope covers, not what the
|
||||||
# configuration happens to name: `--db PATH` would otherwise leave them on
|
# configuration names: a `--db PATH` selection is outside the
|
||||||
# the default database, and `--db-name NAME` on the whole set.
|
# configuration, and a `--db-name NAME` selection is narrower than it.
|
||||||
if scope.covers_multiple:
|
if scope.covers_multiple:
|
||||||
capability_config, capability_db_path = config, None
|
capability_config, capability_db_path = config, None
|
||||||
else:
|
else:
|
||||||
|
|
|
||||||
|
|
@ -148,9 +148,8 @@ class ChatApp(App):
|
||||||
# a client whose __aenter__ failed.
|
# a client whose __aenter__ failed.
|
||||||
await client.__aenter__()
|
await client.__aenter__()
|
||||||
self.client = client
|
self.client = client
|
||||||
# The capabilities read through this one, rather than each opening its
|
# Lent to the capabilities: already the databases they were built for,
|
||||||
# own: it is already the databases they were built for, and lending it
|
# and one connection per database however many capabilities read it.
|
||||||
# means one connection per database instead of one per capability.
|
|
||||||
for capability in self._capabilities:
|
for capability in self._capabilities:
|
||||||
capability.borrowed_rag = client
|
capability.borrowed_rag = client
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -138,8 +138,8 @@ class DocumentFilterModal(ModalScreen):
|
||||||
self._search = ""
|
self._search = ""
|
||||||
self._page = 0
|
self._page = 0
|
||||||
# Selections outside the page stay applied, and a checkbox is the only
|
# Selections outside the page stay applied, and a checkbox is the only
|
||||||
# way to remove one, so they are reachable through their own listing
|
# way to remove one, so their own listing keeps them reachable while
|
||||||
# rather than appended to this one, which the page bound has to hold.
|
# this one holds the page bound.
|
||||||
self._listing_selected = False
|
self._listing_selected = False
|
||||||
|
|
||||||
def compose(self) -> ComposeResult:
|
def compose(self) -> ComposeResult:
|
||||||
|
|
|
||||||
|
|
@ -46,8 +46,7 @@ async def search(
|
||||||
limit = client._config.search.limit
|
limit = client._config.search.limit
|
||||||
|
|
||||||
resolved = _resolved_search_type(query, search_type)
|
resolved = _resolved_search_type(query, search_type)
|
||||||
# One database embeds inside the repository, which returns early for a filter
|
# The repository embeds late, so a filter matching nothing never embeds.
|
||||||
# that matches nothing, so a text query that finds no documents never embeds.
|
|
||||||
query_vector = (
|
query_vector = (
|
||||||
None if isinstance(query, str) else await _embed_query(client, query, resolved)
|
None if isinstance(query, str) else await _embed_query(client, query, resolved)
|
||||||
)
|
)
|
||||||
|
|
@ -92,9 +91,8 @@ async def search_sources(
|
||||||
return []
|
return []
|
||||||
selected = await client.clients_for(names)
|
selected = await client.clients_for(names)
|
||||||
if len(selected) == 1:
|
if len(selected) == 1:
|
||||||
# One database is an ordinary search, whatever the client covers: fusion
|
# One database is an ordinary search: fusion would replace its hybrid
|
||||||
# would replace its hybrid scores with ranks, and embedding up front
|
# scores with ranks, and embedding up front would defeat the late embed.
|
||||||
# would embed for a filter the repository can see matches nothing.
|
|
||||||
return await selected[0].search(
|
return await selected[0].search(
|
||||||
query, limit, search_type, filter, include_images
|
query, limit, search_type, filter, include_images
|
||||||
)
|
)
|
||||||
|
|
@ -102,10 +100,9 @@ async def search_sources(
|
||||||
if resolved != "fts":
|
if resolved != "fts":
|
||||||
client._require_one_embedder(selected)
|
client._require_one_embedder(selected)
|
||||||
|
|
||||||
# One over-fetch decision, one query vector, and one reranker, for the whole
|
# One over-fetch decision, one query vector and one reranker for the whole
|
||||||
# set. The databases in a selection share an embedder, so the vector is the
|
# set: the databases share an embedder, and deciding per database would have
|
||||||
# same wherever it is computed, and deciding the over-fetch per database would
|
# each consult its own reranker.
|
||||||
# have each consult its own reranker.
|
|
||||||
fetch_limit = _fetch_limit(client, query, limit)
|
fetch_limit = _fetch_limit(client, query, limit)
|
||||||
query_vector = await _embed_query(selected[0], query, resolved)
|
query_vector = await _embed_query(selected[0], query, resolved)
|
||||||
text = query if isinstance(query, str) else ""
|
text = query if isinstance(query, str) else ""
|
||||||
|
|
|
||||||
|
|
@ -509,9 +509,9 @@ def locate_database(location: str) -> tuple[str, Path | None]:
|
||||||
"""Split a configured location into (uri, db_path).
|
"""Split a configured location into (uri, db_path).
|
||||||
|
|
||||||
A value with a scheme is a `lancedb.uri`; anything else is a local path.
|
A value with a scheme is a `lancedb.uri`; anything else is a local path.
|
||||||
Routing a local path through `uri` would have `ConnectionMode` classify it as
|
`ConnectionMode` classifies a `uri` as object storage and opens it without
|
||||||
object storage, which opens it without the existence check a local database
|
the existence check a local database gets, so a local path never travels
|
||||||
gets.
|
as one.
|
||||||
"""
|
"""
|
||||||
if "://" in location:
|
if "://" in location:
|
||||||
return location, None
|
return location, None
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue