From 11644c7f4393d4531d7b332f12e1da583c253aac Mon Sep 17 00:00:00 2001 From: Yiorgis Gozadinos Date: Tue, 18 Aug 2026 12:38:02 +0300 Subject: [PATCH] Trim comments, docstrings and docs to what they need to say Also drops two things that were stale rather than merely verbose: the CLI docs note for 0.75.0, which was the only release-tagged note in the docs tree while the CHANGELOG already records that existing databases need `haiku-rag migrate`; and "(created or corrected)" from the migration log line, left over from the earlier behaviour that replaced wrong-typed indexes. --- docs/cli.md | 3 -- haiku_rag_slim/haiku/rag/store/engine.py | 36 ++++--------------- .../haiku/rag/store/upgrades/v0_75_0.py | 21 ++--------- tests/store/test_indexes.py | 28 ++++----------- tests/store/test_v0_75_0_migration.py | 14 +++----- tests/test_client.py | 3 +- 6 files changed, 21 insertions(+), 84 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 9a3df4a4..06c2b726 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -359,9 +359,6 @@ Migration completed successfully. !!! tip Back up your database before running migrations. While migrations are designed to be safe, having a backup provides peace of mind for production databases. -!!! note "Upgrading to 0.75.0" - The 0.75.0 migration adds scalar indexes: BTree on `documents.id`, `chunks.id` and `chunks.document_id`, and Bitmap on `document_items.label`. It rewrites no table data, so it is far cheaper than the earlier data migrations, but building an index reads the whole column it indexes. On a large database, and particularly on object storage, budget for reading `chunks.id` and `chunks.document_id` in full. Indexes already present with the expected type are left alone, so a database that already carries the full set migrates instantly. - ### Download Models Download required runtime models: diff --git a/haiku_rag_slim/haiku/rag/store/engine.py b/haiku_rag_slim/haiku/rag/store/engine.py index 5464fc57..002283a1 100644 --- a/haiku_rag_slim/haiku/rag/store/engine.py +++ b/haiku_rag_slim/haiku/rag/store/engine.py @@ -177,16 +177,7 @@ def get_document_items_arrow_schema() -> pa.Schema: def index_specs(table_name: str) -> list[tuple[str, Bitmap | BTree | FTS]]: - """The index set a haiku.rag table is expected to carry. - - Single source of truth for every path that creates a table: initialization, - migration, and the drop-and-recreate paths in the repositories. - - `label` gets a Bitmap rather than a BTree because it holds around ten - distinct values across every item of every document, and Bitmap is the - low-cardinality equality case. The FTS options are load-bearing: - `with_position` enables phrase queries and keeping stop words lets them match. - """ + """The index set each table carries.""" match table_name: case "documents": return [("id", BTree())] @@ -194,6 +185,7 @@ def index_specs(table_name: str) -> list[tuple[str, Bitmap | BTree | FTS]]: return [("id", BTree()), ("uri", BTree())] case "chunks": return [ + # Positions and stop words are required for phrase queries. ("content_fts", FTS(with_position=True, remove_stop_words=False)), ("id", BTree()), ("document_id", BTree()), @@ -210,27 +202,11 @@ def index_specs(table_name: str) -> list[tuple[str, Bitmap | BTree | FTS]]: async def ensure_indexes(table: lancedb.AsyncTable, table_name: str) -> list[str]: - """Ensure an index of the declared type covers each declared column. + """Create any declared index missing from a column. Returns the columns indexed. - Returns the columns it indexed, so callers can report what changed. - - The condition is the presence of the declared *type*, not that the column's - index happens to be that type. Checking coverage alone would let a wrong-typed - index satisfy the check: a BTree on `label` covers the column while losing the - low-cardinality equality lookup a Bitmap gives. - - Nothing is ever dropped or converted away from. Two index types over one - column can be deliberate, since they serve different query shapes (BTree for - equality and range, `Fm` for `contains`), so an index this function did not - declare is left in place even on a column it does. Undeclared columns are - untouched entirely, so a vector index on `chunks` survives. The one thing it - will overwrite is an index at LanceDB's default name for a declared column, - `{column}_idx`, which is the name this function itself creates. - - Skipping matters as much as creating: `create_index(replace=True)` rebuilds an - identical index, writing a fresh index and a new table version rather than - no-oping, and leaves the previous index behind until the next vacuum. On a - large table over object storage that is a full column sort per pass. + Matches on index type, not column coverage, so a BTree does not satisfy a + declared Bitmap. Never drops or converts an index it did not declare. + Re-creating is not free: `create_index(replace=True)` rebuilds. """ covering: dict[str, set[str]] = {} for index in await table.list_indices(): diff --git a/haiku_rag_slim/haiku/rag/store/upgrades/v0_75_0.py b/haiku_rag_slim/haiku/rag/store/upgrades/v0_75_0.py index 8aa2f4ab..f71b7001 100644 --- a/haiku_rag_slim/haiku/rag/store/upgrades/v0_75_0.py +++ b/haiku_rag_slim/haiku/rag/store/upgrades/v0_75_0.py @@ -7,29 +7,14 @@ logger = logging.getLogger(__name__) async def _apply_index_hot_lookup_keys(store: Store) -> None: - """Bring an existing database up to the declared index set. + """Add the declared indexes to a database created before 0.75.0. - Adds what earlier versions never created: BTree on `documents.id`, - `chunks.id` and `chunks.document_id`, and a Bitmap on `document_items.label`. - Without them those lookups scan the column, which on object storage is - network I/O on paths that run per document. - - Does not materialize rows in Python or rewrite table data. The cost is the - index builds themselves, which read the indexed columns to sort them. - - `ensure_indexes` skips a column already indexed with the declared type, so a - database that already has the full set (a merged one, say) comes through - untouched rather than re-sorting every indexed column. Columns it does not - declare are left alone, so a vector index or an externally added index - survives. + Rewrites no rows. Each index build reads the column it indexes. """ for table_name, table in store._tables().items(): applied = await ensure_indexes(table, table_name) if applied: - logger.info( - f"Indexed {table_name} (created or corrected): " - f"{', '.join(sorted(applied))}" - ) + logger.info(f"Indexed {table_name}: {', '.join(sorted(applied))}") upgrade_index_hot_lookup_keys = Upgrade( diff --git a/tests/store/test_indexes.py b/tests/store/test_indexes.py index 5bbd3a7e..3fc2c542 100644 --- a/tests/store/test_indexes.py +++ b/tests/store/test_indexes.py @@ -36,7 +36,7 @@ async def _covering(table, column: str) -> list[tuple[str, str]]: @pytest.mark.asyncio async def test_fresh_database_indexes_every_hot_lookup_key(temp_db_path): - """A new database carries the full index set, not a subset.""" + """A new database carries the full index set.""" async with Store(temp_db_path, create=True) as store: for name, table in store._tables().items(): expected = EXPECTED_INDEXED_COLUMNS.get(name, set()) @@ -45,8 +45,7 @@ async def test_fresh_database_indexes_every_hot_lookup_key(temp_db_path): @pytest.mark.asyncio async def test_ensure_indexes_skips_existing_instead_of_rebuilding(temp_db_path): - """`create_index(replace=True)` rebuilds an identical index and writes a new - table version, so a second pass must skip rather than replace.""" + """A second pass must not rebuild: replace=True writes a new version.""" async with Store(temp_db_path, create=True) as store: table = store.chunks_table version_before = await table.version() @@ -59,10 +58,7 @@ async def test_ensure_indexes_skips_existing_instead_of_rebuilding(temp_db_path) @pytest.mark.asyncio async def test_ensure_indexes_corrects_an_index_of_the_wrong_type(temp_db_path): - """A column indexed with the wrong type must be re-indexed. `label` is the - live case: a BTree over ~ten distinct values loses the low-cardinality - equality lookup a Bitmap gives, and column coverage alone cannot see it. - """ + """A wrong-typed index does not satisfy the declared one.""" async with Store(temp_db_path, create=True) as store: table = store.document_items_table await table.create_index("label", config=BTree(), replace=True) @@ -80,10 +76,7 @@ async def test_ensure_indexes_corrects_an_index_of_the_wrong_type(temp_db_path): async def test_ensure_indexes_adds_the_declared_type_beside_a_custom_index( temp_db_path, ): - """A wrong-typed index under a custom name must neither satisfy the check nor - be destroyed. `replace=True` replaces by name and the default name is derived - from the column, so a custom-named index is invisible to it either way. - """ + """A custom-named index neither satisfies the check nor is destroyed.""" async with Store(temp_db_path, create=True) as store: table = store.document_items_table await table.drop_index("label_idx") @@ -100,10 +93,7 @@ async def test_ensure_indexes_adds_the_declared_type_beside_a_custom_index( async def test_ensure_indexes_keeps_an_operator_index_on_a_declared_column( temp_db_path, ): - """Two index types over one column can be deliberate, serving different query - shapes. An index this function did not declare is left alone even on a column - it does, so a migration never deletes an operator's index. - """ + """An index we did not declare survives, even on a declared column.""" async with Store(temp_db_path, create=True) as store: table = store.document_items_table await table.create_index("label", config=BTree(), name="operator_label") @@ -116,8 +106,7 @@ async def test_ensure_indexes_keeps_an_operator_index_on_a_declared_column( @pytest.mark.asyncio async def test_delete_all_restores_the_full_index_set(temp_db_path): - """delete_all drops and recreates tables; the recreated tables must come - back with the same indexes a fresh database gets.""" + """Recreated tables come back with the full index set.""" async with Store(temp_db_path, create=True) as store: repo = DocumentRepository(store) await repo.create(Document(content="A document")) @@ -131,10 +120,7 @@ async def test_delete_all_restores_the_full_index_set(temp_db_path): @pytest.mark.asyncio async def test_delete_all_keeps_picture_data_as_large_binary(temp_db_path): - """document_items must be recreated from the Arrow schema, which declares - picture_data as large_binary. The 32-bit `binary` type overflows its offsets - once a fragment holds enough embedded pictures. - """ + """picture_data must survive delete_all as large_binary, not binary.""" async with Store(temp_db_path, create=True) as store: repo = DocumentRepository(store) await repo.create(Document(content="A document")) diff --git a/tests/store/test_v0_75_0_migration.py b/tests/store/test_v0_75_0_migration.py index 4c7e10e4..3b996d51 100644 --- a/tests/store/test_v0_75_0_migration.py +++ b/tests/store/test_v0_75_0_migration.py @@ -6,8 +6,7 @@ from haiku.rag.store.models import Document from haiku.rag.store.repositories.document import DocumentRepository from haiku.rag.store.upgrades.v0_75_0 import _apply_index_hot_lookup_keys -# What a pre-0.75.0 database carried: no index on `documents` at all, FTS only on -# `chunks`, and `document_items` without the `label` Bitmap. +# The indexes a pre-0.75.0 database lacked. LEGACY_DROPPED = { "documents": ["id_idx"], "chunks": ["id_idx", "document_id_idx"], @@ -67,10 +66,7 @@ async def test_keeps_every_row(temp_db_path): @pytest.mark.asyncio async def test_is_a_no_op_on_an_already_indexed_database(temp_db_path): - """An already-migrated database must not be re-indexed. `create_index` with - replace=True rebuilds, so a second run would re-sort every indexed column - and write a new version of every table. - """ + """A second run must not re-index: replace=True rebuilds.""" async with Store(temp_db_path, create=True, skip_migration_check=True) as store: await _apply_index_hot_lookup_keys(store) versions = {name: await t.version() for name, t in store._tables().items()} @@ -84,8 +80,7 @@ async def test_is_a_no_op_on_an_already_indexed_database(temp_db_path): @pytest.mark.asyncio async def test_replaces_a_wrong_typed_legacy_index(temp_db_path): - """A database that indexed `label` with a BTree gets the Bitmap it should - have, rather than being left alone because the column was covered.""" + """A BTree on `label` does not satisfy the declared Bitmap.""" async with Store(temp_db_path, create=True, skip_migration_check=True) as store: await store.document_items_table.create_index( "label", config=BTree(), replace=True @@ -99,8 +94,7 @@ async def test_replaces_a_wrong_typed_legacy_index(temp_db_path): @pytest.mark.asyncio async def test_leaves_undeclared_indexes_alone(temp_db_path): - """A merged or externally built database may carry indexes haiku.rag never - creates. The migration must not drop them.""" + """Indexes haiku.rag never declared are not dropped.""" async with Store(temp_db_path, create=True, skip_migration_check=True) as store: await store.document_meta_table.create_index( "title", config=BTree(), replace=True diff --git a/tests/test_client.py b/tests/test_client.py index 94f240a5..e6e379a2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -995,8 +995,7 @@ async def test_client_update_document_replaces_rows_with_bounded_versions( ): """Updating one document should replace stale rows with bounded versions. - auto_vacuum is off: a background vacuum optimizes every table, which writes - versions of its own and would land inside the window being measured. + auto_vacuum is off: its writes would land inside the measured window. """ dim = Config.embeddings.model.vector_dim config = Config.model_copy(deep=True)