Issue #1223: Define data portability implementation plan

[cbeach47]

Description of Change

Adds the implementation plan for epic #1223 (MDR import/export portability) as a proposal doc.
No code changes — this is the design and ticket-triage work that the epic's ~25 issues need before
anyone estimates them.

Problem. The epic's work was spread across ~25 issues with no shared design, several of them
written against assumptions that no longer hold. Export was known to be broken but not how
broken, and nobody had established what a portable file actually has to contain.

What the doc does. Every claim is verified against main rather than reasoned from the issue
text. That turned up material the epic did not have:

• Three live export failures, not two. Exporting any PartnerLIF raises AttributeError
  because the base-model lookup filters on Type == "OrgLIF". It is currently masked by MDR /import_export/export/{id} 500: check_base passed to entity service functions that don't accept it #1210,
  which fails first — so PR Issue #1210: fix MDR /import_export/export/{id} 500 from check_base passed to entity service functions #1212 will expose it. Seed model 18 (Org2 LIF) triggers it.
• Six gaps no ticket covers, including two tables export never emits (base-model inclusions and
  value mappings), no update path for schemas at all, and a third kind of reference — the
  TargetEntityId column added in V1.6 — that is a raw row ID already carried on the exported
  attribute record and handled nowhere on import.
• Four tickets already satisfied — MDR schema export: generate_openapi_schema 500s (uncaught KeyError) when a named Embedded ancestor precedes a Reference — find_children and add_ref disagree on key spelling #1252 (fixed and closed), Fetch OpenAPI schema from MDR instead of file #717 (done), most of Ensure the MDR API enforces unique names of _anything_ that can be exportable (schemas or mappings) #746
  (8 uniqueness rules already exist), and one bullet of MDR schema round-trip: true data-portability test matrix (follow-up to #1007) #1063 (the round-trip test already runs in
  CI against real Postgres).
• Five decisions recorded, most notably identifying mappings by a generated PortableKey
  rather than by name — names cannot work, and the reason usually given for that (118 duplicates in
  seed) is itself wrong, since all 118 are soft-deleted history.
