test: empty _OWED_BOTH_ROOTS — all eleven gates now sweep both roots (#689) - #699
Merged
Conversation
…#689) The guard added in #692 recorded eleven test modules that swept kb/communities where data/isolates holds the same content. This converts all eleven and empties the list. Checked BEFORE converting, not hoped for afterwards: the truncation gate examined 66 isolate snippets and flagged 0, no isolate interaction participant sits outside its taxonomy, and the auditor reports 0 issues of any kind across the 4 records. So this adds coverage rather than a defect backlog -- the value is that a defect arriving in an isolate tomorrow is visible to eleven gates that could not have seen it. `communitymech.paths` gains `record_files()` and `record_path()`. `default_record_roots()` answers "which directories" and almost every caller then wrote the same glob line; eleven skipped that step entirely and globbed one root. One function, so a module cannot get half the corpus by writing slightly less code than the module beside it. Two of the eleven needed more than a directory. `test_ncbi_domain_scope` and `test_gtdb_near_tie_marker` iterate `document["taxonomy"]`, and a CommonTaxon in kb/taxa has no such key -- the trap `taxon_blocks.iter_taxon_descriptors` was written for in #656. They now use `taxon_descriptor_roots()` with that walker, and reach 16 isolate and 2 kb/taxa descriptors that were previously invisible; gtdb_classification blocks seen: 752 -> 755. `test_network_auditor` was the sharpest. It passed `communities_dir=` explicitly, silently replacing the auditor's own `default_record_roots()` default -- the shape #350 fixed IN the auditor and left standing in its test. The override is gone, so the default is under test as much as the corpus is, and an assertion now requires an isolate to be among the audited records. `test_snippet_rendering_artefacts` monkeypatched the COMMUNITIES constant to drive its mutation check over one record. Sweeping both roots makes the walk a call rather than a directory, so the seam moves to a `_record_files()` indirection; the mutation check still fires. The scanner guard is recalibrated against `_COMMUNITY_ONLY` instead of a written count. The population legitimately FELL from 14 to 3, so a fixed threshold would have had to be lowered -- and a threshold lowered to match reality measures nothing. What must stay true is that the scanner still sees the modules classified as single-root. Verified: 2831 passed, 16 skipped. Conversion proven non-vacuous by collection -- the 4 isolate records now appear as parametrised cases in test_snippet_truncation, which collected only kb/communities before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial review of the previous commit. Four findings, all in my own work.
1. THE GUARD WAS WEAKER THAN IT LOOKED. `_sweeping_test_modules()` skipped any
module mentioning `record_files`/`default_record_roots`, treating the
presence of a NAME as proof of its USE. test_no_vacuous_go_annotations
imported record_files, never called it -- its sweep read
`(corpus or COMMUNITIES).glob(...)`, which the conversion's pattern did not
match -- and ruff then deleted the now-unused import, leaving a module that
looked converted, was not, and passed this scan twice over.
Detection is now structural: a module that still globs a constant bound to
kb/communities is flagged whatever else it says. A genuinely converted module
cannot trip it, because it no longer has such a glob.
2. That stricter scan immediately surfaced two more, both previously hidden:
test_gtdb_status_writer swept one root for curated GTDB pins, and
test_ncbi_domain_scope had a SECOND sweep at line 134 that I missed while
converting line 82 -- a module I had already claimed to convert.
3. test_interaction_participants_outside_taxonomy swept both roots but then
constructed `NetworkIntegrityAuditor(COMMUNITIES)`, handing the auditor one
root -- the same override I had just removed from test_network_auditor, one
file away. Three modules had dead COMMUNITIES constants left behind.
4. The stricter scan then produced a FALSE positive:
test_isolates_are_covered_by_id_checks was flagged for a docstring that
DISCUSSES reverting a loop to `Path("kb/communities").glob("*.yaml")` -- it
describes the defect it exists to prevent. The scan now strips docstrings and
comments first; a guard that cannot tell code from commentary teaches people
to reword their explanations.
Reindenting the status-writer sweep also moved its `assert curation_note`
outside the `if block.get("curated")`, so it briefly asserted a note on every
descriptor and reported a curated pin in a record containing zero occurrences of
`curated:`. Chased to the edit rather than "fixed" in the corpus, and
mutation-checked afterwards: blanking the note in the pinned record fails the
test, restoring it passes.
2833 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #689.
The guard added in #692 recorded eleven test modules that swept
kb/communitieswheredata/isolatesholds the same content. This converts alleleven and empties the list.
Checked before converting, not hoped for afterwards
data/isolatesSo this adds coverage rather than a defect backlog. The value is that a defect
arriving in an isolate tomorrow is visible to eleven gates that could not have
seen it.
The shared helper
communitymech.pathsgainsrecord_files()andrecord_path().default_record_roots()answers which directories, and almost every callerthen wrote the same glob line — eleven skipped that step entirely and globbed one
root. One function, so a module cannot get half the corpus by writing slightly
less code than the module beside it.
Three needed more than a directory swap
test_ncbi_domain_scopeandtest_gtdb_near_tie_markeriteratedocument["taxonomy"], and aCommonTaxoninkb/taxahas no such key —exactly the trap
taxon_blocks.iter_taxon_descriptorswas written for in Six independent copies of 'where the records live', and they already disagree #656.They now use
taxon_descriptor_roots()with that walker and reach 16 isolateand 2 kb/taxa descriptors that were previously invisible;
gtdb_classificationblocks seen 752 → 755.test_network_auditorpassedcommunities_dir=explicitly, silentlyreplacing the auditor's own
default_record_roots()default — the shape data/isolates/** now triggers validate-strict, but that job does not validate isolates #350fixed in the auditor and left standing in its test. The override is gone, so
the default is under test as much as the corpus is, and an assertion now
requires an isolate among the audited records.
test_snippet_rendering_artefactsmonkeypatched theCOMMUNITIESconstantto drive its mutation check over one record. Sweeping both roots makes the walk
a call rather than a directory, so the seam moves to a
_record_files()indirection; the mutation check still fires.
The scanner guard is recalibrated, not lowered
It asserted
len(found) >= 10. The population legitimately fell from 14 to 3when the eleven were converted, so a fixed threshold would have had to be
lowered — and a threshold lowered to match reality measures nothing. It now
calibrates against
_COMMUNITY_ONLY: the scanner must still see every moduleclassified as single-root, which is what the checks below it are about.
Verification
Proven non-vacuous by collection rather than by reading: the 4 isolate records
now appear as parametrised cases in
test_snippet_truncation, which collectedonly
kb/communitiesbefore.Known unrelated failure:
test_no_snippet_stops_mid_wordfails locally ondata/isolates/Methylobacterium_REE_Ewaste_Platform.yaml, driven by an untrackedlocal cache; it skips on a clean checkout.