fix(validation): repair broken run_validation call signatures#372
Open
blakenan-bellese wants to merge 1 commit into
Open
fix(validation): repair broken run_validation call signatures#372blakenan-bellese wants to merge 1 commit into
blakenan-bellese wants to merge 1 commit into
Conversation
The validation-run path (run_validation) drifted out of sync with two refactors in fhir_client.py and crashed every run: 1. wipe_patient_data() gained a required keyword-only `base_url` argument but the call site omitted it → every run failed instantly with "missing 1 required keyword-only argument: 'base_url'". 2. gather_patient_data() now returns a GatherResult dataclass, but the code used the return value as a plain resource list → every patient errored with "'GatherResult' object is not iterable". Fixes mirror the correct usage already present in orchestrator.py: - wipe_patient_data(base_url=settings.MEASURE_ENGINE_URL, strict=False) - resources = gather_result.resources The drift went undetected because TestRunValidation mocked the OLD contracts (gather returning a list, wipe asserted only "called once"). Updated the mocks to return GatherResult and strengthened the wipe assertion to require base_url=MEASURE_ENGINE_URL, so this regression cannot recur silently. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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
1 participant
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
run_validationinbackend/app/services/validation.py) had drifted out of sync with two refactors infhir_client.py, so every validation run crashed — it never produced results.wipe_patient_data()gained a required keyword-onlybase_urlarg; the call site omitted it → run failed instantly withmissing 1 required keyword-only argument: 'base_url'.gather_patient_data()now returns aGatherResultdataclass; the code used it as a plain resource list → every patient errored with'GatherResult' object is not iterable.Both fixes mirror the correct usage already present in
orchestrator.py(base_url=settings.MEASURE_ENGINE_URL, andgather_result.resources).Related issue
Type of change
Root cause / why it wasn't caught
TestRunValidationmocked the old contracts —gather_patient_datareturning alist, and only assertingwipe_patient_datawas "called once". So the unit tests passed against code that could never work at runtime. This PR updates the mocks to returnGatherResultand strengthens the wipe assertion to requirebase_url=MEASURE_ENGINE_URL, so the regression cannot recur silently.Checklist
TestRunValidationmocks + wipe regression guard)Test plan
Run locally (all green):
cd backend && ruff check app/ tests/ && ruff format --check app/ tests/→ cleancd backend && python3 -m pytest tests/ --ignore=tests/integration→ 503 passedUSE_PREBAKED=1 ./scripts/run-integration-tests.sh --ignore=...(the six PR-check ignores) → 29 passed, 7 skippedEnd-to-end: uploaded
CMS1028FHIRPCSevereOBComps(141 test patients) via/validation/upload-bundle, thenPOST /validation/run— previously crashed before any results; now completes and returns per-patient pass/fail/error (43 pass / 2 fail / 96 eval-error). The remaining eval errors are a separate measure-evaluation issue, not part of this fix.🤖 Generated with Claude Code