Merge pull request #70 from smart-on-fhir/mikix/docref-quirks

Add some missing US Core checks now that we have attachment info
smart-on-fhir · Jan 9, 2025 · 0cf12eb · 0cf12eb
2 parents bdbe8ba + 37d3d3b
commit 0cf12eb
Show file tree

Hide file tree

Showing 16 changed files with 54 additions and 77 deletions.
diff --git a/cumulus_library_data_metrics/__init__.py b/cumulus_library_data_metrics/__init__.py
@@ -1,3 +1,3 @@
 """Data Metrics study for Cumulus Library"""
 
-__version__ = "6.1.0"
+__version__ = "7.0.0"
diff --git a/cumulus_library_data_metrics/meta/version.jinja b/cumulus_library_data_metrics/meta/version.jinja
@@ -1,2 +1,2 @@
 CREATE TABLE {{ study_prefix }}__meta_version AS
-SELECT 1 AS data_package_version;
+SELECT 2 AS data_package_version;
diff --git a/cumulus_library_data_metrics/q_valid_us_core_v4/README.md b/cumulus_library_data_metrics/q_valid_us_core_v4/README.md
@@ -55,21 +55,6 @@ All possible rows for the resource in question.
 In the case of Observation profiles,
 it's just the count of rows with the category in question.
 
-### A note on the DiagnosticReport profile
-
-The DiagnosticReport profiles mark `presentedForm` as "Must Support".
-
-But since Cumulus ETL strips these fields as possible PHI,
-this metric does not examine them.
-
-### A note on the DocumentReference profile
-
-The DocumentReference profile requires one or both of `content.attachment.data`
-and `content.attachment.url`.
-
-But since Cumulus ETL strips these fields as PHI,
-this metric does not require them.
-
 ### A note on Observation profiles
 
 Unlike the other resources, which check all rows, Observations are kind of a wild

diff --git a/cumulus_library_data_metrics/us_core_v4/diagnosticreport_lab_must_support.jinja b/cumulus_library_data_metrics/us_core_v4/diagnosticreport_lab_must_support.jinja
@@ -2,9 +2,6 @@
 -- There is another non-lab-report-specific profile,
 -- but this is the DiagnosticReport profile that covers the LAB rows.
 
--- CUMULUS-QUIRK
--- Doesn't look for "presentedForm", because Cumulus strips that field.
-
 {% import 'us_core_v4/utils.jinja' as core_utils %}
 
 WITH

diff --git a/cumulus_library_data_metrics/us_core_v4/diagnosticreport_note_must_support.jinja b/cumulus_library_data_metrics/us_core_v4/diagnosticreport_note_must_support.jinja
@@ -2,9 +2,6 @@
 -- There is another lab-report-specific profile,
 -- but this is the "base DiagnosticReport" profile that covers the bare minimum.
 
--- CUMULUS-QUIRK
--- Doesn't look for "presentedForm", because Cumulus strips that field.
-
 {% import 'us_core_v4/utils.jinja' as core_utils %}
 
 WITH
@@ -15,7 +12,8 @@ tmp_simplified AS (
         id,
         {{ utils.is_reference_of_type('encounter', 'Encounter') }} AS valid_encounter,
         issued IS NOT NULL AS valid_issued,
-        performer IS NOT NULL AS valid_performer
+        performer IS NOT NULL AS valid_performer,
+        presentedForm IS NOT NULL AS valid_presented_form
     FROM tmp_slice
 )
 
@@ -24,6 +22,7 @@ set ns.fields = [
     'valid_encounter',
     'valid_issued',
     'valid_performer',
+    'valid_presented_form',
 ]
 %}
 

diff --git a/cumulus_library_data_metrics/us_core_v4/documentreference_mandatory.jinja b/cumulus_library_data_metrics/us_core_v4/documentreference_mandatory.jinja
@@ -3,23 +3,11 @@
 -- CUMULUS-QUIRK
 -- Doesn't look at the us-core-6 attachment data/url check, because Cumulus strips those fields.
 
+{% import 'attachment_utils.jinja' as attachment_utils %}
 {% import 'us_core_v4/utils.jinja' as core_utils %}
 
 WITH
-tmp_attachment_flat AS (
-    SELECT
-        id,
-
-        {% if schema["content"]["attachment"] %}
-        u.content.attachment.contenttype
-        {% else %}
-        CAST(NULL AS VARCHAR) AS contenttype
-        {% endif %}
-
-    FROM {{ src }},
-        UNNEST(content) AS u (content)
-),
-
+tmp_attachment_flat AS {{ attachment_utils.extract_attachments(src, schema) }},
 tmp_type_codes_flat AS {{ utils.extract_codes_flat(src, 'type')}},
 
 tmp_grouped AS (
@@ -44,8 +32,12 @@ tmp_grouped AS (
         ) AS valid_type,
 
         BOOL_AND(
-            att.contenttype IS NOT NULL
-        ) AS valid_content_type
+            att.content_type IS NOT NULL
+        ) AS valid_content_type,
+
+        BOOL_AND(
+            att.has_data OR att.has_url
+        ) AS valid_us_core_6
 
     FROM {{ src }} AS src
     LEFT JOIN tmp_attachment_flat AS att ON att.id = src.id
