Remove the five scripts that never ran, and guard the cause (#410, #523) - #522
Open
realmarcin wants to merge 2 commits into
Open
Remove the five scripts that never ran, and guard the cause (#410, #523)#522realmarcin wants to merge 2 commits into
realmarcin wants to merge 2 commits into
Conversation
They imported `communitymech.literature_enhanced`, a module absent from all 498 commits — they fail before `--help`. #487 already fixed the one working tool that invoked one; what was left was the decision the tests recorded as open: port them, or drop them. Dropped. Porting was never an import swap. Their CLI flags advertise a 6-tier PDF cascade with "fallback mirrors" and `LiteratureFetcher` has no PDF surface, so a port meant *building* that — retrieving publisher PDFs through mirrors is not something to add speculatively. The need underneath it is open-access full text, and `scripts/cache_fulltext.py` serves it: the #183 sweep used it to cache full text for 64 of 125 references. docs/pdf_fetching_capability.md now maps each removed script to what to use instead. The more useful change is to the guard. `_KNOWN_BROKEN` was five names and three tests keeping the list honest — that records breakage, it does not prevent it, and a sixth script importing a sixth phantom module would just have been added to it. Every `from communitymech.X import ...` in scripts/ is now resolved against the installed package, so the next one fails at the moment it is written. The list stays, empty, because the removed names are still dead pointers for any working tool that prints them. Two things caught while doing it, both by tests already here: emptying the constant made it `_KNOWN_BROKEN: set[str] = set()`, an AnnAssign, which the sibling test's Assign-only AST walk stopped finding — it went red rather than passing on an empty set. And the >= 5 bound now rests on the removed names, or it would have started passing on nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three docs and .gitignore said "NOT FUNCTIONAL ... tracked in #410". True until this PR; now stale in a new way, because the files are gone — a reader looks for a script that is not there and cannot tell whether it was deleted or they are on the wrong branch. They now say REMOVED and name the replacement. The gap that let this sit: nothing walked docs/ for script references. #410's guard and the one replacing it both check scripts/ — print and subprocess calls in Python files. A curator following a runbook is reading prose, which is exactly where neither looks. The new test checks existence, not tone, and that is a correction of my own first attempt. I filed #523 claiming AUTOMATION_TOOLS.md gave a bare unwarned instruction; it did not — the warning sat two lines above the command, and I had grepped for the script name and read only the line it matched. Third time this session a line-scoped scan has missed the prose that negates the hit. Whether a reference is adequately caveated is a judgement; whether the file exists is a fact, and only the fact belongs in a test. pdf_fetching_capability.md is exempt by name: it carries a document-level banner saying everything below it describes software that was never here, which a per-line check cannot see, and rewriting it would destroy the record it exists to keep. Mutation-checked: appending a reference to a nonexistent script reddens it. 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
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.
What was left of #410
All five remaining scripts began
from communitymech.literature_enhanced import EnhancedLiteratureFetcher. That module is absent from all 498 commits — including the one that added the scripts. They fail at import; even--helpdoes not work.#487 already fixed the one working tool that invoked one. What remained was the decision the tests recorded as deliberately open: port them, or drop them.
Dropped, and why
Porting was never an import swap — the scripts' own docstrings say so. Their CLI flags advertise a 6-tier PDF cascade with "fallback mirrors";
LiteratureFetcherhas no PDF surface at all. A port meant building that, and retrieving publisher PDFs through mirrors is not something to add speculatively.The real need underneath is open-access full text, and
scripts/cache_fulltext.pyalready serves it — the #183 sweep used it to cache full text for 64 of 125 references. Reversible:git revertbrings them back, and a test says what they'd need first.The guard is the more useful half
_KNOWN_BROKENwas five names plus three tests keeping the list honest. That records breakage; it does not prevent it — a sixth script importing a sixth phantom module would just have joined the list.test_no_script_imports_a_module_that_does_not_existnow resolves everyfrom communitymech.X import ...inscripts/against the installed package, so the next one fails when it is written. The list stays, empty, because the removed names are still dead pointers for any tool that prints them.Two things the existing tests caught
_KNOWN_BROKEN: set[str] = set()— anAnnAssign. The sibling test'sAssign-only AST walk stopped finding it and went red rather than passing on an empty set.assert len(dead) >= 5would have started passing on nothing. It now rests on the removed names — a gone file is as dead a pointer as one that cannot start.test_that_guard_can_actually_failexists because a list-driven check passes cleanly once its list is empty, which is exactly this file's new state.Round two: the docs (#523)
Nothing walked
docs/for script references — both guards checkscripts/only, and a curator following a runbook is reading prose. Three docs and.gitignoresaid "NOT FUNCTIONAL … tracked in #410", true until this PR and now stale in a new way: the files are gone, so a reader can't tell deletion from being on the wrong branch. They now say REMOVED and name the replacement.I filed #523 on a false premise and corrected it. I claimed
AUTOMATION_TOOLS.mdgave a bare unwarned instruction. It didn't — the warning sat two lines above the command, and I had grepped for the script name and read only the matching line. Third time this session a line-scoped scan missed the prose that negates the hit. The test that resulted checks existence, not tone: whether a reference is adequately caveated is a judgement, whether the file exists is a fact, and only the fact belongs in a test.pdf_fetching_capability.mdis exempt by name — it carries a document-level banner that a per-line check cannot see, and rewriting it would destroy the record it exists to keep.Checks
uv run pytest tests/— 2371 passed, 16 skippedjust validate-strict,just lint,just check-docs-current— exit 0communitymechmoduleCloses #410, closes #523.
🤖 Generated with Claude Code