diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/a2b3c4d5e6f8_add_gin_index_source_memory_ids.py b/hindsight-api-slim/hindsight_api/alembic/versions/a2b3c4d5e6f8_add_gin_index_source_memory_ids.py index f05cf510e2..c8ad569ddf 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/a2b3c4d5e6f8_add_gin_index_source_memory_ids.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/a2b3c4d5e6f8_add_gin_index_source_memory_ids.py @@ -40,20 +40,20 @@ def _get_schema_prefix() -> str: def _pg_upgrade() -> None: schema = _get_schema_prefix() - # CREATE INDEX CONCURRENTLY cannot run inside a transaction block. - # Commit the current Alembic transaction first. - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " - f"ON {schema}memory_units USING GIN (source_memory_ids) " - f"WHERE source_memory_ids IS NOT NULL" - ) + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block; an + # autocommit_block runs it outside Alembic's migration transaction. + with op.get_context().autocommit_block(): + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " + f"ON {schema}memory_units USING GIN (source_memory_ids) " + f"WHERE source_memory_ids IS NOT NULL" + ) def _pg_downgrade() -> None: schema = _get_schema_prefix() - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") def upgrade() -> None: diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/b3c4d5e6f7g8_add_temporal_date_indexes.py b/hindsight-api-slim/hindsight_api/alembic/versions/b3c4d5e6f7g8_add_temporal_date_indexes.py index cc37eaf10d..3aae565f52 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/b3c4d5e6f7g8_add_temporal_date_indexes.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/b3c4d5e6f7g8_add_temporal_date_indexes.py @@ -37,37 +37,35 @@ def _get_schema_prefix() -> str: def _pg_upgrade() -> None: schema = _get_schema_prefix() - # Partial index on occurred_start (covers "occurred_start BETWEEN $4 AND $5") - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_occurred_start " - f"ON {schema}memory_units(bank_id, fact_type, occurred_start) " - f"WHERE occurred_start IS NOT NULL" - ) - # Partial index on occurred_end (covers "occurred_end BETWEEN $4 AND $5") - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_occurred_end " - f"ON {schema}memory_units(bank_id, fact_type, occurred_end) " - f"WHERE occurred_end IS NOT NULL" - ) - # Partial index on mentioned_at (covers "mentioned_at BETWEEN $4 AND $5") - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_mentioned_at " - f"ON {schema}memory_units(bank_id, fact_type, mentioned_at) " - f"WHERE mentioned_at IS NOT NULL" - ) + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block; an + # autocommit_block runs each statement outside Alembic's migration transaction. + with op.get_context().autocommit_block(): + # Partial index on occurred_start (covers "occurred_start BETWEEN $4 AND $5") + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_occurred_start " + f"ON {schema}memory_units(bank_id, fact_type, occurred_start) " + f"WHERE occurred_start IS NOT NULL" + ) + # Partial index on occurred_end (covers "occurred_end BETWEEN $4 AND $5") + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_occurred_end " + f"ON {schema}memory_units(bank_id, fact_type, occurred_end) " + f"WHERE occurred_end IS NOT NULL" + ) + # Partial index on mentioned_at (covers "mentioned_at BETWEEN $4 AND $5") + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_bank_mentioned_at " + f"ON {schema}memory_units(bank_id, fact_type, mentioned_at) " + f"WHERE mentioned_at IS NOT NULL" + ) def _pg_downgrade() -> None: schema = _get_schema_prefix() - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_mentioned_at") - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_occurred_end") - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_occurred_start") + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_mentioned_at") + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_occurred_end") + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_bank_occurred_start") def upgrade() -> None: diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/c1a2b3d4e5f6_enable_pg_trgm_and_entities_trgm_index.py b/hindsight-api-slim/hindsight_api/alembic/versions/c1a2b3d4e5f6_enable_pg_trgm_and_entities_trgm_index.py index 327b35b73a..748c04cfbf 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/c1a2b3d4e5f6_enable_pg_trgm_and_entities_trgm_index.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/c1a2b3d4e5f6_enable_pg_trgm_and_entities_trgm_index.py @@ -47,17 +47,18 @@ def _pg_upgrade() -> None: schema = _get_schema_prefix() # GIN index on canonical_name enables sub-millisecond trigram similarity queries # (% operator, similarity()) instead of full-table scans across all bank entities. - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS entities_canonical_name_trgm_idx " - f"ON {schema}entities USING GIN (canonical_name gin_trgm_ops)" - ) + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block. + with op.get_context().autocommit_block(): + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS entities_canonical_name_trgm_idx " + f"ON {schema}entities USING GIN (canonical_name gin_trgm_ops)" + ) def _pg_downgrade() -> None: schema = _get_schema_prefix() - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}entities_canonical_name_trgm_idx") + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}entities_canonical_name_trgm_idx") # Note: not dropping pg_trgm extension as other indexes may depend on it diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/d2e3f4a5b6c7_add_memory_links_expansion_indexes.py b/hindsight-api-slim/hindsight_api/alembic/versions/d2e3f4a5b6c7_add_memory_links_expansion_indexes.py index a237fc6c0a..fff8252231 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/d2e3f4a5b6c7_add_memory_links_expansion_indexes.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/d2e3f4a5b6c7_add_memory_links_expansion_indexes.py @@ -50,39 +50,35 @@ def _get_schema_prefix() -> str: def _pg_upgrade() -> None: schema = _get_schema_prefix() - # CREATE INDEX CONCURRENTLY cannot run inside a transaction block. - # Commit the current Alembic transaction, then issue each CONCURRENTLY - # statement in its own implicit autocommit transaction. - # IF NOT EXISTS makes each statement idempotent if the migration is retried. - - # Index for the semantic *incoming* direction in link_expansion_retrieval.py. - # Replaces the BitmapAnd of idx_memory_links_to_unit ∩ idx_memory_links_link_type - # with a single composite index scan. - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_to_type_weight " - f"ON {schema}memory_links(to_unit_id, link_type, weight DESC)" - ) - - # Covering index for entity co-occurrence expansion. - # Enables an index-only scan: entity_id and to_unit_id are read from the - # index leaf pages instead of the heap, eliminating ~2 500 random heap-page - # reads per expansion query. - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_entity_covering " - f"ON {schema}memory_links(from_unit_id) " - f"INCLUDE (to_unit_id, entity_id) " - f"WHERE link_type = 'entity'" - ) + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block; an + # autocommit_block runs each statement outside Alembic's migration + # transaction. IF NOT EXISTS makes each statement idempotent on retry. + with op.get_context().autocommit_block(): + # Index for the semantic *incoming* direction in link_expansion_retrieval.py. + # Replaces the BitmapAnd of idx_memory_links_to_unit ∩ idx_memory_links_link_type + # with a single composite index scan. + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_to_type_weight " + f"ON {schema}memory_links(to_unit_id, link_type, weight DESC)" + ) + + # Covering index for entity co-occurrence expansion. + # Enables an index-only scan: entity_id and to_unit_id are read from the + # index leaf pages instead of the heap, eliminating ~2 500 random heap-page + # reads per expansion query. + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_entity_covering " + f"ON {schema}memory_links(from_unit_id) " + f"INCLUDE (to_unit_id, entity_id) " + f"WHERE link_type = 'entity'" + ) def _pg_downgrade() -> None: schema = _get_schema_prefix() - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_entity_covering") - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_to_type_weight") + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_entity_covering") + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_to_type_weight") def upgrade() -> None: diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/d4e5f6g7h8i9_gin_source_memory_ids_fastupdate_off.py b/hindsight-api-slim/hindsight_api/alembic/versions/d4e5f6g7h8i9_gin_source_memory_ids_fastupdate_off.py index 9272de71a6..7accf64b32 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/d4e5f6g7h8i9_gin_source_memory_ids_fastupdate_off.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/d4e5f6g7h8i9_gin_source_memory_ids_fastupdate_off.py @@ -33,26 +33,27 @@ def _get_schema_prefix() -> str: def _pg_upgrade() -> None: schema = _get_schema_prefix() - # DROP + CREATE CONCURRENTLY must run outside a transaction block. - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " - f"ON {schema}memory_units USING GIN (source_memory_ids) " - f"WITH (fastupdate=off) " - f"WHERE source_memory_ids IS NOT NULL" - ) + # DROP + CREATE CONCURRENTLY must run outside a transaction block; an + # autocommit_block runs them outside Alembic's migration transaction. + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " + f"ON {schema}memory_units USING GIN (source_memory_ids) " + f"WITH (fastupdate=off) " + f"WHERE source_memory_ids IS NOT NULL" + ) def _pg_downgrade() -> None: schema = _get_schema_prefix() - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " - f"ON {schema}memory_units USING GIN (source_memory_ids) " - f"WHERE source_memory_ids IS NOT NULL" - ) + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_units_source_memory_ids") + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_units_source_memory_ids " + f"ON {schema}memory_units USING GIN (source_memory_ids) " + f"WHERE source_memory_ids IS NOT NULL" + ) def upgrade() -> None: diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/e1b2c3d4f5a6_drop_unused_indexes.py b/hindsight-api-slim/hindsight_api/alembic/versions/e1b2c3d4f5a6_drop_unused_indexes.py index 8c598bd98b..881c335cbf 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/e1b2c3d4f5a6_drop_unused_indexes.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/e1b2c3d4f5a6_drop_unused_indexes.py @@ -75,13 +75,13 @@ def _schema_prefix() -> str: def _pg_upgrade() -> None: schema = _schema_prefix() - # DROP INDEX CONCURRENTLY cannot run inside a transaction block; commit - # the Alembic transaction and issue each statement in its own implicit - # autocommit transaction. IF EXISTS makes each statement idempotent - # across schemas that already dropped (or never had) the index. - for index_name in _PG_INDEXES_TO_DROP: - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}{index_name}") + # DROP INDEX CONCURRENTLY cannot run inside a transaction block; an + # autocommit_block drops out of Alembic's migration transaction so each + # statement runs in its own autocommit. IF EXISTS makes each statement + # idempotent across schemas that already dropped (or never had) the index. + with op.get_context().autocommit_block(): + for index_name in _PG_INDEXES_TO_DROP: + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}{index_name}") def _pg_downgrade() -> None: @@ -89,39 +89,40 @@ def _pg_downgrade() -> None: # Recreate the dropped indexes in the same shape the prior migrations used, # so a downgrade leaves the schema in the state the previous head expected. - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_entity_covering " - f"ON {schema}memory_links(from_unit_id) " - f"INCLUDE (to_unit_id, entity_id) " - f"WHERE link_type = 'entity'" - ) - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_from_unit ON {schema}memory_links(from_unit_id)" - ) - op.execute("COMMIT") - op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_to_unit ON {schema}memory_links(to_unit_id)") - op.execute("COMMIT") - op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_link_type ON {schema}memory_links(link_type)") - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_entities_canonical_name ON {schema}entities(canonical_name)" - ) - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS entities_canonical_name_trgm_idx " - f"ON {schema}entities USING GIN (canonical_name gin_trgm_ops)" - ) - op.execute("COMMIT") - op.execute( - f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_documents_retain_params " - f"ON {schema}documents USING GIN (retain_params)" - ) - op.execute("COMMIT") - op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_documents_content_hash ON {schema}documents(content_hash)") - op.execute("COMMIT") - op.execute(f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_unit_entities_entity ON {schema}unit_entities(entity_id)") + # CREATE INDEX CONCURRENTLY cannot run inside a transaction block. + with op.get_context().autocommit_block(): + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_entity_covering " + f"ON {schema}memory_links(from_unit_id) " + f"INCLUDE (to_unit_id, entity_id) " + f"WHERE link_type = 'entity'" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_from_unit ON {schema}memory_links(from_unit_id)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_to_unit ON {schema}memory_links(to_unit_id)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_memory_links_link_type ON {schema}memory_links(link_type)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_entities_canonical_name ON {schema}entities(canonical_name)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS entities_canonical_name_trgm_idx " + f"ON {schema}entities USING GIN (canonical_name gin_trgm_ops)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_documents_retain_params " + f"ON {schema}documents USING GIN (retain_params)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_documents_content_hash ON {schema}documents(content_hash)" + ) + op.execute( + f"CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_unit_entities_entity ON {schema}unit_entities(entity_id)" + ) def upgrade() -> None: diff --git a/hindsight-api-slim/hindsight_api/alembic/versions/e9b2c7d1f3a4_drop_entity_memory_links.py b/hindsight-api-slim/hindsight_api/alembic/versions/e9b2c7d1f3a4_drop_entity_memory_links.py index 344d5aa65b..a8bbf9c33b 100644 --- a/hindsight-api-slim/hindsight_api/alembic/versions/e9b2c7d1f3a4_drop_entity_memory_links.py +++ b/hindsight-api-slim/hindsight_api/alembic/versions/e9b2c7d1f3a4_drop_entity_memory_links.py @@ -37,33 +37,35 @@ def _pg_upgrade() -> None: schema = _pg_schema_prefix() # Drop the partial covering index first so the bulk DELETE doesn't churn it. - # CREATE/DROP INDEX CONCURRENTLY must run outside a transaction block. - op.execute("COMMIT") - op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_entity_covering") - - # Delete entity rows. Chunked to keep individual transactions small on - # large banks (the perf-medium bench had ~345k entity rows; production - # banks can be much larger). - op.execute( - f""" - DO $$ - DECLARE - deleted INTEGER; - BEGIN - LOOP - DELETE FROM {schema}memory_links - WHERE ctid IN ( - SELECT ctid FROM {schema}memory_links - WHERE link_type = 'entity' - LIMIT 50000 - ); - GET DIAGNOSTICS deleted = ROW_COUNT; - EXIT WHEN deleted = 0; - COMMIT; - END LOOP; - END$$; - """ - ) + # DROP INDEX CONCURRENTLY, and the DO block's per-batch COMMIT, both require + # running outside Alembic's migration transaction — an autocommit_block + # commits it and switches the connection to autocommit for the duration. + with op.get_context().autocommit_block(): + op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {schema}idx_memory_links_entity_covering") + + # Delete entity rows. Chunked to keep individual transactions small on + # large banks (the perf-medium bench had ~345k entity rows; production + # banks can be much larger). + op.execute( + f""" + DO $$ + DECLARE + deleted INTEGER; + BEGIN + LOOP + DELETE FROM {schema}memory_links + WHERE ctid IN ( + SELECT ctid FROM {schema}memory_links + WHERE link_type = 'entity' + LIMIT 50000 + ); + GET DIAGNOSTICS deleted = ROW_COUNT; + EXIT WHEN deleted = 0; + COMMIT; + END LOOP; + END$$; + """ + ) def _pg_downgrade() -> None: diff --git a/hindsight-api-slim/pyproject.toml b/hindsight-api-slim/pyproject.toml index 8eca0a7e01..5916240fc1 100644 --- a/hindsight-api-slim/pyproject.toml +++ b/hindsight-api-slim/pyproject.toml @@ -18,7 +18,11 @@ dependencies = [ "fastapi[standard]>=0.120.3", "uvicorn>=0.38.0", "wsproto>=1.0.0", - "sqlalchemy>=2.0.44", + # Cap below 2.1: SQLAlchemy 2.1 switches the default `postgresql://` DBAPI + # from psycopg2 to psycopg (v3), which we don't ship — a bare install would + # fail migrations with "No module named 'psycopg'". Pin to the tested 2.0 + # line (which keeps psycopg2 the default driver) until psycopg3 is adopted. + "sqlalchemy>=2.0.44,<2.1", "alembic>=1.17.1", "pgvector>=0.4.1", "greenlet>=3.2.4,<3.4.0", # 3.4.0 lacks arm64 wheels for manylinux_2_41 diff --git a/hindsight-api-slim/tests/test_migration_shape.py b/hindsight-api-slim/tests/test_migration_shape.py index 0585308840..6ff4e2dfb0 100644 --- a/hindsight-api-slim/tests/test_migration_shape.py +++ b/hindsight-api-slim/tests/test_migration_shape.py @@ -54,10 +54,75 @@ def test_migration_uses_dialect_dispatcher(path: Path) -> None: def _calls_run_for_dialect(fn: ast.FunctionDef) -> bool: for node in ast.walk(fn): - if ( - isinstance(node, ast.Call) - and isinstance(node.func, ast.Name) - and node.func.id == "run_for_dialect" - ): + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == "run_for_dialect": return True return False + + +def _is_manual_commit(node: ast.AST) -> bool: + """True if ``node`` is ``op.execute("COMMIT")`` (any casing/whitespace).""" + if not (isinstance(node, ast.Call) and node.args): + return False + func = node.func + if not (isinstance(func, ast.Attribute) and func.attr == "execute"): + return False + if not (isinstance(func.value, ast.Name) and func.value.id == "op"): + return False + first = node.args[0] + return isinstance(first, ast.Constant) and isinstance(first.value, str) and first.value.strip().upper() == "COMMIT" + + +@pytest.mark.parametrize("path", _migration_files(), ids=lambda p: p.name) +def test_migration_uses_autocommit_block_not_manual_commit(path: Path) -> None: + """Ban the ``op.execute("COMMIT")`` trick for escaping the migration transaction. + + ``CREATE/DROP INDEX CONCURRENTLY`` (and procedural ``COMMIT`` in ``DO`` blocks) + must run outside Alembic's migration transaction. The manual-COMMIT trick + happens to work on psycopg2 but breaks on psycopg/SQLAlchemy 2.1, where the + next statement re-opens a transaction and PostgreSQL rejects CONCURRENTLY. + Use ``with op.get_context().autocommit_block():`` instead. + """ + tree = ast.parse(path.read_text(), filename=str(path)) + offenders = [node.lineno for node in ast.walk(tree) if _is_manual_commit(node)] + assert not offenders, ( + f'{path.name}: op.execute("COMMIT") at line(s) {offenders}. ' + "Wrap CONCURRENTLY DDL in `with op.get_context().autocommit_block():` instead " + "of manually committing — the COMMIT trick fails on psycopg/SQLAlchemy 2.1." + ) + + +def _executes_concurrently_ddl(tree: ast.AST) -> bool: + """True if any string passed to ``op.execute(...)`` contains CONCURRENTLY.""" + for node in ast.walk(tree): + if not (isinstance(node, ast.Call) and node.args): + continue + func = node.func + if not (isinstance(func, ast.Attribute) and func.attr == "execute"): + continue + arg = node.args[0] + if isinstance(arg, ast.Constant) and isinstance(arg.value, str) and "CONCURRENTLY" in arg.value.upper(): + return True + return False + + +def _uses_autocommit_block(tree: ast.AST) -> bool: + return any(isinstance(node, ast.Attribute) and node.attr == "autocommit_block" for node in ast.walk(tree)) + + +@pytest.mark.parametrize("path", _migration_files(), ids=lambda p: p.name) +def test_migration_concurrently_ddl_runs_in_autocommit_block(path: Path) -> None: + """``CONCURRENTLY`` DDL must run inside an ``autocommit_block()``. + + PostgreSQL rejects ``CREATE/DROP INDEX CONCURRENTLY`` inside a transaction + block, and Alembic wraps every migration in one. The only safe escape is + ``with op.get_context().autocommit_block():``. This guards both the + manual-COMMIT trick and a CONCURRENTLY statement with no escape at all. + """ + tree = ast.parse(path.read_text(), filename=str(path)) + if not _executes_concurrently_ddl(tree): + pytest.skip("no CONCURRENTLY DDL") + assert _uses_autocommit_block(tree), ( + f"{path.name}: runs CONCURRENTLY DDL but never opens an autocommit_block(). " + "Wrap it in `with op.get_context().autocommit_block():` — CONCURRENTLY cannot " + "run inside Alembic's migration transaction." + ) diff --git a/skills/hindsight-docs/references/developer/mcp-server.md b/skills/hindsight-docs/references/developer/mcp-server.md index 73efb6d748..3af50fd9f4 100644 --- a/skills/hindsight-docs/references/developer/mcp-server.md +++ b/skills/hindsight-docs/references/developer/mcp-server.md @@ -185,7 +185,7 @@ Search memories to provide personalized responses. | `query` | string | Yes | Natural language search query | | `max_tokens` | integer | No | Maximum tokens to return (default: 4096) | | `budget` | string | No | Search thoroughness: `low`, `mid`, or `high` (default: `high`) | -| `types` | list[string] | No | Filter by fact type: `world`, `experience`, `opinion`. Defaults to all | +| `types` | list[string] | No | Filter by fact type: `world`, `experience`, `observation`. Defaults to all | | `tags` | list[string] | No | Filter memories by tags | | `tags_match` | string | No | Tag matching mode: `any` (default) or `all` | | `query_timestamp` | string | No | ISO 8601 timestamp — recall as if asking at this point in time; anchors relative temporal expressions and recency scoring | @@ -396,7 +396,7 @@ Browse stored memories with optional filtering and pagination. | Parameter | Type | Required | Description | |-----------|------|----------|-------------| -| `type` | string | No | Filter by fact type: `world`, `experience`, or `opinion` | +| `type` | string | No | Filter by fact type: `world`, `experience`, or `observation` | | `q` | string | No | Search query to filter memories | | `limit` | integer | No | Maximum number of results (default: 100) | | `offset` | integer | No | Number of results to skip for pagination (default: 0) | @@ -541,7 +541,7 @@ Clear all memories from a bank without deleting the bank itself. Optionally filt | Parameter | Type | Required | Description | |-----------|------|----------|-------------| -| `type` | string | No | Fact type to clear: `world`, `experience`, or `opinion`. If not specified, clears all | +| `type` | string | No | Fact type to clear: `world`, `experience`, or `observation`. If not specified, clears all | --- diff --git a/uv.lock b/uv.lock index 258bfa5fea..e4225dbd68 100644 --- a/uv.lock +++ b/uv.lock @@ -1716,7 +1716,7 @@ requires-dist = [ { name = "rich", specifier = ">=13.0.0" }, { name = "safetensors", marker = "extra == 'local-ml'", specifier = ">=0.6.2" }, { name = "sentence-transformers", marker = "extra == 'local-ml'", specifier = ">=3.3.0" }, - { name = "sqlalchemy", specifier = ">=2.0.44" }, + { name = "sqlalchemy", specifier = ">=2.0.44,<2.1" }, { name = "testcontainers", marker = "extra == 'test'", specifier = ">=4.0.0" }, { name = "tiktoken", specifier = ">=0.12.0" }, { name = "torch", marker = "extra == 'local-ml'", specifier = ">=2.6.0", index = "https://download.pytorch.org/whl/cpu" },