feat: Integrate oehrpy SDK for type-safe EHRBase composition building#27
feat: Integrate oehrpy SDK for type-safe EHRBase composition building#27platzhersh merged 2 commits intomainfrom
Conversation
Replace manual FLAT JSON construction with oehrpy VitalSignsBuilder and wrap raw httpx EHRBase client with oehrpy's async EHRBaseClient. This eliminates brittle path strings, adds IDE autocomplete, and validates compositions before submission to EHRBase. Changes: - Add oehrpy>=0.1.0 dependency to pyproject.toml - Refactor ehrbase/client.py to wrap oehrpy EHRBaseClient - Create openehr/ integration layer with build_vital_signs_flat helper - Replace _build_flat_composition with VitalSignsBuilder in observations service - Update ehrbase/templates.py to handle oehrpy EHRBaseError - Remove dead update_composition from ehrbase/compositions.py - Add ehrbase_timeout and ehrbase_verify_ssl config options - Add 15 tests for oehrpy builder, composition helper, and client wrapper - Add PRD-0005 documenting the integration plan https://claude.ai/code/session_01LPyj4pivdyzSZgcYuoQbb8
platzhersh
changed the title
WalkthroughIntegrates the oehrpy SDK into the backend: adds oehrpy dependency, new openehr helper module for building vital-signs FLAT compositions, replaces direct HTTPX-based EHRBase calls with an oehrpy-backed EHRBaseClient wrapper, updates observation service to use the builder, and adds tests and PRD doc. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant ObsSvc as ObservationService
participant Wrapper as EHRBaseClient (wrapper)
participant SDK as oehrpy EHRBaseClient
participant EHR as EHRBase Server
App->>ObsSvc: create vital signs (data)
ObsSvc->>Wrapper: build payload (build_vital_signs_flat) + create_composition(...)
Wrapper->>SDK: _ensure_connected() -> connect (if needed)
SDK-->>Wrapper: connected client
Wrapper->>SDK: create_composition(ehr_id, template_id, composition, format)
SDK->>EHR: HTTP request (server)
EHR-->>SDK: HTTP response / result
SDK-->>Wrapper: result (composition id / metadata)
Wrapper-->>ObsSvc: structured dict (ids, metadata)
ObsSvc-->>App: return composition result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Fix import sorting and line length violations. https://claude.ai/code/session_01LPyj4pivdyzSZgcYuoQbb8
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@api/src/config.py`:
- Around line 19-20: _update: The _ensure_connected() method currently creates
OehrpyClient without using the new config options; modify the OehrpyClient
instantiation inside _ensure_connected() to pass
timeout=settings.ehrbase_timeout and verify_ssl=settings.ehrbase_verify_ssl
along with base_url, username, and password so the client honors ehrbase_timeout
and ehrbase_verify_ssl from settings when constructed.
In `@api/src/ehrbase/client.py`:
- Around line 22-30: In _ensure_connected(), pass the EHRBase configuration
values from settings into the OehrpyClient constructor (include the timeout and
SSL verification flags—use the OehrpyClient API parameter names, e.g.,
timeout=<settings.ehrbase_timeout> and verify_ssl or
verify=<settings.ehrbase_verify_ssl> as required) so the client is constructed
with those options; also avoid calling connect() on every invocation by only
calling await self._client.connect() when you create the client (i.e., inside
the if self._client is None block) rather than unconditionally, keeping the rest
of the method returning self._client.
In `@api/src/openehr/compositions.py`:
- Around line 3-6: The import block at the top (symbols datetime, Any,
VitalSignsBuilder) is unsorted per CI; run ruff to auto-fix or manually reorder
imports to satisfy isort: ensure standard-library imports (from datetime import
datetime, from typing import Any) come first, then a blank line, then
third-party imports (from openehr_sdk.templates import VitalSignsBuilder), and
re-run ruff check --fix api/src/openehr/compositions.py to validate.
In `@api/tests/test_oehrpy_integration.py`:
- Around line 3-9: The import block in test_oehrpy_integration.py is unsorted;
reorder the imports so third‑party packages come first (e.g., pytest,
openehr_sdk imports like EHRBaseClient as OehrpyClient and VitalSignsBuilder)
followed by local project imports (src.ehrbase.client EHRBaseClient and
src.openehr.compositions build_vital_signs_flat, VITAL_SIGNS_TEMPLATE_ID), or
simply run isort on the file to apply the project’s import ordering rules.
- Around line 94-104: Both async test functions test_client_creation and
test_client_close_when_not_connected are missing return type hints; update their
definitions to include the explicit return type `-> None` (e.g., async def
test_client_creation(...) -> None:) to comply with the project's type hinting
rules, leaving the bodies and assertions unchanged and keeping references to
EHRBaseClient and its _client attribute as-is.
- Around line 107-120: Convert the synchronous, untyped test methods in
TestOehrpyClientDirect to async functions with full type hints for parameters
(e.g., async def test_client_creation(self) -> None becomes async def
test_client_creation(self) -> None and similarly include parameter types where
applicable), and do the same for test_client_with_auth; ensure OehrpyClient
instantiation remains but awaits any async setup if required. Also update the
other classes mentioned (TestVitalSignsBuilder and TestBuildVitalSigns) to
follow the same async-only/type-hint policy: make their test methods async, add
explicit parameter type annotations and return type None, and adjust any
fixtures/usage to await async calls so the tests remain correct.
- Around line 67-89: The test methods in class TestBuildVitalSigns
(test_build_with_bp_and_pulse, test_build_bp_only, test_build_pulse_only,
test_template_id_matches) lack return type hints; update each method signature
to include the synchronous return annotation "-> None" (e.g., def
test_build_with_bp_and_pulse(self) -> None:) so they comply with the api/**/*.py
policy while keeping the tests synchronous; no other behavioral changes are
needed.
🧹 Nitpick comments (3)
api/src/openehr/compositions.py (1)
31-36: Consider handling partial blood pressure data explicitly.If only
systolicor onlydiastolicis provided, the blood pressure entry is silently omitted. This may be intentional, but callers might expect validation or a warning when partial data is supplied.💡 Optional: Add explicit handling for partial data
+ # Validate blood pressure requires both values + if (systolic is None) != (diastolic is None): + import logging + logging.warning("Partial blood pressure data provided; both systolic and diastolic required") + if systolic is not None and diastolic is not None: builder.add_blood_pressure(docs/prd/0005-oehrpy-sdk-integration.md (1)
127-170: Consider adding language hint for fenced code blocks.The ASCII diagram code blocks at lines 127 and 190 lack language specifiers, which triggers markdownlint MD040. Adding
textorplaintextwould resolve this.📝 Optional fix
-``` +```text ┌─────────────────────────────────────────────────────────────────┐api/src/observations/service.py (1)
51-57: Consider exposing composer_name parameter.The
build_vital_signs_flatfunction accepts acomposer_nameparameter (defaulting to"CIS System"), but the service always uses the default. If the caller's identity should be recorded in the composition, consider threading it through fromVitalSignsCreateor authentication context.
| ehrbase_timeout: float = 30.0 | ||
| ehrbase_verify_ssl: bool = True |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "ehrbase_timeout|ehrbase_verify_ssl" --type=py -B 2 -A 2Repository: platzhersh/open-cis
Length of output: 353
🏁 Script executed:
rg -n "EHRBaseClient" --type=py -B 3 -A 5Repository: platzhersh/open-cis
Length of output: 4589
🏁 Script executed:
cat -n api/src/config.pyRepository: platzhersh/open-cis
Length of output: 963
🏁 Script executed:
cat -n api/src/ehrbase/client.pyRepository: platzhersh/open-cis
Length of output: 6448
Pass ehrbase_timeout and ehrbase_verify_ssl to the OehrpyClient initialization.
The new config settings are defined but never consumed. Update the _ensure_connected() method in api/src/ehrbase/client.py (lines 24–28) to pass timeout and verify_ssl parameters when instantiating OehrpyClient:
self._client = OehrpyClient(
base_url=settings.ehrbase_url,
username=settings.ehrbase_user,
password=settings.ehrbase_password,
timeout=settings.ehrbase_timeout,
verify_ssl=settings.ehrbase_verify_ssl,
)🤖 Prompt for AI Agents
In `@api/src/config.py` around lines 19 - 20, _update: The _ensure_connected()
method currently creates OehrpyClient without using the new config options;
modify the OehrpyClient instantiation inside _ensure_connected() to pass
timeout=settings.ehrbase_timeout and verify_ssl=settings.ehrbase_verify_ssl
along with base_url, username, and password so the client honors ehrbase_timeout
and ehrbase_verify_ssl from settings when constructed.
| async def _ensure_connected(self) -> OehrpyClient: | ||
| if self._client is None: | ||
| auth = None | ||
| if settings.ehrbase_user and settings.ehrbase_password: | ||
| auth = httpx.BasicAuth(settings.ehrbase_user, settings.ehrbase_password) | ||
|
|
||
| self._client = httpx.AsyncClient( | ||
| base_url=self.base_url, | ||
| auth=auth, | ||
| headers={ | ||
| "Content-Type": "application/json", | ||
| "Accept": "application/json", | ||
| }, | ||
| self._client = OehrpyClient( | ||
| base_url=settings.ehrbase_url, | ||
| username=settings.ehrbase_user, | ||
| password=settings.ehrbase_password, | ||
| ) | ||
| await self._client.connect() | ||
| return self._client |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the ehrbase client file
find . -type f -name "client.py" | grep -E "api|ehrbase"Repository: platzhersh/open-cis
Length of output: 113
🏁 Script executed:
# Examine the settings to see if timeout/verify_ssl are defined
find . -type f -name "settings.py" | head -10Repository: platzhersh/open-cis
Length of output: 45
🏁 Script executed:
# Check imports in the ehrbase client file
head -40 api/src/ehrbase/client.py 2>/dev/null || find . -path "*/api/*/ehrbase/client.py" -exec head -40 {} \;Repository: platzhersh/open-cis
Length of output: 1405
🌐 Web query:
oehrpy OehrpyClient constructor parameters timeout verify_ssl httpx AsyncClient
💡 Result:
I couldn’t find any public docs/source for oehrpy or OehrpyClient specifically (it may be private, renamed, or misspelled), so I can’t quote its exact constructor signature.
That said, many OpenAPI-generated Python clients expose the same constructor kwargs and use them to build an internal httpx.Client / httpx.AsyncClient. In those clients, the constructor typically accepts:
base_url: strcookies: dict[str, str](default cookies sent on every request)headers: dict[str, str](default headers sent on every request)timeout: httpx.Timeout | None(maximum request time;httpx.TimeoutExceptionif exceeded) [1]verify_ssl: bool | str | ssl.SSLContext(whether to verify TLS certs; maps to httpxverify=) [1]follow_redirects: bool(defaultFalse) [1]httpx_args: dict[str, Any](extra kwargs forwarded intohttpx.Client(...)andhttpx.AsyncClient(...)) [1]
And httpx.AsyncClient(...) itself supports verify=... and timeout=... among many other kwargs that can be passed via httpx_args [2].
If you paste the OehrpyClient class (or link to its repo / PyPI page), I can confirm the exact parameter names and defaults for your version.
Sources: [1] [2]
🏁 Script executed:
# Find and examine settings/config file
find . -type f -name "settings.py" -o -name "config.py" | head -10Repository: platzhersh/open-cis
Length of output: 82
🏁 Script executed:
# Search for ehrbase_timeout and ehrbase_verify_ssl in the codebase
rg "ehrbase_timeout|ehrbase_verify_ssl" --type pyRepository: platzhersh/open-cis
Length of output: 168
🏁 Script executed:
# Check for requirements, pyproject.toml, or setup files for openehr_sdk version
find . -type f \( -name "requirements*.txt" -o -name "pyproject.toml" -o -name "setup.py" -o -name "setup.cfg" -o -name "poetry.lock" \) | head -10Repository: platzhersh/open-cis
Length of output: 83
🌐 Web query:
openehr_sdk EHRBaseClient constructor parameters timeout verify
💡 Result:
I couldn’t find any EHRBaseClient class/constructor in the ehrbase/openEHR_SDK project (the Java SDK). The SDK’s REST client entry point is DefaultRestClient (see the project’s own usage hint pointing to client/src/test/.../defaultrestclient for examples), not EHRBaseClient. (discourse.openehr.org)
So there are no EHRBaseClient(..., timeout=..., verify=...) constructor parameters to document for ehrbase/openEHR_SDK.
If your goal is to control timeouts / TLS verification with EHRbase:
- Server-side timeouts (e.g., long AQL queries / uploads) are configured in EHRbase via Spring Boot properties (e.g., Hikari connection timeout, Tomcat connection timeout). (discourse.openehr.org)
- Client-side TLS “verify” in Java is typically controlled by the JVM truststore (not a per-constructor
verifyflag).
If you meant a different library actually named openehr_sdk (Python or otherwise), paste:
- the package URL (PyPI/GitHub), and
- the exact import + constructor call you’re using,
and I’ll map the correcttimeout/verifyparameters for that client.
Citations:
- 1: https://discourse.openehr.org/t/openehr-sdk-dependency-problem/6041?utm_source=openai
- 2: https://discourse.openehr.org/t/how-to-change-default-timeout-in-ehrbase/5501?utm_source=openai
🏁 Script executed:
# Read the config file to see all settings
cat -n api/src/config.pyRepository: platzhersh/open-cis
Length of output: 963
🏁 Script executed:
# Check pyproject.toml for openehr_sdk version
cat -n api/pyproject.toml | head -50Repository: platzhersh/open-cis
Length of output: 1476
🌐 Web query:
openehr_sdk python package PyPI EHRBaseClient
💡 Result:
- There doesn’t appear to be a Python PyPI package named
openehr_sdk(or similar) that provides anEHRBaseClient/EHRBaseClientclass. The “openEHR SDK” from EHRbase is a Java SDK/client library (repo:ehrbase/openEHR_SDK). [1][2] - EHRbase itself is accessed primarily via the official openEHR REST API, and EHRbase publishes an OpenAPI specification you can use to generate a Python client (or you can call it directly with
requests). [3][4]
Sources:
[1] https://github.com/ehrbase/openEHR_SDK
[2] https://www.ehrbase.org/
[3] https://docs.ehrbase.org/docs/EHRbase/Explore/openEHR-REST
[4] https://docs.ehrbase.org/api/hip-ehrbase/admin
🌐 Web query:
oehrpy PyPI package EHRBaseClient
💡 Result:
I couldn’t find any PyPI package named oehrpy (or a Python class named EHRBaseClient associated with it) via public indexing/search.
What does exist in the EHRbase ecosystem is an “EHRbase Client Library” referenced in the official EHRbase docs—but it’s a Java client/code-generator workflow (generate Java classes from an OPT), not a Python package. [1]
If you’re trying to talk to an EHRbase server from Python, the supported/standard approach is to call the official openEHR REST API endpoints that EHRbase implements. EHRbase documents that it implements openEHR REST API v1.0.2, and its API docs point you to the openEHR REST spec for endpoint details. [2] [3]
Sources:
[1] (docs.ehrbase.org)
[2] (docs.ehrbase.org)
[3] (docs.ehrbase.org)
Citations:
- 1: https://docs.ehrbase.org/docs/EHRbase/openEHR-Introduction/Load-Data?utm_source=openai
- 2: https://docs.ehrbase.org/docs/EHRbase/Explore/openEHR-REST?utm_source=openai
- 3: https://docs.ehrbase.org/api/hip-ehrbase/openehr?utm_source=openai
🏁 Script executed:
# Search for any references to oehrpy in the codebase and verify import path
rg "oehrpy|openehr_sdk" --type py -B2 -A2Repository: platzhersh/open-cis
Length of output: 4196
🏁 Script executed:
# Check if there are any type stubs or additional config for oehrpy
find . -name "*.pyi" -o -name "py.typed" | head -20Repository: platzhersh/open-cis
Length of output: 45
🏁 Script executed:
# Search for any local openehr_sdk or oehrpy package definition
find . -type f -name "__init__.py" | xargs grep -l "EHRBaseClient\|openehr_sdk\|oehrpy" 2>/dev/null | head -20Repository: platzhersh/open-cis
Length of output: 92
🏁 Script executed:
# Check for git submodules
cat .gitmodules 2>/dev/null || echo "No .gitmodules file"Repository: platzhersh/open-cis
Length of output: 82
🏁 Script executed:
# Look for any package directories that might define openehr_sdk
find . -type d -name "openehr*" -o -type d -name "*oehr*" 2>/dev/null | head -20Repository: platzhersh/open-cis
Length of output: 109
🏁 Script executed:
# Check full client.py file to see all methods and how _ensure_connected is used
cat -n api/src/ehrbase/client.pyRepository: platzhersh/open-cis
Length of output: 6448
Pass ehrbase_timeout and ehrbase_verify_ssl from settings to OehrpyClient constructor.
Settings define ehrbase_timeout and ehrbase_verify_ssl (PR objectives), but they are not wired into the OehrpyClient initialization. Update the constructor call to include these configuration parameters. Confirm the exact parameter names match the OehrpyClient API (e.g., timeout, verify_ssl/verify, or similar).
Also verify whether connect() must be called on every _ensure_connected() invocation or if it can be deferred to the initial connection setup.
🤖 Prompt for AI Agents
In `@api/src/ehrbase/client.py` around lines 22 - 30, In _ensure_connected(), pass
the EHRBase configuration values from settings into the OehrpyClient constructor
(include the timeout and SSL verification flags—use the OehrpyClient API
parameter names, e.g., timeout=<settings.ehrbase_timeout> and verify_ssl or
verify=<settings.ehrbase_verify_ssl> as required) so the client is constructed
with those options; also avoid calling connect() on every invocation by only
calling await self._client.connect() when you create the client (i.e., inside
the if self._client is None block) rather than unconditionally, keeping the rest
of the method returning self._client.
| from datetime import datetime | ||
| from typing import Any | ||
|
|
||
| from openehr_sdk.templates import VitalSignsBuilder |
There was a problem hiding this comment.
Fix import sorting to resolve pipeline failure.
The CI pipeline reports that the import block is un-sorted.
🔧 Proposed fix
-from datetime import datetime
-from typing import Any
-
-from openehr_sdk.templates import VitalSignsBuilder
+from datetime import datetime
+from typing import Any
+
+from openehr_sdk.templates import VitalSignsBuilderRun the following to auto-fix:
ruff check --fix api/src/openehr/compositions.py🧰 Tools
🪛 GitHub Actions: Backend CI
[warning] 3-3: Import block is un-sorted or un-formatted. Organize imports.
🤖 Prompt for AI Agents
In `@api/src/openehr/compositions.py` around lines 3 - 6, The import block at the
top (symbols datetime, Any, VitalSignsBuilder) is unsorted per CI; run ruff to
auto-fix or manually reorder imports to satisfy isort: ensure standard-library
imports (from datetime import datetime, from typing import Any) come first, then
a blank line, then third-party imports (from openehr_sdk.templates import
VitalSignsBuilder), and re-run ruff check --fix api/src/openehr/compositions.py
to validate.
| class TestBuildVitalSigns: | ||
| """Test the Open CIS composition helper.""" | ||
|
|
||
| def test_build_with_bp_and_pulse(self): | ||
| flat = build_vital_signs_flat( | ||
| systolic=120, | ||
| diastolic=80, | ||
| pulse_rate=72, | ||
| ) | ||
| assert isinstance(flat, dict) | ||
| assert len(flat) > 0 | ||
|
|
||
| def test_build_bp_only(self): | ||
| flat = build_vital_signs_flat(systolic=130, diastolic=85) | ||
| assert isinstance(flat, dict) | ||
|
|
||
| def test_build_pulse_only(self): | ||
| flat = build_vital_signs_flat(pulse_rate=68) | ||
| assert isinstance(flat, dict) | ||
|
|
||
| def test_template_id_matches(self): | ||
| assert VITAL_SIGNS_TEMPLATE_ID == "IDCR - Vital Signs Encounter.v1" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "test_oehrpy_integration.py" -o -name "*oehrpy*" | head -20Repository: platzhersh/open-cis
Length of output: 143
🏁 Script executed:
cat -n ./api/tests/test_oehrpy_integration.py | sed -n '60,95p'Repository: platzhersh/open-cis
Length of output: 1386
🏁 Script executed:
cat -n ./api/tests/test_oehrpy_integration.py | head -40Repository: platzhersh/open-cis
Length of output: 1782
🏁 Script executed:
find . -path "*/src/openehr/compositions.py" -o -path "*/src/openehr/compositions/*"Repository: platzhersh/open-cis
Length of output: 96
🏁 Script executed:
cat -n ./api/src/openehr/compositions.py | grep -A 20 "def build_vital_signs_flat"Repository: platzhersh/open-cis
Length of output: 863
🏁 Script executed:
cat -n ./api/tests/test_oehrpy_integration.py | sed -n '1,100p' | grep -E "^\s*(12|15|23|33|70|79|83|87|94).*def"Repository: platzhersh/open-cis
Length of output: 403
🏁 Script executed:
cat -n ./api/tests/test_oehrpy_integration.py | sed -n '87,110p'Repository: platzhersh/open-cis
Length of output: 908
🏁 Script executed:
cat -n ./api/tests/test_oehrpy_integration.py | sed -n '107,150p'Repository: platzhersh/open-cis
Length of output: 610
🏁 Script executed:
wc -l ./api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 105
Add return type hints to test methods.
Test methods are missing return type hints (-> None), which violates the api/**/*.py policy. However, these tests should remain synchronous since they test a synchronous helper function.
🛠️ Suggested update for this class
class TestBuildVitalSigns:
"""Test the Open CIS composition helper."""
- def test_build_with_bp_and_pulse(self):
+ def test_build_with_bp_and_pulse(self) -> None:
flat = build_vital_signs_flat(
systolic=120,
diastolic=80,
pulse_rate=72,
)
assert isinstance(flat, dict)
assert len(flat) > 0
- def test_build_bp_only(self):
+ def test_build_bp_only(self) -> None:
flat = build_vital_signs_flat(systolic=130, diastolic=85)
assert isinstance(flat, dict)
- def test_build_pulse_only(self):
+ def test_build_pulse_only(self) -> None:
flat = build_vital_signs_flat(pulse_rate=68)
assert isinstance(flat, dict)
- def test_template_id_matches(self):
+ def test_template_id_matches(self) -> None:
assert VITAL_SIGNS_TEMPLATE_ID == "IDCR - Vital Signs Encounter.v1"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class TestBuildVitalSigns: | |
| """Test the Open CIS composition helper.""" | |
| def test_build_with_bp_and_pulse(self): | |
| flat = build_vital_signs_flat( | |
| systolic=120, | |
| diastolic=80, | |
| pulse_rate=72, | |
| ) | |
| assert isinstance(flat, dict) | |
| assert len(flat) > 0 | |
| def test_build_bp_only(self): | |
| flat = build_vital_signs_flat(systolic=130, diastolic=85) | |
| assert isinstance(flat, dict) | |
| def test_build_pulse_only(self): | |
| flat = build_vital_signs_flat(pulse_rate=68) | |
| assert isinstance(flat, dict) | |
| def test_template_id_matches(self): | |
| assert VITAL_SIGNS_TEMPLATE_ID == "IDCR - Vital Signs Encounter.v1" | |
| class TestBuildVitalSigns: | |
| """Test the Open CIS composition helper.""" | |
| def test_build_with_bp_and_pulse(self) -> None: | |
| flat = build_vital_signs_flat( | |
| systolic=120, | |
| diastolic=80, | |
| pulse_rate=72, | |
| ) | |
| assert isinstance(flat, dict) | |
| assert len(flat) > 0 | |
| def test_build_bp_only(self) -> None: | |
| flat = build_vital_signs_flat(systolic=130, diastolic=85) | |
| assert isinstance(flat, dict) | |
| def test_build_pulse_only(self) -> None: | |
| flat = build_vital_signs_flat(pulse_rate=68) | |
| assert isinstance(flat, dict) | |
| def test_template_id_matches(self) -> None: | |
| assert VITAL_SIGNS_TEMPLATE_ID == "IDCR - Vital Signs Encounter.v1" |
🤖 Prompt for AI Agents
In `@api/tests/test_oehrpy_integration.py` around lines 67 - 89, The test methods
in class TestBuildVitalSigns (test_build_with_bp_and_pulse, test_build_bp_only,
test_build_pulse_only, test_template_id_matches) lack return type hints; update
each method signature to include the synchronous return annotation "-> None"
(e.g., def test_build_with_bp_and_pulse(self) -> None:) so they comply with the
api/**/*.py policy while keeping the tests synchronous; no other behavioral
changes are needed.
| @pytest.mark.asyncio | ||
| async def test_client_creation(self): | ||
| client = EHRBaseClient() | ||
| assert client.base_url is not None | ||
| assert client._client is None | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_client_close_when_not_connected(self): | ||
| client = EHRBaseClient() | ||
| await client.close() | ||
| assert client._client is None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file to understand its context
wc -l api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 103
🏁 Script executed:
# Check if the file exists and explore its structure
head -110 api/tests/test_oehrpy_integration.py | tail -30Repository: platzhersh/open-cis
Length of output: 908
🏁 Script executed:
# Look at the full context around those lines to understand the test class
sed -n '85,110p' api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 779
🏁 Script executed:
# Check other test files in the api/tests directory to see if they follow type hint patterns
fd --type f "test_.*\.py" api/tests/ | head -5Repository: platzhersh/open-cis
Length of output: 152
🏁 Script executed:
# Check if there are any other async test methods with type hints in the test files
rg "async def test.*\) ->" api/tests/ --context 2Repository: platzhersh/open-cis
Length of output: 45
🏁 Script executed:
# Check for type hints in all test methods in the test file
grep -n "def test_" api/tests/test_oehrpy_integration.py | head -15Repository: platzhersh/open-cis
Length of output: 733
🏁 Script executed:
# Look at all methods in the test file to see if ANY have type hints
grep -n "def.*->.*:" api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 45
🏁 Script executed:
# Check production code to see if type hints are consistently used
rg "async def" api/ --type py --glob "!test*" -A 1 | head -30Repository: platzhersh/open-cis
Length of output: 1493
🏁 Script executed:
# Check if there's pytest configuration or guidelines that might exempt test files
cat pytest.ini 2>/dev/null || cat pyproject.toml 2>/dev/null | grep -A 10 -B 10 "pytest"Repository: platzhersh/open-cis
Length of output: 45
🏁 Script executed:
# Look at the beginning of the test file to understand its imports and structure
head -30 api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 1165
Add return type hints to async tests.
Type hints are required everywhere in api/**/*.py per coding guidelines. Both test methods are missing -> None return type annotations.
🛠️ Suggested update
- async def test_client_creation(self):
+ async def test_client_creation(self) -> None:
client = EHRBaseClient()
assert client.base_url is not None
assert client._client is None
@@
- async def test_client_close_when_not_connected(self):
+ async def test_client_close_when_not_connected(self) -> None:
client = EHRBaseClient()
await client.close()
assert client._client is None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.mark.asyncio | |
| async def test_client_creation(self): | |
| client = EHRBaseClient() | |
| assert client.base_url is not None | |
| assert client._client is None | |
| @pytest.mark.asyncio | |
| async def test_client_close_when_not_connected(self): | |
| client = EHRBaseClient() | |
| await client.close() | |
| assert client._client is None | |
| `@pytest.mark.asyncio` | |
| async def test_client_creation(self) -> None: | |
| client = EHRBaseClient() | |
| assert client.base_url is not None | |
| assert client._client is None | |
| `@pytest.mark.asyncio` | |
| async def test_client_close_when_not_connected(self) -> None: | |
| client = EHRBaseClient() | |
| await client.close() | |
| assert client._client is None |
🤖 Prompt for AI Agents
In `@api/tests/test_oehrpy_integration.py` around lines 94 - 104, Both async test
functions test_client_creation and test_client_close_when_not_connected are
missing return type hints; update their definitions to include the explicit
return type `-> None` (e.g., async def test_client_creation(...) -> None:) to
comply with the project's type hinting rules, leaving the bodies and assertions
unchanged and keeping references to EHRBaseClient and its _client attribute
as-is.
| class TestOehrpyClientDirect: | ||
| """Test oehrpy client can be instantiated.""" | ||
|
|
||
| def test_client_creation(self): | ||
| client = OehrpyClient(base_url="http://localhost:8080/ehrbase") | ||
| assert client is not None | ||
|
|
||
| def test_client_with_auth(self): | ||
| client = OehrpyClient( | ||
| base_url="http://localhost:8080/ehrbase", | ||
| username="admin", | ||
| password="admin", | ||
| ) | ||
| assert client is not None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -path "*api/tests/test_oehrpy_integration.py" -type fRepository: platzhersh/open-cis
Length of output: 101
🏁 Script executed:
fd "test_oehrpy_integration.py" -t fRepository: platzhersh/open-cis
Length of output: 99
🏁 Script executed:
git ls-files | grep -i "test_oehrpy"Repository: platzhersh/open-cis
Length of output: 99
🏁 Script executed:
wc -l api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 103
🏁 Script executed:
sed -n '100,130p' api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 697
🏁 Script executed:
cat -n api/tests/test_oehrpy_integration.pyRepository: platzhersh/open-cis
Length of output: 4881
Make direct SDK tests async + typed.
These tests are sync and untyped, which violates the async-only/type-hint policy for api/**/*.py. Note: TestVitalSignsBuilder and TestBuildVitalSigns in the same file have the same issue and should also be updated. Additionally, type hints must include parameter types, not just return types—the suggested fix should be:
🛠️ Complete update for this class
- class TestOehrpyClientDirect:
+ `@pytest.mark.asyncio`
+ class TestOehrpyClientDirect:
"""Test oehrpy client can be instantiated."""
- def test_client_creation(self):
+ async def test_client_creation(self) -> None:
client = OehrpyClient(base_url="http://localhost:8080/ehrbase")
assert client is not None
- def test_client_with_auth(self):
+ async def test_client_with_auth(self) -> None:
client = OehrpyClient(
base_url="http://localhost:8080/ehrbase",
username="admin",
password="admin",
)
assert client is not None🤖 Prompt for AI Agents
In `@api/tests/test_oehrpy_integration.py` around lines 107 - 120, Convert the
synchronous, untyped test methods in TestOehrpyClientDirect to async functions
with full type hints for parameters (e.g., async def test_client_creation(self)
-> None becomes async def test_client_creation(self) -> None and similarly
include parameter types where applicable), and do the same for
test_client_with_auth; ensure OehrpyClient instantiation remains but awaits any
async setup if required. Also update the other classes mentioned
(TestVitalSignsBuilder and TestBuildVitalSigns) to follow the same
async-only/type-hint policy: make their test methods async, add explicit
parameter type annotations and return type None, and adjust any fixtures/usage
to await async calls so the tests remain correct.
2 participants
Summary
Replaces raw HTTP calls and manual JSON construction with the oehrpy v0.1.0 Python openEHR SDK, providing type-safe composition building, validation, and improved developer experience for EHRBase integration.
Key Changes
>=0.1.0) topyproject.tomlfor type-safe openEHR RM models and EHRBase clientsrc/ehrbase/client.py):httpx.AsyncClientwithopenehr_sdk.client.EHRBaseClient_ensure_connected()lifecycle managementsrc/observations/service.py:_build_flat_composition()method with fragile path stringsVitalSignsBuilderfrom oehrpy for type-safe vital signs compositionEHRBaseErrorexceptionssrc/openehr/compositions.py):build_vital_signs_flat()helper usingVitalSignsBuildersrc/config.py):ehrbase_timeoutandehrbase_verify_sslsettings for oehrpy clientsrc/ehrbase/templates.py):EHRBaseErrorexception handling for template conflictstests/test_oehrpy_integration.py):VitalSignsBuilderoutput validationdocs/prd/0005-oehrpy-sdk-integration.md):Implementation Details
Type Safety & Validation
"vital_signs_observations/vital_signs/blood_pressure/systolic|magnitude")Backward Compatibility
Performance
Error Handling
httpx.HTTPStatusErrorwithEHRBaseErrorfor cleaner exception handlingTesting
All existing tests pass. New tests validate:
Related Issues
Addresses pain points documented in
docs/FLAT_FORMAT_VERSIONS.mdregarding manual JSON construction errors and lack of validation.https://claude.ai/code/session_01LPyj4pivdyzSZgcYuoQbb8
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.