Report where a named remote database actually is
A URI-backed database is constructed with a local `db_path` nothing connects to, and the info modal reported that, so `--db-name` on an S3 database named a local directory. `location` reads the configuration instead, and the modal labels it as one. `InfoModal`'s `db_path` argument is gone: both callers passed None, so only a test reached the branch that used it.
This commit is contained in:
parent
cfb880e8a0
commit
9210642f65
7 changed files with 75 additions and 47 deletions
|
|
@ -389,7 +389,7 @@ class ChatApp(App):
|
||||||
|
|
||||||
from haiku.rag.inspector.widgets.info_modal import InfoModal
|
from haiku.rag.inspector.widgets.info_modal import InfoModal
|
||||||
|
|
||||||
await self.push_screen(InfoModal(self.client, None))
|
await self.push_screen(InfoModal(self.client))
|
||||||
|
|
||||||
def on_citation_widget_selected(self, event: CitationWidget.Selected) -> None:
|
def on_citation_widget_selected(self, event: CitationWidget.Selected) -> None:
|
||||||
"""Handle citation selection."""
|
"""Handle citation selection."""
|
||||||
|
|
|
||||||
|
|
@ -184,6 +184,13 @@ class HaikuRAG:
|
||||||
return self._session.source
|
return self._session.source
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
@property
|
||||||
|
def location(self) -> "Path | str | None":
|
||||||
|
"""Where the database this client reads is, or None while covering a set."""
|
||||||
|
if not isinstance(self._session, SingleDatabaseSession):
|
||||||
|
return None
|
||||||
|
return self._session.location
|
||||||
|
|
||||||
async def reader_for(self, source: str | None) -> "HaikuRAG | None":
|
async def reader_for(self, source: str | None) -> "HaikuRAG | None":
|
||||||
"""The client that can read `source` — itself, where it reads one
|
"""The client that can read `source` — itself, where it reads one
|
||||||
database.
|
database.
|
||||||
|
|
|
||||||
|
|
@ -82,6 +82,14 @@ class SingleDatabaseSession:
|
||||||
self._last_vacuum_at: float | None = None
|
self._last_vacuum_at: float | None = None
|
||||||
self._vacuum_dirty = False
|
self._vacuum_dirty = False
|
||||||
|
|
||||||
|
@property
|
||||||
|
def location(self) -> Path | str:
|
||||||
|
"""Configured URI or local path for this database.
|
||||||
|
|
||||||
|
Not `db_path`, which is a placeholder where a URI holds the database.
|
||||||
|
"""
|
||||||
|
return self.config.lancedb.uri or self.db_path
|
||||||
|
|
||||||
async def open(self) -> "SingleDatabaseSession":
|
async def open(self) -> "SingleDatabaseSession":
|
||||||
"""Connect, validate, and build the repositories."""
|
"""Connect, validate, and build the repositories."""
|
||||||
failure: str | None = None
|
failure: str | None = None
|
||||||
|
|
|
||||||
|
|
@ -133,7 +133,7 @@ class InspectorApp(App):
|
||||||
if self.client:
|
if self.client:
|
||||||
from haiku.rag.inspector.widgets.info_modal import InfoModal
|
from haiku.rag.inspector.widgets.info_modal import InfoModal
|
||||||
|
|
||||||
await self._switch_modal(InfoModal(self.client, None))
|
await self._switch_modal(InfoModal(self.client))
|
||||||
|
|
||||||
async def on_search_modal_chunk_selected(
|
async def on_search_modal_chunk_selected(
|
||||||
self, message: SearchModal.ChunkSelected
|
self, message: SearchModal.ChunkSelected
|
||||||
|
|
|
||||||
|
|
@ -14,16 +14,13 @@ if TYPE_CHECKING:
|
||||||
from haiku.rag.client import HaikuRAG
|
from haiku.rag.client import HaikuRAG
|
||||||
|
|
||||||
|
|
||||||
def reported_database(client: "HaikuRAG", db_path: "Path | None") -> "Path | None":
|
def reported_location(client: "HaikuRAG") -> "Path | str | None":
|
||||||
"""The database whose statistics to report, or None where a set is covered.
|
"""Where the database to report on is, or None where a set is covered.
|
||||||
|
|
||||||
A caller passing no path leaves the choice to the client, and a client given
|
What the client ended up covering is what says which of the two it is: a
|
||||||
one named database opens it rather than covering a set. Only what the client
|
client given one named database opens it rather than covering a set.
|
||||||
ended up covering says which of the two this is.
|
|
||||||
"""
|
"""
|
||||||
if client.covers_multiple:
|
return client.location
|
||||||
return None
|
|
||||||
return db_path if db_path is not None else client.store.db_path
|
|
||||||
|
|
||||||
|
|
||||||
async def database_lines(client: "HaikuRAG") -> list[str]:
|
async def database_lines(client: "HaikuRAG") -> list[str]:
|
||||||
|
|
@ -160,10 +157,9 @@ class InfoModal(ModalScreen):
|
||||||
}
|
}
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, client: "HaikuRAG", db_path: Path | None):
|
def __init__(self, client: "HaikuRAG"):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
self.client = client
|
self.client = client
|
||||||
self.db_path = db_path
|
|
||||||
self._content_widget = Static("Loading...")
|
self._content_widget = Static("Loading...")
|
||||||
|
|
||||||
def compose(self) -> ComposeResult:
|
def compose(self) -> ComposeResult:
|
||||||
|
|
@ -176,8 +172,8 @@ class InfoModal(ModalScreen):
|
||||||
"""Load and display database info."""
|
"""Load and display database info."""
|
||||||
lines: list[str] = []
|
lines: list[str] = []
|
||||||
|
|
||||||
db_path = reported_database(self.client, self.db_path)
|
location = reported_location(self.client)
|
||||||
if db_path is None:
|
if location is None:
|
||||||
# Covering a set: report each database under its configured name, and
|
# Covering a set: report each database under its configured name, and
|
||||||
# each on its own, so one that cannot be opened costs its own block
|
# each on its own, so one that cannot be opened costs its own block
|
||||||
# rather than the whole panel. Names only, no paths — a location
|
# rather than the whole panel. Names only, no paths — a location
|
||||||
|
|
@ -188,7 +184,7 @@ class InfoModal(ModalScreen):
|
||||||
for block in blocks:
|
for block in blocks:
|
||||||
lines.extend(block)
|
lines.extend(block)
|
||||||
else:
|
else:
|
||||||
lines.append(f"[bold $accent]path[/bold $accent]: {db_path}")
|
lines.append(f"[bold $accent]location[/bold $accent]: {location}")
|
||||||
lines.extend(await database_lines(self.client))
|
lines.extend(await database_lines(self.client))
|
||||||
|
|
||||||
lines.append("[bold]Versions[/bold]")
|
lines.append("[bold]Versions[/bold]")
|
||||||
|
|
|
||||||
|
|
@ -131,6 +131,22 @@ class TestOpeningDatabases:
|
||||||
assert [client.source for client in covering] == ["alpha"]
|
assert [client.source for client in covering] == ["alpha"]
|
||||||
|
|
||||||
|
|
||||||
|
class TestReportingWhereADatabaseIs:
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_one_database_reports_its_location_and_a_set_none(self, tmp_path):
|
||||||
|
"""A set has no single location to report. What the CLI and the info
|
||||||
|
modal print comes from here."""
|
||||||
|
config = _config(tmp_path, ["alpha", "beta"])
|
||||||
|
await _seed(config, "alpha", ["alpha document about cats"])
|
||||||
|
await _seed(config, "beta", ["beta document about cats"])
|
||||||
|
|
||||||
|
async with HaikuRAG(config=config) as covering:
|
||||||
|
assert covering.location is None
|
||||||
|
|
||||||
|
async with HaikuRAG(config=config, sources=["alpha"]) as one:
|
||||||
|
assert one.location == tmp_path / "alpha.lancedb"
|
||||||
|
|
||||||
|
|
||||||
class TestClosingASet:
|
class TestClosingASet:
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_every_database_opened_is_released(self, tmp_path):
|
async def test_every_database_opened_is_released(self, tmp_path):
|
||||||
|
|
|
||||||
|
|
@ -333,43 +333,44 @@ async def test_inspector_open_failure_surfaces_real_error(tmp_path):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
class TestReportedDatabase:
|
class TestReportedLocation:
|
||||||
"""`db_path=None` means the client chose, and a client handed one named
|
"""A URI-backed database is constructed with a placeholder local path, so
|
||||||
database opens it rather than covering a set."""
|
what the modal prints has to come from the configuration."""
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _client(federated, store_path=None):
|
def _session(location: str):
|
||||||
client = MagicMock()
|
from haiku.rag.client.scope import DatabaseScope
|
||||||
client.covers_multiple = len(federated) > 1
|
from haiku.rag.client.session import SingleDatabaseSession, default_db_path
|
||||||
client.source_names = tuple(federated)
|
from haiku.rag.config.models import AppConfig, LanceDBConfig
|
||||||
client.store.db_path = store_path
|
|
||||||
return client
|
|
||||||
|
|
||||||
def test_a_covered_set_reports_no_single_database(self):
|
config = AppConfig(lancedb=LanceDBConfig(databases={"alpha": location}))
|
||||||
from haiku.rag.inspector.widgets.info_modal import reported_database
|
[ref] = DatabaseScope.resolve(config, database_name="alpha").databases
|
||||||
|
one, db_path = ref.connection(config)
|
||||||
client = self._client({"a": "/a.lancedb", "b": "/b.lancedb"})
|
return SingleDatabaseSession(
|
||||||
|
db_path if db_path is not None else default_db_path(one),
|
||||||
assert reported_database(client, None) is None
|
one,
|
||||||
|
source="alpha",
|
||||||
def test_one_named_database_reports_the_path_it_opened(self):
|
|
||||||
"""The path is None because the client resolved the name, not because
|
|
||||||
there is a set to cover."""
|
|
||||||
from haiku.rag.inspector.widgets.info_modal import reported_database
|
|
||||||
|
|
||||||
client = self._client({}, store_path=Path("/data/alpha.lancedb"))
|
|
||||||
|
|
||||||
assert reported_database(client, None) == Path("/data/alpha.lancedb")
|
|
||||||
|
|
||||||
def test_an_explicit_path_is_reported_as_given(self):
|
|
||||||
from haiku.rag.inspector.widgets.info_modal import reported_database
|
|
||||||
|
|
||||||
client = self._client({}, store_path=Path("/data/other.lancedb"))
|
|
||||||
|
|
||||||
assert reported_database(client, Path("/data/given.lancedb")) == Path(
|
|
||||||
"/data/given.lancedb"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_a_covered_set_reports_no_location(self):
|
||||||
|
from haiku.rag.inspector.widgets.info_modal import reported_location
|
||||||
|
|
||||||
|
client = MagicMock()
|
||||||
|
client.location = None
|
||||||
|
|
||||||
|
assert reported_location(client) is None
|
||||||
|
|
||||||
|
def test_a_named_remote_database_reports_its_uri(self):
|
||||||
|
session = self._session("s3://bucket/alpha.lancedb")
|
||||||
|
|
||||||
|
assert isinstance(session.db_path, Path)
|
||||||
|
assert session.location == "s3://bucket/alpha.lancedb"
|
||||||
|
|
||||||
|
def test_a_named_local_database_reports_its_path(self):
|
||||||
|
session = self._session("/data/alpha.lancedb")
|
||||||
|
|
||||||
|
assert session.location == Path("/data/alpha.lancedb")
|
||||||
|
|
||||||
|
|
||||||
class TestReportingReusesTheConnection:
|
class TestReportingReusesTheConnection:
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue