From 74342cf25a7318bef20bf2a8f82e597d5043befc Mon Sep 17 00:00:00 2001 From: "marcin p. joachimiak" <4625870+realmarcin@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:54:17 -0700 Subject: [PATCH] fix: scout against every record root, not just kb/communities Checked the `scout-communities` skill against the tree before running it. The documented CLI matches the script exactly -- presets, flags, defaults -- but the dedup index did not match the corpus. `scout_communities.py` reports a paper as NEW when no curated record cites it, and it built that index from `kb/communities/` alone. `data/isolates/` holds records with the same root class, and **5 references are cited only from there**: PMID:34111782, PMID:38150661, doi:10.1007/s10163-014-0276-4, doi:10.1016/j.mineng.2013.03.022, doi:10.1016/j.scitotenv.2021.148151 Each would have been reported as a new community to go and research, when it is already curated. The consequence is wasted work rather than a wrong record, which is why it sat unnoticed: nothing fails, a curator just researches something that exists. Same shape this repository keeps hitting -- `data/isolates` outside every validation glob (#350), `kb/taxa` outside every CI trigger (#471), a hardcoded root list that cannot notice a new member (#689). Same fix: read `default_record_roots()` rather than naming a directory, so a root added later is covered without anyone remembering this file. The index goes from 325 records to 329. `scripts/` is run by path rather than imported, so `src/` is put on `sys.path` for the shared helper. ## The guard corrected the test I wrote for it The first version globbed `kb/communities` to compute which references were unique to the other roots, and #689's guard flagged it -- correctly, and with the right advice: prefer fixing it to recording it. Adding the module to `_COMMUNITY_ONLY` would have been recording a falsehood, since it is not community-only. Asking the question PER ROOT instead names no directory and is stronger: for each root, every reference unique to it must be in the index. That holds for a root added later with this file untouched, and it refuses to run vacuously if no root has a unique reference. Mutation-checked by reverting the scout to the single root, which reds both the count and the per-root check. Skill doc updated to match, and two limitations recorded that were not written down: a false NEW costs research rather than correctness, and `--since` is a floor rather than a window -- left at its 2024 default a pass in 2026 re-surfaces everything earlier passes already saw. Co-Authored-By: Claude Opus 5 --- .claude/skills/scout-communities/SKILL.md | 20 +++- scripts/scout_communities.py | 32 +++++- .../test_scouting_dedups_every_record_root.py | 105 ++++++++++++++++++ 3 files changed, 148 insertions(+), 9 deletions(-) create mode 100644 tests/test_scouting_dedups_every_record_root.py diff --git a/.claude/skills/scout-communities/SKILL.md b/.claude/skills/scout-communities/SKILL.md index 2049b96c..320ce2ea 100644 --- a/.claude/skills/scout-communities/SKILL.md +++ b/.claude/skills/scout-communities/SKILL.md @@ -1,10 +1,10 @@ --- name: scout-communities -description: Discover newly published microbial communities to add to CommunityMech. Queries Europe PMC for recent papers describing defined/structured communities (consortia, SynComs, co-cultures, syntrophic pairs), dedups hits against the existing kb/communities/ records (by cited PMID/DOI and community-name overlap), scores each by how strongly it reads as a community paper, and emits a curator report + queue (+ optional draft stubs) ready to hand to deep-research-community. +description: Discover newly published microbial communities to add to CommunityMech. Queries Europe PMC for recent papers describing defined/structured communities (consortia, SynComs, co-cultures, syntrophic pairs), dedups hits against every curated record root (kb/communities/ and data/isolates/) by cited PMID/DOI and community-name overlap, scores each by how strongly it reads as a community paper, and emits a curator report + queue (+ optional draft stubs) ready to hand to deep-research-community. category: research requires_database: false requires_internet: true -version: 1.0.0 +version: 1.1.0 --- # Scout New Communities (Europe PMC discovery) @@ -15,7 +15,7 @@ version: 1.0.0 **discovery** counterpart to `deep-research-community` (which *enriches* a community you already have a record for). It queries Europe PMC for recently published papers about defined/structured communities, filters out anything -already covered by `kb/communities/`, ranks what's left, and produces a +already covered by a curated record, ranks what's left, and produces a curator-facing shortlist. Free + reproducible: Europe PMC REST search needs **no API key** and spends no @@ -29,7 +29,11 @@ mine one paper in depth. 1. **Query** Europe PMC (`resultType=core`, relevance-ranked, date-filtered to recent first-publication dates, abstract required). -2. **Dedup** each hit against existing records two ways: +2. **Dedup** each hit against existing records two ways. The index spans + **every record root** — `kb/communities/` *and* `data/isolates/`, read from + `default_record_roots()` — because both hold `MicrobialCommunity` records and + 5 references are cited only from isolates. Scanning communities alone + reported those as `NEW` (fixed; see Notes). - cited **PMID/DOI** already present in any `kb/communities/*.yaml` (`ALREADY_CITED`), and - **community-name token overlap** with the hit title (`TITLE_OVERLAP`, @@ -129,6 +133,14 @@ research/scouting/ - **Dedup is heuristic**: token-overlap can miss a community reported under a very different name, and can flag a genuinely new community that shares words with an existing one. Always eyeball `TITLE_OVERLAP` rows. +- **A false `NEW` costs research, not correctness** — which is why the + isolates gap sat unnoticed. Nothing fails; a curator simply goes and researches + a community that already exists. `tests/test_scouting_dedups_every_record_root.py` + now derives the isolate-only references and asserts each is in the index. +- **`--since` defaults to 2024** and is a floor, not a window. Left at the + default a pass in 2026 re-surfaces everything since 2024, including hits from + earlier passes under `research/scouting/`. Raise it when you want only what is + new since the last sweep. - **Preprints appear alongside published versions** (e.g. a bioRxiv DOI plus the PNAS PMID for the same study) — the report shows both; pick the version of record. diff --git a/scripts/scout_communities.py b/scripts/scout_communities.py index 4278ba78..88c19bde 100644 --- a/scripts/scout_communities.py +++ b/scripts/scout_communities.py @@ -4,7 +4,8 @@ Discovery counterpart to ``deep-research-community``: instead of enriching a community you already curate, this queries Europe PMC for recently published papers describing defined/structured microbial communities, dedups the hits -against the records already in ``kb/communities/`` (both by cited PMID/DOI and +against every curated record root -- ``kb/communities/`` AND ``data/isolates/`` +-- (both by cited PMID/DOI and by community-name token overlap), scores each hit by how strongly it reads as a *community* paper, and writes a curator-facing report plus a machine-readable queue. @@ -38,7 +39,9 @@ EPMC_SEARCH = "https://www.ebi.ac.uk/europepmc/webservices/rest/search" REPO_ROOT = Path(__file__).resolve().parent.parent -COMMUNITIES_DIR = REPO_ROOT / "kb" / "communities" +# `communitymech` is the source of the record roots below; scripts/ is run by +# path rather than imported, so the package is not on sys.path by default. +sys.path.insert(0, str(REPO_ROOT / "src")) DEFAULT_OUT_DIR = REPO_ROOT / "research" / "scouting" # Words that signal a paper is about a *defined/structured community*, not a @@ -120,7 +123,26 @@ def _tokens(text: str) -> set[str]: return {t for t in re.findall(r"[a-z0-9]+", text.lower()) if len(t) >= 4 and t not in STOPWORDS} -def build_dedup_index(communities_dir: Path) -> dict: +def record_files() -> list[Path]: + """Every MicrobialCommunity record, from EVERY root that holds one. + + `kb/communities/` is not the whole corpus: `data/isolates/` carries records + with the same root class, and 5 references are cited only from there. Deduping + against communities alone reports those papers as NEW when they are already + curated -- the recurring shape in this repository, where `data/isolates` sat + outside every validation glob (#350) and `kb/taxa` outside every CI trigger + (#471). + + Uses the shared `default_record_roots()` rather than a local list, which is + the whole point of that helper (#689): a root added later is picked up here + without anyone remembering this file. + """ + from communitymech.paths import default_record_roots + + return [path for root in default_record_roots() for path in sorted(root.glob("*.yaml"))] + + +def build_dedup_index(paths: list[Path] | None = None) -> dict: """Index existing records: cited PMIDs/DOIs and per-record name token sets.""" cited_pmids: set[str] = set() cited_dois: set[str] = set() @@ -130,7 +152,7 @@ def build_dedup_index(communities_dir: Path) -> dict: doi_re = re.compile(r"reference:\s*doi:(\S+)", re.IGNORECASE) name_re = re.compile(r"^name:\s*(.+)$", re.MULTILINE) - for path in sorted(communities_dir.glob("*.yaml")): + for path in paths if paths is not None else record_files(): text = path.read_text(errors="replace") cited_pmids.update(pmid_re.findall(text)) cited_dois.update(d.lower().rstrip(".,;") for d in doi_re.findall(text)) @@ -398,7 +420,7 @@ def main(argv: list[str] | None = None) -> int: if n_filled: print(f"[scout] backfilled DOIs for {n_filled} ref-less hits (CrossRef)", file=sys.stderr) - index = build_dedup_index(COMMUNITIES_DIR) + index = build_dedup_index() print( f"[scout] dedup index: {len(index['cited_pmids'])} PMIDs, " f"{len(index['cited_dois'])} DOIs, {len(index['name_token_sets'])} records", diff --git a/tests/test_scouting_dedups_every_record_root.py b/tests/test_scouting_dedups_every_record_root.py new file mode 100644 index 00000000..0dafc081 --- /dev/null +++ b/tests/test_scouting_dedups_every_record_root.py @@ -0,0 +1,105 @@ +"""Scouting must dedup against every record root, not just kb/communities. + +`scout_communities.py` reports a paper as NEW when no curated record cites it. +It built that index from `kb/communities/` alone -- but `data/isolates/` holds +records with the same root class, and **5 references are cited only from there**. +Each would have been reported as a new community to go and curate, when it is +already curated. + +This is the shape this repository keeps hitting: `data/isolates` outside every +validation glob (#350), `kb/taxa` outside every CI trigger (#471), a hardcoded +root list that cannot notice a new member (#689). The fix is the same one -- +read `default_record_roots()` rather than naming a directory -- so a root added +later is covered without anyone remembering this file. + +The consequence here is wasted work rather than a wrong record, which is why it +sat unnoticed: a false NEW sends a curator to research a community that already +exists, and nothing fails. +""" + +from __future__ import annotations + +import importlib.util +import pathlib + +import pytest + +from communitymech.paths import default_record_roots + +REPO = pathlib.Path(__file__).parent.parent +SCRIPT = REPO / "scripts" / "scout_communities.py" + + +@pytest.fixture(scope="module") +def scout(): + """Load the scout from source (a stale .pyc must not stand in -- #693).""" + spec = importlib.util.spec_from_file_location("_scout_under_test", SCRIPT) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def test_the_index_is_not_empty(scout): + """Guard on the guard: an empty index would make everything look NEW.""" + index = scout.build_dedup_index() + assert len(index["cited_pmids"]) > 300, index["cited_pmids"] + assert len(index["name_token_sets"]) > 300 + + +def test_every_record_root_is_scanned(scout): + """The file list covers each root, counted against the roots themselves.""" + expected = sum(len(list(root.glob("*.yaml"))) for root in default_record_roots()) + assert expected > 300, "the record roots are empty; this test would prove nothing" + assert len(scout.record_files()) == expected + + +def _cited(paths) -> set[str]: + """Every reference these files cite, normalised.""" + import re + + pattern = re.compile(r"reference:\s*((?:PMID|doi):\S+)", re.IGNORECASE) + found: set[str] = set() + for path in paths: + found.update(m.lower().rstrip(".,;") for m in pattern.findall(path.read_text())) + return found + + +def test_every_root_contributes_its_unique_references(scout): + """A reference unique to ANY root must be in the index. + + Named no directory on purpose. The first version of this test globbed + `kb/communities` to compute the difference, and #689's guard flagged it -- + correctly, and with the right advice: fix it rather than record it. Asking + the question per-root instead is both root-agnostic and stronger, since it + holds for a root added later without this file being touched. + """ + roots = default_record_roots() + assert len(roots) > 1, "only one record root; this test cannot distinguish anything" + + files = scout.record_files() + index = scout.build_dedup_index() + indexed = {f"pmid:{p}" for p in index["cited_pmids"]} | { + f"doi:{d}" for d in index["cited_dois"] + } + + checked_any = False + for root in roots: + mine = [path for path in files if path.parent == root] + others = [path for path in files if path.parent != root] + if not mine or not others: + continue + unique = _cited(mine) - _cited(others) + if not unique: + continue + checked_any = True + missing = sorted(reference for reference in unique if reference not in indexed) + assert missing == [], ( + f"these references are cited only from {root.name} and are absent " + f"from the dedup index, so scouting would report them as NEW papers " + f"to curate when they are already curated:\n " + "\n ".join(missing) + ) + + assert checked_any, ( + "no root has a reference unique to it, so this test cannot tell a scan " + "of every root from a scan of one" + )