Make pgvector and the model registry schema-aware; fix sqlite-vec's engine reconstruction by nicoloesch · Pull Request #57 · AustralianCancerDataNetwork/omop-emb

nicoloesch · 2026-09-07T06:21:13Z

Depends on AustralianCancerDataNetwork/omop-alchemy#57. Requires pyproject.toml version update for oa-configurator, orm-loader, and omop-alchemy

Summary

What changed

table_exists, index management, and model-registry lookups all ignored the connection's configured schema, so a non-default-schema pgvector deployment silently read/wrote the wrong tables.
- Fixed via oa-configurator's schema-aware primitives, with new non-default-schema regression coverage.
The model registry now lives in its own reserved schema (MODEL_REGISTRY_SCHEMA), claimed via SchemaClaim(schema_tag="registry", physical_schema=MODEL_REGISTRY_SCHEMA, reserved=True), independent of whichever schema the embedding store itself resolves to.
Registry rows are now keyed by the owning vector-store entry (database_config_name), so two stores sharing one physical database only ever see their own models.
open_vector_store_writer/open_vector_store_reader replace the old single-mode opener: the writer claims the registry schema, installs the extension, and ensures the table; the reader registers nothing, runs no DDL, and is read-only at the interface level (EmbeddingStoreReader) as well as the database level.
@writes marks every mutating method on the backend; a test asserts every public member is classified once and that subclass overrides keep the marker.
Both the writer and the reader now fail closed with MisplacedRegistryError when a registry table exists in a schema other than MODEL_REGISTRY_SCHEMA, instead of the reader silently reporting zero models.
RegistryManager.__init__ verifies the registry's schema claim at construction instead of discovering a problem on first use.
The pgvector backend's drop-table and index-rebuild statements are now guarded against schema drift the same way table creation already was.
Registering a model now creates its storage table before writing the registry row, so a failed table creation never leaves an orphan row behind.
resolve_registry_schema() renamed resolve_registry_physical_schema(), matching the physical_schema_of naming used stack-wide.
ReadOnlyEmbeddingStore's schema/self.schema renamed physical_schema throughout to prevent ambiguity between a schema tag and a physical schema.
Adopts physical_schema_of stack-wide.
New test_schema_provenance_guard.py: registry-drift and primary-schema-drift regression tests against live Postgres.

Removed

PGVectorEmbeddingBackend.from_db_url() (dead code, zero callers).
read_only.py and the read-only store types, resolve_backend*, create_sqlitevec_engine, the pgvector index manager and metric plumbing (superseded by the writer/reader split above).

Breaking changes

create_sqlitevec_engine() is removed; pass an already-resolved Engine to the sqlite-vec backend directly.
resolve_backend() and the read-only store types are removed in favour of open_vector_store_writer/open_vector_store_reader.

Checklist

Applied exactly one label (breaking)
Tests pass locally (uv run pytest -q): 181 passed
Tests pass locally (uv run pytest -q -m postgresql): 59 passed

…urator, adaption of the config, update CI

…figurator, guard schemas properly

… schema across databases

gkennos

This PR imports new oa-configurator schema helpers and depends on the corresponding omop-alchemy table-role changes, but pyproject.toml still accepts any oa-configurator>=1,<2 and omop-alchemy>=1,<2. That permits clean installs which satisfy metadata while lacking the interfaces and semantics used here.

Please set the lower bounds to the first releases containing the assumed fixes, refresh the lockfile, and add a composed compatibility job using those exact artifacts. Also coordinate this package's major version with omop-graph's optional dependency range so graph can actually install the release that contains these breaking changes.

…lways writes and there is no reason to have read-only access at the moment

…N resolving, Remove old metric/index guided mechanisms and plumbing that not longer is required

…, explicit verification of registration schema

nicoloesch · 2026-10-07T06:42:43Z

Since the last review: registry rows are now keyed by the owning vector-store entry, so two stores sharing one physical database never see each other's models. The reader now fails closed on a misplaced registry the same way the writer already did. RegistryManager verifies its schema claim at construction instead of on first use. The pgvector backend's drop-table and index-rebuild statements are now guarded against schema drift like table creation already was. Body updated. Counts: 181 default, 59 with -m postgresql.

Copilot

🟡 Changes recommended

Dependency declarations, registry validation, registration ordering, and fail-closed behavior contain unresolved correctness issues.

8 open findings