@@ -63,7 +55,8 @@ tmp_simplified AS (
         valid_type,
         category IS NOT NULL AS valid_category,
         {{ utils.is_reference_of_type('subject', 'Patient') }} AS valid_subject,
-        valid_content_type
+        valid_content_type,
+        valid_us_core_6
     FROM tmp_grouped
 )
 
@@ -74,6 +67,7 @@ set ns.fields = [
     'valid_category',
     'valid_subject',
     'valid_content_type',
+    'valid_us_core_6',
 ]
 %}
 

diff --git a/cumulus_library_data_metrics/us_core_v4/documentreference_must_support.jinja b/cumulus_library_data_metrics/us_core_v4/documentreference_must_support.jinja
@@ -7,7 +7,7 @@
 
 WITH
 
-{% if schema["content"]["format"] %}
+{% if schema["content"]["format"]["code"] or schema["content"]["format"]["system"] %}
 tmp_content_flat AS (
     SELECT
         id,

diff --git a/cumulus_library_data_metrics/us_core_v4/profiles.py b/cumulus_library_data_metrics/us_core_v4/profiles.py
@@ -2,6 +2,7 @@
 
 from typing import ClassVar
 
+from cumulus_library_data_metrics import resource_info
 from cumulus_library_data_metrics.base import MetricMixin
 
 
@@ -17,17 +18,14 @@ class UsCoreV4Mixin(MetricMixin):
             ],
         },
         "DocumentReference": {
-            "content": [
-                "attachment",
-                "format",
-            ],
             "context": {
                 "encounter": {},
                 "period": {
                     "start": {},
                     "end": {},
                 },
             },
+            **resource_info.DOCREF_ATTACHMENT_SCHEMA,
         },
         "Encounter": {
             "hospitalization": [
@@ -89,7 +87,11 @@ def add_metric_queries(self) -> None:
         # - category/loinc: property to slice on for Observations
         # - mandatory_split: some profiles have a lot of mandatory fields, which can cause
         #   performance issues when cubing. This is a recommended hint of how many tables to
-        #   split any mandatory cube into.
+        #   split any mandatory cube into. Note that this argument gets passed on to the metric
+        #   classes that implement this mixin, and _optionally_ used. For example, the
+        #   q_valid_us_core_v4 metric does not care about this argument, because it doesn't cube.
+        #   The c_us_core_v4_count metric *does* care about it, when cubing. It will split its
+        #   output tables into multiple tables, to keep CUBE time low.
 
         # Observation is so big, that if it falls over in Athena, let's know early.
         # So we run these first,
@@ -109,7 +111,7 @@ def add_metric_queries(self) -> None:
         self.make_table(src="Condition")
         self.make_table(src="DiagnosticReport", name="Lab")
         self.make_table(src="DiagnosticReport", name="Note")
-        self.make_table(src="DocumentReference")
+        self.make_table(src="DocumentReference", mandatory_split=2)
         self.make_table(src="Encounter")
         self.make_table(src="Immunization")
         self.make_table(src="Medication")

diff --git a/tests/data/c_us_core_v4_count/general/documentreference/0.ndjson b/tests/data/c_us_core_v4_count/general/documentreference/0.ndjson
@@ -1,3 +1,3 @@
-{"resourceType": "DocumentReference", "id": "mandatory-plus-support", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain"}, "format": {"display": "X"}}], "date": "2012-10-12", "author": [], "context": {"encounter": [{"reference": "Encounter/X"}], "period": {"end": "2015"}}}
-{"resourceType": "DocumentReference", "id": "mandatory-only", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain"}}]}
+{"resourceType": "DocumentReference", "id": "mandatory-plus-support", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain", "data": "xxx"}, "format": {"display": "X"}}], "date": "2012-10-12", "author": [], "context": {"encounter": [{"reference": "Encounter/X"}], "period": {"end": "2015"}}}
+{"resourceType": "DocumentReference", "id": "mandatory-only", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain", "data": "xxx"}}]}
 {"resourceType": "DocumentReference", "id": "support-only", "date": "2012-10-12", "author": [], "content": [{"format": {"display": "X"}}], "context": {"encounter": [{"reference": "Encounter/X"}], "period": {"start": "2014"}}}
diff --git a/tests/data/c_us_core_v4_count/general/expected_documentreference_mandatory.csv b/tests/data/c_us_core_v4_count/general/expected_documentreference_mandatory.csv
@@ -1,4 +1,4 @@
-cnt,year,status,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid
-1,cumulus__none,current,true,true,true,true,true,true
-1,2015,current,true,true,true,true,true,true
-1,2014,cumulus__none,false,false,false,false,false,false
+cnt,year,status,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid_us_core_6,valid
+1,cumulus__none,current,true,true,true,true,true,true,true
+1,2015,current,true,true,true,true,true,true,true
+1,2014,cumulus__none,false,false,false,false,false,false,false
diff --git a/tests/data/meta/general/expected_version.csv b/tests/data/meta/general/expected_version.csv
@@ -1,2 +1,2 @@
 data_package_version
-1
+2
diff --git a/tests/data/t_us_core_v4/docref-low-schema/expected_documentreference_mandatory.csv b/tests/data/t_us_core_v4/docref-low-schema/expected_documentreference_mandatory.csv
@@ -1,2 +1,2 @@
-id,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid
-nothing,false,false,false,false,false,false
+id,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid_us_core_6,valid
+nothing,false,false,false,false,false,false,false
diff --git a/tests/data/t_us_core_v4/mandatory/documentreference/0.ndjson b/tests/data/t_us_core_v4/mandatory/documentreference/0.ndjson
@@ -1,4 +1,4 @@
-{"resourceType": "DocumentReference", "id": "valid", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain"}}]}
+{"resourceType": "DocumentReference", "id": "valid", "status": "current", "category": [{"text": "X"}], "type": {"coding": [{"system": "http://loinc.org", "code": "11534-5"}]}, "subject": {"reference": "Patient/A"}, "content": [{"attachment": {"contentType": "text/plain", "url": "xxx"}}]}
 {"resourceType": "DocumentReference", "id": "type-empty", "type": {}}
 {"resourceType": "DocumentReference", "id": "type-bad-system", "type": {"coding": [{"system": "nope", "code": "11534-5"}]}}
 {"resourceType": "DocumentReference", "id": "type-null-flavor-good", "type": {"coding": [{"system": "http://terminology.hl7.org/CodeSystem/v3-NullFlavor", "code": "UNK"}]}}

diff --git a/tests/data/t_us_core_v4/mandatory/expected_documentreference_mandatory.csv b/tests/data/t_us_core_v4/mandatory/expected_documentreference_mandatory.csv
@@ -1,14 +1,14 @@
-id,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid
-valid,true,true,true,true,true,true
-type-null-flavor-good,false,true,false,false,false,false
-type-null-flavor-bad,false,false,false,false,false,false
-type-multiple-good,false,true,false,false,false,false
-type-multiple-bad,false,false,false,false,false,false
-type-empty,false,false,false,false,false,false
-type-bad-system,false,false,false,false,false,false
-subject-group,false,false,false,false,false,false
-nothing,false,false,false,false,false,false
-content-no-contenttype,false,false,false,false,false,false
-content-no-attachment,false,false,false,false,false,false
-content-multiple-good,false,false,false,false,true,false
-content-multiple-bad,false,false,false,false,false,false
+id,valid_status,valid_type,valid_category,valid_subject,valid_content_type,valid_us_core_6,valid
+valid,true,true,true,true,true,true,true
+type-null-flavor-good,false,true,false,false,false,false,false
+type-null-flavor-bad,false,false,false,false,false,false,false
+type-multiple-good,false,true,false,false,false,false,false
+type-multiple-bad,false,false,false,false,false,false,false
+type-empty,false,false,false,false,false,false,false
+type-bad-system,false,false,false,false,false,false,false
+subject-group,false,false,false,false,false,false,false
+nothing,false,false,false,false,false,false,false
+content-no-contenttype,false,false,false,false,false,true,false
+content-no-attachment,false,false,false,false,false,false,false
+content-multiple-good,false,false,false,false,true,false,false
+content-multiple-bad,false,false,false,false,false,false,false
diff --git a/tests/data/t_us_core_v4/must-support/diagnosticreport/notes.ndjson b/tests/data/t_us_core_v4/must-support/diagnosticreport/notes.ndjson
@@ -1,3 +1,3 @@
-{"resourceType": "DiagnosticReport", "id": "valid", "encounter": {"reference": "Encounter/A"}, "issued": "2022", "performer": []}
+{"resourceType": "DiagnosticReport", "id": "valid", "encounter": {"reference": "Encounter/A"}, "issued": "2022", "performer": [], "presentedForm": [{"contentType": "text/html"}]}
 {"resourceType": "DiagnosticReport", "id": "wrong-encounter-type", "encounter": {"reference": "EpisodeOfCare/A"}}
 {"resourceType": "DiagnosticReport", "id": "nothing"}
diff --git a/tests/data/t_us_core_v4/must-support/expected_diagnosticreport_note_must_support.csv b/tests/data/t_us_core_v4/must-support/expected_diagnosticreport_note_must_support.csv
@@ -1,4 +1,4 @@
-id,valid_encounter,valid_issued,valid_performer,valid
-wrong-encounter-type,false,false,false,false
-valid,true,true,true,true
-nothing,false,false,false,false
+id,valid_encounter,valid_issued,valid_performer,valid_presented_form,valid
+wrong-encounter-type,false,false,false,false,false
+valid,true,true,true,true,true
+nothing,false,false,false,false,false
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,7 +7,7 @@ @@
     WITH
-    {% if schema["content"]["format"] %}
+    {% if schema["content"]["format"]["code"] or schema["content"]["format"]["system"] %}
     tmp_content_flat AS (
         SELECT
             id,
@@ Expand Down @@