docs: Add ADR-0006 and PRD-0006 for FHIR terminology server integration#37
docs: Add ADR-0006 and PRD-0006 for FHIR terminology server integration#37platzhersh wants to merge 5 commits intomainfrom
Conversation
Add architecture decision record selecting Snowstorm Lite as the FHIR terminology server, and product requirements document detailing phased integration plan for terminology services in Open CIS. https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
WalkthroughAdds FHIR Terminology Server integration: ADR & PRD docs, Docker Compose Snowstorm Lite service, env entries, an async Python TerminologyClient with Pydantic schemas, FastAPI terminology router endpoints, config settings, and setup wiring including readiness checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend
participant API as OpenCIS API
participant Terminology as Snowstorm Lite (FHIR)
Frontend->>API: GET /api/terminology/lookup?system=...&code=...
API->>Terminology: GET /CodeSystem/$lookup or /CodeSystem/$validate-code
Terminology-->>API: FHIR lookup/validate response
API-->>Frontend: CodeLookupResult / ValidationResult
Frontend->>API: GET /api/terminology/expand?url=...
API->>Terminology: POST/GET /ValueSet/$expand
Terminology-->>API: ValueSet expansion bundle
API-->>Frontend: ValueSetExpansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@docs/adr/0006-fhir-terminology-server-selection.md`:
- Around line 163-179: Pin the terminology-server image (replace image:
snomedinternational/snowstorm-lite:latest with a specific version, e.g.,
snomedinternational/snowstorm-lite:2.3.1) and update the healthcheck block (the
healthcheck test currently using ["CMD","curl",...]) so it does not assume curl
is present: either verify the chosen image tag includes curl and keep the curl
command, or replace the test with a tool guaranteed in the image (or a CMD-SHELL
wrapper that falls back to wget or an HTTP check script) and ensure the
healthcheck uses that confirmed command (refer to the terminology-server service
and its healthcheck test).
In `@docs/prd/0006-terminology-server.md`:
- Around line 53-91: The fenced diagram blocks in
docs/prd/0006-terminology-server.md are missing language identifiers (triggering
markdownlint warnings); update each triple-backtick block (starting with the
large ASCII diagram block around the Open CIS Frontend/Backend diagram) to
include an appropriate language tag (e.g., text or yaml or http) after the
opening ```; apply this change consistently to all fenced code blocks in the
file so markdownlint stops flagging them.
- Around line 760-763: The example URL "http://snomed.info/sct?fhir_vs=ecl/<<
73211009" contains an unencoded space and the raw "<<" ECL operators; update the
example to URL-encode the ECL segment (encode "<<" as %3C%3C and the space as
%20) so the fhir_vs query becomes ecl/%3C%3C%2073211009 (i.e., replace the
unencoded "ecl/<< 73211009" occurrence with its URL-encoded form).
- Around line 118-132: Change the image from
snomedinternational/snowstorm-lite:latest to
snomedinternational/snowstorm-lite:2.3.1 to pin the deployment, and update the
healthcheck block (the healthcheck.test entry) because curl is not available
inside the Snowstorm Lite container: either replace the test with a
container-internal command that exists (e.g., use a wget --spider check if you
control the image), or build a derived image that installs curl/wget and point
the healthcheck.test to use that tool, or remove the in-container healthcheck
and run the HTTP probe from the host/Orchestrator instead; ensure your change
targets the image: and healthcheck.test fields in the terminology-server
service.
🧹 Nitpick comments (1)
docs/prd/0006-terminology-server.md (1)
163-168: Avoid mutable default inCodeLookupResult.properties.
properties: dict = {}risks shared state across instances. UseField(default_factory=dict)instead.♻️ Suggested fix
-from pydantic import BaseModel +from pydantic import BaseModel, Field ... -class CodeLookupResult(BaseModel): +class CodeLookupResult(BaseModel): code: str system: str display: str version: Optional[str] = None - properties: dict = {} + properties: dict = Field(default_factory=dict)
| ``` | ||
| ┌─────────────────────────────────────────────────────────────────────┐ | ||
| │ Open CIS Frontend │ | ||
| │ (Vue 3 + TypeScript) │ | ||
| │ ┌─────────────────┐ ┌─────────────────┐ ┌─────────────────────┐ │ | ||
| │ │ Code Picker │ │ Validation UI │ │ Terminology Browser │ │ | ||
| │ │ Components │ │ Feedback │ │ (optional) │ │ | ||
| │ └────────┬────────┘ └────────┬────────┘ └──────────┬──────────┘ │ | ||
| └───────────┼─────────────────────┼─────────────────────┼────────────┘ | ||
| │ │ │ | ||
| ▼ ▼ ▼ | ||
| ┌─────────────────────────────────────────────────────────────────────┐ | ||
| │ Open CIS Backend │ | ||
| │ (FastAPI + Python) │ | ||
| │ ┌─────────────────────────────────────────────────────────────┐ │ | ||
| │ │ Terminology Service Layer │ │ | ||
| │ │ • TerminologyClient (FHIR API wrapper) │ │ | ||
| │ │ • CodeValidator │ │ | ||
| │ │ • ValueSetResolver │ │ | ||
| │ │ • TerminologyCache (Redis/in-memory) │ │ | ||
| │ └────────────────────────┬────────────────────────────────────┘ │ | ||
| │ │ │ | ||
| │ ┌────────────────────────┼────────────────────────────────────┐ │ | ||
| │ │ EHRBase │ FHIR Terminology Server │ │ | ||
| │ │ Client │ Client │ │ | ||
| │ └───────────┬────────────┴──────────────┬─────────────────────┘ │ | ||
| └──────────────┼───────────────────────────┼─────────────────────────┘ | ||
| │ │ | ||
| ▼ ▼ | ||
| ┌──────────────────────────┐ ┌─────────────────────────────────────┐ | ||
| │ EHRBase │ │ FHIR Terminology Server │ | ||
| │ (Clinical Data) │ │ (Snowstorm Lite / HAPI FHIR) │ | ||
| │ │ │ │ | ||
| │ • Compositions │ │ • SNOMED CT │ | ||
| │ • Templates │ │ • ICD-10 │ | ||
| │ • AQL Queries │ │ • LOINC │ | ||
| │ │ │ • Custom ValueSets │ | ||
| └──────────────────────────┘ └─────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to fenced code blocks.
Markdownlint flags several blocks without a language tag; please add a language (e.g., text, yaml, http) starting with the first diagram block here and apply consistently across the file.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/prd/0006-terminology-server.md` around lines 53 - 91, The fenced diagram
blocks in docs/prd/0006-terminology-server.md are missing language identifiers
(triggering markdownlint warnings); update each triple-backtick block (starting
with the large ASCII diagram block around the Open CIS Frontend/Backend diagram)
to include an appropriate language tag (e.g., text or yaml or http) after the
opening ```; apply this change consistently to all fenced code blocks in the
file so markdownlint stops flagging them.
| Use ECL in ValueSet URLs: | ||
| ``` | ||
| http://snomed.info/sct?fhir_vs=ecl/<< 73211009 | ||
| ``` |
There was a problem hiding this comment.
URL-encode ECL query strings in examples.
The ECL example includes spaces and << which should be URL-encoded to avoid confusion when users copy/paste the URL.
✅ Example correction
-http://snomed.info/sct?fhir_vs=ecl/<< 73211009
+http://snomed.info/sct?fhir_vs=ecl/%3C%3C%2073211009📝 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.
| Use ECL in ValueSet URLs: | |
| ``` | |
| http://snomed.info/sct?fhir_vs=ecl/<< 73211009 | |
| ``` | |
| Use ECL in ValueSet URLs: |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 761-761: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/prd/0006-terminology-server.md` around lines 760 - 763, The example URL
"http://snomed.info/sct?fhir_vs=ecl/<< 73211009" contains an unencoded space and
the raw "<<" ECL operators; update the example to URL-encode the ECL segment
(encode "<<" as %3C%3C and the space as %20) so the fhir_vs query becomes
ecl/%3C%3C%2073211009 (i.e., replace the unencoded "ecl/<< 73211009" occurrence
with its URL-encoded form).
- Add Snowstorm Lite to docker-compose.yml with persistent volume - Create terminology client library with caching (TTLCache) - Implement API endpoints: lookup, validate, expand, search, subsumes - Register terminology router at /api/terminology - Add terminology server config to Settings and .env.example - Update setup script to wait for terminology server readiness Implements PRD-0006 Phase 1 (Infrastructure & Basic Integration). https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 15-20: Reorder the TERMINOLOGY_* environment variables in
.env.example to match dotenv-linter's expected sequence; specifically, place the
keys in alphabetical order: TERMINOLOGY_ADMIN_PASSWORD, TERMINOLOGY_CACHE_TTL,
TERMINOLOGY_SERVER_TIMEOUT, TERMINOLOGY_SERVER_URL, TERMINOLOGY_VALIDATION_MODE
so the linter no longer flags ordering issues.
In `@api/src/terminology/client.py`:
- Around line 73-83: The cache key for lookup currently uses only system and
code causing different requests with different properties to collide; update the
cache_key construction in the method where cache_key, _cache, properties, _get,
and _parse_lookup_response are used to include a deterministic representation of
properties (e.g., join or JSON-serialize the properties argument or use a sorted
tuple) so requests with different properties produce distinct keys before
checking/setting self._cache.
In `@scripts/setup.sh`:
- Around line 27-33: The readiness loop uses `curl -s` which treats HTTP 4xx/5xx
as success; update the `until` curl invocation inside the loop to use `curl
--fail` (e.g., `curl --fail -s` or `curl --fail -sS`) so non-2xx responses make
the command fail and the loop continues; modify the `until curl -s
http://localhost:8081/fhir/metadata` call in the loop to include `--fail` (and
optionally `-S` to show errors) to avoid false-positive readiness.
🧹 Nitpick comments (4)
docker-compose.yml (1)
37-42: Pin the Snowstorm Lite image tag for reproducibility.Using
:latestmakes dev setups non-deterministic and can break unexpectedly. Please pin to a tested version or digest.📌 Example pin
- image: snomedinternational/snowstorm-lite:latest + image: snomedinternational/snowstorm-lite:5.5.0 # TODO: replace with tested tag/digestapi/src/config.py (1)
22-26: Constrainterminology_validation_modeto allowed values.This will prevent invalid configuration at startup.
✅ Suggested typing
-from pydantic_settings import BaseSettings +from typing import Literal +from pydantic_settings import BaseSettings @@ - terminology_validation_mode: str = "warn" # strict | warn | off + terminology_validation_mode: Literal["strict", "warn", "off"] = "warn"api/src/terminology/client.py (1)
24-36: Async-only guideline violation in__init__and parser helpers.These are synchronous functions inside
api/**, which conflicts with the async-only guideline. If the rule is strict, consider async parser helpers (and an async factory for construction), or document an explicit exception.🧩 Example conversion for parser helpers
- result = self._parse_lookup_response(data, system, code) + result = await self._parse_lookup_response(data, system, code) @@ - return self._parse_validation_response(data) + return await self._parse_validation_response(data) @@ - return self._parse_expansion_response(data, url) + return await self._parse_expansion_response(data, url) @@ - def _parse_lookup_response( + async def _parse_lookup_response( @@ - def _parse_validation_response(self, data: dict[str, Any]) -> ValidationResult: + async def _parse_validation_response(self, data: dict[str, Any]) -> ValidationResult: @@ - def _parse_expansion_response(self, data: dict[str, Any], url: str) -> ValueSetExpansion: + async def _parse_expansion_response(self, data: dict[str, Any], url: str) -> ValueSetExpansion:As per coding guidelines,
api/**/*.py: All Python functions must be async.Also applies to: 176-240
api/src/main.py (1)
76-86: Consider adding terminology server status to health check.The health check currently reports only database connectivity. Given the new terminology server dependency, you might consider extending this to report terminology server availability as well—perhaps under a separate key to preserve the graceful degradation model where terminology unavailability results in "degraded" rather than "unhealthy" status.
This aligns with the PRD's success metrics around monitoring and the three-tier health model.
💡 Possible enhancement
`@app.get`("/health") async def health_check(): db_connected = prisma.is_connected() - status = "healthy" if db_connected else "degraded" + terminology_available = terminology_client.is_available() # If such method exists + status = "healthy" if db_connected else "degraded" response_data = { "status": status, "database": "connected" if db_connected else "disconnected", + "terminology": "available" if terminology_available else "unavailable", } if not db_connected: return JSONResponse(status_code=503, content=response_data) return response_data
| # Terminology server (Snowstorm Lite) - for API service | ||
| TERMINOLOGY_SERVER_URL=http://localhost:8081/fhir | ||
| TERMINOLOGY_SERVER_TIMEOUT=30 | ||
| TERMINOLOGY_CACHE_TTL=300 | ||
| TERMINOLOGY_VALIDATION_MODE=warn | ||
| TERMINOLOGY_ADMIN_PASSWORD=admin |
There was a problem hiding this comment.
Reorder terminology env keys to satisfy dotenv-linter.
The linter warns about ordering; if enforced, this will break CI. Consider reordering to the expected sequence.
🔧 Proposed reorder
-TERMINOLOGY_SERVER_URL=http://localhost:8081/fhir
-TERMINOLOGY_SERVER_TIMEOUT=30
-TERMINOLOGY_CACHE_TTL=300
-TERMINOLOGY_VALIDATION_MODE=warn
-TERMINOLOGY_ADMIN_PASSWORD=admin
+TERMINOLOGY_ADMIN_PASSWORD=admin
+TERMINOLOGY_CACHE_TTL=300
+TERMINOLOGY_SERVER_TIMEOUT=30
+TERMINOLOGY_SERVER_URL=http://localhost:8081/fhir
+TERMINOLOGY_VALIDATION_MODE=warn📝 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.
| # Terminology server (Snowstorm Lite) - for API service | |
| TERMINOLOGY_SERVER_URL=http://localhost:8081/fhir | |
| TERMINOLOGY_SERVER_TIMEOUT=30 | |
| TERMINOLOGY_CACHE_TTL=300 | |
| TERMINOLOGY_VALIDATION_MODE=warn | |
| TERMINOLOGY_ADMIN_PASSWORD=admin | |
| # Terminology server (Snowstorm Lite) - for API service | |
| TERMINOLOGY_ADMIN_PASSWORD=admin | |
| TERMINOLOGY_CACHE_TTL=300 | |
| TERMINOLOGY_SERVER_TIMEOUT=30 | |
| TERMINOLOGY_SERVER_URL=http://localhost:8081/fhir | |
| TERMINOLOGY_VALIDATION_MODE=warn |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 17-17: [UnorderedKey] The TERMINOLOGY_SERVER_TIMEOUT key should go before the TERMINOLOGY_SERVER_URL key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The TERMINOLOGY_CACHE_TTL key should go before the TERMINOLOGY_SERVER_TIMEOUT key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The TERMINOLOGY_ADMIN_PASSWORD key should go before the TERMINOLOGY_CACHE_TTL key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.example around lines 15 - 20, Reorder the TERMINOLOGY_* environment
variables in .env.example to match dotenv-linter's expected sequence;
specifically, place the keys in alphabetical order: TERMINOLOGY_ADMIN_PASSWORD,
TERMINOLOGY_CACHE_TTL, TERMINOLOGY_SERVER_TIMEOUT, TERMINOLOGY_SERVER_URL,
TERMINOLOGY_VALIDATION_MODE so the linter no longer flags ordering issues.
| # Wait for Terminology Server to be ready | ||
| echo "⏳ Waiting for Terminology Server (Snowstorm Lite) to be ready..." | ||
| until curl -s http://localhost:8081/fhir/metadata > /dev/null 2>&1; do | ||
| sleep 5 | ||
| echo " Still waiting..." | ||
| done | ||
| echo "✅ Terminology Server is ready!" |
There was a problem hiding this comment.
Use curl --fail to prevent false-positive readiness.
curl -s returns success on HTTP 4xx/5xx, so the loop can exit before the terminology server is actually ready.
🧩 Suggested fix
-until curl -s http://localhost:8081/fhir/metadata > /dev/null 2>&1; do
+until curl -fsS http://localhost:8081/fhir/metadata > /dev/null 2>&1; do📝 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.
| # Wait for Terminology Server to be ready | |
| echo "⏳ Waiting for Terminology Server (Snowstorm Lite) to be ready..." | |
| until curl -s http://localhost:8081/fhir/metadata > /dev/null 2>&1; do | |
| sleep 5 | |
| echo " Still waiting..." | |
| done | |
| echo "✅ Terminology Server is ready!" | |
| # Wait for Terminology Server to be ready | |
| echo "⏳ Waiting for Terminology Server (Snowstorm Lite) to be ready..." | |
| until curl -fsS http://localhost:8081/fhir/metadata > /dev/null 2>&1; do | |
| sleep 5 | |
| echo " Still waiting..." | |
| done | |
| echo "✅ Terminology Server is ready!" |
🤖 Prompt for AI Agents
In `@scripts/setup.sh` around lines 27 - 33, The readiness loop uses `curl -s`
which treats HTTP 4xx/5xx as success; update the `until` curl invocation inside
the loop to use `curl --fail` (e.g., `curl --fail -s` or `curl --fail -sS`) so
non-2xx responses make the command fail and the loop continues; modify the
`until curl -s http://localhost:8081/fhir/metadata` call in the loop to include
`--fail` (and optionally `-S` to show errors) to avoid false-positive readiness.
Add Snowstorm Lite / FHIR terminology server references to port config, service URLs, tech stack tables, env vars, API endpoints, docker logs, and code organization sections. https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
The Snowstorm Lite container does not include curl. Switch healthcheck to wget --spider which is available in the image. Pin the image tag to 2.3.1 for reproducible deployments. https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
Requests with different properties args now produce distinct cache keys, preventing stale/incorrect cache hits. https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
2 participants
Summary
This PR introduces comprehensive documentation for integrating a FHIR Terminology Server into Open CIS, addressing the critical gap in clinical terminology support. The documentation includes both an Architecture Decision Record (ADR) and a detailed Product Requirements Document (PRD).
Key Changes
ADR-0006: FHIR Terminology Server Selection
PRD-0006: FHIR Terminology Server Integration
$lookup,$validate-code,$expand,$subsumes,$translateNotable Implementation Details
Context
This documentation was inspired by feedback from Ian McNicoll (openEHR Industry Partner) suggesting that FHIR terminology server integration is a natural next step for any clinical system. The decision to use Snowstorm Lite balances Open CIS's minimal-footprint philosophy with the need for robust, standards-based terminology services.
Related Issues
Addresses the need for:
https://claude.ai/code/session_01KMYg2BqpZGM1k98QuyD8yY
Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.