What
repo_tests/_reach.py's discover callable is handed a root on purpose: reach_declarations_test.py::test_no_guard_can_succeed_against_an_empty_tree and ::test_discovery_honours_the_root_it_is_given hand every declaration an empty directory and require the sweep to come back empty, which is the one mechanical proof that a floor can fire.
Some discover callables reach for repo_root() part-way down instead of using that root, so part of their discovery answers a question about the REAL tree while the rest answers one about the root they were given. Where that happens the empty-tree mutation still passes — but for the wrong reason, and it reads identically to the honest case.
Confirmed instance
repo_tests/docs_liquid_tags_are_defined_17930_test.py:
_liquid_rendered_docs(root) → _non_excluded_docs(root) → excluded_dirs()
excluded_dirs() reads repo_root() / "docs" / "_config.yml", never root / ...
Against the empty tree the sweep still returns [], so both meta-tests pass — but because (empty / "docs").rglob(...) matches nothing, not because the exclude list was read from the tree under test. The root is honoured by the walk and ignored by the predicate that shapes it.
Why this is not a drive-by fix
Changing it to excluded_dirs(root) makes the empty tree a case the function has to answer for, and the obvious answer is a silent one:
- on the empty tree
root / "docs" / "_config.yml" does not exist, so read_text raises FileNotFoundError;
Reach.population converts any raise into ReachDiscoveryError, and the empty-tree test needs an empty RESULT, not an exception — so the fix fails the meta-test it was meant to strengthen;
- the obvious remedy — catch and return
frozenset() — introduces a silent-empty path in the REAL tree too: an unreadable or malformed _config.yml would then mean "nothing is excluded", docs/archives/ (107 files, carrying tags Liquid does not define) would enter the population, and the guard would start reporting files the site never renders. That is the correct-about-the-text / wrong-about-the-build failure the guard's own docstring exists to prevent.
The honest fix has to keep "this root has no config" and "this config could not be read" distinguishable. That is a design decision about _reach's contract, not a one-liner, which is why it is filed rather than folded into the PR that found it.
Scope — a class, not an instance
A transitive AST sweep of repo_tests/*.py for declarations whose discover (or anything it calls) invokes repo_root() returns 7 candidates:
| guard |
discover |
reaches repo_root() in |
async_sqlalchemy_declares_greenlet_17432_test.py |
_declares_sqlalchemy |
itself |
docs_liquid_tags_are_defined_17930_test.py |
_liquid_rendered_docs |
excluded_dirs |
dpkg_tests_installed_state_not_knowledge_17387_test.py |
_code_lines_using_dpkg_list |
itself |
dpkg_tests_installed_state_not_knowledge_17387_test.py |
_dpkg_registrations |
itself |
one_claim_registry_16653_test.py |
_iter_sources |
itself |
slm_frontend_bare_ui_literals_test.py |
_components |
itself |
slm_frontend_i18n_parity_test.py |
_en_message_keys |
itself |
These are candidates, not confirmed defects. Only the second was checked by hand. A repo_root() call inside a discover is legitimate where it names a reference the root is not supposed to change; each of the other six needs reading before it is called anything.
Two measurement notes on that table, because the sweep is the kind of instrument this repo keeps catching:
- The first version followed call edges one hop and returned 6 — it missed the very instance that prompted the issue, which sits two hops down (
_liquid_rendered_docs → _non_excluded_docs → excluded_dirs). The table above is the transitive version. A one-hop answer here is correct, complete, and about a different question.
- It only sees calls by bare name within one module. A helper imported from another module, or reached through an attribute, is invisible to it, so 7 is a floor on the candidate set and not a count of it.
Acceptance criteria
Context
Found while fixing the same guard's population under #17930. Related: #15826 (tree-scanning guards that can report clean having examined nothing), #17844 (declare()'s what= is unverified free text, so a guard can be floored correctly over the wrong population) — all three are the same family: the reach machinery proves something adjacent to the thing it reads as proving.
What
repo_tests/_reach.py'sdiscovercallable is handed arooton purpose:reach_declarations_test.py::test_no_guard_can_succeed_against_an_empty_treeand::test_discovery_honours_the_root_it_is_givenhand every declaration an empty directory and require the sweep to come back empty, which is the one mechanical proof that a floor can fire.Some discover callables reach for
repo_root()part-way down instead of using thatroot, so part of their discovery answers a question about the REAL tree while the rest answers one about the root they were given. Where that happens the empty-tree mutation still passes — but for the wrong reason, and it reads identically to the honest case.Confirmed instance
repo_tests/docs_liquid_tags_are_defined_17930_test.py:_liquid_rendered_docs(root)→_non_excluded_docs(root)→excluded_dirs()excluded_dirs()readsrepo_root() / "docs" / "_config.yml", neverroot / ...Against the empty tree the sweep still returns
[], so both meta-tests pass — but because(empty / "docs").rglob(...)matches nothing, not because the exclude list was read from the tree under test. The root is honoured by the walk and ignored by the predicate that shapes it.Why this is not a drive-by fix
Changing it to
excluded_dirs(root)makes the empty tree a case the function has to answer for, and the obvious answer is a silent one:root / "docs" / "_config.yml"does not exist, soread_textraisesFileNotFoundError;Reach.populationconverts any raise intoReachDiscoveryError, and the empty-tree test needs an empty RESULT, not an exception — so the fix fails the meta-test it was meant to strengthen;frozenset()— introduces a silent-empty path in the REAL tree too: an unreadable or malformed_config.ymlwould then mean "nothing is excluded",docs/archives/(107 files, carrying tags Liquid does not define) would enter the population, and the guard would start reporting files the site never renders. That is the correct-about-the-text / wrong-about-the-build failure the guard's own docstring exists to prevent.The honest fix has to keep "this root has no config" and "this config could not be read" distinguishable. That is a design decision about
_reach's contract, not a one-liner, which is why it is filed rather than folded into the PR that found it.Scope — a class, not an instance
A transitive AST sweep of
repo_tests/*.pyfor declarations whosediscover(or anything it calls) invokesrepo_root()returns 7 candidates:repo_root()inasync_sqlalchemy_declares_greenlet_17432_test.py_declares_sqlalchemydocs_liquid_tags_are_defined_17930_test.py_liquid_rendered_docsexcluded_dirsdpkg_tests_installed_state_not_knowledge_17387_test.py_code_lines_using_dpkg_listdpkg_tests_installed_state_not_knowledge_17387_test.py_dpkg_registrationsone_claim_registry_16653_test.py_iter_sourcesslm_frontend_bare_ui_literals_test.py_componentsslm_frontend_i18n_parity_test.py_en_message_keysThese are candidates, not confirmed defects. Only the second was checked by hand. A
repo_root()call inside a discover is legitimate where it names a reference the root is not supposed to change; each of the other six needs reading before it is called anything.Two measurement notes on that table, because the sweep is the kind of instrument this repo keeps catching:
_liquid_rendered_docs→_non_excluded_docs→excluded_dirs). The table above is the transitive version. A one-hop answer here is correct, complete, and about a different question.Acceptance criteria
_reach.declare's contract states what adiscovermust do when the root it is given has no configuration to read, and how that differs from a configuration it cannot readdocs_liquid_tags_are_defined_17930_test.pyhonours itsrootend to end, with a test proving the malformed-config case still FAILS rather than yielding an empty exclude setContext
Found while fixing the same guard's population under #17930. Related: #15826 (tree-scanning guards that can report clean having examined nothing), #17844 (
declare()'swhat=is unverified free text, so a guard can be floored correctly over the wrong population) — all three are the same family: the reach machinery proves something adjacent to the thing it reads as proving.