The Python tier has no static lane: 10 dead imports, and shell gets ShellCheck while tools/ gets nothing nf-8ip ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/669 by tools/beads/import_github.py's reconcile pass.

Filed unclaimed from a quiet poll tick, nf-dev-probe-gate. Measured at 4b785ac, and the negative results are included because they are half the value.

The asymmetry

presubmit.py gates three lanes: dotnet, python, shellcheck. The shell tier — a handful of files that CLAUDE.md restricts to "resolve paths and exec one thing" — has a static analyser wired into the gate. The Python tier, which is tools/ plus the root entry points and is the thing CLAUDE.md calls "the product", has only unittest. Nothing checks a name that does not exist, an import that does nothing, or a symbol that moved.

No linter config exists anywhere in the repo (no ruff.toml, .flake8, pyproject.toml, setup.cfg), and none is installed on this box.

What that costs today — 10 dead imports, hand-verified

A ~40-line stdlib ast pass over all 57 Python files, excluding __future__ imports and anything carrying a deliberate # noqa side-effect marker:

| file | dead import | |---|---| | presubmit.py:42 | re | | presubmit.py:48 | Iterable | | tools/gen_packs.py:31 | sys | | tools/nflib/__init__.py:50 | shutil | | tools/office/audit_sessions.py:77 | Iterable | | tools/pr/poll.py:44 | subprocess | | tools/pr/poll.py:47 | zlib | | tools/pr/poll.py:49 | Iterable | | tools/reviewer/dispatch.py:74 | field | | tools/tests/test_publish.py:18 | json |

Six of the ten hand-checked line by line; the rest match the same pattern. Individually trivial. Collectively they are the visible half of a class whose invisible half is *a name that no longer exists*, and poll.py's subprocess/zlib are the interesting entries: they are residue from a shape that changed, which is notes/history.md's recurring story. #479 already records one of these being caught by hand, in review, one at a time — which is the expensive way.

⚠️ The negative results, so nobody re-derives them

A linter would NOT have caught the bug that sent me looking. Writing the flake-retry tests tonight I passed drive_main(swept=…) into a helper whose body opens with swept = [] — the parameter was silently discarded, and the test asserted against a value that never arrived. It failed only because I happened to assert an exact timestamp. pylint's redefined-argument-from-local fires on loop and with targets, not on a plain assignment, so no default rule set catches it. Anyone filing "a linter would have saved us" should know that it would not have.

Parameter shadowing is not a problem in this repo. Same AST pass found 22 parameters rebound in-function; 17 are x = x.strip()-style normalization (idiomatic) and the rest are guarded if x is None: x = … defaults (correct). I checked dispatch.py:168 and test_spawn_dev.py:287 by hand expecting bugs and found neither. Zero defects in that class — worth recording so the next person does not spend the same half hour.

What is actually worth deciding
  1. Which lane. ruff is one binary, no config needed for F401/F821, and runs in well under a second on 57 files — it would sit beside shellcheck in presubmit.py naturally. ⚠️ It is a new external dependency on three boxes, and #504 is the standing example of what that costs: the shellcheck lane silently skipped on the sandbox box for weeks because the exe was installed but not on PATH. Whatever lands must fail loudly when the tool is missing, not skip.
  2. Or no dependency at all. The measurement above is stdlib ast and took 40 lines. A tools/tests/test_no_dead_imports.py in the existing python lane would catch F401 with zero new tooling, zero install story, and no --doctor line. Less coverage than ruff (no undefined-name check), but it cannot silently skip.

My recommendation is (2) first — it is one test file, it runs everywhere the suite already runs, and it converts this issue into a gate tonight — and ruff later if undefined-name coverage is wanted, once someone owns the install story on all three boxes.

Related: #504 (a gating lane that silently skipped), #479 (a dead import found by hand in review), #614 / *Nothing checks the claim* (the chunk this belongs to).

Dependencies

None.

Comments

No comments.

Add a comment