feat: add hip, ankle/foot, and wrist anatomy prompt templates (#10, #11, #12) - #50
Conversation
Addresses three previously unaddressed issues: - #43: build_prompt() now loads detailed anatomy templates from prompts/anatomy/*.txt (knee/shoulder/spine), with cached lookup and fallback to built-in hints. The shipped templates were dead code before. Adds an abdomen hint so all README-advertised regions are covered. - #44: add `medcheck providers` and `medcheck models` discovery commands. - #42: fix README "Report Bug" link (bug_report.md -> bug_report.yml). Updates CHANGELOG and README; adds unit tests for the new loader and CLI commands (72 passing). https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
…versal - Add missing blank line so `ruff format --check` passes (lint CI was red). - Slug-validate the anatomy region before building a filesystem path in load_anatomy_instructions(), preventing path traversal (CodeRabbit review). - Add regression test for the traversal guard. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
…dd offline provider stub Addresses three High-severity issues: - #26: web server now binds 127.0.0.1 by default (was 0.0.0.0); add optional X-API-Key auth on /api endpoints via MEDCHECK_API_KEY, and warn when binding to a non-loopback host without a key configured. - #28: validate the easyRadiology linkToERI download URL (HTTPS + host allowlist) and disable redirects to prevent SSRF to internal/metadata hosts. - #36: add LocalLLMProvider stub so the advertised "local" offline fallback degrades gracefully (check_available()=False, actionable NotImplementedError) instead of crashing with "No LLM provider available"; register it in the vision router and `medcheck models`. Updates README, .env.example, CHANGELOG; adds unit tests for all three (89 passing, coverage 66%). https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
…validation Addresses four Medium/High-severity privacy issues: - #27: gate external (cloud) LLM transmission behind explicit consent (--allow-cloud-llm flag, MEDCHECK_ALLOW_EXTERNAL_LLM env, interactive prompt); vision_analysis raises instead of silently sending PHI. - #29: stop logging the raw patient name to stdout — log a non-reversible hash of the patient ID instead. - #30: stop echoing portal access codes into ValueError messages. - #33: add a Pydantic AnalyzeRequest schema with validation for POST /api/analyze (auth was already added in #46). Docs: SECURITY.md "Handling of Patient Data" section, README + .env.example. Tests: +5 (consent gate, credential redaction, request validation); 93 passing. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
- ingest: stop logging study description (free-text DICOM field may contain PHI) - vision_analysis: honour explicit LLM provider preference (wires up --model) instead of hardcoding "claude"; default to on-device "local" without consent so offline execution stays reachable - docs: soften compliance/anonymity wording (drop "(HIPAA/GDPR)" assertion and "non-reversible" claim; describe pseudonymisation accurately) 94 tests passing. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
Consent (--allow-cloud-llm / MEDCHECK_ALLOW_EXTERNAL_LLM) now only permits falling back to an external provider; it no longer flips the implicit default from on-device 'local' to 'claude'. Addresses CodeRabbit review on #47. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
…larity Addresses four Medium-severity reliability/supply-chain issues: - #38: add configurable timeout (MEDCHECK_LLM_TIMEOUT) + retry with exponential backoff (MEDCHECK_LLM_RETRIES) to all three LLM providers via a shared call_with_retries helper; transient failures no longer crash the pipeline and surface as a clear LLMProviderError. - #39: run Docker containers as a non-root 'medcheck' user. - #37: vendor htmx locally (src/medcheck/web/static/htmx.min.js) with an SRI hash instead of loading from the unpkg CDN; air-gapped friendly. - #34: clarify the misleading bandit nosec on the Crypto import (pycryptodome, not pycrypto; B413 can't distinguish the shared namespace) and add an AES-CBC round-trip test that verifies the decryption path. 102 tests passing; coverage 67%. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
- base: only retry transient errors (timeouts/connection/429/5xx), fail fast on permanent ones (auth/4xx); add is_transient_error() helper - claude/openai: pass max_retries=0 so call_with_retries is the sole retry controller (avoids SDK retry stacking) - remove unused `# noqa: BLE001` directive (RUF100 lint failure) - add fake-SDK provider tests (claude/openai/gemini happy path + missing key), lifting LLM provider patch coverage to ~100% (fixes codecov/patch) - web test now also asserts the htmx SRI integrity + crossorigin attributes 113 tests passing. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
…rror) Coerce the attempt count to >=1 so the loop always runs once and failures wrap as LLMProviderError instead of raising UnboundLocalError. Addresses CodeRabbit review on #48. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
authenticate() required a non-empty dob, implying DOB verification — but dob is never sent to the portal or checked by this client. Gating on it created a false security boundary. Authentication now relies on the access code only; dob remains accepted but is documented as not verified. Updates README + .env.example to stop overstating DOB verification. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
Closes #10, #11, #12. Adds detailed MRI analysis templates (modelled on knee.txt) that the prompt loader picks up automatically, and extends detect_anatomy() with hip/ankle/wrist keyword patterns (incl. German terms). Removes the over-broad "gelenk" keyword from the knee pattern so it no longer mis-matches Handgelenk (wrist) / Sprunggelenk (ankle); "Knie" still matches knee. 115 tests passing. https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
📝 WalkthroughWalkthroughThis PR extends the MedCheck anatomy analysis system to recognize and analyze hip, ankle/foot, and wrist MRI scans. It adds keyword patterns to enable detection, introduces three detailed anatomical analysis prompt templates, and includes comprehensive test coverage validating both keyword detection and template loading. ChangesAnatomy Templates and Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/medcheck/pipeline/preprocess.py (2)
22-22: 💤 Low valueThe "carpal" keyword may match metacarpal scans.
"Metacarpal" (hand/finger bones) contains "carpal" and would incorrectly classify as wrist. If the system processes hand/finger MRIs separately, consider using a word boundary anchor or a more specific pattern.
♻️ Proposed refinement using word boundaries
- (r"wrist|handgelenk|carpal|tfcc|scaphoid", "wrist"), + (r"wrist|handgelenk|\bcarpal\b|tfcc|scaphoid", "wrist"),Alternatively, if metacarpal scans are rare or always labeled with more specific terms (e.g., "metacarpophalangeal"), the current pattern may be acceptable.
🤖 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 `@src/medcheck/pipeline/preprocess.py` at line 22, The regex mapping that maps ("wrist") currently includes "carpal" which will match "metacarpal"; update the pattern in the tuple that contains (r"wrist|handgelenk|carpal|tfcc|scaphoid", "wrist") to use word-boundary anchors (e.g., \bcarpal\b or wrap each wrist token with \b...\b) or otherwise make "carpal" more specific so it won't match "metacarpal"; locate and modify this pattern in preprocess.py where the wrist mapping tuple is defined (the tuple with the regex and "wrist") and adjust the regex accordingly.
21-21: ⚡ Quick winTighten the generic
footmatch in the ankle regex (line 21)Repo-wide search only finds the
foottoken in this regex (no other description examples to gauge actual DICOM wording), so false-positive risk can’t be quantified here—but it’s still safer to restrictfoot(e.g., to a whole-word/anchored match) to avoid mapping more specific terms like “forefoot/midfoot” or “foot_*” into ankle.🤖 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 `@src/medcheck/pipeline/preprocess.py` at line 21, The current ankle mapping regex tuple (r"ankle|sprunggelenk|achilles|foot|fuß|fuss|calcaneus|hindfoot", "ankle") is too permissive for the token "foot" and can false-match terms like "forefoot" or "midfoot"; update that regex in src/medcheck/pipeline/preprocess.py (the ankle mapping tuple) to restrict "foot" to a whole-word match (e.g., replace the plain foot/füß/fuss token with a word-boundary form such as \bfoot\b and similarly \bfuß\b/\bfuss\b in the raw string) so only standalone “foot” maps to ankle while preserving the other alternatives.
🤖 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.
Nitpick comments:
In `@src/medcheck/pipeline/preprocess.py`:
- Line 22: The regex mapping that maps ("wrist") currently includes "carpal"
which will match "metacarpal"; update the pattern in the tuple that contains
(r"wrist|handgelenk|carpal|tfcc|scaphoid", "wrist") to use word-boundary anchors
(e.g., \bcarpal\b or wrap each wrist token with \b...\b) or otherwise make
"carpal" more specific so it won't match "metacarpal"; locate and modify this
pattern in preprocess.py where the wrist mapping tuple is defined (the tuple
with the regex and "wrist") and adjust the regex accordingly.
- Line 21: The current ankle mapping regex tuple
(r"ankle|sprunggelenk|achilles|foot|fuß|fuss|calcaneus|hindfoot", "ankle") is
too permissive for the token "foot" and can false-match terms like "forefoot" or
"midfoot"; update that regex in src/medcheck/pipeline/preprocess.py (the ankle
mapping tuple) to restrict "foot" to a whole-word match (e.g., replace the plain
foot/füß/fuss token with a word-boundary form such as \bfoot\b and similarly
\bfuß\b/\bfuss\b in the raw string) so only standalone “foot” maps to ankle
while preserving the other alternatives.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 357f4b29-8ce5-425c-9e0a-0ee58e9fbc2d
📒 Files selected for processing (7)
CHANGELOG.mdsrc/medcheck/pipeline/preprocess.pysrc/medcheck/prompts/anatomy/ankle.txtsrc/medcheck/prompts/anatomy/hip.txtsrc/medcheck/prompts/anatomy/wrist.txttests/unit/test_pipeline/test_preprocess.pytests/unit/test_pipeline/test_vision_analysis.py
Closes
Changes
Adds three detailed MRI analysis templates, modelled on the existing
knee.txt, which the prompt loader (load_anatomy_instructions(), added in #43) picks up automatically:prompts/anatomy/hip.txt— labrum, cartilage, FAI (cam/pincer), AVN, gluteal/iliopsoas tendons, bursaeprompts/anatomy/ankle.txt— Achilles, peroneal & tib-post tendons, ATFL/CFL/deltoid ligaments, talar OCD, plantar fasciaprompts/anatomy/wrist.txt— TFCC, SL/LT ligaments, scaphoid/lunate (AVN), tendons, carpal tunnel, DRUJExtends
detect_anatomy()with hip/ankle/wrist keyword patterns (including German terms: Hüfte, Sprunggelenk, Handgelenk).Note: removed the over-broad
gelenkkeyword from the knee pattern — it previously would have mis-matchedHandgelenk(wrist) andSprunggelenk(ankle).Knie/kneestill match knee, and the existing"Gelenke^Knie"test case still passes.Validation
ruff✅ ·mypy✅ · 115 tests pass (+5: detect_anatomy for hip/ankle/wrist + template-loading assertions)https://claude.ai/code/session_01KYcUxCGAHaTPrRHTzJCWQz
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests