Both cost a full revert cycle in the wsg_grabber work: - ruff TC001 rewrites a re-exported annotation-only class into the TYPE_CHECKING block. __all__ does not stop it. Lints clean, AttributeError at import. Always finish a re-export split by resolving the name in a real interpreter. - Whatever the tests patch (patch.object(ui, "tk", ...)) or call directly (mpv._note_errors) is pinned to its module and cannot move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
11 KiB
Resume: enforce the 250-line file cap
Paste this whole file to a fresh Claude session opened in
~/testsAndMisc. It is self-contained. Continuesrefactor_claude_todo.md, which is still the spec — read it too, but the decisions below override it where they differ.
Where things stand
192 files over 250 lines (was 198). All work so far is committed and pushed
to main; git status is clean apart from pre-existing unrelated edits to
README.md, billsplit/analysis_options.yaml, focus_owner/analysis_options.yaml
— leave those three alone, they are not yours.
Done already (do not redo):
| Commit | What |
|---|---|
~/utils |
plans/ + sessions/ exempted from the cap (specs/ still capped) |
17b20e5 |
New no-inline-python pre-commit hook + the one violation fixed |
004b4b0 |
CLAUDE.md 259 → 141 lines |
72a5e28 |
poker-stakes/ tracked in git (58 files) |
73b85f5 |
poker-stakes/ 4 violations → 0 |
The live worklist
Do not work from a list in this file — it goes stale. Run:
bash ~/utils/scripts/check_file_length.sh --all
Current distribution: linux_configuration 84, python_pkg 50,
phone_focus_mode 13, kcd2_dice_solver 12, focus_owner 11, billsplit 6,
reverse_survivors/meta/docs 4 each, .github/bucket_catch 2 each.
48 of them are near-misses at 251–300 lines — highest count-drop per unit
of effort, do these first.
Decisions already made (do not re-ask)
- Full clearance to
--allexit 0. When context runs out, write a fresh handoff like this one rather than stopping mid-way. - CI checks only files in the push/PR range, not
--all. docs/superpowers/plans/**+sessions/**are exempt — already done.poker-stakes/is tracked — already done.- For prose: automate what can be automated, delete the automated lines, keep only HOW, never WHY.
- Shell verification is
bash -n+shellcheck+systemctl catpath checks. Never execute enforcement scripts that mutate the phone or the live system. - One commit per logical unit.
The constraint that will bite you
Wire the file-length gate LAST, only once --all already exits 0.
meta/scripts/ci_mirror.sh:109 runs pre-commit run --all-files as a
pre-push hook, and .github/workflows/pre-commit.yml:105 does the same in
CI. Registering the hook while violations remain makes every git push fail
until the last file is fixed. Do not scope around it with exclude:/files: —
that is the allowlist the spec forbids.
Final commit, once and only once the tree is clean:
- id: file-length
name: file length <= 250 lines
entry: bash /home/kuhy/utils/scripts/check_file_length.sh
language: system
Plus .github/workflows/file-length.yml modelled on
~/todo/.github/workflows/file-length.yml (checks out kuhyx/utils into
.utils), scoped to changed files. Then prove it fails: stage a deliberately
251-line file, confirm git commit aborts, delete it.
The Python re-export form — settled empirically, do not re-derive
When a split moves a public name out of a module that callers/tests still
reach through the old path, the re-export must be marked with __all__:
from python_pkg.pkg._new_module import show_logs, summarise
__all__ = ["build_parser", "main", ..., "show_logs", "summarise"]
The PEP 484 alias form from x import y as y does not work here: ruff
raises PLC0414 (useless-import-alias), auto-fixes it away, and then removes
the now-"unused" import under F401 — silently breaking cli.summarise.
Verified on wsg_grabber/cli.py; __all__ passes ruff + mypy + pylint clean.
Splits that only move private helpers (leading _, no external caller)
need no shim at all — check with grep first, as in catalog.py.
__all__ only needs the re-exported names, not every public name in the
module — names defined locally stay reachable either way. Verified against the
full gate on store.py.
__all__ does not save you from ruff's TC001. It suppresses F401, but
a re-exported class used only in annotations gets rewritten into the
TYPE_CHECKING block, which lints perfectly and raises AttributeError at
import time. This nearly shipped a broken ui.Callbacks that cli.py
constructs. A class that must be reachable at runtime from module X stays
defined in module X. Always finish a re-export split with
PYTHONPATH=. python3 -c "from python_pkg.pkg import mod; mod.TheName"
Two other things the gate will not catch for you:
- Check what the tests patch before choosing a seam.
test_ui.pydoespatch.object(ui, "tk", fake), so every line touchingtkinterhad to stay inui.py; moving widget construction out errored 22 tests. Grep the test file forpatch.object(<module>first and treat those names as pinned. - Tests that call private methods pin them too —
test_player.pycallsmpv._note_errors(...), so that method stayed as a one-line delegate to the moved implementation. Worth ~5–8 lines on every file with this shape, which matters: three splits so far landed 2–6 lines over the cap and needed a second seam. Budget the new import block and__all__before deciding a two-way split is enough.
Splitting a class: if tests call private methods as bound attributes
(worker._fetch_thread(...) — grep the tests first), module-level functions
will not do; use mixins. The shape that passes every gate with no
suppression (settled on downloader.py, do not re-derive):
# _worker_base.py
class WorkerBase(ABC):
_deps: WorkerDeps # bare annotations for read-only attributes
_downloaded: int = 0 # real default for any attribute the mixin ASSIGNS
@abstractmethod
def _publish(self, event: DownloadEvent) -> None:
"""Supplied by the concrete Worker."""
Both mixins inherit WorkerBase; Worker(ScanMixin, DownloadMixin). Why not
the obvious alternatives:
- Naming a class
*Mixindoes not get you pylint'sno-memberexemption here —ignored-checks-for-mixinsis not configured for it. Don't rely on it. raise NotImplementedErrorstub bodies are uncovered lines under the 100% branch gate.if TYPE_CHECKING: def _publish(...) -> None: ...satisfies mypy but leaves pylint reportingno-member.- A bare annotation (
_downloaded: int) is not enough for an attribute the mixin assigns to (+= 1); pylint wants a real default.
Watch the MRO: Worker(ScanMixin, DownloadMixin) means ScanMixin wins any
name collision.
Coverage: meta/pyproject.toml sets source = ["python_pkg"], so a new
sibling module is measured automatically. Confirmed: new modules show up in
the report at 100% rather than being silently skipped. Add no new branches
(no if TYPE_CHECKING guards beyond what moved, no defensive try/except).
Split recipes
- Shell — extract into
lib/*.shsourced by a thin entry script,set -euo pipefailin each. Followphone_focus_mode/lib/andlinux_configuration/scripts/lib/common.sh. - Python — extract cohesive helpers into sibling modules; keep the public API stable by re-exporting.
- TS/Dart — extract components; re-export from the original path so no importer changes.
- Tests — split by describe/test-group. Coverage must not drop.
Never game the cap: no one-lining, no deleting tests, no moving code into an exempt extension, no suppressions.
Never edit files while a push is running
git push runs pre-commit, which stashes unstaged changes and restores them
afterwards. Editing a file in that window makes the restore fail with
patch does not apply, and the push aborts — while git push | tail still
reports exit 0, because the pipe masks it. This cost two silent failed pushes
and one silently reverted split.
Rules that follow:
- Stage (
git add) before runningpre-commit run --files ...on a work-in-progress tree, or the same stash cycle can revert the edit. - Push only with a clean tree, and do not start other file edits until it returns.
- After any push, confirm with
git status --short --branchthat it does not still sayahead N. Never trust the exit code alone.
Traps that cost time in the last session
- Test fixtures and coverage. When splitting a test file, shared helpers
must go where the coverage config already excludes them (in
poker-stakesthat issrc/test/**, matched by path). Putting them beside the source makes the coverage tool count them as production code and the 100% gate fails. Move the file; do not add acoverage.excludeentry. end-of-file-fixeraborts commits. Files built withsed/catoften lack a trailing newline. The hook fixes it and then fails the commit ("files were modified by this hook"). Justgit addagain and re-commit.- Formatters silently revert edits. A PostToolUse formatter stripped an
added import twice. After editing,
grepthe file to confirm the change survived before running anything. - Contracts. Staging ≥4 code files requires a fresh
docs/superpowers/contracts/*.jsonin that same commit, on top of the per-commitdocs/superpowers/evidence/*.json. Required keys:title,objective,acceptance_criteria,out_of_scope,verifier. Validate withpython3 meta/scripts/validate_contract.py <file>. invariants.test.tsin poker-stakes flakes on its 5s timeout under parallel coverage load. It passes standalone in ~2.6s. Not a regression — do not "fix" it by weakening the test.- Long commands: background anything over ~60s rather than blocking.
phone_focus_mode — highest blast radius, do it LAST
Two silent failure modes:
deploy.shhas a hardcoded flat push list (search foradb_cmd push, around lines 374–383) with nolib/subdirectory. Every new split lib must be added there in the same commit, or the phone sources a missing file at runtime — gate green, tests green, device broken.config.sh(556 lines) is sourced by 11 scripts. Keepconfig.shas the entry point that re-sources its own parts, so all 11 callers keep working untouched.
Static check after each split: grep deploy.sh's push list and every
source/. line in phone_focus_mode/*.sh and confirm each new lib appears.
Suggested order
- The 48 near-misses (251–300) — mechanical, biggest count drop.
python_pkg(50) —pytest-coveragegates each changed package on commit.linux_configuration(84) — largest; check systemd unit paths still resolve.kcd2_dice_solver,focus_owner,billsplit, the rest.phone_focus_mode(13) — with the guard above.- Gate wiring + CI workflow — last.
Done condition
cd ~/testsAndMisc && bash ~/utils/scripts/check_file_length.sh --all # exit 0
pre-commit run --all-files # passes
Plus: a staged 251-line file makes git commit fail.