From 6713ef34cf5eadd07aafbf8ab32a12f6afe08580 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Mon, 17 Aug 2026 11:01:44 +0200 Subject: [PATCH] Retarget the next-session brief at test-writing The previous brief pointed at kcd2_dice_solver; those four files and twelve more are done, so it was stale. The new one reflects what the user asked for after seeing the stopping report: write the missing tests instead of stopping at the coverage wall, change the source-text tests where a split needs it, and delete the Flutter Windows scaffolding rather than working around it. Also spells out the deployed-copy blocker in plain terms, since the previous brief's phrasing did not land: install_leechblock.sh is copied to /usr/local/share/digital_wellbeing/ and pacman_wrapper.sh:831 runs THAT copy on every pacman invocation, so an entry+lib/ split breaks pacman on the live machine. Verified the root-owned copy is still there. Records the billsplit/windows/ deletion as a four-part job, not a git rm -- .gitignore carries two un-ignore rules for it and analysis_options.yaml excludes it -- and flags macos/ and ios/ as the same question to ask rather than assume. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY --- NEXT_SESSION_PROMPT.md | 407 +++++++++++++++++++++++------------------ 1 file changed, 225 insertions(+), 182 deletions(-) diff --git a/NEXT_SESSION_PROMPT.md b/NEXT_SESSION_PROMPT.md index 355a7b3d..83eefcb6 100644 --- a/NEXT_SESSION_PROMPT.md +++ b/NEXT_SESSION_PROMPT.md @@ -1,217 +1,260 @@ -# Next session: split the `kcd2_dice_solver` files over the 250-line cap +# Next session: write the missing tests, then split what they cover > **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. -## The job +## Why this brief is different -Four files in `kcd2_dice_solver/` are over the repo-wide 250-line cap. Split -all four, and prove each one with the project's own test suite: +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. -| File | Lines | Over by | Has a dedicated test file | -| ------------------------ | ----- | ------- | ----------------------------- | -| `src/core/search.ts` | 417 | 167 | `src/core/search.test.ts` | -| `src/core/scoring.ts` | 361 | 111 | `src/core/scoring.test.ts` | -| `src/core/badgeValue.ts` | 333 | 83 | `src/core/badgeValue.test.ts` | -| `src/App.tsx` | 306 | 56 | (component — see note below) | +Then it **stopped**, because the remaining files fall into three buckets and +none of them can be split honestly without doing something else first: -**Do as many as you can, in that order** — biggest first, because `search.ts` -is the one most likely to teach you something the others reuse. Each file is an -independent, committable unit of work: finish one, verify it, commit it, then -start the next. Do not batch all four into one commit. +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. -## Why this target set (read this — it changes how you work) +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. -The last three sessions split **shell installers**, where the only way to prove -a split was a bespoke execution-trace harness, and where three of the last four -candidates turned out to be unverifiable or blocked. This target set is -categorically better: +## The job, in priority order -- `npx vitest run` — **288 tests across 24 files, ~4 seconds** -- `npx vitest run --coverage` — **100% statements / branches / functions / - lines, enforced by config thresholds** (`vite.config.ts`, - `thresholds: { branches: 100, lines: 100, ... }`) -- `npm run lint` — `tsc --noEmit && eslint src`, currently clean +### 1. Delete `billsplit/windows/` (and decide about `macos/`, `ios/`) -All three pass at `5dce82f`. That is your baseline: a split is correct when all -three still pass and coverage is still 100%. You do **not** need the shell trace -harness here, and you should not use it. +`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.** -**Do not fall into the shell-split mindset.** No `trace_shell_split.sh`, no -`--prefix`, no manifest diffing. TypeScript with a real test suite and a 100% -coverage gate is a far stronger verifier than anything that harness produces. +This is not a one-line `git rm`. Three other places know about that directory: -## Baseline — capture before touching anything +- `.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. -```bash -cd ~/testsAndMisc/kcd2_dice_solver -npx vitest run # expect: 24 files, 288 tests, all passed -npx vitest run --coverage # expect: 100% on all four metrics -npm run lint # expect: exit 0, no output -``` +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. -If any of these is already failing, stop and say so — something changed since -`5dce82f` and the baseline is not what this brief claims. +**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. -## The gate is LOCAL ONLY — nothing will catch you +**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. -`kcd2_dice_solver` has **no CI workflow** (`.github/workflows/` has entries for -billsplit, poker-stakes and reverse-survivors, but not this project) and is -**not covered by pre-commit**. So: +**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/`. -- A broken split here passes every commit hook and every push. -- You must run the three commands above yourself, before each commit. -- 100% coverage is enforced by vitest's own thresholds, not by any hook — if - you move code into a new file and the coverage report drops below 100%, the - `--coverage` run fails. That is your safety net; do not bypass it by - splitting without re-running it. +### 2. Write tests for the Python that has none, then split it -## How to split (the actual method) +These are over the cap and were rejected last session purely on coverage. +Measured with +`pytest linux_configuration/tests/test_usage_report_*.py --cov=`: -These are ES modules with named exports and explicit `.ts` import extensions. -The pattern: +| File | Lines | Coverage today | +| -------------------------- | ----- | -------------- | +| `_usage_report_parsing.py` | 425 | 39% | +| `usage_report.py` | 425 | 47% | +| `_usage_report_render.py` | 310 | **21%** | -1. **Read the file and find the seam.** `search.ts` exports `SET_SIZE`, - `EXHAUSTIVE_LIMIT`, `InventoryEntry`, `SearchResult`, `SetCandidate`, - `SearchOptions`, `groupInventory`, `countCandidates`, `findBestSet`. The - types + small helpers vs. the search engine itself is the obvious cut. -2. **Move a coherent group into a sibling file** (`src/core/.ts`). -3. **Re-export from the original** so every existing importer and every test - keeps working unchanged: - ```ts - export { groupInventory, countCandidates } from "./searchGrouping.ts"; - export type { InventoryEntry, SetCandidate } from "./searchTypes.ts"; - ``` - This is the key move — it means you do **not** have to touch the test files - or any consumer, which keeps the change small and the verification honest. -4. Re-run the three commands. +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). -**Prefer re-exporting over rewriting imports.** If you find yourself editing -`*.test.ts` files, stop and ask whether a re-export would have avoided it. A -split that requires changing its own tests is a split that changed behaviour. +**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. -Check who imports the file before you move anything: +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. -```bash -grep -rn 'from "./search.ts"\|from "../core/search.ts"' src/ | grep -v 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. -### `App.tsx` is different +Also over the cap and completely untested — same treatment, lower priority: -It is a React component, not a pure module, and it has no dedicated -`App.test.ts`. Its coverage comes from other tests exercising it. Split it by -extracting **presentational subcomponents** or **custom hooks** into -`src/components/` or `src/hooks/`, not by cutting it at an arbitrary line. -Because its verification is weaker than the three core modules, do it **last**, -and if coverage drops below 100% after extracting, that is a real signal — the -extracted code is no longer being exercised. Fix it by making the extraction -smaller, not by lowering the threshold. +- `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. ## Rules that will bite you -- **No suppressions, ever.** No `// eslint-disable`, no `@ts-ignore`, no - `@ts-expect-error`, no lowering a coverage threshold. If a split seems to - require one, the seam is wrong — restructure instead. -- **Do not touch `vite.config.ts`'s `thresholds` or `exclude` list.** Adding a - new file to `exclude` to dodge coverage is the exact failure this gate exists - to prevent. -- **Fixtures go in `src/test/`.** Coverage `exclude` matches `src/test/**` by - path — a fixture placed beside the source gets counted as production code and - breaks the 100% gate. -- **Match the existing import style**: explicit `.ts` extensions - (`from "./counts.ts"`), `export type` for type-only re-exports. `tsc ---noEmit` will catch you if you get this wrong, so run `npm run lint`. -- Every commit touching code needs an evidence JSON in - `docs/superpowers/evidence/-.json`. Copy - `docs/superpowers/evidence/template.json`; `validate_evidence.py` rejects - empty `verification[]` and the phrases "should work", "probably fine", - "seems right". **Staging ≥4 code files additionally requires a fresh - `docs/superpowers/contracts/*.json`** — validate with - `python3 meta/scripts/validate_contract.py `. One file per commit keeps - you under that threshold. -- `pre-commit run --files ` before committing, as always. -- Work directly on `main`; commit and push. `git stash` and branch creation are - blocked by hooks. `git push` runs `ci-mirror` (clean-venv install + - `pre-commit --all-files` + pytest) and takes minutes — **never edit files - while a push is running**. Background the push and keep reading the next - file, but do not write. -- **Do not wire the file-length pre-commit hook.** It must land last, only once - `bash ~/utils/scripts/check_file_length.sh --all` exits 0. 55 files are still +- **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. +- **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. +- 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. -## Definition of done (per file) - -- The file and everything split out of it are **under 250 lines** - (`bash ~/utils/scripts/check_file_length.sh --all` no longer lists them). -- `npx vitest run` — still 288 tests, all passing. -- `npx vitest run --coverage` — still 100% on all four metrics. -- `npm run lint` — still exit 0. -- No test file was modified (or, if one was, you can explain exactly why a - re-export could not avoid it). -- Zero suppressions added; `vite.config.ts` untouched. -- Committed and pushed with evidence. - -## If you finish all four - -`reverse_survivors/` (`src/core/sim.ts` 361, `src/core/types.ts` 295) is the -next-best set — same TypeScript shape, and it **does** have CI -(`.github/workflows/reverse-survivors-ci.yml`), so check what that workflow -gates on and run it locally before pushing. - -After that, `bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts` -(386) plus its 332-line test file. - -## Do not attempt these (all measured, do not re-derive) - -Shell targets, in the order someone would naively pick them: - -- **`install_leechblock.sh` (485)** — traces perfectly (exit 0, 93 files, all - hashes stable) and is still unsplittable in an entry+`lib/` shape. It is - **deployed as a single file**: `install_pacman_wrapper.sh` copies it to - `/usr/local/share/digital_wellbeing/`, and `pacman_wrapper.sh:831` prefers - that deployed copy on **every pacman invocation**. An entry whose - `SCRIPT_DIR` has no `lib/` dies on its first `source`. The trace cannot see - this — it runs the repo copy. Splitting it is a two-file job (teach the - pacman installer to deploy `lib/`, then re-baseline that file's 16-entry - manifest). -- **`nvidia_troubleshoot.sh` (336)** — its trace dies at step 3 because - `backup_file` writes `/etc/profile.backup.`, a new _sibling_ in - unbound `/etc`. 239 of its 336 lines never execute, so keeping the unproven - ones in the entry leaves it at ~265 — still over the cap. -- **`block_compulsive_opening.sh` (705)** — same deployed-copy blocker; - `install_all` copies the running script into `/usr/local/bin`. -- Blocked on named blockers, not line count: `check_and_enable_services.sh` - (1301), `steam_compatibility.sh` (663), `libre_translate.sh` (488), - `enforce_vbox_hosts.sh` (443). See `refactor_claude_todo_resume.md`. - -`docs/shell-split-verification.md` is itself 272 lines and over the cap. Split -it only if you are already doing docs work; it is not urgent. - -## Context: what landed in the last session - -Four commits, all pushed, all verified: - -- `26965ba` — `install_usage_monitoring.sh` 290 → 53-line entry + 5 libs -- `4eedc17` — `install_pacman_wrapper.sh` 316 → 128-line entry + 4 libs -- `6066201` — documented the deployed-copy blocker and two harness limits -- `5dce82f` — **deleted** `setup_thorium_startup.sh` (443) rather than - splitting it: Thorium was removed from this machine in favour of - chromium/librewolf, so the autostart it installs had been failing at every - boot (`thorium-browser: line 33: /https://www.fitatu.com/app/planner: No such -file`) while systemd reported `status=0/SUCCESS`. Removing - `check_thorium_startup()` from `check_and_enable_services.sh` was the - load-bearing part — its `report_and_fix` re-ran the installer whenever the - service was not enabled. - -The lesson worth carrying: **check whether the thing is still used before -splitting it.** One of the four "targets" was dead code protecting a dead -feature, and deleting it was worth more than any split. - -## Known pre-existing failures (not yours — do not fix) +## Known pre-existing failures — 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`. -- Repo-wide `jscpd` reports ~2.5% duplication from the working tree but 1.47% - at HEAD in a clean worktree — the excess is entirely vendored - `.venv`/`.ci-mirror-venv` site-packages. Don't chase it. +- 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".