Skip to content

Restore curation the editor's stale saves reverted, and fail CI on deletions - #78

Merged
KrishnaTO merged 1 commit into
mainfrom
claude/disease-confirmations-removal-2b28f7
Aug 31, 2026
Merged

Restore curation the editor's stale saves reverted, and fail CI on deletions#78
KrishnaTO merged 1 commit into
mainfrom
claude/disease-confirmations-removal-2b28f7

Conversation

@KrishnaTO

Copy link
Copy Markdown
Owner

Curators kept confirming cross-references that later vanished from the ontology. Hemophilia B Leyden (ARI:0001098) is the clean specimen: linikujp confirmed MONDO:0850054 and ORPHA:617930 on 21 August, and by 30 August the disease held neither the ids nor any record that anyone had reviewed it.

The cause is not curation. An editor save writes the whole ontology from a copy loaded when the session started, so it reverts anything merged into the branch since. 295fb23 deleted that confirmation three seconds after 17e616c merged it in cleanly. 0f03b91, labelled a review of one disease, changed 453 lines. Two curators' saves on 17 August landed on byte-identical stale content, which points at a shared server-side copy rather than per-user browser state.

Between 17 and 30 August that cost main 208 synonyms, 57 clinical subtypes, ~100 review records, and three completed cleanup branches. The audit went from 0 errors to 66.

What this changes

Restores the lost curation, from every commit reachable from main rather than one snapshot, so losses from the earlier saves come back too. Synonyms 490 → 708, clinical subtypes 355 → 429, recorded reviews 194 → 305. Verified afterwards: all 603 distinct (disease, author, review) records that ever reached main are present, and none was invented.

Stores 30 confirmed cross-references that had never been written to a disease, across 16 diseases — 12 MONDO, 10 Orphanet, 3 UMLS, one each of DOID, NCIt, MeSH, ICD-10, OMIM. Confirming a term only ever affirmed an id the disease already held, so confirming one the registry lacked recorded a judgment with no data behind it. confirmed-not-stored is at zero for the first time since the check existed.

Re-applies the three reverted cleanups — 61 ICD-9 codes under ARI_ICD10, the two MONDO:-prefixed values, the I00-I02 and 390-392.99 ranges. Cross-references are now derived from the mapping set rather than from any snapshot: flagged ids dropped, confirmed ids stored. That reproduces @aaronabend's own correction in 1f18f16 — Multiple sclerosis keeps OMOP 4027727 and SNOMED 24700007 — without special-casing it.

Fixes three mapping errors that had been invisible. The two exports disagreed about which OMOP concept is Multiple sclerosis; the equivalencies rows were the inverted pair. Three judgments were recorded twice, because a curator reviewed a pair, the record was wiped, the pair resurfaced as unreviewed, and a second curator confirmed the same terms again. The earlier judgment is kept each time, so @linikujp keeps the ARI:0001019 credit that was taken once already.

Lets CI fail on absence. Validate mappings ran with --since BASE_SHA and reported only rows a branch added or rewrote, so a save that deleted 385 lines passed clean. check_deletions now compares the ontology against the PR base and reports record-deleted, xref-deleted and disease-deleted. A flagged cross-reference may still go — that judgment is in the mapping set — but a synonym, a subtype, a changelog entry, or an id no curator ruled against may not. Removing a malformed value is exempt, so repairing an ICD-9 code is not mistaken for a reversal.

Replayed against 0f03b91, the commit that started this: 24 errors, where CI previously reported none.

Restores the weekly audit's sight. The editor began writing a tenth comment column into ari.sssom.tsv; the header check rejected it, split_rows returned nothing, and every SSSOM row check was skipped in silence — including the two that exist to catch exactly this. Ports the header fix from fix/sssom-comment-column (validator only, none of that branch's data) and widens mapping_date to accept the ISO 8601 timestamp the app writes.

Not in this PR

The two fixes that stop it happening again are app-side and belong in KrishnaTO/ARI-metadata-manager:

  1. A save must apply a diff, not a snapshot. Merging main in first does not help. Until this ships, the next publish can revert this PR.
  2. A confirmation must write the id. The 30 stranded ids are stored now, but the next confirmation of a term the registry lacks will strand itself the same way.

Left for a curator

  • Some diseases carry the same review twice under different timestamps — the app re-recording a judgment whose record had been wiped. Collapsing those is a curation call, and does not belong in a change that adds a rule saying ARI_ChangeLog is append-only.
  • Chronic Lyme disease (ARI:0001065) now holds four OMOP concepts with no judgment on any: two curated on 16 August, two the stale save put back. Unioned rather than guessed — this pair needs a decision.

Verification

python .github/scripts/validate_mappings.py            # 0 errors, 6 warnings  (was 66 errors)
python .github/scripts/validate_mappings.py --since main   # 0 errors, 0 warnings

