diff --git a/CHANGELOG.md b/CHANGELOG.md index a77c7aaa..b8d1de05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Removed + +- `lancedb.uri`. Write `lancedb.databases: {NAME: }`; a config carrying + `uri` fails to load with that message. + ### Changed - `qa.max_searches` counts search units: searches a model emits in one @@ -10,6 +15,17 @@ - Searches in one model response deduplicate their results: evidence a sibling search already showed collapses to a reference line, and a picture attaches once per response. +- Every database has a name: the key in `lancedb.databases`, or the path's stem + for `--db PATH`, `db_path=` and the default database, which is the entry + `haiku.rag` under `storage.data_dir` and selectable by that name. + `SearchResult.source`, `Document.source` and `Citation.source` always carry it. +- `db_path=` beside a configured `lancedb.databases` raises + `AmbiguousDatabaseError` (`HaikuRAG`, `create_capability`, `create_mcp_server`, + `Sandbox`). `haiku-rag --db PATH` and `haiku-ingester --db PATH` open that path + whatever is configured. +- `DatabaseRef(name, location, given)` replaces `DatabaseRef(name, uri, db_path)`; + `DatabaseScope.at(path)` added; `locate_database` returns `Path | str`. + `IngesterApp(config, scope)` takes a resolved scope in place of `db_path`. - `Store(location, config)`, `connect_lancedb(location, config)`, `gather_database_info(location, config)` and `run_doctor(config, location, ...)` take the database location, a path or a URI. `ConnectionMode.of(location)` diff --git a/docs/capabilities/index.md b/docs/capabilities/index.md index 1c5f985a..e56cd40c 100644 --- a/docs/capabilities/index.md +++ b/docs/capabilities/index.md @@ -145,12 +145,6 @@ Applications serving AG-UI should adapt the agent with Pydantic AI's `AGUIAdapte ## Database Selection -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. -5. `config.storage.data_dir / "haiku.rag.lancedb"`. +RAG and analysis capabilities cover the databases the configuration places: [`lancedb.databases`](../configuration/storage.md#multiple-databases), or with nothing configured the default database `haiku.rag` under `storage.data_dir`. The `db_path` argument, or `HAIKU_RAG_DB`, places one database where the configuration places none; beside `lancedb.databases` either raises `AmbiguousDatabaseError`. Passing a client through `rag=` bypasses this selection. The capability uses the databases covered by that client and does not close it. diff --git a/docs/cli.md b/docs/cli.md index d1a33d7f..f9baf5d6 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -12,7 +12,7 @@ The `haiku-rag` CLI provides complete document management functionality. Per-command options: - - `--db` - Specify custom database path + - `--db` - Open the database at this path, named by its stem, whatever the configuration places - `-h` - Show help for specific command Example: diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 165fa13e..ada2c533 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -85,10 +85,9 @@ ingester: delete_orphans: true lancedb: - uri: "" # Empty for local, or db://, s3://, az://, gs:// - api_key: "" + databases: {} # Name-to-location map; empty places haiku.rag under data_dir + api_key: "" # LanceDB Cloud (db://) credentials region: "" - databases: {} # Name-to-location map to search multiple at once; excludes uri embeddings: model: diff --git a/docs/configuration/storage.md b/docs/configuration/storage.md index 1f486f0f..34c41b87 100644 --- a/docs/configuration/storage.md +++ b/docs/configuration/storage.md @@ -68,18 +68,19 @@ Vacuum also folds new rows into the full-text index. Search stays correct withou This is an upstream limitation rather than a `haiku.rag` setting. Compaction bounds itself by row count instead of bytes, and LanceDB's async API exposes no batch size or fragment target to override it. Tracked at [lancedb/lancedb#2325](https://github.com/lancedb/lancedb/issues/2325). The requirement above will drop once compaction batches by bytes. -### Changing the Default Database Path +### Placing the Database -`storage.data_dir` holds the default database, always called `haiku.rag.lancedb`. To put the database somewhere else for every command, give `lancedb.uri` a local path: +`lancedb.databases` maps a name to a location, a local path or a URI, and is the one way to place databases. With nothing configured, the database is the entry `haiku.rag` at `/haiku.rag.lancedb`. To put one database somewhere else, name it: ```yaml lancedb: - uri: /data/notes.lancedb + databases: + notes: /data/notes.lancedb ``` -An explicit `--db PATH` overrides `lancedb.uri` for that invocation. +The name is what `source` carries in search results, citations and documents, and what `--db-name` and `sources` select. The default database answers to `haiku.rag`. -This places one database without naming it. Its `source` is `None` in search results, citations and documents, since only [`lancedb.databases`](#multiple-databases) assigns the names that carry provenance. A path here changes where the database lives, not what it is called. +An explicit `--db PATH` on the command line opens that database instead, named by the path's stem, whatever is configured. From Python, `db_path` places the database only where the configuration places none: beside `lancedb.databases` it raises `AmbiguousDatabaseError`. A value with no scheme is a local path wherever it is configured, so `haiku-rag init` creates it and every command that opens an existing database requires it to exist. A mistyped path fails rather than becoming a new empty database. @@ -109,28 +110,31 @@ async with HaikuRAG(create=True) as client: The [default location](index.md#configuration-file-locations) is platform-specific (e.g., `~/Library/Application Support/haiku.rag/` on macOS). -Opening a nonexistent unnamed local database raises `FileNotFoundError`, naming its path. This prevents accidental database creation from typos or misconfigured paths. A database named in `lancedb.databases` raises `SourceUnavailableError` instead, naming the database and not its location. +Opening a nonexistent local database given as a path raises `FileNotFoundError`, naming the path. This prevents accidental database creation from typos or misconfigured paths. A database placed by `lancedb.databases` raises `SourceUnavailableError` instead, naming the database and not its location. ## Remote Storage -For remote storage, use the `lancedb` settings with various backends: +For remote storage, give the database a URI as its location. Credentials and storage options are connection settings, shared by every database in the configuration: ```yaml # LanceDB Cloud lancedb: - uri: db://your-database-name + databases: + papers: db://your-database-name api_key: your-api-key region: us-west-2 # optional # Amazon S3 lancedb: - uri: s3://my-bucket/my-table + databases: + papers: s3://my-bucket/my-table storage_options: region: us-east-1 # Amazon S3 with explicit credentials lancedb: - uri: s3://my-bucket/my-table + databases: + papers: s3://my-bucket/my-table storage_options: aws_access_key_id: YOUR_ACCESS_KEY aws_secret_access_key: YOUR_SECRET_KEY @@ -138,7 +142,8 @@ lancedb: # S3-compatible (SeaweedFS, Tigris, etc.) lancedb: - uri: s3://my-bucket/my-table + databases: + papers: s3://my-bucket/my-table storage_options: endpoint: http://localhost:8333 aws_access_key_id: YOUR_ACCESS_KEY @@ -148,21 +153,24 @@ lancedb: # Azure Blob Storage lancedb: - uri: az://my-container/my-table + databases: + papers: az://my-container/my-table # Google Cloud Storage lancedb: - uri: gs://my-bucket/my-table + databases: + papers: gs://my-bucket/my-table # HDFS lancedb: - uri: hdfs://namenode:port/path/to/table + databases: + papers: hdfs://namenode:port/path/to/table ``` - **LanceDB Cloud** (`db://`): Requires `api_key` and `region`. Table optimization and indexing are managed server-side. - **Object storage** (`s3://`, `gs://`, `az://`, `hdfs://`): Uses `storage_options` for credentials and endpoint configuration. Authentication can also be provided via environment variables (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, etc.) or cloud provider SDK defaults (AWS CLI, Azure CLI, gcloud). - **S3-compatible stores** (MinIO, Tigris, etc.): Set `endpoint` in `storage_options`. When using `http://` endpoints, also set `allow_http: "true"`. -- **Local path** (no scheme): `uri` also takes a local path, which is how the default database is pointed elsewhere. See [Changing the Default Database Path](#changing-the-default-database-path). +- **Local path** (no scheme): a location without a scheme is a local path. See [Placing the Database](#placing-the-database). The `storage_options` keys are case-insensitive and passed directly to the underlying object store library. Available keys depend on the backend. See the [LanceDB storage docs](https://lancedb.com/docs/storage/) for details. @@ -192,7 +200,7 @@ The recommended layout for production is "different buckets, same account, separ Each process picks up its own credentials from the AWS default chain (env vars, IAM instance role, AWS profile), so no credentials are hard-coded in the configuration files. -`haiku-ingester` writes the database the configuration places, so a `lancedb.uri` needs no further option. `--db PATH` overrides it. When `lancedb.databases` contains more than one database the ingester has no way to name which it writes, and refuses to start with `AmbiguousDatabaseError`: give each database its own ingester process, each with a configuration naming a single database, or select one with `--db PATH`. A one-entry mapping is unambiguous and is accepted. +`haiku-ingester` writes the database the configuration places, so a one-entry `lancedb.databases` needs no further option. `--db PATH` overrides it. When `lancedb.databases` contains more than one database the ingester has no way to name which it writes, and refuses to start with `AmbiguousDatabaseError`: give each database its own ingester process, each with a configuration naming a single database, or select one with `--db PATH`. ## Multiple Databases @@ -206,7 +214,7 @@ lancedb: notes: /data/notes.lancedb ``` -A location can be a URI or local path. `databases` and `uri` are mutually exclusive. +A location can be a URI or local path. Results, documents, and citations use the configured name as `source`. An unavailable configured database raises `SourceUnavailableError`, which names the database and not its location, so a location never travels in an error a consumer might render or log. A migration, configuration or read-only failure keeps its own type, with the database named in the message. Commands that report on a database, such as `info`, still show where it is. @@ -283,7 +291,7 @@ haiku-rag --db-name papers list # one of them haiku-rag --db-name papers migrate ``` -`--db-name` selects an entry from `lancedb.databases`, including remote entries. `--db` selects a local path and overrides the configured location. A single-database command requires one of these options when multiple databases are configured. A configured set of one is selected automatically. +`--db-name` selects an entry from `lancedb.databases`, including remote entries, and `haiku.rag` when nothing is configured. `--db` opens a local path, named by its stem, whatever is configured. A single-database command requires one of these options when multiple databases are configured. A configured set of one is selected automatically. Each database is created, migrated and vacuumed on its own: diff --git a/docs/overview.md b/docs/overview.md index b8fb305e..95927e16 100644 --- a/docs/overview.md +++ b/docs/overview.md @@ -35,7 +35,8 @@ snapshot is only meaningful while this process is the only writer. ## Storage LanceDB is embedded, so there is no server. The same code runs against a local -directory, S3, GCS, Azure or LanceDB Cloud by changing `lancedb.uri`. +directory, S3, GCS, Azure or LanceDB Cloud by changing a database's location in +`lancedb.databases`. Tables are versioned. Vacuum collapses old versions on a retention window, and [tags](cli.md) name a state across all tables so a database can be restored to diff --git a/docs/python.md b/docs/python.md index 3a70f006..c075ea47 100644 --- a/docs/python.md +++ b/docs/python.md @@ -27,7 +27,7 @@ async with HaikuRAG("path/to/database.lancedb", read_only=True) as client: `async with` is the lifecycle. A caller that owns the client some other way releases it with `await client.aclose()`, which does the same work for every client shape. `client.close()` closes the connection to one database and nothing else, since draining the background vacuum and releasing the embedder and reranker are awaitable; it refuses a client covering several. !!! note - Databases must be explicitly created with `create=True` or via `haiku-rag init` before use. Opening a nonexistent unnamed local database raises `FileNotFoundError`, naming its path; one named in `lancedb.databases` raises `SourceUnavailableError`, which names the database rather than its location. + Databases must be explicitly created with `create=True` or via `haiku-rag init` before use. Opening a nonexistent local database given as a path raises `FileNotFoundError`, naming the path; one placed by `lancedb.databases` raises `SourceUnavailableError`, which names the database rather than its location. A path beside a configured `lancedb.databases` raises `AmbiguousDatabaseError`. !!! note Read-only mode is useful for safely accessing databases without risk of modification. It blocks all write operations and downgrades an embedding provider/name mismatch to a warning instead of raising `ConfigMismatchError`. @@ -268,8 +268,8 @@ On the constructor `sources=[]` means something else. Passing `sources` alongsid ```python client.covers_multiple # whether the client covers more than one database -client.source_names # configured names, in order -client.source # one configured name, or None for a set or unnamed database +client.source_names # database names, in order; known before the client opens +client.source # the one database's name, or None for a set owner = await client.reader_for("papers") # the client reading that database papers, wiki = await client.clients_for(["papers", "wiki"]) @@ -277,6 +277,17 @@ papers, wiki = await client.clients_for(["papers", "wiki"]) `reader_for` and `clients_for` open databases lazily and return borrowed clients. They remain valid while the covering client is open and inherit its read-only mode. The covering client owns and closes their database sessions. +To learn what a configuration covers without opening anything, resolve it: + +```python +from haiku.rag.client import DatabaseScope + +for ref in DatabaseScope.resolve(config).databases: + print(ref.name, ref.location) # "haiku.rag", Path(".../haiku.rag.lancedb") when nothing is configured +``` + +`DatabaseScope.resolve` is pure: it reads the configuration and classifies each location as a local path or a URI. + ### Filtering Search Results Filter search results to only include chunks from documents matching specific criteria: diff --git a/evaluations/evaluations/benchmark.py b/evaluations/evaluations/benchmark.py index 182d5977..063a8a85 100644 --- a/evaluations/evaluations/benchmark.py +++ b/evaluations/evaluations/benchmark.py @@ -49,6 +49,13 @@ async def evaluate_dataset( if document_filter is not None: console.print(f"Document filter: {document_filter}", style="dim") + if db_path is not None and config.lancedb.databases: + raise ValueError( + "--db PATH places the database where the configuration places none, " + f"and this configuration names {', '.join(config.lancedb.databases)} " + "in lancedb.databases. Drop --db to evaluate the configured set." + ) + if not skip_db: if spec.uses_configured_databases(config, db_path): raise ValueError( @@ -157,7 +164,11 @@ def run( config: Path | None = typer.Option( None, "--config", help="Path to haiku.rag YAML config file." ), - db: Path | None = typer.Option(None, "--db", help="Override the database path."), + db: Path | None = typer.Option( + None, + "--db", + help="Database path, where the configuration places no database.", + ), skip_db: bool = typer.Option( False, "--skip-db", help="Skip updating the evaluation db." ), diff --git a/evaluations/evaluations/config.py b/evaluations/evaluations/config.py index 8a75c413..90431dd3 100644 --- a/evaluations/evaluations/config.py +++ b/evaluations/evaluations/config.py @@ -89,9 +89,9 @@ class DatasetSpec: ) -> bool: """Whether `lancedb.databases` places the databases to evaluate over. - A path names one database and wins over the configuration, both when it - comes from `--db` and when the client resolves it. True for a mapping of - one, which is a configured database like any other and keeps its name. + `--db PATH` places the database where the configuration places none; + `evaluate_dataset` refuses the two together. True for a mapping of one, + which is a configured database like any other and keeps its name. """ return bool(config.lancedb.databases) and override_path is None diff --git a/evaluations/tests/test_benchmark.py b/evaluations/tests/test_benchmark.py index b471ac0f..5529f23c 100644 --- a/evaluations/tests/test_benchmark.py +++ b/evaluations/tests/test_benchmark.py @@ -711,6 +711,37 @@ class TestEvaluateDatasetJudgeModel: assert mock_qa.call_args[1]["judge_model"] is custom_judge +@pytest.mark.asyncio +async def test_a_db_path_beside_configured_databases_is_refused(tmp_path) -> None: + """`--db` places the database where the configuration places none; beside + `lancedb.databases` the run refuses before touching anything.""" + from haiku.rag.config.models import LanceDBConfig + + config = AppConfig( + lancedb=LanceDBConfig(databases={"alpha": str(tmp_path / "a.lancedb")}) + ) + spec = DatasetSpec( + key="test", + db_filename="test.lancedb", + document_loader=lambda: None, # type: ignore[arg-type] # ty: ignore[invalid-argument-type] + document_mapper=lambda doc: None, + qa_loader=lambda: [], # type: ignore[arg-type] # ty: ignore[invalid-argument-type] + qa_case_builder=lambda idx, doc: None, # type: ignore[arg-type] # ty: ignore[invalid-argument-type] + ) + + with pytest.raises(ValueError, match="alpha"): + await evaluate_dataset( + spec=spec, + config=config, + skip_db=True, + skip_retrieval=True, + skip_qa=True, + limit=None, + name=None, + db_path=tmp_path / "other.lancedb", + ) + + class TestExperimentMetadataTargets: def test_default_target_is_rag_capability(self) -> None: result = build_experiment_metadata( diff --git a/haiku_rag_slim/haiku/rag/app.py b/haiku_rag_slim/haiku/rag/app.py index a8bb4962..0c71ec47 100644 --- a/haiku_rag_slim/haiku/rag/app.py +++ b/haiku_rag_slim/haiku/rag/app.py @@ -86,7 +86,7 @@ class HaikuRAGApp: @property def display_path(self) -> "Path | str": """What a one-database command calls the database it opened.""" - return self._one.db_path or self._one.uri + return self._one.location @property def database_missing(self) -> bool: diff --git a/haiku_rag_slim/haiku/rag/cli.py b/haiku_rag_slim/haiku/rag/cli.py index cbdbaf77..ed6cd813 100644 --- a/haiku_rag_slim/haiku/rag/cli.py +++ b/haiku_rag_slim/haiku/rag/cli.py @@ -90,9 +90,10 @@ def resolve_scope( """The databases a command works on, resolved once. 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. + the same thing said twice, that a human typing `--db PATH` means that + database whatever is configured, and whether this command can read more + than one. Everything else — an unknown name, the default location — is + `DatabaseScope.resolve`'s to answer. """ from haiku.rag.client.scope import DatabaseScope @@ -100,9 +101,9 @@ def resolve_scope( raise AmbiguousDatabaseError( "pass --db or --db-name, not both: they name the same thing" ) - scope = DatabaseScope.resolve( - get_config(), database_name=_db_name, database_path=db - ) + if db is not None: + return DatabaseScope.at(db) + scope = DatabaseScope.resolve(get_config(), database_name=_db_name) if scope.covers_multiple and not covers_set: raise AmbiguousDatabaseError( f"lancedb.databases names {', '.join(sorted(scope.names))}; this " diff --git a/haiku_rag_slim/haiku/rag/client/__init__.py b/haiku_rag_slim/haiku/rag/client/__init__.py index 965d2ff2..4c0e9cb1 100644 --- a/haiku_rag_slim/haiku/rag/client/__init__.py +++ b/haiku_rag_slim/haiku/rag/client/__init__.py @@ -128,17 +128,17 @@ class HaikuRAG: """Initialize the RAG client with a database path. Args: - db_path: Path or string path to the database. When omitted, resolves - ``lancedb.databases``, then ``lancedb.uri``, then the default - path under ``storage.data_dir``. + db_path: Path or string path to the database, named by its stem. + Valid where the configuration places no database; beside + ``lancedb.databases`` it raises ``AmbiguousDatabaseError``. + When omitted, the configured databases are covered, or the + default database ``haiku.rag`` under ``storage.data_dir``. config: Configuration to use. Defaults to the current global config. skip_validation: Whether to skip configuration validation on database load. create: Whether to create the database if it doesn't exist. read_only: Whether to open the database in read-only mode. - sources: Names from ``config.lancedb.databases`` this client covers, - None for all of them. Only that setting names databases, so a - name raises when ``lancedb.uri`` placed the database, and is - rejected alongside ``db_path``, which says the same thing + sources: Names of the databases this client covers, None for all of + them. Rejected alongside ``db_path``, which says the same thing another way. ``[]`` raises too: a client over no database can do nothing, unlike ``sources=[]`` on a search, which is a selection of nothing to search. @@ -165,24 +165,31 @@ class HaikuRAG: @property def covers_multiple(self) -> bool: - """Whether this client reads from more than one database.""" - return isinstance(self._session, FederatedSession) + """Whether this client reads from more than one database. + + Known before the client enters: coverage is a fact of the resolved + scope. + """ + if self._session is not None: + return isinstance(self._session, FederatedSession) + return self._resolve_scope().covers_multiple @property def source_names(self) -> tuple[str, ...]: - """The configured databases this client covers, in configured order. + """The databases this client covers, by name, in configured order. - A single database contributes its own name, or nothing where the - configuration named none. + Known before the client enters: coverage is a fact of the resolved + scope. """ if isinstance(self._session, FederatedSession): return self._session.names - return () if self.source is None else (self.source,) + if isinstance(self._session, SingleDatabaseSession): + return (self._session.source,) + return self._resolve_scope().names @property def source(self) -> str | None: - """The configured database this client reads, or None while covering a - set or reading a database the configuration did not name.""" + """The database this client reads, or None while covering a set.""" if isinstance(self._session, SingleDatabaseSession): return self._session.source return None @@ -403,9 +410,8 @@ class HaikuRAG: `lender` is the client that opened it, whose reranker this one borrows. """ - client = cls( - session.db_path, config=session.config, read_only=session.read_only - ) + client = cls(config=session.config, read_only=session.read_only) + client._scope = DatabaseScope((session.ref,)) client._session = session client._owns_session = False client._lender = lender @@ -851,7 +857,7 @@ class HaikuRAG: if unknown: raise UnknownDatabaseError( f"unknown database(s) {', '.join(sorted(set(unknown)))}; this " - f"client covers {', '.join(sorted(covered)) or 'a single unnamed database'}" + f"client covers {', '.join(sorted(covered))}" ) async def clients_covering( @@ -875,8 +881,8 @@ class HaikuRAG: return [] if sources != [self.source]: raise UnknownDatabaseError( - f"unknown database(s) {', '.join(sources) or '(none)'}; this " - f"client covers {self.source or 'a single unnamed database'}" + f"unknown database(s) {', '.join(sources)}; this client covers " + f"{self.source}" ) return [self] @@ -898,9 +904,6 @@ class HaikuRAG: if not await self.clients_covering(sources): return [] results = await search(self, query, limit, search_type, filter, include_images) - # A database named in config keeps its name even when it is the only one - # this client covers. Only an unnamed `lancedb.uri` database leaves - # source unset. for result in results: result.source = self.source return results diff --git a/haiku_rag_slim/haiku/rag/client/scope.py b/haiku_rag_slim/haiku/rag/client/scope.py index 6c53af75..cb243016 100644 --- a/haiku_rag_slim/haiku/rag/client/scope.py +++ b/haiku_rag_slim/haiku/rag/client/scope.py @@ -1,3 +1,4 @@ +from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path @@ -8,45 +9,60 @@ from haiku.rag.store.exceptions import ( ) from haiku.rag.utils import locate_database +DEFAULT_DATABASE_FILENAME = "haiku.rag.lancedb" + + +def database_name(path: Path) -> str: + """The name a database at `path` answers to: the path's stem.""" + if not path.stem: + raise ValueError(f"a database at {path} has no name: the path has no stem") + return path.stem + @dataclass(frozen=True) class DatabaseRef: - """A resolved database location, and the configured name it answers to. + """A resolved database: the name it answers to, and where it is. - Exactly one of ``uri`` and ``db_path`` is set. ``name`` is the key from - ``lancedb.databases``, and the only identity that leaves the configuration: - it travels in results, citations and errors, where a location must not. - None where nothing names the database. + ``name`` is the key from ``lancedb.databases``, or the stem of a path the + caller gave. It is the only identity that leaves the configuration: it + travels in results, citations and errors, where a location must not. + ``location`` is a local path, or a URI. ``given`` marks a path the caller + gave, whose errors may name it: the caller already knows where it is. """ - name: str | None - uri: str - db_path: Path | None + name: str + location: Path | str + given: bool = False def __post_init__(self) -> None: - if bool(self.uri) == (self.db_path is not None): + if not self.name.strip(): + raise ValueError(f"a database at {self.location} has no name") + if isinstance(self.location, str): + if not self.location.strip(): + raise ValueError(f"database {self.name!r} has no location") + object.__setattr__(self, "location", locate_database(self.location)) + if self.given and not isinstance(self.location, Path): raise ValueError( - "a database is either a URI or a local path: " - f"got uri={self.uri!r} and db_path={self.db_path!r}" + f"database {self.name!r} is given as a path, and {self.location} " + "is a URI" ) @classmethod - def at(cls, path: Path | str, *, name: str | None = None) -> "DatabaseRef": + def at(cls, path: Path | str) -> "DatabaseRef": """A database at a path the caller named, taken as given.""" - return cls(name=name, uri="", db_path=Path(path)) + path = Path(path) + return cls(name=database_name(path), location=path, given=True) @classmethod - def configured(cls, name: str | None, location: str) -> "DatabaseRef": - """A database the configuration placed, by ``lancedb.uri`` or an entry in - ``lancedb.databases``. A location carrying a scheme is a URI, anything - else a local path.""" - uri, db_path = locate_database(location) - return cls(name=name, uri=uri, db_path=db_path) + def configured(cls, name: str, location: str | Path) -> "DatabaseRef": + """A database the configuration placed. A location carrying a scheme is + a URI, anything else a local path.""" + return cls(name=name, location=location) @property - def location(self) -> Path | str: - """Where the database is: its path, or its URI.""" - return self.db_path if self.db_path is not None else self.uri + def db_path(self) -> Path | None: + """The local path, or None for a database behind a URI.""" + return self.location if isinstance(self.location, Path) else None @dataclass(frozen=True) @@ -66,6 +82,14 @@ class DatabaseScope: if not self.databases: raise ValueError("a scope covers at least one database") + @classmethod + def at(cls, path: Path | str) -> "DatabaseScope": + """One database at a path the caller named, whatever is configured. + + The CLI's ``--db``: a human typing a path means that database. + """ + return cls((DatabaseRef.at(path),)) + @classmethod def resolve( cls, @@ -76,9 +100,10 @@ class DatabaseScope: ) -> "DatabaseScope": """The databases named by `config` and at most one selector. - A path names one database that nothing calls anything; a name selects one - of the configured set and keeps its name. With no selector the configured - set is covered in configuration order, a set of one included. + The configuration places databases: ``lancedb.databases``, or where it + names none, the default database under ``storage.data_dir`` as the entry + ``haiku.rag``. A name selects one of them. A path places a database + where the configuration places none, and is refused beside one it does. """ if database_name is not None and database_path is not None: raise AmbiguousDatabaseError( @@ -86,33 +111,37 @@ class DatabaseScope: "pass one of them" ) - declared = config.lancedb.databases + configured = config.lancedb.databases if database_path is not None: - return cls((DatabaseRef.at(database_path),)) + if configured: + raise AmbiguousDatabaseError( + "a database path and lancedb.databases both place the " + f"database: db_path={Path(database_path)} and databases " + f"name {', '.join(sorted(configured))}; pass one of them" + ) + return cls.at(database_path) + + declared: Mapping[str, str | Path] = configured or { + "haiku.rag": config.storage.data_dir / DEFAULT_DATABASE_FILENAME + } if database_name is not None: if database_name not in declared: raise UnknownDatabaseError( f"unknown database {database_name!r}; lancedb.databases names " - f"{', '.join(sorted(declared)) or 'nothing'}" + f"{', '.join(sorted(declared))}" ) return cls( (DatabaseRef.configured(database_name, declared[database_name]),) ) - if declared: - return cls( - tuple( - DatabaseRef.configured(name, location) - for name, location in declared.items() - ) + return cls( + tuple( + DatabaseRef.configured(name, location) + for name, location in declared.items() ) - - if config.lancedb.uri: - return cls((DatabaseRef.configured(None, config.lancedb.uri),)) - - return cls((DatabaseRef.at(config.storage.data_dir / "haiku.rag.lancedb"),)) + ) def select(self, names: list[str]) -> "DatabaseScope": """The databases in this scope named by `names`, in the order given. @@ -123,7 +152,7 @@ class DatabaseScope: raise ValueError( "sources=[] selects no database; pass None for all of them" ) - by_name = {ref.name: ref for ref in self.databases if ref.name is not None} + by_name = {ref.name: ref for ref in self.databases} missing = [name for name in names if name not in by_name] if missing: raise UnknownDatabaseError( @@ -139,5 +168,5 @@ class DatabaseScope: @property def names(self) -> tuple[str, ...]: - """The configured names covered, in order. Empty where none is named.""" - return tuple(ref.name for ref in self.databases if ref.name is not None) + """The names of the databases covered, in order.""" + return tuple(ref.name for ref in self.databases) diff --git a/haiku_rag_slim/haiku/rag/client/session.py b/haiku_rag_slim/haiku/rag/client/session.py index 01dec0b3..5b3a83b3 100644 --- a/haiku_rag_slim/haiku/rag/client/session.py +++ b/haiku_rag_slim/haiku/rag/client/session.py @@ -47,9 +47,8 @@ class SingleDatabaseSession: """One database: its store, its repositories, and their lifecycle. Everything that needs a store lives here, so nothing above has to ask whether - it has one. Built from the resolved reference: ``source`` is the configured - name it answers to, or None where nothing names it, and the store receives - its location. + it has one. Built from the resolved reference: ``source`` is the name it + answers to, and the store receives its location. ``ref``, ``config``, ``read_only`` and ``source`` are readable: a client borrowing this session reports them as its own. @@ -74,7 +73,7 @@ class SingleDatabaseSession: self._vacuum_dirty = False @property - def source(self) -> str | None: + def source(self) -> str: return self.ref.name @property @@ -107,12 +106,12 @@ class SingleDatabaseSession: raise except _NAMEABLE_FAILURES as error: # The message keeps its remedy and gains the database's name. - if self.source is None: + if self.ref.given: raise raise type(error)(f"database {self.source!r}: {error}") from error except Exception as error: - # Without a name there is nothing to report in the location's place. - if self.source is None: + # A path the caller gave may be named: the caller knows it already. + if self.ref.given: raise failure = type(error).__name__ if failure is not None: @@ -266,9 +265,7 @@ class FederatedSession: skip_validation: bool = False, read_only: bool = False, ) -> None: - self._refs: dict[str, DatabaseRef] = { - ref.name: ref for ref in scope.databases if ref.name is not None - } + self._refs: dict[str, DatabaseRef] = {ref.name: ref for ref in scope.databases} self._config = config self._skip_validation = skip_validation self._read_only = read_only diff --git a/haiku_rag_slim/haiku/rag/config/models.py b/haiku_rag_slim/haiku/rag/config/models.py index c32020a0..40516a5b 100644 --- a/haiku_rag_slim/haiku/rag/config/models.py +++ b/haiku_rag_slim/haiku/rag/config/models.py @@ -102,12 +102,12 @@ class LanceDBConfig(ConfigModel): The cache sizes are per process, since the session is shared across connections. - `databases` maps a name to a location, for searching multiple at once. The - name is what results and citations carry, so a location never leaves the - configuration. Mutually exclusive with `uri`. + `databases` maps a name to a location, a local path or a URI, and is the one + way to place databases. The name is what results and citations carry, so a + location never leaves the configuration. Empty means the default database, + `haiku.rag`, under `storage.data_dir`. """ - uri: str = "" api_key: str = "" region: str = "" storage_options: dict[str, str] = Field(default_factory=dict) @@ -116,13 +116,18 @@ class LanceDBConfig(ConfigModel): index_cache_size_bytes: int | None = Field(default=None, ge=0) metadata_cache_size_bytes: int | None = Field(default=None, ge=0) - @model_validator(mode="after") - def _one_way_of_naming_databases(self) -> "LanceDBConfig": - if self.uri and self.databases: + @model_validator(mode="before") + @classmethod + def _uri_names_its_replacement(cls, data: Any) -> Any: + if isinstance(data, dict) and "uri" in data: raise ValueError( - "lancedb.uri and lancedb.databases are mutually exclusive: " - "use uri for one unnamed location, or databases for named ones" + "lancedb.uri was removed; write lancedb.databases: {NAME: " + f"{data['uri']!r}}} instead" ) + return data + + @model_validator(mode="after") + def _every_database_is_named_and_placed(self) -> "LanceDBConfig": for name, location in self.databases.items(): # A blank name is falsy, so source routing reads it as absent; a # blank location resolves to the working directory. diff --git a/haiku_rag_slim/haiku/rag/ingester/app.py b/haiku_rag_slim/haiku/rag/ingester/app.py index 4de65630..f3236497 100644 --- a/haiku_rag_slim/haiku/rag/ingester/app.py +++ b/haiku_rag_slim/haiku/rag/ingester/app.py @@ -4,7 +4,6 @@ import signal from collections.abc import Callable from contextlib import asynccontextmanager from datetime import UTC, datetime -from pathlib import Path from typing import TYPE_CHECKING from pydantic import BaseModel @@ -22,6 +21,8 @@ from haiku.rag.ingester.workers.retry import RetryPolicy if TYPE_CHECKING: from sqlalchemy.ext.asyncio import AsyncEngine + from haiku.rag.client.scope import DatabaseScope + logger = logging.getLogger(__name__) _MANIFEST_EXTRA_KEY = "_manifest" @@ -72,14 +73,14 @@ class IngesterApp: WorkerPool, and a HaikuRAG client for the worker pool to ingest through. """ - def __init__(self, *, config: AppConfig, db_path: Path | None = None): + def __init__(self, *, config: AppConfig, scope: "DatabaseScope | None" = None): + """The ingester over the database `scope` covers, or the one the + configuration places when no scope is handed in.""" from haiku.rag.client.scope import DatabaseScope from haiku.rag.store.exceptions import AmbiguousDatabaseError self._config = config - # `--db` is an explicit override; None leaves placement to the - # configuration. - self._scope = DatabaseScope.resolve(config, database_path=db_path) + self._scope = scope if scope is not None else DatabaseScope.resolve(config) if self._scope.covers_multiple: raise AmbiguousDatabaseError( "haiku-ingester writes one database, and lancedb.databases " diff --git a/haiku_rag_slim/haiku/rag/ingester/cli.py b/haiku_rag_slim/haiku/rag/ingester/cli.py index 4a1684ca..5b764048 100644 --- a/haiku_rag_slim/haiku/rag/ingester/cli.py +++ b/haiku_rag_slim/haiku/rag/ingester/cli.py @@ -42,6 +42,7 @@ from haiku.rag.store.exceptions import ( # noqa: E402 ) if TYPE_CHECKING: + from haiku.rag.client.scope import DatabaseScope from haiku.rag.ingester.app import BatchProgress, BatchProgressCallback _cli = typer.Typer( @@ -218,6 +219,14 @@ def _load_manifest(path: Path) -> BatchManifest: return BatchManifest.model_validate(data) +def _scope_for(db: Path | None) -> "DatabaseScope | None": + """`--db PATH` is the operator's explicit override: that database, whatever + is configured. None leaves placement to the configuration.""" + from haiku.rag.client.scope import DatabaseScope + + return DatabaseScope.at(db) if db is not None else None + + @_cli.command("serve") def serve( db: Path | None = typer.Option( @@ -259,7 +268,7 @@ def serve( app_config.ingester.api.port = port if root_path is not None: app_config.ingester.api.root_path = root_path - app = IngesterApp(config=app_config, db_path=db) + app = IngesterApp(config=app_config, scope=_scope_for(db)) asyncio.run(app.serve(api=not no_api)) @@ -318,7 +327,7 @@ async def _run_batch( ) -> None: from haiku.rag.ingester.app import IngesterApp - app = IngesterApp(config=app_config, db_path=db_path) + app = IngesterApp(config=app_config, scope=_scope_for(db_path)) if dry_run: report = await app.run_batch_dry_run() if report.failed_sweeps: diff --git a/haiku_rag_slim/haiku/rag/mcp.py b/haiku_rag_slim/haiku/rag/mcp.py index 569c66db..f5f808a3 100644 --- a/haiku_rag_slim/haiku/rag/mcp.py +++ b/haiku_rag_slim/haiku/rag/mcp.py @@ -32,9 +32,9 @@ def create_mcp_server( """Create an MCP server over one database. Args: - db_path: Path to the database file, or None to let `config` place it. A - path overrides a configured `lancedb.uri`: for a URI-backed - database, pass None. + db_path: Path to the database file, where `config` places none; or + None to serve the database the configuration places. Beside + `lancedb.databases` a path raises `AmbiguousDatabaseError`. config: Configuration to use. read_only: If True, write tools (add_document_*, delete_document) are not registered. """ diff --git a/haiku_rag_slim/haiku/rag/utils.py b/haiku_rag_slim/haiku/rag/utils.py index 812740ea..3ab93a9f 100644 --- a/haiku_rag_slim/haiku/rag/utils.py +++ b/haiku_rag_slim/haiku/rag/utils.py @@ -549,17 +549,15 @@ def raise_missing_extra(module: str, extra: str, exc: ModuleNotFoundError) -> No ) from exc -def locate_database(location: str) -> tuple[str, Path | None]: - """Split a configured location into (uri, db_path). +def locate_database(location: str) -> Path | str: + """A configured location as a URI, or as a local path. - A value with a scheme is a `lancedb.uri`; anything else is a local path. - `ConnectionMode` classifies a `uri` as object storage and opens it without - the existence check a local database gets, so a local path never travels - as one. + A value with a scheme is a URI, which `ConnectionMode` opens without the + existence check a local database gets; anything else is a local path. """ if "://" in location: - return location, None - return "", Path(location) + return location + return Path(location) def get_default_data_dir() -> Path: diff --git a/tests/capabilities/test_capabilities.py b/tests/capabilities/test_capabilities.py index a3202542..8ea532ff 100644 --- a/tests/capabilities/test_capabilities.py +++ b/tests/capabilities/test_capabilities.py @@ -124,30 +124,28 @@ class TestACapabilityFollowsTheConfiguredLocation: """A capability nobody handed a client opens one for itself, at the database the configuration places.""" - def _config(self, tmp_path, uri: str) -> AppConfig: + def _config(self, tmp_path, location: str) -> AppConfig: from haiku.rag.config.models import LanceDBConfig, StorageConfig return AppConfig( - lancedb=LanceDBConfig(uri=uri), + lancedb=LanceDBConfig(databases={"notes": location}), storage=StorageConfig(data_dir=tmp_path / "elsewhere"), ) - def test_a_configured_uri_is_left_to_the_client(self, tmp_path): - """A path overrides a configured location, so the capability passes - None and the client resolves the configured URI.""" + def test_a_configured_location_is_the_capability_scope(self, tmp_path): located = tmp_path / "notes.lancedb" for factory in (create_rag, create_analysis): [local] = factory( config=self._config(tmp_path, str(located)) ).scope.databases - assert local == DatabaseRef.configured(None, str(located)) + assert local == DatabaseRef("notes", located) remote = self._config(tmp_path, "s3://bucket/one.lancedb") [ref] = factory(config=remote).scope.databases - assert ref == DatabaseRef(None, "s3://bucket/one.lancedb", None) + assert ref == DatabaseRef("notes", "s3://bucket/one.lancedb") @pytest.mark.asyncio - async def test_it_opens_the_database_the_uri_places(self, tmp_path): + async def test_it_opens_the_database_the_configuration_places(self, tmp_path): from haiku.rag.client import HaikuRAG located = tmp_path / "notes.lancedb" @@ -162,19 +160,26 @@ class TestACapabilityFollowsTheConfiguredLocation: finally: await capability._close() - def test_an_explicit_path_still_overrides_the_configured_uri(self, tmp_path): + def test_a_path_beside_the_configured_placement_is_refused(self, tmp_path): + from haiku.rag.store.exceptions import AmbiguousDatabaseError + config = self._config(tmp_path, str(tmp_path / "notes.lancedb")) chosen = tmp_path / "chosen.lancedb" - assert _placed(create_rag(db_path=chosen, config=config)) == chosen + for factory in (create_rag, create_analysis): + with pytest.raises(AmbiguousDatabaseError, match="notes"): + factory(db_path=chosen, config=config) - def test_the_environment_still_overrides_the_configured_uri( + def test_the_environment_beside_the_configured_placement_is_refused( self, tmp_path, monkeypatch ): + from haiku.rag.store.exceptions import AmbiguousDatabaseError + config = self._config(tmp_path, "s3://bucket/one.lancedb") monkeypatch.setenv("HAIKU_RAG_DB", str(tmp_path / "from-env.lancedb")) - assert _placed(create_rag(config=config)) == tmp_path / "from-env.lancedb" + with pytest.raises(AmbiguousDatabaseError, match="notes"): + create_rag(config=config) @pytest.mark.asyncio @@ -1691,22 +1696,18 @@ class TestMultipleCollectionsInstructions: (create_rag, rag_text), (create_analysis, analysis_text), ): - for config in (AppConfig(), self._config(alpha="/a.lancedb")): - capability = factory(db_path=Path("/tmp/x.lancedb"), config=config) - assert capability.instruction_text == baseline() + one_at_a_path = factory(db_path=Path("/tmp/x.lancedb"), config=AppConfig()) + assert one_at_a_path.instruction_text == baseline() + one_configured = factory(config=self._config(alpha="/a.lancedb")) + assert one_configured.instruction_text == baseline() - def test_an_explicit_path_opens_one_database(self): - """A path names one database, whatever the configuration names.""" - from haiku.rag.capabilities.analysis import instructions as analysis_text - from haiku.rag.capabilities.rag import instructions as rag_text + def test_a_path_beside_a_configured_set_is_refused(self): + from haiku.rag.store.exceptions import AmbiguousDatabaseError config = self._config(alpha="/a.lancedb", beta="/b.lancedb") - for factory, baseline in ( - (create_rag, rag_text), - (create_analysis, analysis_text), - ): - capability = factory(db_path=Path("/tmp/one.lancedb"), config=config) - assert capability.instruction_text == baseline() + for factory in (create_rag, create_analysis): + with pytest.raises(AmbiguousDatabaseError, match="alpha, beta"): + factory(db_path=Path("/tmp/one.lancedb"), config=config) def test_a_lent_client_covering_one_database_is_instructed_as_before(self): from haiku.rag.capabilities.analysis import instructions as analysis_text diff --git a/tests/ingester/test_api.py b/tests/ingester/test_api.py index 876aa0bd..6e86ef55 100644 --- a/tests/ingester/test_api.py +++ b/tests/ingester/test_api.py @@ -838,11 +838,11 @@ async def test_database_503_when_no_database_is_configured(state): @pytest.mark.asyncio async def test_the_report_follows_a_configured_uri(tmp_path, jobs, sync): - """A configured `lancedb.uri` places the database, so the report opens that + """A configured location places the database, so the report opens that and not the local default.""" db_path = tmp_path / "configured.lancedb" await _seed_lancedb(db_path) - config = AppConfig(lancedb=LanceDBConfig(uri=str(db_path))) + config = AppConfig(lancedb=LanceDBConfig(databases={"configured": str(db_path)})) state = APIState( config=config, job_repo=jobs, diff --git a/tests/ingester/test_cli.py b/tests/ingester/test_cli.py index df30d594..54cfaf3e 100644 --- a/tests/ingester/test_cli.py +++ b/tests/ingester/test_cli.py @@ -504,31 +504,54 @@ def test_cli_entry_point_exits_on_store_state_errors(monkeypatch, capsys, error) class TestPlacingTheIngesterDatabase: """The ingester writes wherever the configuration places the database, and - resolves that once. A path is an explicit override of a configured - `lancedb.uri`, so no local default stands in for one.""" + resolves that once. `--db PATH` is the operator's explicit override and + constructs the scope directly.""" @staticmethod def _app(config: AppConfig, db_path=None): from haiku.rag.ingester.app import IngesterApp + from haiku.rag.ingester.cli import _scope_for - return IngesterApp(config=config, db_path=db_path) + return IngesterApp(config=config, scope=_scope_for(db_path)) - def test_a_configured_uri_becomes_the_scope(self, tmp_path): - config = AppConfig(lancedb=LanceDBConfig(uri="s3://bucket/prod.lancedb")) + def test_a_configured_location_becomes_the_scope(self, tmp_path): + config = AppConfig( + lancedb=LanceDBConfig(databases={"prod": "s3://bucket/prod.lancedb"}) + ) [ref] = self._app(config)._scope.databases - assert ref.uri == "s3://bucket/prod.lancedb" + assert ref.name == "prod" + assert ref.location == "s3://bucket/prod.lancedb" assert ref.db_path is None - def test_an_override_names_the_database(self, tmp_path): - config = AppConfig(lancedb=LanceDBConfig(uri="s3://bucket/prod.lancedb")) + def test_an_override_names_the_database_by_its_stem(self, tmp_path): + config = AppConfig( + lancedb=LanceDBConfig(databases={"prod": "s3://bucket/prod.lancedb"}) + ) override = tmp_path / "local.lancedb" [ref] = self._app(config, override)._scope.databases - assert ref.db_path == override - assert ref.uri == "" + assert ref.name == "local" + assert ref.location == override + + def test_the_override_is_the_cli_s_alone(self, tmp_path): + """`IngesterApp` takes a resolved scope, so a Python caller has no path + to slip past the configuration; only the CLI constructs one.""" + from haiku.rag.client.scope import DatabaseScope + from haiku.rag.ingester.cli import _scope_for + + assert _scope_for(None) is None + assert _scope_for(tmp_path / "local.lancedb") == DatabaseScope.at( + tmp_path / "local.lancedb" + ) + + def test_a_python_caller_cannot_pass_a_path(self): + from haiku.rag.ingester.app import IngesterApp + + with pytest.raises(TypeError): + IngesterApp(config=AppConfig(), db_path="/db/other.lancedb") # type: ignore[call-arg] # ty: ignore[unknown-argument] def test_one_configured_database_is_accepted(self, tmp_path): """A one-entry mapping names which database to write.""" @@ -567,6 +590,10 @@ class TestPlacingTheIngesterDatabase: f" a: {tmp_path / 'a.lancedb'}\n" f" b: {tmp_path / 'b.lancedb'}\n" ) + import haiku.rag.config as config_module + + # The CLI caches the loaded configuration process-wide. + monkeypatch.setattr(config_module, "_config", None) monkeypatch.setattr(sys, "argv", ["haiku-ingester", "run-batch"]) monkeypatch.setenv("HAIKU_RAG_CONFIG_PATH", str(config_file)) diff --git a/tests/ingester/test_run_batch.py b/tests/ingester/test_run_batch.py index 4ef04041..c4d259b9 100644 --- a/tests/ingester/test_run_batch.py +++ b/tests/ingester/test_run_batch.py @@ -14,6 +14,7 @@ import pytest from haiku.rag.client import HaikuRAG from haiku.rag.client.exceptions import UnsupportedSourceError +from haiku.rag.client.scope import DatabaseScope from haiku.rag.config import ( APIConfig, AppConfig, @@ -121,7 +122,7 @@ async def test_run_batch_drains_upserts(tmp_path, use_client): use_client(client) report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch() assert report.succeeded == 2 @@ -146,7 +147,7 @@ async def test_run_batch_reports_progress(tmp_path, use_client): progress = [] report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch(progress_callback=progress.append) assert report.succeeded == 2 @@ -171,7 +172,9 @@ async def test_run_batch_prunes_orphans(tmp_path, use_client): db_path = tmp_path / "db.lancedb" # First batch ingests both files and records sync_state for each. - first = await IngesterApp(config=config, db_path=db_path).run_batch() + first = await IngesterApp( + config=config, scope=DatabaseScope.at(db_path) + ).run_batch() assert first.succeeded == 2 client.delete_document.assert_not_awaited() @@ -179,7 +182,9 @@ async def test_run_batch_prunes_orphans(tmp_path, use_client): # sync_state but not on disk -> enqueues a DELETE for it. (tmp_path / "b.md").unlink() - second = await IngesterApp(config=config, db_path=db_path).run_batch() + second = await IngesterApp( + config=config, scope=DatabaseScope.at(db_path) + ).run_batch() # a.md is unchanged (same mtime) so it's not re-ingested; only the orphan # delete runs. @@ -198,7 +203,7 @@ async def test_run_batch_reports_dead_on_permanent_failure(tmp_path, use_client) use_client(client) report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch() assert report.succeeded == 0 @@ -217,7 +222,9 @@ async def test_run_batch_recovered_doc_is_not_counted_as_dead(tmp_path, use_clie failing = _mock_client() failing.create_document_from_source.side_effect = UnsupportedSourceError("nope") use_client(failing) - first = await IngesterApp(config=config, db_path=db_path).run_batch() + first = await IngesterApp( + config=config, scope=DatabaseScope.at(db_path) + ).run_batch() assert first.dead == 1 healthy = _mock_client() @@ -226,7 +233,9 @@ async def test_run_batch_recovered_doc_is_not_counted_as_dead(tmp_path, use_clie # records the revision in sync_state, so a plain re-run no longer retries an # unchanged file — recovery needs the content (mtime) to change. (tmp_path / "a.md").write_text("hello again") - second = await IngesterApp(config=config, db_path=db_path).run_batch() + second = await IngesterApp( + config=config, scope=DatabaseScope.at(db_path) + ).run_batch() assert second.dead == 0 assert second.succeeded == 1 @@ -248,7 +257,7 @@ async def test_run_batch_reports_failed_sweep( with caplog.at_level("ERROR", logger="haiku.rag.ingester.pollers.base"): report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch() assert report.failed_sweeps == ["local"] @@ -264,7 +273,7 @@ async def test_run_batch_empty_source_returns_immediately(tmp_path, use_client): report = await asyncio.wait_for( IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch(), timeout=5.0, ) @@ -287,7 +296,9 @@ async def test_run_batch_dry_run_reports_manifest_without_mutating_queue(tmp_pat finally: await engine.dispose() - report = await IngesterApp(config=config, db_path=db_path).run_batch_dry_run() + report = await IngesterApp( + config=config, scope=DatabaseScope.at(db_path) + ).run_batch_dry_run() assert report.failed_sweeps == [] assert report.manifest.version == 1 @@ -322,7 +333,7 @@ async def test_run_batch_from_manifest_drains_changes_without_sweeping( monkeypatch.setattr(PollerManager, "sweep_all", sweep_all) report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest( _manifest( BatchChange( @@ -350,7 +361,7 @@ async def test_run_batch_from_manifest_rejects_stale_upsert_revision( use_client(client) report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest( _manifest( BatchChange( @@ -378,7 +389,7 @@ async def test_run_batch_from_manifest_delete_uses_manifest_even_if_file_reappea use_client(client) report = await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest( _manifest( BatchChange( @@ -431,7 +442,7 @@ async def test_run_batch_from_manifest_resumes_same_manifest_work(tmp_path, use_ await engine.dispose() report = await IngesterApp( - config=config, db_path=tmp_path / "db.lancedb" + config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest(manifest) assert report.succeeded == 1 @@ -456,7 +467,7 @@ async def test_run_batch_from_manifest_rejects_non_manifest_pending_work( with pytest.raises(ValueError, match="non-manifest pending work"): await IngesterApp( - config=config, db_path=tmp_path / "db.lancedb" + config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest( _manifest( BatchChange( @@ -505,7 +516,7 @@ async def test_run_batch_from_manifest_rejects_different_manifest_pending_work( with pytest.raises(ValueError, match="non-manifest pending work"): await IngesterApp( - config=config, db_path=tmp_path / "db.lancedb" + config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest(manifest) @@ -526,7 +537,7 @@ async def test_run_batch_from_manifest_rejects_unrelated_pending_work( with pytest.raises(ValueError, match="non-manifest pending work"): await IngesterApp( - config=config, db_path=tmp_path / "db.lancedb" + config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest( _manifest( BatchChange( @@ -554,7 +565,7 @@ async def test_run_batch_from_manifest_rejects_duplicate_changes(tmp_path, use_c with pytest.raises(ValueError, match="duplicate change"): await IngesterApp( - config=_config(tmp_path), db_path=tmp_path / "db.lancedb" + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") ).run_batch_from_manifest(_manifest(change, change)) @@ -583,7 +594,9 @@ async def test_run_batch_aborts_when_all_workers_die( with caplog.at_level("ERROR", logger="haiku.rag.ingester.app"): report = await asyncio.wait_for( - IngesterApp(config=config, db_path=tmp_path / "db.lancedb").run_batch(), + IngesterApp( + config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb") + ).run_batch(), timeout=10.0, ) @@ -608,7 +621,7 @@ async def test_serve_starts_workers_pollers_and_shuts_down(tmp_path, use_client, use_client(_mock_client()) config = _config(tmp_path) config.ingester.api = APIConfig(enabled=api, host="127.0.0.1", port=0) - app = IngesterApp(config=config, db_path=tmp_path / "db.lancedb") + app = IngesterApp(config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb")) task = asyncio.create_task(app.serve(api=api)) try: @@ -653,7 +666,7 @@ async def test_stop_pool_warns_when_shutdown_grace_elapses(tmp_path, caplog): """When a worker doesn't stop within the shutdown grace, _stop_pool logs a warning and still drains any pending cancel-cleanup releases.""" config = _config(tmp_path, shutdown_grace_s=0.01) - app = IngesterApp(config=config, db_path=tmp_path / "db.lancedb") + app = IngesterApp(config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb")) pool = _SlowPool() app._pool = pool @@ -694,7 +707,9 @@ async def test_run_batch_closes_sources_after_pool_stops( ): (tmp_path / "a.md").write_text("hello") use_client(_mock_client()) - app = IngesterApp(config=_config(tmp_path), db_path=tmp_path / "db.lancedb") + app = IngesterApp( + config=_config(tmp_path), scope=DatabaseScope.at(tmp_path / "db.lancedb") + ) order = _record_close_order(monkeypatch) await app.run_batch() @@ -707,7 +722,7 @@ async def test_serve_closes_sources_after_pool_stops(tmp_path, use_client, monke use_client(_mock_client()) config = _config(tmp_path) config.ingester.api = APIConfig(enabled=False) - app = IngesterApp(config=config, db_path=tmp_path / "db.lancedb") + app = IngesterApp(config=config, scope=DatabaseScope.at(tmp_path / "db.lancedb")) order = _record_close_order(monkeypatch) task = asyncio.create_task(app.serve(api=False)) diff --git a/tests/multi_db/helpers.py b/tests/multi_db/helpers.py index 3802d810..ebfc565f 100644 --- a/tests/multi_db/helpers.py +++ b/tests/multi_db/helpers.py @@ -40,8 +40,8 @@ async def _restore_embedder(config, name, *, provider=None, model_name=None): import lancedb - _, db_path = locate_database(config.lancedb.databases[name]) - assert db_path is not None + db_path = locate_database(config.lancedb.databases[name]) + assert not isinstance(db_path, str) db = await lancedb.connect_async(str(db_path.resolve())) table = await db.open_table("settings") rows = ( diff --git a/tests/multi_db/test_documents.py b/tests/multi_db/test_documents.py index 54a08776..83ad766e 100644 --- a/tests/multi_db/test_documents.py +++ b/tests/multi_db/test_documents.py @@ -459,8 +459,9 @@ class TestDocumentsNameTheirDatabase: assert by_uri is not None and by_uri.source == "alpha" @pytest.mark.asyncio - async def test_one_database_leaves_the_source_unset(self, tmp_path, temp_db_path): - """Nothing names the database when there is only one to name.""" + async def test_one_database_at_a_path_is_named_by_its_stem( + self, tmp_path, temp_db_path + ): async with HaikuRAG(temp_db_path, create=True) as rag: dim = get_config().embeddings.model.vector_dim doc = DoclingDocument(name="solo") @@ -472,6 +473,7 @@ class TestDocumentsNameTheirDatabase: ) [listed] = await rag.list_documents() - assert listed.source is None + assert listed.source == temp_db_path.stem assert listed.id is not None - assert (await rag.get_document_by_id(listed.id)).source is None + by_id = await rag.get_document_by_id(listed.id) + assert by_id is not None and by_id.source == temp_db_path.stem diff --git a/tests/multi_db/test_scope.py b/tests/multi_db/test_scope.py index 9453ae34..e83e8d98 100644 --- a/tests/multi_db/test_scope.py +++ b/tests/multi_db/test_scope.py @@ -8,6 +8,7 @@ from haiku.rag.client.scope import DatabaseScope from haiku.rag.config.models import AppConfig, LanceDBConfig from haiku.rag.store.exceptions import ( AmbiguousDatabaseError, + SourceUnavailableError, UnknownDatabaseError, ) from haiku.rag.utils import locate_database @@ -18,18 +19,13 @@ from tests.multi_db.helpers import ( class TestConfig: - def test_databases_and_uri_are_mutually_exclusive(self): - with pytest.raises(ValidationError, match="databases"): - LanceDBConfig( - uri="s3://b/one.lancedb", databases={"one": "s3://b/one.lancedb"} - ) - - def test_databases_alone_is_fine(self): + def test_databases_is_the_one_placement(self): config = LanceDBConfig(databases={"one": "s3://b/one.lancedb"}) assert config.databases == {"one": "s3://b/one.lancedb"} - def test_uri_alone_is_fine(self): - assert LanceDBConfig(uri="s3://b/one.lancedb").databases == {} + def test_uri_is_refused_naming_the_replacement(self): + with pytest.raises(ValidationError, match="lancedb.databases"): + LanceDBConfig.model_validate({"uri": "s3://b/one.lancedb"}) class TestNamingIsRequired: @@ -51,18 +47,18 @@ class TestNamingIsRequired: class TestNamingADatabaseDirectly: @pytest.mark.asyncio - async def test_an_explicit_db_path_wins_over_the_configured_set( + async def test_a_db_path_beside_the_configured_set_is_refused( self, tmp_path, temp_db_path ): - """A caller that names a path means that database, not the configured - set: the CLI resolves `--db` to one and must not fan out instead.""" + """The configuration places databases; a path beside it is a second + placement, and the refusal names both.""" config = _config(tmp_path, ["alpha", "beta"]) - await _seed(config, "alpha", ["alpha document about cats"]) - async with HaikuRAG(temp_db_path, config=config, create=True) as rag: - assert not rag.covers_multiple - assert rag.source is None - assert rag.store.db_path == temp_db_path + with pytest.raises(AmbiguousDatabaseError, match="alpha") as raised: + async with HaikuRAG(temp_db_path, config=config, create=True): + pass + assert str(temp_db_path) in str(raised.value) + assert not temp_db_path.exists() @pytest.mark.asyncio async def test_one_configured_database_is_opened_by_name(self, tmp_path): @@ -79,42 +75,43 @@ class TestNamingADatabaseDirectly: class TestOneConfiguredLocation: - """`lancedb.uri` places one unnamed database, at a URI or at a local path.""" + """One entry in `lancedb.databases` places one named database, at a URI or + at a local path.""" def _config(self, location) -> AppConfig: - return AppConfig(lancedb=LanceDBConfig(uri=str(location))) + return AppConfig(lancedb=LanceDBConfig(databases={"notes": str(location)})) @pytest.mark.asyncio - async def test_a_local_uri_opens_the_configured_database(self, tmp_path): + async def test_a_local_location_opens_the_configured_database(self, tmp_path): located = tmp_path / "notes.lancedb" config = self._config(located) async with HaikuRAG(config=config, create=True) as rag: assert rag.store.db_path == located - # It places a database without naming one: only `lancedb.databases` - # assigns the name results and citations carry. - assert rag.source is None + assert rag.source == "notes" assert located.exists() @pytest.mark.asyncio - async def test_an_explicit_path_overrides_a_local_uri(self, tmp_path): - """`--db` overrides the configured location for one invocation.""" + async def test_a_path_beside_the_configured_database_is_refused(self, tmp_path): config = self._config(tmp_path / "configured.lancedb") chosen = tmp_path / "chosen.lancedb" - async with HaikuRAG(chosen, config=config, create=True) as rag: - assert rag.store.db_path == chosen - assert chosen.exists() + with pytest.raises(AmbiguousDatabaseError, match="notes"): + async with HaikuRAG(chosen, config=config, create=True): + pass + assert not chosen.exists() assert not (tmp_path / "configured.lancedb").exists() @pytest.mark.asyncio - async def test_a_local_uri_that_does_not_exist_is_refused(self, tmp_path): - """A schemeless location is a local path and must exist.""" + async def test_a_local_location_that_does_not_exist_is_refused(self, tmp_path): + """A schemeless location is a local path and must exist. The error names + the configured database, never its location.""" config = self._config(tmp_path / "typo.lancedb") - with pytest.raises(FileNotFoundError): + with pytest.raises(SourceUnavailableError, match="notes") as caught: async with HaikuRAG(config=config): pass + assert "typo.lancedb" not in str(caught.value) assert not (tmp_path / "typo.lancedb").exists() def test_a_uri_with_a_scheme_stays_a_uri(self, tmp_path): @@ -167,15 +164,12 @@ class TestSessionsOwnTheRef: class TestLocate: def test_a_scheme_is_a_uri(self): - assert locate_database("s3://bucket/one.lancedb") == ( - "s3://bucket/one.lancedb", - None, - ) + assert locate_database("s3://bucket/one.lancedb") == "s3://bucket/one.lancedb" def test_anything_else_is_a_local_path(self): - uri, db_path = locate_database("/data/one.lancedb") - assert uri == "" - assert db_path is not None and str(db_path) == "/data/one.lancedb" + from pathlib import Path + + assert locate_database("/data/one.lancedb") == Path("/data/one.lancedb") class TestSelection: @@ -250,10 +244,39 @@ class TestPlacingADatabase: assert {r.source for r in results} == {"alpha"} @pytest.mark.asyncio - async def test_an_unnamed_database_names_nothing(self, temp_db_path): + async def test_a_database_at_a_path_is_named_by_its_stem(self, temp_db_path): async with HaikuRAG(temp_db_path, create=True) as rag: - assert rag.source_names == () - assert rag.source is None + assert rag.source_names == (temp_db_path.stem,) + assert rag.source == temp_db_path.stem + + @pytest.mark.asyncio + async def test_the_default_database_is_selectable_by_name(self, tmp_path): + """Nothing configured is the one entry `haiku.rag`, an ordinary + configured database that `sources` can name.""" + from haiku.rag.config.models import StorageConfig + + config = AppConfig(storage=StorageConfig(data_dir=tmp_path)) + + async with HaikuRAG(config=config, sources=["haiku.rag"], create=True) as rag: + assert rag.source == "haiku.rag" + assert rag.store.db_path == tmp_path / "haiku.rag.lancedb" + + def test_coverage_is_known_before_the_client_opens(self, tmp_path): + """Coverage is a fact of the resolved scope, readable before entering, + and `source_names` and `covers_multiple` agree on it.""" + config = _config(tmp_path, ["alpha", "beta"]) + + covering = HaikuRAG(config=config) + assert covering.source_names == ("alpha", "beta") + assert covering.covers_multiple + + narrowed = HaikuRAG(config=config, sources=["beta"]) + assert narrowed.source_names == ("beta",) + assert not narrowed.covers_multiple + + at_path = HaikuRAG(tmp_path / "other.lancedb") + assert at_path.source_names == ("other",) + assert not at_path.covers_multiple @pytest.mark.asyncio async def test_the_reader_for_a_database_is_the_client_holding_it(self, tmp_path): @@ -314,10 +337,10 @@ class TestPlacingADatabase: await alpha.reader_for("beta") @pytest.mark.asyncio - async def test_an_unnamed_database_refuses_any_name(self, temp_db_path): - """Nothing names it, so no name can be the one it covers.""" + async def test_a_database_at_a_path_answers_to_its_stem_alone(self, temp_db_path): async with HaikuRAG(temp_db_path, create=True) as rag: - with pytest.raises(UnknownDatabaseError, match="single unnamed database"): + assert await rag.reader_for(temp_db_path.stem) is rag + with pytest.raises(UnknownDatabaseError, match=temp_db_path.stem): await rag.reader_for("anything") @pytest.mark.asyncio diff --git a/tests/multi_db/test_search.py b/tests/multi_db/test_search.py index 00ace6a5..b3222a35 100644 --- a/tests/multi_db/test_search.py +++ b/tests/multi_db/test_search.py @@ -71,7 +71,7 @@ class TestFederatedSearch: class TestSingleDatabaseUnchanged: @pytest.mark.asyncio - async def test_source_is_unset_without_configured_databases(self, temp_db_path): + async def test_source_is_the_stem_without_configured_databases(self, temp_db_path): async with HaikuRAG(temp_db_path, create=True) as rag: doc = DoclingDocument(name="one") doc.add_text(label=DocItemLabel.TEXT, text="a document about cats") @@ -89,7 +89,7 @@ class TestSingleDatabaseUnchanged: results = await rag.search("cats", search_type="fts") assert results - assert all(r.source is None for r in results) + assert all(r.source == temp_db_path.stem for r in results) class TestOneQueryVector: diff --git a/tests/sandbox/test_sandbox.py b/tests/sandbox/test_sandbox.py index 5999f894..ed4a8a2d 100644 --- a/tests/sandbox/test_sandbox.py +++ b/tests/sandbox/test_sandbox.py @@ -110,8 +110,7 @@ class TestSandboxListDocuments: assert result.success assert "1" in result.stdout assert "Test Document" in result.stdout - # Nothing names this database, so there is no name to report. - assert "None" in result.stdout + assert temp_db_path.stem in result.stdout class TestSandboxSearch: diff --git a/tests/sandbox/test_sandbox_multi_db.py b/tests/sandbox/test_sandbox_multi_db.py index b5e233cf..98327483 100644 --- a/tests/sandbox/test_sandbox_multi_db.py +++ b/tests/sandbox/test_sandbox_multi_db.py @@ -159,12 +159,14 @@ class TestTheSandboxConstructors: @pytest.mark.asyncio async def test_the_public_constructor_resolves_the_path_it_is_given(self, tmp_path): + from haiku.rag.config.models import AppConfig + config = _config(tmp_path, ["alpha", "beta"]) await _seed(config, "alpha", ["alpha document about cats"]) sandbox = Sandbox( db_path=tmp_path / "alpha.lancedb", - config=config, + config=AppConfig(), context=AnalysisContext(), ) @@ -202,19 +204,20 @@ class TestTheSandboxCoversWhatTheCapabilityCovers: """The sandbox covers the scope the capability resolved, as handed over.""" from haiku.rag.capabilities.analysis import AnalysisState, create_capability + from haiku.rag.config.models import AppConfig config = _config(tmp_path, ["alpha", "beta"]) await _seed(config, "alpha", ["alpha document about cats"]) capability = create_capability( - db_path=tmp_path / "alpha.lancedb", config=config, defer_loading=False + db_path=tmp_path / "alpha.lancedb", config=AppConfig(), defer_loading=False ) capability.state = AnalysisState() sandbox = await capability._ensure_sandbox() try: assert sandbox._scope is capability.scope - assert capability.scope.names == () + assert capability.scope.names == ("alpha",) finally: await capability._close() diff --git a/tests/store/test_database_info.py b/tests/store/test_database_info.py index 3930ca08..6b251b14 100644 --- a/tests/store/test_database_info.py +++ b/tests/store/test_database_info.py @@ -126,7 +126,9 @@ async def test_gather_database_info_connects_to_the_location_it_is_given(): from haiku.rag.config.models import LanceDBConfig - config = AppConfig(lancedb=LanceDBConfig(uri="s3://elsewhere/other.lancedb")) + config = AppConfig( + lancedb=LanceDBConfig(databases={"other": "s3://elsewhere/other.lancedb"}) + ) with patch( "haiku.rag.store.info.connect_lancedb", new_callable=AsyncMock ) as mock_connect: diff --git a/tests/test_app.py b/tests/test_app.py index f0812ef5..ebe43f9b 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -456,8 +456,10 @@ async def test_create_index_rebuilds_an_existing_one(app, client): def test_show_settings_hides_secrets(tmp_path): - config = AppConfig(lancedb=LanceDBConfig(uri="db://x", api_key="secret-value")) - app = HaikuRAGApp(scope=for_path(tmp_path / "db", config), config=config) + config = AppConfig( + lancedb=LanceDBConfig(databases={"x": "db://x"}, api_key="secret-value") + ) + app = HaikuRAGApp(scope=DatabaseScope.resolve(config), config=config) app.console = Console(record=True, width=200) app.show_settings() @@ -476,7 +478,7 @@ def test_show_settings_renders_the_shape_a_config_file_has(tmp_path): lancedb=LanceDBConfig(databases={"alpha": "/tmp/a.lancedb"}), storage=StorageConfig(data_dir=tmp_path), ) - app = HaikuRAGApp(scope=for_path(tmp_path / "db", config), config=config) + app = HaikuRAGApp(scope=DatabaseScope.resolve(config), config=config) app.console = Console(record=True, width=200) app.show_settings() @@ -515,7 +517,7 @@ def test_show_settings_survives_a_narrow_console_and_bracketed_values(tmp_path): def test_remote_uri_is_the_display_path(tmp_path): - config = AppConfig(lancedb=LanceDBConfig(uri="s3://bucket/path")) + config = AppConfig(lancedb=LanceDBConfig(databases={"path": "s3://bucket/path"})) app = HaikuRAGApp(scope=for_path(None, config), config=config) assert app.display_path == "s3://bucket/path" diff --git a/tests/test_cli.py b/tests/test_cli.py index 202de516..7e52270b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -158,6 +158,17 @@ class TestOneDatabaseCommands: [ref] = resolve_scope(Path("/db/other.lancedb")).databases assert ref.db_path == Path("/db/other.lancedb") + def test_naming_a_path_overrides_a_configured_set(self, monkeypatch): + """`--db` is the operator's explicit override: it constructs the scope + directly, where a Python caller passing a path beside `databases` is + refused.""" + self._install(monkeypatch, alpha="/db/a.lancedb", beta="/db/b.lancedb") + + scope = resolve_scope(Path("/db/other.lancedb")) + + assert scope.names == ("other",) + assert not scope.covers_multiple + def test_no_configured_databases_is_allowed(self, monkeypatch, tmp_path): import haiku.rag.config as config_module @@ -207,7 +218,6 @@ class TestSelectingADatabaseByName: "notes": "/data/notes.lancedb", "other": "/data/o.lancedb", } - assert config.lancedb.uri == "" def test_an_unknown_name_names_the_configured_ones(self, monkeypatch): self._install(monkeypatch, alpha="/data/a.lancedb", beta="/data/b.lancedb") @@ -225,11 +235,15 @@ class TestSelectingADatabaseByName: assert "bucket" not in str(raised.value) - def test_selecting_nothing_reports_an_empty_mapping(self, monkeypatch): + def test_an_unknown_name_with_nothing_configured_names_the_default( + self, monkeypatch + ): + """Nothing configured is the one entry `haiku.rag`, which the message + offers.""" self._install(monkeypatch) monkeypatch.setattr("haiku.rag.cli._db_name", "papers") - with pytest.raises(UnknownDatabaseError, match="nothing"): + with pytest.raises(UnknownDatabaseError, match="haiku.rag"): resolve_scope(None) def test_the_callback_selects_before_a_command_runs(self, tmp_path, monkeypatch): @@ -288,7 +302,6 @@ class TestSelectingADatabaseByName: cli, ["--config", str(config_file), "--db-name", "alpha", "settings"] ) # Naming one leaves the configuration naming both. - assert get_config().lancedb.uri == "" assert set(get_config().lancedb.databases) == {"alpha", "beta"} runner.invoke(cli, ["--config", str(config_file), "settings"]) @@ -384,12 +397,12 @@ class TestResolvingTheDatabasePath: resolve_scope(Path("/data/other.lancedb")) -class TestConfiguredLocalUri: - """`lancedb.uri` with a local path.""" +class TestConfiguredLocalDatabase: + """One entry in `lancedb.databases` with a local path.""" def _config_file(self, tmp_path, located: Path) -> Path: config_file = tmp_path / "haiku.rag.yaml" - config_file.write_text(f"lancedb:\n uri: {located}\n") + config_file.write_text(f"lancedb:\n databases:\n notes: {located}\n") return config_file def _fresh(self, monkeypatch) -> None: @@ -429,7 +442,7 @@ class TestConfiguredLocalUri: assert "does not exist" in result.output assert not located.exists() - def test_db_overrides_the_configured_uri(self, tmp_path, monkeypatch): + def test_db_overrides_the_configured_database(self, tmp_path, monkeypatch): self._fresh(monkeypatch) configured = tmp_path / "configured.lancedb" chosen = tmp_path / "chosen.lancedb" diff --git a/tests/test_config.py b/tests/test_config.py index 263fd099..a5665499 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -800,3 +800,19 @@ def test_complete_example_matches_the_defaults(): } assert not drifted, f"documented value != default: {drifted}" + + +def test_lancedb_uri_is_refused_with_the_replacement_named(): + """`lancedb.uri` no longer places a database; a config carrying it fails to + load with the `databases` spelling to use instead.""" + from haiku.rag.config.models import LanceDBConfig + + with pytest.raises(ValidationError) as raised: + AppConfig.model_validate({"lancedb": {"uri": "s3://bucket/notes.lancedb"}}) + + message = str(raised.value) + assert "lancedb.uri" in message + assert "lancedb.databases" in message + + with pytest.raises(ValidationError, match="lancedb.databases"): + LanceDBConfig.model_validate({"uri": "/data/notes.lancedb"}) diff --git a/tests/test_database_autocreate.py b/tests/test_database_autocreate.py index d34c9033..67102306 100644 --- a/tests/test_database_autocreate.py +++ b/tests/test_database_autocreate.py @@ -55,7 +55,8 @@ def test_default_db_path_comes_from_storage_data_dir(tmp_path): [ref] = HaikuRAG(config=config)._resolve_scope().databases - assert ref.db_path == tmp_path / "haiku.rag.lancedb" + assert ref.location == tmp_path / "haiku.rag.lancedb" + assert ref.name == "haiku.rag" @pytest.mark.asyncio diff --git a/tests/test_database_scope.py b/tests/test_database_scope.py index b13b075f..942247d1 100644 --- a/tests/test_database_scope.py +++ b/tests/test_database_scope.py @@ -2,7 +2,7 @@ from pathlib import Path import pytest -from haiku.rag.client.scope import DatabaseRef, DatabaseScope +from haiku.rag.client.scope import DatabaseRef, DatabaseScope, database_name from haiku.rag.config.models import AppConfig, LanceDBConfig, StorageConfig from haiku.rag.store.exceptions import ( AmbiguousDatabaseError, @@ -25,23 +25,32 @@ class TestResolution: config, database_name="alpha", database_path=Path("/data/other.lancedb") ) - def test_a_path_names_one_unnamed_database(self): - """A path says which database, not what it is called, even where the - configuration names one.""" - config = _config(databases={"alpha": "/data/alpha.lancedb"}) - - scope = DatabaseScope.resolve(config, database_path=Path("/data/other.lancedb")) + def test_a_path_places_the_database_where_the_configuration_places_none(self): + scope = DatabaseScope.resolve(_config(), database_path="/data/other.lancedb") assert scope.databases == (DatabaseRef.at("/data/other.lancedb"),) - assert scope.names == () + assert scope.names == ("other",) assert not scope.covers_multiple + def test_a_path_beside_a_configured_placement_is_refused(self): + """The configuration places databases; a path beside it is a second + placement, and the refusal names both.""" + config = _config(databases={"alpha": "/data/alpha.lancedb", "beta": "b://b"}) + + with pytest.raises(AmbiguousDatabaseError) as raised: + DatabaseScope.resolve(config, database_path=Path("/data/other.lancedb")) + + message = str(raised.value) + assert "/data/other.lancedb" in message + assert "alpha" in message and "beta" in message + assert "lancedb.databases" in message + def test_a_named_database_keeps_its_name(self): config = _config(databases={"alpha": "/data/alpha.lancedb", "beta": "b://b"}) scope = DatabaseScope.resolve(config, database_name="beta") - assert scope.databases == (DatabaseRef("beta", "b://b", None),) + assert scope.databases == (DatabaseRef("beta", "b://b"),) assert scope.names == ("beta",) def test_an_unknown_name_is_refused(self): @@ -66,42 +75,38 @@ class TestResolution: scope = DatabaseScope.resolve(config) - assert scope.databases == ( - DatabaseRef.configured("alpha", "/data/alpha.lancedb"), - ) + assert scope.databases == (DatabaseRef("alpha", Path("/data/alpha.lancedb")),) assert not scope.covers_multiple - def test_a_bare_uri_is_one_unnamed_database(self): - scope = DatabaseScope.resolve(_config(uri="s3://bucket/one.lancedb")) - - assert scope.databases == (DatabaseRef(None, "s3://bucket/one.lancedb", None),) - - def test_a_bare_uri_without_a_scheme_is_a_local_path(self): - """`lancedb.uri` places one database the same way an entry in - `lancedb.databases` does, so a schemeless value is a path and gets the - existence check a local database gets.""" - scope = DatabaseScope.resolve(_config(uri="/data/notes.lancedb")) - - [ref] = scope.databases - assert ref.name is None - assert ref.db_path == Path("/data/notes.lancedb") - assert ref.uri == "" - - def test_a_path_selects_the_database_over_a_configured_uri(self): - """`--db` exists to override what is configured.""" - config = _config(uri="s3://bucket/one.lancedb") - - scope = DatabaseScope.resolve(config, database_path=Path("/data/local")) - - [ref] = scope.databases - assert ref.location == Path("/data/local") - - def test_nothing_configured_falls_back_to_the_data_directory(self, tmp_path): + def test_nothing_configured_is_the_default_entry(self, tmp_path): + """No `databases` reads as one entry, `haiku.rag`, under the data + directory: an ordinary configured database in every respect.""" config = AppConfig(storage=StorageConfig(data_dir=tmp_path)) scope = DatabaseScope.resolve(config) - assert scope.databases == (DatabaseRef.at(tmp_path / "haiku.rag.lancedb"),) + assert scope.databases == ( + DatabaseRef("haiku.rag", tmp_path / "haiku.rag.lancedb"), + ) + assert scope.names == ("haiku.rag",) + + def test_the_default_entry_is_selectable_by_name(self, tmp_path): + config = AppConfig(storage=StorageConfig(data_dir=tmp_path)) + + by_name = DatabaseScope.resolve(config, database_name="haiku.rag") + selected = DatabaseScope.resolve(config).select(["haiku.rag"]) + + assert by_name == selected == DatabaseScope.resolve(config) + + def test_the_default_entry_does_not_hide_a_configured_set(self, tmp_path): + """The default stands in only where nothing is configured.""" + config = AppConfig( + storage=StorageConfig(data_dir=tmp_path), + lancedb=LanceDBConfig(databases={"alpha": "/data/alpha.lancedb"}), + ) + + with pytest.raises(UnknownDatabaseError, match="haiku.rag"): + DatabaseScope.resolve(config, database_name="haiku.rag") def test_the_environment_is_not_consulted(self, monkeypatch, tmp_path): """HAIKU_RAG_DB is honoured by the capability entry point alone; @@ -120,8 +125,7 @@ class TestResolution: ) [ref] = scope.databases - assert ref.db_path == Path("s3://bucket/looks-like-a-uri.lancedb") - assert ref.uri == "" + assert ref.location == Path("s3://bucket/looks-like-a-uri.lancedb") def test_a_configured_location_with_a_scheme_is_a_uri(self): """A configured value is a URI or a path depending on its scheme, which is @@ -130,22 +134,16 @@ class TestResolution: [ref] = DatabaseScope.resolve(config).databases - assert ref.uri == "s3://bucket/alpha.lancedb" + assert ref.location == "s3://bucket/alpha.lancedb" assert ref.db_path is None - def test_a_database_is_a_uri_or_a_path(self): - """A ref holding both, or neither, is refused at construction. + def test_a_configured_location_without_a_scheme_is_a_path(self): + config = _config(databases={"alpha": "/data/alpha.lancedb"}) - The message names what it was given: this is a programming error raised - in the caller's own process, not one an operator or a model ever sees. - """ - with pytest.raises(ValueError, match="either a URI or a local path") as both: - DatabaseRef(None, "s3://bucket/a.lancedb", Path("/data/a.lancedb")) - assert "s3://bucket/a.lancedb" in str(both.value) + [ref] = DatabaseScope.resolve(config).databases - with pytest.raises(ValueError, match="either a URI or a local path") as neither: - DatabaseRef(None, "", None) - assert "db_path=None" in str(neither.value) + assert ref.location == Path("/data/alpha.lancedb") + assert ref.db_path == Path("/data/alpha.lancedb") def test_a_scope_covers_at_least_one_database(self): """Every resolution reaches a database, and the sessions built from a @@ -154,23 +152,69 @@ class TestResolution: DatabaseScope(()) -class TestLocation: - """One value says where a database is: a path for a local one, a URI string - for a remote one. Storage connects to it as given.""" +class TestTheReference: + """Constructed directly, a reference still holds what it advertises.""" - def test_a_local_location_is_a_path(self): - config = _config(databases={"alpha": "/data/alpha.lancedb"}) - [ref] = DatabaseScope.resolve(config).databases + def test_a_schemeless_string_location_is_a_path(self): + ref = DatabaseRef("x", "local.lancedb") - assert ref.location == Path("/data/alpha.lancedb") + assert ref.location == Path("local.lancedb") + assert ref.db_path == Path("local.lancedb") - def test_a_uri_location_is_the_uri(self): - config = _config(databases={"alpha": "s3://bucket/alpha.lancedb"}) - [ref] = DatabaseScope.resolve(config).databases + def test_a_location_with_a_scheme_stays_a_uri(self): + ref = DatabaseRef("x", "s3://bucket/x.lancedb") - assert ref.location == "s3://bucket/alpha.lancedb" + assert ref.location == "s3://bucket/x.lancedb" + assert ref.db_path is None - def test_a_path_the_caller_gave_is_its_location(self): - assert DatabaseRef.at("/data/other.lancedb").location == Path( - "/data/other.lancedb" + def test_a_blank_name_is_refused(self): + with pytest.raises(ValueError, match="no name"): + DatabaseRef("", "/data/x.lancedb") + with pytest.raises(ValueError, match="no name"): + DatabaseRef(" ", "s3://bucket/x.lancedb") + + def test_a_blank_location_is_refused(self): + """A blank string would resolve to the working directory.""" + with pytest.raises(ValueError, match="no location"): + DatabaseRef("x", "") + with pytest.raises(ValueError, match="no location"): + DatabaseRef("x", " ") + + def test_a_given_database_is_a_local_path(self): + """Only a path can be given: a given database's errors name its + location, and a URI must never travel that way.""" + assert DatabaseRef("x", "local.lancedb", given=True).location == Path( + "local.lancedb" ) + with pytest.raises(ValueError, match="is a URI"): + DatabaseRef("x", "s3://bucket/x.lancedb", given=True) + + +class TestNamingAPath: + """A path the caller gave is named by its stem, the one rule for the + default database and for `--db`.""" + + def test_the_stem_names_the_database(self): + assert database_name(Path("/data/foo.lancedb")) == "foo" + assert database_name(Path("relative.lancedb")) == "relative" + assert database_name(Path("/data/haiku.rag.lancedb")) == "haiku.rag" + + def test_a_path_with_no_stem_is_refused(self): + with pytest.raises(ValueError, match="no name"): + database_name(Path("/")) + + def test_at_names_the_path_it_is_given(self): + """A given path is marked as such: its errors may name it, since the + caller already knows where it is.""" + assert DatabaseRef.at("/data/other.lancedb") == DatabaseRef( + "other", Path("/data/other.lancedb"), given=True + ) + assert not DatabaseRef.configured("other", "/data/other.lancedb").given + + def test_a_scope_at_a_path_ignores_the_configuration(self): + """The CLI's `--db` is a human's explicit override: it constructs the + scope directly and consults no configuration.""" + scope = DatabaseScope.at(Path("/data/other.lancedb")) + + assert scope.databases == (DatabaseRef.at("/data/other.lancedb"),) + assert scope.names == ("other",) diff --git a/tests/test_info.py b/tests/test_info.py index 12273529..6f06d2be 100644 --- a/tests/test_info.py +++ b/tests/test_info.py @@ -212,7 +212,7 @@ async def test_app_info_uses_connect_lancedb_for_remote(tmp_path): """info() should use connect_lancedb() instead of direct lancedb.connect() for remote URIs.""" config = AppConfig( lancedb=LanceDBConfig( - uri="s3://bucket/path", + databases={"path": "s3://bucket/path"}, storage_options={"endpoint": "http://localhost:9000"}, ) ) @@ -368,7 +368,7 @@ async def test_app_init_skips_exists_check_for_remote(tmp_path): """init() should not check db_path.exists() for remote URIs.""" config = AppConfig( lancedb=LanceDBConfig( - uri="s3://bucket/path", + databases={"path": "s3://bucket/path"}, storage_options={"endpoint": "http://localhost:9000"}, ) ) @@ -389,7 +389,7 @@ async def test_app_history_skips_exists_check_for_remote(tmp_path): """history() should not check db_path.exists() for remote URIs.""" config = AppConfig( lancedb=LanceDBConfig( - uri="s3://bucket/path", + databases={"path": "s3://bucket/path"}, storage_options={"endpoint": "http://localhost:9000"}, ) ) @@ -416,7 +416,7 @@ async def test_app_tag_rendering_escapes_markup(tmp_path): config = AppConfig( lancedb=LanceDBConfig( - uri="s3://bucket/path", + databases={"path": "s3://bucket/path"}, storage_options={"endpoint": "http://localhost:9000"}, ) ) @@ -450,7 +450,7 @@ async def test_app_history_survives_tag_annotation_failure(tmp_path): config = AppConfig( lancedb=LanceDBConfig( - uri="s3://bucket/path", + databases={"path": "s3://bucket/path"}, storage_options={"endpoint": "http://localhost:9000"}, ) ) diff --git a/tests/test_lancedb_connection.py b/tests/test_lancedb_connection.py index f3997b93..6238fbab 100644 --- a/tests/test_lancedb_connection.py +++ b/tests/test_lancedb_connection.py @@ -62,7 +62,9 @@ class TestConnectLancedb: async def test_the_configured_uri_is_not_consulted(self, temp_db_path): """Storage connects to the location it is handed; placement is the caller's, and the configuration's own `uri` never redirects it.""" - config = AppConfig(lancedb=LanceDBConfig(uri="s3://elsewhere/db.lancedb")) + config = AppConfig( + lancedb=LanceDBConfig(databases={"elsewhere": "s3://elsewhere/db.lancedb"}) + ) with patch( "haiku.rag.store.engine.lancedb.connect_async", new_callable=AsyncMock ) as mock_connect: @@ -140,7 +142,9 @@ class TestStoreConnectionMode: @pytest.mark.asyncio async def test_a_local_store_ignores_the_configured_uri(self, temp_db_path): - config = AppConfig(lancedb=LanceDBConfig(uri="s3://elsewhere/db.lancedb")) + config = AppConfig( + lancedb=LanceDBConfig(databases={"elsewhere": "s3://elsewhere/db.lancedb"}) + ) async with Store(temp_db_path, config=config, create=True) as store: assert store._connection_mode == ConnectionMode.LOCAL assert store.db_path == temp_db_path diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 16177122..7b465b45 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -665,6 +665,18 @@ class TestMCPClientLifetime: with pytest.raises(AmbiguousDatabaseError, match="alpha, beta"): create_mcp_server(config=config) + def test_the_public_factory_refuses_a_path_beside_a_configured_set(self, tmp_path): + """A path and `lancedb.databases` both place the database.""" + from haiku.rag.config.models import AppConfig, LanceDBConfig + from haiku.rag.store.exceptions import AmbiguousDatabaseError + + config = AppConfig( + lancedb=LanceDBConfig(databases={"alpha": str(tmp_path / "a")}) + ) + + with pytest.raises(AmbiguousDatabaseError, match="alpha"): + create_mcp_server(tmp_path / "other.lancedb", config=config) + @pytest.mark.asyncio async def test_the_command_hands_the_server_its_resolved_database( self, monkeypatch @@ -697,7 +709,7 @@ class TestMCPClientLifetime: [ref] = seen["scope"].databases assert ref.name == "prod" - assert ref.uri == "s3://bucket/prod.lancedb" + assert ref.location == "s3://bucket/prod.lancedb" # The caller's configuration, not one derived from the ref. assert seen["config"].lancedb.databases == {"prod": "s3://bucket/prod.lancedb"} diff --git a/tests/test_s3_integration.py b/tests/test_s3_integration.py index 5d814425..81443e2f 100644 --- a/tests/test_s3_integration.py +++ b/tests/test_s3_integration.py @@ -41,12 +41,18 @@ def _make_config() -> AppConfig: unique_prefix = uuid4().hex[:8] return AppConfig( lancedb=LanceDBConfig( - uri=f"s3://{S3_BUCKET}/test-{unique_prefix}", + databases={"test": f"s3://{S3_BUCKET}/test-{unique_prefix}"}, storage_options=S3_STORAGE_OPTIONS, ) ) +def _uri(config: AppConfig) -> str: + """The one configured S3 location.""" + [uri] = config.lancedb.databases.values() + return uri + + @pytest.fixture def config(): """A config pointing at a unique S3 prefix, cleaned up after the test. @@ -58,7 +64,7 @@ def config(): config = _make_config() yield config - bucket, _, prefix = config.lancedb.uri.removeprefix("s3://").partition("/") + bucket, _, prefix = _uri(config).removeprefix("s3://").partition("/") store = make_s3_store(bucket, S3_STORAGE_OPTIONS) paths = [obj["path"] for batch in store.list(prefix=f"{prefix}/") for obj in batch] if paths: @@ -73,7 +79,7 @@ def _remote_scope(config: AppConfig) -> DatabaseScope: """ scope = DatabaseScope.resolve(config) [ref] = scope.databases - assert ref.db_path is None and ref.uri.startswith("s3://") + assert ref.db_path is None and str(ref.location).startswith("s3://") return scope @@ -89,7 +95,7 @@ async def _remote_client(config: AppConfig): async def test_store_connect_and_create(tmp_path, config): from haiku.rag.store.info import get_database_stats - async with Store(config.lancedb.uri, config=config, create=True) as store: + async with Store(_uri(config), config=config, create=True) as store: stats = await get_database_stats(store.db) assert stats["documents"]["exists"] assert stats["chunks"]["exists"] @@ -97,7 +103,7 @@ async def test_store_connect_and_create(tmp_path, config): @pytest.mark.asyncio async def test_store_vacuum(tmp_path, config): - async with Store(config.lancedb.uri, config=config, create=True) as store: + async with Store(_uri(config), config=config, create=True) as store: await store.vacuum() @@ -106,7 +112,7 @@ async def test_store_add_document(tmp_path, config): from haiku.rag.store.info import get_database_stats from haiku.rag.store.schema import DocumentRecord - async with Store(config.lancedb.uri, config=config, create=True) as store: + async with Store(_uri(config), config=config, create=True) as store: doc = DocumentRecord(content="The quick brown fox jumps over the lazy dog.") await store.documents_table.add([doc]) @@ -164,7 +170,7 @@ async def test_app_info(capsys, config): out = capsys.readouterr().out assert "path:" in out - assert config.lancedb.uri in out + assert _uri(config) in out assert "documents: 1" in out