Update dependencies for new schema APIs · New
Validate all required registry columns · New
Detect misplaced registries in other schemas · New
Require and validate all sidecar metadata · New
Raise when registered storage table is missing · New
Close search reader resources · New
Replace unreleased migration URL placeholder · New
Import Capabilities in example · New

What changed in this PR

Makes vector stores and the model registry schema-aware, introduces explicit reader/writer access, and improves pgvector/sqlite-vec lifecycle handling.

Changes:

Adds reserved-schema registry management and per-store model scoping.
Splits read-only and writable store opening paths.
Reworks index, population, filtering, bundle, and test infrastructure.

File	Description
`src/omop_emb/backends/base_backend.py`	Adds reader/writer lifecycle and storage guards.
`tests/test_sqlitevec.py`	Updates sqlite-vec lifecycle coverage.
`tests/test_schema_provenance_guard.py`	Tests schema-drift protection.
`tests/test_resolve_backend.py`	Tests writer backend resolution.
`tests/test_registry.py`	Expands registry-layout coverage.
`tests/test_registry_location.py`	Tests misplaced registries.
`tests/test_read_only_population.py`	Tests read-only population flows.
`tests/test_pgvector.py`	Expands schema and index coverage.
`tests/test_pgvector_index_sql.py`	Tests new index SQL helpers.
`tests/test_pgvector_index_manager.py`	Removes obsolete manager tests.
`tests/test_pgvector_descriptor.py`	Tests descriptor isolation.
`tests/test_metric_checks.py`	Tests query-only metric validation.
`tests/test_in_values.py`	Tests large single-parameter filters.
`tests/test_faiss_cache.py`	Updates FAISS metadata coverage.
`tests/test_embedding_generation.py`	Uses routed CDM sessions.
`tests/test_embedding_bundle.py`	Adds bundle-version compatibility tests.
`tests/test_config.py`	Tests unsupported backend handling.
`tests/test_cli_search.py`	Tests optional CDM enrichment.
`tests/test_cli_pgvector_snapshot.py`	Updates registry query tests.
`tests/shared_backend_tests.py`	Adapts shared backend contracts.
`tests/conftest.py`	Reworks database fixtures.
`src/omop_emb/utils/errors.py`	Adds registry and read-only errors.
`src/omop_emb/utils/embedding_utils.py`	Updates CDM session terminology.
`src/omop_emb/utils/cdm.py`	Introduces routed session factories.
`src/omop_emb/storage/faiss/faiss_cache.py`	Adds dedicated cache metadata.
`src/omop_emb/population.py`	Uses reader and routed sessions.
`src/omop_emb/model_registry/model_registry_types.py`	Documents scoped storage identifiers.
`src/omop_emb/model_registry/model_registry_orm.py`	Adds reserved-schema registry handling.
`src/omop_emb/model_registry/model_registry_manager.py`	Adds scoped, atomic registration.
`src/omop_emb/model_registry/__init__.py`	Exports new registry APIs.
`src/omop_emb/config.py`	Adds registry and CDM helpers.
`src/omop_emb/cli/cli_maintenance.py`	Uses reader/writer store APIs.
`src/omop_emb/cli/cli_embeddings.py`	Uses routed CDM and store access.
`src/omop_emb/cli/cli_diagnostics.py`	Updates read-only diagnostics.
`src/omop_emb/backends/sqlitevec/sqlitevec_sql.py`	Replaces temporary-table filtering.
`src/omop_emb/backends/sqlitevec/sqlitevec_backend.py`	Uses resolved engines and read-only mode.
`src/omop_emb/backends/sqlitevec/__init__.py`	Removes engine factory export.
`src/omop_emb/backends/read_only.py`	Removes legacy read-only wrapper.
`src/omop_emb/backends/pgvector/pg_sql.py`	Adds schema-aware DDL and filtering.
`src/omop_emb/backends/pgvector/pg_index_manager.py`	Removes obsolete index managers.
`src/omop_emb/backends/pgvector/pg_backend.py`	Reworks pgvector indexing and access.
`src/omop_emb/backends/pgvector/__init__.py`	Removes manager exports.
`src/omop_emb/backends/index_config.py`	Generalizes config deserialization.
`src/omop_emb/backends/embedding_table.py`	Adds metadata comparison behavior.
`src/omop_emb/backends/db_utils.py`	Adds dialect-specific bound-list filtering.
`src/omop_emb/backends/base_index_manager.py`	Removes obsolete abstraction.
`src/omop_emb/backends/__init__.py`	Exports reader/writer APIs.
`src/omop_emb/__init__.py`	Updates public exports.
`pyproject.toml`	Separates default and database tests.
`docs/usage/interface-guide.md`	Documents new store interfaces.
`docs/usage/installation.md`	Updates configurator syntax.
`docs/usage/configuration.md`	Documents registry ownership.
`docs/usage/cli.md`	Documents metric-free bundles.
`docs/usage/backend-selection.md`	Updates index behavior guidance.
`Dockerfile`	Removes obsolete container definition.
`CHANGELOG.md`	Notes model ownership constraints.
`.github/workflows/ci.yml`	Splits SQLite and PostgreSQL CI.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

