refactor: a private name must have a reader - #3583
cagataycali merged 2 commits into
Conversation
_VALID_MODES in strands_robots.robot named the three mode spellings while every site that decides or reports a mode spelled them inline, so the constant documented an owner that did not exist. _PEERS_VERSION in strands_robots.mesh.session was incremented under the registry lock at every insert, eviction, prune and clear and consulted by nothing - four write sites and three global declarations maintaining a change signal no cache ever read. Both go. tests/test_no_private_name_is_defined_and_never_read.py pins the invariant for module-level private names and private methods across the package, the module-scope half of what test_no_private_state_is_written_and_never_read.py holds for instance attributes. It fails on the previous tree naming exactly these two.
yinsong1986
left a comment
There was a problem hiding this comment.
Summary
Removes two private module-level names that are defined and never read anywhere in the tree — _VALID_MODES in strands_robots.robot (a constant every mode-deciding site bypassed by spelling the literals inline) and _PEERS_VERSION in strands_robots.mesh.session (a version counter incremented under _PEERS_LOCK at four sites and consulted by nothing) — and pins the invariant with a new whole-tree grader, tests/test_no_private_name_is_defined_and_never_read.py. Verified independently: neither name is read in strands_robots/, tests/, tests_integ/, examples/ or scripts/, and neither appears in a string literal, so the removal changes no observable behavior; all _PEERS mutations stay under the existing lock unchanged. This matches AGENTS.md's "No dead code" rule directly.
What's good
- The grader excludes its own file from the mention-tree read, so naming the offenders in its docstring cannot vacuously clear them, and it carries a self-contained synthetic-module test plus a non-vacuity check (
file_count > 200,definitions_seen > 1000) so a scan that walks nothing cannot pass silently. - The exemption ledger (
DEFINED_WITHOUT_A_READER_BY_DESIGN) requires a stated reason per entry and is graded for it — the same shape the repo's other tree-wide invariants use. - Walk root is a module-level literal (
REPO_ROOT / "strands_robots"), the shape the whole-tree-check roster collects on arrival (PR Workflow step 2), and the:mod:xref in the docstring resolves to a module that exists onmain. - Composition against the 9 open siblings is reported in the description, which is exactly the diligence a new walk-based grader owes (each sibling reports the same two names, so no sibling goes red on its next base absorb).
- Changelog fragment present and correctly named; net -11 LOC in the package; no host paths, no emojis.
robot.py_VALID_MODESmesh/session.py_PEERS_VERSIONglobal, 4+= 1, 0 readsWhat Two private module-level names are defined and never read in the tree. Both go, with the seven statements that maintained the second.
Why The module-scope half of #3580.
_VALID_MODESnamed the three mode spellings while every site deciding a mode spelled them inline;_PEERS_VERSIONcounted registry changes for a cache never built. Neither is reachable fromtests/,examples/orscripts/, nor named in a string literal.Tests
test_no_private_name_is_defined_and_never_read.pypins the invariant for module-level private names and private methods, failing pre-fix on exactly these two. Net −11 LOC.