From 631c5218270b2cdf0cfe37bbbadb019abde9f89c Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Wed, 29 Jul 2026 13:51:32 -0700 Subject: [PATCH] Fix: address code review feedback on table gateways - Document sqlite3.Row row_factory precondition in module docstring - Strengthen test_create_is_idempotent to verify populated table survives - Collapse three roundtrip tests into one @pytest.mark.parametrize Co-Authored-By: Claude Sonnet 5 --- src/paperless_ai/tables.py | 8 ++++ src/paperless_ai/tests/test_tables.py | 64 +++++++++++---------------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/paperless_ai/tables.py b/src/paperless_ai/tables.py index 1a25e1e0b..bc8e962a0 100644 --- a/src/paperless_ai/tables.py +++ b/src/paperless_ai/tables.py @@ -3,6 +3,14 @@ vec0 table. Each method takes the sqlite3.Connection to operate on explicitly, rather than owning one -- the store swaps connections during compact()/migration, and migrations always work across two connections (src_conn, dst_conn) at once. + +PRECONDITION: Callers must set conn.row_factory = sqlite3.Row before passing a +connection to any of these gateways' read methods. The read methods across all +three classes (DocumentChunksTable.chunk_ids_for_document, IndexMetaTable._get, +DocumentMetaTable.all_modified_times, DocumentMetaTable.copy_all) use +row["column_name"] dictionary-style indexing, which requires sqlite3.Row as the +row factory -- without it, sqlite3.Row is not set, rows are returned as plain +tuples, and tuple indices must be integers, raising TypeError. """ import sqlite3 diff --git a/src/paperless_ai/tests/test_tables.py b/src/paperless_ai/tests/test_tables.py index 7ac99a185..51cf79b93 100644 --- a/src/paperless_ai/tests/test_tables.py +++ b/src/paperless_ai/tests/test_tables.py @@ -26,13 +26,14 @@ class TestDocumentChunksTable: GIVEN: - A bare sqlite3 connection WHEN: - - create() is called twice + - create() is called, a row is inserted, then create() is called again THEN: - - No error is raised and the table exists + - No error is raised and the row survives uncorrupted """ DocumentChunksTable.create(conn) + DocumentChunksTable.insert_many(conn, [ChunkRow("c1", 1)]) DocumentChunksTable.create(conn) - assert DocumentChunksTable.count(conn) == 0 + assert DocumentChunksTable.chunk_ids_for_document(conn, 1) == ["c1"] def test_insert_many_then_lookup_by_document_id( self, @@ -228,48 +229,35 @@ class TestDocumentMetaTable: class TestIndexMetaTable: - def test_dim_roundtrip(self, conn: sqlite3.Connection) -> None: + @pytest.mark.parametrize( + ("setter_name", "getter_name", "value"), + [ + ("set_dim", "get_dim", 384), + ("set_embed_model", "get_embed_model", "model-a"), + ("set_schema_version", "get_schema_version", 2), + ], + ) + def test_typed_accessor_roundtrip( + self, + conn: sqlite3.Connection, + setter_name: str, + getter_name: str, + value: int | str, + ) -> None: """ GIVEN: - An empty index_meta table WHEN: - - set_dim() is called then get_dim() is read back + - A typed accessor's setter is called then the getter is read back THEN: - - The same int value is returned + - The same value is returned, correctly typed (int or str) """ IndexMetaTable.create(conn) - assert IndexMetaTable.get_dim(conn) is None - IndexMetaTable.set_dim(conn, 384) - assert IndexMetaTable.get_dim(conn) == 384 - - def test_embed_model_roundtrip(self, conn: sqlite3.Connection) -> None: - """ - GIVEN: - - An empty index_meta table - WHEN: - - set_embed_model() is called then get_embed_model() is read back - THEN: - - The same string value is returned - """ - IndexMetaTable.create(conn) - assert IndexMetaTable.get_embed_model(conn) is None - IndexMetaTable.set_embed_model(conn, "model-a") - assert IndexMetaTable.get_embed_model(conn) == "model-a" - - def test_schema_version_roundtrip(self, conn: sqlite3.Connection) -> None: - """ - GIVEN: - - An empty index_meta table - WHEN: - - set_schema_version() is called then get_schema_version() is - read back - THEN: - - The same int value is returned - """ - IndexMetaTable.create(conn) - assert IndexMetaTable.get_schema_version(conn) is None - IndexMetaTable.set_schema_version(conn, 2) - assert IndexMetaTable.get_schema_version(conn) == 2 + getter = getattr(IndexMetaTable, getter_name) + setter = getattr(IndexMetaTable, setter_name) + assert getter(conn) is None + setter(conn, value) + assert getter(conn) == value def test_total_inserts_starts_at_zero(self, conn: sqlite3.Connection) -> None: """