Release v 1.2#197
Merged
Merged
Conversation
pendingintent
commented
Apr 20, 2026
pendingintent
commented
Owner
- Introduced biomedical concept groupings
- Sidebar menu navigation added
- Data set specializatgions are no longer automapped. The user is shown a list of DSS associated with a BC from which to assign a DSS (if required).
- DSS are no longer used as BiomedicalConceptProperty values in the USDM JSON
- Stale AliasCode/Code associations are fixed
- Synthetic subject data for NCT01797120 has been generated and included for loading into the database (if required)
- New CT browsers added for SDTM, CDASH, Define-XML, protocol and DDF
- No database lookups used for DDF or protocol CT. Replaced with CDISC Library API calls
Merging the new features for biomedical concept groupings.
…tiple DSS to assign from BC
…ayed in more tabular format
…reation of stale codes in future
…es (C174222) now loaded from API
…RM data origin types (C188727) now loaded from API
pendingintent
added
bug
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Release v1.2 updates the SoA Workbench’s biomedical concept workflow and controlled-terminology UX, shifting several terminology sources from local SQLite tables to live CDISC Library API-backed browsers, adding concept groupings, and adjusting USDM generation accordingly.
Changes:
- Added biomedical concept grouping UI/DB support and updated concepts/DSS assignment UI behavior.
- Replaced legacy DDF/Protocol terminology tables & pages with new CDISC Library CT browser routers/templates (plus SDTM/CDASH/Define-XML browsers).
- Updated USDM generation utilities to use CDISC API lookups (and added BC surrogate USDM component), along with migrations to fix stale code chains and remove obsolete data.
Reviewed changes
Copilot reviewed 55 out of 95 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_terminology_date.py | Removed tests tied to legacy DDF/Protocol terminology table schema. |
| tests/test_routers_arms.py | Simplified UI arm tests to expect normal UI responses (no try/except for missing legacy tables). |
| tests/TEST_FILES_REVIEW.md | Removed internal test review document. |
| tests/ROUTER_TESTS_README.md | Removed internal router test summary document. |
| src/usdm/usdm_utils.py | Updated DSS/code enrichment and BC API helpers; shifted terminology enrichment to CDISC Library CT utilities. |
| src/usdm/generate_encounters.py | Updated encounter type code enrichment to use CDISC Library DDF CT (via cached utilities). |
| src/usdm/generate_biomedical_concepts.py | Updated BC generation to source synonyms/reference from BC API and removed DSS-derived properties output. |
| src/soa_builder/web/templates/sdtm_terminology.html | Added SDTM CT browser UI template. |
| src/soa_builder/web/templates/protocol_terminology_audit.html | Removed legacy Protocol terminology audit UI template. |
| src/soa_builder/web/templates/protocol_terminology.html | Removed legacy Protocol terminology UI template. |
| src/soa_builder/web/templates/protocol_controlled_terminology.html | Added Protocol CT browser UI template. |
| src/soa_builder/web/templates/dss_cell.html | Updated DSS assignment cell UI for multi-assignment/lazy option loading. |
| src/soa_builder/web/templates/define_xml_terminology.html | Added Define-XML CT browser UI template. |
| src/soa_builder/web/templates/ddf_terminology_audit.html | Removed legacy DDF terminology audit UI template. |
| src/soa_builder/web/templates/ddf_controlled_terminology.html | Added DDF CT browser UI template. |
| src/soa_builder/web/templates/concepts_cell.html | Updated concepts cell UI to support concept groups + surrogate/group removal; added local styling. |
| src/soa_builder/web/templates/concept_groups.html | Added concept group management UI template. |
| src/soa_builder/web/templates/concept_detail.html | Expanded concept detail UI (summary + DEC table + formatted JSON). |
| src/soa_builder/web/templates/cdash_terminology.html | Updated template to CDASH CT browser behavior and presentation. |
| src/soa_builder/web/templates/base.html | Reworked sidebar into nested menus with new navigation entries for CT browsers and concept groups. |
| src/soa_builder/web/templates/activities.html | Removed DSS auto-assign UI and added concept-group UID computation for rendering. |
| src/soa_builder/web/static/style.css | Added nested sidebar submenu styling and hover behavior. |
| src/soa_builder/web/static/images/image-7.png | Added screenshot asset used by docs/help. |
| src/soa_builder/web/static/images/image-4.png | Added screenshot asset used by docs/help. |
| src/soa_builder/web/static/help/bc_groupings.html | Added static help page for biomedical concept groupings. |
| src/soa_builder/web/routers/usdm_json.py | Added USDM component entry/build path for BC surrogates. |
| src/soa_builder/web/routers/sdtm_terminology.py | Added SDTM CT JSON + UI router backed by cached CDISC Library CT fetch. |
| src/soa_builder/web/routers/protocol_controlled_terminology.py | Added Protocol CT JSON + UI router backed by cached CDISC Library CT fetch. |
| src/soa_builder/web/routers/define_xml_terminology.py | Added Define-XML CT JSON + UI router backed by cached CDISC Library CT fetch. |
| src/soa_builder/web/routers/ddf_controlled_terminology.py | Added DDF CT JSON + UI router backed by cached CDISC Library CT fetch. |
| src/soa_builder/web/routers/cdash_terminology.py | Added CDASH CT JSON + UI router backed by cached CDISC Library CT fetch. |
| src/soa_builder/web/routers/bc_surrogates.py | Extended concepts-cell re-rendering to include concept groups and group metadata. |
| src/soa_builder/web/migrate_database.py | Added migrations for concept groups, DSS assignment table, dropping legacy terminology tables, and code-chain cleanup. |
| scripts/populate_biomedical_concept_property.py | Removed one-time script for DSS-derived BC property population. |
| pyproject.toml | Added pytest-cov to dev extras (CI install path uses .[dev]). |
| files/pilot_LLZT_protocol.xlsx | Added protocol file artifact. |
| files/SoA_breast_cancer.csv | Removed CSV artifact. |
| files/README.md | Added inventory/description of files under files/. |
| docs/biomedical_concept_grouping.md | Added short doc describing concept grouping feature and architecture. |
| docs/SoA-Workbench-UAT-Testing.docx | Added UAT testing document artifact. |
| docs/ScheduledDecisionInstances.md | Added documentation describing ScheduledDecisionInstance usage in USDM. |
| docs/NCT01797120-results.md | Added results summary doc for NCT01797120 bundle. |
| docs/Create TDD.docx | Added document artifact for TDD creation. |
| docs/BIOMEDICAL_CONCEPT_WORKFLOW.md | Added/updated BC workflow documentation with new UI screenshots. |
| docs/BIOMEDICAL_CONCEPT_GROUPINGS.md | Added/updated concept grouping documentation with screenshots. |
| docs/BC_WORKFLOW.md | Removed older BC workflow doc (superseded by newer docs). |
| changelog/fix-dss-assignments_17ef8f2.md | Added design/change plan doc for DSS assignment overhaul. |
| changelog/biomedical-concept-grouping_60ce0d1.md | Added changelog doc for concept grouping implementation. |
| .github/workflows/ci.yml | Updated CI install strategy and narrowed workflow branch triggers. |
Comment on lines
+124
to
+126
| except Exception as exc: | ||
| logger.exception("Failed to render Protocol CT page") | ||
| raise HTTPException(500, f"Failed to render Protocol CT: {exc}") |
There was a problem hiding this comment.
Raising HTTPException with the caught exception text ({exc}) can leak internal details to end users (and may vary per failure). Prefer a generic 500 message in the response and rely on structured logging (logger.exception) for the full traceback.
Comment on lines
+128
to
+130
| except Exception as exc: | ||
| logger.exception("Failed to render Define-XML terminology page") | ||
| raise HTTPException(500, f"Failed to render Define-XML terminology: {exc}") |
There was a problem hiding this comment.
Raising HTTPException with the caught exception text ({exc}) can leak internal details to end users. Prefer a generic 500 message in the response and keep the exception details only in logs via logger.exception(...).
Comment on lines
167
to
+178
| @functools.lru_cache(maxsize=256) | ||
| def _get_biomedical_concept_synonyms(concept_code: str) -> List[str]: | ||
| """Fetch the synonyms of a biomedical concept using the CDISC API. Cached per code.""" | ||
| def _get_biomedical_concept_data(concept_code: str) -> Dict[str, Any]: | ||
| """Fetch the full CDISC Biomedical Concept API response. Cached per code.""" | ||
| url = URL_PREFIX + "mdr/bc/biomedicalconcepts/" + concept_code | ||
| try: | ||
| resp = requests.get(url, headers=_build_api_headers(), timeout=15) | ||
| if resp.status_code != 200: | ||
| return [] | ||
| data = resp.json() | ||
| return data.get("synonyms", []) | ||
| return {} | ||
| return resp.json() | ||
| except (requests.RequestException, ValueError) as e: | ||
| print(f"Error fetching biomedical concept synonyms: {e}") | ||
| return [] | ||
| print(f"Error fetching biomedical concept: {e}") | ||
| return {} |
There was a problem hiding this comment.
This helper logs API failures with print(...). In the web app / CI context that makes logs harder to route and correlate. Prefer using a module logger (and logger.exception for tracebacks) instead of printing to stdout.
Comment on lines
210
to
214
| def _get_type_code_tuple(soa_id: int, code_uid: str) -> Tuple[str, str, str, str]: | ||
| """Fetch type codes for ARMS only. These values are stored in the protocol_terminology table.""" | ||
| """Fetch ARM type codes enriched from CDISC Library Protocol CT.""" | ||
| from soa_builder.web.utils import get_protocol_ct_rows, get_protocol_ct_term | ||
|
|
||
| conn = _connect() |
There was a problem hiding this comment.
The annotated return type Tuple[str, str, str, str] doesn’t match what the function actually returns (lists of codes/decodes/systems/versions). This makes the typing misleading for callers. Update the signature to return Tuple[List[str], List[str], List[str], List[str]] (and likewise for the other *_tuple helpers updated in this PR).
Comment on lines
+8
to
+11
| # Encounter type codes are resolved via CDISC Library DDF CT. | ||
| def _get_type_code_tuple(soa_id: int, code_uid: str) -> Tuple[str, str, str, str]: | ||
| """Fetch type codes for ENCOUNTERS only. These values are stored in the ddf_terminology table.""" | ||
| """Enrich ENCOUNTER type code_association rows via CDISC Library DDF CT.""" | ||
| from soa_builder.web.utils import get_ddf_ct_rows, get_ddf_ct_term |
There was a problem hiding this comment.
The annotated return type Tuple[str, str, str, str] doesn’t match the actual return value (lists of codes/decodes/systems/versions). Update the type annotation to Tuple[List[str], List[str], List[str], List[str]] (and consider using concrete typing like list[str] for consistency).
Comment on lines
+61
to
+70
| @router.get("/sdtm/terminology", response_class=JSONResponse) | ||
| def get_sdtm_terminology( | ||
| search: Optional[str] = None, | ||
| code: Optional[str] = None, | ||
| codelist_name: Optional[str] = None, | ||
| codelist_code: Optional[str] = None, | ||
| limit: int = 50, | ||
| offset: int = 0, | ||
| refresh: bool = False, | ||
| ): |
There was a problem hiding this comment.
New SDTM terminology endpoints/routes are introduced here, but there don’t appear to be any tests covering them (no matches in tests for /sdtm/terminology or /ui/sdtm/terminology). Given the repo has extensive router test coverage elsewhere, add tests for these new terminology routers (at least the JSON endpoints and basic filtering/pagination).
Comment on lines
79
to
+84
| "extensionAttributes": [], | ||
| "name": name, | ||
| "label": label, | ||
| "synonyms": synonyms, | ||
| "reference": reference | ||
| or "https://evsexplore.semantics.cancer.gov/evsexplore/concept/ncit/", | ||
| "properties": _get_biomedical_concept_properties(soa_id, id), | ||
| "reference": reference, | ||
| "properties": [], |
There was a problem hiding this comment.
_get_biomedical_concept_reference() can return an empty string when the BC API is unavailable/non-200, and the generator now emits that directly as reference. If consumers expect a stable URL, consider keeping a deterministic fallback (e.g., construct the evsexplore NCIt URL from concept_code) when the API response doesn’t include href.
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
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.