diff --git a/docs/python-split-recipes.md b/docs/python-split-recipes.md index 1ebebbfb..02c20385 100644 --- a/docs/python-split-recipes.md +++ b/docs/python-split-recipes.md @@ -167,3 +167,41 @@ 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`). + +## The seam-selection rule (settled over 8 packages) + +Generalised from `python_pkg`; full detail in `docs/python-split-recipes.md`. + +**A block may move if every MOD-relative patch that reaches it comes from one +test module. If two test modules patch a shared collaborator, keep the +collaborator and move its callers instead.** The caller resolves the name +through its *own* module's globals, so importing it into the new module makes +the patch land there. + +Corollaries that each cost a failed run to learn: + +- **`test_X.py` needing no edit is the signal you cut in the right place.** + It held on four of five source splits in `brother_printer`. +- **Never blanket-`sed` a MOD.** Decide per test *by which function is under + test*. `test_challenge_part3.py` needed 13 of its strings repointed and 4 + left alone. +- **A mispointed patch does not always fail — it can HANG or do real work.** + `code_tutor` spun for two minutes running a real interactive flow; + `cups_queue` would have restarted the live CUPS daemon. Always run under + `timeout`, and assert the targets resolve *before* invoking pytest. +- **When splitting a test file, copy the WHOLE preamble** (docstring, imports, + fixture builders) into the new file and let ruff's `F401` pass prune it. + Hand-writing the import block failed on every early attempt; the copy-then- + prune order removed 82 imports correctly in `wsg_grabber` with none missed. + It does **not** cover module-level `def`s and constants sitting *between* + sections — `_make_verifier`, `_QUESTIONS`, `_logger` and + `_SAME_FIELD_X_TOLERANCE_PX` were each left behind that way. Grep for them. +- **Budget two cuts.** Nine of the splits landed 1–20 lines over after the + first seam. Pure-data blocks (prompt strings, skip tables, SVG fragments) + are the cheapest second cut and have no patch targets. +- **Prefer a `conftest.py` fixture over a duplicated builder** — that is where + `_make_verifier` went, and it removed a shadowing copy of `_make_live_mock`. + +Two helper scripts are in the scratchpad, not committed (they are throwaway): +they cut a module at a banner comment or at a named `def`/`class` and copy the +preamble. Rewriting them takes about five minutes. diff --git a/docs/shell-split-recipes.md b/docs/shell-split-recipes.md new file mode 100644 index 00000000..dc7ed237 --- /dev/null +++ b/docs/shell-split-recipes.md @@ -0,0 +1,29 @@ +# Shell split recipes for the 250-line cap + +Split out of `refactor_claude_todo_resume.md` (which is itself capped). +Applies to `linux_configuration/` and `phone_focus_mode/`. + +`linux_configuration` and `phone_focus_mode` have **no test suite and no +coverage gate**, so none of the above transfers. Decision 6 governs: +`bash -n` + `shellcheck` + `systemctl cat` path checks, and **never execute an +enforcement script that mutates the live system or the phone.** + +- After each split, `bash -n` and `shellcheck` the entry script *and* every new + lib. Neither catches a lib that is never sourced. +- So also: for each new `lib/*.sh`, grep the entry script for the `source`/`.` + line naming it, and confirm the path resolves from where the script actually + runs (`${BASH_SOURCE[0]%/*}` or absolute — never a bare relative `./lib/`). +- **Two live units point at repo paths** and break if a file moves or is + renamed. `systemctl cat` them after touching either: + - `/etc/systemd/system/dns-blocklist-refresh.service` → + `scripts/single_use/features/setup_dns_blocker.sh refresh` + - `/etc/systemd/system/media-organizer.service` → + `scripts/single_use/utils/organize_downloads.sh` + + Every other in-repo unit references an installed copy under `/usr/local/`, + so those are only at risk if the *installer* is what you split. +- Prefer files that **nothing** references first — pure lint risk. +- Watch **jscpd** on the first multi-lib commit: repeated `set -euo pipefail` + + source-guard headers across several new libs is exactly what pushes + duplication toward the 2% threshold, and it only fires on the real + `git commit`. diff --git a/refactor_claude_todo_resume.md b/refactor_claude_todo_resume.md index 789ab56e..dce5dbd0 100644 --- a/refactor_claude_todo_resume.md +++ b/refactor_claude_todo_resume.md @@ -6,27 +6,45 @@ ## Where things stand -**183 files** over 250 lines (was 198). Everything is committed and pushed to -`main` at `9e688c5`; the tree is clean. The three unrelated edits an earlier -handoff warned about (`README.md`, the two `analysis_options.yaml`) are gone. +**142 files** over 250 lines (was 183). Everything is committed and pushed to +`main` at `4b2cd0c`; the tree is clean. -Done already (do not redo): +**`python_pkg/` is DONE — 50 violations → 0.** Do not reopen it. Cleared in +this order, one package per tranche, pushing after each: `brother_printer` +(15→0, suite pinned at 402), `code_tutor` (13→0, 216→217), then `android_ui`, +`app_icons`, `token_audit`, `focus_policy`, `random_jpg` (57/44/81/137/14), +then `wsg_grabber`'s six test modules (277). -- `~/utils`: `plans/` + `sessions/` exempted from the cap (`specs/` still capped) -- `17b20e5` new `no-inline-python` pre-commit hook + its one violation -- `004b4b0` `CLAUDE.md` 259 → 141 -- `72a5e28` + `73b85f5` `poker-stakes/` tracked, then 4 violations → 0 -- `77c6d9f`…`d86965f` **every `wsg_grabber` source file** — `catalog` 251→175, - `cli` 282→247, `review` 330→196, `net` 335→109, `store` 382→202, - `downloader` 420→238, `player` 373→235, `ui` 398→242 -- `e03fbd1` `wsg_grabber/tests/test_review.py` 297 → 112 + two new modules -- `2832b78` `brother_printer/display.py` 427 → 342 — **partial, still a - violation.** Extracted `_supply.py` + `_severity.py` and deduped the - toner/drum bars. The last ~92 lines were blocked on `mock.patch` pinning, - which the decision below now unblocks. +Earlier sessions had already done: the `~/utils` exemptions, the +`no-inline-python` hook, `CLAUDE.md`, `poker-stakes/`, and every `wsg_grabber` +**source** file. `git log --oneline --grep='cap'` is the authoritative list. +## What is left + +| Directory | Count | +|---|---| +| `linux_configuration` | 84 | +| `phone_focus_mode` | 13 (do LAST) | +| `kcd2_dice_solver` | 12 | +| `focus_owner` | 11 | +| `billsplit` | 6 | +| `reverse_survivors` / `meta` / `docs` | 4 each | +| `.github` / `bucket_catch` | 2 each | + +Within `linux_configuration` the bulk is `scripts/single_use/` (39 across +`features`/`utils`/`fixes`), then `scripts/periodic_background/` (10) and +`tests/` (4). + +## The seam-selection rule + +Settled over eight `python_pkg` packages and written up in +`docs/python-split-recipes.md` under "The seam-selection rule". In one line: +**a block may move if every patch reaching it comes from one test module; if +two modules patch a shared collaborator, keep the collaborator and move its +callers.** Read the doc before splitting anything with tests. + ## The live worklist Do not work from a list in this file — it goes stale. Run: @@ -36,18 +54,14 @@ bash ~/utils/scripts/check_file_length.sh --all ``` **Every count in this file is a snapshot and will be wrong by the time you read -it. Run the script.** As of `ac02a23` the shape was `linux_configuration` 84, -`python_pkg` 41, `phone_focus_mode` 13, `kcd2_dice_solver` 12, `focus_owner` -11, `billsplit` 6, `reverse_survivors`/`meta`/`docs` 4 each, -`.github`/`bucket_catch` 2 each — quoted only to tell you where the bulk sits, -never to work from. +it. Run the script.** The table above is a snapshot at `4b2cd0c`, quoted only +to tell you where the bulk sits, never to work from. -Within `python_pkg`: `brother_printer` 15, `code_tutor` 13, `wsg_grabber` 6 -(tests only), then singles. - -**Do the source files in a package before its tests.** Proven on `wsg_grabber`: -the test files then split along seams the source already established, instead -of you inventing a new boundary for them. +**Do the source files in a package before its tests.** Proven on `wsg_grabber` +and again on all seven `python_pkg` packages: the test files then split along +seams the source already established, instead of you inventing a new boundary +for them. On `brother_printer` this meant four of the five source splits needed +**no test edit at all**. ## Decisions already made (do not re-ask) @@ -68,6 +82,14 @@ of you inventing a new boundary for them. stalled on `brother_printer` because of it. 9. Per-commit gate is `pre-commit run --files `; the real `git commit` is the full gate (it adds jscpd). Push per directory tranche, not per commit. +10. **When a seam splits code covered by a per-file-ignore, fix the lint — + do not copy the ignore onto the new file** (user's call, 2026-08-16). + Done for `code_tutor/cli.py`: `S607` fixed with `shutil.which`, `PERF203` + by extracting the probe so the poll loop carries no `try`/`except`. Both + entries deleted from `meta/pyproject.toml`; the repo now has two fewer + suppressions. The `which()` fix added a branch, so a covering test was + written (216 → 217) rather than excluding it. Expect the same question on + any remaining per-file-ignore you split through — ask, don't assume. ## The constraint that will bite you @@ -93,13 +115,19 @@ Plus `.github/workflows/file-length.yml` modelled on `.utils`), scoped to changed files. Then prove it fails: stage a deliberately 251-line file, confirm `git commit` aborts, delete it. -## Python split recipes +## Language recipes -`docs/python-split-recipes.md` holds the settled Python rules: the `__all__` -re-export form, the `TC001` trap that lints clean and breaks at runtime, the -mixin shape for splitting a class, what the tests pin in place, and the -coverage behaviour of new sibling modules. **Read it before the first Python -split** — each rule there cost a failed gate run or a revert to establish. +- `docs/python-split-recipes.md` — the `__all__` re-export form, the `TC001` + trap that lints clean and breaks at runtime, the mixin shape for splitting a + class, what the tests pin in place, and the coverage behaviour of new sibling + modules. `python_pkg` is done, so this now matters only if a stray `.py` + turns up outside it. +- `docs/shell-split-recipes.md` — **read this before the first + `linux_configuration` split.** What replaces the pass count when there is no + test suite, the two live systemd units that point at repo paths, and the + jscpd trap on the first multi-lib commit. + +Each rule in both cost a failed gate run or a revert to establish. ## Split recipes