Resolve the CURIEs the research reports suggest (#243) - #260
Conversation
The sweep's candidate-node tables pair a human label with an ontology id, and nothing resolved them. A sample against OLS found the long tail materially wrong. This checks all of them, against OAK, and writes a triage backlog. Of 1768 checkable (id, label) pairs across 354 reports: OK_LABEL 952 label is the canonical or a synonym OK_IN_ROW 289 the row says what the id means, elsewhere in the row DRIFT 425 the row never states what the id actually means OBSOLETE 39 resolves to an obsolete term UNRESOLVED 28 the id is not in the ontology at all UNKNOWN 35 a prefix outside the grounding policy 490 distinct actionable suggestions. `CHEBI:14341` and `CHEBI:42899` are both offered as "ectoine" and neither exists; `GO:0009405` "pathogenesis" is obsolete and the label agrees, which is exactly the case a label-only check cannot see. A report, not a gate: exits 0 regardless and is deliberately out of `qc`. Nobody is going to hand-edit 353 files of provider output, so failing a build on their contents would gate work on data no one intends to fix in place, and the extraction from prose tables is heuristic. The blocking gate stays where the curated data is — validate-products over mappings/*.tsv, which is where these suggestions land if a curator accepts one. Not built on validate_id_label_correspondence.py: it is vendored byte-identical and drift-checked against CultureMech, so extending it here would fail vendored-sync. Its config vocabulary is mirrored instead. Two defects the canary and first full run found. Allowing digits in the CURIE prefix swept up fatty-acid shorthand (C16:0, C18:1) as 78 bogus findings that buried the real ones. And DRIFT mixed real mis-groundings with lexical variants of the right term — CHEBI does not list "proton" as a synonym of hydron — so each row now carries a similarity score and the output sorts least-similar first, putting ectoine/beta-caryophyllene above fumarate/fumaric acid. Canaried on one report before the full run: 13 pairs, rows on disk, and it found the obsolete GO:0009405. 287 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 4m 35s —— View job ReviewThe premise is right and the findings I spot-checked are real — 🟡
|
There was a problem hiding this comment.
Premise is right and the findings I spot-checked are real. Three blockers:
- The
NO_ADAPTERprefix check is case-sensitive, so lowercasedoi:andmetpo:fall through — 25 of the 35UNKNOWN_PREFIXrows aredoi:10fragments from DOIs in evidence tables. Same failure mode asC16:0, and it inflates the 490-item headline. Ontologies.lookupturns any adapter exception intoUNRESOLVED. A failed semsql download would rewrite the committed TSV to ~1200 bogus unresolved rows and exit 0. The vendoredvalidate_id_label_correspondence.pykeepsADAPTER_ERRORseparate fromID_NOT_FOUNDfor exactly this reason.- The sorted, deduplicated backlog exists only as 15 lines of stdout. The TSV is written in corpus order with the OK rows interleaved, so the artifact the PR is named for isn't on disk.
Plus smaller items (OBSOLETE sorts last because it scores 1.0; obsolescence inferred from the label string; the tracked TSV has no staleness check; recipe naming) — full detail in the review comment.
Four review findings. The backlog existed only in stdout. `distinct` was ranked and deduplicated, then used to print fifteen lines; the TSV was the raw per-occurrence dump in corpus order with 1241 OK_* rows interleaved. The 467-item list this is for was nowhere on disk, and nobody triages from a console scrollback. Now written to reports/research_grounding_backlog.tsv, ranked, deduplicated, with an occurrence count so a curator sees how many sites one decision fixes. (#263) An adapter failure was reported as "id not in the ontology". A failed semsql download would have rewritten the committed TSV to ~1200 UNRESOLVED rows and exited 0 — a broken tool reading as a catastrophic corpus finding, durably, because the artifact is tracked. ADAPTER_ERROR is now its own verdict and prints a warning. It earned its keep immediately: the casefold fix below left _adapter() keyed on the uppercase map, and this surfaced it as 14 ADAPTER_ERRORs instead of silently reporting them as missing ids. (#262) Prefix membership was case-sensitive, so `doi:10.1371/...` truncated to `doi:10` and landed in UNKNOWN_PREFIX — 25 of 35 such rows were citation fragments, and lowercase `metpo:1000650` was being read as a claimed LABEL. Same failure mode as C16:0. Casefolded: UNKNOWN_PREFIX 35 → 10, actionable 490 → 467. (#261) Obsolete findings sorted last. classify() returned similarity 1.0 for OBSOLETE and the sort was ascending, so all 39 ranked below every DRIFT and GO:0009405 — the case this leads with as the one a label check cannot see — never appeared in its own summary. Ordering is now by verdict rank first. Obsolescence also reads OAK's deprecation flag rather than inferring from an `obsolete ` label prefix, which CHEBI does not follow. (#264) Also: renamed report-research-groundings, since every audit-* recipe here is a qc member and this deliberately isn't; documented why it stays out of audit-derived-reports (that recipe must run offline, this needs OAK semsql); and stamped corpus counts into the backlog so its vintage is visible without a timestamp that would churn on every run. 291 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 3m 29s —— View job ReviewTwo of the three blockers are fully fixed and I verified them against the regenerated artifacts. The third is fixed for the case it was reported under but not for the case the new comment claims it covers. ✅ Confirmed fixed
🟡 The empty-adapter case still reports ~1200 ids as "not in the ontology"
It doesn't cover the stub.
🔵 Smaller things
Nothing here touches I could not run the test suite —
|
There was a problem hiding this comment.
Two of the three blockers are fully fixed and verified against the regenerated artifacts: the prefix casefolding (UNKNOWN_PREFIX 35 -> 10, remaining 10 are real SO:/EFO: suggestions, zero DOI rows in the backlog), and the ranked/deduplicated backlog now existing as reports/research_grounding_backlog.tsv with an ordering test. The smaller items — OBSOLETE outranking DRIFT by verdict, owl:deprecated read off entity_metadata_map, the vintage stamp, the report-* naming — are all done.
One 🟡 remains. Ontologies.lookup's docstring (scripts/audit_research_groundings.py:147-149) says ADAPTER_ERROR covers 'an empty sqlite stub', but it only fires when get_adapter or adapter.label raises. A 0-byte or schema-only semsql opens without error and returns label()==None for every id, so every pair falls through to UNRESOLVED — the exact scenario the blocker described: ~1200 bogus 'not in the ontology' rows written to a committed TSV, exit 0. validate_id_label_correspondence.py separates LOAD_FAILED from EMPTY_ADAPTER for this reason; its _is_empty (:264-286) is an O(1) entities() peek with a positive stub confirmation when the probe raises. Mirror that, or narrow the docstring to the exception path it actually handles — the current wording is the more dangerous half.
Two 🔵: the PR body still quotes the pre-fix numbers (1768/490/35 vs the artifacts' 1733/467/10), and merged terms (GO:0009878, GO:0015992) still read as UNRESOLVED where the replacement is what a curator wants.
Full detail in the comment. Nothing touches data/traits/** or the curated mapping tables, so no history/ record is expected.
The #262 fix returned ADAPTER_ERROR only when get_adapter or label() RAISED. Its docstring claimed to cover "an empty sqlite stub" as well, and did not: a 0-byte or schema-only semsql opens cleanly and returns None for every label, so every pair fell through to UNRESOLVED — precisely the failure #262 was about, with the committed backlog silently becoming ~1200 "not in the ontology" rows at exit 0. The wording was the dangerous half, since it told the next reader the case was handled. Now probes one entity at adapter-open time, cached per prefix, and reports the prefix on stderr. A probe that RAISES is deliberately not treated as empty — a partially-migrated live ontology fails the same way, and calling that empty would hide real findings. Same reasoning as AdapterPool._is_empty in the vendored validator, which separates LOAD_FAILED from EMPTY_ADAPTER for this reason. Both directions tested with stub adapters. The real run is unchanged — 1733 pairs checked, 467 distinct findings — which is the point: the probe finds the live adapters non-empty and costs one entity per prefix. 293 tests pass, `just qc` exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @realmarcin's task in 2m 59s —— View job ReviewThe last blocker is fixed, and fixed properly rather than by narrowing the docstring. Approving. ✅ The empty-adapter case is closed
✅ Artifacts and body now agreeCounted directly out of the committed files rather than trusting the summary line:
The last commit touches only the script and tests, and 🔵 One thing, still optionalMerged terms remain Not raising the leading I could not run the test suite — Nothing here touches
|
There was a problem hiding this comment.
The last blocker is closed properly rather than by narrowing the docstring. _is_empty (scripts/audit_research_groundings.py:176-202) probes one entity at adapter-open time, caches per prefix, and returns ADAPTER_ERROR for every lookup under an empty prefix — and deliberately does NOT treat a raising probe as empty, which is the half that's easy to get backwards (a partially-migrated live ontology fails the same way, and calling it empty would suppress real findings). Both directions are tested, and the failure is now loud on stderr plus a corpus-wide warning.
Verified the artifacts by counting them rather than trusting the summary: drift TSV is 952/425/289/39/28/10 = 1733 checked, backlog is 395+36+26+10 = 467 distinct, both matching the vintage stamp and the updated PR body. GO:0009405 now sits at backlog line 45 with 3 occurrences, which is what the verdict-first ranking was for. The last commit touches only script and tests, and _is_empty cannot change a verdict for a non-empty adapter, so unchanged artifacts is the correct outcome.
One remaining 🔵, optional: merged terms still read as UNRESOLVED. GO:0015992 is 7 of the 26 distinct unresolved rows and GO:0009878 another 2 — a third of that bucket is two merged ids, where the replacement is the answer a curator wants. Reasonable as a follow-up.
I could not run pytest (not on this job's allowlist), so the tests are reviewed by reading and by tracing the stubs against the code. Nothing touches data/traits/** or the curated mapping tables, so no history/ record is expected and validate-products is unaffected.
Closes #243.
The sweep's candidate-node tables pair a human label with an ontology identifier, and nothing resolved them. A sample against OLS during the #241 review found the long tail materially wrong. This checks all of them against OAK and writes a triage backlog to
reports/research_grounding_drift.tsv.What the corpus actually contains
1733 (id, label) pairs checked against an ontology, across 354 reports:
OK_LABELOK_IN_ROWDRIFTOBSOLETEUNRESOLVEDUNKNOWN_PREFIXSO:,EFO:)467 distinct actionable suggestions, written to
reports/research_grounding_backlog.tsv— ranked by verdict then least-similar-first, deduplicated, with an occurrence count so one decision closes every site.reports/research_grounding_drift.tsvkeeps every occurrence for locating them afterwards.CHEBI:14341andCHEBI:42899are both offered as "ectoine" and neither exists.GO:0009405is offered as "pathogenesis" — the label agrees with GO's, and the term is obsolete, which is precisely the case a label comparison alone cannot see.doi,ec,metpo,ncbitaxonandtraitmechprefixes are named as having no OAK adapter rather than skipped by omission, so a typo likeCHBEI:still surfaces as unknown instead of vanishing. Matching is casefolded, because the reports spell prefixes inconsistently.A report, not a gate
Exits 0 whatever it finds, and is deliberately not in
just qc:The blocking gate stays where the curated data is —
just validate-productsovermappings/*.tsv, which is where these suggestions land if a curator accepts one. The value here is seeing "this does not resolve to what the report claims" before that lift.Why not extend the existing validator
scripts/validate_id_label_correspondence.pydoes exactly this check for the curated TSVs and was the obvious host. It is vendored byte-identical across the Mech repos and diffed against CultureMech by thevendored-syncjob, so editing it here fails CI; the propagation path runs through the hub. This is a TraitMech-local script that mirrors its config vocabulary — same adapters, same canonical-or-synonym policy — so the two read alike.Six defects found by running it and by review, not by reading it
Per the canary rule, one report first (13 pairs, rows verified on disk, and it found the obsolete
GO:0009405), then the full corpus. The full run surfaced both problems:C16:0,C18:1produced 78 bogusUNKNOWN_PREFIXfindings that buried the real ones. The prefix pattern is now letters-only; no ontology prefix these reports use carries a digit. 78 → 35.DRIFTmixed two populations. Genuine mis-groundings sat beside lexical variants of the right term, because CHEBI does not list "proton" as a synonym of hydron andfumarateis not literally fumaric acid. Each row now carries a similarity score and the output sorts least-similar first, soectoine/(-)-beta-caryophyllene leads andfumarate/fumaric acid trails. There is no cutoff — the score orders, it does not filter.Verification
Review also found four more, all fixed here: the ranked backlog existed only in stdout (#263); an adapter failure was reported as "id not in the ontology", so a failed semsql download would have rewritten the committed TSV to ~1200 findings at exit 0 (#262, #265); case-sensitive prefix matching sent 25 truncated
doi:10citation fragments into the backlog (#261); andOBSOLETEscored 1.0 under an ascending sort, burying all 39 obsolete findings below everyDRIFT— including theGO:0009405this body leads with (#264).Left open and filed: #266, merged terms reading as "never existed" when the successor id is the answer.
The tests stub the adapter and cover extraction and verdicts — where both defects were.
test_a_lexical_variant_scores_above_a_wholesale_mismatchpins the ordering the report depends on.🤖 Generated with Claude Code