Skip to content

Include vocabulary and domain ID in NearestConceptMatch#49

Open
nicoloesch wants to merge 2 commits into
mainfrom
44-search-metadata
Open

Include vocabulary and domain ID in NearestConceptMatch#49
nicoloesch wants to merge 2 commits into
mainfrom
44-search-metadata

Conversation

@nicoloesch

@nicoloesch nicoloesch commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

NearestConceptMatch now carries domain_id, vocabulary_id, and is_active alongside the existing is_standard, and all three embedding backends (pgvector, sqlite-vec, and the FAISS sidecar) populate them consistently. Previously only is_standard was returned by KNN queries, whereas domain_id/vocabulary_id weren't exposed at all. is_active was documented as CDM-enriched but actually already got populated during ingestion.

These four fields all come from the embedding table's own filter columns (written from the CDM at ingestion time), so they're now always populated regardless of whether an omop_cdm_engine is configured for the read interface. concept_name remains the only field that requires a CDM engine at query time to "save" some storage.

As part of this, ConceptEmbeddingRecord.is_valid no longer defaults to True. It's now required, matching is_standard, so every call site has to state a concept's validity explicitly instead of silently inheriting a default. This was already done in production code and only had to be adapted in test sites.

Checklist

  • Applied exactly one label (breaking, feature, fix, dependencies, or chore)
  • Tests pass locally (uv run pytest -q)
  • Lint passes (uv run ruff check .)

@nicoloesch
nicoloesch requested a review from gkennos July 22, 2026 00:58
@nicoloesch nicoloesch added the feature New backwards-compatible functionality. MINOR: x.y+1.z label Jul 22, 2026

@gkennos gkennos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add at least one test that asserts end to end that new fields are populated on returned matches

Comment thread tests/test_sqlitevec.py
Comment thread tests/test_faiss_cache.py
Comment thread tests/conftest.py
@nicoloesch
nicoloesch requested a review from gkennos July 23, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New backwards-compatible functionality. MINOR: x.y+1.z

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add 'domain' and 'vocabulary_id' fields into return values of embedding search

2 participants