From 4610c33a40a224e06cf96844692b75dbd7a6fbc8 Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:57:06 -0700 Subject: [PATCH] Measure curation_history where the gap actually is (#325) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #325 counted curation_history on 2 of 311 records and read it as coverage. That framing does not survive reading the two entries: both record a cross-reference between two ANME/SRB records — same community type, different studies, candidates for consolidation — which git cannot express and no other field holds. It is an event log. A record that has had no such event correctly has none, and backfilling 310 would mean inventing them. So 0.0 is the right threshold rather than a lenient one, and the config now says why instead of reading as a capitulation. The gap is on the writing side, and it is measurable. scripts/audit_writers.py already reports it: 26 tools write record YAML and 16 append nothing. A script that rewrites 300 records in place and leaves no trace is what #325 was pointing at; the low record count is the symptom. This does not fix those 16. It stops the set growing silently and splits it into the two groups a fix treats differently — 7 exempt, each with a written reason (writes research/ or data/, creates rather than edits, or is the mechanism a caller records through), and 9 owed, named individually so the backlog is itemised rather than implied. A new writer in neither list fails, which forces the call when the tool is written. Wiring the 9 needs a shared text-append helper: they are line editors, not YAML round-trippers, so record_curation_event does not reach them. gtdb_ground.py has one, privately. Lifting it into curate/ is the follow-up, deliberately not done here. Mutation-checked: a new script that writes a record and records nothing reddens the gate. Co-Authored-By: Claude Opus 5 --- conf/qc_config.yaml | 8 ++ tests/test_writers_leave_a_trace.py | 180 ++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 tests/test_writers_leave_a_trace.py diff --git a/conf/qc_config.yaml b/conf/qc_config.yaml index 68b84f6d..a333e200 100644 --- a/conf/qc_config.yaml +++ b/conf/qc_config.yaml @@ -52,6 +52,14 @@ slots: # NOT a statement that provenance does not matter. The corpus timestamp on the # dashboard rests on those 2 records, and the generator labels it as such # ("from 2 of 312 records"), so the gap is visible rather than silent. + # + # And 0.0 is the right target rather than a low one, because this is an event + # log and not a coverage metric. The 2 records that carry history record a + # cross-reference between two ANME/SRB records — a fact git cannot express and + # no other field holds. A record that has had no such event correctly has + # none; a backfill would invent them. What is worth measuring instead is on + # the writing side: 16 of 26 record-writing tools append nothing, itemised + # and gated in tests/test_writers_leave_a_trace.py (#325). - path: curation_history threshold: 0.0 required: false diff --git a/tests/test_writers_leave_a_trace.py b/tests/test_writers_leave_a_trace.py new file mode 100644 index 00000000..8943385e --- /dev/null +++ b/tests/test_writers_leave_a_trace.py @@ -0,0 +1,180 @@ +"""Every tool that edits a record either leaves a trace or says why not (#325). + +#325 counted `curation_history` on **2 of 311** records and read it as a +coverage gap. That framing does not survive looking at the two entries: both +record a cross-reference between two ANME/SRB records — "these describe the same +community type from different studies, candidates for consolidation" — which is +a fact about the corpus that git cannot express and no other field carries. + +`curation_history` is an **event log, not a coverage metric**. A record that has +had no such event correctly has none, and backfilling 310 records would mean +inventing events. That is why the QC threshold is 0.0: not a capitulation, a +category correction. + +The real gap is on the writing side, and it is measurable. `scripts/audit_writers.py` +finds **26 scripts that write record YAML, of which 16 append nothing**. A tool +that rewrites 300 records in place and leaves no trace is the thing #325 was +pointing at — the low record count is a symptom of it. + +This file does not fix those 16. It stops the set growing silently, and splits +it into the two groups the fix applies to differently: + +* **exempt** — writes something that is not a curated record (`research/`, + `data/ingredients/`), or creates records rather than editing them, where git + already shows everything. +* **owed** — edits existing `kb/communities` records in place and should append + an event. Named individually so the backlog is itemised rather than implied. + +A new writer in neither list fails, which forces the decision when the tool is +written instead of in the next audit. +""" + +from __future__ import annotations + +import csv +import pathlib +import subprocess + +import pytest + +REPO = pathlib.Path(__file__).parent.parent + +# Writes YAML, but not a curated community record — so there is no record whose +# history it could belong to. +_EXEMPT = { + "scripts/enrich_edison_response.py": "writes research/communities, not kb records", + "scripts/research_community_edison.py": "writes research/communities, not kb records", + "scripts/suggest_related_ingredients.py": "writes data/ingredients, not kb records", + "scripts/scout_communities.py": ( + "creates new records rather than editing existing ones; a creation is " + "fully visible in git, which is not true of an in-place edit" + ), + "src/communitymech/validation/write_validated.py": ( + "the shared validated-write helper — it writes on behalf of a caller, " + "and the event belongs to the operation, not to the mechanism" + ), + "src/communitymech/network/batch_reporter.py": ( + "drives llm_repair, which records the event itself (llm_repair.py:244); " + "recording here too would double-log every repair" + ), + "src/communitymech/cli.py": ( + "dispatcher — the commands it invokes carry their own events, and an " + "event recorded here would name the CLI rather than the operation" + ), +} + +# Edits existing kb/communities records in place and appends nothing. Each is +# work, not a decision: the trace is owed. Listed so the number is itemised +# rather than a headline, and so a fix removes a line here. +_OWED = { + "scripts/apply_strain_designations.py", + "scripts/apply_taxonomy_corrections.py", + "scripts/backfill_metals.py", + "scripts/chebi_fix_apply.py", + "scripts/drop_obsolete_go_bp.py", + "scripts/fix_reference_formats.py", + "scripts/suggest_related_media.py", + "scripts/term_fix_apply.py", + "scripts/term_remap.py", +} + + +@pytest.fixture(scope="module") +def audit() -> list[dict[str, str]]: + """The audit's own output, not a reimplementation of it. + + Re-deriving "does this write YAML" in the test would give two answers to one + question, and the one that drifts is the one nobody runs. + """ + result = subprocess.run( + ["uv", "run", "python", "scripts/audit_writers.py"], + capture_output=True, + text=True, + cwd=REPO, + timeout=900, + ) + assert result.returncode == 0, result.stderr[-500:] + return list(csv.DictReader(result.stdout.splitlines(), delimiter="\t")) + + +def test_the_audit_sees_the_writers(audit): + """Guard: an audit returning nothing makes every check below vacuous.""" + writers = [row for row in audit if row["writes_yaml"] == "yes"] + assert len(writers) >= 20, f"only {len(writers)} YAML writers found; audit_writers.py broke" + + +def test_every_writer_is_classified(audit): + """A new tool must say whether it owes a trace, when it is written.""" + unclassified = sorted( + row["path"] + for row in audit + if row["writes_yaml"] == "yes" + and row["appends_curation_history"] == "no" + and row["path"] not in _EXEMPT + and row["path"] not in _OWED + ) + assert unclassified == [], ( + "these write record YAML and append no curation event, and are in " + "neither list:\n" + + "\n".join(f" {path}" for path in unclassified) + + "\n\nAdd `record_curation_event(...)`, or classify it: `_EXEMPT` " + "with a reason if it does not touch kb/communities, `_OWED` if it " + "does and the trace is simply not written yet (#325)." + ) + + +def test_neither_list_has_rotted(audit): + """A tool that gained a trace must leave `_OWED`, or the list is a fiction.""" + appending = {row["path"] for row in audit if row["appends_curation_history"] == "yes"} + fixed = sorted(_OWED & appending) + assert fixed == [], f"these now append a curation event and should come out of _OWED: {fixed}" + + known = {row["path"] for row in audit} + gone = sorted((set(_EXEMPT) | _OWED) - known) + assert gone == [], f"these are classified but the audit no longer sees them: {gone}" + + +def test_the_exemptions_have_reasons(): + """An allow-list without reasons is how the next one gets waved through.""" + blank = sorted(path for path, why in _EXEMPT.items() if not (why or "").strip()) + assert blank == [], f"exemptions need a reason: {blank}" + assert not (set(_EXEMPT) & _OWED), "a writer cannot be both exempt and owed" + + +def test_the_owed_backlog_has_not_grown(audit): + """#325's real number, bounded. + + 16 of 26 writers appended nothing when this was measured — 9 owed, 5 exempt, + and 2 that the audit counts but which are covered elsewhere. The point of a + bound is that adding a tenth owed writer is a decision someone makes on + purpose, not a drift. + """ + assert len(_OWED) <= 9, ( + f"{len(_OWED)} writers now owe a curation trace, up from 9. Adding one " + f"is a choice worth defending — the alternative is calling " + f"`record_curation_event` in the new tool (#325)." + ) + + +def test_the_two_records_that_do_have_history_are_event_logs_not_stubs(): + """The evidence for the reframing above, asserted rather than assumed. + + If these ever become empty or auto-generated boilerplate, the argument that + `curation_history` records what git cannot stops holding, and the 0.0 QC + threshold would need revisiting. + """ + import yaml + + with_history = { + path.name: (yaml.safe_load(path.read_text(encoding="utf-8")) or {}).get("curation_history") + for path in sorted((REPO / "kb/communities").glob("*.yaml")) + } + populated = {name: events for name, events in with_history.items() if events} + assert populated, "no record carries curation_history; the reframing needs re-checking" + for name, events in populated.items(): + for event in events: + assert event.get("action"), f"{name}: a curation event with no action" + assert len((event.get("changes") or "").split()) >= 5, ( + f"{name}: a curation event whose `changes` says almost nothing — " + f"an event log of stubs is the coverage metric #325 mistook this for" + )