fix LDL explainer text formatting#38
Conversation
github-project-automation
Bot
moved this to In Progress
in MyHeart Counts Project Planning
📝 WalkthroughWalkthroughThe English and Spanish LDL explanations were reformatted into Markdown tables, preserving thresholds, scores, and descriptions; the Spanish text also received a typo correction. The English and Spanish blood pressure explanations were converted to tables listing category thresholds and added a note for differing systolic and diastolic categories. The study definition revision increased from 41 to 42. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids`+en-US.md:
- Around line 11-19: Make the LDL units explicit for every range in the tables
at
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+en-US.md
lines 11-19 and
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+es-US.md
lines 11-19 by adding mg/dL to the Range column header and removing it from
individual cells, or by including mg/dL consistently in every range; apply the
same approach in both language files.
- Around line 10-12: Add a blank line between the introductory paragraph and the
Markdown table in BloodLipids+en-US.md at lines 10-12, and apply the equivalent
spacing change in BloodLipids+es-US.md at lines 10-12.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9654c2a3-51e8-4572-9ca4-242a87c2f511
📒 Files selected for processing (2)
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+en-US.mdSources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+es-US.md
| Our scoring system helps you see your risk at a glance: | ||
| | Range | Score | What it means | | ||
| | :--------: | :---: | :--------------------------------------------------------------- | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a blank line before each Markdown table.
The table currently follows the introductory paragraph directly, triggering markdownlint MD058 and risking inconsistent rendering across Markdown processors.
Proposed fix
Our scoring system helps you see your risk at a glance:
+
| Range | Score | What it means |Apply the equivalent blank line in the Spanish resource.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Our scoring system helps you see your risk at a glance: | |
| | Range | Score | What it means | | |
| | :--------: | :---: | :--------------------------------------------------------------- | | |
| Our scoring system helps you see your risk at a glance: | |
| | Range | Score | What it means | | |
| | :--------: | :---: | :--------------------------------------------------------------- | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 11-11: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
📍 Affects 2 files
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+en-US.md#L10-L12(this comment)Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+es-US.md#L10-L12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids`+en-US.md
around lines 10 - 12, Add a blank line between the introductory paragraph and
the Markdown table in BloodLipids+en-US.md at lines 10-12, and apply the
equivalent spacing change in BloodLipids+es-US.md at lines 10-12.
Source: Linters/SAST tools
| | Range | Score | What it means | | ||
| | :--------: | :---: | :--------------------------------------------------------------- | | ||
| | <55 mg/dL | 100 | Excellent; maximal protection, especially for high-risk patients | | ||
| | 55–69 | 90 | Very good; ideal for heart disease patients | | ||
| | 70–99 | 80 | Optimal for most adults | | ||
| | 100–129 | 60 | Slightly above ideal; risk starts to rise | | ||
| | 130–159 | 40 | High risk; consider lifestyle changes or medication | | ||
| | 160–189 | 20 | Very high risk | | ||
| | ≥190 mg/dL | 0 | Severe risk; urgent action needed | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the LDL units explicit for every range.
Only the first and last rows include mg/dL; readers may interpret the middle ranges as unitless. Put the unit in the column header and remove it from individual cells, or include it consistently in every range.
🧰 Tools
🪛 LanguageTool
[style] ~14-~14: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...-risk patients | | 55–69 | 90 | Very good; ideal for heart disease patients ...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.23.0)
[warning] 11-11: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
📍 Affects 2 files
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+en-US.md#L11-L19(this comment)Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+es-US.md#L11-L19
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids`+en-US.md
around lines 11 - 19, Make the LDL units explicit for every range in the tables
at
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+en-US.md
lines 11-19 and
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodLipids+es-US.md
lines 11-19 by adding mg/dL to the Range column header and removing it from
individual cells, or by including mg/dL consistently in every range; apply the
same approach in both language files.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.md (1)
2-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the source that actually defines these blood pressure categories.
The listed numbers match the AHA/ACC 2017 hypertension guideline thresholds, while the linked WHO fact sheet generally defines hypertension as ≥140/90 mmHg and does not define “Elevated”, “Hyportension stage 1”, or “Hypertension stage 2” with these exact ranges. Update both
BloodPressure+en-US.mdandBloodPressure+es-US.mdto cite AHA/ACC (or another source that clearly defines these categories) instead of the WHO hypertension page.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure`+en-US.md around lines 2 - 19, The blood pressure category thresholds cite an incompatible WHO source. Update the links in BloodPressure+en-US.md lines 2-19 and BloodPressure+es-US.md lines 2-21 to cite the AHA/ACC 2017 guideline, or another authoritative source that explicitly defines the listed Normal, Elevated, Hypertension stage 1, and Hypertension stage 2 ranges; leave the category content unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure`+en-US.md:
- Around line 12-19: Clarify the blood-pressure category rules in both locale
documents: update
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.md
(lines 12-19) and
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+es-US.md
(lines 14-21) to state that Normal and Elevated require both systolic and
diastolic thresholds, while Hypertension stage 1 and stage 2 apply when either
threshold is met.
---
Outside diff comments:
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure`+en-US.md:
- Around line 2-19: The blood pressure category thresholds cite an incompatible
WHO source. Update the links in BloodPressure+en-US.md lines 2-19 and
BloodPressure+es-US.md lines 2-21 to cite the AHA/ACC 2017 guideline, or another
authoritative source that explicitly defines the listed Normal, Elevated,
Hypertension stage 1, and Hypertension stage 2 ranges; leave the category
content unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 043246b9-bd7c-4a86-8c0d-ffb80c3696fd
📒 Files selected for processing (2)
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.mdSources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+es-US.md
| | Category | Systolic (mmHg) | Diastolic (mmHg) | | ||
| | :------------------- | :-------------: | :--------------: | | ||
| | Normal | <120 | <80 | | ||
| | Elevated | 120–129 | <80 | | ||
| | Hypertension stage 1 | 130–139 | 80–89 | | ||
| | Hypertension stage 2 | ≥140 | ≥90 | | ||
|
|
||
| _If your two numbers fall into different categories, the higher category applies._ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg 'BloodPressure\+.*(en-US|es-US)\.md$' || true
echo
echo "== en-US file =="
file_en="$(git ls-files | rg 'Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure\+en-US\.md$' | head -n1 || true)"
if [ -n "${file_en:-}" ]; then
cat -n "$file_en"
fi
echo
echo "== es-US file =="
file_es="$(git ls-files | rg 'Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure\+es-US\.md$' | head -n1 || true)"
if [ -n "${file_es:-}" ]; then
cat -n "$file_es"
fi
echo
echo "== nearby blood pressure explainer files =="
git ls-files 'Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure*.md' | sort
echo
echo "== search for blood pressure threshold sources/wording in repo =="
rg -n --hidden -i 'Blood Pressure|hypertension|120–129|stage 1|stage 2|WHO|AHA|ACC|higher category|se aplica la categoría' Sources || trueRepository: SchmiedmayerLab/MyHeartCounts-StudyDefinitions
Length of output: 44844
🌐 Web query:
AHA ACC blood pressure categories normal elevated stage 1 stage 2 higher number applies table
💡 Result:
The American Heart Association (AHA) and American College of Cardiology (ACC) classify blood pressure for adults into four distinct categories based on average office blood pressure readings [1][2]. When systolic (the top number) and diastolic (the bottom number) readings fall into two different categories, the higher category is used for classification [3][4]. Blood Pressure Categories Category | Systolic (mm Hg) | | Diastolic (mm Hg) --- | --- | --- | --- Normal | Less than 120 | and | Less than 80 Elevated | 120–129 | and | Less than 80 Stage 1 Hypertension | 130–139 | or | 80–89 Stage 2 Hypertension | 140 or higher | or | 90 or higher In addition to these categories, severe hypertension and hypertensive emergency are defined as a systolic pressure higher than 180 mm Hg and/or a diastolic pressure higher than 120 mm Hg [5][6]. If a patient presents with these readings and symptoms such as chest pain, shortness of breath, back pain, numbness, weakness, vision changes, or difficulty speaking, immediate medical attention is required [5][7]. Clinical guidelines emphasize that individuals with readings in two different categories should always be designated to the higher, more serious category [3][4]. Hypertension treatment decisions are based not only on these category assignments but also on the patient's overall cardiovascular risk profile, including comorbidities and target organ damage [1][8].
Citations:
- 1: https://www.ahajournals.org/doi/10.1161/CIR.0000000000001356
- 2: https://www.ahajournals.org/doi/10.1161/HYP.0000000000000065
- 3: https://www.ahajournals.org/doi/10.1161/HYP.0000000000000066
- 4: https://www.jacc.org/doi/10.1016/j.jacc.2017.11.006
- 5: https://www.heart.org/en/health-topics/high-blood-pressure/understanding-blood-pressure-readings
- 6: https://www.heart.org/en/health-topics/high-blood-pressure/the-facts-about-high-blood-pressure/high-bp-top-10
- 7: https://www.heart.org/-/media/files/health-topics/high-blood-pressure/hbp-rainbow-chart-english.pdf
- 8: https://reference.medscape.com/cc2/p10/guideline-aha-acc-detect-manage-high-blood-pressure-2026a10008xo
Clarify the logical operator for each blood-pressure category in both locales.
Normal and elevated categories require both systolic and diastolic thresholds; stage 1 and 2 apply when either threshold is met. State this rule explicitly in Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.md and the equivalent Spanish text in Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+es-US.md.
📍 Affects 2 files
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.md#L12-L19(this comment)Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+es-US.md#L14-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure`+en-US.md
around lines 12 - 19, Clarify the blood-pressure category rules in both locale
documents: update
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+en-US.md
(lines 12-19) and
Sources/MHCStudyDefinitionExporter/Resources/hhdExplainer/BloodPressure+es-US.md
(lines 14-21) to state that Normal and Elevated require both systolic and
diastolic thresholds, while Hypertension stage 1 and stage 2 apply when either
threshold is met.
PSchmiedmayer
left a comment
PSchmiedmayer
left a comment
There was a problem hiding this comment.
Good to merge; Coderabbit comments might be considered if you want to.
2 participants
♻️ Current situation & Problem
issue: the LDL explainer text currently renders the list of ranges/scores/etc as a single paragraph, which doesn't exactly look pretty.
⚙️ Release Notes
📚 Documentation
n/a
✅ Testing
n/a
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: