[paynejd]

closes OpenConceptLab/ocl_issues#3030 (part of OpenConceptLab/ocl_issues#3029)

A supplement source can now say which code system it supplements, and FHIR CodeSystem reads, writes and searches carry it. A supplement's FHIR read no longer gains a made-up display, an extra English designation, or concept class and datatype properties. The first users are French and Brazilian Portuguese ICD-10 supplements, shaped like HL7 Denmark's http://hl7.dk/fhir/core/CodeSystem/icd10-da on tx.fhir.org.

This PR adds a migration: sources/0047_source_supplements adds three nullable columns to sources (no default, no data migration). Reversing it drops them.

Changes

• Source (core/sources/models.py): new fields supplements (canonical URL, optionally |version), supplement_type and supplement_global. clean_supplements requires supplements when content_type is supplement (FHIR csd-4) and allows at most one non-empty |version; the URL itself goes through URIValidator like canonical_url. New versions copy all three from HEAD. Source.is_supplement is the shared check.
• REST (core/sources/serializers.py): create, update and read carry the three fields on sources and source versions. The bulk importer's SourceImporter accepts them.
• FHIR CodeSystem (core/code_systems/serializers.py):
  • supplements, and the codesystem-supplement-type (valueCode) and codesystem-supplement-global (valueBoolean) extensions, on read and write.
  • A supplement's concepts (CodeSystemSupplementConceptSerializer) are a code and its names, plus inactive when retired. No conceptclass or datatype on the concepts or in the CodeSystem's property list.
  • Write: data['display'] is no longer read unchecked, so a concept with designations and no display loads. For a supplement nothing is added. A display that is sent becomes the preferred name in the CodeSystem's language. If that name also came as a designation with its own use, it is still listed as a designation.
  • A supplement stays content: supplement past FHIR_CODE_SYSTEM_CONCEPT_LIMIT; its concepts are left out and the read is tagged SUBSETTED.
