Fix sp_d_morbidity_report_postprocessing: rewrite user-comment join via NRT staging CSV#837
Open
nullflux wants to merge 1 commit into
Open
Fix sp_d_morbidity_report_postprocessing: rewrite user-comment join via NRT staging CSV#837nullflux wants to merge 1 commit into
nullflux wants to merge 1 commit into
Conversation
…aging CSV
The step "Generating ##SAS_morb_Rpt_User_Comment" joined the Order
observation to itself via root.morb_rpt_uid = obs.observation_uid and
then filtered obs.obs_domain_cd_st_1 IN ('C_Order','C_Result'), which
is impossible because the Order row's obs_domain_cd_st_1 is 'Order'.
The temp table was therefore always empty and MORB_RPT_USER_COMMENT
never populated.
Replaced the broken self-join with a staging-side walk: the upstream
NRT row #nrt_morbidity_observation already carries
followup_observation_uid as a CSV of the Order's children (mixed
C_Order / C_Result / Result), so this step expands that CSV via
CROSS APPLY string_split and joins to #morb_obs_reference filtered to
obs_domain_cd_st_1 = 'C_Result' to pull the user-comment row.
Stays entirely within RDB_MODERN staging — no cross-DB read of
nbs_odse.dbo.act_relationship, consistent with the
sp_nrt_*_postprocessing layer's "NRT-only" convention (cross-DB ODSE
joins live exclusively in sp_*_event SPs).
Verified locally: applied the routine, truncated MORB_RPT_USER_COMMENT,
ran sp_d_morbidity_report_postprocessing for the seeded morb (uid
20080010), confirmed 1 row populated with the seeded user-comment
text. job_flow_log shows step 19 (build ##SAS_morb_Rpt_User_Comment)
and step 27 (Insert into morb_Rpt_User_Comment) both at row_count=1
with no errors.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
nullflux
added a commit
that referenced
this pull request
May 20, 2026
nullflux
added a commit
that referenced
this pull request
May 20, 2026
…PR routines in orchestrator Two changes: 1. Extend bug #5b's patient_id-NULL fix to the 10 multi-condition nrt_investigation variants (22000010-22000100) and the Tetanus variant (22000200). The original cherry-pick (bb7cec9) only set patient_id on the foundation Inv (20000100); the Tier 3 variants left it NULL, so the same DELETE-by-sentinel-PATIENT_UID cascade that blocks HEPATITIS_DATAMART would block any condition-datamart SP whose query path goes through F_PAGE_CASE -> D_PATIENT lookup. 2. New orchestrator step 2.5 apply_pending_pr_routines pulls each pending-PR routine (PRs #837, #839, #840) from its bug branch via `git show` and applies it to the freshly-reset DB before fixtures. Without this the orchestrator aborts at step 8 on ldf_answers_tetanus.sql (bug #7 early-RETURN guard fires on a SUB data_type ldf_uid, and bug #8 SUBSTRING fires on the tetanus datamart SP). Remove a branch from the list once its PR merges and the baseline image is refreshed. End-to-end coverage uplift vs. coverage_tier_3.md baseline: HEPATITIS_DATAMART 0 -> 1 (5b) COVID_CASE_DATAMART 0 -> 1 (5b on 22000070) BMIRD_STREP_PNEUMO_DATAMART 0 -> 1 (5b on 22000100) F_PAGE_CASE 1 -> 6 (multi-condition + 5b) LDF_DIMENSIONAL_DATA 0 -> 5 (PR #839 bug-7) MORB_RPT_USER_COMMENT 0 -> 1 (PR #837 bug-3) Still 0 (separate per-condition blockers, not 5b-related): HEPATITIS_CASE, HEP100, F_STD_PAGE_CASE, STD_HIV_DATAMART, TB_DATAMART, VAR_DATAMART, LDF_TETANUS, LDF_HEPATITIS — see coverage_hep_datamart_investigation.md for per-condition diagnoses. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Thoughts on adding a unit test to cover this scenario? |
Contributor
|
can you explain what is meant by staging CSV? |
Contributor
I think its just that the UID column is an intermediate comma separated list of values, not a single UID.
|
Contributor
|
have you been able to repro this in the UI? I haven't been able to get data into this table via either MasterETL or RTR. the data dictionary says this table is for comments on an "externally entered Morbidity Report" so maybe there's some non-UI route for morb reports? edit: 🕳️ 🐇 |
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
4 participants
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.
Description
The user-comment query at lines 802-816 of
016-sp_nrt_morbidity_report_postprocessing-001.sqljoined the morb Order observation to itself and then filteredobs_domain_cd_st_1 IN ('C_Order','C_Result'). This is impossible, since the Order row's domain is'Order'. The temp table was always empty andMORB_RPT_USER_COMMENTnever populated.This PR rewrites it to walk Order to C_Result via the staging CSV
nrt_morbidity_observation.followup_observation_uid(which the NRT layer already projects from the act_relationship graph), then filter#morb_obs_referencetoobs_domain_cd_st_1 = 'C_Result'for the user-comment row'sactivity_to_time,add_user_id, and linked text.Verified locally by truncating
MORB_RPT_USER_COMMENT, ran the SP for a morb Order from the ODSE generated test fixtures (uid=20080010), with table then showing the expected row.job_flow_logsteps 19 and 27 at row_count=1, no errors.Related Issue
APP-471
Additional Notes
No
testData/unitfixture — exercising the SP needs ~10 seeded tables across both DBs. End-to-end repro is inutilities/comparison-fixtures/bugs/03_morb_rpt_user_comment/repro.sql.Checklist