nicoloesch added 6 commits September 1, 2026 22:56

Test per dialect with new method, schema-aware changes from oa-config…

a850d26

…urator, adaption of the config, update CI

Drop docker

7017baf

Update CI

d8e11ca

Carry ResolvedDatabase downstream to other methods adhering to oa-con…

ce5c705

…figurator, guard schemas properly

Schema-aware Registry

a9017d6

Incorporate dialect and schema_translate_map changes and keys

01a886f

nicoloesch added the fix Bug fix, backwards-compatible. PATCH: x.y.z+1 label Sep 7, 2026

nicoloesch added 8 commits September 15, 2026 00:17

Properly pass down schema, tag tables with schema, share the registry…

b3fc640

… schema across databases

remove underscore-prefixing for public API method

fb4cfbc

Increase test coverage

74e783d

Correctly raise on an unsupporter backend, include tests for it

d1a072a

Remove Role following oa-configurator changes

cbbab3f

Follow-up from internal review

523142a

Disambiguate physical schema from schema tag

7b0216e

Correct naming and wiring

87d6010

nicoloesch added breaking Incompatible API change. MAJOR: x+1.y.z and removed fix Bug fix, backwards-compatible. PATCH: x.y.z+1 labels Sep 23, 2026

nicoloesch added 2 commits September 24, 2026 00:16

Docstring rectifications

4142223

Updated Docs

d0fd1c7

gkennos self-requested a review September 25, 2026 01:35

gkennos requested changes Sep 25, 2026

View reviewed changes

Comment thread src/omop_emb/model_registry/model_registry_orm.py Outdated

Comment thread src/omop_emb/model_registry/model_registry_orm.py Outdated

nicoloesch added 6 commits October 1, 2026 02:19

Update to new oa-config stuff

b1f66b2

Properly wire up extensions, make it write only since create_engine a…

71ba533

…lways writes and there is no reason to have read-only access at the moment

Updatet tests, read-only inspections

3db1337

Bring back read-only bits with proper interfaces, remove cumbersome I…

3d029cb

…N resolving, Remove old metric/index guided mechanisms and plumbing that not longer is required

Update from manual review

fd47c1a

Schema guards, fixing smaller bugs, LRU cache for pg table descriptor…

3eb50ff

…, explicit verification of registration schema

nicoloesch requested a review from gkennos October 7, 2026 06:45

Address PR comments

5889ec6

nicoloesch and others added 3 commits October 9, 2026 03:46

Legacy test cleanup

30a20e4

Thread through the split vocab changes

65af920

fixes for model naming clashes

c1513d8

gkennos requested a balanced review from Copilot October 10, 2026 00:18

Copilot started reviewing on behalf of gkennos October 10, 2026 00:18

View session

Copilot AI reviewed Oct 10, 2026

View reviewed changes

changes for configurator 2.x

5ff4325

gkennos approved these changes Oct 11, 2026

View reviewed changes

gkennos merged commit 131898d into main Oct 11, 2026
3 checks passed

gkennos deleted the fix/schema-translate-map-pgvector-registry branch October 11, 2026 00:18

Navigation Menu

Repository navigation

Make pgvector and the model registry schema-aware; fix sqlite-vec's engine reconstruction - #57

Make pgvector and the model registry schema-aware; fix sqlite-vec's engine reconstruction#57
gkennos merged 27 commits into
mainfrom
fix/schema-translate-map-pgvector-registry

nicoloesch commented Sep 7, 2026 •

edited

Loading

Uh oh!

gkennos left a comment

Uh oh!

Uh oh!

Uh oh!

nicoloesch commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nicoloesch commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Removed

Breaking changes

Checklist

Uh oh!

gkennos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nicoloesch commented Oct 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nicoloesch commented Sep 7, 2026 •

edited

Loading