• FHIR search (core/fhir/search.py): /fhir/CodeSystem/ takes supplements (a URL matches any version of it; url|version matches that version only, as tx.fhir.org does) and content-mode (the content the read shows; system http://hl7.org/fhir/codesystem-content-mode). Both are listed in the CapabilityStatement and Swagger.

Tests

• New: SourceSupplementsTest (REST create/update/read/versions and validation), CodeSystemSupplementTest (FHIR write and read, an update, the concept limit, designations without a display), CodeSystemSupplementSearchTest (both search parameters), and a bulk importer test.
• Full suite locally, run as CI runs it (PYTHONHASHSEED=2): 3,224 tests with TEST_ES=off --exclude-tag es --parallel=4, then 630 with --tag es, all OK. pylint -j0 core/ 10.00, no messages.

Decisions to confirm

1. supplements is required only for content_type: supplement, and any other source may set it. FHIR's csd-4 only goes that way.
2. The supplement type and global flag are two more Source fields, supplement_type and supplement_global, kept as the publisher gives them. Nothing applies them yet; whether a global flag can affect another owner's code system is D4 in Supplements: decide how OCL finds, scopes, turns on and merges supplements ocl_issues#3033.
3. A supplement concept's display is its preferred name in the CodeSystem's language, and that name isn't repeated as a designation. FHIR defines the display as the designation in the resource language. Without one, the read has designations only, like HL7 Denmark's supplement.
4. A supplement over the concept limit (1,000) stays supplement, its concepts left out and tagged SUBSETTED, instead of not-present. ICD-10 supplements have about 10,000 concepts, so their FHIR read won't list concepts, the same as the base ICD-10 today.
5. content-mode matches the content the read shows, so a large non-supplement code system is not-present, not its stored mode.
6. A non-supplement concept with designations and no display gets no extra name (before, this was a 500). A concept with no names at all still gets its code as an English name, as before.
7. supplements is matched as text. Checking that it resolves, and that a supplement's codes exist in it, is Supplements: check a supplement against the code system it supplements ocl_issues#3032.

Not in this PR

• A supplement concept with neither a display nor a designation (properties only) is refused with a 400: OCL requires a concept to have a name, and the write doesn't invent one.
• The FHIR CodeSystem write still ignores a concept's save errors for any other reason, as before this PR. Fixing that means making the write one transaction.
• supplements isn't indexed in Elasticsearch, and the REST source list doesn't filter on it (D1 in Supplements: decide how OCL finds, scopes, turns on and merges supplements ocl_issues#3033).
• Other extensions on a written CodeSystem are still dropped, as before.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AaZ4qTHAarCyLb2cuys8Yd

[paynejd]

Codex review, pass 1 (codex-cli 0.169.0, commit 0ac202d)

Verdict: findings (2 P1).

• F1.1 (P1) core/code_systems/serializers.py:117: Property-only supplement concepts are silently discarded during FHIR import.
• F1.2 (P1) core/code_systems/serializers.py:151: Removing the display name also removes explicitly supplied designation-use metadata.

Responses

Fixed in 99ccf1e:

• F1.1: fixed. A supplement concept with neither a display nor a designation is now refused with a 400, before anything is saved, instead of being dropped by the name check after the write reports success. A supplement concept that carries only properties can't be stored until OCL allows a concept without a name, which is outside this ticket (listed under "Not in this PR"). Test: test_a_supplement_concept_without_a_name_is_refused_not_dropped. The write also ignores concept errors for other reasons (create() and update() don't read concept.errors). That's older than this PR; fixing it means making the write one transaction, so it's left out here.
• F1.2: fixed. When the display's name has a use of its own, it also stays in the designations; a name that only came from the display isn't repeated. Test: test_a_designation_with_a_use_is_kept_when_it_is_also_the_display.

findings.json

{
  "reviewer": "codex-cli 0.169.0",
  "perspective": "defects",
  "mode": "full",
  "commit": "0ac202de",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "F1.1",
      "severity": "P1",
      "tag": null,
      "file": "core/code_systems/serializers.py",
      "line": 117,
      "claim": "Property-only supplement concepts are silently discarded during FHIR import.",
      "scenario": "POST a supplement containing a concept with code and property inactive=false, without display or designations. This branch leaves names empty. BasicConceptValidator rejects the concept during Concept.persist_new(), which deletes it and returns errors. CodeSystemDetailSerializer.create() ignores those errors, so the request reports success and publishes a version missing the concept.",
      "action": "Allow nameless supplement concepts through the model validator without inventing names, propagate concept persistence errors, and add an import regression test.",
      "lines_delta": null,
      "confidence": "high"
    },
    {
      "id": "F1.2",
      "severity": "P1",
      "tag": null,
      "file": "core/code_systems/serializers.py",
      "line": 151,
      "claim": "Removing the display name also removes explicitly supplied designation-use metadata.",
      "scenario": "Write a French supplement concept with display='Choléra' and designation={language:'fr', value:'Choléra', use:{code:'synonym'}}. add_preferred_name() promotes that designation, and this filter removes it from the read response. Only display remains, losing the explicit synonym designation; writing that response back loses its stored use too.",
      "action": "Retain designations carrying a distinct explicit use when their text also supplies display. Add a round-trip test covering matching display and typed designation values.",
      "lines_delta": null,
      "confidence": "high"
    }
  ]
}

[paynejd]

Codex review, pass 2 (codex-cli 0.169.0, commit 99ccf1e)

Verdict: clean. No findings.

Earlier findings: F1.1 resolved, F1.2 resolved.

findings.json

{
  "reviewer": "codex-cli 0.169.0",
  "perspective": "defects",
  "mode": "verify",
  "commit": "99ccf1e8",
  "verdict": "clean",
  "prior": [
    {
      "id": "F1.1",
      "status": "resolved"
    },
    {
      "id": "F1.2",
      "status": "resolved"
    }
  ],
  "findings": []
}

[paynejd]

Simplification review (fresh Claude subagent, commit 99ccf1e)

Verdict: findings (1 dup-test, 1 shrink, 1 yagni).

