Fix Pre-Eclampsia field printing boolean instead of YES/NO on ANC label - #898
Open
Hastings2004 wants to merge 253 commits into
Open
Hastings2004 wants to merge 253 commits into
Hastings2004 wants to merge 253 commits into
Conversation
# Issue 1. Concept references returned by screened_today method were not matching the hardcoded strings in build_report `methods` # Fix 1. added condition to only retrieved short concept names insead of fully specified 2. formatting references between screening methods to use lower case
…_coded filter in pregnant/breastfeeding queries
maternal_status.rb:
- load_pregnant_women / load_breast_feeding: replace LEFT JOIN self-join
on obs (find-latest-per-group anti-pattern, O(N²)) with INNER JOIN on
a derived table using MAX(obs_datetime) GROUP BY person_id
cohort_builder.rb:
- total_pregnant_women / total_breastfeeding_women:
- Add FORCE INDEX (idx_obs_fast_lookup) to guarantee tpo-first join order
(without it MySQL chose enc-first when value_coded is in JOIN ON,
causing catastrophic 943s vs the original 160s/144s)
- Move value_coded = 1065 from post-GROUP-BY HAVING into the obs JOIN ON
condition so MySQL applies it as an index push-down during the obs scan
- Remove unnecessary ORDER BY obs.obs_datetime DESC
Benchmark result: 160s -> 11s and 144s -> 2s (-93% / -99%)
Performance improvements: - cohort_builder.rb: parallelise phase-1 temp-table loads (4 threads) with race-condition-safe ordering (other_patient_types → register_start_date → order_details in thread 1; art_start_date, reason_for_art, enrollment start_date in parallel threads 2-4) - cohort_builder.rb: FORCE INDEX hints on hot obs/orders queries - cohort_builder.rb: load_tmp_max_adherence driven from temp_patient_outcomes (~25k rows) instead of full obs concept scan - cohort_builder.rb: load_temp_obs_last_visit precomputed in background thread for near-instant pregnant/breastfeeding lookups - cohort_builder.rb: latest_art_adherence two-query approach with FORCE INDEX - cohort_builder.rb: update_cummulative_outcomes parallelised (2 passes) - cohort_builder.rb: load_max_drug_orders FORCE INDEX (57s→17s) - cohort_builder.rb: update_patient_current_medication staged parallel INSERTs - cohort_builder.rb: total_patients_on_arvs_and_ipt/cpt JOIN rewrite - cohort_builder.rb: side-effects via temp_last_se_visit precomputation - outcomes.rb: load_patient_current_medication DATE comparison fix - side_effects.rb: parallel side-effect loading Correctness fixes (all 148 indicators match dev branch baseline exactly): - load_art_start_date: remove stray voided/IS NOT NULL filters; use obs_datetime < cutoff (not value_datetime) - load_patient_current_medication: DATE(o.start_date) = DATE(mdo.start_date) fixes unknown_regimen over-count (+129) - load_phase1_parallel: move load_temp_other_patient_types into thread 1 chain to eliminate race condition (fixes total_registered -24 + cascade) - latest_art_adherence: two-query not_adherent-first approach so patients with mixed obs at same visit are correctly classified as not_adherent - load_tmp_max_adherence: remove 4-year lower bound (was excluding 7 patients whose last valid ARV-linked adherence obs predated the window) - load_temp_obs_last_visit: remove 1-year pre-filter; drive directly from temp_max_drug_orders.start_date (fixes breastfeeding -1) - load_temp_reason_for_starting_art: MAX(date_created) tiebreaker matches dev ORDER BY obs_datetime DESC, date_created DESC LIMIT 1 exactly - pregnant_females_all_ages: revert re_initiated_check to stored function call to preserve patient_date_enrolled() enrollment-period guard Add bin/verify_cohort_report.rb: snapshot/compare tool for regression testing Add bin/benchmark_cohort.rb: timing benchmark script
- Add CohortProgress service (app/services/cohort_progress.rb)
- File-backed store under tmp/cohort_progress/<key>.json
- Atomic writes via tmp+rename (thread-safe, race-condition free)
- 13 named stages with monotonically increasing pct weights
- key() is name-only (MD5) so writer and reader always match
- Instrument cohort_builder.rb with CohortProgress.step! at each stage:
prepare(0%) > phase1(2%) > enroll(14%) > demographics(19%) >
cum_outcome(24%) > preloads(49%) > outcomes(55%) > regimens(60%) >
side_effects(70%) > adherence(78%) > preg_bf(90%) > tpt_fp_bp(95%) > done(100%)
concurrent regenerate while a job is already running)
- Add cohort_progress collection action to ProgramReportsController
returns { step, label, pct, done, elapsed_seconds }
- Add GET /api/v1/programs/:program_id/reports/cohort_progress route
as resources collection action (avoids :id conflict)
…id lock table overflow INSERT INTO ... SELECT under REPEATABLE READ acquires shared next-key locks on every scanned obs/orders row. On large obs tables this exceeds the InnoDB lock table capacity (bounded by innodb_buffer_pool_size), causing: Mysql2::Error: The total number of locks exceeds the lock table size Fix: switch the session to READ UNCOMMITTED before the INSERT and restore REPEATABLE READ in an ensure block. This is safe because the obs/orders data is not modified concurrently during the cohort run - it is a read-only reporting scan over stable data.
Maria drugs
Enhance LIMS integration and improve drug management
Update OpenMRS Metadata
…b identifiers to prevent duplicate notifications
Fix cohort data issues
Pre-Eclampsia was missing the boolean-to-text ternary that other history fields (Asthma, Diabetes, Epilepsy) already use, causing raw true/false to print on the visit summary label instead of YES/NO.
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
7 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.
PR Description
Problem
The Pre-Eclampsia field on the ANC visit history label was printing the literal text
"true"or"false"instead of"YES"/"NO", regardless of the actual patient value.Root Cause
In
app/services/anc_service/patient_history_label.rb, every other history field (Asthma, Diabetes, Epilepsy, etc.) converts its value using a ternary that maps to"NO"/"YES"text. The Pre-Eclampsia field was missing this conversion — it evaluated a boolean comparison directly and passed the rawtrue/falseresult to.to_s, so the boolean itself got printed instead of a readable label.Fix
? "NO" : "YES"ternary used by the other fields, so Pre-Eclampsia now converts correctly."YES"regardless of the error) with a neutral"-"default.Impact
Small, isolated fix — one method, no changes to data or business logic elsewhere. Brings Pre-Eclampsia in line with the existing pattern used by all other fields in the same label.