Fix v35 participant PHV regression in FHS-ingest#364
Merged
amc-corey-cox merged 1 commit intomainfrom Feb 26, 2026
Merged
Conversation
The v35 merge (PR #358) systematically changed associated_participant populated_from to match associated_visit populated_from in multi-visit blocks, overwriting the correct participant PHV with the visit PHV. Restores correct participant PHVs from v33 for 8 pht tables (234 blocks across 71 files) and updates the corresponding uuid5 str() expressions. pht002889 is excluded as it had a pre-existing duplicate that needs separate curator review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Restores correct participant PHV usage in FHS-ingest transformation specs after the v35 migration and subsequent uuid5 identifier rollout, ensuring participant identifiers and visit identifiers are derived from the appropriate PHV fields.
Changes:
- Reverts
associated_participant.populated_fromin affected multi-visit blocks to the correct participant PHVs. - Updates uuid5 expressions so
str()uses the participant PHV while visit-name selection continues to use the visit PHV. - Applies these corrections across the affected PHT tables/files called out in the PR description.
Reviewed changes
Copilot reviewed 71 out of 71 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| priority_variables_transform/FHS-ingest/whtbld_ct.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/waist_circ.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/visit.yaml | Fix Visit id/associated_participant PHV usage for the impacted block. |
| priority_variables_transform/FHS-ingest/valv_hrtdis.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/troponin.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/triglyc_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tot_chol_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_vasodil.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_statin.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_nstat_med.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_med_diab.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_insulin.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_diuret.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_calchanblk.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_betablk.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_angiorecepblk.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_alphablk.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_aldorecepblk.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/tak_aceinhib.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/stroke.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/spo2.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/sodium_blood.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/sleep_duration_daily.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/rdbld_ct.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/qrs_ekg.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/pmv.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/platelet_ct.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/monocyte_ncnc_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/mcv.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/mchc.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/mch.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/lvh_ekg.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/ldl.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hypertension.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hypert_trt.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hrtrt.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hist_my_inf.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hist_hrtfail.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hist_hrtdis.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hist_cvd.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hist_cor_angio.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hip_circ.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hemo_a1c.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hemo.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hemat.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/hdl.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/glucose_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/fvc.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/fev1.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/fast_gluc_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/eosinophil_ncnc_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/demography.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/crp.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/creat_urin.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/creat_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/copd.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/cig_smok.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/bun.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/bmi.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/bilirubin_tot.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/bdy_wgt.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/bdy_hgt.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/basophil_ncnc_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/asthma.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/ast_sgot.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/apnea_hypop_index.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/angina.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/alt_sgpt.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/albumin_urine.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/albumin_bld.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
| priority_variables_transform/FHS-ingest/afib.yaml | Fix participant PHV and uuid5 str() PHV for affected multi-visit blocks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
csiege
requested changes
Feb 26, 2026
Collaborator
csiege
left a comment
csiege
left a comment
There was a problem hiding this comment.
Summarizing issues here:
Fix expsoure_provenance → exposure_provenance typo.
All tak_*.yaml, hypert_trt.yaml, med_use.yaml (~16 files)
Fix OMOP:380035630 → OMOP:38003563 ethnicity code | demography.yaml (pht009760 blocks)
Fix exposure_provenance value (seeing both across Cohorts)
Should is be: PATIENT SELF-REPORTED MEDICATION
Or this: PATIENT_SELF-REPORTED_MEDICATION
All tak_*.yaml (FHS might be right but seeing two variations across cohorts)
Collaborator
Author
|
@csiege These are all important QA problems but they are unrelated to this PR. We definitely want to fix these but I'd like to get this in so we revert the PHV rewrite that happened in the bump to FHS v35. I'll make an issue for these other items. We have a couple of other curation issues that I've discovered and made issues for as well. I think these are valuable but I don't want to get side-tracked and risk making more mistakes. |
This was referenced Feb 26, 2026
Collaborator
Author
|
@csiege Good catches — these are all pre-existing in the v35 data and not introduced by this PR (which only fixes the I've captured all three in #366 and added it as a sub-issue of the new epic #367 (QA issues found in FHS-ingest update). We'll address these after this bug-fix PR is merged. |
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.
Summary
associated_participant.populated_fromPHVs in 234 multi-visit blocks across 71 FHS-ingest YAML filesstr()references in uuid5 visit expressions to use the participant PHV (not the visit PHV)Background
The v35 merge (PR #358) systematically changed
associated_participant.populated_fromto matchassociated_visit.populated_fromin multi-visit blocks, overwriting the correct participant PHV with the visit PHV. The uuid5 addition (PR #359) then propagated these wrong PHVs intostr()expressions.Note: pht002889 (phv00172159) is excluded — it had a pre-existing duplicate in v33 that needs separate curator review.
Test plan
validate_ingest_yamls.pypasses all filesstr()uses participant PHV,case()uses visit PHV🤖 Generated with Claude Code