diff --git a/NEXT_SESSION_PROMPT.md b/NEXT_SESSION_PROMPT.md index 83eefcb6..90db89b5 100644 --- a/NEXT_SESSION_PROMPT.md +++ b/NEXT_SESSION_PROMPT.md @@ -1,260 +1,155 @@ -# Next session: write the missing tests, then split what they cover +# Next session: keep splitting, tests first > **Paste this whole file into a fresh Claude session opened in `~/testsAndMisc`.** > It is self-contained. Do not go looking for the previous session's context. -## Why this brief is different +## Where things stand -The previous session split 16 files across kcd2_dice_solver, reverse_survivors, -bucket_catch, focus_owner, phone_focus_mode, linux_configuration and docs. It -took the over-cap count from 51 to 40. Every one of those splits was verified by -a test suite that already existed. +Over-cap: **41 → 37** (29 shell, 4 python, 2 kotlin, 1 dart, 1 markdown — the +markdown is this file). 12 commits, `6713ef3..ff1a1b6`, all pushed. -Then it **stopped**, because the remaining files fall into three buckets and -none of them can be split honestly without doing something else first: +The C++ category is empty and the `system-maintenance/bin/` python is done: +every module there is under the cap at 100% line+branch coverage. -1. no test coverage of the code that would move, -2. tests that assert on the file's **source text**, so a split breaks them, -3. generated platform scaffolding that should not be edited at all. +## The method, which worked six times -The user has now explicitly authorised all three: **write the tests**, **change -the source-text tests**, and **delete the generated scaffolding**. That is this -session's job, in that order of value. +1. **Measure coverage of the block you intend to move, first.** The failure + this catches is real: `_usage_report_parsing.py` was 39% covered and the + uncovered 61% was almost exactly the half a split would extract. Splitting + that first would have proved nothing. +2. **Tests land in their own commit, before the split.** Not negotiable — a + split verified by tests written alongside it is verified against the + post-split shape. +3. **Move code verbatim, then prove it.** Write a throwaway script that parses + HEAD and the new modules with `ast`, normalises each top-level def through + `ast.dump(ast.parse(ast.unparse(node)))`, and asserts identity. It caught a + literal `×` typed where the original had a `×` escape. Formatting-blind, + logic-sensitive; ~40 lines, worth rewriting each time. +4. **Rewrite import sites to name the owning module.** No re-export barrels. + Check the _whole_ graph: one split shipped with `usage_report.py` still + importing `_fmt_h` through the render module, and the commit message claimed + otherwise. +5. **Run the actual program.** Every unit test here stubs subprocesses, so only + a real run proves the import graph resolves. Run every entry point the split + touched, not just one. +6. Re-run every gate, commit one split at a time, with evidence. -## The job, in priority order +## Traps that cost time this session, in order of cost -### 1. Delete `billsplit/windows/` (and decide about `macos/`, `ios/`) +- **`monkeypatch.setattr` must target the module whose namespace resolves the + name** — the caller's, not the one the function moved to. Getting this wrong + on `_run_since` made `main()` call the real function, which read the + machine's actual atop logs and **hung the suite** rather than failing. If a + pytest run stalls after a split, this is why. +- **Splitting a test file can put it over the cap.** Two commits added + over-cap files while fixing something else (`DropZone.test.tsx` 250 → 256 on + a lint fix; two new test files at 373 and 272). Check + `check_file_length.sh --all` _before_ committing, not after. +- **Don't move too much at once.** The `usage_report.py` split needed two + functions relocated after their first placement made an import cycle, plus + four modules' worth of patch targets re-pointed. Smaller slices are cheaper. +- **`TYPE_CHECKING` imports bite when code moves.** `Path`, `_fmt_h` and + `dataclass` each ended up importable only for type checking while used at + runtime. All three failed at first call; 100% coverage surfaced them at once. +- **A backgrounded `git push | tail` hides failure.** One push reported exit 0 + while printing `error: failed to push some refs`. Verify with + `git status -sb` showing no `[ahead N]`. -`billsplit/windows/runner/win32_window.cpp` is 288 lines and over the cap. It is -Flutter-generated Windows runner boilerplate — the previous session refused to -split it because `flutter create` regenerates it and hand-editing it is -pointless. The user's answer: **the Windows target is not wanted, so remove it -rather than work around it.** +## What to do next -This is not a one-line `git rm`. Three other places know about that directory: +### 1. `meta/scripts/optimize_vscode.py` (498, untested) -- `.gitignore` lines ~404-405 carry `!billsplit/windows/runner/resources/app_icon.ico` - and `!billsplit/windows/runner/runner.exe.manifest` un-ignore rules (the repo - blocks binaries by default; these are the exceptions that let the Windows icon - be committed). -- `billsplit/analysis_options.yaml:9` excludes `windows/**` from analysis. -- `.binary-allowlist` may list the icon — check it. +The largest remaining python file and completely uncovered. Same recipe: +coverage commit, then split. Take a **smaller first slice** than the last one. -So: remove the directory, remove those three hooks into it, and add -`billsplit/windows/` to `.gitignore` so `flutter create`/`flutter build` cannot -silently resurrect it as an untracked-then-committed directory. +### 2. The three `transcribe_*` files (356 / 302 / 289, untested) -**Ask the user before widening scope:** `billsplit/macos/` (28 tracked files) -and `billsplit/ios/` (40) are the same kind of unused scaffolding, and -`billsplit/linux/` (10) may or may not be in use. The user only named Windows. -Do Windows, then ask whether macOS/iOS should follow — do not assume. +`linux_configuration/scripts/single_use/misc/testsAndMisc-bash/tools/`. +`_transcribe_diarize.py`, `transcribe_fw.py`, `transcribe_helpers.py`. Check +whether they import heavy ML deps at module scope — if so, testing them needs +the import stubbed, and `python-mpv`-style "loads a native lib at import" is a +known CI-breaker in this repo. -**Verify:** `cd billsplit && flutter analyze` (expect no new issues) and -`flutter test` (expect the existing suite to pass unchanged). The billsplit CI -workflow is `.github/workflows/billsplit-ci.yml` — read what it builds before -you delete anything, and confirm it does not build Windows. +### 3. Shell, where the remaining 29 are -**Done:** `win32_window.cpp` gone from `check_file_length.sh --all`, `flutter -analyze` and `flutter test` clean, and a fresh `flutter create .` in that -directory would not re-add a tracked `windows/`. +`docs/shell-split-verification.md` has the harness. Two named blockers: -### 2. Write tests for the Python that has none, then split it +**Scripts copied to a system location and run from there.** +`install_leechblock.sh` (485) is copied to `/usr/local/share/digital_wellbeing/` +and `pacman_wrapper.sh:831` prefers that deployed copy on **every pacman +invocation**. Split the repo copy into an entry that sources `lib/`, and the +deployed copy — which has no `lib/` beside it — dies on its first `source`. +Every `pacman -S` on this machine then fails. The trace harness cannot see +this: it runs the repo copy, where `lib/` exists. Same for +`block_compulsive_opening.sh` (705), whose `install_all` copies the running +script into `/usr/local/bin`. **Teach the installer to deploy `lib/` first, in +its own commit, then re-baseline its manifest, then split.** -These are over the cap and were rejected last session purely on coverage. -Measured with -`pytest linux_configuration/tests/test_usage_report_*.py --cov=`: +**Source-text tests.** `test_pacman_wrapper_security.sh` asserts things like +`grep -q 'source .*pacman_lock_lib.sh' pacman_wrapper.sh`. It **is** in CI +(`.github/workflows/shell-tests.yml`, the `arch-tests` list). Rewriting those +assertions is authorised, but the replacement must prove the same property of +the split whole — grep across entry **and** libs, or better, source the library +and assert the behaviour. State in the evidence how the new assertion is at +least as strong. -| File | Lines | Coverage today | -| -------------------------- | ----- | -------------- | -| `_usage_report_parsing.py` | 425 | 39% | -| `usage_report.py` | 425 | 47% | -| `_usage_report_render.py` | 310 | **21%** | - -They live in -`linux_configuration/scripts/periodic_background/system-maintenance/bin/`. -Their existing tests are `linux_configuration/tests/test_usage_report_since.py`, -`test_usage_report_merge.py`, `test_usage_report_modes.py` (30 tests total, all -passing — the previous session split that one 480-line test file three ways). - -**The order matters and is not negotiable:** raise coverage on the block you -intend to move **first**, in its own commit, then split in a second commit. A -split verified by tests written in the same commit proves nothing, because the -tests were written against the post-split shape. - -Start with `_usage_report_render.py` at 21% — it is the smallest (310, over by 60) and the most under-tested, so it has the best ratio of test-writing effort -to lines freed. Rendering functions take data and return strings; they are the -easiest thing in this list to test. - -Note: **coverage on these files is not enforced by any gate.** `meta/pyproject.toml` -sets `source = ["python_pkg"]`, so these `linux_configuration` scripts are -outside the 100% bar. You are raising coverage because it is the only way to -make a split verifiable, not because a hook demands it. - -Also over the cap and completely untested — same treatment, lower priority: - -- `meta/scripts/optimize_vscode.py` (498) -- `linux_configuration/.../tools/_transcribe_diarize.py` (356) -- `linux_configuration/.../tools/transcribe_fw.py` (302) -- `linux_configuration/.../tools/transcribe_helpers.py` (289) - -### 3. The shell files whose tests grep their source text - -The user has authorised changing these tests. The clearest case: - -`linux_configuration/scripts/periodic_background/digital_wellbeing/pacman/pacman_wrapper.sh` -(929 lines, over by 679) is covered by -`linux_configuration/tests/test_pacman_wrapper_security.sh`, which asserts -things like `grep -q 'source .*pacman_lock_lib.sh' "$WRAPPER_DIR/pacman_wrapper.sh"`. -Move a line into a lib and that assertion fails — by design, since its whole -purpose is "this security-relevant line is present in this file". - -**When you rewrite such an assertion, it must not get weaker.** The current test -proves a specific line exists in a specific file. The replacement must prove the -same property of the split whole — e.g. grep across the entry **and** its libs, -or better, source the library and assert the behaviour. Do not simply delete the -assertion or relax it to a pattern that would also match a broken split. State -in the evidence exactly how the new assertion is at least as strong. - -`test_pacman_wrapper_security.sh` **is** in CI -(`.github/workflows/shell-tests.yml`, the second list, alongside -`test_hosts_guard_pacman_integration.sh`), so breaking it breaks the build. -Run it before and after. - -### 4. What is genuinely blocked, and what "blocked" means - -The user asked what "the brief measured and blocked seven by name" meant. Here -it is in plain terms. Four shell scripts cannot be split into an -entry + `lib/` shape for one shared reason, and it is not a testing problem: - -**These scripts are copied to a system location and run from there.** -`install_leechblock.sh` is copied to `/usr/local/share/digital_wellbeing/` -(verify: `ls -la /usr/local/share/digital_wellbeing/` — a root-owned copy is -there right now), and `pacman_wrapper.sh:831` prefers that deployed copy on -**every pacman invocation**. Split the repo copy into an entry that does -`source "$SCRIPT_DIR/lib/foo.sh"`, and the deployed copy — which has no `lib/` -next to it — dies on its first `source` line. Every `pacman -S` on this machine -then fails. - -The trace harness cannot see this, because it runs the repo copy where `lib/` -does exist. That is why "it traced green" is not evidence here. - -Affected: `install_leechblock.sh` (485), `block_compulsive_opening.sh` (705, -`install_all` copies the running script into `/usr/local/bin`). - -**Splitting these is a two-file job**: first teach the installer to deploy -`lib/` alongside the entry, then re-baseline the installer's manifest, then -split. That is a legitimate task — it is just bigger than a split, and it must -be done in that order or you break pacman on the user's live machine. - -Separately, `nvidia_troubleshoot.sh` (336) has a different blocker: its trace -dies at step 3 because `backup_file` writes `/etc/profile.backup.` into -unbound `/etc`, so 239 of its 336 lines never execute under the harness. -Keeping only proven lines in the entry still leaves ~265 — over the cap. It -needs either a better harness or hand-verification. - -`check_and_enable_services.sh` (1301), `steam_compatibility.sh` (663), -`libre_translate.sh` (488), `enforce_vbox_hosts.sh` (443) have their own named -blockers in `refactor_claude_todo_resume.md`. - -**None of this is "do not touch".** It is "here is the specific thing that will -break, and the order you must do it in". If you take one of these on, the -deploy-`lib/`-first step is the whole task. - -## The method (unchanged, it worked 16 times) - -1. **Measure coverage of the code you intend to move, before choosing a seam.** - File-level percentage is a starting point, not the answer — the question is - whether the tests reach _the block you are moving_. A file at 56% whose - covered half is exactly what you extract is a fine target; a file at 65% - whose uncovered lines are precisely the extraction is not. -2. **Capture every gate at HEAD first.** Tests, coverage totals, lint, build. - Note the _totals_ (e.g. `863/375/181/810`), not just "100%" — unchanged - totals prove no statement left test reach. -3. Move code **verbatim**. If totals shift, you should be able to name the - added function. -4. **Rewrite import sites to name the owning module.** No re-export barrels — - the user asked for this explicitly last session. -5. Re-run every gate. Commit one file's split at a time, with evidence. +`nvidia_troubleshoot.sh` (336) has a different blocker: its trace dies at step 3 +because `backup_file` writes into an unbound `/etc`, so 239 of 336 lines never +execute under the harness. ## Rules that will bite you - **No suppressions, ever.** No `# noqa`, `# type: ignore`, `eslint-disable`, - `@ts-ignore`, `# shellcheck disable`, no lowering a coverage threshold. Two - splits last session tempted one; both were fixed by changing the code - instead. If a split seems to need one, the seam is wrong. + `# shellcheck disable`, no lowered coverage threshold. Several were tempted + this session; all were resolved by changing the code. If a split seems to + need one, the seam is wrong. - **Every commit touching code needs evidence** in `docs/superpowers/evidence/-.json` (copy `template.json`). - `validate_evidence.py` rejects empty `verification[]` and "should work" / - "probably fine" / "seems right". **Staging ≥4 code files additionally needs a - fresh `docs/superpowers/contracts/*.json`** — validate with - `python3 meta/scripts/validate_contract.py `. -- `pre-commit run --files ` before committing. Note **`prettier` and - `ci-mirror` run on pre-push, not pre-commit** — a commit can pass and the - push still fail on markdown formatting. Run `npx prettier --write` on any - `.md` you touch. + Staging **≥4 code files also needs** a fresh `docs/superpowers/contracts/*.json`. + Validate both with `meta/scripts/validate_{evidence,contract}.py`. +- `pre-commit run --files ` before committing. **`prettier` and + `ci-mirror` run on pre-push, not pre-commit** — a commit can pass and the push + still fail. Run `npx prettier --write` on any `.md` you touch, including this one. - Work directly on `main`. `git stash` and branch creation are blocked by hooks. - `git push` runs `ci-mirror` and takes minutes — background it and **do not - edit files while it runs**. -- **Do not wire the file-length pre-commit hook.** It lands last, only once - `bash ~/utils/scripts/check_file_length.sh --all` exits 0. 40 files are still - over. -- New sourced shell libs: match the sibling convention (shebang **and** the - executable bit — a `chmod 755` was needed last session), source them via - `"$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh"`, and write - `# shellcheck source=.sh` relative to the script's own directory - (`.shellcheckrc` sets `source-path=SCRIPTDIR`). -- Python fixtures go where coverage excludes them; check `[tool.coverage]` in - `meta/pyproject.toml` before placing one. +- **Do not wire the file-length pre-commit hook.** It lands last, once + `bash ~/utils/scripts/check_file_length.sh --all` exits 0. 37 files are still over. +- Cap pytest memory: `systemd-run --user --scope -p MemoryMax=2G -p MemorySwapMax=0`. +- New sourced shell libs: shebang **and** the executable bit, sourced via + `"$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib.sh"`, with + `# shellcheck source=.sh` relative to the script's own directory. -## Known pre-existing failures — not yours, do not fix silently +## Known pre-existing state — not yours, do not fix silently -- **`bucket_catch/packages/frontend` has 45 eslint errors.** `npm run lint` - exits 1. These were _unmasked_, not caused, by a fix last session: `lint` is - `tsc --noEmit && eslint src`, and a broken `tsc` had been short-circuiting the - `&&` so eslint had never run on that package at all. 18 - `no-non-null-assertion`, 10 `unbound-method`, 9 - `non-nullable-type-assertion-style`, 5 `no-unnecessary-type-assertion`, plus - `require-await`, `no-this-alias`, `no-deprecated`. Fixing them needs per-site - judgment and the user's no-suppressions rule means **asking first**. `npm run -build` and `npm run coverage` there are green (145 tests, 100%). -- `bash linux_configuration/tests/test_security_hardening.sh` exits 1 with - exactly one failure, `❌ FAIL: Compulsive block wrappers installed`. It - belongs to `block_compulsive_opening.sh`. +- **`bucket_catch/packages/frontend` has 4 eslint errors** (was 45). Each is + documented with measured reasoning in + `docs/superpowers/evidence/bucket-catch-eslint-2026-08-17.json`. The one worth + knowing: `usePuzzleGameLoop.ts:129`'s `Map.get(...)!` cannot be fixed with + `?? Infinity` — that was tried and measured at 219 branches / 99.54%, because + the default side is unreachable. Trading a lint error for a coverage failure + is not a fix. `npm run coverage` is green: 145 tests, 100%. +- `bash linux_configuration/tests/test_security_hardening.sh` exits 1 with one + failure, `❌ FAIL: Compulsive block wrappers installed`. It belongs to + `block_compulsive_opening.sh`. +- `_compute_window` in `_usage_report_run.py` is dead: documented as "kept for + backwards import compatibility", zero callers anywhere (verified by grep over + `.py` and `.sh`). It is tested, so deleting it is safe and cheap. - Repo-wide `jscpd` reports ~2.5% from the working tree but 1.47% at HEAD in a clean worktree — the excess is vendored `.venv` site-packages. Don't chase it. ## Testing notes specific to this repo -- **Cap pytest memory**: run it under - `systemd-run --user --scope -p MemoryMax=2G -p MemorySwapMax=0`. A reader - thread on an unconfigured `MagicMock` once ate 21 GB and OOM-killed the box. -- `focus_owner` gradle needs JDK 21: prefix every `gradlew` call with - `JAVA_HOME=/usr/lib/jvm/java-21-openjdk`. And use `--rerun-tasks` — a plain - `gradlew test` reports `UP-TO-DATE` and proves nothing. -- `phone_focus_mode`'s shell tests are **not in CI**. `shell-tests.yml` uses an - explicit file list covering `linux_configuration/tests/` only. If you add a - test file there expecting CI to run it, add it to that list too. -- For a **test-file** split, the discriminating check is the test **count** - (and file count), not a green run: a new file whose name falls outside the - runner's glob is silently never collected and everything still passes. - -## Where the previous session left off - -16 commits, `bb9b162..113cada`, all pushed and verified. Over-cap: 51 → 40 -(29 shell, 7 python, 2 kotlin, 1 dart, 1 cpp). - -Split and verified: all four `kcd2_dice_solver` files; `reverse_survivors` -`sim.ts` + `types.ts`; `bucket_catch` `usePuzzleGameLoop.ts` + its test file -(plus the typecheck repair); `focus_owner` `EnforcementDecision.kt` + -`FocusPolicy.kt`; `phone_focus_mode` `adb_common.sh` + its test file; -`linux_configuration` `atop_agg.c` + `test_usage_report_since.py`; -`docs/shell-split-verification.md`. - -Two lessons worth carrying: - -- **Measure, don't assume.** `atop_agg.c` was written off as untestable C; its - Makefile had `test` and `coverage` targets all along and it turned out to be - the best-covered file left (92.9% lines / 100% functions). -- **A declaration-name scan undercounts.** `FocusPolicy.parse` looked untested - by name but every test fixture goes through it. Ask "do the tests reach this - block", not "is this identifier mentioned in a test file". +- `linux_configuration/tests` **is** in CI, but never by name: `pyproject.toml` + sets `testpaths`, so the bare `pytest` in `python-tests.yml` collects it, and + `meta/scripts/pytest_changed_packages.py` hard-codes the directory into the + pre-commit hook. Behaviour is gated; coverage is not (`--cov=python_pkg` only). + A broken split of these files **does** break the build. +- `name-tests-test` requires every `.py` under `tests/` to be named `test_*.py`, + including in subdirectories. A shared helper module there is not possible + without a hook exclusion; put helpers in `conftest.py` or in the one file that + needs them. +- For a **test-file** split the discriminating check is the test **count**, not + a green run: a file outside the runner's glob is silently never collected. +- `phone_focus_mode`'s shell tests are **not** in CI. `shell-tests.yml` uses an + explicit file list covering `linux_configuration/tests/` only. +- `focus_owner` gradle needs `JAVA_HOME=/usr/lib/jvm/java-21-openjdk` and + `--rerun-tasks`; a plain `gradlew test` reports `UP-TO-DATE` and proves nothing.