Skip to content

refactor: extract deterministic FHIR reference helper#553

Open
prakashiitp wants to merge 2 commits into
maziyarpanahi:masterfrom
prakashiitp:om-359-deterministic-urn-helper
Open

refactor: extract deterministic FHIR reference helper#553
prakashiitp wants to merge 2 commits into
maziyarpanahi:masterfrom
prakashiitp:om-359-deterministic-urn-helper

Conversation

@prakashiitp

Copy link
Copy Markdown

Pull Request

Description

This PR extracts the deterministic urn:uuid generation logic from the FHIR Bundle assembler into a reusable helper so exporters can pre-compute stable cross-resource references before Bundle assembly.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test addition/improvement

Changes Made

  • Added deterministic_fullurl() helper in openmed.clinical.exporters.fhir.references
  • Reused the shared helper in the FHIR Bundle assembler
  • Documented the deterministic reference contract in the module docstring
  • Added tests verifying helper consistency and exporter-provided deterministic URN handling

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this change with different models/inputs

Validation:

python -m pytest tests/unit/clinical/test_fhir_references.py -q
python -m pytest tests/unit/clinical/test_fhir_bundle.py -q
python -m pytest tests/ -q

Result:

  • 1612 passed
  • 16 skipped
  • 13 warnings

Documentation

  • I have updated the documentation accordingly
  • I have added docstrings to new functions/classes
  • I have updated the CHANGELOG.md

Code Quality

  • I ran make format, make lint, and make format-check
  • For Swift/OpenMedKit changes, I ran make format-swift and make lint-swift
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Dependencies

  • I have not added any new dependencies
  • OR I have added new dependencies and they are justified because: ____

Checklist

  • I have read the contributing guidelines
  • My commits have clear, descriptive messages
  • I have squashed/organized my commits appropriately

Related Issues

Closes #544

Screenshots/Examples

N/A

@prakashiitp

Copy link
Copy Markdown
Author

@maziyarpanahi

Implemented the requested changes for #544 and opened this PR. All tests are passing locally (1612 passed).

Please let me know if any further changes are needed. Thank you for your review!

@maziyarpanahi maziyarpanahi added good first issue Good for newcomers help wanted Extra attention is needed improvement Hardening / refactor of existing code P3 Strategic roadmap-v2 OpenMed V2 roadmap backlog labels Jun 22, 2026

@maziyarpanahi maziyarpanahi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you @prakashiitp. I reviewed this against #544 / OM-359 and added one maintainer follow-up commit: test: pin deterministic FHIR fullUrl contract.

What I changed:

  • re-exported deterministic_fullurl from openmed.clinical.exporters.fhir so exporters have a stable public import path;
  • added a module-level contract to openmed/clinical/exporters/fhir/references.py explaining that helper-generated urn:uuid references survive Bundle assembly;
  • pinned the legacy UUID seed with an exact expected urn:uuid regression;
  • added an exact Bundle-output regression covering deterministic fullUrls and literal ResourceType/id reference rewriting;
  • let Ruff clean up formatting in the touched FHIR files.

Verification on the current PR checkout:

  • PYTHONPATH=/private/tmp/openmed-pr-553 /Users/maziyar/Developer/openmed/.venv/bin/python -m pytest tests/unit/clinical/test_fhir_references.py tests/unit/clinical/test_fhir_bundle.py -q -> 17 passed
  • /Users/maziyar/Developer/openmed/.venv/bin/ruff check openmed/clinical/exporters/fhir/__init__.py openmed/clinical/exporters/fhir/references.py openmed/clinical/exporters/fhir/bundle.py tests/unit/clinical/test_fhir_references.py -> passed
  • /Users/maziyar/Developer/openmed/.venv/bin/ruff format --check openmed/clinical/exporters/fhir/__init__.py openmed/clinical/exporters/fhir/references.py openmed/clinical/exporters/fhir/bundle.py tests/unit/clinical/test_fhir_references.py -> passed

I also copied the labels from #544 onto the PR. The branch is mergeable with no conflicts; GitHub has not attached hosted checks to the new head commit yet, so I verified the touched behavior locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good first issue Good for newcomers help wanted Extra attention is needed improvement Hardening / refactor of existing code P3 Strategic roadmap-v2 OpenMed V2 roadmap backlog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add deterministic urn:uuid reference helpers for cross-resource linking

2 participants