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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
Krzysztof Rudnicki 2026-08-17 11:01:44 +02:00
parent 113cada08c
commit 6713ef34cf

View File

@ -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=<the bin dir>`:
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/<name>.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.<stamp>` 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/<slug>-<date>.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 <file>`. One file per commit keeps
you under that threshold.
- `pre-commit run --files <changed>` 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/<slug>-<date>.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 <file>`.
- `pre-commit run --files <changed>` 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=<name>.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.<stamp>`, 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".