diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9d47eb2..5dac962 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,14 +35,8 @@ jobs: --password postgres \ --database-name test_db \ --test-only false - uv run omop-config databases add test_cdm_db \ - --kind cdm \ - --connection pg_test \ - --schema-name public - uv run omop-config databases add test_vector_db \ - --kind generic \ - --connection pg_test \ - --schema-name public + uv run omop-config databases add cdm test_cdm_db --connection pg_test --cdm-schema public + uv run omop-config databases add generic test_vector_db --connection pg_test --schema-name embeddings uv run omop-config vector-stores add test_embeddings \ --backend-type pgvector \ --database test_vector_db diff --git a/.gitignore b/.gitignore index e20d926..27ade95 100644 --- a/.gitignore +++ b/.gitignore @@ -19,4 +19,5 @@ settings.json site/ _knowledge/ data/ -.env \ No newline at end of file +.env +*.db \ No newline at end of file diff --git a/README.md b/README.md index e3b7c6f..a8bf159 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ Start with the [documentation home](docs/index.md), or choose a path directly: - [Initial local setup](docs/from-scratch.md) walks through a fresh CDM-backed deployment. - [Integrations](docs/usage/integrations.md) shows MCP, REST, and Python calls. - [Configuration](docs/usage/configuration.md) explains shared-stack references and optional capabilities. +- [Upgrading from 0.6](docs/usage/upgrading-from-0.6.md) lists the 0.7 configuration and runtime changes. - [Tools overview](docs/tools/overview.md) maps jobs to the discoverable MCP surface. ## Quick start diff --git a/config/groundworkers.example.toml b/config/groundworkers.example.toml index 5c43f76..f6be44a 100644 --- a/config/groundworkers.example.toml +++ b/config/groundworkers.example.toml @@ -1,4 +1,4 @@ -# groundworkers example config — oa-configurator 1.x StackConfig format +# groundworkers example config — oa-configurator 2.0 StackConfig format # # Copy to a path selected by the setup console and fill in your values. # Full reference: https://oa-configurator.readthedocs.io (or run omop-config --help) @@ -33,19 +33,19 @@ database_name = "cava" # Databases — logical roles over those connections # --------------------------------------------------------------------------- -# kind = "cdm" bundles the CDM, vocabulary, and results roles. vocab_connection -# and vocab_schema fall back to the primary connection and schema when unset. +# kind = "cdm" bundles the CDM, vocabulary, and results roles. The role schema +# fields fall back to the primary schema when unset. Omit these fields for SQLite. [databases.cdm_db] kind = "cdm" connection = "cdm_main" -schema_name = "public" +cdm_schema = "public" vocab_schema = "public" -# The embedding store's database. kind = "generic": no CDM role-splitting. +# The embedding store's database. Keep its schema distinct from the CDM schema. [databases.emb_db] kind = "generic" connection = "cdm_main" -schema_name = "public" +schema_name = "groundworkers" # --------------------------------------------------------------------------- # Providers and models — endpoints, then the named models served through them diff --git a/docs/adapters/cdm.md b/docs/adapters/cdm.md index a3cbcf9..07f25ef 100644 --- a/docs/adapters/cdm.md +++ b/docs/adapters/cdm.md @@ -1,32 +1,30 @@ # CDM Adapter -`CDMAdapter` holds the SQLAlchemy engine and session factory for a single CDM database connection. It is the shared connection layer used by both `VocabService` and `OmopGraphAdapter`. +`CDMAdapter` holds the primary engine, vocabulary engine, and routed session factory for a resolved CDM database. It is the shared connection layer used by `VocabService` and `OmopGraphAdapter`. ## What it is `CDMAdapter` is a thin connection-management object. It does not issue queries itself. Its purpose is to: -- hold a `sqlalchemy.Engine` pointed at the CDM database -- expose a `sessionmaker` for callers that need scoped sessions -- be constructed once per CDM database in `build_application()` and shared across the - components that need it +- hold the primary and vocabulary `sqlalchemy.Engine` objects +- expose a routed session factory for callers that query role-tagged OMOP tables +- be constructed once per CDM database in `build_application()` and shared across the components that need it ## Who uses it | Component | How | |---|---| | `VocabService` | Takes a `CDMAdapter` as its only constructor argument. Uses the session factory to open per-call sessions for vocabulary table queries. | -| `OmopGraphAdapter` | Shares the same engine (no separate connection pool) so omop-graph and vocabulary queries use the same connection pool. | +| `OmopGraphAdapter` | Uses the routed session factory so vocabulary and primary-role graph tables reach their configured databases. | The adapter is created once per CDM database at application startup and wired through `build_application()`. It is not per-request. ## Construction -`CDMAdapter` is config-agnostic: it takes an already-constructed `sqlalchemy.Engine`, not a config object. Only `build_application()` reads config and calls `sqlalchemy.create_engine()`. This means: +`CDMAdapter` is config-agnostic. It accepts an engine and optional routed factory, while `build_application()` creates them through `omop_alchemy.config.create_cdm_engines()` and `omop_alchemy.cross_database.cdm_sessionmaker()`: -- the adapter is testable by passing a SQLite engine or an in-memory engine directly -- changing the connection source means changing `build_application()` only; - `CDMAdapter` and every service that uses it are untouched +- the adapter remains testable by passing a SQLite engine directly +- changing the connection source means changing `build_application()` only;`CDMAdapter` and every service that uses it are untouched ## Usage from `build_application()` @@ -48,6 +46,6 @@ app.services.vocab # backed by app.adapters.cdm ## Lifecycle -`CDMAdapter` connects lazily: the engine is constructed at adapter creation time, but the first database connection is deferred until the first query. `is_available()` performs a lightweight probe (`SELECT 1`) to confirm connectivity without blocking unrelated tool calls. +`build_app_config()` creates role engines through `create_cdm_engines(..., register_claims=False)`. Startup fails fast when either database role is unreachable and prints the scrubbed reason with a database recovery hint. `CDMAdapter.is_available()` also performs a lightweight Core query (`SELECT 1`). -`close()` disposes the engine and releases the connection pool. Call it explicitly in teardown (for example in a test fixture `yield`/`finally` block) when you need to ensure connections are returned promptly. Process-level startup and shutdown wiring belongs in the application or transport layer, not in the adapter itself. +`close()` disposes both role engines. Call it explicitly in teardown (for example in a test fixture `yield`/`finally` block) when you need to ensure connections are returned promptly. Process-level startup and shutdown wiring belongs in the application or transport layer, not in the adapter itself. diff --git a/docs/adapters/omop_emb.md b/docs/adapters/omop_emb.md index b1a42f5..44d2163 100644 --- a/docs/adapters/omop_emb.md +++ b/docs/adapters/omop_emb.md @@ -10,15 +10,15 @@ The adapter is responsible for: - optional on-the-fly query encoding through an embedding client - exposing backend availability and registered model metadata -The adapter does **not** resolve stack config itself. `build_application(...)` constructs it from the already-resolved `omop-emb` package config and any required engines. +The adapter does **not** resolve stack config itself. `build_application(...)` constructs it from Groundworkers' `[tools.groundworkers]` `embedding_model_name` and `vector_store_name` references. ## Backends `groundworkers` supports the same primary storage backends it wires from `omop-emb`: -| Backend | omop-emb config | +| Backend | Groundworkers stack config | |---|---| -| `sqlitevec` | `backend = "sqlitevec"` plus `sqlite_path` | +| `sqlitevec` | a `[vector_stores.*]` entry with `backend_type = "sqlitevec"` and a database reference | | `pgvector` | a `[vector_stores.*]` entry with `backend_type = "pgvector"` and a `database` reference | FAISS remains a sidecar acceleration layer rather than a standalone primary backend. @@ -34,7 +34,7 @@ No live model API is required. ### Text search mode -Requires an embedding client configured through `omop-emb` package settings. +Requires `embedding_model_name` and `vector_store_name`, plus a configured embedding provider/model. - `embedding_search` encodes a text query on the fly - `embedding_search_batch` encodes and searches multiple text queries with one provider/index batch, returning rows aligned to input order diff --git a/docs/development/integration.md b/docs/development/integration.md index 1ab91bd..d91a8eb 100644 --- a/docs/development/integration.md +++ b/docs/development/integration.md @@ -18,7 +18,8 @@ app = build_application(config) |---|---| | `groundworkers` | `GroundworkersConfig` | | `cdm_database` | `ResolvedCDMDatabase` | -| `cdm_engine`, `vocabulary_engine` | `sqlalchemy.Engine` | +| `cdm_engine`, `cdm_vocab_engine` | `sqlalchemy.Engine` | +| `cdm_session_factory` | Routed SQLAlchemy session factory for primary, vocabulary, and results roles | | `embedding_model` | `ResolvedModel \| None` | | `vector_store` | `ResolvedVectorStore \| None` | @@ -42,6 +43,8 @@ services.source_planning A CDM-only configuration is fully supported. Graph availability follows the resolved CDM database — there is no separate section to enable. +The engines are created by `omop_alchemy.config.create_cdm_engines(..., register_claims=False)`, and `cdm_session_factory` comes from `omop_alchemy.cross_database.cdm_sessionmaker`. Use the session factory for queries that may touch more than one OMOP role. On PostgreSQL, engine creation checks role schema claims eagerly, so the configured databases must be reachable while `build_app_config()` runs. + ## Grounding free text ```python @@ -127,6 +130,6 @@ operation = ( controller = ConfigWizardController(cdm_setup_workflow(operation), service) ``` -The same shape covers `MODEL_SETUP_TARGET` / `model_setup_workflow` and `LLM_SETUP_TARGET` / `llm_setup_workflow`. The provider decides create-versus-update from the current file, owns revision checking and redaction, and returns `conflicted` rather than clobbering another writer. `model_discoverer` is the injected seam for live model inventory; supply your own to avoid network calls in tests. +The same shape covers `MODEL_SETUP_TARGET` / `model_setup_workflow`, `LLM_SETUP_TARGET` / `llm_setup_workflow`, and `VECTOR_STORE_SETUP_TARGET` / `vector_store_setup_workflow`. The provider decides create-versus-update from the current file, owns revision checking and redaction, and returns `conflicted` rather than clobbering another writer. `model_discoverer` is the injected seam for live model inventory; supply your own to avoid network calls in tests. This requires the `tui` extra for Groundskeeping, but not Textual — the provider and controller are host-agnostic. diff --git a/docs/development/plugins.md b/docs/development/plugins.md index fa4791c..11db6c0 100644 --- a/docs/development/plugins.md +++ b/docs/development/plugins.md @@ -59,7 +59,7 @@ For example, a plugin recommending comparator cohorts might reason like this. | Use case | What you need | How you get it | |---|---|---| -| Look up existing OMOP concepts | Read access to the CDM vocabulary connection Groundworkers already opened | `context.cdm_engine` / `context.resolver` | +| Look up existing OMOP concepts | Read access through Groundworkers' routed vocabulary session | `context.cdm_session_factory` / `context.resolver` | | Rank or filter by similarity to concepts Groundworkers already embedded | The concept embeddings Groundworkers already populated, via omop-emb | `context.vector_store` + `context.embedding_backend_factory` | | Ask an LLM to help decide something | The chat model Groundworkers already configured, via omop-llm | `context.chat_backend_factory` | @@ -76,12 +76,14 @@ The concept-embedding row and the free-text-embedding row look similar but are n ### Assumptions this table is making -- **Vocabulary only, so far.** Every example above reads OMOP *vocabulary* i.e. operations over concepts, concept relationships, concept embeddings, or the host's shared grounding cascade. Nothing about `context.cdm_engine` restricts a plugin to vocabulary tables specifically, however. These tables are available over the same CDM connection groundworkers itself uses, and a plugin *could* query clinical/person-level tables (`condition_occurrence`, `drug_exposure`, etc.) through it just as easily, subject to whatever environmental and procedural access controls actually govern that data. +- **Vocabulary only, so far.** Every example above reads OMOP vocabulary: concepts, concept relationships, concept embeddings, or the host's shared grounding cascade. `context.cdm_session_factory` routes vocabulary and primary roles, including on split-vocabulary deployments. Clinical/person-level queries need a database role explicitly configured for them. - **Shared mapping access.** `PluginContext.mapping_context` is an optional,read-only capability façade over the host's mapping and grounding policy. Use its operations when a plugin needs candidate bundles, text grounding, or concept context. `concept_shared_standard_ancestor(concept_ids, max_depth=5)` returns the unique nearest shared standard ancestor when one exists; raise `max_depth` to widen the hierarchy search. The underlying vocabulary, graph, and service objects are deliberately not exposed. Do not construct a second `omop-graph` or vocabulary adapter from `context.cdm_engine`. The field is `None` only when the host did not compose services for the plugin context. ## Build and register a new plugin -`build()` receives `PluginContext`. The context provides the resolved CDM database and engine, vector store, shared lazy model backend factories, the optional read-only `mapping_context`, and a deliberately narrow resolver for independent named OA resources. Return `None` when a prerequisite is unavailable. Groundworkers then keeps serving its core capabilities and reports why the plugin was not activated. +`build()` receives `PluginContext`. The context provides the resolved CDM database and primary engine, routed `cdm_session_factory`, vocabulary engine, vector store, shared lazy model backend factories, the optional read-only `mapping_context`, and a deliberately narrow resolver for independent named OA resources. Return `None` when a prerequisite is unavailable. Groundworkers then keeps serving its core capabilities and reports why the plugin was not activated. + +Use `context.cdm_session_factory` for role-tagged OMOP queries. `context.cdm_engine` remains available for primary-role access. On split-vocabulary stacks, plugins must use the routed factory; an Engine-based OmopGraphAdapter or DomainNameResolver cannot route vocabulary queries and will fail with upgrade guidance. `register()` adds MCP tools, prompts, or resources against the state returned by `build()`. The shared MCP registration wrapper supplies the same safe error translation used by core tools. Code-defined prompts can continue to derive their arguments from the callable signature. A plugin publishing data-defined prompts can pass explicit string metadata with `PromptArgument`; this keeps dynamic pack fields out of Python closure signatures: @@ -158,7 +160,8 @@ Schemas requiring live discovery or a conditional flow (and therefore not fittin ```python from typing import Annotated, ClassVar -from oa_configurator import PackageConfigBase, Sensitive +from oa_configurator import PackageConfigBase +from oa_configurator.refs import Sensitive, Secret from pydantic import Field @@ -167,10 +170,23 @@ class ExampleConfig(PackageConfigBase): api_token: Annotated[str | None, Sensitive()] = Field(default=None) ``` -The `oa_configurator.Secret` alias is equivalent to `Annotated[str | None, Sensitive()]`. Prefer a `RefTo` field when a plugin can reuse an existing database, model, provider, or vector-store entry. The plugin then stores only the logical reference name; sensitivity remains declared on the referenced schema, such as `ConnectionConfig.password` or `ProviderConfig.api_key`, and the generic recursive configuration workflow preserves those declarations. +The `Secret` alias is equivalent to `Annotated[str | None, Sensitive()]`. Prefer a `RefTo` field when a plugin can reuse an existing database, model, provider, or vector-store entry. The plugin then stores only the logical reference name; sensitivity remains declared on the referenced schema, such as `ConnectionConfig.password` or `ProviderConfig.api_key`, and the generic recursive configuration workflow preserves those declarations. This protects display and logging surfaces; it does not encrypt persisted configuration. Plugin documentation must describe the storage expectations for any secret it introduces. +## Upgrading plugins to Groundworkers 0.7 + +1. Require `oa-configurator>=2,<3` and `groundworkers>=0.7,<0.8` in the plugin package. +2. Import `Sensitive` and `Secret` from `oa_configurator.refs`, and load files with `load_stack_config(path)`. +3. Create CDM role engines with `omop_alchemy.config.create_cdm_engines(resolved, register_claims=False)` for read-only plugin access; use `context.cdm_session_factory` when Groundworkers provides the database. +4. Remove schema fields from SQLite database entries. For PostgreSQL CDM entries, use `cdm_schema`; vector-store database entries continue to use `schema_name`. +5. Keep accepting `context.cdm_engine` for integrations that need the primary SQLAlchemy engine. The routed factory handles vocabulary, primary, and results roles for ORM queries. +6. Treat `context.embedding_backend_factory()` as a read-only omop-emb 3.0 `EmbeddingStoreReader`. It is shared with Groundworkers tools and status; reader methods no longer accept `metric_type`, and model enumeration is `get_registered_models()`. Do not use it to register models, create tables, or write vectors. Use a plugin-owned store and explicit setup operation for plugin data. +7. Pass `context.cdm_session_factory` when constructing `OmopGraphAdapter`; Engine mode only works for colocated databases. `context.cdm_engine` is the primary-role engine and cannot query vocabulary on a split deployment. +8. Use the current public error codes: `LEGACY_REGISTRY`, `MISPLACED_REGISTRY`, `MISSING_STORAGE_TABLE`, `MODEL_REGISTRATION_CONFLICT`, and `READ_ONLY_STORE`. +9. Update `describe()` consumers for the `llm_model`, `stack`, `groundworkers`, and `app_name` keys and `cdm_schema` / `vocab_schema` names. The old `base.sql` schema/engine helpers have been removed; use oa-configurator's `physical_schema_of`, `qualified`, `supports_schemas`, and `schema_for_role` APIs. +10. Update direct `AppConfig` construction for the required `stack`, `groundworkers`, `cdm_database`, `cdm_engine`, `cdm_vocab_engine`, `cdm_session_factory`, `embedding_model`, `llm_model`, `vector_store`, and `knowledge_root` fields; prefer `build_app_config()` in plugins and tests. + ## Setup steps Plugin data preparation belongs in registered setup steps. A plugin may expose a `setup_steps` tuple of `PluginSetupStep` instances. Each step declares a stable key, operator copy, typed arguments, and a `build_plan(values, config_path)` callable that returns a `MaintenancePlan`. diff --git a/docs/index.md b/docs/index.md index f189092..ff32109 100644 --- a/docs/index.md +++ b/docs/index.md @@ -69,6 +69,7 @@ The output includes the active tools, prompts, resources, plugins, redacted conf - [Initial local setup](from-scratch.md) for a fresh local deployment - [Integrations](usage/integrations.md) for MCP, REST, and direct Python usage - [Configuration](usage/configuration.md) for the shared-stack config model and runtime combinations +- [Upgrading from 0.6](usage/upgrading-from-0.6.md) for the 0.7 configuration and plugin changes - [Tools overview](tools/overview.md) for the discoverable MCP surface ## Relationship to groundcrew diff --git a/docs/tools/embedding.md b/docs/tools/embedding.md index 63d0e63..d133766 100644 --- a/docs/tools/embedding.md +++ b/docs/tools/embedding.md @@ -1,6 +1,6 @@ # Embedding Tools -Five tools expose the embedding index. All require `omop_emb` to be enabled in the config. +Five tools expose the embedding index. They require `embedding_model_name` and `vector_store_name` in `[tools.groundworkers]`. ## `embedding_index_status` @@ -23,12 +23,16 @@ Returns the status of the embedding backend and all registered models. Takes no } ``` -When the backend is unavailable or no models are registered, `"available"` is `false` and `"models"` is an empty array. This tool never returns an error dict — it degrades gracefully. +When the backend is unavailable or no models are registered, `"available"` is `false` and `"models"` is an empty array. This tool never returns an error dict — it degrades gracefully. A legacy registry, misplaced registry, or missing model table is reported in `detail` with a specific explanation. + +Groundworkers opens the store through omop-emb's read-only reader for status and query tools. A pre-3.0 store reports `LEGACY_REGISTRY`. The migration script is tracked in oa-configurator issue #40; once it ships, Groundworkers will show omop-emb's migration command. Groundworkers does not migrate the store. ## `embedding_neighbours` Returns the nearest embedding-space neighbours for one OMOP concept by looking up its existing vector in the index. +By default `standard_only` and `active_only` are both `true`, matching the partial HNSW index predicate. Set either to `false` to widen the result set. + ```json {"concept_id": 4119419, "limit": 10, "model_name": "qwen3-embedding:0.6b"} ``` @@ -66,9 +70,10 @@ Encodes a text query on the fly and returns the nearest concepts by vector simil All parameters except `query` are optional. -!!! warning "Requires `api_base` configuration" +!!! warning "Requires a configured embedding model" `embedding_search` must encode the query string using a live embedding model. - This requires `omop_emb.api_base` (and `api_key`) to be configured. Without it + Configure `embedding_model_name` in `[tools.groundworkers]` and give its + `[models.*]` entry a provider in `[providers.*]`. Without a live model client, the tool returns `BACKEND_UNAVAIL`. ## `embedding_search_batch` @@ -110,4 +115,4 @@ Encodes a text string into one query embedding vector. } ``` -Like `embedding_search`, this requires `omop_emb.api_base` and `omop_emb.api_key` to be configured. +Like `embedding_search`, this requires a configured `embedding_model_name` and a live provider client. diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 0db1da5..8d6a0dc 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -64,7 +64,7 @@ database_name = "omop" [databases.cdm_db] kind = "cdm" connection = "cdm_main" -schema_name = "public" +cdm_schema = "public" vocab_schema = "public" [providers.local_ollama] @@ -86,6 +86,11 @@ structured_output = true backend_type = "pgvector" database = "vector_db" +[databases.vector_db] +kind = "generic" +connection = "cdm_main" +schema_name = "groundworkers" + [tools.groundworkers] cdm_db = "cdm_db" embedding_model_name = "embedding_model" @@ -169,7 +174,7 @@ REST is selected explicitly with `--transport rest`; it is not started alongside | `rest_port` | `int` | `8080` | | `rest_base_path` | `str` | `/v1` | -`base_path` is validated to begin with `/`. +`rest_base_path` is validated to begin with `/`. ### `llm_model_name` and chat @@ -201,9 +206,9 @@ Groundworkers-owned grounding policy. omop-graph's traversal limits are per-call | `grounding_min_fulltext_overlap` | `float` | `0.0` | | `grounding_max_depth` | `int` (1-10) | `5` | -`min_fulltext_overlap` must be between `0.0` and `1.0`. It is the minimum proportion of query tokens that must appear in a matched concept name for a full-text hit to be accepted; below the threshold grounding falls through to the next tier. +`grounding_min_fulltext_overlap` must be between `0.0` and `1.0`. It is the minimum proportion of query tokens that must appear in a matched concept name for a full-text hit to be accepted; below the threshold grounding falls through to the next tier. -`max_depth` bounds the hierarchy distance between a grounding candidate and a required parent concept, or the identity-hop count when grounding runs without `parent_ids`. +`grounding_max_depth` bounds the hierarchy distance between a grounding candidate and a required parent concept, or the identity-hop count when grounding runs without `parent_ids`. ### Concept flag contract @@ -289,6 +294,10 @@ Both references are required. With only one configured, status reports an action The store must be populated before the embedding tier can return anything. Population is explicit (see the Embeddings setup section) and never starts at query time. The server holds the graph read-only (`write=False`), so Groundworkers encodes query text for the embedding tier but never writes vectors. +Groundworkers opens vector stores with omop-emb's read-only reader for MCP tools, status, coverage, and setup diagnostics. Explicit store initialization uses omop-emb's writer. A database supports one vector store per model: reuse the existing store or choose another database when registering a model that is already present. On PostgreSQL, the wizard stores vectors in the dedicated `groundworkers` schema. omop-emb also creates `omop_emb_registry`, and oa-configurator may create `oa_configurator_provenance`. SQLite uses its database file without a schema. + +If the store predates omop-emb 3.0, Groundworkers reports `LEGACY_REGISTRY`. The migration script is tracked in oa-configurator issue #40; once it ships, Groundworkers will show omop-emb's migration command. Groundworkers does not migrate legacy stores. + ### With `groundworkers.llm_model_name` configured You additionally get: @@ -342,4 +351,4 @@ mapping = app.services.mapping Service attributes are `None` only when their prerequisites are not available in the resolved runtime. -`config.describe()` returns a redacted summary keyed by `database`, `model`, and `vector_store`. Passwords and API keys are masked; safe URLs never carry credentials. +`config.describe()` returns a redacted summary keyed by `database`, `model`, `llm_model`, `vector_store`, `stack`, `groundworkers`, and `app_name`. CDM schema keys are `cdm_schema` and `vocab_schema`; generic/vector-store databases continue to use `schema_name`. Passwords and API keys are masked; safe URLs never carry credentials. diff --git a/docs/usage/setup_journey.md b/docs/usage/setup_journey.md index 716c051..59a0569 100644 --- a/docs/usage/setup_journey.md +++ b/docs/usage/setup_journey.md @@ -5,11 +5,11 @@ The setup console is organized by the capability being configured: 1. **Overview** — CDM readiness, optional capabilities, and integration output; 2. **Database** — physical connections and logical CDM database entries; 3. **Graph** — CDM-backed search and graph preparation; -4. **Chat Model** — provider and model entries for LLM-backed features; -5. **Embeddings** — vector-store setup, coverage, and population; -6. **Performance** — graph, trigram, and embedding-index readiness; +4. **Performance** — graph, trigram, and embedding-index readiness; +5. **LLM provider** — provider and model entries for language features; +6. **Embeddings** — vector-store setup, coverage, and population; 7. **Chat** — checks for the selected chat model; -8. **View Configuration** — redacted stack configuration; and +8. **Configuration** — redacted stack configuration; and 9. **Runs** — progress and history for durable maintenance work. For a minimal local setup, use **Overview → Configure CDM → Test connections**. Optional capabilities can be configured later. Configuration changes show the redacted entries that will be written before they are applied. diff --git a/docs/usage/testing.md b/docs/usage/testing.md index 6bd4d1a..233d7e0 100644 --- a/docs/usage/testing.md +++ b/docs/usage/testing.md @@ -18,7 +18,7 @@ pytest tests/unit/test_concept_tools.py Integration tests that query the OMOP database require a `.env` file at the repo root (or an environment-variable equivalent): ```dotenv -OMOP_CDM_DB_URL=postgresql://user:pass@localhost:5432/omop +OA_CONFIG_PATH=/path/to/stack.toml ``` Unit tests mock the adapter layer and run without a live database. @@ -31,4 +31,4 @@ Unit tests mock the adapter layer and run without a live database. | Resolver tools | `concept_ground`: empty query rejection, result ordering by score, match_kind on exact hits | | Embedding tools | `embedding_index_status`, `embedding_neighbours`, `embedding_search`, `embedding_search_batch`, `embedding_encode`: correct responses, clamped limits, aligned batch rows, error propagation | | Server registry | `create_server` with and without each adapter; system tools always present; adapter-conditional tool registration | -| Config validation | `OmopGraphConfig` schema name validation; `OmopEmbConfig` enabled-without-url guard; extra field rejection | +| Config validation | oa-configurator stack validation, Groundworkers package fields, and extra field rejection | diff --git a/docs/usage/upgrading-from-0.6.md b/docs/usage/upgrading-from-0.6.md new file mode 100644 index 0000000..ea7586b --- /dev/null +++ b/docs/usage/upgrading-from-0.6.md @@ -0,0 +1,31 @@ +# Upgrading from Groundworkers 0.6 + +Groundworkers 0.7 uses the oa-configurator 2.0 stack and supports a vocabulary database separate from the primary CDM database. + +## Configuration changes + +- Rename the former database `schema_name` setting to `cdm_schema` for a CDM database. Use `vocab_schema` when vocabulary has a distinct schema. Generic databases, including vector-store databases, still use `schema_name`. +- SQLite has no schemas. Remove schema settings from SQLite database entries; setup ignores those fields and tells you when they are unavailable. +- A PostgreSQL vector store colocated with the CDM uses the dedicated `groundworkers` schema. Its database is a generic entry on the CDM connection: + + ```toml + [databases.groundworkers_vectors] + kind = "generic" + connection = "cdm_main" + schema_name = "groundworkers" + ``` + +- Add databases with the 2.0 CLI syntax, for example: + + ```bash + omop-config databases add cdm clinical --connection cdm_main --cdm-schema public + omop-config databases add generic vectors --connection cdm_main --schema-name groundworkers + ``` + +- Startup now fails fast if either the primary or vocabulary endpoint cannot be reached. The console prints the first scrubbed cause and a database recovery hint. +- A pre-3.0 embedding store reports `LEGACY_REGISTRY`. The migration script is tracked in oa-configurator issue #40; once it ships, Groundworkers will provide the omop-emb migration command. +- To split the vocabulary, set `vocab_connection` on the CDM database entry. Groundworkers 0.7 routes vocabulary reads to that connection, including setup diagnostics and graph searches. + +## Plugin changes + +See [Upgrading plugins to Groundworkers 0.7](../development/plugins.md#upgrading-plugins-to-groundworkers-07) for the full API checklist. Plugins should require `groundworkers>=0.7,<0.8`, pass `cdm_session_factory` for OMOP graph/vocabulary access, and treat `embedding_backend_factory()` as a read-only `EmbeddingStoreReader`. diff --git a/pyproject.toml b/pyproject.toml index caf9abc..254edaf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,11 +14,11 @@ dependencies = [ "psycopg[binary]>=3.1,<4", "python-dotenv>=1,<2", "psutil>=5.9,<8", - "oa-configurator>=1.3.0,<2", - "omop-alchemy>=1.1,<2", - "omop-graph>=2.1,<3", - "omop-emb>=2.1,<3", - "omop-llm>=1,<2", + "oa-configurator>=2.0.0,<3", + "omop-alchemy>=2.0.0,<3", + "omop-graph>=3.0.0,<4", + "omop-emb>=3.0.0,<4", + "omop-llm>=1.0.2,<2", "omop-semantics>=0.6,<1", "uvicorn[standard]>=0.29,<1.0", ] @@ -31,7 +31,7 @@ pdf = [ "pdfplumber>=0.10", ] tui = [ - "groundskeeping>=0.8.1,<1", + "groundskeeping>=0.9.1,<1", "textual>=0.60", "rich>=13,<15", ] @@ -48,10 +48,10 @@ all_source = [ "python-docx>=1.0", ] embedding-pgvector = [ - "omop-emb[pgvector]>=2.1,<3", + "omop-emb[pgvector]>=3.0.0,<4", ] embedding-faiss = [ - "omop-emb[faiss-cpu]>=2.1,<3", + "omop-emb[faiss-cpu]>=3.0.0,<4", ] dev = [ "httpx>=0.27,<1", diff --git a/src/groundworkers/adapters/cdm.py b/src/groundworkers/adapters/cdm.py index 0a1d49f..78d6c82 100644 --- a/src/groundworkers/adapters/cdm.py +++ b/src/groundworkers/adapters/cdm.py @@ -1,8 +1,10 @@ from __future__ import annotations -from sqlalchemy import text +from collections.abc import Callable + +from sqlalchemy import literal, select from sqlalchemy.engine import Engine -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import Session, sessionmaker class CDMAdapter: @@ -16,15 +18,32 @@ class CDMAdapter: Use session() for services that need a scoped session context manager. """ - def __init__(self, engine: Engine) -> None: + def __init__( + self, + engine: Engine, + *, + vocab_engine: Engine | None = None, + session_factory: Callable[[], Session] | None = None, + ) -> None: self._engine = engine - self._session_factory = sessionmaker(engine) + self._vocab_engine = vocab_engine or engine + self._session_factory = session_factory or sessionmaker(bind=engine) @property def engine(self) -> Engine: """The underlying SQLAlchemy engine.""" return self._engine + @property + def vocab_engine(self) -> Engine: + """The engine routed to the OMOP vocabulary role.""" + return self._vocab_engine + + @property + def session_factory(self) -> Callable[[], Session]: + """The session factory routed across the primary and vocabulary roles.""" + return self._session_factory + def session(self): """Return a session context manager.""" return self._session_factory() @@ -33,7 +52,7 @@ def is_available(self) -> bool: """Return True if the CDM database is reachable (SELECT 1 probe).""" try: with self._engine.connect() as conn: - conn.execute(text("SELECT 1")) + conn.execute(select(literal(1))) return True except Exception: return False @@ -41,3 +60,5 @@ def is_available(self) -> bool: def close(self) -> None: """Dispose the engine and release the connection pool.""" self._engine.dispose() + if self._vocab_engine is not self._engine: + self._vocab_engine.dispose() diff --git a/src/groundworkers/adapters/omop_emb.py b/src/groundworkers/adapters/omop_emb.py index 8ed1ae3..3bb9b4f 100644 --- a/src/groundworkers/adapters/omop_emb.py +++ b/src/groundworkers/adapters/omop_emb.py @@ -8,18 +8,18 @@ import numpy as np from omop_emb import ( - EmbeddingBackend, EmbeddingConceptFilter, EmbeddingModelRecord, EmbeddingReaderInterface, EmbeddingRole, MetricType, ) +from omop_emb.backends import EmbeddingStoreReader from omop_llm import ModelBackend -from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session from groundworkers.base.concept_payload import serialise_nearest_match -from groundworkers.base.errors import GroundworkersError +from groundworkers.base.errors import GroundworkersError, embedding_store_error from groundworkers.base.results import enum_value logger = logging.getLogger(__name__) @@ -36,11 +36,11 @@ class OmopEmbAdapter: def __init__( self, *, - backend_factory: Callable[[], EmbeddingBackend], + backend_factory: Callable[[], EmbeddingStoreReader], backend_type: str | None, default_model_name: str | None = None, model_backend_factory: Callable[[], ModelBackend] | None = None, - cdm_engine: Engine | None = None, + cdm_session_factory: Callable[[], Session] | None = None, faiss_cache_dir: str | None = None, configuration_detail: str | None = None, ) -> None: @@ -48,10 +48,10 @@ def __init__( self._backend_type = backend_type self._default_model_name = default_model_name self._model_backend_factory = model_backend_factory - self._cdm_engine = cdm_engine + self._cdm_session_factory = cdm_session_factory self._faiss_cache_dir = faiss_cache_dir self._configuration_detail = configuration_detail - self._backend: EmbeddingBackend | None = None + self._backend: EmbeddingStoreReader | None = None self._model_backend: ModelBackend | None = None self._reader_cache: dict[tuple[str, str, str], EmbeddingReaderInterface] = {} self._reader_cache_lock = threading.Lock() @@ -104,8 +104,12 @@ def close(self) -> None: with self._reader_cache_lock: self._reader_cache.clear() + backend = self._backend self._backend = None self._model_backend = None + close_backend = getattr(backend, "close", None) + if callable(close_backend): + close_backend() def resolve_model_name(self, model_name: str | None = None) -> str: """Resolve a caller-supplied or configured registered model name.""" @@ -117,14 +121,10 @@ def index_status(self) -> dict[str, Any]: try: backend = self._get_backend() - records = EmbeddingReaderInterface.list_registered_models(backend=backend) + records = backend.get_registered_models() models: list[dict[str, Any]] = [] for record in records: - metric_type = record.metric_type or MetricType.COSINE - concept_count = backend.get_embedding_count( - model_name=record.model_name, - metric_type=metric_type, - ) + concept_count = backend.get_embedding_count(model_name=record.model_name) models.append( { "model_name": record.model_name, @@ -147,8 +147,12 @@ def index_status(self) -> dict[str, Any]: except GroundworkersError as exc: detail = exc.message except Exception as exc: + translated = embedding_store_error(exc) + if translated is not None: + detail = translated.message + else: # Broad except: converted to a safe status category. - detail = f"Embedding store inspection failed with {type(exc).__name__}." + detail = f"Embedding store inspection failed with {type(exc).__name__}." return { "available": False, "backend_type": self._backend_type, @@ -181,6 +185,9 @@ def get_neighbours( "NOT_FOUND", f"Concept {concept_id} is not present in the embedding index", ) from exc + except Exception as exc: + _raise_embedding_store_error(exc) + raise if concept_id not in vectors: raise GroundworkersError( "NOT_FOUND", @@ -239,6 +246,7 @@ def search( faiss_index_config=record.index_config, ) except Exception as exc: + _raise_embedding_store_error(exc) logger.exception( "Embedding search failed for model %r", record.model_name, @@ -275,6 +283,7 @@ def encode(self, text: str, model_name: str | None) -> dict[str, Any]: "The configured embedding model returned invalid vectors.", ) from exc except Exception as exc: + _raise_embedding_store_error(exc) logger.exception( "Embedding encode failed for model %r", record.model_name, @@ -324,6 +333,7 @@ async def async_search( faiss_index_config=record.index_config, ) except Exception as exc: + _raise_embedding_store_error(exc) logger.exception( "Async embedding search failed for model %r", record.model_name, @@ -393,6 +403,7 @@ async def async_search_batch( except GroundworkersError: raise except Exception as exc: + _raise_embedding_store_error(exc) logger.exception( "Async batch embedding search failed for model %r", record.model_name, @@ -454,11 +465,14 @@ async def async_encode(self, text: str, model_name: str | None) -> dict[str, Any "vector": row.tolist(), } - def _get_backend(self) -> EmbeddingBackend: + def _get_backend(self) -> EmbeddingStoreReader: if self._backend is None: try: self._backend = self._backend_factory() except Exception as exc: + translated = embedding_store_error(exc) + if translated is not None: + raise translated from exc raise GroundworkersError( "BACKEND_UNAVAIL", "The configured embedding store is unavailable.", @@ -468,10 +482,13 @@ def _get_backend(self) -> EmbeddingBackend: def _resolve_model_record(self, model_name: str | None) -> EmbeddingModelRecord: backend = self._get_backend() requested_name = model_name or self._default_model_name - records = EmbeddingReaderInterface.list_registered_models( - backend=backend, - model_name=requested_name, - ) + try: + records = backend.get_registered_models(model_name=requested_name) + except Exception as exc: + translated = embedding_store_error(exc) + if translated is not None: + raise translated from exc + raise if requested_name is not None: if not records: raise GroundworkersError( @@ -500,14 +517,18 @@ def _build_reader(self, record: EmbeddingModelRecord) -> EmbeddingReaderInterfac if cached is not None: return cached - reader = EmbeddingReaderInterface( - model=record.model_name, - backend=self._get_backend(), - metric_type=metric_type, - omop_cdm_engine=self._cdm_engine, - provider_type=provider_type, - faiss_cache_dir=self._faiss_cache_dir, - ) + try: + reader = EmbeddingReaderInterface( + model=record.model_name, + backend=self._get_backend(), + metric_type=metric_type, + cdm_session_factory=self._cdm_session_factory, + provider_type=provider_type, + faiss_cache_dir=self._faiss_cache_dir, + ) + except Exception as exc: + _raise_embedding_store_error(exc) + raise self._reader_cache[cache_key] = reader return reader @@ -555,7 +576,7 @@ def _build_concept_filter( ) @staticmethod - def _backend_type_from_backend(backend: EmbeddingBackend) -> str | None: + def _backend_type_from_backend(backend: EmbeddingStoreReader) -> str | None: return enum_value(getattr(backend, "backend_type", None)) @staticmethod @@ -571,3 +592,9 @@ def _embedding_array(vectors: object, *, expected_rows: int) -> np.ndarray: if result.shape[0] != expected_rows: raise RuntimeError(f"Expected {expected_rows} embeddings, got {result.shape[0]}") return result + + +def _raise_embedding_store_error(exc: BaseException) -> None: + translated = embedding_store_error(exc) + if translated is not None: + raise translated from exc diff --git a/src/groundworkers/adapters/omop_graph.py b/src/groundworkers/adapters/omop_graph.py index b603481..07ec549 100644 --- a/src/groundworkers/adapters/omop_graph.py +++ b/src/groundworkers/adapters/omop_graph.py @@ -11,7 +11,7 @@ Domain, Vocabulary, ) -from omop_graph.extensions.omop_alchemy import PredicateKind +from omop_graph.extensions.omop_alchemy import PredicateKind, RelationshipMapping from omop_graph.graph.kg import KnowledgeGraph, KnowledgeGraphEmbeddingConfiguration from omop_graph.graph.nodes import LabelMatchKind from omop_graph.graph.paths import find_shortest_paths_batch @@ -19,9 +19,10 @@ from omop_graph.reasoning.grounding import GroundingConstraints, ground_term from omop_graph.reasoning.resolvers import ResolverPipeline from omop_graph.reasoning.resolvers.resolvers import EmbeddingResolver -from sqlalchemy import func, select, text +from sqlalchemy import func, select from sqlalchemy.engine import Engine -from sqlalchemy.exc import NoResultFound +from sqlalchemy.exc import DBAPIError, NoResultFound +from sqlalchemy.orm import Session, sessionmaker from groundworkers.base.concept_payload import serialise_concept_view from groundworkers.base.domain_names import DomainNameResolver @@ -30,7 +31,7 @@ if TYPE_CHECKING: import numpy as np from oa_configurator import ResolvedModel - from omop_emb import EmbeddingBackend + from omop_emb.backends import EmbeddingStoreReader from omop_llm import ModelBackend logger = logging.getLogger(__name__) @@ -63,24 +64,24 @@ class OmopGraphAdapter: def __init__( self, - engine: Engine, + engine: Engine | Callable[[], Session], *, - embedding_backend_factory: Callable[[], EmbeddingBackend] | None = None, + cdm_session_factory: Callable[[], Session] | None = None, + embedding_backend_factory: Callable[[], EmbeddingStoreReader] | None = None, resolved_embedding_model: ResolvedModel | None = None, model_backend_factory: Callable[[], ModelBackend] | None = None, embedding_metric: str = "cosine", faiss_cache_dir: str | None = None, ) -> None: - # No `vocab_schema` parameter. It cannot be honoured and never was: no - # omop-alchemy model declares `schema="vocab"`, so oa-configurator's - # "vocab" translate-map key is unreachable from the ORM and vocabulary - # tables resolve through the primary schema like every other CDM table - # (see omop_alchemy/config.py, `vocabulary_identity`). The engine handed - # in already carries the full map, so if omop-alchemy ever tags its - # vocabulary models the routing starts working here with no change. - # Re-adding the parameter would restore a caller-visible argument that - # silently changes nothing. - self.engine = engine + self._engine_compatibility_mode = isinstance(engine, Engine) and cdm_session_factory is None + if isinstance(engine, Engine): + self.engine: Engine | None = engine + self._cdm_session_factory = cdm_session_factory or sessionmaker( + bind=engine + ) + else: + self.engine = None + self._cdm_session_factory = cdm_session_factory or engine self._embedding_backend_factory = embedding_backend_factory self._resolved_embedding_model = resolved_embedding_model self._model_backend_factory = model_backend_factory @@ -90,7 +91,9 @@ def __init__( self._embedding_configured = False self._kg: KnowledgeGraph | None = None self._model_backend: ModelBackend | None = None - self._domain_names = DomainNameResolver(engine) + self._domain_names = DomainNameResolver( + engine if self._engine_compatibility_mode else self._cdm_session_factory + ) @property def embedding_resolver_active(self) -> bool: @@ -565,9 +568,20 @@ def _get_kg(self) -> KnowledgeGraph: # Fail fast with a clear message if the database is unreachable before # KnowledgeGraph has a chance to raise something opaque. try: - with self.engine.connect() as conn: - conn.execute(text("SELECT 1")) + with self._cdm_session_factory() as session: + session.execute(select(Concept.concept_id).limit(1)) + session.execute( + select(RelationshipMapping.relationship_id).limit(1), + bind_arguments={"mapper": RelationshipMapping}, + ) except Exception as exc: + if self._engine_compatibility_mode: + raise GroundworkersError( + "BACKEND_UNAVAIL", + "This deployment keeps the vocabulary on its own database; " + "pass cdm_session_factory (PluginContext.cdm_session_factory) " + "when constructing OmopGraphAdapter.", + ) from exc raise GroundworkersError( "BACKEND_UNAVAIL", f"Cannot connect to database ({type(exc).__name__}).", @@ -598,7 +612,9 @@ def _get_kg(self) -> KnowledgeGraph: logger.warning("%s", self._embedding_configuration_error) try: - self._kg = KnowledgeGraph(cdm_engine=self.engine, emb_config=emb_config) + self._kg = KnowledgeGraph( + self._cdm_session_factory, emb_config=emb_config + ) except Exception as exc: if emb_config is None: self._embedding_configured = False @@ -610,7 +626,7 @@ def _get_kg(self) -> KnowledgeGraph: self._embedding_configuration_error = self._embedding_failure_detail(exc) logger.warning("%s", self._embedding_configuration_error) try: - self._kg = KnowledgeGraph(cdm_engine=self.engine, emb_config=None) + self._kg = KnowledgeGraph(self._cdm_session_factory, emb_config=None) except Exception as lexical_exc: raise self._wrap_graph_error(lexical_exc, default_code="BACKEND_UNAVAIL") return self._kg @@ -683,6 +699,16 @@ def _is_not_found(exc: Exception) -> bool: def _wrap_graph_error(exc: Exception, *, default_code: str) -> GroundworkersError: if isinstance(exc, GroundworkersError): return exc + current: BaseException | None = exc + seen: set[int] = set() + while current is not None and id(current) not in seen: + seen.add(id(current)) + if isinstance(current, DBAPIError): + return GroundworkersError( + "BACKEND_UNAVAIL", + f"The graph database is unavailable ({type(current).__name__}).", + ) + current = current.__cause__ or current.__context__ msg = str(exc) if "relationship classification" in msg or "relationship_mapping" in msg: return GroundworkersError( diff --git a/src/groundworkers/app.py b/src/groundworkers/app.py index eb224fd..f4e26a3 100644 --- a/src/groundworkers/app.py +++ b/src/groundworkers/app.py @@ -36,7 +36,7 @@ ) if TYPE_CHECKING: - from omop_emb import EmbeddingBackend + from omop_emb.backends import EmbeddingStoreReader from omop_llm import ModelBackend logger = logging.getLogger(__name__) @@ -85,22 +85,24 @@ class GroundworkersApp: def build_adapters(config: AppConfig) -> Adapters: adapters = Adapters() - adapters.cdm = CDMAdapter(config.cdm_engine) + adapters.cdm = CDMAdapter( + config.cdm_engine, + vocab_engine=config.cdm_vocab_engine, + session_factory=config.cdm_session_factory, + ) resolved_store = config.vector_store resolved_model = config.embedding_model - shared_backend: EmbeddingBackend | None = None + shared_backend: EmbeddingStoreReader | None = None - def get_embedding_backend() -> EmbeddingBackend: + def get_embedding_backend() -> EmbeddingStoreReader: nonlocal shared_backend if shared_backend is None: if resolved_store is None: raise RuntimeError("No vector store is configured.") - from omop_emb.backends import ( - resolve_backend_from_resolved_vector_store, - ) + from omop_emb.backends import open_vector_store_reader - shared_backend = resolve_backend_from_resolved_vector_store(resolved_store) + shared_backend = open_vector_store_reader(resolved_store) return shared_backend shared_model_backend: ModelBackend | None = None @@ -129,7 +131,7 @@ def get_model_backend() -> ModelBackend: backend_type=resolved_store.backend_type, default_model_name=(resolved_model.model if resolved_model else None), model_backend_factory=(get_model_backend if resolved_model else None), - cdm_engine=config.cdm_engine, + cdm_session_factory=config.cdm_session_factory, faiss_cache_dir=resolved_store.faiss_cache_dir, configuration_detail=configuration_detail, ) @@ -149,6 +151,7 @@ def get_model_backend() -> ModelBackend: complete_embedding = resolved_store is not None and resolved_model is not None adapters.omop_graph = OmopGraphAdapter( engine=config.cdm_engine, + cdm_session_factory=config.cdm_session_factory, embedding_backend_factory=( get_embedding_backend if complete_embedding else None ), @@ -251,6 +254,8 @@ def _build_plugin_context( resolver=PluginConfigResolver(Resolver(config.stack)), cdm_database=config.cdm_database, cdm_engine=config.cdm_engine, + cdm_session_factory=config.cdm_session_factory, + cdm_vocab_engine=config.cdm_vocab_engine, vector_store=config.vector_store, embedding_backend_factory=adapters.embedding_backend_factory, embedding_model_backend_factory=adapters.embedding_model_backend_factory, diff --git a/src/groundworkers/application/setup/configuration.py b/src/groundworkers/application/setup/configuration.py index 86113de..e9e2c46 100644 --- a/src/groundworkers/application/setup/configuration.py +++ b/src/groundworkers/application/setup/configuration.py @@ -1,4 +1,4 @@ -"""Load and persist the authoritative oa-configurator 1.x stack.""" +"""Load and persist the authoritative oa-configurator 2.0 stack.""" from __future__ import annotations @@ -21,7 +21,7 @@ ConfigurationState, SetupIssue, ) -from groundworkers.config import GroundworkersConfig, split_vocabulary_connection +from groundworkers.config import GroundworkersConfig class ConfigurationConflictError(RuntimeError): @@ -127,15 +127,7 @@ def save_configuration( "This configuration is read-only; use its controlling source to edit it." ) validated = StackConfig.model_validate(stack.model_dump(mode="python")) - groundworkers = GroundworkersConfig.validate_candidate(validated) - split = split_vocabulary_connection(validated, groundworkers.cdm_db) - if split is not None: - primary, vocabulary = split - raise ValueError( - f"The CDM database reads its vocabulary from connection {vocabulary!r} and " - f"its clinical tables from {primary!r}. Groundworkers uses one connection " - "for both; separate them with vocab_schema instead." - ) + GroundworkersConfig.validate_candidate(validated) destination = Path(path).expanduser().resolve() existed = destination.exists() if existed: @@ -167,7 +159,7 @@ def save_configuration( def _incomplete_issues(stack: StackConfig) -> tuple[SetupIssue, ...]: try: - groundworkers = GroundworkersConfig.validate_candidate(stack) + GroundworkersConfig.validate_candidate(stack) except (ConfigurationError, ValidationError, ValueError) as exc: return ( SetupIssue( @@ -181,20 +173,6 @@ def _incomplete_issues(stack: StackConfig) -> tuple[SetupIssue, ...]: ) # Reference-valid but unrunnable: the runtime refuses this at bootstrap, so # the console must not present it as merely unverified. - split = split_vocabulary_connection(stack, groundworkers.cdm_db) - if split is not None: - primary, vocabulary = split - return ( - SetupIssue( - code="vocabulary_connection_split", - field=f"databases.{groundworkers.cdm_db}.vocab_connection", - message=( - f"The CDM database reads its vocabulary from connection {vocabulary!r} " - f"and its clinical tables from {primary!r}. Groundworkers uses one " - "connection for both; separate them with vocab_schema instead." - ), - ), - ) return () diff --git a/src/groundworkers/application/setup/configuration_provider.py b/src/groundworkers/application/setup/configuration_provider.py index f17021b..57ff84f 100644 --- a/src/groundworkers/application/setup/configuration_provider.py +++ b/src/groundworkers/application/setup/configuration_provider.py @@ -48,6 +48,7 @@ StackConfig, is_sensitive, plan_configure, + supports_schemas, ) from omop_llm import canonical_model_name, supported_providers from pydantic import BaseModel, ValidationError @@ -67,6 +68,7 @@ EmbeddingStoreSnapshot, EmbeddingStoreState, ) +from groundworkers.base.errors import scrub_error_message from groundworkers.config import GroundworkersConfig CDM_SETUP_TARGET: Final = ConfigTarget( @@ -110,12 +112,13 @@ MODEL_SOURCE_NEW: Final = "new" # Where the vectors are stored. `cdm` derives the store's database from the CDM -# entry -- same connection, same schema -- so the ordinary case needs no answers +# entry -- same connection and dedicated schema -- so the ordinary case needs no answers # at all; `separate` opens the step that asks for them. oa-configurator requires # a vector store to name a *generic* database entry and a CDM entry is a distinct # kind, so `cdm` still writes an entry; it just derives every field of it. STORE_LOCATION_CDM: Final = "cdm" STORE_LOCATION_SEPARATE: Final = "separate" +GROUNDWORKERS_SCHEMA: Final = "groundworkers" # Suffix for the derived entry. The name is a config-level label only: the # connection and schema underneath it are the CDM's own. CDM_VECTOR_DATABASE_SUFFIX: Final = "_vectors" @@ -315,14 +318,17 @@ def submit( # choice with what the store actually holds. future_fields = (self._model_source_field(session),) else: + choices, default, choice_help = _model_choice_options( + session.fields, "model_choice", models + ) future_fields = ( FieldSpec( key=choice_key, label="Model", kind=FieldKind.CHOICE, - choices=tuple(ChoiceOption(name, name) for name in models), - default=models[0], - help="Models reported by the accepted provider endpoint.", + choices=choices, + default=default, + help=choice_help, ), ) elif session.target == VECTOR_STORE_SETUP_TARGET and step_key == "location": @@ -397,16 +403,19 @@ def _model_choice_field( names = ( session.registered_models if use_registered else session.provider_models ) + choices, default, choice_help = _model_choice_options( + session.fields, "model_choice", names + ) return FieldSpec( key="model_choice", label="Model", kind=FieldKind.CHOICE, - choices=tuple(ChoiceOption(name, name) for name in names), - default=names[0] if names else None, + choices=choices, + default=default, help=( "Registered in the embedding store, so vectors already exist." if use_registered - else "Reported by the provider. Populate embeddings after saving." + else choice_help ), ) @@ -415,14 +424,16 @@ def plan(self, draft: ConfigDraft) -> ConfigPlan: self._invalidate_plan(session) try: candidate = self._candidate(session) - except (ConfigurationError, ValidationError, ValueError, TypeError): + except (ConfigurationError, ValidationError, ValueError, TypeError) as exc: + validation_detail = scrub_error_message(" ".join(str(exc).splitlines())) return ConfigPlan( target=session.target, operation=session.operation, diff=ConfigDiff(session.target, ()), issues=( ValidationIssue( - "The proposed configuration is incomplete or invalid." + validation_detail + or "The proposed configuration is incomplete or invalid." ), ), expected_revision=session.expected_revision, @@ -805,7 +816,7 @@ def cdm_setup_workflow( ConfigWorkflowStep( "cdm", "Describe the CDM", - ("cdm_db_name", "schema_name", "vocab_schema", "results_schema"), + ("cdm_db_name", "cdm_schema", "vocab_schema", "results_schema"), ), ), ) @@ -865,7 +876,7 @@ def vector_store_setup_workflow( Location is asked first and defaults to the CDM database, because that is what the rest of the tier already assumes: the embedding adapter enriches - from ``cdm_engine`` and coverage counts the CDM's own concepts. Somewhere + from the routed CDM session factory and coverage counts the CDM's own concepts. Somewhere else is a real option, but it is the opt-in, and answering it is the only reason the database step exists. """ @@ -934,7 +945,6 @@ def _cdm_fields(stack: StackConfig) -> tuple[FieldSpec, ...]: database = stack.databases.get(cdm_name) connection_name = getattr(database, "connection", "cdm_main") connection = stack.connections.get(connection_name) - fresh = connection is None dialect = getattr(connection, "dialect", "postgresql+psycopg") return ( FieldSpec( @@ -995,20 +1005,43 @@ def _cdm_fields(stack: StackConfig) -> tuple[FieldSpec, ...]: help="Keep the generated name unless reusing or resolving a collision.", ), FieldSpec( - "schema_name", + "cdm_schema", "CDM schema", - default=getattr(database, "schema_name", "public" if fresh else "main"), + required=False, + disabled=not _supports_schema_dialect(dialect), + help=( + "SQLite has no schemas; this value is ignored." + if not _supports_schema_dialect(dialect) + else None + ), + default=( + getattr(database, "cdm_schema", None) + if database is not None + else ("public" if _supports_schema_dialect(dialect) else None) + ), ), FieldSpec( "vocab_schema", "Vocabulary schema", required=False, + disabled=not _supports_schema_dialect(dialect), + help=( + "SQLite has no schemas; this value is ignored." + if not _supports_schema_dialect(dialect) + else None + ), default=getattr(database, "vocab_schema", None), ), FieldSpec( "results_schema", "Results schema", required=False, + disabled=not _supports_schema_dialect(dialect), + help=( + "SQLite has no schemas; this value is ignored." + if not _supports_schema_dialect(dialect) + else None + ), default=getattr(database, "results_schema", None), ), ) @@ -1122,6 +1155,15 @@ def _vector_store_fields(stack: StackConfig) -> tuple[FieldSpec, ...]: _, cdm = _cdm_entry(stack) connections = tuple(stack.connections) or ("cdm_main",) location = _store_location_field(stack, database) + store_connection_name = ( + connection_name or getattr(cdm, "connection", None) or connections[0] + ) + store_connection = stack.connections.get(store_connection_name) + store_schema = getattr(database, "schema_name", None) + if store_schema is None and _supports_schema_dialect( + getattr(store_connection, "dialect", None) + ): + store_schema = GROUNDWORKERS_SCHEMA return ( location, FieldSpec( @@ -1149,9 +1191,7 @@ def _vector_store_fields(stack: StackConfig) -> tuple[FieldSpec, ...]: "store_connection_name", "Connection", kind=FieldKind.CHOICE, - default=connection_name - or getattr(cdm, "connection", None) - or connections[0], + default=store_connection_name, choices=tuple(ChoiceOption(name, name) for name in connections), help="An existing connection. Add one through CDM setup or omop-config.", ), @@ -1159,13 +1199,17 @@ def _vector_store_fields(stack: StackConfig) -> tuple[FieldSpec, ...]: "store_schema_name", "Schema", required=False, - # No hardcoded fallback: 'public' is a PostgreSQL name, and it was - # being offered for SQLite stores and for CDMs whose own schema is - # something else. Blank means no override, which is right whenever - # there is nothing better to copy. - default=getattr(database, "schema_name", None) - or getattr(cdm, "schema_name", None), - help="Blank uses the connection's own default schema.", + disabled=not _supports_schema_dialect( + getattr(store_connection, "dialect", None) + ), + help=( + "SQLite has no schemas; this value is ignored." + if not _supports_schema_dialect( + getattr(store_connection, "dialect", None) + ) + else "Blank uses the connection's own default schema." + ), + default=store_schema, ), FieldSpec( "faiss_cache_dir", @@ -1214,7 +1258,7 @@ def _store_location_field( ) stored = ( STORE_LOCATION_CDM - if database is not None and _matches_cdm(database, cdm) + if database is not None and _matches_cdm(database, cdm, stack) else STORE_LOCATION_SEPARATE if database is not None else STORE_LOCATION_CDM @@ -1238,11 +1282,18 @@ def _store_location_field( ) -def _matches_cdm(database: object, cdm: CDMDatabaseConfig) -> bool: - return ( - getattr(database, "connection", None) == cdm.connection - and getattr(database, "schema_name", None) == cdm.schema_name +def _matches_cdm( + database: object, cdm: CDMDatabaseConfig, stack: StackConfig +) -> bool: + if getattr(database, "connection", None) != cdm.connection: + return False + connection = stack.connections.get(cdm.connection) + schema = ( + GROUNDWORKERS_SCHEMA + if _supports_schema_dialect(getattr(connection, "dialect", None)) + else None ) + return getattr(database, "schema_name", None) == schema def _backend_type_field( @@ -1388,12 +1439,15 @@ def _cdm_set_dict(answers: Mapping[str, object]) -> dict[str, object]: database: dict[str, object] = { "name": str(answers["cdm_db_name"]), "connection": connection, - "schema_name": str(answers["schema_name"]), } - for key in ("vocab_schema", "results_schema"): - value = answers.get(key) - if value not in (None, ""): - database[key] = value + if _supports_schema_dialect(str(answers["dialect"])): + schema = answers.get("cdm_schema") + if schema not in (None, ""): + database["cdm_schema"] = schema + for key in ("vocab_schema", "results_schema"): + value = answers.get(key) + if value not in (None, ""): + database[key] = value return {"cdm_db": database} @@ -1426,8 +1480,8 @@ def _store_database( A vector store must reference a ``GenericDatabaseConfig`` and the CDM entry is a ``CDMDatabaseConfig``, so "in the CDM database" cannot be expressed by pointing at the CDM entry itself. It is expressed instead as a generic entry - that copies the CDM's connection and schema, which is the same physical - place. + on the CDM's connection. PostgreSQL stores vectors in a dedicated + ``groundworkers`` schema; SQLite has no schema. """ if _store_location(answers) == STORE_LOCATION_CDM: cdm_name, cdm = _cdm_entry(stack) @@ -1440,8 +1494,9 @@ def _store_database( "kind": "generic", "connection": cdm.connection, } - if cdm.schema_name: - database["schema_name"] = cdm.schema_name + cdm_connection = stack.connections.get(cdm.connection) + if _supports_schema_dialect(getattr(cdm_connection, "dialect", None)): + database["schema_name"] = GROUNDWORKERS_SCHEMA return database database = { "name": str(answers["store_database_name"]), @@ -1449,7 +1504,10 @@ def _store_database( "connection": str(answers["store_connection_name"]), } schema = answers.get("store_schema_name") - if schema not in (None, ""): + connection = stack.connections.get(str(answers["store_connection_name"])) + if schema not in (None, "") and _supports_schema_dialect( + getattr(connection, "dialect", None) + ): database["schema_name"] = schema return database @@ -1952,6 +2010,31 @@ def _prefix_text(value: object) -> str | None: return text or None +def _supports_schema_dialect(dialect: object) -> bool: + if not isinstance(dialect, str): + return False + try: + return supports_schemas(dialect.split("+", maxsplit=1)[0]) + except ValueError: + return False + + +def _model_choice_options( + fields: tuple[FieldSpec, ...], key: str, discovered: Sequence[str] +) -> tuple[tuple[ChoiceOption, ...], str | None, str]: + stored = next((field.default for field in fields if field.key == key), None) + stored = stored if isinstance(stored, str) else None + options = tuple(ChoiceOption(name, name) for name in discovered) + if stored and stored not in discovered: + options = (ChoiceOption(stored, f"{stored} (stored; not discovered)"), *options) + return options, stored, ( + f"Stored model {stored!r} was not reported by the provider. It remains selected; verify it before changing." + ) + return options, stored or (discovered[0] if discovered else None), ( + "Models reported by the accepted provider endpoint. Populate embeddings after saving to use a newly selected model." + ) + + def _optional_text(value: object) -> str | None: if value is None: return None diff --git a/src/groundworkers/application/setup/databases.py b/src/groundworkers/application/setup/databases.py index 39a2798..5367ccc 100644 --- a/src/groundworkers/application/setup/databases.py +++ b/src/groundworkers/application/setup/databases.py @@ -7,21 +7,31 @@ from __future__ import annotations import errno +import logging import socket import time -from collections.abc import Callable +from collections.abc import Callable, Mapping from typing import Any from oa_configurator import ( # type: ignore[import-untyped] ResolvedCDMDatabase, Resolver, + Role, + physical_schema_of, + supports_schemas, ) -from omop_emb.backends.embedding_table import ( - CONCEPT_METADATA_COLUMNS, - EMBEDDING_COLUMN_NAME, +from omop_alchemy.config import create_cdm_engines +from sqlalchemy import ( + Column, + MetaData, + Table, + create_engine, + exists, + func, + inspect, + literal, + select, ) -from omop_emb.model_registry import ModelRegistry -from sqlalchemy import create_engine, inspect, select, text from sqlalchemy.engine import Connection, Engine from sqlalchemy.exc import NoSuchModuleError from sqlalchemy.exc import TimeoutError as SQLAlchemyTimeoutError @@ -35,9 +45,9 @@ DiagnosticSeverity, ResourceDiagnostic, ) +from groundworkers.base.errors import embedding_store_error from groundworkers.base.sql import ( normalization_key_expression_sql, - quote_identifier, ) from groundworkers.config import GroundworkersConfig @@ -48,6 +58,7 @@ "concept_synonym", "relationship", ) +logger = logging.getLogger(__name__) GRAPH_TABLES = ("relationship_class", "relationship_mapping") GRAPH_FTS_COLUMNS = { "concept": "concept_name_tsvector", @@ -108,16 +119,6 @@ "idx_concept_synonym_name_normkey", ), ) -# Taken from omop-emb rather than restated: these describe tables omop-emb owns -# and creates, so a column added there must not need a matching edit here to stay -# checkable. -EMBEDDING_REGISTRY_TABLE = ModelRegistry.__tablename__ -EMBEDDING_REQUIRED_COLUMNS = ( - *(column.name for column in CONCEPT_METADATA_COLUMNS), - EMBEDDING_COLUMN_NAME, -) - - def resolve_database_targets( snapshot: ConfigurationSnapshot, ) -> tuple[DatabaseTarget, ...]: @@ -131,14 +132,11 @@ def resolve_database_targets( cdm = resolver.resolve_database(groundworkers.cdm_db) if not isinstance(cdm, ResolvedCDMDatabase): return () - cdm_schema = cdm.schema_name or ( - "main" if cdm.connection.url.startswith("sqlite") else "public" - ) + schema_capable = supports_schemas(cdm.connection.dialect_name) + cdm_schema = cdm.schema_for_role(Role.PRIMARY) if schema_capable else None + vocab_schema = cdm.schema_for_role(Role.VOCAB) if schema_capable else None embedding_target: DatabaseTarget | None = None expected_embedding_model_name: str | None = None - embedding_safe_url: str | None = None - embedding_connection_url: str | None = None - embedding_schema: str | None = None if groundworkers.embedding_model_name is not None: expected_embedding_model_name = resolver.resolve_model( @@ -146,20 +144,17 @@ def resolve_database_targets( ).model if groundworkers.vector_store_name is not None: embedding = resolver.resolve_vector_store(groundworkers.vector_store_name) - embedding_safe_url = embedding.database.connection.safe_url - embedding_connection_url = embedding.database.connection.url - embedding_schema = embedding.database.schema_name embedding_target = DatabaseTarget( key="database.embedding", label="Embedding store", database_entry_name=embedding.database.name, connection_name=embedding.database.connection.name, safe_url=embedding.database.connection.safe_url, - cdm_schema=embedding_schema or "main", - vocabulary_schema=embedding_schema or "main", + cdm_schema=None, + vocabulary_schema=None, connection_url=embedding.database.connection.url, role="embedding", - embedding_schema=embedding_schema, + resolved_vector_store=embedding, ) targets = [ @@ -170,9 +165,10 @@ def resolve_database_targets( connection_name=cdm.connection.name, safe_url=cdm.connection.safe_url, cdm_schema=cdm_schema, - vocabulary_schema=cdm.vocab_schema, + vocabulary_schema=vocab_schema, connection_url=cdm.connection.url, role="cdm", + resolved_cdm_database=cdm, ), DatabaseTarget( key="database.graph", @@ -181,9 +177,10 @@ def resolve_database_targets( connection_name=cdm.connection.name, safe_url=cdm.connection.safe_url, cdm_schema=cdm_schema, - vocabulary_schema=cdm.vocab_schema, + vocabulary_schema=vocab_schema, connection_url=cdm.connection.url, role="graph", + resolved_cdm_database=cdm, ), DatabaseTarget( key="database.groundworkers", @@ -192,18 +189,18 @@ def resolve_database_targets( connection_name=cdm.connection.name, safe_url=cdm.connection.safe_url, cdm_schema=cdm_schema, - vocabulary_schema=cdm.vocab_schema, + vocabulary_schema=vocab_schema, connection_url=cdm.connection.url, role="groundworkers", - embedding_schema=embedding_schema, expected_embedding_model_name=expected_embedding_model_name, - embedding_safe_url=embedding_safe_url, - embedding_connection_url=embedding_connection_url, + resolved_vector_store=( + embedding_target.resolved_vector_store + if embedding_target is not None + else None + ), + resolved_cdm_database=cdm, ), ] - # No separate vocabulary target: a CDM entry naming a second connection for - # its vocabulary is rejected before it gets here, by `split_vocabulary_connection` - # via `_incomplete_issues`, so `cdm.vocab_connection` is always `cdm.connection`. if embedding_target is not None: targets.append(embedding_target) return tuple(targets) @@ -217,17 +214,19 @@ def verify_database_target( ) -> ConnectionResult: """Execute a bounded read-only liveness query against one target.""" + if ( + target.resolved_cdm_database is not None + and target.role in {"cdm", "graph", "groundworkers"} + and engine_factory is create_engine + ): + return _verify_cdm_target(target, clock=clock) engine: Engine | None = None started = clock() try: engine = engine_factory(target.connection_url) with engine.connect() as connection: - connection.execute(text("SELECT 1")) - diagnostics = _diagnostics_for_target( - connection, - target, - engine_factory=engine_factory, - ) + connection.execute(select(literal(1))) + diagnostics = _diagnostics_for_target(connection, connection, target) except Exception as exc: # Broad except: returned as a redacted category. return ConnectionResult( @@ -249,31 +248,97 @@ def verify_database_target( ) -def _diagnostics_for_target( - connection: Connection, +def _verify_cdm_target( target: DatabaseTarget, *, - engine_factory: Callable[[str], Engine], + clock: Callable[[], float], +) -> ConnectionResult: + engines: tuple[Engine, Engine] | None = None + started = clock() + try: + resolved_cdm_database = target.resolved_cdm_database + if resolved_cdm_database is None: + raise ValueError("CDM database configuration is missing") + engines = create_cdm_engines( + resolved_cdm_database, register_claims=False + ) + primary_engine, vocab_engine = engines + with primary_engine.connect() as primary, vocab_engine.connect() as vocab: + try: + primary.execute(select(literal(1))) + except Exception as exc: + raise RuntimeError("Primary database role is unavailable") from exc + try: + vocab.execute(select(literal(1))) + except Exception as exc: + raise RuntimeError("Vocabulary database role is unavailable") from exc + diagnostics = _diagnostics_for_target(primary, vocab, target) + except Exception as exc: + logger.warning("Database target %s verification failed", target.key, exc_info=True) + failure = classify_connection_error(exc) + lowered_chain = " ".join(str(item).lower() for item in _exception_chain(exc)) + role_label = next( + ( + role + for role in ("vocabulary", "primary") + if f"{role} database role is unavailable" in lowered_chain + ), + None, + ) + if role_label is not None: + failure = _failure( + failure.kind, + f"The {role_label} database role is unavailable. {failure.detail}", + f"Check the {role_label} database connection. {failure.next_action}", + ) + return ConnectionResult( + target_key=target.key, + connected=False, + latency_ms=None, + safe_url=target.safe_url, + failure=failure, + ) + finally: + if engines is not None: + seen: set[int] = set() + for engine in engines: + if id(engine) not in seen: + seen.add(id(engine)) + engine.dispose() + return ConnectionResult( + target_key=target.key, + connected=True, + latency_ms=round((clock() - started) * 1000, 1), + safe_url=target.safe_url, + diagnostics=tuple(diagnostics), + ) + + +def _diagnostics_for_target( + primary: Connection, + vocab: Connection, + target: DatabaseTarget, ) -> tuple[ResourceDiagnostic, ...]: if target.role == "embedding": - return _embedding_diagnostics(connection, schema=target.embedding_schema) + return _embedding_diagnostics(target) if target.role == "graph": - return _graph_diagnostics(connection, target.cdm_schema) + return _graph_diagnostics( + primary, vocab, target.cdm_schema, target.vocabulary_schema + ) if target.role == "groundworkers": return _groundworkers_diagnostics( - connection, - target, - engine_factory=engine_factory, + vocab, target ) if target.role == "cdm": - return _cdm_diagnostics(connection, target.cdm_schema) + return _cdm_diagnostics(vocab, target.vocabulary_schema) return () def _cdm_diagnostics( connection: Connection, - schema: str, + schema: str | None, ) -> tuple[ResourceDiagnostic, ...]: + schema = physical_schema_of(connection, schema_tag=Role.VOCAB) inspector = inspect(connection) existing = set(inspector.get_table_names(schema=schema)) missing = tuple(table for table in CDM_TABLES if table not in existing) @@ -293,12 +358,20 @@ def _cdm_diagnostics( def _graph_diagnostics( - connection: Connection, - schema: str, + primary: Connection, + vocab: Connection, + cdm_schema: str | None, + vocabulary_schema: str | None, ) -> tuple[ResourceDiagnostic, ...]: - diagnostics: list[ResourceDiagnostic] = list(_cdm_diagnostics(connection, schema)) - inspector = inspect(connection) - existing_tables = set(inspector.get_table_names(schema=schema)) + cdm_schema = physical_schema_of(primary, schema_tag=Role.PRIMARY) + vocabulary_schema = physical_schema_of(vocab, schema_tag=Role.VOCAB) + diagnostics: list[ResourceDiagnostic] = list( + _cdm_diagnostics(vocab, vocabulary_schema) + ) + primary_inspector = inspect(primary) + vocab_inspector = inspect(vocab) + existing_tables = set(primary_inspector.get_table_names(schema=cdm_schema)) + vocabulary_tables = set(vocab_inspector.get_table_names(schema=vocabulary_schema)) missing_graph_tables = tuple( table for table in GRAPH_TABLES if table not in existing_tables ) @@ -307,20 +380,22 @@ def _graph_diagnostics( _warning( "graph_tables_missing", "Graph relationship-classification tables are missing from " - f"schema {schema!r}: {', '.join(missing_graph_tables)}.", + f"schema {cdm_schema!r}: {', '.join(missing_graph_tables)}.", ) ) else: diagnostics.append( _info( "graph_tables_present", - f"Graph relationship-classification tables are present in schema {schema!r}.", + f"Graph relationship-classification tables are present in schema {cdm_schema!r}.", ) ) empty_tables = tuple( table for table in GRAPH_TABLES - if _table_is_empty(connection, schema, table) + if _table_is_empty( + primary, cdm_schema, table, schema_tag=Role.PRIMARY + ) ) if empty_tables: diagnostics.append( @@ -331,69 +406,85 @@ def _graph_diagnostics( ) ) - fulltext_sidecar_unpopulated: list[str] = [] - for table, column in GRAPH_FTS_COLUMNS.items(): - if table not in existing_tables: - continue - columns = _column_names(connection, schema, table) - if column not in columns: + if vocab.dialect.name == "postgresql": + fulltext_sidecar_unpopulated: list[str] = [] + for table, column in GRAPH_FTS_COLUMNS.items(): + if table not in vocabulary_tables: + continue + columns = _column_names(vocab, vocabulary_schema, table) + if column not in columns: + diagnostics.append( + _warning( + "fulltext_sidecar_missing", + f"Full-text sidecar column {table}.{column} is missing.", + ) + ) + elif _column_has_null_values( + vocab, + vocabulary_schema, + table, + column, + schema_tag=Role.VOCAB, + ): + fulltext_sidecar_unpopulated.append(f"{table}.{column}") + if fulltext_sidecar_unpopulated: diagnostics.append( _warning( - "fulltext_sidecar_missing", - f"Full-text sidecar column {table}.{column} is missing.", + "fulltext_sidecar_unpopulated", + "Full-text sidecar values are NULL for: " + + ", ".join(fulltext_sidecar_unpopulated) + + ". Run full-text population before relying on lexical grounding.", ) ) - elif _column_has_null_values(connection, schema, table, column): - fulltext_sidecar_unpopulated.append(f"{table}.{column}") - if fulltext_sidecar_unpopulated: - diagnostics.append( - _warning( - "fulltext_sidecar_unpopulated", - "Full-text sidecar values are NULL for: " - + ", ".join(fulltext_sidecar_unpopulated) - + ". Run full-text population before relying on lexical grounding.", - ) + missing_fts_indexes = tuple( + name + for name in GRAPH_FTS_INDEXES + if not _index_exists(vocab_inspector, vocabulary_schema, name) ) - missing_fts_indexes = tuple( - name for name in GRAPH_FTS_INDEXES if not _index_exists(inspector, schema, name) - ) - if missing_fts_indexes: - diagnostics.append( - _warning( - "fulltext_indexes_missing", - f"Full-text GIN indexes are missing: {', '.join(missing_fts_indexes)}.", + if missing_fts_indexes: + diagnostics.append( + _warning( + "fulltext_indexes_missing", + f"Full-text GIN indexes are missing: {', '.join(missing_fts_indexes)}.", + ) + ) + else: + diagnostics.append( + _info("fulltext_indexes_present", "Full-text GIN indexes are present.") ) - ) - else: - diagnostics.append( - _info("fulltext_indexes_present", "Full-text GIN indexes are present.") - ) - missing_functional = tuple( - label - for table, column, label, aliases in GRAPH_FUNCTIONAL_INDEX_TARGETS - if table in existing_tables - and not _functional_lower_index_exists( - connection, - inspector, - schema, - table, - column, - aliases, - ) - ) - if missing_functional: - diagnostics.append( - _warning( - "functional_indexes_missing", - "Functional lower-name indexes are missing: " - f"{', '.join(missing_functional)}.", + missing_functional = tuple( + label + for table, column, label, aliases in GRAPH_FUNCTIONAL_INDEX_TARGETS + if table in vocabulary_tables + and not _functional_lower_index_exists( + vocab, + vocab_inspector, + vocabulary_schema, + table, + column, + aliases, ) ) - else: - diagnostics.append( - _info("functional_indexes_present", "Functional text indexes are present.") - ) + if missing_functional: + diagnostics.append( + _warning( + "functional_indexes_missing", + "Functional lower-name indexes are missing: " + f"{', '.join(missing_functional)}.", + ) + ) + elif not vocabulary_tables: + diagnostics.append( + _warning( + "functional_indexes_unchecked", + "Functional text indexes could not be checked because vocabulary tables were not found.", + ) + ) + else: + diagnostics.append( + _info("functional_indexes_present", "Functional text indexes are present.") + ) return tuple(diagnostics) @@ -401,11 +492,9 @@ def _graph_diagnostics( def _groundworkers_diagnostics( connection: Connection, target: DatabaseTarget, - *, - engine_factory: Callable[[str], Engine], ) -> tuple[ResourceDiagnostic, ...]: diagnostics: list[ResourceDiagnostic] = [] - schema = target.cdm_schema + schema = physical_schema_of(connection, schema_tag=Role.VOCAB) inspector = inspect(connection) existing_tables = set(inspector.get_table_names(schema=schema)) missing_trigram = tuple( @@ -421,7 +510,7 @@ def _groundworkers_diagnostics( aliases, ) ) - if missing_trigram: + if connection.dialect.name == "postgresql" and missing_trigram: diagnostics.append( _warning( "trigram_indexes_missing", @@ -430,7 +519,7 @@ def _groundworkers_diagnostics( "Groundworkers can still run, but constrained substring fallback searches may be slower.", ) ) - else: + elif connection.dialect.name == "postgresql": present_trigram = tuple( label for table, column, label, aliases in GROUNDWORKERS_TRIGRAM_INDEX_TARGETS @@ -465,24 +554,17 @@ def _groundworkers_diagnostics( _normalized_index_diagnostics(connection, schema, existing_tables) ) - diagnostics.extend( - _groundworkers_embedding_model_diagnostics( - target, - engine_factory=engine_factory, - ) - ) + diagnostics.extend(_groundworkers_embedding_model_diagnostics(target)) return tuple(diagnostics) def _groundworkers_embedding_model_diagnostics( target: DatabaseTarget, - *, - engine_factory: Callable[[str], Engine], ) -> tuple[ResourceDiagnostic, ...]: expected_model_name = target.expected_embedding_model_name if not expected_model_name: return () - if not target.embedding_connection_url: + if target.resolved_vector_store is None: return ( _warning( "grounding_embedding_model_unchecked", @@ -490,38 +572,25 @@ def _groundworkers_embedding_model_diagnostics( ), ) - engine: Engine | None = None + store = None try: - engine = engine_factory(target.embedding_connection_url) - with engine.connect() as connection: - connection.execute(text("SELECT 1")) - inspector = inspect(connection) - existing_tables = set( - inspector.get_table_names(schema=target.embedding_schema) - ) - if EMBEDDING_REGISTRY_TABLE not in existing_tables: - return ( - _warning( - "grounding_embedding_registry_missing", - "Groundworkers grounding model could not be checked because the omop-emb model registry table is missing.", - ), - ) - rows = _embedding_registry_rows(connection, schema=target.embedding_schema) + from omop_emb.backends import open_vector_store_reader + + store = open_vector_store_reader(target.resolved_vector_store) + record = store.get_registered_model(model_name=expected_model_name) except Exception as exc: - # Broad except: reported as a redacted warning. failure = classify_connection_error(exc) return ( _warning( - "grounding_embedding_model_unchecked", + f"grounding_{failure.kind.value}", f"Groundworkers grounding model {expected_model_name!r} could not be checked. {failure.detail}", ), ) finally: - if engine is not None: - engine.dispose() + if store is not None: + store.close() - registered_model_names = {str(row["model_name"]) for row in rows} - if expected_model_name in registered_model_names: + if record is not None: return ( _info( "grounding_embedding_model_registered", @@ -536,23 +605,39 @@ def _groundworkers_embedding_model_diagnostics( ) -def _embedding_diagnostics( - connection: Connection, - *, - schema: str | None = None, -) -> tuple[ResourceDiagnostic, ...]: - inspector = inspect(connection) - existing_tables = set(inspector.get_table_names(schema=schema)) - if EMBEDDING_REGISTRY_TABLE not in existing_tables: +def _embedding_diagnostics(target: DatabaseTarget) -> tuple[ResourceDiagnostic, ...]: + if target.resolved_vector_store is None: return ( _warning( - "embedding_registry_missing", - "Connected, but the omop-emb model registry table is missing.", + "embedding_registry_unconfigured", + "No omop-emb vector store is configured.", ), ) + store = None + try: + from omop_emb.backends import open_vector_store_reader - rows = _embedding_registry_rows(connection, schema=schema) - if not rows: + store = open_vector_store_reader(target.resolved_vector_store) + if not store.initialized: + return ( + _warning( + "embedding_registry_missing", + "Connected, but the omop-emb model registry table is missing.", + ), + ) + records = store.get_registered_models() + models = tuple( + (record, store.get_embedding_count(model_name=record.model_name)) + for record in records + ) + except Exception as exc: + failure = classify_connection_error(exc) + return (_warning(f"embedding_{failure.kind.value}", failure.detail),) + finally: + if store is not None: + store.close() + + if not models: return ( _warning( "embedding_registry_empty", @@ -563,95 +648,34 @@ def _embedding_diagnostics( diagnostics: list[ResourceDiagnostic] = [ _info( "embedding_registry_present", - f"{len(rows)} embedding model(s) are registered.", + f"{len(models)} embedding model(s) are registered.", ) ] - for row in rows: - table_name = str(row["storage_identifier"]) - if table_name not in existing_tables: - diagnostics.append( - _warning( - "embedding_table_missing", - f"Storage table {table_name!r} for model {row['model_name']!r} is missing.", - ) - ) - continue - columns = _column_names(connection, schema, table_name) - missing_columns = tuple( - column for column in EMBEDDING_REQUIRED_COLUMNS if column not in columns - ) - if missing_columns: - diagnostics.append( - _warning( - "embedding_table_invalid", - f"Storage table {table_name!r} is missing columns: {', '.join(missing_columns)}.", - ) - ) - continue - count = _count_table_rows(connection, schema, table_name) + for record, count in models: if count == 0: diagnostics.append( _warning( "embedding_table_empty", - f"Storage table {table_name!r} for model {row['model_name']!r} has no embeddings.", + f"Model {record.model_name!r} has no embeddings.", ) ) else: diagnostics.append( _info( "embedding_table_valid", - f"Model {row['model_name']!r} metadata is valid and {count} embedding row(s) are present.", + f"Model {record.model_name!r} has {count} embedding row(s).", ) ) return tuple(diagnostics) -def _embedding_registry_rows(connection: Connection, *, schema: str | None = None): - """Read omop-emb's registry through omop-emb's own mapping. - - The hand-written ``SELECT ... FROM model_registry`` this replaces named the - table unqualified, and ``text()`` is exempt from ``schema_translate_map`` -- - so a store configured with a schema was written by omop-emb into that schema - and read back here from the search path's default one. Going through - ``ModelRegistry`` puts both sides on the same translate map, and keeps the - column list owned by the package that defines it. - """ - - statement = select( - ModelRegistry.model_name, - ModelRegistry.storage_identifier, - ModelRegistry.dimensions, - ModelRegistry.index_type, - ModelRegistry.metric_type, - ).order_by(ModelRegistry.model_name) - return ( - connection.execute(statement, execution_options=_schema_options(schema)) - .mappings() - .all() - ) - - -def _schema_options(schema: str | None) -> dict[str, Any]: - """Execution options that point unqualified ORM/Core tables at *schema*. - - omop-emb declares its tables without a schema, expecting the caller's - translate map to place them -- which is what - ``ResolvedDatabase.create_engine`` does. The engines here are built straight - from a connection URL and carry no map, so it is supplied per statement. - Per statement rather than via ``Connection.execution_options``, which - mutates the connection and would leave the map in place for everything - executed on it afterwards. - """ - - if schema is None: - return {} - return {"schema_translate_map": {None: schema}} - - def classify_connection_error(exc: BaseException) -> ClassifiedFailure: """Map provider and database exceptions to redacted operator guidance.""" chain = tuple(_exception_chain(exc)) + store_failure = _embedding_store_failure(chain) + if store_failure is not None: + return store_failure lowered = " ".join(str(item).lower() for item in chain) sqlstate = next( ( @@ -666,6 +690,16 @@ def classify_connection_error(exc: BaseException) -> ClassifiedFailure: "", ) + if sqlstate == "3D000": + return _failure(ConnectionFailureKind.DATABASE_MISSING, "The configured database does not exist.", "Check the database name in the stack configuration.") + if sqlstate == "3F000": + return _failure(ConnectionFailureKind.SCHEMA_MISSING, "A configured database schema does not exist.", "Check the schema names in the stack configuration.") + if sqlstate == "42P01": + return _failure(ConnectionFailureKind.TABLE_MISSING, "A required database table does not exist.", "Check the database schema and setup status.") + oa_failure = _oa_configuration_failure(chain) + if oa_failure is not None: + return oa_failure + if any( isinstance(item, (ModuleNotFoundError, NoSuchModuleError)) for item in chain ): @@ -716,7 +750,6 @@ def classify_connection_error(exc: BaseException) -> ClassifiedFailure: "password authentication", 'role "', "role '", - "does not exist", "unauthorized", "forbidden", "no pg_hba.conf entry", @@ -727,7 +760,7 @@ def classify_connection_error(exc: BaseException) -> ClassifiedFailure: "The service rejected the configured user or credentials.", "Check the host, port, user and password; confirm the configured role exists on the database that answered.", ) - if sqlstate == "3D000" or any( + if any( phrase in lowered for phrase in ("database does not exist", "unknown database", "not found") ): @@ -745,6 +778,10 @@ def classify_connection_error(exc: BaseException) -> ClassifiedFailure: "The service was reached but the verification query failed.", "Check the configured database and schema mappings.", ) + logger.error( + "Unclassified database verification failure", + exc_info=(type(exc), exc, exc.__traceback__), + ) return _failure( ConnectionFailureKind.OTHER, "The connection check failed for an unclassified reason.", @@ -752,6 +789,49 @@ def classify_connection_error(exc: BaseException) -> ClassifiedFailure: ) +def _oa_configuration_failure( + chain: tuple[BaseException, ...], +) -> ClassifiedFailure | None: + messages = { + "UnregisteredSchemaTagError": "A model uses an unregistered database schema tag.", + "CrossDatabaseStatementError": "A query attempted to join tables from separate databases.", + "UnboundExecutionError": "A database query has no configured role binding.", + "SchemaOwnershipError": "The configured schema belongs to another database role.", + "SchemaRegistryOutdatedError": "The database schema registry needs migration.", + "SchemaDriftError": "The configured database schema differs from its registered state.", + } + for item in chain: + name = type(item).__name__ + if name not in messages: + continue + hint = ( + " Run 'omop-config acknowledge-schema-migration' after reviewing the schema change." + if name in {"SchemaRegistryOutdatedError", "SchemaDriftError"} + else " Check the stack database role and schema configuration." + ) + return _failure(ConnectionFailureKind.QUERY, messages[name], messages[name] + hint) + return None + + +def _embedding_store_failure( + chain: tuple[BaseException, ...], +) -> ClassifiedFailure | None: + kinds = { + "LEGACY_REGISTRY": ConnectionFailureKind.LEGACY_REGISTRY, + "MISPLACED_REGISTRY": ConnectionFailureKind.MISPLACED_REGISTRY, + "MISSING_STORAGE_TABLE": ConnectionFailureKind.MISSING_STORAGE_TABLE, + "MODEL_REGISTRATION_CONFLICT": ConnectionFailureKind.MODEL_REGISTRATION_CONFLICT, + "READ_ONLY_STORE": ConnectionFailureKind.READ_ONLY_STORE, + } + for item in chain: + translated = embedding_store_error(item) + if translated is not None: + return _failure( + kinds[translated.code], translated.message, translated.message + ) + return None + + def _failure( kind: ConnectionFailureKind, detail: str, next_action: str ) -> ClassifiedFailure: @@ -866,13 +946,16 @@ def normalized_index_status( column_name: str, ) -> bool | None: """Return True/False for the K index, or None when listing failed.""" - definitions = _postgres_index_definitions(connection, schema, table_name) - if definitions is None: + indexes = _reflected_indexes(connection, schema, table_name) + if indexes is None: return None expected = _squash_index_sql( normalization_key_expression_sql(connection, column_name) ) - return any(expected in _squash_index_sql(indexdef) for indexdef in definitions) + return any( + expected in _squash_index_sql(" ".join(index.get("expressions") or ())) + for index in indexes + ) def _squash_index_sql(sql: str) -> str: @@ -890,14 +973,21 @@ def _trigram_lower_index_exists( column_name: str, aliases: tuple[str, ...], ) -> bool: - if connection.dialect.name == "postgresql" and _postgres_lower_index_exists( - connection, - schema, - table_name, - column_name, - required_fragment="gin_trgm_ops", - ): - return True + if connection.dialect.name == "postgresql": + indexes = _reflected_indexes(connection, schema, table_name) or () + for index in indexes: + options = index.get("dialect_options") or {} + postgres_ops = options.get("postgresql_ops") or {} + opclasses = postgres_ops.values() if isinstance(postgres_ops, dict) else () + using = options.get("postgresql_using") + expression = " ".join(index.get("expressions") or ()) + if ( + str(using).lower() == "gin" + and any("gin_trgm_ops" in str(op).lower() for op in opclasses) + and _index_defines_lower_expression(expression, column_name) + ): + return True + return False try: indexes = inspector.get_indexes(table_name, schema=schema) except Exception: @@ -916,35 +1006,25 @@ def _postgres_lower_index_exists( ) -> bool: return any( _index_defines_lower_expression( - indexdef, + " ".join(index.get("expressions") or ()), column_name, required_fragment=required_fragment, ) - for indexdef in (_postgres_index_definitions(connection, schema, table_name) or ()) + for index in (_reflected_indexes(connection, schema, table_name) or ()) ) -def _postgres_index_definitions( +def _reflected_indexes( connection: Connection, schema: str | None, table_name: str, -) -> tuple[str, ...] | None: +) -> tuple[Mapping[str, Any], ...] | None: try: - rows = connection.execute( - text( - """ - SELECT indexdef - FROM pg_indexes - WHERE schemaname = :schema - AND tablename = :table_name - """ - ), - {"schema": schema or "public", "table_name": table_name}, - ) + indexes = inspect(connection).get_indexes(table_name, schema=schema) except Exception: - # Broad except: callers fall back to reflected names or report absence. + # Callers fall back to reflected names or report absence. return None - return tuple(str(row[0]) for row in rows) + return tuple(indexes) def _index_defines_lower_expression( @@ -972,24 +1052,6 @@ def _column_names( schema: str | None, table_name: str, ) -> set[str]: - if connection.dialect.name == "postgresql": - rows = connection.execute( - text( - """ - SELECT column_name - FROM information_schema.columns - WHERE table_schema = :schema - AND table_name = :table_name - """ - ), - {"schema": schema or "public", "table_name": table_name}, - ) - return {str(row[0]) for row in rows} - if connection.dialect.name == "sqlite": - rows = connection.execute( - text(f"PRAGMA table_info({quote_identifier(connection, table_name)})") - ) - return {str(row[1]) for row in rows} return { item["name"] for item in inspect(connection).get_columns(table_name, schema=schema) @@ -997,9 +1059,15 @@ def _column_names( def _table_is_empty( - connection: Connection, schema: str | None, table_name: str + connection: Connection, + schema: str | None, + table_name: str, + *, + schema_tag: Role, ) -> bool: - return _count_table_rows(connection, schema, table_name) == 0 + return _count_table_rows( + connection, schema, table_name, schema_tag=schema_tag + ) == 0 def _column_has_null_values( @@ -1007,14 +1075,21 @@ def _column_has_null_values( schema: str | None, table_name: str, column_name: str, + *, + schema_tag: Role, ) -> bool: """Return whether a full-text sidecar contains any unpopulated rows.""" - qualified = _qualified_name(connection, schema, table_name) - column = quote_identifier(connection, column_name) + table = _role_table(connection, schema, table_name, schema_tag=schema_tag) return bool( connection.execute( - text(f"SELECT EXISTS (SELECT 1 FROM {qualified} WHERE {column} IS NULL)") + select( + exists( + select(literal(1)) + .select_from(table) + .where(table.c[column_name].is_(None)) + ) + ) ).scalar() ) @@ -1023,20 +1098,31 @@ def _count_table_rows( connection: Connection, schema: str | None, table_name: str, + *, + schema_tag: Role, ) -> int: - qualified = _qualified_name(connection, schema, table_name) + table = _role_table(connection, schema, table_name, schema_tag=schema_tag) return int( - connection.execute(text(f"SELECT count(*) FROM {qualified}")).scalar() or 0 + connection.execute(select(func.count()).select_from(table)).scalar() + or 0 ) -def _qualified_name(connection: Connection, schema: str | None, table_name: str) -> str: - if schema: - return ( - f"{quote_identifier(connection, schema)}." - f"{quote_identifier(connection, table_name)}" - ) - return quote_identifier(connection, table_name) +def _role_table( + connection: Connection, + schema: str | None, + table_name: str, + *, + schema_tag: Role, +) -> Table: + columns = inspect(connection).get_columns(table_name, schema=schema) + table = Table( + table_name, + MetaData(), + *(Column(column["name"], column["type"]) for column in columns), + schema=schema_tag.value, + ) + return table def _exception_chain(exc: BaseException): diff --git a/src/groundworkers/application/setup/embedding_artifacts.py b/src/groundworkers/application/setup/embedding_artifacts.py index f500909..41a7c7f 100644 --- a/src/groundworkers/application/setup/embedding_artifacts.py +++ b/src/groundworkers/application/setup/embedding_artifacts.py @@ -3,8 +3,6 @@ from collections.abc import Iterable from pathlib import Path -from omop_emb.storage.embedding_bundle import ExportMetadata - from groundworkers.application.setup.models import ( ArtifactCompatibility, ArtifactDiscovery, @@ -99,9 +97,10 @@ def _inspect_bundle(path: Path) -> ArtifactMetadata: # Imported lazily: reading exported bundles is an optional capability behind the # `embedding-artifacts` extra, and a core install must still import this module. import h5py + from omop_emb.storage.embedding_bundle import BundleMetadata with h5py.File(path, "r") as bundle: - metadata = ExportMetadata.from_h5_attrs(bundle.attrs) + metadata = BundleMetadata.from_h5_attrs(bundle.attrs) vocabularies = _optional_csv(bundle.attrs.get("groundworkers_vocabularies")) standard_only = _optional_bool(bundle.attrs.get("groundworkers_standard_only")) valid_only = _optional_bool(bundle.attrs.get("groundworkers_valid_only")) @@ -109,7 +108,11 @@ def _inspect_bundle(path: Path) -> ArtifactMetadata: path=path, model_name=metadata.model_name, dimensions=metadata.dimensions, - metric=metadata.metric_type.value, + metric=( + metadata.index_config.metric_type.value + if metadata.index_config.metric_type is not None + else "cosine" + ), provider=metadata.provider_type, row_count=metadata.row_count, vocabularies=vocabularies, diff --git a/src/groundworkers/application/setup/embedding_coverage.py b/src/groundworkers/application/setup/embedding_coverage.py index 4a0bc1b..b292f87 100644 --- a/src/groundworkers/application/setup/embedding_coverage.py +++ b/src/groundworkers/application/setup/embedding_coverage.py @@ -3,8 +3,7 @@ from collections.abc import Callable, Mapping from typing import Any -from omop_emb.config import MetricType - +from groundworkers.application.setup.databases import classify_connection_error from groundworkers.application.setup.models import ( CoverageScope, CoverageSnapshot, @@ -33,30 +32,16 @@ def load_coverage( blocker=FILTERED_AGGREGATE_BLOCKER, metadata={"omop_emb_capability": "unfiltered_by_vocabulary_only"}, ) - aggregate = getattr(backend, "get_embedding_count_by_vocabulary", None) - if not callable(aggregate): - return CoverageSnapshot( - scope=scope, - available=False, - blocker=( - "This omop-emb version does not expose pushed-down embedding counts " - "by vocabulary." - ), - metadata={"omop_emb_capability": "missing"}, - ) try: eligible = _coerce_counts(eligible_counter(scope)) embedded = _coerce_counts( - aggregate( - model_name=scope.model_name, - metric_type=MetricType(scope.metric), - ) + backend.get_embedding_count_by_vocabulary(model_name=scope.model_name) ) - except Exception: + except Exception as exc: return CoverageSnapshot( scope=scope, available=False, - blocker="Coverage counts could not be loaded from the configured stores.", + blocker=classify_connection_error(exc).detail, ) return calculate_coverage(scope, eligible=eligible, embedded=embedded) diff --git a/src/groundworkers/application/setup/embedding_population.py b/src/groundworkers/application/setup/embedding_population.py index 2ab7b14..8ad7889 100644 --- a/src/groundworkers/application/setup/embedding_population.py +++ b/src/groundworkers/application/setup/embedding_population.py @@ -8,7 +8,9 @@ from typing import Any, cast from groundskeeping.contracts import Command, CommandPlan, CommandStep -from oa_configurator import Resolver, safe_endpoint +from oa_configurator import ResolvedCDMDatabase, Resolver, safe_endpoint +from omop_alchemy.config import create_cdm_engines +from omop_alchemy.cross_database import cdm_sessionmaker from omop_llm import canonical_model_name from sqlalchemy.engine import Engine @@ -27,6 +29,7 @@ EmbeddingIndexSnapshot, EmbeddingPopulationRequest, ) +from groundworkers.base.errors import embedding_store_error from groundworkers.base.results import enum_value from groundworkers.config import GroundworkersConfig @@ -66,15 +69,15 @@ def load_embedding_coverage_report( vector_store = resolver.resolve_vector_store( groundworkers.vector_store_name ) - store = inspect_resolved_vector_store(vector_store) - cdm_engine, _ = _cdm_engine_and_schema(snapshot) + store = open_vector_store_reader(vector_store) + cdm_session_factory, cdm_engines = _cdm_session_factory_and_engines(snapshot) try: canonical_model = _canonical_model_name( resolved_model.model, provider_kind=resolved_model.provider.provider, ) plan = plan_population( - cdm_engine, + cdm_session_factory, store, model_name=canonical_model, scope=_population_scope( @@ -83,7 +86,8 @@ def load_embedding_coverage_report( ), ) finally: - cdm_engine.dispose() + for engine in cdm_engines: + engine.dispose() index = _embedding_index_snapshot( store=store, canonical_model=canonical_model, @@ -139,6 +143,9 @@ class -- turned an actionable answer into a dead end. keep the class-name-only form. URLs are scrubbed from whatever is shown, so a message that does carry one cannot leak a credential either way. """ + store_error = embedding_store_error(exc) + if store_error is not None: + return store_error.message if isinstance(exc, ValueError): detail = _scrub_urls(str(exc).strip()) if detail: @@ -232,12 +239,23 @@ def replace_coverage_metadata(coverage: CoverageSnapshot, *, plan) -> CoverageSn ) -def _cdm_engine_and_schema(snapshot: ConfigurationSnapshot) -> tuple[Engine, str]: +def _cdm_session_factory_and_engines( + snapshot: ConfigurationSnapshot, +) -> tuple[object, tuple[Engine, ...]]: assert snapshot.stack is not None resolver = Resolver(snapshot.stack) groundworkers = GroundworkersConfig.validate_candidate(snapshot.stack) database = resolver.resolve_database(groundworkers.cdm_db) - return database.create_engine(), database.schema_name or "main" + if not isinstance(database, ResolvedCDMDatabase): + raise ValueError("Groundworkers cdm_db must resolve to a CDM database.") + primary_engine, vocab_engine = create_cdm_engines( + database, register_claims=False + ) + session_factory = cdm_sessionmaker( + database, primary=primary_engine, vocab=vocab_engine + ) + engines = (primary_engine,) if vocab_engine is primary_engine else (primary_engine, vocab_engine) + return session_factory, engines def _embedding_index_snapshot( @@ -245,7 +263,7 @@ def _embedding_index_snapshot( store, canonical_model: str, ) -> EmbeddingIndexSnapshot: - record = store.model(canonical_model) + record = store.get_registered_model(model_name=canonical_model) if record is None: return EmbeddingIndexSnapshot(model_name=canonical_model, registered=False) physical_indexes = store.physical_indexes(canonical_model) @@ -264,21 +282,21 @@ def _canonical_model_name(model_name: str, *, provider_kind: str) -> str: return canonical_model_name(provider_kind, model_name) -def inspect_resolved_vector_store(resolved): - """Load the optional read-only omop-emb contract at use time.""" +def open_vector_store_reader(resolved): + """Load the optional read-only omop-emb store at use time.""" - from omop_emb.backends import inspect_resolved_vector_store as inspect_store + from omop_emb.backends import open_vector_store_reader as open_reader - return inspect_store(resolved) + return open_reader(resolved) -def plan_population(cdm_engine, store, *, model_name: str, scope): +def plan_population(cdm_session_factory, store, *, model_name: str, scope): """Load the optional identity-planning contract at use time.""" population = cast(Any, import_module("omop_emb.population")) return population.plan_population( - cdm_engine, + cdm_session_factory, store, model_name=model_name, scope=scope, diff --git a/src/groundworkers/application/setup/embedding_registry.py b/src/groundworkers/application/setup/embedding_registry.py index 119d62c..0c35779 100644 --- a/src/groundworkers/application/setup/embedding_registry.py +++ b/src/groundworkers/application/setup/embedding_registry.py @@ -54,13 +54,13 @@ def list_registered_models(snapshot: ConfigurationSnapshot) -> EmbeddingStoreSna ) try: - from omop_emb.backends import inspect_resolved_vector_store + from omop_emb.backends import open_vector_store_reader - store = inspect_resolved_vector_store(resolved) - records = store.registered_models() + store = open_vector_store_reader(resolved) + records = store.get_registered_models() models_list = [] for record in records: - embeddings = store.stored_embeddings(record.model_name) + count = store.get_embedding_count(model_name=record.model_name) models_list.append( RegisteredEmbeddingModel( model_name=record.model_name, @@ -68,8 +68,8 @@ def list_registered_models(snapshot: ConfigurationSnapshot) -> EmbeddingStoreSna dimensions=int(record.dimensions), metric=(record.metric_type.value if record.metric_type is not None else None), index_type=record.index_type.value, - has_embeddings=bool(embeddings), - concept_count=len(embeddings), + has_embeddings=bool(count), + concept_count=count, ) ) models = tuple(models_list) diff --git a/src/groundworkers/application/setup/embedding_setup.py b/src/groundworkers/application/setup/embedding_setup.py index 1a7e748..e809104 100644 --- a/src/groundworkers/application/setup/embedding_setup.py +++ b/src/groundworkers/application/setup/embedding_setup.py @@ -9,7 +9,8 @@ Resolver, safe_endpoint, # type: ignore[import-untyped] ) -from omop_emb import EmbeddingBackend, MetricType +from omop_emb import EmbeddingBackend +from omop_emb.backends import EmbeddingStoreReader from omop_llm import ( ModelBackend, build_model_backend_from_resolved, @@ -63,9 +64,9 @@ def initialize_embedding_store( groundworkers.vector_store_name ) if initializer is None: - from omop_emb.backends import initialize_resolved_vector_store + from omop_emb.backends import open_vector_store_writer - initializer = initialize_resolved_vector_store + initializer = open_vector_store_writer backend = initializer(resolved) try: records = backend.get_registered_models() @@ -82,16 +83,12 @@ def initialize_embedding_store( index_type=required_enum_value(record.index_type), has_embeddings=backend.has_any_embeddings( model_name=record.model_name, - metric_type=record.metric_type or MetricType.COSINE, - _model_record=record, ), ) for record in records ) finally: - engine = getattr(backend, "emb_engine", None) - if engine is not None: - engine.dispose() + backend.close() except Exception as exc: return EmbeddingStoreSnapshot( state=EmbeddingStoreState.UNREACHABLE, @@ -172,7 +169,7 @@ def load_embedding_configuration( def probe_embedding_store( - backend_factory: Callable[[], EmbeddingBackend] | None, + backend_factory: Callable[[], EmbeddingStoreReader] | None, *, backend_type: str | None, ) -> EmbeddingStoreSnapshot: @@ -189,11 +186,8 @@ def probe_embedding_store( records = backend.get_registered_models() models = [] for record in records: - metric = record.metric_type or MetricType.COSINE has_embeddings = backend.has_any_embeddings( model_name=record.model_name, - metric_type=metric, - _model_record=record, ) models.append( RegisteredEmbeddingModel( diff --git a/src/groundworkers/application/setup/graph_maintenance.py b/src/groundworkers/application/setup/graph_maintenance.py index 5b703a7..d2f44ce 100644 --- a/src/groundworkers/application/setup/graph_maintenance.py +++ b/src/groundworkers/application/setup/graph_maintenance.py @@ -9,6 +9,7 @@ from __future__ import annotations import shutil +import sys from dataclasses import dataclass from enum import StrEnum from importlib import resources @@ -212,4 +213,5 @@ def start_graph_remediation_run( def _executable(name: str) -> str: - return shutil.which(name) or name + beside_python = Path(sys.executable).with_name(name) + return str(beside_python) if beside_python.is_file() else (shutil.which(name) or name) diff --git a/src/groundworkers/application/setup/models.py b/src/groundworkers/application/setup/models.py index 6f8f49a..e7e8a7c 100644 --- a/src/groundworkers/application/setup/models.py +++ b/src/groundworkers/application/setup/models.py @@ -68,17 +68,13 @@ class DatabaseTarget: database_entry_name: str connection_name: str safe_url: str - cdm_schema: str - vocabulary_schema: str + cdm_schema: str | None + vocabulary_schema: str | None connection_url: str = field(repr=False) role: str = "cdm" - # The schema omop-emb's registry and storage tables live in, when this target - # touches them. Distinct from cdm_schema, which is never None and falls back - # to a dialect default; None here means "no override, use the search path". - embedding_schema: str | None = None expected_embedding_model_name: str | None = None - embedding_safe_url: str | None = None - embedding_connection_url: str | None = field(default=None, repr=False) + resolved_vector_store: Any | None = field(default=None, repr=False) + resolved_cdm_database: Any | None = field(default=None, repr=False) def __repr__(self) -> str: return ( @@ -97,7 +93,14 @@ class ConnectionFailureKind(StrEnum): TIMEOUT = "timeout" AUTHENTICATION = "authentication" DATABASE_MISSING = "database_missing" + SCHEMA_MISSING = "schema_missing" + TABLE_MISSING = "table_missing" DRIVER_MISSING = "driver_missing" + LEGACY_REGISTRY = "legacy_registry" + MISPLACED_REGISTRY = "misplaced_registry" + MISSING_STORAGE_TABLE = "missing_storage_table" + MODEL_REGISTRATION_CONFLICT = "model_registration_conflict" + READ_ONLY_STORE = "read_only_store" QUERY = "query_failure" OTHER = "other" diff --git a/src/groundworkers/application/setup/performance_maintenance.py b/src/groundworkers/application/setup/performance_maintenance.py index 8e483f6..c6b89b3 100644 --- a/src/groundworkers/application/setup/performance_maintenance.py +++ b/src/groundworkers/application/setup/performance_maintenance.py @@ -12,9 +12,16 @@ import sqlalchemy as sa from groundskeeping.contracts import Command, CommandPlan, CommandStep -from oa_configurator import Resolver +from oa_configurator import ( + ResolvedCDMDatabase, + Resolver, + Role, + physical_schema_of, + qualified, +) from omop_alchemy.cdm.model.vocabulary import Concept, Concept_Synonym -from sqlalchemy import text +from omop_alchemy.config import create_cdm_engines +from sqlalchemy import Column, Index, MetaData, String, Table, func from sqlalchemy.engine import Connection from groundworkers.application.setup.configuration import load_configuration @@ -31,7 +38,6 @@ ) from groundworkers.base.sql import ( normalization_key_expression, - quote_identifier, ) from groundworkers.config import GroundworkersConfig @@ -123,30 +129,35 @@ def start_performance_run( @contextmanager def _postgres_cdm( snapshot: ConfigurationSnapshot, *, purpose: str -) -> Iterator[tuple[Connection, str | None, str]]: - """Yield a transaction on the PostgreSQL CDM, its schema, and a table prefix.""" +) -> Iterator[tuple[Connection, str | None]]: + """Yield a transaction on the PostgreSQL vocabulary role and its schema.""" if snapshot.stack is None: raise ValueError("A usable stack configuration is required.") config = GroundworkersConfig.validate_candidate(snapshot.stack) database = Resolver(snapshot.stack).resolve_database(config.cdm_db) - engine = database.create_engine() + if not isinstance(database, ResolvedCDMDatabase): + raise TypeError("Groundworkers cdm_db must resolve to a CDM database.") + primary_engine, vocab_engine = create_cdm_engines( + database, register_claims=False + ) try: - if engine.dialect.name != "postgresql": + if vocab_engine.dialect.name != "postgresql": raise ValueError(f"{purpose} require a PostgreSQL CDM database.") - schema = getattr(database, "schema_name", None) - prefix = f"{quote_identifier(engine, schema)}." if schema else "" - with engine.begin() as connection: - yield connection, schema, prefix + schema = database.schema_for_role(Role.VOCAB) + with vocab_engine.begin() as connection: + yield connection, schema finally: - engine.dispose() + primary_engine.dispose() + if vocab_engine is not primary_engine: + vocab_engine.dispose() def populate_trigram_indexes(snapshot: ConfigurationSnapshot) -> None: """Create the optional PostgreSQL trigram indexes Groundworkers can use.""" - with _postgres_cdm(snapshot, purpose="Trigram indexes") as (connection, _, prefix): - connection.execute(text("CREATE EXTENSION IF NOT EXISTS pg_trgm")) + with _postgres_cdm(snapshot, purpose="Trigram indexes") as (connection, _schema): + connection.exec_driver_sql("CREATE EXTENSION IF NOT EXISTS pg_trgm") for table, column, index_name in ( ("concept", "concept_name", "idx_concept_lower_name_trgm"), ( @@ -155,13 +166,19 @@ def populate_trigram_indexes(snapshot: ConfigurationSnapshot) -> None: "idx_concept_synonym_lower_name_trgm", ), ): - connection.execute( - text( - f"CREATE INDEX IF NOT EXISTS {quote_identifier(connection, index_name)} " - f"ON {prefix}{quote_identifier(connection, table)} USING gin " - f"(lower({quote_identifier(connection, column)}) gin_trgm_ops)" - ) + light_table = Table( + table, + MetaData(), + Column(column, String), + schema=Role.VOCAB.value, ) + index = Index( + index_name, + func.lower(light_table.c[column]).label("lower_name"), + postgresql_using="gin", + postgresql_ops={"lower_name": "gin_trgm_ops"}, + ) + connection.execute(sa.schema.CreateIndex(index, if_not_exists=True)) def populate_normalized_indexes(snapshot: ConfigurationSnapshot) -> None: @@ -176,7 +193,6 @@ def populate_normalized_indexes(snapshot: ConfigurationSnapshot) -> None: with _postgres_cdm(snapshot, purpose="Normalized-name indexes") as ( connection, schema, - prefix, ): for table, column, _label, index_name in GROUNDWORKERS_NORMALIZED_INDEX_TARGETS: model_table = model_tables[table] @@ -207,8 +223,14 @@ def populate_normalized_indexes(snapshot: ConfigurationSnapshot) -> None: sa.schema.DropIndex(index, if_exists=True) ) connection.execute(sa.schema.CreateIndex(index)) - qualified = f"{prefix}{quote_identifier(connection, table)}" - connection.exec_driver_sql(f"ANALYZE {qualified}") + table_name = qualified( + connection, + table, + physical_schema=physical_schema_of( + connection, schema_tag=Role.VOCAB + ), + ) + connection.exec_driver_sql(f"ANALYZE {table_name}") def _main(argv: Sequence[str] | None = None) -> int: diff --git a/src/groundworkers/base/domain_names.py b/src/groundworkers/base/domain_names.py index af3a97a..cfda14a 100644 --- a/src/groundworkers/base/domain_names.py +++ b/src/groundworkers/base/domain_names.py @@ -1,17 +1,39 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING +from collections.abc import Callable +from typing import Any +from oa_configurator import CrossDatabaseStatementError from omop_alchemy.cdm.model.vocabulary import Domain from sqlalchemy import select - -if TYPE_CHECKING: - from sqlalchemy.engine import Engine +from sqlalchemy.engine import Engine logger = logging.getLogger(__name__) +def _is_missing_domain_table(exc: Exception) -> bool: + """Recognize database errors caused specifically by the absent domain table.""" + pending: list[object] = [exc] + seen: set[int] = set() + while pending: + current = pending.pop() + if id(current) in seen: + continue + seen.add(id(current)) + if "no such table" in str(current).lower(): + return True + if getattr(current, "sqlstate", None) == "42P01" or getattr( + current, "pgcode", None + ) == "42P01": + return True + for attr in ("orig", "__cause__", "__context__"): + nested = getattr(current, attr, None) + if nested is not None: + pending.append(nested) + return False + + class DomainNameResolver: """Resolve a loosely-typed domain to its canonical ``domain_id``. @@ -20,8 +42,13 @@ class DomainNameResolver: restarts the server anyway. """ - def __init__(self, engine: Engine) -> None: - self._engine = engine + def __init__(self, engine: Engine | Callable[[], Any]) -> None: + if isinstance(engine, Engine): + self._engine: Engine | None = engine + self._session_factory: Callable[[], Any] | None = None + else: + self._engine = None + self._session_factory = engine self._by_lowered: dict[str, str] | None = None def _load(self) -> dict[str, str]: @@ -30,12 +57,32 @@ def _load(self) -> dict[str, str]: mapping: dict[str, str] = {} try: - with self._engine.connect() as conn: - rows = conn.execute(select(Domain.domain_id, Domain.domain_name)).all() - except Exception: + if self._session_factory is not None: + with self._session_factory() as session: + rows = session.execute( + select(Domain.domain_id, Domain.domain_name) + ).all() + else: + assert self._engine is not None + with self._engine.connect() as conn: + rows = conn.execute( + select(Domain.domain_id, Domain.domain_name) + ).all() + except Exception as exc: logger.warning("Could not read the domain table; domain names unresolved.", exc_info=True) - self._by_lowered = {} - return self._by_lowered + if self._engine is not None and ( + isinstance(exc, CrossDatabaseStatementError) or _is_missing_domain_table(exc) + ): + raise RuntimeError( + "Engine mode cannot route vocabulary queries. If this deployment " + "keeps the vocabulary on its own database, pass " + "cdm_session_factory (PluginContext.cdm_session_factory) " + "to DomainNameResolver." + ) from exc + if self._engine is not None: + raise + # Leave the cache empty so transient outages are retried next time. + return mapping for domain_id, domain_name in rows: if not domain_id: diff --git a/src/groundworkers/base/errors.py b/src/groundworkers/base/errors.py index a21bf37..ecc72c0 100644 --- a/src/groundworkers/base/errors.py +++ b/src/groundworkers/base/errors.py @@ -14,6 +14,11 @@ "FORMAT_BINARY_DECODE", "FORMAT_UNRECOGNISED", "MISSING_DEPENDENCY", + "LEGACY_REGISTRY", + "MISPLACED_REGISTRY", + "MISSING_STORAGE_TABLE", + "MODEL_REGISTRATION_CONFLICT", + "READ_ONLY_STORE", "INTERNAL_ERROR", ] @@ -78,10 +83,62 @@ def to_dict(self) -> dict[str, str | bool]: return {"error": True, "code": self.code, "message": self.message} +def embedding_store_error(exc: BaseException) -> GroundworkersError | None: + """Translate known omop-emb 3.0 store failures into public tool errors.""" + + try: + from omop_emb.model_registry.model_registry_orm import MIGRATION_COMMAND + from omop_emb.utils.errors import ( + LegacyRegistryError, + MisplacedRegistryError, + MissingStorageTableError, + ModelRegistrationConflictError, + ReadOnlyStoreError, + ) + except ImportError: + return None + + if isinstance(exc, LegacyRegistryError): + message = str(exc).lower() + if "recognis" in message or "recogniz" in message or "partial" in message: + return GroundworkersError("LEGACY_REGISTRY", str(exc)) + return GroundworkersError( + "LEGACY_REGISTRY", + "This embedding store predates omop-emb 3.0. The migration script is " + "tracked in oa-configurator issue #40; once it ships, run: " + f"{MIGRATION_COMMAND}", + ) + if isinstance(exc, MisplacedRegistryError): + return GroundworkersError( + "MISPLACED_REGISTRY", + "The omop-emb registry is outside the schema resolved for the 'registry' " + "tag. Correct the vector-store schema configuration or move the registry " + "to the resolved schema.", + ) + if isinstance(exc, MissingStorageTableError): + return GroundworkersError( + "MISSING_STORAGE_TABLE", + "The embedding registry refers to a model whose storage table is missing.", + ) + if isinstance(exc, ModelRegistrationConflictError): + field = getattr(exc, "conflict_field", "model metadata") + return GroundworkersError( + "MODEL_REGISTRATION_CONFLICT", + f"The model registration conflicts on {field}. Only one store per model " + "per database is supported; reuse the existing store or choose another database.", + ) + if isinstance(exc, ReadOnlyStoreError): + return GroundworkersError( + "READ_ONLY_STORE", "This embedding store is read-only; write operations are refused." + ) + return None + + __all__ = [ "ERROR_CODES", "ErrorCode", "GroundworkersError", + "embedding_store_error", "internal_error_response", "scrub_error_message", ] diff --git a/src/groundworkers/base/sql.py b/src/groundworkers/base/sql.py index 9b0e019..5416c31 100644 --- a/src/groundworkers/base/sql.py +++ b/src/groundworkers/base/sql.py @@ -13,13 +13,11 @@ "KEY_DROP_TOKEN_PATTERN", "STOP_PHRASE_SQL_PATTERN", "NormalizationProfile", - "effective_schema", "normalization_expression", "normalization_index_expression_sql", "normalization_key_expression", "normalization_key_expression_sql", "normalized_key", - "quote_identifier", ] @@ -46,29 +44,6 @@ class NormalizationProfile(StrEnum): KEY_DROP_TOKEN_PATTERN = "(?:" + "|".join(sorted(KEY_DROP_TOKENS)) + ")" -def quote_identifier(bind: Engine | Connection, name: str) -> str: - """Quote *name* as an identifier for whatever dialect *bind* speaks. - - Delegates to SQLAlchemy's own identifier preparer rather than wrapping the - value in double quotes, which is correct only for dialects that use them. - """ - return bind.dialect.identifier_preparer.quote(name) - - -def effective_schema(bind: Engine | Connection) -> str | None: - """The schema that schema-unqualified tables resolve to on *bind*. - - oa-configurator expresses schema placement entirely through SQLAlchemy's - ``schema_translate_map`` and sets no ``search_path``. That map is applied - when SQLAlchemy *compiles* a statement, so ORM and Core constructs are - routed for free -- but reflection is not: ``Inspector`` takes a literal - schema name and ignores the map completely. Anything that inspects rather - than queries has to be told where to look, and this is where that answer - comes from, so reflection and generated SQL cannot disagree. - """ - return bind.get_execution_options().get("schema_translate_map", {}).get(None) - - def normalization_expression( column, *, diff --git a/src/groundworkers/bootstrap.py b/src/groundworkers/bootstrap.py index 150aa58..b343d1d 100644 --- a/src/groundworkers/bootstrap.py +++ b/src/groundworkers/bootstrap.py @@ -1,33 +1,32 @@ from __future__ import annotations -from importlib import import_module from pathlib import Path -from typing import Any, cast from oa_configurator import ( # type: ignore[import-untyped] ConfigurationError, ResolvedCDMDatabase, Resolver, + Role, StackConfig, load_stack_config, ) +from omop_alchemy.config import create_cdm_engines +from omop_alchemy.cross_database import cdm_sessionmaker from groundworkers.config import ( AppConfig, GroundworkersConfig, - split_vocabulary_connection, ) +class DatabaseUnavailableError(ConfigurationError): + """A valid stack whose configured CDM endpoints cannot be initialized.""" + + def build_app_config(*, config_path: str | Path | None = None) -> AppConfig: """Resolve the stack configuration into Groundworkers' runtime view.""" - configurator = cast(Any, import_module("oa_configurator")) - stack = ( - configurator.load_stack_config_from_path(config_path) - if config_path is not None - else load_stack_config() - ) + stack = load_stack_config(config_path) return build_app_config_from_stack(stack) @@ -41,17 +40,32 @@ def build_app_config_from_stack(stack: StackConfig) -> AppConfig: raise TypeError( f"Groundworkers cdm_db {groundworkers.cdm_db!r} must reference a CDM database." ) - split = split_vocabulary_connection(stack, groundworkers.cdm_db) - if split is not None: - primary, vocabulary = split - raise ConfigurationError( - f"Groundworkers cdm_db {groundworkers.cdm_db!r} names connection " - f"{vocabulary!r} for its vocabulary and {primary!r} for the CDM. " - "Groundworkers reads both through one engine; put the vocabulary on " - "the CDM connection and use vocab_schema to separate them." + try: + cdm_engine, cdm_vocab_engine = create_cdm_engines( + cdm_database, register_claims=False ) - - cdm_engine = cdm_database.create_engine(future=True) + except Exception as exc: + detail = " ".join(str(exc).splitlines()) + for role in (Role.PRIMARY, Role.VOCAB): + connection = cdm_database.connection_for_role(role) + detail = detail.replace(connection.url, connection.safe_url) + primary = cdm_database.connection_for_role(Role.PRIMARY) + vocab = cdm_database.connection_for_role(Role.VOCAB) + endpoints = ( + "the configured CDM database endpoint" + if primary.url == vocab.url + else "the primary and vocabulary database endpoints" + ) + raise DatabaseUnavailableError( + f"Could not initialize Groundworkers CDM database {cdm_database.name!r}. " + f"Check {endpoints}, credentials, configured schemas, and user access. " + f"({type(exc).__name__}: {detail})" + ) from exc + cdm_session_factory = cdm_sessionmaker( + cdm_database, + primary=cdm_engine, + vocab=cdm_vocab_engine, + ) embedding_model = ( resolver.resolve_model(groundworkers.embedding_model_name) @@ -81,6 +95,8 @@ def build_app_config_from_stack(stack: StackConfig) -> AppConfig: groundworkers=groundworkers, cdm_database=cdm_database, cdm_engine=cdm_engine, + cdm_vocab_engine=cdm_vocab_engine, + cdm_session_factory=cdm_session_factory, embedding_model=embedding_model, llm_model=llm_model, vector_store=vector_store, diff --git a/src/groundworkers/config.py b/src/groundworkers/config.py index 2e9d226..4b9880a 100644 --- a/src/groundworkers/config.py +++ b/src/groundworkers/config.py @@ -1,5 +1,6 @@ from __future__ import annotations +from collections.abc import Callable from dataclasses import dataclass from pathlib import Path from typing import Annotated, Any, ClassVar @@ -12,12 +13,14 @@ ResolvedCDMDatabase, ResolvedModel, ResolvedVectorStore, + Role, StackConfig, VectorStoreConfig, safe_endpoint, ) from pydantic import Field, field_validator from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session class GroundworkersConfig(PackageConfigBase): @@ -183,37 +186,6 @@ def validate_rest_base_path(cls, value: str) -> str: return value.rstrip("/") or "/" -def split_vocabulary_connection( - stack: StackConfig, cdm_db: str -) -> tuple[str, str] | None: - """Report a CDM entry whose vocabulary sits on a second physical connection. - - Groundworkers reads the vocabulary through the CDM engine and nothing else: - the knowledge graph, the vocabulary service, and the embedding tier are all - handed ``AppConfig.cdm_engine``, with only the vocabulary *schema* applied on - top. A ``vocab_connection`` naming a different server is therefore not a - supported split -- it would be silently ignored and the vocabulary schema - looked for on the CDM server instead. Reported here so both the runtime and - the setup console can refuse it in the same terms, rather than plumbing a - second engine no caller reads. - - Returns - ------- - tuple[str, str] or None - The ``(primary, vocabulary)`` connection names when they differ, or - ``None`` when the entry is absent, is not a CDM entry, or names one - connection for both roles. - """ - - database = stack.databases.get(cdm_db) - if not isinstance(database, CDMDatabaseConfig): - return None - vocabulary = database.vocab_connection - if vocabulary is None or vocabulary == database.connection: - return None - return (database.connection, vocabulary) - - @dataclass(frozen=True, repr=False) class AppConfig: """Resolved runtime configuration owned and consumed by Groundworkers. @@ -232,6 +204,8 @@ class AppConfig: groundworkers: GroundworkersConfig cdm_database: ResolvedCDMDatabase cdm_engine: Engine + cdm_vocab_engine: Engine + cdm_session_factory: Callable[[], Session] embedding_model: ResolvedModel | None llm_model: ResolvedModel | None vector_store: ResolvedVectorStore | None @@ -275,13 +249,13 @@ def describe(self) -> dict[str, Any]: "name": database.name, "connection": database.connection.name, "safe_url": safe_endpoint(database.connection.safe_url), - "schema_name": database.schema_name, + "cdm_schema": database.schema_for_role(Role.PRIMARY), "vocabulary_connection": database.vocab_connection.name, "vocabulary_safe_url": safe_endpoint( database.vocab_connection.safe_url ), - "vocabulary_schema": database.vocab_schema, - "results_schema": database.results_schema, + "vocab_schema": database.schema_for_role(Role.VOCAB), + "results_schema": database.schema_for_role(Role.RESULTS), }, "model": _describe_model(self.embedding_model), "llm_model": _describe_model(self.llm_model), diff --git a/src/groundworkers/plugins.py b/src/groundworkers/plugins.py index cfdfec0..b9950eb 100644 --- a/src/groundworkers/plugins.py +++ b/src/groundworkers/plugins.py @@ -267,6 +267,8 @@ class PluginContext: embedding_model_backend_factory: Callable[[], Any] | None chat_backend_factory: Callable[[], Any] | None mapping_context: MappingContext | None = None + cdm_session_factory: Callable[[], Any] | None = None + cdm_vocab_engine: Engine | None = None class GroundworkersPlugin(Protocol): diff --git a/src/groundworkers/server.py b/src/groundworkers/server.py index fbed66f..e5e3851 100644 --- a/src/groundworkers/server.py +++ b/src/groundworkers/server.py @@ -23,8 +23,9 @@ from oa_configurator import ConfigurationError from groundworkers.app import GroundworkersApp, build_application +from groundworkers.base.errors import scrub_error_message from groundworkers.base.server import GroundworkersMCPServer -from groundworkers.bootstrap import build_app_config +from groundworkers.bootstrap import DatabaseUnavailableError, build_app_config from groundworkers.config import AppConfig, GroundworkersConfig from groundworkers.services.semantic_projection.service import SemanticProjectionService from groundworkers.tools.concept_tools import register_concept_tools @@ -293,10 +294,17 @@ def _configuration_failure( detail = f"Configuration file not found at {selected}." elif "Malformed TOML" in str(exc): detail = f"Configuration at {selected} is not valid TOML." - elif isinstance(exc, ConfigurationError): + elif isinstance(exc, DatabaseUnavailableError): + detail = " ".join(scrub_error_message(str(exc)).splitlines()).strip() + elif isinstance(exc, (ConfigurationError, ValueError)): detail = f"Configuration at {selected} is incomplete or invalid." + first_line = scrub_error_message(str(exc).splitlines()[0]).strip() + if first_line: + detail += f" {first_line}" else: detail = f"Configuration at {selected} could not be loaded." + if isinstance(exc, DatabaseUnavailableError): + return detail return f"{detail} Run '{recovery}' to choose or repair it." diff --git a/src/groundworkers/services/graph.py b/src/groundworkers/services/graph.py index f24608a..eea607c 100644 --- a/src/groundworkers/services/graph.py +++ b/src/groundworkers/services/graph.py @@ -292,17 +292,29 @@ def _query_classified_edges( outbound: bool, ) -> list[dict[str, Any]]: cr = Concept_Relationship - rm = _RELATIONSHIP_MAPPING seed_col = cr.concept_id_1 if outbound else cr.concept_id_2 other_col = cr.concept_id_2 if outbound else cr.concept_id_1 - - # predicate_kind / predicate_subkind are Postgres enum columns, so cast to - # text for comparison and to return plain strings. - subkind_text = cast(rm.c.predicate_subkind, Text) + rm = RelationshipMapping.__table__ + mapping_rows = session.execute( + select( + rm.c.relationship_id, + cast(rm.c.predicate_subkind, Text).label("predicate_subkind"), + ).where(cast(rm.c.predicate_kind, Text) == predicate_kind), + bind_arguments={"mapper": RelationshipMapping}, + ).all() + subkind_by_relationship = { + row.relationship_id: row.predicate_subkind for row in mapping_rows + } + relationship_ids = [ + relationship_id + for relationship_id, subkind in subkind_by_relationship.items() + if not predicate_subkinds or subkind in predicate_subkinds + ] + if not relationship_ids: + return [] stmt = ( select( cr.relationship_id, - subkind_text.label("predicate_subkind"), Concept.concept_id, Concept.concept_name, Concept.vocabulary_id, @@ -312,15 +324,15 @@ def _query_classified_edges( cr.invalid_reason, ) .select_from(cr) - .join(rm, rm.c.relationship_id == cr.relationship_id) .join(Concept, Concept.concept_id == other_col) - .where(seed_col == concept_id, cast(rm.c.predicate_kind, Text) == predicate_kind) + .where( + seed_col == concept_id, + cr.relationship_id.in_(relationship_ids), + ) ) - if predicate_subkinds: - stmt = stmt.where(subkind_text.in_(list(predicate_subkinds))) if active_only: stmt = stmt.where(cr.is_valid_expr()) - stmt = stmt.order_by(subkind_text, Concept.concept_name).limit(limit) + stmt = stmt.order_by(Concept.concept_name).limit(limit) id_key = "target_concept_id" if outbound else "source_concept_id" name_key = "target_concept_name" if outbound else "source_concept_name" @@ -329,7 +341,7 @@ def _query_classified_edges( edges.append( { "relationship_id": row.relationship_id, - "predicate_subkind": row.predicate_subkind, + "predicate_subkind": subkind_by_relationship[row.relationship_id], id_key: int(row.concept_id), name_key: row.concept_name, "vocabulary_id": row.vocabulary_id, diff --git a/src/groundworkers/services/vocab.py b/src/groundworkers/services/vocab.py index 288507c..d963819 100644 --- a/src/groundworkers/services/vocab.py +++ b/src/groundworkers/services/vocab.py @@ -7,6 +7,7 @@ from dataclasses import dataclass, field from typing import Any +from oa_configurator import Role, physical_schema_of from omop_alchemy.cdm.model.vocabulary import ( Concept, Concept_Ancestor, @@ -24,7 +25,6 @@ from groundworkers.base.errors import GroundworkersError from groundworkers.base.sql import ( NormalizationProfile, - effective_schema, normalization_expression, normalization_key_expression, normalized_key, @@ -129,7 +129,7 @@ class VocabService: def __init__(self, cdm: CDMAdapter) -> None: self._cdm = cdm - self._domain_names = DomainNameResolver(cdm.engine) + self._domain_names = DomainNameResolver(cdm.vocab_engine) self._fts_name_sidecar: bool | None = None self._fts_synonym_sidecar: bool | None = None @@ -149,9 +149,10 @@ def _detect_fts_sidecars(self) -> None: """ if self._fts_name_sidecar is not None: return - schema = effective_schema(self._cdm.engine) + engine = self._cdm.vocab_engine + schema = physical_schema_of(engine, schema_tag=Role.VOCAB) try: - inspector = sa_inspect(self._cdm.engine) + inspector = sa_inspect(engine) concept_cols = { c["name"] for c in inspector.get_columns("concept", schema=schema) } @@ -172,8 +173,8 @@ def _detect_fts_sidecars(self) -> None: schema, exc_info=True, ) - self._fts_name_sidecar = False - self._fts_synonym_sidecar = False + self._fts_name_sidecar = None + self._fts_synonym_sidecar = None @property def fts_available(self) -> bool: @@ -413,7 +414,7 @@ def search_normalized_with_plan( with self._cdm.session() as session: if ( - self._cdm.engine.dialect.name == "postgresql" + self._cdm.vocab_engine.dialect.name == "postgresql" and plan != "unindexed_scan" ): session.execute( @@ -527,7 +528,7 @@ def _normalized_search_plan( include_synonyms: bool, allow_unindexed_scan: bool, ) -> str: - engine = self._cdm.engine + engine = self._cdm.vocab_engine indexed = engine.dialect.name == "postgresql" and self._has_normalized_index( "concept", "concept_name" ) @@ -549,7 +550,7 @@ def _normalized_search_plan( ) def _has_normalized_index(self, table: str, column_name: str) -> bool: - engine = self._cdm.engine + engine = self._cdm.vocab_engine key = (engine, table, column_name) now = time.monotonic() with _NORMALIZED_INDEX_CACHE_LOCK: @@ -564,12 +565,21 @@ def _has_normalized_index(self, table: str, column_name: str) -> bool: with engine.connect() as connection: found = normalized_index_matches( connection, - effective_schema(connection), + physical_schema_of(connection, schema_tag=Role.VOCAB), table, column_name, ) - except Exception: - found = False + except Exception as exc: + logger.warning( + "Could not check normalized-name index %s.%s; search availability is unknown.", + table, + column_name, + exc_info=True, + ) + raise GroundworkersError( + "BACKEND_UNAVAIL", + f"Could not check the normalized-name index for {table}.{column_name} ({type(exc).__name__}).", + ) from exc with _NORMALIZED_INDEX_CACHE_LOCK: _NORMALIZED_INDEX_CACHE[key] = (found, now) return found diff --git a/src/groundworkers/transports/rest/api.py b/src/groundworkers/transports/rest/api.py index 74dc53e..460e5a8 100644 --- a/src/groundworkers/transports/rest/api.py +++ b/src/groundworkers/transports/rest/api.py @@ -154,6 +154,11 @@ def _status_code_for_error(code: str) -> int: "NOT_FOUND": 404, "BACKEND_UNAVAIL": 503, "MISSING_DEPENDENCY": 503, + "LEGACY_REGISTRY": 503, + "MISPLACED_REGISTRY": 503, + "MISSING_STORAGE_TABLE": 503, + "MODEL_REGISTRATION_CONFLICT": 409, + "READ_ONLY_STORE": 409, "QUERY_ERROR": 500, "INTERNAL_ERROR": 500, }.get(code, 500) diff --git a/src/groundworkers/tui/pages/setup.py b/src/groundworkers/tui/pages/setup.py index 8bd323a..8e14726 100644 --- a/src/groundworkers/tui/pages/setup.py +++ b/src/groundworkers/tui/pages/setup.py @@ -8,6 +8,7 @@ ConfigWizardController, ConfigWorkflowSpec, MutationOperation, + UnavailableMutationService, resolve_operation, ) from groundskeeping.contracts import ( @@ -579,12 +580,11 @@ def _open_config_wizard( model_discoverer=discover_provider_models, on_applied=self._session.refresh_configuration, ) - context.open_wizard( - ConfigWizardController( - workflow(resolve_operation(service, target)), - service, - ) - ) + try: + operation = resolve_operation(service, target) + context.open_wizard(ConfigWizardController(workflow(operation), service)) + except UnavailableMutationService as exc: + context.notify(str(exc), severity="warning") def _start_connection_checks(self, context: PageContext) -> None: targets = resolve_database_targets(self._session.configuration) diff --git a/src/groundworkers/tui/presenters/overview.py b/src/groundworkers/tui/presenters/overview.py index b284ad4..92e2d48 100644 --- a/src/groundworkers/tui/presenters/overview.py +++ b/src/groundworkers/tui/presenters/overview.py @@ -55,11 +55,12 @@ def landing( embedding_reconciliation: ModelReconciliation | None = None, ) -> SurfaceView: if not snapshot.usable: + issues = " ".join(issue.message for issue in snapshot.issues) return EmptyView( - title="Start Groundworkers", + title=("Repair Groundworkers configuration" if issues else "Start Groundworkers"), message=( - f"Configuration destination: {snapshot.path}. " - "Choose a location, then configure the required CDM connection." + f"Configuration: {snapshot.path}. " + + (f"Issues: {issues}" if issues else "Choose a location, then configure the required CDM connection.") ), status=SemanticStatus.WARNING, actions=( diff --git a/tests/integration/test_omop_graph_adapter.py b/tests/integration/test_omop_graph_adapter.py index fc002f0..4a42df5 100644 --- a/tests/integration/test_omop_graph_adapter.py +++ b/tests/integration/test_omop_graph_adapter.py @@ -1,7 +1,11 @@ import os import pytest -from sqlalchemy import text +from oa_configurator import Role, physical_schema_of +from omop_alchemy.cdm.model.vocabulary import Concept, Concept_Relationship +from sqlalchemy import exists, func, select +from sqlalchemy import inspect as sa_inspect +from sqlalchemy.orm import aliased from groundworkers.app import build_adapters from groundworkers.bootstrap import build_app_config @@ -9,17 +13,6 @@ from groundworkers.services.grounding import ConceptGroundingService -def _vocab_schema(adapter) -> str: - """Return the schema selected by the resolved SQLAlchemy engine.""" - - schema_map = adapter.engine.get_execution_options().get("schema_translate_map", {}) - return str( - schema_map.get(None) - or adapter.engine.dialect.default_schema_name - or "public" - ) - - def _load_graph_adapter(): try: import omop_graph # noqa: F401 @@ -48,41 +41,47 @@ def _load_graph_adapter(): pytest.skip("omop_graph adapter was not built") if not adapter.is_available(): pytest.skip("omop_graph backend is not available in this environment") + adapter._integration_vocab_engine = config.cdm_vocab_engine return adapter def _find_nonstandard_condition_term(adapter) -> str: - schema = _vocab_schema(adapter) - stmt = text( - f""" - SELECT c.concept_name - FROM {schema}.concept AS c - JOIN {schema}.concept_relationship AS cr - ON cr.concept_id_1 = c.concept_id - AND lower(cr.relationship_id) = 'maps to' - AND cr.invalid_reason IS NULL - JOIN {schema}.concept AS s - ON s.concept_id = cr.concept_id_2 - AND s.standard_concept = 'S' - AND s.domain_id = 'Condition' - AND s.invalid_reason IS NULL - WHERE c.domain_id = 'Condition' - AND c.standard_concept IS NULL - AND c.invalid_reason IS NULL - AND NOT EXISTS ( - SELECT 1 - FROM {schema}.concept AS c2 - WHERE lower(c2.concept_name) = lower(c.concept_name) - AND c2.domain_id = 'Condition' - AND c2.standard_concept IN ('S', 'C') - AND c2.invalid_reason IS NULL - ) - ORDER BY c.concept_id - LIMIT 1 - """ + standard = aliased(Concept) + same_name = aliased(Concept) + stmt = ( + select(Concept.concept_name) + .join( + Concept_Relationship, + (Concept_Relationship.concept_id_1 == Concept.concept_id) + & (func.lower(Concept_Relationship.relationship_id) == "maps to") + & Concept_Relationship.invalid_reason.is_(None), + ) + .join( + standard, + (standard.concept_id == Concept_Relationship.concept_id_2) + & (standard.standard_concept == "S") + & (standard.domain_id == "Condition") + & standard.invalid_reason.is_(None), + ) + .where( + Concept.domain_id == "Condition", + Concept.standard_concept.is_(None), + Concept.invalid_reason.is_(None), + ~exists( + select(1).where( + func.lower(same_name.concept_name) + == func.lower(Concept.concept_name), + same_name.domain_id == "Condition", + same_name.standard_concept.in_(("S", "C")), + same_name.invalid_reason.is_(None), + ) + ), + ) + .order_by(Concept.concept_id) + .limit(1) ) - with adapter.engine.connect() as conn: - row = conn.execute(stmt).first() + with adapter._cdm_session_factory() as session: + row = session.execute(stmt).first() if row is None: pytest.skip( "no uniquely named non-standard Condition concept with a standard mapping was found" @@ -93,23 +92,25 @@ def _find_nonstandard_condition_term(adapter) -> str: def _skip_if_parent_lookup_is_unindexed(adapter) -> None: """Skip hierarchy smoke checks when the live vocabulary DB is not shaped for them.""" - schema = _vocab_schema(adapter) - stmt = text( - """ - SELECT indexdef - FROM pg_indexes - WHERE schemaname = :schema - AND tablename = 'concept_ancestor' - """ - ) try: - with adapter.engine.connect() as conn: - rows = conn.execute(stmt, {"schema": schema}).scalars().all() + schema = physical_schema_of( + adapter._integration_vocab_engine, schema_tag=Role.VOCAB + ) + indexes = sa_inspect(adapter._integration_vocab_engine).get_indexes( + "concept_ancestor", schema=schema + ) except Exception as exc: # Broad except: integration environment preflight. pytest.skip(f"could not inspect concept_ancestor indexes: {exc}") - if not any("(descendant_concept_id" in indexdef.lower() for indexdef in rows): + if not any( + "descendant_concept_id" in (index.get("column_names") or []) + or any( + "descendant_concept_id" in str(expression).lower() + for expression in (index.get("expressions") or ()) + ) + for index in indexes + ): pytest.skip( "concept_ancestor lacks a descendant-leading index; " "parent hierarchy lookup is an environment smoke test, not an application regression test" @@ -256,17 +257,13 @@ def test_standard_flags_distinguish_s_from_c(): """The strict flag contract distinguishes raw OMOP ``S`` and ``C`` rows.""" adapter = _load_graph_adapter() - schema = _vocab_schema(adapter) - stmt = text( - f""" - SELECT standard_concept, MIN(concept_id) AS concept_id - FROM {schema}.concept - WHERE standard_concept IN ('S', 'C') - GROUP BY standard_concept - """ + stmt = ( + select(Concept.standard_concept, func.min(Concept.concept_id)) + .where(Concept.standard_concept.in_(("S", "C"))) + .group_by(Concept.standard_concept) ) - with adapter.engine.connect() as conn: - sampled = {row[0]: int(row[1]) for row in conn.execute(stmt).all()} + with adapter._cdm_session_factory() as session: + sampled = {row[0]: int(row[1]) for row in session.execute(stmt).all()} if set(sampled) != {"S", "C"}: pytest.skip("vocabulary does not contain both 'S' and 'C' concepts") @@ -282,33 +279,31 @@ def test_classification_concept_is_not_standard(): """A classification concept is never exposed as a standard mapping target.""" adapter = _load_graph_adapter() - schema = _vocab_schema(adapter) - stmt = text( - f""" - SELECT concept_name - FROM {schema}.concept - WHERE standard_concept = 'C' - AND domain_id = 'Drug' - AND invalid_reason IS NULL - AND NOT EXISTS ( - SELECT 1 - FROM {schema}.concept AS other - WHERE lower(other.concept_name) = lower(concept.concept_name) - AND other.standard_concept = 'S' - ) - ORDER BY concept_id - LIMIT 1 - """ + other = aliased(Concept) + stmt = ( + select(Concept.concept_name) + .where( + Concept.standard_concept == "C", + Concept.domain_id == "Drug", + Concept.invalid_reason.is_(None), + ~exists( + select(1).where( + func.lower(other.concept_name) == func.lower(Concept.concept_name), + other.standard_concept == "S", + ) + ), + ) + .order_by(Concept.concept_id) + .limit(1) ) - with adapter.engine.connect() as conn: - row = conn.execute(stmt).first() + with adapter._cdm_session_factory() as session: + row = session.execute(stmt).first() if row is None: pytest.skip("no uniquely named classification Drug concept was found") - with adapter.engine.connect() as conn: - concept_id = conn.execute( - text(f"SELECT concept_id FROM {schema}.concept WHERE concept_name = :name"), - {"name": row[0]}, + with adapter._cdm_session_factory() as session: + concept_id = session.execute( + select(Concept.concept_id).where(Concept.concept_name == row[0]) ).scalar_one() classification = adapter.get_concept(int(concept_id)) diff --git a/tests/support/omop.py b/tests/support/omop.py new file mode 100644 index 0000000..f99af29 --- /dev/null +++ b/tests/support/omop.py @@ -0,0 +1,15 @@ +"""SQLAlchemy test engines routed through OMOP 2.0 role tags.""" + +from sqlalchemy.engine import Engine + + +def route_sqlite_omop_roles(engine: Engine) -> Engine: + """Route OMOP roles to SQLite's default database for lightweight tests.""" + + return engine.execution_options( + schema_translate_map={ + "primary": None, + "vocab": None, + "results": None, + } + ) diff --git a/tests/support/seed_postgres_fixture.py b/tests/support/seed_postgres_fixture.py index 9eef78f..f2f4f89 100644 --- a/tests/support/seed_postgres_fixture.py +++ b/tests/support/seed_postgres_fixture.py @@ -10,6 +10,7 @@ import os from datetime import date +from oa_configurator import Resolver, load_stack_config from omop_alchemy.cdm.model.vocabulary import ( # noqa: F401 Concept, Concept_Ancestor, @@ -41,7 +42,10 @@ def _database_url() -> str: def seed() -> None: engine = create_engine(_database_url(), future=True) - bootstrap(engine, create=True) + cdm = Resolver( + load_stack_config(os.environ["GROUNDWORKERS_CONFIG_PATH"]) + ).resolve_database("test_cdm_db") + bootstrap(cdm, create=True) concepts = [ (201826, "Type 2 diabetes mellitus", "Condition", "SNOMED", "Clinical Finding", "S", "44054006"), diff --git a/tests/support/stack_config.py b/tests/support/stack_config.py index 99b28e8..0ffe446 100644 --- a/tests/support/stack_config.py +++ b/tests/support/stack_config.py @@ -1,4 +1,4 @@ -"""oa-configurator 1.x stack fixtures shared across migration tests.""" +"""oa-configurator 2.0 stack fixtures shared across migration tests.""" from __future__ import annotations @@ -28,28 +28,37 @@ def build_cdm_stack( connection_name: str = "cdm_main", database_name: str = "cdm_db", sqlite_path: str = ":memory:", - schema_name: str = "main", - vocab_schema: str | None = "main", + cdm_schema: str | None = None, + vocab_schema: str | None = None, results_schema: str | None = None, groundworkers: dict[str, object] | None = None, ) -> StackConfig: """Build the smallest valid Groundworkers CDM-only stack.""" + if any((cdm_schema, vocab_schema, results_schema)): + connection = ConnectionConfig( + dialect="postgresql+psycopg", + host="postgres.example.test", + port=5432, + user="groundworkers", + password="fixture-password", + database_name="cdm", + ) + else: + connection = ConnectionConfig(dialect="sqlite", database_name=sqlite_path) + tool = { "cdm_db": database_name, **(groundworkers or {}), } return StackConfig( connections={ - connection_name: ConnectionConfig( - dialect="sqlite", - database_name=sqlite_path, - ) + connection_name: connection }, databases={ database_name: CDMDatabaseConfig( connection=connection_name, - schema_name=schema_name, + cdm_schema=cdm_schema, vocab_schema=vocab_schema, results_schema=results_schema, ) @@ -81,7 +90,7 @@ def build_embedding_stack( dialect="sqlite", database_name="embeddings.db", ) - schema_name = "main" + schema_name = None else: connection = ConnectionConfig( dialect="postgresql+psycopg", @@ -94,10 +103,10 @@ def build_embedding_stack( schema_name = "groundworkers" stack.connections[vector_connection_name] = connection - stack.databases[vector_database_name] = GenericDatabaseConfig( - connection=vector_connection_name, - schema_name=schema_name, - ) + generic_database = {"connection": vector_connection_name} + if schema_name is not None: + generic_database["schema_name"] = schema_name + stack.databases[vector_database_name] = GenericDatabaseConfig(**generic_database) stack.providers[provider_name] = ProviderConfig( provider="ollama", base_url="http://models.example.test", @@ -159,7 +168,6 @@ def build_invalid_reference_stack(issue: InvalidReference) -> StackConfig: elif issue == "wrong_cdm_kind": stack.databases["cdm_db"] = GenericDatabaseConfig( connection="cdm_main", - schema_name="main", ) elif issue == "missing_model": tool["embedding_model_name"] = "missing_model" diff --git a/tests/test_bootstrap_config.py b/tests/test_bootstrap_config.py index 7fbb705..382f938 100644 --- a/tests/test_bootstrap_config.py +++ b/tests/test_bootstrap_config.py @@ -8,12 +8,20 @@ CDMDatabaseConfig, ConfigurationError, ConnectionConfig, - load_stack_config_from_path, + Resolver, + Role, + load_stack_config, ) from oa_configurator.io import save_stack_config +from sqlalchemy import create_engine -from groundworkers.bootstrap import build_app_config_from_stack +from groundworkers.bootstrap import ( + DatabaseUnavailableError, + build_app_config, + build_app_config_from_stack, +) from groundworkers.config import GroundworkersConfig +from groundworkers.server import _configuration_failure from tests.support.stack_config import ( add_chat_model, build_cdm_stack, @@ -67,7 +75,7 @@ def test_config_loading_contract_relied_on_by_bootstrap( config_path.chmod(0o644) with caplog.at_level(logging.WARNING, logger="oa_configurator.logging_config"): - stack = load_stack_config_from_path(config_path) + stack = load_stack_config(config_path) assert stack.loaded_path == config_path # Gained by adopting upstream: the local copy never warned about a config @@ -82,7 +90,47 @@ def test_malformed_config_is_rejected(tmp_path: Path) -> None: ) with pytest.raises(ValueError, match="Malformed TOML"): - load_stack_config_from_path(config_path) + load_stack_config(config_path) + + +def test_startup_failure_includes_scrubbed_legacy_config_reason(tmp_path: Path) -> None: + path = tmp_path / "legacy.toml" + path.write_text( + '[connections.main]\ndialect = "sqlite"\ndatabase_name = ":memory:"\n' + '[databases.cdm_db]\nkind = "cdm"\nconnection = "main"\n' + 'schema_name = "public"\npassword = "review-secret"\n', + encoding="utf-8", + ) + + with pytest.raises(ConfigurationError) as raised: + build_app_config(config_path=path) + message = _configuration_failure(raised.value, config_path=str(path)) + + assert "1.x" in message or "schema_name" in message or "upgrade" in message + assert "review-secret" not in message + + +def test_startup_failure_identifies_closed_database_without_credentials( + tmp_path: Path, +) -> None: + path = tmp_path / "closed-port.toml" + path.write_text( + '[connections.main]\ndialect = "postgresql+psycopg"\n' + 'host = "127.0.0.1"\nport = 1\nuser = "review"\n' + 'password = "review-secret"\ndatabase_name = "closed"\n' + '[databases.cdm_db]\nkind = "cdm"\nconnection = "main"\n' + '[tools.groundworkers]\ncdm_db = "cdm_db"\n', + encoding="utf-8", + ) + + with pytest.raises(DatabaseUnavailableError) as raised: + build_app_config(config_path=path) + message = _configuration_failure(raised.value, config_path=str(path)) + + assert "Check the configured CDM database endpoint" in message + assert "both role databases" not in message + assert message.count("(") == message.count(")") + assert "review-secret" not in message def test_config_validation_error_does_not_echo_rejected_secret( @@ -101,7 +149,7 @@ def test_config_validation_error_does_not_echo_rejected_secret( ) with pytest.raises(ConfigurationError) as raised: - load_stack_config_from_path(config_path) + load_stack_config(config_path) assert "rejected-secret" not in str(raised.value) @@ -113,7 +161,7 @@ def test_build_app_config_resolves_relative_packs_root(tmp_path: Path) -> None: config_path, ) - config = build_app_config_from_stack(load_stack_config_from_path(config_path)) + config = build_app_config_from_stack(load_stack_config(config_path)) assert config.knowledge_root == (tmp_path / "relative-packs").resolve() @@ -131,19 +179,15 @@ def test_a_vocabulary_connection_naming_the_cdm_connection_is_accepted() -> None stack = build_cdm_stack() stack.databases["cdm_db"] = CDMDatabaseConfig( connection="cdm_main", - schema_name="main", vocab_connection="cdm_main", - vocab_schema="vocab", ) config = build_app_config_from_stack(stack) - assert config.cdm_database.vocab_schema == "vocab" + assert config.cdm_database.vocab_connection.name == "cdm_main" -def test_distinct_vocabulary_connection_is_refused() -> None: - """The graph, the vocabulary service, and the embedding tier all read the CDM - engine; a second connection would be silently ignored.""" +def test_distinct_vocabulary_connection_is_routed_to_a_second_engine() -> None: stack = build_cdm_stack() stack.connections["vocab_main"] = ConnectionConfig( dialect="sqlite", @@ -151,14 +195,38 @@ def test_distinct_vocabulary_connection_is_refused() -> None: ) stack.databases["cdm_db"] = CDMDatabaseConfig( connection="cdm_main", - schema_name="main", vocab_connection="vocab_main", - vocab_schema="main", ) - with pytest.raises(ConfigurationError, match="vocab_schema"): + config = build_app_config_from_stack(stack) + + assert config.cdm_vocab_engine.url.database == "vocabulary.db" + assert config.cdm_engine.url.database == ":memory:" + config.cdm_engine.dispose() + config.cdm_vocab_engine.dispose() + + +def test_cdm_engine_initialization_error_includes_safe_cause(monkeypatch) -> None: + import groundworkers.bootstrap as bootstrap + + stack = build_cdm_stack(cdm_schema="clinical", vocab_schema="vocabulary") + resolved = Resolver(stack).resolve_database("cdm_db") + unsafe_url = resolved.connection_for_role(Role.PRIMARY).url + + def fail(*_args, **_kwargs): + raise RuntimeError(f"schema registry rejected {unsafe_url}") + + monkeypatch.setattr(bootstrap, "create_cdm_engines", fail) + + with pytest.raises(ConfigurationError) as raised: build_app_config_from_stack(stack) + message = str(raised.value) + assert "Check the configured CDM database endpoint" in message + assert "RuntimeError: schema registry rejected" in message + assert "fixture-password" not in message + assert "***" in message + def test_optional_model_and_vector_store_are_resolved_without_building_backends() -> ( None @@ -177,7 +245,7 @@ def test_invalid_cdm_reference_is_rejected_at_bootstrap(issue: str) -> None: build_app_config_from_stack(build_invalid_reference_stack(issue)) # type: ignore[arg-type] -def test_describe_masks_connection_and_provider_secrets() -> None: +def test_describe_masks_connection_and_provider_secrets(monkeypatch) -> None: stack = build_embedding_stack() add_chat_model( stack, @@ -197,6 +265,11 @@ def test_describe_masks_connection_and_provider_secrets() -> None: "https://user:pass@models.example.test/v1?api_key=query-secret#fragment-secret" ) + import groundworkers.bootstrap as bootstrap + + primary = create_engine("sqlite:///:memory:") + monkeypatch.setattr(bootstrap, "create_cdm_engines", lambda *_args, **_kw: (primary, primary)) + monkeypatch.setattr(bootstrap, "cdm_sessionmaker", lambda *_args, **_kw: lambda: None) config = build_app_config_from_stack(stack) rendered = repr(config.describe()) runtime_repr = repr(config) @@ -216,7 +289,7 @@ def test_the_shipped_example_config_loads() -> None: example = Path(__file__).resolve().parents[1] / "config" / "groundworkers.example.toml" - config = GroundworkersConfig.validate_candidate(load_stack_config_from_path(example)) + config = GroundworkersConfig.validate_candidate(load_stack_config(example)) assert config.cdm_db == "cdm_db" assert config.embedding_model_name == "embedding_model" @@ -249,7 +322,7 @@ def test_endpoint_redaction_contract_relied_on_by_describe() -> None: assert safe_endpoint(None) is None -def test_describe_leaks_no_value_declared_sensitive_by_the_schema() -> None: +def test_describe_leaks_no_value_declared_sensitive_by_the_schema(monkeypatch) -> None: """Trust the Sensitive() declaration; verify the pipeline honours it. Asserts no field oa-configurator marks Sensitive() reaches `describe()`, @@ -268,6 +341,11 @@ def test_describe_leaks_no_value_declared_sensitive_by_the_schema() -> None: database_name="omop", ) + import groundworkers.bootstrap as bootstrap + + primary = create_engine("sqlite:///:memory:") + monkeypatch.setattr(bootstrap, "create_cdm_engines", lambda *_args, **_kw: (primary, primary)) + monkeypatch.setattr(bootstrap, "cdm_sessionmaker", lambda *_args, **_kw: lambda: None) rendered = build_app_config_from_stack(stack).describe() for provider in stack.providers.values(): diff --git a/tests/test_rest_api.py b/tests/test_rest_api.py index b78ea8b..5da16b2 100644 --- a/tests/test_rest_api.py +++ b/tests/test_rest_api.py @@ -13,6 +13,7 @@ ) from groundworkers.services.source_planning.warnings import PlanningWarning from groundworkers.transports.rest import create_rest_app +from groundworkers.transports.rest.api import _status_code_for_error from tests.support.stack_config import build_cdm_stack @@ -251,3 +252,17 @@ def plan_source_assisted(self, content: bytes, *, filename: str | None = None, c "code": "FORMAT_UNRECOGNISED", "message": "unrecognised source format", } + + +@pytest.mark.parametrize( + ("code", "status"), + ( + ("LEGACY_REGISTRY", 503), + ("MISPLACED_REGISTRY", 503), + ("MISSING_STORAGE_TABLE", 503), + ("MODEL_REGISTRATION_CONFLICT", 409), + ("READ_ONLY_STORE", 409), + ), +) +def test_embedding_store_error_codes_map_to_conflict_or_unavailable(code, status): + assert _status_code_for_error(code) == status diff --git a/tests/test_server_registry.py b/tests/test_server_registry.py index 8ca7a8a..65165b4 100644 --- a/tests/test_server_registry.py +++ b/tests/test_server_registry.py @@ -24,11 +24,11 @@ def _stack_without_backends(): def _stack_with_cdm(): - return build_cdm_stack(schema_name="omop", vocab_schema="omop_vocab") + return build_cdm_stack() def test_cdm_only_stack_serves_graph_tools_without_embedding_tools(): - """A CDM-only 1.x stack gets vocabulary, graph, and lexical grounding. + """A CDM-only stack gets vocabulary, graph, and lexical grounding. Graph availability follows the resolved CDM database. It is not gated on an [tools.omop_graph] section: omop-graph 2.x made that config internal, so @@ -113,7 +113,7 @@ def resolve(resolved_store): return backend monkeypatch.setattr( - "omop_emb.backends.resolve_backend_from_resolved_vector_store", resolve + "omop_emb.backends.open_vector_store_reader", resolve ) adapters = build_adapters(config) diff --git a/tests/unit/test_configuration_provider.py b/tests/unit/test_configuration_provider.py index d6d4dd4..0fe218b 100644 --- a/tests/unit/test_configuration_provider.py +++ b/tests/unit/test_configuration_provider.py @@ -1,5 +1,6 @@ from __future__ import annotations +import tomllib from dataclasses import asdict from pathlib import Path @@ -22,8 +23,10 @@ from oa_configurator import ( ConfigurationError, GenericDatabaseConfig, + Resolver, + Role, StackConfig, - load_stack_config_from_path, + load_stack_config, save_stack_config, ) @@ -37,6 +40,7 @@ ) from groundworkers.config import GroundworkersConfig from tests.support import build_cdm_stack +from tests.support.stack_config import build_embedding_stack CDM_SUBMISSIONS = ( ( @@ -51,8 +55,8 @@ "cdm", { "cdm_db_name": "cdm_db", - "schema_name": "main", - "vocab_schema": "main", + "cdm_schema": None, + "vocab_schema": None, "results_schema": None, }, ), @@ -68,6 +72,55 @@ def test_groundworkers_config_uses_real_1x_references() -> None: assert candidate.databases["cdm_db"].connection == "cdm_main" +@pytest.mark.parametrize( + ("dialect", "cdm_schema", "vocab_schema"), + [("sqlite", None, None), ("postgresql+psycopg", "clinical", "vocabulary")], +) +def test_cdm_wizard_output_round_trips_through_stack_resolver( + tmp_path: Path, dialect: str, cdm_schema: str | None, vocab_schema: str | None +) -> None: + path = tmp_path / f"{dialect}.toml" + sqlite_path = tmp_path / "cdm.db" + sqlite_path.touch() + service = GroundworkersConfigMutationService(path) + controller = ConfigWizardController(cdm_setup_workflow(MutationOperation.CREATE), service) + controller.start() + controller.submit({"connection_name": "cdm_main", "dialect": dialect}) + if dialect != "sqlite": + controller.submit( + { + "host": "postgres.example.test", + "port": 5432, + "user": "groundworkers", + } + ) + controller.submit({"database_name": "omop"}) + else: + controller.submit({"database_name": str(sqlite_path)}) + controller.submit( + { + "cdm_db_name": "cdm_db", + "cdm_schema": cdm_schema, + "vocab_schema": vocab_schema, + "results_schema": None, + } + ) + assert controller.apply().status is WizardResultStatus.APPLIED + + loaded = load_stack_config(path) + resolved = Resolver(loaded).resolve_database("cdm_db") + assert resolved.connection.dialect_name == ( + "sqlite" if dialect == "sqlite" else "postgresql" + ) + assert resolved.schema_for_role(Role.PRIMARY) == cdm_schema + assert resolved.schema_for_role(Role.VOCAB) == vocab_schema + if dialect == "sqlite": + saved = tomllib.loads(path.read_text(encoding="utf-8")) + cdm_entry = saved["databases"]["cdm_db"] + assert "cdm_schema" not in cdm_entry + assert "schema_name" not in cdm_entry + + def test_groundworkers_config_rejects_missing_and_wrong_kind_references() -> None: missing = StackConfig(tools={"groundworkers": {"cdm_db": "missing"}}) wrong_kind = StackConfig( @@ -75,7 +128,6 @@ def test_groundworkers_config_rejects_missing_and_wrong_kind_references() -> Non databases={ "cdm_db": GenericDatabaseConfig( connection="cdm_main", - schema_name="main", ) }, tools={"groundworkers": {"cdm_db": "cdm_db"}}, @@ -162,9 +214,9 @@ def test_provider_updates_an_existing_cdm_configuration(tmp_path: Path) -> None: review = controller.submit( { "cdm_db_name": "cdm_db", - "schema_name": "clinical", - "vocab_schema": "vocabulary", - "results_schema": "results", + "cdm_schema": None, + "vocab_schema": None, + "results_schema": None, } ).snapshot assert isinstance(review.step, ReviewStep) @@ -174,7 +226,7 @@ def test_provider_updates_an_existing_cdm_configuration(tmp_path: Path) -> None: reloaded = load_configuration(config_path=path) assert reloaded.stack is not None assert reloaded.stack.connections["cdm_main"].database_name == "updated.db" - assert reloaded.stack.databases["cdm_db"].schema_name == "clinical" + assert reloaded.stack.databases["cdm_db"].cdm_schema is None def test_model_discovery_refreshes_only_the_future_choice_and_keeps_secrets_safe( @@ -255,6 +307,31 @@ def discover(provider: str, base_url: str | None, api_key: str | None): assert canary in path.read_text(encoding="utf-8") +@pytest.mark.parametrize( + ("discovered", "flagged"), + ((("stored-model", "other-model"), False), (("other-model",), True)), +) +def test_model_update_preserves_stored_choice_and_flags_missing_inventory( + tmp_path: Path, discovered: tuple[str, ...], flagged: bool +) -> None: + path = tmp_path / "config.toml" + save_stack_config(build_embedding_stack(model_name="stored-model"), path) + controller = ConfigWizardController( + model_setup_workflow(MutationOperation.UPDATE), + GroundworkersConfigMutationService( + path, model_discoverer=lambda *_args: discovered + ), + ) + controller.start() + controller.submit({}) + model = controller.submit({"model_source": "new"}).snapshot + choice = next(field for field in model.step.fields if field.key == "model_choice") + + assert choice.default == "stored-model" + assert any(option.value == "stored-model" for option in choice.choices) + assert ("not reported" in (choice.help or "")) is flagged + + def test_stale_and_mismatched_apply_intents_are_consumed_and_safe( tmp_path: Path, ) -> None: @@ -443,8 +520,8 @@ def _reach_cdm_review(controller: ConfigWizardController): return controller.submit( { "cdm_db_name": "cdm_db", - "schema_name": "main", - "vocab_schema": "main", + "cdm_schema": None, + "vocab_schema": None, "results_schema": None, } ).snapshot @@ -565,7 +642,7 @@ def test_an_asymmetric_model_preselects_its_convention_and_writes_the_pair( controller.submit({"prefix_convention": "nomic"}) assert controller.apply().status is WizardResultStatus.APPLIED - model = load_stack_config_from_path(path).models["embedding_model"] + model = load_stack_config(path).models["embedding_model"] # The trailing space is part of the prefix and must survive the round trip. assert model.document_prefix == "search_document: " assert model.query_prefix == "search_query: " @@ -613,5 +690,5 @@ def _drive(convention: str): controller.submit({"document_prefix": "index: ", "query_prefix": "ask: "}) assert controller.apply().status is WizardResultStatus.APPLIED - model = load_stack_config_from_path(path).models["embedding_model"] + model = load_stack_config(path).models["embedding_model"] assert (model.document_prefix, model.query_prefix) == ("index: ", "ask: ") diff --git a/tests/unit/test_domain_names.py b/tests/unit/test_domain_names.py index 504353a..2cb5470 100644 --- a/tests/unit/test_domain_names.py +++ b/tests/unit/test_domain_names.py @@ -11,13 +11,15 @@ import pytest import sqlalchemy as sa from omop_alchemy.cdm.model.vocabulary import Domain +from sqlalchemy.orm import sessionmaker from groundworkers.base.domain_names import DomainNameResolver +from tests.support.omop import route_sqlite_omop_roles @pytest.fixture() def resolver() -> DomainNameResolver: - engine = sa.create_engine("sqlite:///:memory:", future=True) + engine = route_sqlite_omop_roles(sa.create_engine("sqlite:///:memory:", future=True)) Domain.__table__.create(engine) with engine.begin() as conn: conn.execute( @@ -84,14 +86,45 @@ def test_unknown_message_names_the_value_and_valid_options(resolver): assert "Condition" in message -def test_an_unreadable_domain_table_does_not_reject_everything(): - """Reference data being unavailable must not fail every search.""" +def test_missing_domain_table_on_engine_has_routing_guidance(): engine = sa.create_engine("sqlite:///:memory:", future=True) # no domain table resolver = DomainNameResolver(engine) + with pytest.raises(RuntimeError, match="Engine mode cannot route vocabulary") as raised: + resolver.canonical("Condition") + assert raised.value.__cause__ is not None + assert "no such table" in str(raised.value.__cause__).lower() + + +def test_engine_connection_errors_are_reraised_unchanged(monkeypatch): + engine = sa.create_engine("sqlite:///:memory:", future=True) + resolver = DomainNameResolver(engine) + original = RuntimeError("database is unavailable") + + def fail_connect(): + raise original + + monkeypatch.setattr(engine, "connect", fail_connect) + with pytest.raises(RuntimeError, match="database is unavailable") as raised: + resolver.canonical("Condition") + assert raised.value is original + + +def test_domain_load_failure_is_retried_after_the_table_appears(): + engine = route_sqlite_omop_roles(sa.create_engine("sqlite:///:memory:", future=True)) + session_factory = sessionmaker(bind=engine) + resolver = DomainNameResolver(session_factory) + assert resolver.canonical("Condition") == "Condition" - assert resolver.known_domains() == () + Domain.__table__.create(engine) + with engine.begin() as connection: + connection.execute( + sa.insert(Domain), + {"domain_id": "Condition", "domain_name": "Condition", "domain_concept_id": 19}, + ) + + assert resolver.canonical("condition") == "Condition" def test_the_table_is_read_once(resolver): diff --git a/tests/unit/test_migration_boundaries.py b/tests/unit/test_migration_boundaries.py index a7d83cb..b7e7855 100644 --- a/tests/unit/test_migration_boundaries.py +++ b/tests/unit/test_migration_boundaries.py @@ -107,7 +107,20 @@ def test_migration_dependencies_use_explicit_public_boundaries() -> None: ) dependencies = metadata["project"]["dependencies"] - assert "omop-llm>=1,<2" in dependencies + assert "oa-configurator>=2.0.0,<3" in dependencies + assert "omop-alchemy>=2.0.0,<3" in dependencies + assert "omop-graph>=3.0.0,<4" in dependencies + assert "omop-emb>=3.0.0,<4" in dependencies + assert "omop-llm>=1.0.2,<2" in dependencies + assert metadata["project"]["optional-dependencies"]["tui"][0] == ( + "groundskeeping>=0.9.1,<1" + ) + assert metadata["project"]["optional-dependencies"]["embedding-pgvector"] == [ + "omop-emb[pgvector]>=3.0.0,<4" + ] + assert metadata["project"]["optional-dependencies"]["embedding-faiss"] == [ + "omop-emb[faiss-cpu]>=3.0.0,<4" + ] # RELEASE BLOCKER. No source pins may survive into a publishable artifact. sources = metadata.get("tool", {}).get("uv", {}).get("sources", {}) diff --git a/tests/unit/test_normalized_index.py b/tests/unit/test_normalized_index.py index 2948e0b..b2d51b7 100644 --- a/tests/unit/test_normalized_index.py +++ b/tests/unit/test_normalized_index.py @@ -4,9 +4,11 @@ import re from contextlib import contextmanager +from types import SimpleNamespace import pytest import sqlalchemy as sa +from oa_configurator import Role from omop_alchemy.cdm.model.vocabulary import Concept, Concept_Synonym from sqlalchemy.dialects import postgresql @@ -25,6 +27,7 @@ normalized_key, ) from groundworkers.services.vocab import normalize_text_for_matching +from tests.support.stack_config import build_cdm_stack def _python_key(text: str) -> str: @@ -122,6 +125,9 @@ def test_normalized_index_ddl_uses_light_tables_and_preserves_orm_metadata( class RecordingConnection: dialect = postgresql.dialect() + def get_execution_options(self): + return {"schema_translate_map": {Role.VOCAB: "vocab"}} + def __init__(self) -> None: self.statements = [] self.raw_statements = [] @@ -137,7 +143,7 @@ def exec_driver_sql(self, statement): @contextmanager def fake_postgres_cdm(_snapshot, *, purpose): assert purpose == "Normalized-name indexes" - yield connection, "cdm", '"cdm".' + yield connection, "cdm" monkeypatch.setattr(performance_maintenance, "_postgres_cdm", fake_postgres_cdm) monkeypatch.setattr( @@ -173,7 +179,7 @@ def fake_postgres_cdm(_snapshot, *, purpose): str( statement.compile( dialect=postgresql.dialect(), - schema_translate_map={None: "cdm"}, + schema_translate_map={Role.VOCAB: "vocab"}, render_schema_translate=True, ) ) @@ -183,19 +189,107 @@ def fake_postgres_cdm(_snapshot, *, purpose): str( statement.compile( dialect=postgresql.dialect(), - schema_translate_map={None: "cdm"}, + schema_translate_map={Role.VOCAB: "vocab"}, render_schema_translate=True, ) ) for statement in drops ] - assert "ON cdm.concept (" in compiled_creates[0] - assert "ON cdm.concept_synonym (" in compiled_creates[1] + assert "ON vocab.concept (" in compiled_creates[0] + assert "ON vocab.concept_synonym (" in compiled_creates[1] assert all("regexp_replace" in statement for statement in compiled_creates) - assert all("cdm." in statement for statement in compiled_drops) + assert all("vocab." in statement for statement in compiled_drops) assert connection.raw_statements == [ - 'ANALYZE "cdm".concept', - 'ANALYZE "cdm".concept_synonym', + 'ANALYZE vocab.concept', + 'ANALYZE vocab.concept_synonym', ] assert Concept.__table__.indexes == concept_indexes_before assert Concept_Synonym.__table__.indexes == synonym_indexes_before + + +def test_trigram_indexes_compile_as_gin_expression_indexes(monkeypatch) -> None: + class RecordingConnection: + dialect = postgresql.dialect() + + def __init__(self) -> None: + self.statements = [] + self.raw_statements = [] + + def execute(self, statement): + self.statements.append(statement) + + def exec_driver_sql(self, statement): + self.raw_statements.append(statement) + + connection = RecordingConnection() + + @contextmanager + def fake_postgres_cdm(_snapshot, *, purpose): + assert purpose == "Trigram indexes" + yield connection, "vocab" + + monkeypatch.setattr(performance_maintenance, "_postgres_cdm", fake_postgres_cdm) + performance_maintenance.populate_trigram_indexes(object()) + + creates = [ + statement + for statement in connection.statements + if isinstance(statement, sa.schema.CreateIndex) + ] + compiled = [ + str( + statement.compile( + dialect=postgresql.dialect(), + schema_translate_map={Role.VOCAB: "vocab"}, + compile_kwargs={"render_schema_translate": True}, + ) + ) + for statement in creates + ] + assert len(compiled) == 2 + assert all("USING gin" in ddl and "gin_trgm_ops" in ddl for ddl in compiled) + assert all("ON __[SCHEMA_vocab]." in ddl and "lower(" in ddl for ddl in compiled) + assert connection.raw_statements == ["CREATE EXTENSION IF NOT EXISTS pg_trgm"] + + +def test_performance_cdm_context_connects_to_vocab_engine(monkeypatch) -> None: + class Connection: + def __enter__(self): + return self + + def __exit__(self, *_args): + return None + + class Engine: + dialect = postgresql.dialect() + + def __init__(self): + self.connection = Connection() + self.opened = False + self.disposed = False + + def begin(self): + self.opened = True + return self.connection + + def dispose(self): + self.disposed = True + + primary, vocab = Engine(), Engine() + + def create_engines(_resolved, *, register_claims): + assert register_claims is False + return primary, vocab + + monkeypatch.setattr(performance_maintenance, "create_cdm_engines", create_engines) + snapshot = SimpleNamespace(stack=build_cdm_stack()) + + with performance_maintenance._postgres_cdm( + snapshot, purpose="Normalized-name indexes" + ) as (connection, _schema): + assert connection is vocab.connection + + assert primary.opened is False + assert vocab.opened is True + assert primary.disposed is True + assert vocab.disposed is True diff --git a/tests/unit/test_omop_emb_adapter.py b/tests/unit/test_omop_emb_adapter.py index 2a75940..54d927d 100644 --- a/tests/unit/test_omop_emb_adapter.py +++ b/tests/unit/test_omop_emb_adapter.py @@ -400,7 +400,7 @@ def __init__(self, **kwargs): "model": "demo-model", "backend": "store", "metric_type": MetricType.COSINE, - "omop_cdm_engine": None, + "cdm_session_factory": None, "provider_type": "openai", "faiss_cache_dir": "faiss-cache", } @@ -435,6 +435,25 @@ def __init__(self, **kwargs): assert len(created) == 2 +def test_legacy_registry_is_reported_as_a_specific_tool_error() -> None: + from omop_emb.utils.errors import LegacyRegistryError + + def fail(): + raise LegacyRegistryError("legacy registry") + + adapter = OmopEmbAdapter( + backend_factory=fail, + backend_type="pgvector", + ) + status = adapter.index_status() + + assert status["available"] is False + assert "predates omop-emb 3.0" in status["detail"] + with pytest.raises(GroundworkersError) as caught: + adapter._get_backend() + assert caught.value.code == "LEGACY_REGISTRY" + + def test_resolve_model_name_uses_registry_record(monkeypatch): adapter = build_adapter() monkeypatch.setattr( diff --git a/tests/unit/test_omop_emb_setup_config.py b/tests/unit/test_omop_emb_setup_config.py index fae7238..f9a4a89 100644 --- a/tests/unit/test_omop_emb_setup_config.py +++ b/tests/unit/test_omop_emb_setup_config.py @@ -3,7 +3,7 @@ from pathlib import Path from groundskeeping.configurator import ConfigWizardController, MutationOperation -from oa_configurator import load_stack_config_from_path, save_stack_config +from oa_configurator import load_stack_config, save_stack_config from groundworkers.application.setup.configuration_provider import ( STORE_LOCATION_CDM, @@ -77,7 +77,7 @@ def test_cdm_setup_saves_companion_cli_sections_with_custom_name( } assert controller.apply().status.value == "applied" - saved = load_stack_config_from_path(path) + saved = load_stack_config(path) assert saved.tools["omop_alchemy"]["cdm_db"] == "warehouse_cdm" assert saved.tools["omop_graph"]["cdm_db"] == "warehouse_cdm" @@ -155,7 +155,7 @@ def test_vector_store_setup_completes_and_saves_both_package_sections( } assert controller.apply().status.value == "applied" - saved = load_stack_config_from_path(path) + saved = load_stack_config(path) assert saved.tools["omop_emb"] == { "cdm_db": "cdm_db", "embedding_model_name": "embedding_model", diff --git a/tests/unit/test_omop_graph_adapter.py b/tests/unit/test_omop_graph_adapter.py index fc4da30..1c13f4d 100644 --- a/tests/unit/test_omop_graph_adapter.py +++ b/tests/unit/test_omop_graph_adapter.py @@ -1,12 +1,21 @@ from __future__ import annotations import asyncio +from pathlib import Path from types import SimpleNamespace +import pytest +from oa_configurator import CDMDatabaseConfig, ConnectionConfig, Resolver +from omop_alchemy.config import create_cdm_engines +from omop_alchemy.cross_database import cdm_sessionmaker from omop_emb import MetricType +from omop_graph import KnowledgeGraph from sqlalchemy import create_engine +from sqlalchemy.exc import OperationalError from groundworkers.adapters.omop_graph import OmopGraphAdapter +from groundworkers.base.domain_names import DomainNameResolver +from tests.support.stack_config import build_cdm_stack def test_async_query_encoding_uses_native_model_api_repeatedly(): @@ -23,7 +32,7 @@ async def async_embed_texts(self, texts, *, role): backend = AsyncOnlyBackend() adapter = OmopGraphAdapter( - create_engine("sqlite:///:memory:"), + _engine_with_graph_tables(), model_backend_factory=lambda: backend, # type: ignore[arg-type,return-value] ) @@ -41,7 +50,7 @@ async def invoke_twice(): def test_embedding_resolver_requires_complete_configuration(): - engine = create_engine("sqlite:///:memory:") + engine = _engine_with_graph_tables() assert OmopGraphAdapter(engine).embedding_resolver_active is False assert ( @@ -66,8 +75,11 @@ def __init__(self, **kwargs): captured["embedding_configuration"] = kwargs class FakeKnowledgeGraph: - def __init__(self, **kwargs): - captured["knowledge_graph"] = kwargs + def __init__(self, cdm_session_factory, *, emb_config): + captured["knowledge_graph"] = { + "cdm_session_factory": cdm_session_factory, + "emb_config": emb_config, + } monkeypatch.setattr( "groundworkers.adapters.omop_graph.KnowledgeGraphEmbeddingConfiguration", @@ -77,7 +89,7 @@ def __init__(self, **kwargs): "groundworkers.adapters.omop_graph.KnowledgeGraph", FakeKnowledgeGraph ) adapter = OmopGraphAdapter( - create_engine("sqlite:///:memory:"), + _engine_with_graph_tables(), embedding_backend_factory=lambda: embedding_backend, # type: ignore[arg-type,return-value] resolved_embedding_model=resolved_model, # type: ignore[arg-type] model_backend_factory=lambda: object(), # type: ignore[arg-type,return-value] @@ -96,7 +108,7 @@ def __init__(self, **kwargs): "faiss_cache_dir": "faiss-cache", } assert captured["knowledge_graph"] == { - "cdm_engine": adapter.engine, + "cdm_session_factory": adapter._cdm_session_factory, "emb_config": captured["embedding_configuration_object"], } assert adapter.embedding_resolver_active is True @@ -106,8 +118,8 @@ def test_invalid_embedding_configuration_falls_back_to_lexical_graph(monkeypatch captured: dict[str, object] = {} class FakeKnowledgeGraph: - def __init__(self, **kwargs): - captured.update(kwargs) + def __init__(self, cdm_session_factory, *, emb_config): + captured.update(cdm_session_factory=cdm_session_factory, emb_config=emb_config) monkeypatch.setattr( "groundworkers.adapters.omop_graph.KnowledgeGraph", FakeKnowledgeGraph @@ -117,7 +129,7 @@ def fail_backend(): raise RuntimeError("store unavailable") adapter = OmopGraphAdapter( - create_engine("sqlite:///:memory:"), + _engine_with_graph_tables(), embedding_backend_factory=fail_backend, resolved_embedding_model=SimpleNamespace(model="demo-model"), # type: ignore[arg-type] ) @@ -135,13 +147,13 @@ def test_cdm_only_configuration_builds_a_lexical_graph(monkeypatch): captured: dict[str, object] = {} class FakeKnowledgeGraph: - def __init__(self, **kwargs): - captured.update(kwargs) + def __init__(self, cdm_session_factory, *, emb_config): + captured.update(cdm_session_factory=cdm_session_factory, emb_config=emb_config) monkeypatch.setattr( "groundworkers.adapters.omop_graph.KnowledgeGraph", FakeKnowledgeGraph ) - adapter = OmopGraphAdapter(create_engine("sqlite:///:memory:")) + adapter = OmopGraphAdapter(_engine_with_graph_tables()) assert isinstance(adapter._get_kg(), FakeKnowledgeGraph) assert captured["emb_config"] is None @@ -157,16 +169,16 @@ def test_graph_rejecting_the_embedding_configuration_keeps_lexical_grounding(mon attempts: list[object] = [] class FakeKnowledgeGraph: - def __init__(self, **kwargs): - attempts.append(kwargs["emb_config"]) - if kwargs["emb_config"] is not None: + def __init__(self, cdm_session_factory, *, emb_config): + attempts.append(emb_config) + if emb_config is not None: raise RuntimeError("embedding store has no vectors for demo-model") monkeypatch.setattr( "groundworkers.adapters.omop_graph.KnowledgeGraph", FakeKnowledgeGraph ) adapter = OmopGraphAdapter( - create_engine("sqlite:///:memory:"), + _engine_with_graph_tables(), embedding_backend_factory=lambda: object(), # type: ignore[arg-type,return-value] resolved_embedding_model=SimpleNamespace(model="demo-model"), # type: ignore[arg-type] ) @@ -196,13 +208,13 @@ def test_embedding_failure_detail_does_not_leak_provider_or_database_secrets(): def test_backend_failure_without_embedding_configuration_still_raises(monkeypatch): class FakeKnowledgeGraph: - def __init__(self, **kwargs): + def __init__(self, cdm_session_factory, *, emb_config): raise RuntimeError("graph unavailable") monkeypatch.setattr( "groundworkers.adapters.omop_graph.KnowledgeGraph", FakeKnowledgeGraph ) - adapter = OmopGraphAdapter(create_engine("sqlite:///:memory:")) + adapter = OmopGraphAdapter(_engine_with_graph_tables()) available, detail = adapter.probe() assert available is False @@ -210,6 +222,83 @@ def __init__(self, **kwargs): assert adapter.embedding_resolver_active is False +def test_real_knowledge_graph_constructs_with_sqlite_adapter(): + engine = create_engine( + "sqlite:///:memory:" + ).execution_options(schema_translate_map={"primary": None, "vocab": None, "results": None}) + from omop_alchemy.cdm.model.vocabulary import Concept + from omop_graph.extensions.omop_alchemy import RelationshipMapping + + Concept.__table__.create(engine) + RelationshipMapping.__table__.create(engine) + with engine.begin() as connection: + connection.execute( + RelationshipMapping.__table__.insert(), + { + "relationship_id": "Maps to", + "predicate_kind": "Hierarchy", + "predicate_subkind": "mapping", + }, + ) + adapter = OmopGraphAdapter(engine) + + graph = adapter._get_kg() + + assert isinstance(graph, KnowledgeGraph) + + +def test_real_knowledge_graph_constructs_with_split_sqlite_databases( + tmp_path: Path, +) -> None: + stack = build_cdm_stack(sqlite_path=str(tmp_path / "primary.db")) + stack.connections["vocab"] = ConnectionConfig( + dialect="sqlite", database_name=str(tmp_path / "vocabulary.db") + ) + stack.databases["cdm_db"] = CDMDatabaseConfig( + connection="cdm_main", vocab_connection="vocab" + ) + resolved = Resolver(stack).resolve_database("cdm_db") + primary_engine, vocab_engine = create_cdm_engines( + resolved, register_claims=False + ) + from omop_alchemy.cdm.model.vocabulary import ( + Concept, + Concept_Class, + Domain, + Vocabulary, + ) + from omop_graph.extensions.omop_alchemy import RelationshipMapping + + RelationshipMapping.__table__.create(primary_engine) + for model in (Concept_Class, Domain, Vocabulary, Concept): + model.__table__.create(vocab_engine) + with primary_engine.begin() as connection: + connection.execute( + RelationshipMapping.__table__.insert(), + { + "relationship_id": "Maps to", + "predicate_kind": "Hierarchy", + "predicate_subkind": "mapping", + }, + ) + session_factory = cdm_sessionmaker( + resolved, primary=primary_engine, vocab=vocab_engine + ) + + try: + available, detail = OmopGraphAdapter(primary_engine).probe() + assert available is False + assert detail is not None and "vocabulary on its own database" in detail + with pytest.raises(RuntimeError, match="vocabulary on its own database"): + DomainNameResolver(primary_engine).known_domains() + graph = OmopGraphAdapter(session_factory)._get_kg() + assert isinstance(graph, KnowledgeGraph) + finally: + primary_engine.dispose() + if vocab_engine is not primary_engine: + vocab_engine.dispose() + + def test_match_kind_names_are_keyed_off_the_upstream_enum(): from omop_graph.graph.nodes import LabelMatchKind @@ -234,3 +323,23 @@ def test_wrap_graph_error_does_not_special_case_parentless_not_implemented(): assert wrapped.message == ( "omop-graph operation failed with NotImplementedError." ) + + +def test_wrap_graph_error_classifies_primary_database_outage_before_setup_text(): + outage = OperationalError("select", {}, ConnectionError("server unavailable")) + wrapped = OmopGraphAdapter._wrap_graph_error(outage, default_code="QUERY_ERROR") + + assert wrapped.code == "BACKEND_UNAVAIL" + assert "database is unavailable" in wrapped.message + + +def _engine_with_graph_tables(): + from omop_alchemy.cdm.model.vocabulary import Concept + from omop_graph.extensions.omop_alchemy import RelationshipMapping + + engine = create_engine("sqlite:///:memory:").execution_options( + schema_translate_map={"primary": None, "vocab": None, "results": None} + ) + Concept.__table__.create(engine) + RelationshipMapping.__table__.create(engine) + return engine diff --git a/tests/unit/test_phase3_overview.py b/tests/unit/test_phase3_overview.py index f4d8b6a..1480c07 100644 --- a/tests/unit/test_phase3_overview.py +++ b/tests/unit/test_phase3_overview.py @@ -26,7 +26,7 @@ def test_missing_configuration_landing_is_a_cdm_recovery_action(tmp_path: Path) ) assert isinstance(view, EmptyView) - assert view.title == "Start Groundworkers" + assert view.title == "Repair Groundworkers configuration" assert view.actions[0].key == "database.configure" diff --git a/tests/unit/test_plugin_configuration.py b/tests/unit/test_plugin_configuration.py index 1373e03..62a283a 100644 --- a/tests/unit/test_plugin_configuration.py +++ b/tests/unit/test_plugin_configuration.py @@ -13,7 +13,7 @@ PackageConfigBase, RefTo, VectorStoreConfig, - load_stack_config_from_path, + load_stack_config, save_stack_config, ) from pydantic import Field @@ -103,7 +103,7 @@ def test_existing_references_and_pydantic_constraint_run_through_controller( assert review.can_apply assert controller.apply().applied - saved = load_stack_config_from_path(path) + saved = load_stack_config(path) config = ComparatorRecommenderConfig.validate_candidate(saved) assert config.comparator_db == "cdm_db" assert config.embedding_model_name == "embedding_model" @@ -137,7 +137,6 @@ def test_recursive_reference_creation_writes_a_complete_oa_stack(tmp_path: Path) "vector_store_name::database::connection::database_name": str( tmp_path / "plugin-vectors.db" ), - "vector_store_name::database::schema_name": "main", "vector_store_name::backend_type": "sqlitevec", "min_databases_default": 2, } @@ -147,7 +146,7 @@ def test_recursive_reference_creation_writes_a_complete_oa_stack(tmp_path: Path) assert "plugin-secret-canary" not in repr(review) assert controller.apply().applied - saved = load_stack_config_from_path(path) + saved = load_stack_config(path) config = ComparatorRecommenderConfig.validate_candidate(saved) assert config.embedding_model_name == "plugin_embedding_model" assert config.vector_store_name == "plugin_vectors" @@ -172,12 +171,12 @@ def test_concurrent_write_after_review_conflicts(tmp_path: Path) -> None: ) assert review.can_apply - changed = load_stack_config_from_path(path) + changed = load_stack_config(path) changed.tools["external_writer"] = {"changed": True} save_stack_config(changed, path) assert controller.apply().status.value == "conflicted" - assert "comparator_recommender" not in load_stack_config_from_path(path).tools + assert "comparator_recommender" not in load_stack_config(path).tools def test_attempted_save_failure_returns_failed(tmp_path: Path, monkeypatch) -> None: diff --git a/tests/unit/test_schema_resolution.py b/tests/unit/test_schema_resolution.py index ebd8f20..cf33e9b 100644 --- a/tests/unit/test_schema_resolution.py +++ b/tests/unit/test_schema_resolution.py @@ -1,251 +1,146 @@ -"""How a non-default CDM schema reaches every statement Groundworkers issues. - -oa-configurator expresses schema placement only through SQLAlchemy's -``schema_translate_map`` and sets no ``search_path``. The map is applied when a -statement is *compiled*, so these tests cover the three places that fall outside -compilation and therefore have to be handled by hand: engine construction, Core -constructs that look translatable but are not, and reflection. -""" +"""Stack 2.0 role routing and vocabulary reflection contracts.""" from __future__ import annotations from pathlib import Path -import pytest import sqlalchemy as sa +from oa_configurator import ConnectionConfig, Role from sqlalchemy.dialects import postgresql import groundworkers.services.graph as graph_service from groundworkers.adapters.cdm import CDMAdapter -from groundworkers.adapters.omop_graph import OmopGraphAdapter from groundworkers.application.setup.databases import ( DatabaseTarget, verify_database_target, ) -from groundworkers.base.sql import effective_schema from groundworkers.bootstrap import build_app_config_from_stack from groundworkers.services.vocab import VocabService from tests.support.stack_config import build_cdm_stack -# --------------------------------------------------------------------------- -# effective_schema -# --------------------------------------------------------------------------- - -def test_effective_schema_reads_the_binds_translate_map() -> None: - engine = sa.create_engine("sqlite://").execution_options( - schema_translate_map={None: "cdm", "vocab": "cdm_vocab"} - ) - - assert effective_schema(engine) == "cdm" - with engine.connect() as connection: - assert effective_schema(connection) == "cdm" - - -def test_effective_schema_is_none_without_a_map() -> None: - """The correct answer for an unconfigured deployment, not a failure. - - ``schema=None`` is what reflection already wants when the dialect's own - ``default_schema_name`` applies. - """ - assert effective_schema(sa.create_engine("sqlite://")) is None - - -def test_effective_schema_ignores_a_map_without_a_default_key() -> None: - engine = sa.create_engine("sqlite://").execution_options( - schema_translate_map={"vocab": "cdm_vocab"} - ) - - assert effective_schema(engine) is None - - -# --------------------------------------------------------------------------- -# The runtime engine -# --------------------------------------------------------------------------- - -def test_the_cdm_engine_carries_the_resolved_translate_map() -> None: - """Built from the database, not the connection. - ``ResolvedConnection.create_engine`` produces an engine with no map at all, - which silently resolves every CDM table through the ambient ``search_path``. - """ - stack = build_cdm_stack(schema_name="cdm", vocab_schema="cdm_vocab") +def test_create_cdm_engines_supplies_each_role_map() -> None: + config = build_app_config_from_stack(build_cdm_stack()) + options = config.cdm_vocab_engine.get_execution_options() + role_map = options["schema_translate_map"] - config = build_app_config_from_stack(stack) + assert "primary" in role_map + assert "vocab" in role_map + assert config.cdm_engine is not None + assert config.cdm_session_factory is not None + config.cdm_engine.dispose() + if config.cdm_vocab_engine is not config.cdm_engine: + config.cdm_vocab_engine.dispose() - assert config.cdm_engine.get_execution_options()["schema_translate_map"] == { - None: "cdm", - "vocab": "cdm_vocab", - } - assert effective_schema(config.cdm_engine) == "cdm" - -def test_the_cdm_engine_map_matches_the_resolved_database() -> None: - stack = build_cdm_stack(schema_name="cdm", vocab_schema=None) - - config = build_app_config_from_stack(stack) - - assert ( - config.cdm_engine.get_execution_options()["schema_translate_map"] - == config.cdm_database.schema_translate_map() - ) - - -# --------------------------------------------------------------------------- -# The relationship_mapping sidecar reference -# --------------------------------------------------------------------------- - -def test_relationship_mapping_is_a_translated_table() -> None: - """A lowercase ``table()`` would compile unqualified even with a map set. - - That is the failure this guards: one silently unqualified table in an - otherwise correctly routed query. - """ +def test_relationship_mapping_is_omop_graphs_translated_table() -> None: rm = graph_service._RELATIONSHIP_MAPPING - assert isinstance(rm, sa.Table) - assert rm.schema is None - + assert rm.schema == Role.PRIMARY compiled = str( sa.select(rm.c.relationship_id).compile( - dialect=postgresql.dialect(), schema_translate_map={None: "cdm"} + dialect=postgresql.dialect(), + schema_translate_map={Role.PRIMARY: "omop"}, + compile_kwargs={"render_schema_translate": True}, ) ) - assert "__[SCHEMA__none].relationship_mapping" in compiled - - -def test_relationship_mapping_is_omop_graphs_own_definition() -> None: - """One definition, not a hand-written copy that can drift from upstream.""" - from omop_graph.extensions.omop_alchemy import RelationshipMapping - - assert graph_service._RELATIONSHIP_MAPPING is RelationshipMapping.__table__ + assert "__[SCHEMA_primary].relationship_mapping" in compiled -def test_services_do_not_use_the_untranslated_table_helper() -> None: - """`sa.table()` looks schema-agnostic but is excluded from translation.""" - offenders = [ - f"{path.name}:{number}" - for path in (Path(graph_service.__file__).parent).glob("**/*.py") - for number, line in enumerate(path.read_text().splitlines(), start=1) - if line.startswith("from sqlalchemy import") - and "table" in [part.strip() for part in line.split("import", 1)[1].split(",")] - ] - - assert offenders == [] - - -# --------------------------------------------------------------------------- -# Reflection -# --------------------------------------------------------------------------- - -def _attached_cdm_engine(tmp_path: Path, *, sidecars: bool) -> sa.Engine: - """An engine whose CDM tables live in a non-default schema. - - SQLite's ``ATTACH`` stands in for a named schema: reflection without an - explicit ``schema=`` raises ``NoSuchTableError`` for these tables, exactly as - an unqualified reflection does on PostgreSQL when the tables are outside - ``search_path``. - """ - engine = sa.create_engine(f"sqlite+pysqlite:///{tmp_path / 'main.db'}") - attached = tmp_path / "cdm.db" +def _split_sqlite_config(tmp_path: Path): + stack = build_cdm_stack(sqlite_path=str(tmp_path / "primary.db")) + stack.connections["vocab"] = ConnectionConfig( + dialect="sqlite", database_name=str(tmp_path / "vocabulary.db") + ) + stack.databases["cdm_db"].vocab_connection = "vocab" + return build_app_config_from_stack(stack) - @sa.event.listens_for(engine, "connect") - def _attach(dbapi_connection, _record): # pragma: no cover - connection hook - dbapi_connection.execute(f"ATTACH DATABASE '{attached}' AS cdm") +def _create_vocab_tables(engine: sa.Engine, *, sidecars: bool) -> None: concept_columns = "concept_id INTEGER, concept_name TEXT" synonym_columns = "concept_id INTEGER, concept_synonym_name TEXT" if sidecars: concept_columns += ", concept_name_tsvector TEXT" synonym_columns += ", concept_synonym_name_tsvector TEXT" with engine.begin() as connection: - connection.exec_driver_sql(f"CREATE TABLE cdm.concept ({concept_columns})") - connection.exec_driver_sql( - f"CREATE TABLE cdm.concept_synonym ({synonym_columns})" - ) + connection.exec_driver_sql(f"CREATE TABLE concept ({concept_columns})") + connection.exec_driver_sql(f"CREATE TABLE concept_synonym ({synonym_columns})") - return engine.execution_options(schema_translate_map={None: "cdm"}) - - -def test_fulltext_sidecars_are_detected_in_the_configured_schema( - tmp_path: Path, -) -> None: - service = VocabService(CDMAdapter(_attached_cdm_engine(tmp_path, sidecars=True))) +def test_vocab_reflection_uses_the_vocab_role_schema(tmp_path: Path) -> None: + config = _split_sqlite_config(tmp_path) + _create_vocab_tables(config.cdm_vocab_engine, sidecars=True) + service = VocabService( + CDMAdapter( + config.cdm_engine, + vocab_engine=config.cdm_vocab_engine, + session_factory=config.cdm_session_factory, + ) + ) assert service.fts_available is True + config.cdm_engine.dispose() + if config.cdm_vocab_engine is not config.cdm_engine: + config.cdm_vocab_engine.dispose() -def test_absent_fulltext_sidecars_are_reported_as_unavailable(tmp_path: Path) -> None: - """Distinguishes "no sidecar" from "could not look" -- both must say False.""" - service = VocabService(CDMAdapter(_attached_cdm_engine(tmp_path, sidecars=False))) - +def test_vocab_reflection_reports_missing_sidecars(tmp_path: Path) -> None: + config = _split_sqlite_config(tmp_path) + _create_vocab_tables(config.cdm_vocab_engine, sidecars=False) + service = VocabService( + CDMAdapter( + config.cdm_engine, + vocab_engine=config.cdm_vocab_engine, + session_factory=config.cdm_session_factory, + ) + ) assert service.fts_available is False + config.cdm_engine.dispose() + if config.cdm_vocab_engine is not config.cdm_engine: + config.cdm_vocab_engine.dispose() -def test_unreadable_vocabulary_logs_rather_than_absorbing_the_failure( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - engine = sa.create_engine(f"sqlite+pysqlite:///{tmp_path / 'empty.db'}") +def test_normalized_index_check_uses_the_split_vocab_engine(tmp_path, monkeypatch) -> None: + config = _split_sqlite_config(tmp_path) + _create_vocab_tables(config.cdm_vocab_engine, sidecars=False) service = VocabService( - CDMAdapter(engine.execution_options(schema_translate_map={None: "absent"})) + CDMAdapter( + config.cdm_engine, + vocab_engine=config.cdm_vocab_engine, + session_factory=config.cdm_session_factory, + ) ) + seen = [] - with caplog.at_level("WARNING"): - assert service.fts_available is False - - assert "absent" in caplog.text + def record_connection(connection, *_args): + seen.append(connection.engine.url.database) + return False + monkeypatch.setattr( + "groundworkers.application.setup.databases.normalized_index_matches", + record_connection, + ) + assert service._has_normalized_index("concept", "concept_name") is False + assert seen == [str(tmp_path / "vocabulary.db")] + config.cdm_engine.dispose() + config.cdm_vocab_engine.dispose() -# --------------------------------------------------------------------------- -# Setup diagnostics -# --------------------------------------------------------------------------- - -def test_readiness_checks_the_schema_the_runtime_reads(tmp_path: Path) -> None: - """Keyed on cdm_schema, not vocabulary_schema. - No omop-alchemy model declares ``schema="vocab"``, so vocabulary tables live - in the primary schema. Checking ``vocabulary_schema`` reports them missing on - any config that sets the two differently -- the inverse of what the runtime - engine finds. - """ +def test_readiness_uses_configured_vocab_schema(tmp_path: Path) -> None: db_path = tmp_path / "omop.db" engine = sa.create_engine(f"sqlite+pysqlite:///{db_path}") with engine.begin() as connection: for name in ( - "concept", - "concept_relationship", - "concept_ancestor", - "concept_synonym", - "relationship", + "concept", "concept_relationship", "concept_ancestor", + "concept_synonym", "relationship", ): connection.exec_driver_sql(f"CREATE TABLE {name} (id INTEGER)") engine.dispose() - target = DatabaseTarget( - key="database.cdm", - label="CDM / vocabulary", - database_entry_name="cdm_db", - connection_name="main", - safe_url=f"sqlite:///{db_path}", - # The tables are in "main"; "temp" is a real but empty SQLite schema. - cdm_schema="main", - vocabulary_schema="temp", - connection_url=f"sqlite:///{db_path}", + key="database.cdm", label="CDM / vocabulary", database_entry_name="cdm_db", + connection_name="main", safe_url=f"sqlite:///{db_path}", + cdm_schema=None, vocabulary_schema=None, connection_url=f"sqlite:///{db_path}", role="cdm", ) - result = verify_database_target(target) - assert result.connected is True - assert [d.code for d in result.diagnostics] == ["cdm_tables_present"] - - -# --------------------------------------------------------------------------- -# The removed parameter -# --------------------------------------------------------------------------- - -def test_the_graph_adapter_rejects_a_vocabulary_schema_argument() -> None: - """It never did anything; accepting it again would be a silent no-op.""" - engine = sa.create_engine("sqlite://") - - with pytest.raises(TypeError): - OmopGraphAdapter(engine, vocab_schema="omop_vocab") # type: ignore[call-arg] + assert [item.code for item in result.diagnostics] == ["cdm_tables_present"] diff --git a/tests/unit/test_setup_artifacts_and_coverage.py b/tests/unit/test_setup_artifacts_and_coverage.py index 83ad87e..f29d892 100644 --- a/tests/unit/test_setup_artifacts_and_coverage.py +++ b/tests/unit/test_setup_artifacts_and_coverage.py @@ -80,8 +80,11 @@ def get_embedding_count_by_vocabulary(self, **_kwargs): def test_unfiltered_coverage_uses_pushed_down_counts() -> None: + calls: list[str] = [] + class Backend: - def get_embedding_count_by_vocabulary(self, **_kwargs): + def get_embedding_count_by_vocabulary(self, *, model_name): + calls.append(model_name) return (("SNOMED", "4"),) result = load_coverage( @@ -93,6 +96,7 @@ def get_embedding_count_by_vocabulary(self, **_kwargs): assert result.available is True assert result.rows[0].pending == 6 assert result.rows[0].coverage_percent == 40.0 + assert calls == ["test-model"] def test_mismatched_counts_are_rejected() -> None: diff --git a/tests/unit/test_setup_configuration.py b/tests/unit/test_setup_configuration.py index f4e2029..870f5f9 100644 --- a/tests/unit/test_setup_configuration.py +++ b/tests/unit/test_setup_configuration.py @@ -24,8 +24,6 @@ [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -130,9 +128,7 @@ def test_save_backs_up_atomically_replaces_and_reloads(tmp_path: Path) -> None: databases={ "cdm_db": CDMDatabaseConfig( connection="main", - schema_name="main", - vocab_schema="main", - ) + ) }, tools={"groundworkers": {"cdm_db": "cdm_db"}}, ) @@ -158,7 +154,7 @@ def test_save_rejects_derived_configuration(tmp_path: Path) -> None: "main": ConnectionConfig(dialect="sqlite", database_name=":memory:") }, databases={ - "cdm_db": CDMDatabaseConfig(connection="main", schema_name="main") + "cdm_db": CDMDatabaseConfig(connection="main") }, tools={"groundworkers": {"cdm_db": "cdm_db"}}, ) diff --git a/tests/unit/test_setup_databases.py b/tests/unit/test_setup_databases.py index 71797ee..0f7b404 100644 --- a/tests/unit/test_setup_databases.py +++ b/tests/unit/test_setup_databases.py @@ -2,14 +2,18 @@ import socket from pathlib import Path +from types import SimpleNamespace +import pytest +from oa_configurator import Resolver from oa_configurator.io import save_stack_config +from omop_emb.utils.errors import MisplacedRegistryError, MissingStorageTableError from sqlalchemy import create_engine, text from groundworkers.application.setup.configuration import load_configuration from groundworkers.application.setup.databases import ( - _embedding_registry_rows, _index_defines_lower_expression, + _trigram_lower_index_exists, classify_connection_error, resolve_database_targets, verify_database_target, @@ -19,7 +23,7 @@ ConnectionFailureKind, DatabaseTarget, ) -from tests.support.stack_config import build_embedding_stack +from tests.support.stack_config import build_cdm_stack, build_embedding_stack VALID_DATABASE_CONFIG = """ [connections.main] @@ -29,8 +33,6 @@ [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -47,15 +49,16 @@ def test_database_targets_are_resolved_with_safe_urls(tmp_path: Path) -> None: assert targets[0].key == "database.cdm" assert targets[1].key == "database.graph" assert targets[2].key == "database.groundworkers" - assert targets[0].safe_url == "sqlite:///:memory:" + assert targets[0].safe_url in {"sqlite:///:memory:", "sqlite:///%3Amemory%3A"} + assert targets[0].cdm_schema is None + assert targets[0].vocabulary_schema is None assert "connection_url" not in repr(targets[0]) -def test_distinct_vocabulary_connection_is_refused_rather_than_targeted( +def test_distinct_vocabulary_connection_is_accepted_by_the_configuration_layer( tmp_path: Path, ) -> None: - """Nothing reads a second engine, so offering it as a target would promise a - split the runtime does not honour.""" + """Split vocabularies are represented by the resolved CDM role targets.""" path = tmp_path / "config.toml" path.write_text( """ @@ -70,9 +73,7 @@ def test_distinct_vocabulary_connection_is_refused_rather_than_targeted( [databases.cdm_db] kind = "cdm" connection = "cdm" -schema_name = "main" vocab_connection = "vocabulary" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -82,9 +83,9 @@ def test_distinct_vocabulary_connection_is_refused_rather_than_targeted( snapshot = load_configuration(config_path=path) - assert snapshot.state is ConfigurationState.INCOMPLETE - assert [issue.code for issue in snapshot.issues] == ["vocabulary_connection_split"] - assert resolve_database_targets(snapshot) == () + assert snapshot.state is ConfigurationState.UNVERIFIED + assert snapshot.issues == () + assert len(resolve_database_targets(snapshot)) == 3 def test_database_verification_records_latency(tmp_path: Path) -> None: @@ -141,11 +142,13 @@ def test_graph_readiness_reports_missing_sidecars_and_indexes(tmp_path: Path) -> assert result.connected is True assert result.has_warnings is True - assert {diagnostic.code for diagnostic in result.diagnostics} >= { - "graph_tables_empty", + codes = {diagnostic.code for diagnostic in result.diagnostics} + assert "graph_tables_empty" in codes + assert not codes & { "fulltext_sidecar_missing", "fulltext_indexes_missing", "functional_indexes_missing", + "functional_indexes_unchecked", } @@ -186,7 +189,7 @@ def test_graph_readiness_reports_unpopulated_fulltext_sidecars(tmp_path: Path) - result = verify_database_target(_target(db_path, role="graph")) - assert "fulltext_sidecar_unpopulated" in { + assert "fulltext_sidecar_unpopulated" not in { diagnostic.code for diagnostic in result.diagnostics } @@ -211,7 +214,29 @@ def test_functional_index_detection_accepts_hand_named_postgres_expressions() -> ) -def test_groundworkers_tuning_reports_missing_trigram_indexes(tmp_path: Path) -> None: +def test_trigram_reflection_checks_postgresql_operator_class(monkeypatch) -> None: + from types import SimpleNamespace + + monkeypatch.setattr( + "groundworkers.application.setup.databases._reflected_indexes", + lambda *_args: ( + { + "name": "arbitrary_name", + "expressions": ["lower((concept_name)::text)"], + "dialect_options": { + "postgresql_using": "gin", + "postgresql_ops": {"lower": "gin_trgm_ops"}, + }, + }, + ), + ) + connection = SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) + assert _trigram_lower_index_exists( + connection, None, "public", "concept", "concept_name", ("legacy_alias",) + ) + + +def test_groundworkers_tuning_skips_postgres_trigram_checks_on_sqlite(tmp_path: Path) -> None: db_path = tmp_path / "omop.db" _create_tables(db_path, ("concept", "concept_synonym")) target = _target(db_path, role="groundworkers") @@ -219,90 +244,120 @@ def test_groundworkers_tuning_reports_missing_trigram_indexes(tmp_path: Path) -> result = verify_database_target(target) assert result.connected is True - assert result.has_warnings is True - assert result.diagnostics[0].code == "trigram_indexes_missing" + assert not { + "trigram_indexes_missing", + "trigram_indexes_unchecked", + } & {diagnostic.code for diagnostic in result.diagnostics} def test_embedding_readiness_reports_valid_registered_model(tmp_path: Path) -> None: - db_path = tmp_path / "emb.db" - engine = create_engine(f"sqlite:///{db_path}") - with engine.begin() as connection: - connection.execute( - text( - """ - CREATE TABLE model_registry ( - model_name TEXT PRIMARY KEY, - storage_identifier TEXT NOT NULL, - dimensions INTEGER NOT NULL, - index_type TEXT NOT NULL, - metric_type TEXT - ) - """ - ) - ) - connection.execute( - text( - """ - INSERT INTO model_registry - (model_name, storage_identifier, dimensions, index_type, metric_type) - VALUES ('test-model', 'emb_test_model', 3, 'FLAT', NULL) - """ - ) - ) - connection.execute( - text( - """ - CREATE TABLE emb_test_model ( - concept_id INTEGER PRIMARY KEY, - domain_id TEXT NOT NULL, - vocabulary_id TEXT NOT NULL, - is_standard BOOLEAN NOT NULL, - is_valid BOOLEAN NOT NULL, - embedding BLOB NOT NULL - ) - """ - ) - ) - connection.execute( - text( - """ - INSERT INTO emb_test_model - (concept_id, domain_id, vocabulary_id, is_standard, is_valid, embedding) - VALUES (1, 'Condition', 'SNOMED', 1, 1, x'000102') - """ - ) - ) - engine.dispose() - - result = verify_database_target(_target(db_path, role="embedding")) + target = _resolved_embedding_targets(tmp_path, registered_models=("test-model",))["database.embedding"] + result = verify_database_target(target) assert result.connected is True - assert result.has_warnings is False + assert result.has_warnings is True assert {diagnostic.code for diagnostic in result.diagnostics} == { "embedding_registry_present", - "embedding_table_valid", + "embedding_table_empty", } -def test_the_registry_is_read_from_the_configured_schema(tmp_path: Path) -> None: - """The hand-written SELECT this replaced named the table unqualified, and - text() is exempt from schema_translate_map -- so a store with a schema was - written by omop-emb into it and read back from the default one.""" - default_path = tmp_path / "default.db" - store_path = tmp_path / "store.db" - _create_registry(store_path, model_name="in-the-store") - _create_registry(default_path, model_name="in-the-default-schema") +def test_registry_reader_uses_the_resolved_registry_tag(tmp_path: Path) -> None: + from oa_configurator import physical_schema_of + from omop_emb.config import REGISTRY_SCHEMA_KEY - engine = create_engine(f"sqlite:///{default_path}") - with engine.connect() as connection: - connection.execute(text(f"ATTACH DATABASE '{store_path}' AS vectors")) + targets = _resolved_embedding_targets(tmp_path, registered_models=("tagged",)) + target = targets["database.embedding"] + from omop_emb.backends import open_vector_store_writer + + writer = open_vector_store_writer(target.resolved_vector_store) + try: + assert physical_schema_of(writer.emb_engine, schema_tag=REGISTRY_SCHEMA_KEY) is None + finally: + writer.close() + result = verify_database_target(target) + assert "embedding_registry_present" in {d.code for d in result.diagnostics} + + +def test_read_paths_use_reader_and_writer_attempts_are_refused(tmp_path: Path) -> None: + from omop_emb.backends import open_vector_store_reader + from omop_emb.utils.errors import ReadOnlyStoreError + + target = _resolved_embedding_targets(tmp_path, registered_models=("read-only",))[ + "database.embedding" + ] + reader = open_vector_store_reader(target.resolved_vector_store) + try: + result = verify_database_target(target) + assert result.connected is True + with pytest.raises(ReadOnlyStoreError): + reader.register_model( + model_name="forbidden-write", + dimensions=3, + provider_type="ollama", + ) + finally: + reader.close() + + +def test_duplicate_model_registration_has_actionable_conflict(tmp_path: Path) -> None: + from omop_emb.backends import open_vector_store_writer + from omop_emb.backends.index_config import FlatIndexConfig + from omop_emb.utils.errors import ModelRegistrationConflictError + + target = _resolved_embedding_targets(tmp_path, registered_models=("duplicate",))[ + "database.embedding" + ] + writer = open_vector_store_writer(target.resolved_vector_store) + try: + with pytest.raises(ModelRegistrationConflictError) as caught: + writer.register_model( + model_name="duplicate", + dimensions=4, + provider_type="ollama", + index_config=FlatIndexConfig(), + ) + finally: + writer.close() + failure = classify_connection_error(caught.value) + assert failure.kind is ConnectionFailureKind.MODEL_REGISTRATION_CONFLICT + assert "one store per model per database" in failure.detail + assert "reuse the existing store or choose another database" in failure.detail + + +def test_legacy_registry_error_is_specific_and_actionable() -> None: + from omop_emb.model_registry.model_registry_orm import MIGRATION_COMMAND + from omop_emb.utils.errors import LegacyRegistryError + + failure = classify_connection_error(LegacyRegistryError("old registry")) + + assert failure.kind is ConnectionFailureKind.LEGACY_REGISTRY + assert "predates omop-emb 3.0" in failure.detail + assert "migration script is tracked" in failure.detail + assert "once it ships" in failure.detail + assert "oa-configurator issue #40" in failure.detail + assert MIGRATION_COMMAND in failure.detail - rows = _embedding_registry_rows(connection, schema="vectors") - unqualified = _embedding_registry_rows(connection, schema=None) - engine.dispose() - assert [row["model_name"] for row in rows] == ["in-the-store"] - assert [row["model_name"] for row in unqualified] == ["in-the-default-schema"] +@pytest.mark.parametrize( + ("error", "kind", "detail"), + ( + ( + MisplacedRegistryError(), + ConnectionFailureKind.MISPLACED_REGISTRY, + "outside the schema resolved", + ), + ( + MissingStorageTableError(), + ConnectionFailureKind.MISSING_STORAGE_TABLE, + "storage table is missing", + ), + ), +) +def test_registry_layout_errors_have_specific_diagnostics(error, kind, detail) -> None: + failure = classify_connection_error(error) + assert failure.kind is kind + assert detail in failure.detail def test_the_embedding_targets_carry_the_stores_schema(tmp_path: Path) -> None: @@ -315,102 +370,60 @@ def test_the_embedding_targets_carry_the_stores_schema(tmp_path: Path) -> None: target.key: target for target in resolve_database_targets(load_configuration(config_path=path)) } + assert targets["database.embedding"].resolved_vector_store.database.schema_name == "groundworkers" + assert targets["database.groundworkers"].resolved_vector_store.database.schema_name == "groundworkers" + assert ( + targets["database.embedding"].resolved_vector_store.database.schema_name + == "groundworkers" + ) - assert targets["database.embedding"].embedding_schema == "groundworkers" - assert targets["database.groundworkers"].embedding_schema == "groundworkers" - assert targets["database.cdm"].embedding_schema is None +def _resolved_embedding_targets(tmp_path: Path, *, registered_models: tuple[str, ...]): + from omop_emb.backends import open_vector_store_writer + from omop_emb.backends.index_config import FlatIndexConfig -def _create_registry(db_path: Path, *, model_name: str) -> None: - engine = create_engine(f"sqlite:///{db_path}") - with engine.begin() as connection: - connection.execute( - text( - """ - CREATE TABLE model_registry ( - model_name TEXT PRIMARY KEY, - storage_identifier TEXT NOT NULL, - dimensions INTEGER NOT NULL, - index_type TEXT NOT NULL, - metric_type TEXT - ) - """ + stack = build_embedding_stack() + stack.connections["embedding_main"].database_name = str(tmp_path / "emb.db") + stack.connections["cdm_main"].database_name = str(tmp_path / "cdm.db") + config_path = tmp_path / "config.toml" + save_stack_config(stack, config_path) + targets = { + target.key: target + for target in resolve_database_targets(load_configuration(config_path=config_path)) + } + cdm_engine = create_engine(targets["database.groundworkers"].connection_url) + with cdm_engine.begin() as connection: + connection.execute(text('CREATE TABLE IF NOT EXISTS "concept" (id INTEGER PRIMARY KEY)')) + connection.execute(text('CREATE TABLE IF NOT EXISTS "concept_synonym" (id INTEGER PRIMARY KEY)')) + cdm_engine.dispose() + store = open_vector_store_writer(targets["database.embedding"].resolved_vector_store) + try: + for model_name in registered_models: + store.register_model( + model_name=model_name, + dimensions=3, + provider_type="ollama", + index_config=FlatIndexConfig(), ) - ) - connection.execute( - text( - "INSERT INTO model_registry " - "(model_name, storage_identifier, dimensions, index_type, metric_type) " - "VALUES (:model_name, 'emb_test_model', 3, 'FLAT', NULL)" - ), - {"model_name": model_name}, - ) - engine.dispose() + finally: + store.close() + return targets def test_groundworkers_tuning_warns_when_grounding_model_is_missing( tmp_path: Path, ) -> None: - db_path = tmp_path / "emb.db" - engine = create_engine(f"sqlite:///{db_path}") - with engine.begin() as connection: - connection.execute(text('CREATE TABLE "concept" (id INTEGER PRIMARY KEY)')) - connection.execute( - text('CREATE TABLE "concept_synonym" (id INTEGER PRIMARY KEY)') - ) - connection.execute( - text( - """ - CREATE TABLE model_registry ( - model_name TEXT PRIMARY KEY, - storage_identifier TEXT NOT NULL, - dimensions INTEGER NOT NULL, - index_type TEXT NOT NULL, - metric_type TEXT - ) - """ - ) - ) - connection.execute( - text( - """ - INSERT INTO model_registry - (model_name, storage_identifier, dimensions, index_type, metric_type) - VALUES ('other-model', 'emb_other_model', 3, 'FLAT', NULL) - """ - ) - ) - connection.execute( - text( - """ - CREATE TABLE emb_other_model ( - concept_id INTEGER PRIMARY KEY, - domain_id TEXT NOT NULL, - vocabulary_id TEXT NOT NULL, - is_standard BOOLEAN NOT NULL, - is_valid BOOLEAN NOT NULL, - embedding BLOB NOT NULL - ) - """ - ) - ) - engine.dispose() - - result = verify_database_target( - _target( - db_path, - role="groundworkers", - expected_model_name="missing-model", - embedding_connection_url=f"sqlite:///{db_path}", - ) - ) + targets = _resolved_embedding_targets(tmp_path, registered_models=("other-model",)) + result = verify_database_target(targets["database.groundworkers"]) assert result.connected is True assert result.has_warnings is True assert {diagnostic.code for diagnostic in result.diagnostics} >= { - "trigram_indexes_missing", "grounding_embedding_model_missing", } + assert "trigram_indexes_missing" not in { + diagnostic.code for diagnostic in result.diagnostics + } def test_connection_errors_are_classified_without_original_message() -> None: @@ -432,26 +445,52 @@ def test_connection_errors_are_classified_without_original_message() -> None: assert "private" not in failure.detail +@pytest.mark.parametrize( + ("sqlstate", "kind"), + ( + ("3D000", ConnectionFailureKind.DATABASE_MISSING), + ("3F000", ConnectionFailureKind.SCHEMA_MISSING), + ("42P01", ConnectionFailureKind.TABLE_MISSING), + ), +) +def test_sqlstate_precedes_ambiguous_does_not_exist_text(sqlstate, kind) -> None: + error = RuntimeError("role does not exist") + error.sqlstate = sqlstate + error.orig = SimpleNamespace(sqlstate=sqlstate) + + assert classify_connection_error(error).kind is kind + + +def test_oa_schema_drift_classification_includes_acknowledgement_hint() -> None: + error = type("SchemaDriftError", (Exception,), {})("schema changed") + + failure = classify_connection_error(error) + + assert failure.kind is ConnectionFailureKind.QUERY + assert "omop-config acknowledge-schema-migration" in failure.next_action + + def _target( db_path: Path, *, role: str, expected_model_name: str | None = None, - embedding_connection_url: str | None = None, ) -> DatabaseTarget: + resolved_cdm = Resolver( + build_cdm_stack(sqlite_path=str(db_path)) + ).resolve_database("cdm_db") return DatabaseTarget( key=f"database.{role}", label=role, database_entry_name="cdm_db", - connection_name="main", - safe_url=f"sqlite:///{db_path}", - cdm_schema="main", - vocabulary_schema="main", - connection_url=f"sqlite:///{db_path}", + connection_name=resolved_cdm.connection.name, + safe_url=resolved_cdm.connection.safe_url, + cdm_schema=None, + vocabulary_schema=None, + connection_url=resolved_cdm.connection.url, role=role, expected_embedding_model_name=expected_model_name, - embedding_safe_url=embedding_connection_url, - embedding_connection_url=embedding_connection_url, + resolved_cdm_database=resolved_cdm, ) diff --git a/tests/unit/test_setup_embeddings.py b/tests/unit/test_setup_embeddings.py index 9e525ae..1bcbc8a 100644 --- a/tests/unit/test_setup_embeddings.py +++ b/tests/unit/test_setup_embeddings.py @@ -75,9 +75,9 @@ def __init__(self, records=(), populated: bool = False) -> None: def get_registered_models(self): return tuple(self.records) - def has_any_embeddings(self, *, model_name, metric_type, _model_record): - assert model_name == _model_record.model_name - assert metric_type is not None + def has_any_embeddings(self, *, model_name): + if self.records: + assert model_name == self.records[0].model_name return self.populated @@ -469,12 +469,12 @@ def close(self) -> None: store = ReadOnlyStore() monkeypatch.setattr( - "groundworkers.application.setup.embedding_population.inspect_resolved_vector_store", + "groundworkers.application.setup.embedding_population.open_vector_store_reader", lambda resolved: captured.setdefault("store", resolved) and store, ) monkeypatch.setattr( - "groundworkers.application.setup.embedding_population._cdm_engine_and_schema", - lambda _snapshot: (DisposableEngine(), "main"), + "groundworkers.application.setup.embedding_population._cdm_session_factory_and_engines", + lambda _snapshot: (lambda: None, (DisposableEngine(),)), ) monkeypatch.setattr( "groundworkers.application.setup.embedding_population.plan_population", @@ -533,7 +533,7 @@ def fail(_store): raise RuntimeError("database-password") monkeypatch.setattr( - "groundworkers.application.setup.embedding_population.inspect_resolved_vector_store", + "groundworkers.application.setup.embedding_population.open_vector_store_reader", fail, ) diff --git a/tests/unit/test_setup_provider_conformance.py b/tests/unit/test_setup_provider_conformance.py index 22566f8..f38dc4c 100644 --- a/tests/unit/test_setup_provider_conformance.py +++ b/tests/unit/test_setup_provider_conformance.py @@ -50,8 +50,8 @@ "cdm", { "cdm_db_name": "warehouse_cdm", - "schema_name": "main", - "vocab_schema": "main", + "cdm_schema": None, + "vocab_schema": None, }, ), ), @@ -179,7 +179,7 @@ def invalid_submission(service, draft): def advance_revision(service): # Another operator writes the file after this provider prepared its plan. - save_stack_config(build_cdm_stack(schema_name="elsewhere"), service._path) + save_stack_config(build_cdm_stack(cdm_schema="elsewhere"), service._path) assert_mutation_service_conformance( _factory(tmp_path, f"{name}-hooked"), diff --git a/tests/unit/test_setup_runtime.py b/tests/unit/test_setup_runtime.py index 8e84505..a54882d 100644 --- a/tests/unit/test_setup_runtime.py +++ b/tests/unit/test_setup_runtime.py @@ -40,8 +40,6 @@ def test_runtime_sections_load_from_plain_tool_mappings_and_redact_secrets( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -72,7 +70,7 @@ def test_runtime_sections_load_from_plain_tool_mappings_and_redact_secrets( # Graph/grounding policy comes from Groundworkers' own configuration, not from # omop-graph's internal package config. assert graph.cdm_database_name == "cdm_db" - assert graph.vocabulary_schema == "main" + assert graph.vocabulary_schema is None assert graph.grounding_max_depth == 4 assert graph.min_fulltext_overlap == 0.25 assert provider is not None @@ -96,8 +94,6 @@ def test_runtime_sections_do_not_treat_package_defaults_as_configuration( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" """, encoding="utf-8", ) @@ -377,8 +373,6 @@ def _write_llm_config( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" diff --git a/tests/unit/test_setup_truth.py b/tests/unit/test_setup_truth.py index e361806..e92f948 100644 --- a/tests/unit/test_setup_truth.py +++ b/tests/unit/test_setup_truth.py @@ -143,7 +143,7 @@ def test_fresh_cdm_fields_are_postgresql_first_and_memory_sqlite_is_rejected( assert fields["host"].default == "localhost" assert fields["port"].default == 5432 assert fields["database_name"].default is None - assert fields["schema_name"].default == "public" + assert fields["cdm_schema"].default == "public" service.submit( draft, diff --git a/tests/unit/test_setup_write_flows.py b/tests/unit/test_setup_write_flows.py index 57fc977..4da533d 100644 --- a/tests/unit/test_setup_write_flows.py +++ b/tests/unit/test_setup_write_flows.py @@ -18,7 +18,7 @@ ConfigWizardController, MutationOperation, ) -from oa_configurator import load_stack_config_from_path, save_stack_config +from oa_configurator import load_stack_config, save_stack_config from groundworkers.application.setup.configuration_provider import ( CDM_SETUP_TARGET, @@ -110,7 +110,7 @@ def test_chat_setup_creates_then_updates_the_same_target(tmp_path: Path) -> None _drive_llm_to_review(controller) assert controller.apply().status.value == "applied" - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) saved = GroundworkersConfig.validate_candidate(stack) assert saved.llm_model_name == "chat_model" assert stack.models["chat_model"].model == "second-model:v1" @@ -131,7 +131,7 @@ def test_chat_setup_creates_then_updates_the_same_target(tmp_path: Path) -> None controller.submit({"llm_model_entry_name": "chat_model", "llm_model_choice": "first-model:v1"}) assert controller.apply().status.value == "applied" - updated_stack = load_stack_config_from_path(path) + updated_stack = load_stack_config(path) updated = GroundworkersConfig.validate_candidate(updated_stack) assert updated.llm_model_name == "chat_model" assert updated_stack.models["chat_model"].model == "first-model:v1" @@ -158,7 +158,7 @@ def test_blank_api_key_on_update_preserves_the_stored_credential( controller.submit({"llm_model_entry_name": "chat_model", "llm_model_choice": "first-model:v1"}) controller.apply() - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) assert stack.providers["chat_provider"].api_key == "original-secret" @@ -183,7 +183,7 @@ def test_cdm_setup_creates_connection_and_database_entries(tmp_path: Path) -> No result = controller.apply() assert result.status.value == "applied" - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) assert "warehouse" in stack.connections assert "warehouse_cdm" in stack.databases assert GroundworkersConfig.validate_candidate(stack).cdm_db == "warehouse_cdm" @@ -419,7 +419,7 @@ def test_concurrent_edit_is_reported_as_conflicted(tmp_path: Path) -> None: _drive_llm_to_review(controller) # Another writer changes the file after the plan was prepared. - other = build_cdm_stack(schema_name="other") + other = build_cdm_stack(cdm_schema="other") save_stack_config(other, path) other_bytes = path.read_text(encoding="utf-8") @@ -457,7 +457,7 @@ def test_embedding_model_name_is_never_reused_as_the_chat_model( _drive_llm_to_review(controller) controller.apply() - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) saved = GroundworkersConfig.validate_candidate(stack) assert saved.llm_model_name == "chat_model" assert stack.models["chat_model"].model == "second-model:v1" diff --git a/tests/unit/test_split_classified_edges.py b/tests/unit/test_split_classified_edges.py new file mode 100644 index 0000000..dce1da6 --- /dev/null +++ b/tests/unit/test_split_classified_edges.py @@ -0,0 +1,96 @@ +from __future__ import annotations + +from datetime import date + +import pytest +from oa_configurator import CDMDatabaseConfig, ConnectionConfig, Resolver +from omop_alchemy.cdm.model.vocabulary import Concept, Concept_Relationship +from omop_alchemy.config import create_cdm_engines +from omop_alchemy.cross_database import cdm_sessionmaker +from omop_graph.extensions.omop_alchemy import RelationshipMapping + +from groundworkers.services.graph import GraphService +from tests.support.stack_config import build_cdm_stack + + +@pytest.mark.parametrize("split", [False, True]) +def test_classified_edges_read_primary_mapping_and_vocab_edges( + tmp_path, split: bool +) -> None: + stack = build_cdm_stack(sqlite_path=str(tmp_path / "primary.db")) + if split: + stack.connections["vocab_db"] = ConnectionConfig( + dialect="sqlite", database_name=str(tmp_path / "vocabulary.db") + ) + stack.databases["cdm_db"] = CDMDatabaseConfig( + connection="cdm_main", vocab_connection="vocab_db" + ) + resolved = Resolver(stack).resolve_database("cdm_db") + primary, vocab = create_cdm_engines(resolved, register_claims=False) + RelationshipMapping.__table__.create(primary) + Concept.__table__.create(vocab) + Concept_Relationship.__table__.create(vocab) + with primary.begin() as connection: + connection.execute( + RelationshipMapping.__table__.insert(), + { + "relationship_id": "Is a", + "predicate_kind": "Hierarchy", + "predicate_subkind": "Taxonomic - up", + }, + ) + with vocab.begin() as connection: + connection.execute( + Concept.__table__.insert(), + { + "concept_id": 2, + "concept_name": "Target", + "domain_id": "Condition", + "vocabulary_id": "SNOMED", + "concept_class_id": "Clinical Finding", + "standard_concept": "S", + "concept_code": "2", + "valid_start_date": date(2000, 1, 1), + "valid_end_date": date(2099, 12, 31), + }, + ) + connection.execute( + Concept_Relationship.__table__.insert(), + { + "concept_id_1": 1, + "concept_id_2": 2, + "relationship_id": "Is a", + "valid_start_date": date(2000, 1, 1), + "valid_end_date": date(2099, 12, 31), + }, + ) + + session_factory = cdm_sessionmaker(resolved, primary=primary, vocab=vocab) + try: + with session_factory() as session: + rows = GraphService._query_classified_edges( + session, + 1, + "Hierarchy", + None, + False, + 10, + outbound=True, + ) + assert rows == [ + { + "relationship_id": "Is a", + "predicate_subkind": "Taxonomic - up", + "target_concept_id": 2, + "target_concept_name": "Target", + "vocabulary_id": "SNOMED", + "domain_id": "Condition", + "concept_class_id": "Clinical Finding", + "standard_concept": "S", + "valid": True, + } + ] + finally: + primary.dispose() + if vocab is not primary: + vocab.dispose() diff --git a/tests/unit/test_tui_app.py b/tests/unit/test_tui_app.py index b68a204..a63fb66 100644 --- a/tests/unit/test_tui_app.py +++ b/tests/unit/test_tui_app.py @@ -343,8 +343,6 @@ def test_database_actions_render_in_workspace_and_verify_connections( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -401,8 +399,6 @@ def test_groundworkers_tuning_row_does_not_offer_configure( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -450,7 +446,6 @@ def test_configure_action_opens_database_wizard(tmp_path: Path) -> None: [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -476,6 +471,52 @@ async def run_check() -> None: asyncio.run(run_check()) +def test_legacy_configure_action_shows_repair_message_without_crashing( + tmp_path: Path, +) -> None: + pytest.importorskip("groundskeeping") + + from groundskeeping.app import OperatorApp + + from groundworkers.application.setup.configuration import load_configuration + from groundworkers.tui.app import build_groundworkers_tui_spec + from groundworkers.tui.presenters.overview import OverviewPresenter + + config_path = tmp_path / "legacy.toml" + config_path.write_text( + '[connections.cdm_main]\ndialect = "sqlite"\ndatabase_name = ":memory:"\n' + '[databases.cdm_db]\nkind = "cdm"\nconnection = "cdm_main"\n' + 'schema_name = "public"\n[tools.groundworkers]\ncdm_db = "cdm_db"\n', + encoding="utf-8", + ) + snapshot = load_configuration(config_path=config_path) + landing = OverviewPresenter().landing( + snapshot, + connections=(), + embedding_coverage=None, + llm_result=None, + graph_ready=False, + integration_ready=False, + ) + assert "Issues:" in landing.message + assert "schema_name" in landing.message or "upgrade" in landing.message.lower() + + async def run_check() -> None: + app = OperatorApp(build_groundworkers_tui_spec(config_path=str(config_path))) + async with app.run_test(size=(100, 32), notifications=True) as pilot: + await pilot.pause() + await pilot.click("#view-action-0") + await pilot.pause() + assert app.is_running + assert not app.query("#wizard-frame") + notifications = list(app._notifications) + assert notifications + assert notifications[0].severity == "warning" + assert notifications[0].message + + asyncio.run(run_check()) + + def test_chat_model_wizard_renders_discovered_model_choices( tmp_path: Path, monkeypatch, @@ -503,8 +544,6 @@ def test_chat_model_wizard_renders_discovered_model_choices( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -581,8 +620,6 @@ def test_llm_provider_detail_renders_model_inventory_table( [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" -vocab_schema = "main" [tools.groundworkers] cdm_db = "cdm_db" diff --git a/tests/unit/test_tui_state.py b/tests/unit/test_tui_state.py index 5f17721..3ab91e5 100644 --- a/tests/unit/test_tui_state.py +++ b/tests/unit/test_tui_state.py @@ -111,7 +111,6 @@ def test_database_failure_status_uses_classified_kind(tmp_path: Path) -> None: [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -125,8 +124,8 @@ def test_database_failure_status_uses_classified_kind(tmp_path: Path) -> None: database_entry_name="cdm_db", connection_name="main", safe_url="sqlite:///:memory:", - cdm_schema="main", - vocabulary_schema="main", + cdm_schema=None, + vocabulary_schema=None, connection_url="sqlite:///:memory:", ) view = DatabasePresenter().landing( @@ -161,7 +160,6 @@ def test_database_warning_status_uses_successful_diagnostics(tmp_path: Path) -> [databases.cdm_db] kind = "cdm" connection = "main" -schema_name = "main" [tools.groundworkers] cdm_db = "cdm_db" @@ -175,8 +173,8 @@ def test_database_warning_status_uses_successful_diagnostics(tmp_path: Path) -> database_entry_name="cdm_db", connection_name="main", safe_url="sqlite:///:memory:", - cdm_schema="main", - vocabulary_schema="main", + cdm_schema=None, + vocabulary_schema=None, connection_url="sqlite:///:memory:", ) view = DatabasePresenter().landing( diff --git a/tests/unit/test_vector_store_setup.py b/tests/unit/test_vector_store_setup.py index 989fccb..d17d4fb 100644 --- a/tests/unit/test_vector_store_setup.py +++ b/tests/unit/test_vector_store_setup.py @@ -1,9 +1,11 @@ from __future__ import annotations +import tomllib from pathlib import Path +import pytest from groundskeeping.configurator import ConfigWizardController, MutationOperation -from oa_configurator import load_stack_config_from_path +from oa_configurator import ConnectionConfig, load_stack_config from oa_configurator.io import save_stack_config from groundworkers.application.setup.configuration import load_configuration @@ -68,11 +70,13 @@ def test_the_journey_creates_the_store_its_database_and_the_reference( "applied" ) - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) config = GroundworkersConfig.validate_candidate(stack) assert config.vector_store_name == "embeddings" assert stack.vector_stores["embeddings"].database == "embedding_db" assert stack.databases["embedding_db"].kind == "generic" + saved = tomllib.loads(path.read_text(encoding="utf-8")) + assert "schema_name" not in saved["databases"]["embedding_db"] def test_the_default_location_puts_the_vectors_in_the_cdm_database( @@ -84,16 +88,100 @@ def test_the_default_location_puts_the_vectors_in_the_cdm_database( assert _drive(path).apply().status.value == "applied" - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) derived = stack.databases["cdm_db_vectors"] cdm = stack.databases["cdm_db"] assert stack.vector_stores["embeddings"].database == "cdm_db_vectors" assert derived.kind == "generic" assert derived.connection == cdm.connection - assert derived.schema_name == cdm.schema_name + assert derived.schema_name == cdm.cdm_schema assert set(stack.connections) == {"cdm_main"} +@pytest.mark.parametrize( + ("dialect", "backend", "cdm_schema", "vocab_schema", "store_schema"), + ( + ("sqlite", "sqlitevec", None, None, None), + ("postgresql", "pgvector", "clinical", "vocabulary", "groundworkers"), + ), +) +def test_cdm_store_location_round_trips_when_the_wizard_reopens( + tmp_path: Path, + dialect: str, + backend: str, + cdm_schema: str | None, + vocab_schema: str | None, + store_schema: str | None, +) -> None: + path = tmp_path / f"{dialect}.toml" + save_stack_config( + build_cdm_stack( + sqlite_path=str(tmp_path / "cdm.db"), + cdm_schema=cdm_schema, + vocab_schema=vocab_schema, + ), + path, + ) + + assert _drive(path, backend_type=backend).apply().status.value == "applied" + + stack = load_stack_config(path) + store_database = stack.databases["cdm_db_vectors"] + assert store_database.schema_name == store_schema + reopened = ConfigWizardController( + vector_store_setup_workflow(MutationOperation.UPDATE), + GroundworkersConfigMutationService(config_path=path), + ) + snapshot = reopened.start() + location = next( + field for field in snapshot.step.fields if field.key == "store_location" + ) + assert location.default == STORE_LOCATION_CDM + + +@pytest.mark.parametrize("same_connection", [False, True]) +def test_separate_store_location_round_trips_on_either_connection( + tmp_path: Path, same_connection: bool +) -> None: + path = tmp_path / "config.toml" + stack = build_cdm_stack( + cdm_schema="clinical", vocab_schema="vocabulary" + ) + if not same_connection: + stack.connections["vectors"] = ConnectionConfig( + dialect="postgresql+psycopg", + host="vectors.example.test", + port=5432, + user="groundworkers", + database_name="vectors", + ) + save_stack_config(stack, path) + database_name = "cdm_db_vectors" if same_connection else "separate_vectors" + connection_name = "cdm_main" if same_connection else "vectors" + + assert _drive( + path, + location=STORE_LOCATION_SEPARATE, + backend_type="pgvector", + store_database_name=database_name, + store_connection_name=connection_name, + store_schema_name="vectors", + ).apply().status.value == "applied" + + reopened = ConfigWizardController( + vector_store_setup_workflow(MutationOperation.UPDATE), + GroundworkersConfigMutationService(config_path=path), + ) + location = next( + field for field in reopened.start().step.fields if field.key == "store_location" + ) + assert location.default == STORE_LOCATION_SEPARATE + saved = load_stack_config(path) + store_database = saved.databases[database_name] + assert store_database.connection == connection_name + assert store_database.schema_name == "vectors" + + def test_the_cdm_location_needs_no_database_answers(tmp_path: Path) -> None: """The database step is the whole cost of the opt-in, so it must not be asked of everyone.""" @@ -124,7 +212,7 @@ def test_the_backend_choice_is_carried_through(tmp_path: Path) -> None: path, location=STORE_LOCATION_SEPARATE, backend_type="pgvector" ).apply() - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) assert stack.vector_stores["embeddings"].backend_type == "pgvector" @@ -155,12 +243,15 @@ def test_the_schema_default_follows_the_cdm_rather_than_public( """'public' is a PostgreSQL name, and it was offered whatever the dialect and whatever the CDM's own schema was.""" path = tmp_path / "config.toml" - save_stack_config(build_cdm_stack(schema_name="omop", vocab_schema="omop"), path) + save_stack_config(build_cdm_stack(cdm_schema="omop", vocab_schema="omop"), path) _drive(path, location=STORE_LOCATION_SEPARATE).apply() - stack = load_stack_config_from_path(path) - assert stack.databases["embedding_db"].schema_name == "omop" + stack = load_stack_config(path) + store_name = GroundworkersConfig.validate_candidate(stack).vector_store_name + assert store_name is not None + store_database = stack.vector_stores[store_name].database + assert stack.databases[store_database].schema_name == "groundworkers" def test_a_blank_faiss_cache_is_left_unset(tmp_path: Path) -> None: @@ -169,7 +260,7 @@ def test_a_blank_faiss_cache_is_left_unset(tmp_path: Path) -> None: _drive(path, faiss_cache_dir="").apply() - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) assert stack.vector_stores["embeddings"].faiss_cache_dir is None @@ -181,7 +272,7 @@ def test_the_store_reuses_a_named_connection_rather_than_defining_one( _drive(path, location=STORE_LOCATION_SEPARATE).apply() - stack = load_stack_config_from_path(path) + stack = load_stack_config(path) assert set(stack.connections) == {"cdm_main"} assert stack.databases["embedding_db"].connection == "cdm_main" diff --git a/tests/unit/test_vocab_service.py b/tests/unit/test_vocab_service.py index c90cda0..0cf8bf4 100644 --- a/tests/unit/test_vocab_service.py +++ b/tests/unit/test_vocab_service.py @@ -9,6 +9,7 @@ Concept_Ancestor, Concept_Relationship, Concept_Synonym, + Domain, ) from sqlalchemy import column as sa_column from sqlalchemy import create_engine, event @@ -19,10 +20,13 @@ from groundworkers.base.sql import normalization_expression from groundworkers.services.vocab import VocabService, normalize_text_for_matching from groundworkers.tools.search_tools import register_search_tools +from tests.support.omop import route_sqlite_omop_roles def _service(tmp_path) -> VocabService: - engine = create_engine(f"sqlite:///{tmp_path / 'vocabulary.db'}") + engine = route_sqlite_omop_roles( + create_engine(f"sqlite:///{tmp_path / 'vocabulary.db'}") + ) @event.listens_for(engine, "connect") def sqlite_normalisation_functions(connection, _record) -> None: @@ -38,10 +42,19 @@ def regexp_replace(value, pattern, replacement, _flags): Concept_Synonym.__table__, Concept_Relationship.__table__, Concept_Ancestor.__table__, + Domain.__table__, ): table.create(engine) with engine.begin() as connection: + connection.execute( + Domain.__table__.insert(), + { + "domain_id": "Condition", + "domain_name": "Condition", + "domain_concept_id": 19, + }, + ) connection.execute( Concept.__table__.insert(), ( diff --git a/uv.lock b/uv.lock index dba1d5d..98f6d0c 100644 --- a/uv.lock +++ b/uv.lock @@ -1091,16 +1091,16 @@ wheels = [ [[package]] name = "groundskeeping" -version = "0.8.1" +version = "0.9.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "oa-configurator" }, { name = "pydantic" }, { name = "textual" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/8b/2b/3a898cd1ab1ea5e5415dd2bb963f4a9f4127e5473fa20adac95a40129299/groundskeeping-0.8.1.tar.gz", hash = "sha256:181c5c478fbed19b94ee6e868b593930de3f1930b211797ab554c84e88eab1e2", size = 211336, upload-time = "2026-08-31T00:08:33.415Z" } +sdist = { url = "https://files.pythonhosted.org/packages/72/da/17a42fd95f145756e012d7dcff33e984ae6dd1c5875ea6a0fb79dbb1ab16/groundskeeping-0.9.1.tar.gz", hash = "sha256:89ad50f94a80895728593aeee6f79cd03f43d32044339e5287b89b6b8e2b977c", size = 211780, upload-time = "2026-10-11T02:14:34.241Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/de/23/56ae96fea86d9e0b69c8b27a347da482b77aa7586b1cc14dab7218c481b7/groundskeeping-0.8.1-py3-none-any.whl", hash = "sha256:6f1cd0f460f0a4ab9d64605e26274b9f09983b434131af445c165754c83f518d", size = 79671, upload-time = "2026-08-31T00:08:32.035Z" }, + { url = "https://files.pythonhosted.org/packages/ac/64/c7bb7fdf4a549bcb229ff342ae5db6b112187fdd00714951a72fc04bfa7c/groundskeeping-0.9.1-py3-none-any.whl", hash = "sha256:a8cc7a008a4ad6c34de95cf6fd27e6a86bc32533e1ffbabd2e0807a17490540f", size = 80317, upload-time = "2026-10-11T02:14:32.543Z" }, ] [[package]] @@ -1174,7 +1174,7 @@ xlsx = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.110,<1.0" }, - { name = "groundskeeping", marker = "extra == 'tui'", specifier = ">=0.8.1,<1" }, + { name = "groundskeeping", marker = "extra == 'tui'", specifier = ">=0.9.1,<1" }, { name = "h5py", marker = "extra == 'embedding-artifacts'", specifier = ">=3.10,<4" }, { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27,<1" }, { name = "ipython", marker = "extra == 'dev'", specifier = ">=8.0" }, @@ -1185,13 +1185,13 @@ requires-dist = [ { name = "mkdocstrings-python", marker = "extra == 'dev'", specifier = ">=2.0.1" }, { name = "mypy", marker = "extra == 'dev'", specifier = ">=1.8" }, { name = "numpy", specifier = ">=1.26,<3" }, - { name = "oa-configurator", specifier = ">=1.3.0,<2" }, - { name = "omop-alchemy", specifier = ">=1.1,<2" }, - { name = "omop-emb", specifier = ">=2.1,<3" }, - { name = "omop-emb", extras = ["faiss-cpu"], marker = "extra == 'embedding-faiss'", specifier = ">=2.1,<3" }, - { name = "omop-emb", extras = ["pgvector"], marker = "extra == 'embedding-pgvector'", specifier = ">=2.1,<3" }, - { name = "omop-graph", specifier = ">=2.1,<3" }, - { name = "omop-llm", specifier = ">=1,<2" }, + { name = "oa-configurator", specifier = ">=2.0.0,<3" }, + { name = "omop-alchemy", specifier = ">=2.0.0,<3" }, + { name = "omop-emb", specifier = ">=3.0.0,<4" }, + { name = "omop-emb", extras = ["faiss-cpu"], marker = "extra == 'embedding-faiss'", specifier = ">=3.0.0,<4" }, + { name = "omop-emb", extras = ["pgvector"], marker = "extra == 'embedding-pgvector'", specifier = ">=3.0.0,<4" }, + { name = "omop-graph", specifier = ">=3.0.0,<4" }, + { name = "omop-llm", specifier = ">=1.0.2,<2" }, { name = "omop-semantics", specifier = ">=0.6,<1" }, { name = "openpyxl", marker = "extra == 'all-source'", specifier = ">=3.1,<4" }, { name = "openpyxl", marker = "extra == 'xlsx'", specifier = ">=3.1,<4" }, @@ -2494,7 +2494,7 @@ wheels = [ [[package]] name = "oa-configurator" -version = "1.3.0" +version = "2.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "click" }, @@ -2505,9 +2505,9 @@ dependencies = [ { name = "typer" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/ea/aa/71f787266a0a2baddf9c6452a43be901690acc5f5888de1d07c8f6adbf9b/oa_configurator-1.3.0.tar.gz", hash = "sha256:70bb6648ce94f9d09f870b02962f40a004d20aec0cfcad45b6a7fd5dcf0fe33a", size = 191240, upload-time = "2026-08-31T00:41:59.351Z" } +sdist = { url = "https://files.pythonhosted.org/packages/e9/62/64bf398b8fe381d138446199be69bc437ef93fdb70aab48cebf51467f6ea/oa_configurator-2.0.0.tar.gz", hash = "sha256:26406e90aeadd4c747646763b80f16025c4c6638afec687517788bc537d0d581", size = 278382, upload-time = "2026-10-10T22:48:42.275Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/d4/cd/8af5f354c87225732fadcf8213fddbccb6e6468868533e84a0be842e2a62/oa_configurator-1.3.0-py3-none-any.whl", hash = "sha256:9b92391c9033fbab78bd5a26932168dd184ca175f391de8099ce537cf71cf6ab", size = 67448, upload-time = "2026-08-31T00:41:58.074Z" }, + { url = "https://files.pythonhosted.org/packages/06/9b/99f0073d97de3545d48cf28c8f39b724d543baf83e8af8d0ba5c8bffbb01/oa_configurator-2.0.0-py3-none-any.whl", hash = "sha256:2186beb5567850ba5bfe4950c2820b909d3746c6fa233a6074a85396a343af35", size = 118027, upload-time = "2026-10-10T22:48:40.571Z" }, ] [[package]] @@ -2581,7 +2581,7 @@ wheels = [ [[package]] name = "omop-alchemy" -version = "1.1.0" +version = "2.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "oa-configurator" }, @@ -2591,15 +2591,16 @@ dependencies = [ { name = "rich" }, { name = "sqlalchemy" }, { name = "typer" }, + { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/10/b6/315c72cba081c4cb5fed6737155047667e9057b92307579200551953e248/omop_alchemy-1.1.0.tar.gz", hash = "sha256:d78905132ad0f8933d8d6fceccbac903f497d40a54634ec9193023b237bb5afb", size = 395165, upload-time = "2026-08-24T06:19:41.929Z" } +sdist = { url = "https://files.pythonhosted.org/packages/10/4b/a925c1db558070a4a807a38456a90dab33ab766e18cd3de890dae94295ca/omop_alchemy-2.0.0.tar.gz", hash = "sha256:403848b2c377ef6200119f4d5280a1b7e1abe0c42e8681fe16a74ee87fa82259", size = 734827, upload-time = "2026-10-10T23:39:29.99Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a8/2c/c51519b2c1ff3b71b1117ca5e829451cbbe0d77d404d3ff8b485bd3e989c/omop_alchemy-1.1.0-py3-none-any.whl", hash = "sha256:31b64099145a6183fb71d6d89ecd662a506b5e9f9ab74b0b117860e1085991eb", size = 233776, upload-time = "2026-08-24T06:19:40.288Z" }, + { url = "https://files.pythonhosted.org/packages/7a/d2/2c271ce7d09b978bf812b698baf9000f1bc56afa2d1915b58c9b079875db/omop_alchemy-2.0.0-py3-none-any.whl", hash = "sha256:17cdbf3d901f23da339b7e4d4598f22f178499733fa326884de6e0de40c78f02", size = 298023, upload-time = "2026-10-10T23:39:28.182Z" }, ] [[package]] name = "omop-emb" -version = "2.1.0" +version = "3.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "h5py" }, @@ -2614,9 +2615,9 @@ dependencies = [ { name = "typing-extensions" }, { name = "urllib3" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/3c/44/1b3022eb8b896def2d0978da4958fa270711ddeee736cd1266ef8dc980fe/omop_emb-2.1.0.tar.gz", hash = "sha256:5aaca2bd52859b7db141bc911873a4dead8fe3356b34db8b2eda4f08814ae511", size = 235460, upload-time = "2026-08-20T12:30:54.632Z" } +sdist = { url = "https://files.pythonhosted.org/packages/60/68/54b0a9538da687f38e995d16fb335d585cd7dbca423be5874fb3f07217ae/omop_emb-3.0.0.tar.gz", hash = "sha256:6ceabae683e9f36cc9250fb0a62a6074a588b5ff5594e8fd8b06c1dcec5a826e", size = 249683, upload-time = "2026-10-11T00:20:20.468Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/ad/9a/f38c4be9ab59d84c5d0a3acfa64de91049d50e8b0a911c4d107b280e2a0a/omop_emb-2.1.0-py3-none-any.whl", hash = "sha256:1f9f91f628fd96fc79ecbc33b0e7e0478daecee444368d4eb84603fd8c82e5b8", size = 95199, upload-time = "2026-08-20T12:30:53.295Z" }, + { url = "https://files.pythonhosted.org/packages/58/a6/e564d7d43b294c592a93f1da737a7e836cb50f592a9127e6b10ef4383471/omop_emb-3.0.0-py3-none-any.whl", hash = "sha256:0e78c7c0ef01a71d1a62498d9fab494aab81d7fa40aa1e712ec097530c5df730", size = 94080, upload-time = "2026-10-11T00:20:18.782Z" }, ] [package.optional-dependencies] @@ -2630,7 +2631,7 @@ pgvector = [ [[package]] name = "omop-graph" -version = "2.1.0" +version = "3.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "oa-configurator" }, @@ -2641,14 +2642,14 @@ dependencies = [ { name = "typer" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/f2/b3/9bd23eaa0c1075279fa37472ddb3b4b1fafb2dd985b85ea258a8521d2436/omop_graph-2.1.0.tar.gz", hash = "sha256:870feca44a91f93c840ea4a84ac44292e962288e91bcd7d0995f99847cad35fd", size = 460091, upload-time = "2026-08-24T08:35:42.495Z" } +sdist = { url = "https://files.pythonhosted.org/packages/2d/40/609efc228073c4d712a4f0328229fa1ffaef317ce180cd52189ff607090b/omop_graph-3.0.0.tar.gz", hash = "sha256:2184dab10323a541b400d42c94e603f33286b36b0d66a7797ce4f35b64d82104", size = 470552, upload-time = "2026-10-11T00:33:15.749Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/90/ef/e896ba1b2711aba66db0b297f72ae694e2cb60e057ccf29f3943808012a7/omop_graph-2.1.0-py3-none-any.whl", hash = "sha256:9de67d14c91fea99be629ed66306ef2e9436d3fb7b2d45fa2d2394d4768fd40b", size = 124996, upload-time = "2026-08-24T08:35:40.965Z" }, + { url = "https://files.pythonhosted.org/packages/ec/14/e86bfac20c51dc4f70596d5b69d1107093b96356543157d7db1a555ec262/omop_graph-3.0.0-py3-none-any.whl", hash = "sha256:7bf0c99f325ba0796c8b0ff2239e68657a08686dcdd0b34aeeda750bd4566e44", size = 126600, upload-time = "2026-10-11T00:33:13.968Z" }, ] [[package]] name = "omop-llm" -version = "1.0.1" +version = "1.0.2" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "any-llm-sdk", extra = ["gemini", "ollama"] }, @@ -2657,9 +2658,9 @@ dependencies = [ { name = "ollama" }, { name = "pydantic" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/f2/ae/dec515aac6273c562b2edd2bea0b996d40a65092213907e2eeb09098fc46/omop_llm-1.0.1.tar.gz", hash = "sha256:cebdb7df66759f516198477c97e882534d8700787b0bae001b43a8f8071be964", size = 183148, upload-time = "2026-08-12T12:23:46.817Z" } +sdist = { url = "https://files.pythonhosted.org/packages/84/92/2147d62967adb61e91a5474ca0278bb3905be394718aad9c4d6427ba0196/omop_llm-1.0.2.tar.gz", hash = "sha256:a80267b95b1f1ad76e41a2285030d68d0c13825216dc7363f6eb87647c83c599", size = 182158, upload-time = "2026-10-11T00:10:42.623Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/1e/14/fc6782952879b08d02bd64c5b7fd1114465d826196681fd69a69e8207db0/omop_llm-1.0.1-py3-none-any.whl", hash = "sha256:aecfa1578286c3fad8f61c33032bd63355f8079ee5915ad2941a87e87cd3796a", size = 25863, upload-time = "2026-08-12T12:23:45.81Z" }, + { url = "https://files.pythonhosted.org/packages/a3/b4/fe6d24752cf7c472a0473f7f484660bc57a7779d29ae1669d387ae76b668/omop_llm-1.0.2-py3-none-any.whl", hash = "sha256:7dbd72df14fccefcf1be09fbcea89b8870c944c91db3dd686bacdcb1bcb544cc", size = 25864, upload-time = "2026-10-11T00:10:41.434Z" }, ] [[package]] @@ -2741,7 +2742,7 @@ wheels = [ [[package]] name = "orm-loader" -version = "1.0.1" +version = "2.0.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "chardet" }, @@ -2750,9 +2751,9 @@ dependencies = [ { name = "pyarrow" }, { name = "sqlalchemy" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/f4/4c/fe51c02e90710fc63b2a6cd9f563f48c0cc8bf2aad6bfb66e806e3c13fcf/orm_loader-1.0.1.tar.gz", hash = "sha256:8ab3c3aa65cf07b02fb23505c04e5898ab339ceb155e8a697b8b0fab089a18b8", size = 140755, upload-time = "2026-08-16T11:09:09.903Z" } +sdist = { url = "https://files.pythonhosted.org/packages/6a/cd/b3a29d383b80e1c282a3ee12a28f00b5608802763dd98ed7b3adc637e630/orm_loader-2.0.0.tar.gz", hash = "sha256:73232ecb5f1474738e54f30fae89999620d57cc41456ae799669a74c690a76b3", size = 171586, upload-time = "2026-10-10T23:22:15.966Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/e8/cc/ab60acffb180da5736c087586d93c71596929428815d5d8ef39567c06593/orm_loader-1.0.1-py3-none-any.whl", hash = "sha256:ebe7eb4644749086eeabb0cf6d5c8961622369bf72181dc2a0a4458136e75507", size = 61010, upload-time = "2026-08-16T11:09:08.52Z" }, + { url = "https://files.pythonhosted.org/packages/1c/61/6687b9c1602deb1c364d4653c641698e3de298b9c4f5dc98c59698455ac7/orm_loader-2.0.0-py3-none-any.whl", hash = "sha256:a118ddc3fcb611d658b7be52f616b641804ae65067efd05753d1bc14a03b52c9", size = 74959, upload-time = "2026-10-10T23:22:14.645Z" }, ] [[package]]