The six remaining warnings are the standing dxcode-without-snomed debt, unchanged. The OWL parses as well-formed XML.

Advances #23 — the stored-id and exact-match subtasks; validating all diseases and listing missing diseases per database are untouched.

🤖 Generated with Claude Code

…letions

The ARI editor writes the whole ontology from a copy loaded when the session
started, so each save reverts anything merged into the branch since. 295fb23
deleted a confirmation three seconds after 17e616c merged it in cleanly;
0f03b91, labelled a review of one disease, changed 453 lines.

Restores 95 changelog entries, 10 synonyms and 17 clinical subtypes from the
merged history, plus 208 synonyms and 57 clinical subtypes from PR #69, the
last commit before the 17 August cliff. Stores 30 confirmed cross-references
that had never been written to a disease, including the MONDO and Orphanet
terms linikujp confirmed for Hemophilia B Leyden on 21 August. Re-applies the
ICD-9 retirement, the MONDO de-prefixing and the ICD-10 range removal, all
overwritten the day after they landed.

Cross-references are now derived from the mapping set rather than from either
snapshot: flagged ids are dropped, confirmed ids stored. Also corrects the
inverted OMOP judgment for Multiple sclerosis in ari.equivalencies.tsv and
removes three judgments recorded twice, keeping the earlier author each time.

Validate mappings ran with --since BASE_SHA and reported only rows a branch
added, so a save that deleted 385 lines passed clean. check_deletions now
compares against the pull request's base and fails on records removed without
a curator judgment behind them; replayed against 0f03b91 it reports 24 errors.
Ports the sssom comment-column header fix so the weekly audit stops skipping
every SSSOM row check in silence, and accepts the ISO timestamp the app writes.

The app-side fixes -- saving a diff instead of a snapshot, and storing an id at
confirmation time -- belong in KrishnaTO/ARI-metadata-manager and are not here.

Audit: 66 errors -> 0 errors, 6 warnings. confirmed-not-stored: 30 -> 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@KrishnaTO
KrishnaTO merged commit b34bc94 into main Aug 31, 2026
1 check passed
@KrishnaTO

Copy link
Copy Markdown
Owner Author

The two app-side fixes are now tracked as KrishnaTO/ARI-metadata-manager#146 — both located in the source:

  • app/routes/publish.py:196 commits the session's stale working copy, while the mapping files three lines below correctly re-read from the source branch and append. That asymmetry is why not one mapping row was lost across the whole incident and the ontology lost hundreds of records.
  • app/ontology_service.py:988 documents that confirming a term writes only the mapping row and a changelog entry, never the id.

Until #146 lands, merging this PR restores the data but does not protect it: a publish from a session that started before the merge will revert it. The new check_deletions will fail that publish rather than let it through — worth warning curators that they may see a red build they did not cause.

@KrishnaTO

Copy link
Copy Markdown
Owner Author

Chronic Lyme disease (ARI:0001065) is now judged — one of the two items this PR had left open.

The disease is Post-Treatment Lyme Disease Syndrome, not Lyme disease: confirmed MeSH D000077342 "Post-Lyme Disease Syndrome", MONDO:0700280, NCIt C119039, UMLS C3890422, with DOID:11729 and icd10cm:A69.2 already rejected and SNOMED recorded as having no term. Resolved the four OMOP concepts against the local Athena vocabulary in data/2-databases:

OMOP Resolves to Verdict
19137845 MeSH D000077342 Post-Lyme Disease Syndrome confirmed — it is the term already confirmed
440638 SNOMED 23502006 Lyme disease flagged wrong
4141757 SNOMED 33937009 Lyme arthritis flagged wrong
37365579 absent from every local vocabulary file left unjudged

The two flagged concepts are the active infection and one of its manifestations — both SNOMED concepts wearing an OMOP id, so storing them contradicted the curator's own "no term in SNOMED" finding.

ARI_DXCODE held 23502006 and 33937009, the SNOMED codes behind those two, so they are flagged as SNOMED as well and the property is now empty. Otherwise the same rejected concepts would have survived under a second property. That clears one dxcode-without-snomed warning: 0 errors, 5 warnings.

37365579 still needs Athena — it resolves in no local file, in any column. I left it stored and unreviewed rather than dropping it: an id nobody can look up is not an id anybody can rule on, and removing it would be exactly the unreviewed deletion this PR adds a check against.

Separately, recording the judgment surfaced a latent bug in date-future: it compared a UTC mapping_date against the local date, so an evening publish west of UTC read as tomorrow's work. CI runners are UTC and never saw it; anyone in Toronto running the validator after 20:00 would have. Fixed in the same commit.

That leaves one open item: the diseases carrying the same review recorded twice under different timestamps.

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.

1 participant