Closed
Conversation
* CI pipeline * Publish action * Refactor type hinting for vocabulary_ids to use builtins.list for consistency * Import builtins conditionally in type checking for improved clarity * Codecov settings in CI * Update README.md to include Codecov badge and change License badge color * Update CHANGELOG for v0.2.0: add parameters to `concepts.get_by_code()` for synonyms and relationships, and update User-Agent header format. * Update CHANGELOG.md * Add website link to README.md * Readme updates * Documentation * Corrections * v1.3.0 preparation * Release preparation * Integration tests * Improved tests * Examples update * Downloads badge * Sponsorship * Add integration tests for standard concept filtering and multiple filters with pagination * Prepare release v1.3.1 * Refactor tests for API key validation and enhance request handling tests. Updated synchronous and asynchronous client tests to use monkeypatching for API key checks. Added new tests for handling raw requests, including error parsing, rate limits, and JSON decoding issues. * Update vocab_version in tests for consistency across mock responses and client configurations. * Extending mapping method with source_codes option * Add semantic and similar search functionality with corresponding types and integration tests - Introduced `semantic` and `similar` search methods in the search resource, allowing for advanced concept searches using neural embeddings and similarity algorithms. - Added new TypedDicts for `SemanticSearchResult`, `SemanticSearchMeta`, `SimilarConcept`, and `SimilarSearchResult` to structure the response data. - Implemented integration tests for semantic search, including filtering and pagination, as well as tests for finding similar concepts by ID and name. - Updated type imports and ensured compatibility with existing search functionality. * Add semantic search examples to README * Refactor README and integration tests to streamline result extraction - Updated README example to reflect changes in the results structure for semantic search. - Refactored integration tests to utilize a new `extract_data` function for consistent handling of results and similar concepts, improving code clarity and maintainability. * Update CI workflow to support multiple branches and manual triggering * Update minimum similarity threshold in search.py from 0.3 to 0.5 for improved filtering accuracy. * v1.4.0 release * Increase rate limit delay in integration tests from 1 second to 2 seconds for improved test reliability. * Update version handling * Prepare v1.4.1 release Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add retry logic for server errors in SyncHTTPClient and AsyncHTTPClientImpl - Implemented retry mechanism for handling server errors (502, 503, 504) in both synchronous and asynchronous HTTP clients. - Added exponential backoff delay for retries to improve resilience against temporary server issues. * Refactor retry condition formatting in SyncHTTPClient and AsyncHTTPClientImpl * Add bulk search functionality to the API - Introduced `bulk_basic` and `bulk_semantic` methods for executing multiple lexical and semantic searches in a single API call, respectively. - Updated the README to include examples for bulk search usage. - Added corresponding types for bulk search inputs and responses in the type definitions. - Implemented integration and unit tests to validate the new bulk search features. * Update type definitions to include bulk search and semantic search types - Added new types for bulk search and semantic search functionalities to the `__all__` exports in `__init__.py`. - Removed previously commented sections for clarity and organization. * Enhance integration tests for bulk search functionality - Updated assertions in `test_bulk_basic_search` to verify that all expected search IDs are present in the results. - Added a check in `test_bulk_semantic_search` to confirm that the SNOMED vocabulary filter is applied to the results. * Prep for v1.5.0 release * Examples update * Update GitHub Actions workflows to use latest action versions - Upgraded `actions/checkout` from v4 to v6 in both `ci.yml` and `publish.yml`. - Updated `codecov/codecov-action` from v4 to v5 in `ci.yml`. - Changed `actions/upload-artifact` and `actions/download-artifact` from v4 to v5 in `publish.yml`. * Add retry logic with exponential backoff and jitter for rate limits and server errors (#6) - Introduced a new `_calculate_retry_delay` function to handle retry delays based on the Retry-After header and exponential backoff with jitter. - Updated `SyncHTTPClient` and `AsyncHTTPClientImpl` to utilize the new retry delay calculation for handling rate limits (429) and server errors (502, 503, 504). - Enhanced retry mechanism to improve resilience against temporary issues. Co-authored-by: alex-omophub <sdk@omophub.com> * Update CHANGELOG for v1.5.1 release --------- Co-authored-by: alex-omophub <sdk@omophub.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 49 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/omophub/resources/concepts.py">
<violation number="1" location="src/omophub/resources/concepts.py:71">
P2: `GetConceptParams` is out of sync with the updated `get`/`get_by_code` signatures, so typing metadata no longer reflects accepted parameters.</violation>
<violation number="2" location="src/omophub/resources/concepts.py:241">
P2: `RelationshipsParams` still models the old API and now conflicts with the new `relationships` method arguments.</violation>
</file>
<file name="src/omophub/resources/vocabularies.py">
<violation number="1" location="src/omophub/resources/vocabularies.py:54">
P2: Removing `include_stats`/`include_domains` from `get()` is a breaking API change that causes runtime `TypeError` for existing keyword-argument callers.</violation>
<violation number="2" location="src/omophub/resources/vocabularies.py:91">
P2: `domains()` removed previously supported keyword args (`vocabulary_ids`, `page`, `page_size`), which breaks existing callers and drops filtering/pagination behavior.</violation>
<violation number="3" location="src/omophub/resources/vocabularies.py:112">
P1: `concepts()` removed previously supported kwargs (`domain_id`, `concept_class_id`, `standard_only`), creating a breaking runtime API change for existing integrations.</violation>
</file>
<file name="examples/map_between_vocabularies.py">
<violation number="1" location="examples/map_between_vocabularies.py:89">
P2: This lookup example no longer enforces standard mappings, so it can return non-standard targets despite the function being documented as finding a standard mapping.</violation>
</file>
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:57">
P2: The integration job condition does not restrict push branches, so it also runs on `master` pushes despite the comment saying main/develop only.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| domain_id: str | None = None, | ||
| concept_class_id: str | None = None, | ||
| standard_only: bool = False, | ||
| search: str | None = None, |
There was a problem hiding this comment.
P1: concepts() removed previously supported kwargs (domain_id, concept_class_id, standard_only), creating a breaking runtime API change for existing integrations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/omophub/resources/vocabularies.py, line 112:
<comment>`concepts()` removed previously supported kwargs (`domain_id`, `concept_class_id`, `standard_only`), creating a breaking runtime API change for existing integrations.</comment>
<file context>
@@ -89,58 +74,83 @@ def stats(self, vocabulary_id: str) -> VocabularyStats:
- domain_id: str | None = None,
- concept_class_id: str | None = None,
- standard_only: bool = False,
+ search: str | None = None,
+ standard_concept: str = "all",
+ include_invalid: bool = False,
</file context>
| *, | ||
| include_relationships: bool = False, | ||
| include_synonyms: bool = False, | ||
| include_hierarchy: bool = False, |
There was a problem hiding this comment.
P2: GetConceptParams is out of sync with the updated get/get_by_code signatures, so typing metadata no longer reflects accepted parameters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/omophub/resources/concepts.py, line 71:
<comment>`GetConceptParams` is out of sync with the updated `get`/`get_by_code` signatures, so typing metadata no longer reflects accepted parameters.</comment>
<file context>
@@ -71,13 +68,17 @@ def get(
*,
include_relationships: bool = False,
include_synonyms: bool = False,
+ include_hierarchy: bool = False,
+ vocab_release: str | None = None,
) -> Concept:
</file context>
| *, | ||
| relationship_type: str | None = None, | ||
| target_vocabulary: str | None = None, | ||
| relationship_ids: str | list[str] | None = None, |
There was a problem hiding this comment.
P2: RelationshipsParams still models the old API and now conflicts with the new relationships method arguments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/omophub/resources/concepts.py, line 241:
<comment>`RelationshipsParams` still models the old API and now conflicts with the new `relationships` method arguments.</comment>
<file context>
@@ -147,106 +174,162 @@ def suggest(
*,
- relationship_type: str | None = None,
- target_vocabulary: str | None = None,
+ relationship_ids: str | list[str] | None = None,
+ vocabulary_ids: str | list[str] | None = None,
+ domain_ids: str | list[str] | None = None,
</file context>
| f"/vocabularies/{vocabulary_id}/stats/domains/{domain_id}" | ||
| ) | ||
|
|
||
| def domains(self) -> dict[str, Any]: |
There was a problem hiding this comment.
P2: domains() removed previously supported keyword args (vocabulary_ids, page, page_size), which breaks existing callers and drops filtering/pagination behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/omophub/resources/vocabularies.py, line 91:
<comment>`domains()` removed previously supported keyword args (`vocabulary_ids`, `page`, `page_size`), which breaks existing callers and drops filtering/pagination behavior.</comment>
<file context>
@@ -89,58 +74,83 @@ def stats(self, vocabulary_id: str) -> VocabularyStats:
+ f"/vocabularies/{vocabulary_id}/stats/domains/{domain_id}"
+ )
+
+ def domains(self) -> dict[str, Any]:
+ """Get all standard OHDSI domains.
+
</file context>
| include_stats: bool = False, | ||
| include_domains: bool = False, | ||
| ) -> Vocabulary: | ||
| def get(self, vocabulary_id: str) -> Vocabulary: |
There was a problem hiding this comment.
P2: Removing include_stats/include_domains from get() is a breaking API change that causes runtime TypeError for existing keyword-argument callers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/omophub/resources/vocabularies.py, line 54:
<comment>Removing `include_stats`/`include_domains` from `get()` is a breaking API change that causes runtime `TypeError` for existing keyword-argument callers.</comment>
<file context>
@@ -51,32 +51,17 @@ def list(
- include_stats: bool = False,
- include_domains: bool = False,
- ) -> Vocabulary:
+ def get(self, vocabulary_id: str) -> Vocabulary:
"""Get vocabulary details.
</file context>
| concept.get("concept_id", 0), | ||
| standard_only=True, | ||
| ) | ||
| mappings = client.mappings.get(concept.get("concept_id", 0)) |
There was a problem hiding this comment.
P2: This lookup example no longer enforces standard mappings, so it can return non-standard targets despite the function being documented as finding a standard mapping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/map_between_vocabularies.py, line 89:
<comment>This lookup example no longer enforces standard mappings, so it can return non-standard targets despite the function being documented as finding a standard mapping.</comment>
<file context>
@@ -89,14 +84,11 @@ def lookup_by_code() -> None:
- concept.get("concept_id", 0),
- standard_only=True,
- )
+ mappings = client.mappings.get(concept.get("concept_id", 0))
- print("\n Standard mappings:")
</file context>
Suggested change
| mappings = client.mappings.get(concept.get("concept_id", 0)) | |
| mappings = client.mappings.get(concept.get("concept_id", 0), standard_only=True) |
|
|
||
| integration: | ||
| # Only run on push to main/develop, not on PRs (saves ~7-8 min per PR) | ||
| if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' |
There was a problem hiding this comment.
P2: The integration job condition does not restrict push branches, so it also runs on master pushes despite the comment saying main/develop only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 57:
<comment>The integration job condition does not restrict push branches, so it also runs on `master` pushes despite the comment saying main/develop only.</comment>
<file context>
@@ -0,0 +1,72 @@
+
+ integration:
+ # Only run on push to main/develop, not on PRs (saves ~7-8 min per PR)
+ if: github.event_name == 'push' || github.event_name == 'workflow_dispatch'
+ runs-on: ubuntu-latest
+ steps:
</file context>
Suggested change
| if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' | |
| if: github.event_name == 'workflow_dispatch' || (github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop')) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds a FHIR-to-OMOP resolver and advanced search (semantic, similar, bulk) to improve concept resolution and discovery. Also hardens HTTP reliability with retries and adds CI and PyPI publish workflows.
New Features
client.fhir.resolve(),resolve_batch(),resolve_codeable_concept()map FHIR codings to OMOP concepts and target CDM tables, with optional recommendations.semantic(),similar(),bulk_basic(), andbulk_semantic()for embedding-based and batch searches; async iterators supported.Migration
vocabulary_idsanddomain_ids(lists) andpage/page_sizeinstead ofvocabulary,domain, andlimit.relationship_type→relationship_ids(list);target_vocabulary→vocabulary_ids;page_sizedefaults to 100;include_reverseadded.target_vocabularyis singular; removed quality/context options; addinclude_invalidand optionalvocab_release.hierarchy.get(); other methods acceptvocabulary_ids(list) andinclude_invalid.vocabularies.stats(vocabulary_id)andvocabularies.domains(vocabulary_id)for details.include_stats.paginate_asyncnow requires an async callable (sync fallback removed).Written for commit 0d9844c. Summary will update on new commits.