Decide #236: a MIM: subject is the escaped file stem, and nothing re-derives it - #688
realmarcin wants to merge 2 commits into
Conversation
…derives it #236 asked whether `MIM:` slugs are paths or opaque ids, and offered three options: rename files on relabel, declare the slug opaque, or require every subject to resolve. The trade-off it described -- stable per-record paths against slug/path agreement -- only exists if the slug is computed from preferred_term. FilenameIndex never renames a file, so a subject derived from the *stem* is both stable and file-backed. The pipeline had already made that choice and nobody had written it down where it could be found: claw's publisher subject_id MIM:<safe_stem> -- stable per-YAML CURIE CurieNormalizer known set built from file stems; anything else is UNKNOWN_SUBJECT, so equivalent_term will not cite it MAPPING_SEMANTICS already said "Keep the SSSOM subject_id", inside a blockquote about spelling corrections Option 2 was wrong on its own terms: a subject that names no file is not harmless, because the resolver treats the slug as a path. This records the rule in its own section and makes everything obey it. The mechanism #236 was filed about was still live. relabel_mapped_record rewrote cols[0] = f"MIM:{sanitize_filename(new_term)}" on every rename while leaving the file where it was -- the exact MIM:2-phenylethylamine / Phenethylamine_Hydrochloride.yaml case the issue cites, whose record the script's own docstring names. A relabel now changes subject_label only. merge_salt_label_duplicates dropped `f"MIM:{lose_label.replace(' ', '_')}"`, which matched only because its three labels happen to equal their stems; a miss drops nothing and leaves an ORPHAN. It now uses the loser's stem -- and captures it before the loser is rewritten to carry the winner's identifier, because FilenameIndex matches on identifier first and a later lookup returns the *winner's* stem. reground_aromatic_hydrocarbon fell back to a label-spelled subject; it now refuses. decompose_py_media_and_ground_categories held a dead block that looped over a list hard-set to [] -- it never emitted a row, but it read and rewrote the published SSSOM on every --apply. Removed. Six writers took the right stem and forgot to escape it, so the 23 records whose stems hold "(", ")" or a Greek letter got subjects the publisher does not emit. create_records_from_groundings compared that unescaped subject against `published`, so such a stem would miss the dedupe and emit a duplicate row. None of the 23 publishes a narrowMatch, so Rule B1's registry slug -- which drops those characters rather than escaping them -- is unaffected. The escape rule had five hand-written copies. Three of them had independently rediscovered the same bug -- the validator's and backfill's docstrings both record "matching on the raw stem misses every record whose filename has a parenthesis" -- and the fourth, check_sssom_subject_files, still had it. They now all call mim_curie_for_stem. The validator's copy carried a comment saying it was inlined "to keep the validator stdlib-only"; that stopped being true when Rule J added a package import, and curie.py is itself stdlib-only. That checker reported 18 subjects as naming no file. All 18 were escaped subjects compared against unescaped stems; measured with the escaping the resolver uses, there were none. The tool declaring the question undecided was producing the evidence that it was. It now escapes, and gates: qc-sssom runs it locally and in CI. Canaried in CI's exact environment -- Python 3.13 with only pyyaml, the package confirmed not importable -- with each step's own exit code: all four qc-sssom steps exit 0. My first inventory for this was cut short by a `head -10` and missed the relabel script, which is the one that matters most. The regression guard scans the whole tree. 13 tests; every guard mutation-checked by reintroducing its defect. Closes #236. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AcKSD9k5BBntNUFyG3E9H9
…es not files (#689, #690, #691) Adversarial review of this branch. Three findings, all fixed here. #689 -- I copied "205 renames to date" from curie.py's docstring into MAPPING_SEMANTICS as evidence that deliberate renames still happen. The alias file holds 196 rows, and quoting a mutable count is what CLAUDE.md says not to do. Worse, the characterisation was wrong. Classifying all 196: escaping only (same stem, now ~HEX) 11 case only 68 a different stem 117 and the first "different stem" entry is MIM:2-phenylethylamine -> MIM:Phenethylamine_Hydrochloride -- the #236 case itself. The alias map has been absorbing subject-derivation drift, not recording file moves. Both texts now say what the map is for, without a number. #690 -- when FilenameIndex found no stem, the merge script printed that the loser "published no SSSOM row to drop". That was a conclusion, not an observation: the index misses a record whose collection copy has drifted from its file on both keys, and then a real row survives as an ORPHAN while the one message that could have flagged it certifies it instead -- the #306/#687 shape. It now looks for rows carrying the loser's label and refuses if any exist. Mutation-checked: restoring the old message fails both tests. #691 -- the regression guard exempted four whole files. Each holds exactly one legitimate `f"MIM:{...}"`, but a second, wrong one added later would have passed. It now exempts those lines by content, and a test fails if an exemption stops matching. Mutation-checked: a second bad subject in a formerly exempt file is caught. 16 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AcKSD9k5BBntNUFyG3E9H9
|
Adversarial review of this branch, read-only. Three findings, filed and fixed here. #689 — I put a stale, mischaracterised number into the specI copied "205 renames to date" from The characterisation was the bigger problem. Classifying all 196:
And the first "different stem" entry is #690 — "nothing to drop" was asserted, not checkedWhen It now looks for published rows carrying the loser's label and refuses, naming them, if any exist. Mutation-checked: restoring the old message fails both tests. #691 — the guard exempted whole filesEach of the four exempt files holds exactly one legitimate Also worth recordingI had two full-suite runs going in the same worktree, and changed files under the older one mid-run — so its result would have described neither version. I stopped it; the one running now is on the final tree. |
Closes #236.
The decision
A
MIM:subject ismim_curie_for_stem(<per-record file stem>)— fixed for the life of the record, never re-derived frompreferred_term. Recorded inMAPPING_SEMANTICS.md§2 → Subject identifiers.#236 framed this as a trade-off between stable per-record paths and slug/path agreement. That trade-off only exists if the slug comes from the label.
FilenameIndexnever renames a file, so a subject derived from the stem gets both.It was already the rule — it just wasn't written where anyone could find it
subject_id MIM:<safe_stem> -- stable per-YAML CURIECurieNormalizerUNKNOWN_SUBJECTMAPPING_SEMANTICS.mdsubject_id" — inside a blockquote about spelling correctionsOption 2 was wrong on its own terms. A subject naming no file is not harmless: the resolver returns
UNKNOWN_SUBJECTandequivalent_termrefuses to cite the mapping.The 18 "unresolved subjects" were a checker bug
check_sssom_subject_files.pycompared subjects against unescaped stems. All 18 were subjects whose stem holds(,)orα:Measured with the escaping the resolver itself uses: zero genuine mismatches. The tool declaring the question undecided was producing the evidence that it was. It now escapes, and gates — in
just qc-sssomand in theqc-sssomworkflow.What still disagreed with the rule
The mechanism #236 was filed about was still live.
relabel_mapped_recordrewrotecols[0] = f"MIM:{sanitize_filename(new_term)}"on every rename while leaving the file where it was. Its own docstring names the exact record #236 cites. A relabel now changessubject_labelonly.merge_salt_label_duplicateshad a trap. It droppedf"MIM:{lose_label.replace(' ', '_')}", which only matched because its three labels equal their stems. Switching to the stem needed care: the loser is rewritten to carry the winner's identifier before the drop, andFilenameIndexmatches identifier first — so a lookup at that point returns the winner's stem and would drop the winner's rows. The stem is now captured before the rewrite, pinned by a test that demonstrates the trap directly.relabel_mapped_record,merge_salt_label_duplicates,reground_aromatic_hydrocarbon(now refuses to guess)create_records_from_groundings*,add_culturemech_gap_labels,move_mapped_out_of_unmapped_collection,promote_resolved_unmapped,promote_microbedecoder_residual,promote_microbedecoder_reviewedvalidate_sssom_invariants,backfill_sssom_surface_forms,build_curie_alias_map,check_sssom_subject_filesdecompose_py_media_and_ground_categories(removed)* compared the unescaped subject against
published, so an escape-needing stem would miss the dedupe and emit a duplicate row.Five copies of the escape rule, and three had independently rediscovered the same bug — the validator's and backfill's docstrings both record it. The fourth copy still had it. All now call
mim_curie_for_stem. The validator's copy said it was inlined "to keep the validator stdlib-only"; that stopped being true when Rule J added a package import, andcurie.pyis stdlib-only anyway.Rule B1 is unaffected. 23 record stems need escaping; none publishes a narrowMatch, and the registry slug drops those characters rather than escaping them (
MIM:Synthetic_Sea_Salts_~28sss~29↔kgmicrobe.compound:synthetic_sea_salts_sss). §2's claim that the two slugs are "always the same" is corrected to say so.Verification
Canaried in CI's exact environment — Python 3.13 with only
pyyaml, package confirmed not importable — with each step's own exit code:Every guard mutation-checked by reintroducing its defect:
qc-sssom(now incl. the subject gate)qc-flat-coverage,check-unified-freshnesslint/format-check/typecheck/check-instruction-refsA correction to my own recommendation
The inventory I gave before starting was cut short by a
head -10and missedrelabel_mapped_record— the one that matters most. The regression guard scans the whole tree rather than trusting a list.Also worth knowing:
just lintonly checkssrc/, so unused names inscripts/accumulate unseen — there are 11 pre-existing in these files alone. That's #534's territory.Full suite is running; I will confirm before asking for the merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AcKSD9k5BBntNUFyG3E9H9