Skip to content

fix(retain): make lazy bank creation atomic (#2695)#2802

Open
ijevin wants to merge 1 commit into
vectorize-io:mainfrom
ijevin:fix/lazy-bank-create-atomicity-2695
Open

fix(retain): make lazy bank creation atomic (#2695)#2802
ijevin wants to merge 1 commit into
vectorize-io:mainfrom
ijevin:fix/lazy-bank-create-atomicity-2695

Conversation

@ijevin

@ijevin ijevin commented Jul 18, 2026

Copy link
Copy Markdown

Summary

  • wrap the dedicated lazy bank-create path in conn.transaction()
  • make the bank INSERT and per-bank vector index DDL commit or roll back atomically
  • add regression coverage that forces vector-index creation to fail and verifies no bank row survives

Why

The dedicated-connection path in get_or_create_bank_profile() acquired an autocommit connection and then called get_or_create_bank_profile_on_conn(). A new bank row could therefore commit before create_bank_vector_indexes() ran. If that DDL failed, the empty bank remained even though setup did not complete.

This implements the fix described by @benfrank241 in #2695: the existing connection-bound helper stays unchanged, while the wrapper that owns the connection supplies the missing transaction boundary.

Test plan

  • uv run pytest tests/test_bank_utils.py -v -n 0
    • verified RED before the production change
    • passes after the transaction wrapper is added
  • uv run pytest tests/test_vector_index.py tests/test_bank_utils.py -v -n 0 — 20 passed
  • uv run ruff check hindsight_api/engine/retain/bank_utils.py tests/test_bank_utils.py
  • uv run ruff format --check hindsight_api/engine/retain/bank_utils.py tests/test_bank_utils.py
  • uv run ty check hindsight_api/engine/retain/bank_utils.py
  • py_compile for both changed Python files

Environment note

A broader local selection that includes DB-backed hierarchical-config tests could not start because the optional pg0-embedded dependency is not installed in this environment. The focused regression test uses a transaction-aware fake connection so it deterministically exercises the pre-fix autocommit behavior without requiring embedded PostgreSQL.

Fixes #2695

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retain reports success although PostgreSQL index creation fails and no document is stored

1 participant