• Four tickets that do not serve portability, with recommendations (Spike: Research & Design Planning (MDR Export/Import Translation) #646 and Export/import translations into the MDR #662 to close as
  superseded; Duplicate CreateTransformationGroupDTO / UpdateTransformationGroupDTO across two modules (definitions differ) #1138 and MDR Transformation Portability: Expose a UX widget to delete a transformation group #774 to keep but outside the epic).

Side effects / limitations. cspell.json gains five words (conftest, demoable,
demoability, runbook, runbooks). The last two are pre-existing findings in INDEX.md that
only surface because this PR touches that file and the cspell hook is scoped to changed files.

The plan defers goal 2 (pulling seed data out of the migrations) to a follow-on effort, per
Decision 2, while keeping the round-trip proof that was the epic's done-test.

How to review. The doc is self-contained and every claim cites a file and line. The parts most
worth a second opinion are the five decisions, the eleven proposed new tickets, and the argument in
"Key the converter off structure, not the type name."

Related Issues

Refs #1223

Type of Change

• Documentation update

Project Area(s) Affected

• Documentation (docs/, READMEs, ARCHITECTURE.md, CLAUDE.md)

---

Checklist

• commit message follows commit guidelines (see commitlint.config.mjs)
• documentation is changed or added (in /docs directory)
• pre-commit hooks have been run successfully

Testing

No code changed, so the usual suites do not apply. Verified:

• cspell passes on all three changed files (the hook is scoped to changed files, so INDEX.md
  had to come clean too).
• All 25 relative links in the doc resolve from its location under docs/operations/proposals/.
• docs/INDEX.md entry added per the docs-index skill convention, appended within the proposals
  section rather than reordering it.
• Placement under operations/proposals/ follows the decision tree in docs/README.md; the
  lifecycle there is "promote to ADR or guide on acceptance", which is the intent once the epic
  closes.

Additional Notes

Two things reviewers may want to push on:

1. The doc records reasoning that turned out wrong, deliberately — a superseded theory about
   why mapping export fails, and a header warning not to infer structural limits from the seed
   data. Both cost a few lines and both were things this review got wrong once before measuring.
2. Eleven new tickets are proposed but not filed. That is intentional: several depend on the
   decisions landing first, and filing them before review would presume the outcome.

🤖 Generated with Claude Code

[bjagg]

This is excellent verification work. I checked the load-bearing claims against the code and all of them hold: the three export failures (including the new PartnerLIF one), the seed counts (9/24 groups, 6/1,378 transformations, 21 models), the 8 unique indexes, #717, and CI run 35801872026. Thank you for recording the reasoning that turned out wrong; that's rare and useful.

Filed from this: the PartnerLIF failure is now #1321. It has a second entry point, GET /datamodels/base/{id}, which 500s for any non-OrgLIF id. It's filed now rather than with the proposed tickets because #1212 exposes it on merge.

Asks:

1. #1063: keep the CI bullet, but make it "fail, don't skip". The Postgres fixture calls pytest.skip when Postgres is missing, and CI gets Postgres only because ubuntu-latest preinstalls it. Skip locally, fail when CI is set.
2. Document-store section: please narrow it to "not needed for this epic". Portability is solvable on Postgres, which your reason 1 shows well. Reason 2 isn't accurate as written: MongoDB supports unique and partial unique indexes, so those rules wouldn't become racy application checks. The broader MDR-storage question belongs on #1132.
3. Decisions → decisions needed. Several of the five are product and architecture choices that haven't been reviewed yet: destructive import (D1), what identifies a mapping (D3), and investing in the ancestor chain (D5), which depends on whether extensions and inclusions stay. Could you present them as proposed, with options and evidence, so they can be decided and recorded as ADRs? The findings and plan stand either way.

An alternative for D3, for that discussion: identify a mapping by (group, target path), with sources and expression as content. That keeps identity stable through source and expression edits, which is your main requirement, without a generated key. It rests on one rule: within a group, mappings and target fields correspond one-to-one. In the three versioned transforms (185 mappings), every well-formed mapping already satisfies it. The only violations are the two target-less CLR rules behind #1144, which the rule would have rejected. JSONata's ??, ?: and $exists() cover fallback sources within one mapping. Target renames imply a new target schema version, and so a new group, so recreating there is expected.

[cbeach47]

From Claude: Thanks — this was a useful review, and all three asks are in as of a8b25ea. Two of them changed the substance rather than the wording, and your D3 alternative is now the chosen option. Details, including the two places where measuring your premises moved them:

1. #1063 — agreed, bullet restored

You're right, and my reasoning was wrong rather than just my conclusion: I read "the test ran in CI" as "the test is guaranteed to run in CI." pr-ci.yml runs on ubuntu-latest with no postgres service and no install step — a search for postgres across .github/workflows/ returns nothing — so the database comes from the runner image alone, and the fixture's pytest.skip would quietly turn all of these green if that ever changed.

Bullet kept and re-scoped to skip locally, fail when CI is set, with the fixture and the workflow both cited so the next reader doesn't have to re-derive it.

2. Document store — narrowed, and reason 2 withdrawn

The section is now "Out of scope for this epic," and the storage question is explicitly #1132's. Reason 2 is withdrawn in the text rather than quietly deleted — MongoDB has unique and partial unique indexes, so the #746 rules port rather than degrading into racy application checks. It wasn't load-bearing; the conclusion rests on the other two points, and the "1 and 2" reference now names them inline instead of pointing at numbers.

The overlay question — extensions and inclusions are an overlay between models, a document is a tree — moved to #1132 as genuinely open, rather than being used here as an argument against the migration.

3. Decisions — options and evidence, recorded in the doc

Each of D1, D3 and D5 now carries the options that were weighed, the evidence, and what each costs, which is what you asked for. The difference is where the calls live: they are recorded in this doc rather than deferred to separate ADRs.

The lean is to reserve ADRs for larger, system-wide decisions and let the smaller related ones live in a living feature guide for import/export. That is also the repo's own documented lifecycle for a proposal — "promote to ADR or guide on acceptance" (docs/README.md:42). Most of these five are local to import/export rather than system-wide, so the guide looks like the better home, and the natural path is for this proposal to become that guide under docs/design/cross-cutting/ once the epic lands, with the decisions as its standing reference.

If one of them is system-wide enough to deserve its own ADR, D3 is the candidate — it constrains what a mapping is allowed to be, which reaches past import/export into authoring. docs/design/adr/metadata_repository/ is sitting there if you think it belongs.

Where they landed: 1a (mirror, with a preflight preview), 3b (yours), 5a (one helper, full chain everywhere). D2 and D4 stay as they were.

Your D3 alternative — adopted, with two corrections from measuring it

I checked the rule rather than taking it, and the core holds exactly: 183 of 183 well-formed mappings, zero duplicate target paths in any group. Groups 29/49/50 at 83/70/30 mappings, distinct targets 83/70/30. The only exceptions are 1671 and 1672 — your two #1144 rules — and the rule would have rejected both at authoring time.

Two premises moved, though:

Many sources → one target is not supported — narrower than "fallback sources live inside one mapping." 179 of 183 mappings record exactly one source, and all four exceptions are unfinished drafts whose expression reads a single source anyway:

Mapping	Sources recorded	What the expression reads
1634 …Race	5 race attributes	Culture.americanIndianOrAlaskan
1636 {Multiple}.Sex	sex, gender	SexAndGender.sex
1653 Address.Period	dateEffective, dateExpired	dateEffective
1578 achievement.name / description	name, description	name

Two of the four write to placeholder targets ({Multiple}, name / description). So the recorded source binding is single; the JSONata expression can still read whatever it needs. Fan-out the other way is normal and stays supported — 15 source paths already feed more than one target.

Target renames don't imply a new schema version. This is the one place the alternative costs something, and the correction is Chris's rather than anything I measured: a target field can be renamed in place, with no new version and so no new group. The mapping's identity changes with it, so an import processes the rename as a delete plus a create and loses notes, contributor and dates. The doc now states that as the accepted price of 3b rather than arguing it away, with the preflight making it visible before anything is written. Surviving a rename in place is 3a's one real advantage, and it costs a column, a migration and a backfill.

Also: Gap F is re-framed around what actually breaks identity. A mapping writing several targets is fine — identity is its target set. What is rejected is two mappings claiming the same target, which was already broken, just silently, since export overwrites at transformation_service.py:1195 and the winner depends on row order.

Beyond the asks

• MDR: PartnerLIF export and GET /datamodels/base/{id} return 500 — base-model lookup only matches OrgLIF #1321 is wired in — both entry points, including the GET /datamodels/base/{id} one my table had missed. NEW-C is gone.
• MDR portability: define the portable file format and the single two-way converter #1333 filed for the converter contract (was NEW-B), so that work can start while the rest settles.
• Gap C was wrong and is now reversed. Non-JSONata rules aren't supported transformations — LIF_Pseudo_Code is the column default, so those rows are drafts. The export filter is therefore correct; the defect is the 400 message, which covers three different causes and tells the user to add a transformation to a group that already has one. Exportable stays 2 of 9 by design, and lossless is scoped to JSONata throughout.
• New finding — a schema upload can break mappings it never mentions. Mappings bind to schema elements by row ID (TransformationAttributes.AttributeId/.EntityId are FKs). Deleting a used attribute hits the FK as a raw 500, since delete_attribute clears EntityAttributeAssociation but not TransformationAttributes; renaming one is worse, because the FK still resolves and the breakage is silent. The preflight now has to follow that key one hop further and block, not warn, where a mapping would be left dangling.
• Merged main (21 commits); full suite green on the result — 993 passed, 0 skipped.

[bjagg]

Thanks for the revision. It's a big improvement: the #1063 bullet, the narrowed storage section with reason 2 withdrawn, options and evidence on every decision, and the Gap C reversal. Your FK finding held up and is now #1338. Checking it against the code changed the picture: the delete path never 500s, because the API's soft delete silently removes every mapping that uses the attribute, and a rename silently breaks them, since EntityIdPath is stored and the expression spells names inline.

We worked through the open decisions today. Here's what changes in the doc.

D1, import mode: explicit mode with an always-on preview. Every import shows the preflight preview. The caller picks merge (create and update only; the default) or replace (the mirror you described: omissions deleted, listed and confirmed). A replace is blocked, not warned, where a delete or rename would leave a mapping dangling (#1338). This is your 1a plus a safe default.

D3, mapping identity: 3b, with three clarifications.

1. Exactly one target per mapping, and no target field produced by two mappings in a group. That replaces "a set of targets is fine". An expression that writes beyond its declared target is a validation error, which is the #1144 failure.
2. Multiple sources are supported and are content, not identity. Import saves every entry in SourceAttributes (transformation_service.py:282, :314), and #1296 has 13 real multi-source rules, e.g. Learner.displayName from firstName + lastName. So "many sources → one target is not supported" should go. Your 183 are right for groups 29/49/50, where the four multi-source mappings are drafts. The declared set must list every field the expression reads.
3. In-place renames of a field a mapping depends on are blocked, with the dependent mappings listed. Renames go through a new schema version or an explicit re-point. That settles the cost you named for 3b, and it's #1338's fix.

D5: 5b, not 5a. No new investment in extensions and inclusions for now. Where import/export has to walk the ancestor chain, keep it local to import/export. Replacing the overlay model (a variant becomes a source schema plus a mapping to target LIF) is the direction for a later phase. Live overlay bugs like #1321 still get fixed.

D2, near-term scope: fixes and design. The three export 500s (#1210, #1211, #1321), #1338's dependency blocking, the converter contract (#1333) and #1142. The round-trip proof and migration slimming move to a later phase; D4 (lossless) stands as the requirement it will meet.

Recording: your lean holds for the local decisions (modes and preview, lossless, converter, scope), which go in the living import/export guide once this is promoted into docs/design/cross-cutting/. Three decisions reach past import/export, so they'll be ADRs: mapping identity (as you suggested), the overlay direction (amending metadata_repository/0004), and the reference model. I'll open those as a separate PR so this one isn't blocked on them.

#1321, #1338 and #1333 are assigned to you for the next sprint. With these edits in, I'm happy to approve.

[bjagg]

Re-reviewed on 660da0e. Every point from my 9/30 review is in, so I'm approving. That covers explicit mode with always-on preview, merge by default and replace listed-and-confirmed, one target per group, multiple sources as content, in-place renames blocked, 5b plus the freeze, the near-term scope, and the FK finding. The design and phases are sound, and I'm fine with you validating the flows empirically as the tickets go.

A few follow-ups, as an appended commit after merge or in your first #1333 PR. Most come from things that landed on main after your last push, so none of it is on you.

Worth fixing soon:

1. Two of the "three live export failures" were fixed yesterday.
   • #1321 (the PartnerLIF base lookup) is fixed by #1348.
   • #1211 (export_multiple unpacking 6 of 7 values) is fixed by #1349.
   • Both have regression tests, so L83's "none of the three has a regression test" no longer holds.
   • #1210 is still live; dev logs show the check_base TypeError today.
   • Where to update it: "What is broken today", Phase 0 and the INDEX line. As written, Phase 0 sends a reader to two finished items. #1348 also adopted exactly the rule you argue for, keying the lookup on having a base model instead of the type name.
2. The overlay ADR link points at the wrong ADR. L384 and L658 say the overlay direction "amends metadata_repository/0004". That error came from my 9/30 review, sorry. The ADR is data_model/0002, and it amends metadata_repository/0008. Please link all three ADRs from #1339 (data_model/0002, data_model/0003, metadata_repository/0009).
3. The mapping path format is decided. L895-897 (and #1333) leave the target path open ("drop it or document it as advisory"). ADR 0009 Decision 1 settles it: name-based EntityIdPath with the numeric model prefixes removed.
4. The INDEX entry describes the earlier decisions. It says "mirror-with-preflight" and "full ancestor-chain resolution", which are 1a and 5a. The doc now says 1c and 5b. The INDEX line is what most people read.
5. An estimate to remove, since this repo is public. L960 says "Most are 1–3 days". Effort estimates belong on the private board.

Small corrections:

• L921 offers transformation_service.py:132 as a structural "self-contained" test, but that line checks Type in [BaseLIF, SourceSchema], which is the type-name proxy.
• L296 says get_base_model_for_given_orglif exists only for export. The GET /datamodels/base/{id} route (datamodel_endpoints.py:142) calls it too, which is the second entry point you name at L81.
• L933 "the UI hardcodes the parent to 1": both defaults (Dialog.tsx:94, DataModelSelector.tsx:93) apply only to PartnerLIF.
• Line anchors: 7 of the 53 now miss. Four import_export_service.py anchors moved +2 with #1349. schema_generation_service.py:802 should be 892-895, mdr_sql_model.py#L26-L28 should be 28-30, and :681,709 should be 682 and 710.
• Bug table and status: #1334, #1335 and #1336 aren't in the bug table. #1171 closed yesterday (#1274). The header still says Proposed, 2026-09-25, and L23 and L980 cite different base commits.
• 5b leftovers: L262 still recommends consolidating the copies, which the 5b section says is out of scope. L279's "Name lookups must search the whole chain" should be scoped to the converter.
• Heads-up: if Gary's draft #1345 merges, it moves components/lif/datatypes/mdr_sql_model.py to components/lif/mdr_sql_model/core.py unchanged. That breaks four links here (L244, L428, L625, L862); the line numbers stay valid.

Two fixes on my side, in #1339, not yours:

• ADR 0009 says "1 of 1,368 mappings". V1.1 seeds 1,378, which is your figure.
• metadata_repository/0008's Data Portability paragraph (the default-ID plus ID-mapping import) still stands, while this plan closes #768 and #775 that implement it. #1339 should amend it.