• S1 (yagni) core/fhir/search.py:249: with_concepts_over_limit() is now a one-line wrapper with a single caller (line ~621). The PR split it from the new concepts_over_limit() only so by_content_mode could reuse the Exists expression, and the wrapper adds nothing on top.
• S2 (shrink) core/sources/models.py:718: clean_supplements has an early return and a nested if, just to separate 'missing' from 'malformed'.
• S3 (dup-test) core/sources/tests/tests.py:851: test_supplements_is_optional_for_other_content makes a whole POST to pin one boolean branch of clean_supplements. The same branch is already hit by the other tests, since every non-supplement source created in the existing suite leaves supplements empty. The test also needs no setup that the sibling tests lack.

Responses

• S1: applied (febca13). with_concepts_over_limit had one caller left; the search annotates with concepts_over_limit() directly.
• S2: declined. The early return says plainly that a source without supplements has nothing more to check. The proposed form saves three lines but not much reading.
• S3: declined. This is the only test of decision 1, a non-supplement source that does set supplements. Other tests leave the field empty, which is a different branch.

findings.json

{
  "reviewer": "claude subagent",
  "perspective": "simplify",
  "mode": "full",
  "commit": "99ccf1e8",
  "verdict": "findings",
  "prior": [],
  "findings": [
    {
      "id": "S1",
      "severity": null,
      "tag": "yagni",
      "file": "core/fhir/search.py",
      "line": 249,
      "claim": "with_concepts_over_limit() is now a one-line wrapper with a single caller (line ~621). The PR split it from the new concepts_over_limit() only so by_content_mode could reuse the Exists expression, and the wrapper adds nothing on top.",
      "scenario": null,
      "action": "Delete with_concepts_over_limit and at its one call site write rows = rows.annotate(concepts_over_limit=concepts_over_limit()).",
      "lines_delta": -5,
      "confidence": "medium"
    },
    {
      "id": "S2",
      "severity": null,
      "tag": "shrink",
      "file": "core/sources/models.py",
      "line": 718,
      "claim": "clean_supplements has an early return and a nested if, just to separate 'missing' from 'malformed'.",
      "scenario": null,
      "action": "self.supplements = self.supplements or None\nif self.is_supplement and not self.supplements:\n    raise ValidationError({'supplements': [SUPPLEMENTS_REQUIRED]})\nversion = (self.supplements or '').split('|')[1:]\nif len(version) > 1 or version == ['']:\n    raise ValidationError({'supplements': [SUPPLEMENTS_FORMAT]})",
      "lines_delta": -3,
      "confidence": "low"
    },
    {
      "id": "S3",
      "severity": null,
      "tag": "dup-test",
      "file": "core/sources/tests/tests.py",
      "line": 851,
      "claim": "test_supplements_is_optional_for_other_content makes a whole POST to pin one boolean branch of clean_supplements. The same branch is already hit by the other tests, since every non-supplement source created in the existing suite leaves supplements empty. The test also needs no setup that the sibling tests lack.",
      "scenario": null,
      "action": "Fold its single assertion (a plain source created with supplements returns 201 and keeps the value) into test_create_update_read_and_versions_carry_supplements, or drop it. Nothing else replaces it.",
      "lines_delta": -6,
      "confidence": "low"
    }
  ]
}

[paynejd]

Codex check of commits 99ccf1e..febca13 (codex-cli 0.169.0)

Verdict: clean. No findings.

findings.json

{
  "reviewer": "codex-cli 0.169.0",
  "perspective": "defects",
  "mode": "final",
  "commit": "febca130",
  "verdict": "clean",
  "prior": [],
  "findings": []
}

[paynejd]

@snyaggarwal this is ready for review. Jon asks you to merge it if you approve; he knows you're working this weekend.

• Migration: this PR adds sources/0047_source_supplements, a new Source field supplements plus supplement_type and supplement_global. All three are nullable columns with no default and no data migration; reversing it drops them.
• Merge order: none needed. It's independent of oclapi2#994 (FHIR: send OCL locales as BCP-47 language tags ocl_issues#3031), and the two branches merge cleanly in either order. A trial merge of both passed core.fhir, core.code_systems, core.concept_maps and the source tests locally (962 tests).
• Review: Codex pass 1 found two P1s (a supplement concept with no name was dropped without an error; a designation that is also the display lost its use). Both are fixed and pass 2 was clean. The simplification review led to one cut, and the final Codex check of that cut was clean. CI is green.
• Decisions to confirm are in the PR description.