Skip to content

fix: preserve Coding answers during profile-constrained extraction - #1101

Closed
rob-reynolds wants to merge 4 commits into
mainfrom
codex/qr-coding-extraction
Closed

rob-reynolds wants to merge 4 commits into
mainfrom
codex/qr-coding-extraction

Conversation

@rob-reynolds

Copy link
Copy Markdown

When a QuestionnaireResponse contains a Coding answer and its extraction profile constrains an Observation value to CodeableConcept, extraction currently drops the answer. This can leave a subsequent PlanDefinition apply paused despite a supplied answer.

Convert Coding to CodeableConcept before assignment when the assignment parent matches the resource profile and the selected element is unsliced and has exactly one type. Preserve the complete Coding and repeated-property cardinality. Nested Extension and sibling-type-slice regressions prevent borrowing an unrelated profile constraint. Existing unsupported conversions and their diagnostic behavior remain unchanged.

Validation

  • Reproduced the original failure on unmodified main and v4.7.0 baselines.
  • 21 conversion tests across DSTU3, R4 and R5, including 27 real extraction-entry cases, all passing. Cases cover differential/snapshot profiles, both choice-path spellings, metadata, repeated values, Boolean false and unsupported slices/multi-type elements.
  • Combined with the separate generated-ID correction, main CR + utility suites: 3,626 passed, 24 skipped, zero failures/errors. Final test-only assertion rerun: 21 passed.
  • A pinned-4.7 runtime comparison exercised 12 apply calls across original, Coding-only and combined configurations. Corrected configurations pause on missing data, reach the expected affirmative/negative activity after answers, and pause again after clearing an answer. Complete returned values and issue sets were compared.
  • Formatting and main-source Checkstyle passed.

This PR contains only the Coding change. Generated IDs are in a separate stacked PR. The tested v4.7.0 backport is available on codex/qr-extraction-backport; upstream currently has no 4.7 maintenance branch to target. No installed runtime or release was replaced.

Convert Coding to CodeableConcept only when the assignment parent matches an unsliced, single-type resource profile element. Preserve metadata and repeated-property cardinality. Real extraction-entry and Extension slice regressions prevent borrowing root or sibling type constraints. Unsupported nested/multi-type conversions and diagnostics remain unchanged. Validate across DSTU3, R4 and R5.
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Formatting check succeeded!

@JPercival
JPercival added this pull request to stack #1107 September 9, 2026 15:49
if (answerType != null && !answerValue.fhirType().equals(answerType)) {
var newAnswerValue =
request.getAdapterFactory().createBase(newBaseForVersion(answerType, request.getFhirVersion()));
newAnswerValue.setValue(VALUE_PATH, answerValue);

@c-schuler c-schuler Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root cause of the defect occurs on this line. Fortunately, the else branch here already routes through transformValueToResource, which is identity for non-Coding. Making the first branch do the same looks like it'd fix this at the root, cover nested value[x] targets, and let the new setAnswerValue guard drop out. Is there a reason you preferred the call-site guard?

Something like this should work (for reference):

if (answerType != null && !answerValue.fhirType().equals(answerType)) {
    var converted = transformValueToResource(request.getFhirVersion(), answerValue);
    if (converted.fhirType().equals(answerType)) {
        answerValue = converted;                 // Coding->CodeableConcept: now matches
    } else {
        var newAnswerValue = createBase(newBaseForVersion(answerType, ...));
        newAnswerValue.setValue(VALUE_PATH, answerValue);   // unchanged fallback + diagnostics
        answerValue = newAnswerValue.get();
    }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for running the suggestion and relaying the results! Agreed that the actual "root fix" is reliable target-type resolution for nested/extension/slice contexts, not just relocating the call. Can we create a ticket for that?

@barhodes barhodes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to look at adding this into the definition based population branch to avoid merge conflicts down the road.

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@rob-reynolds

Copy link
Copy Markdown
Author

Superseded by #1108, which targets feature-definition-based-population.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants