diff --git a/NEXT_SESSION_PROMPT.md b/NEXT_SESSION_PROMPT.md index da639ed8..355a7b3d 100644 --- a/NEXT_SESSION_PROMPT.md +++ b/NEXT_SESSION_PROMPT.md @@ -1,202 +1,217 @@ -# Next session: `install_leechblock.sh` is BLOCKED — read this first +# Next session: split the `kcd2_dice_solver` files over the 250-line cap > **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. -## Do not split `install_leechblock.sh` +## The job -The previous brief pointed here. It was wrong, and the reason is worth reading -before picking any target. +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: -`install_leechblock.sh` traces beautifully — exit 0, 93 written files, all 93 -hashes byte-identical across runs with `--stub pkill`. By every signal the -harness gives you, it is the best-conditioned target left. **It is still -unsplittable in an entry+`lib/` shape**, because it is DEPLOYED as a single -file: +| 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) | -- `install_pacman_wrapper.sh` `cp`s it to - `/usr/local/share/digital_wellbeing/install_leechblock.sh` (confirmed - present on this machine, byte-identical to the repo copy, no `lib/`). -- `pacman_wrapper.sh:831` _prefers that deployed copy_, and - `auto_install_leechblock "$@"` runs on **every pacman invocation**. -- `check_and_enable_services.sh:46` also references it, and - `periodic-system-maintenance.timer` is active (hourly). +**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. -An entry whose `SCRIPT_DIR` has no `lib/` dies on its first `source` under -`set -e`. **The trace cannot see this** — it runs the repo copy, where `lib/` -exists. This is the same blocker as `block_compulsive_opening.sh`. +## Why this target set (read this — it changes how you work) -Splitting it is possible but is a _two-file_ job: teach the pacman installer to -deploy `lib/` too, which changes that file's 16-entry trace manifest and forces -a re-baseline of a file that was verified last session. Decide that deliberately -if you take it on; don't stumble into it. +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: -**Generalise the lesson:** before designing seams, run +- `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 + +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. + +**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. + +## Baseline — capture before touching anything ```bash -grep -rn '' linux_configuration/ --include='*.sh' \ - | grep -v ':' +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 ``` -A target invoked as `$CONFIG_DIR/scripts/...` (from the repo) is safe; one that -is `cp`'d somewhere and run from there is not. +If any of these is already failing, stop and say so — something changed since +`5dce82f` and the baseline is not what this brief claims. -## What landed in the last session (do not redo) +## The gate is LOCAL ONLY — nothing will catch you -Two splits, both committed and pushed, both proven by an empty before/after -trace diff: +`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: -- `install_usage_monitoring.sh` 290 → 53-line entry + 5 libs (`26965ba`) -- `install_pacman_wrapper.sh` 316 → 128-line entry + 4 libs (`4eedc17`) +- 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. -Read the `4eedc17` diff first — it is the closest worked example, and three of -its lessons apply directly here: +## How to split (the actual method) -- **Keep path/variable definitions in the ENTRY script, not a `lib/paths.sh`.** - A definitions-only lib assigns without referencing, which is SC2034 on every - variable, and suppressions are forbidden. Libs that only _reference_ globals - are fine. `export` silences it but leaks the variables into every child - process — a behaviour change the trace cannot see. Don't. -- **The pre-commit hook runs `shfmt -w` on staged shell files** and re-stages - them, rewriting `cat > "x" << 'EOF'` into `cat >"x" <<'EOF'`. Run `shfmt -w` - yourself first, then verify, so the tested state is the committed state. -- **Enumerate every test that greps the target's source text before designing - the seams.** The pacman split had eight such assertions across two test libs; - one (`chattr +i`) was missed on the first pass and failed loudly. Run: - ```bash - grep -rn 'install_leechblock' linux_configuration/tests/ meta/ .github/ - ``` +These are ES modules with named exports and explicit `.ts` import extensions. +The pattern: -Repo-wide `jscpd` reports 2.51% 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; measure in a worktree if you need a real number. +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. -## The job: pick a target, then prove it +**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. -There is no pre-chosen target this session (see "Ruled out" below for why). -The method is what carries over, and it is not negotiable: **prove the split by -executing the script before and after, and diffing the traces.** +Check who imports the file before you move anything: ```bash -# 1. Does anything deploy this file elsewhere? If yes, STOP (see above). -grep -rn '' linux_configuration/ --include='*.sh' | grep -v ':' - -# 2. Baseline at HEAD, BEFORE editing. Fresh, non-existent prefix. -meta/scripts/trace_shell_split.sh --prefix /tmp/pfx-before \ - --stub pkill --out /tmp/before.txt - -# 3. Run it twice at HEAD and diff, to learn what varies legitimately -# (timestamps, curl progress, the echoed prefix path) before you trust it. - -# 4. Split. Then re-trace into a DIFFERENT fresh prefix and diff. +grep -rn 'from "./search.ts"\|from "../core/search.ts"' src/ | grep -v test ``` -**Coverage decides the seams.** Whatever the trace does not execute stays in -the entry script, because a split that relocates unexecuted code cannot be -verified by the diff. Read the trace's stdout against the target's guarded -blocks and enumerate what was skipped before choosing where to cut. +### `App.tsx` is different + +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. ## Rules that will bite you -- **Keep variable ASSIGNMENTS in the entry; move FUNCTION DEFINITIONS freely.** - A lib that assigns without referencing is SC2034 on every variable, and - suppressions are forbidden. A lib that only references globals is clean. - `export` silences SC2034 while leaking the variables into every child - process — a behaviour change the trace cannot see. Don't. -- **Verify with `pre-commit run shellcheck --files …`, not a bare - `shellcheck -x`.** The latter follows `source=` directives and hid 36 SC2034 - findings in a standalone lib; the former is the real gate. -- **The pre-commit hook runs `shfmt -w` on staged shell files** and re-stages - them, rewriting `cat > "x" << 'EOF'` into `cat >"x" <<'EOF'`. Run `shfmt -w` - yourself first, then verify, so the tested state is the committed state. -- **Enumerate every test that greps the target's source text before cutting.** - The pacman split had eight such assertions across two test libs; one - (`chattr +i`) was missed on the first pass and failed loudly. -- **Wrapping top-level code in a function changes what `set -e` sees.** A bare - `[[ -n $x ]] && arr+=("$x")` as a function's last statement becomes its - return value; append `|| true` if so. -- New `.sh` libs need the executable bit, a `#!/bin/bash` shebang, and **no - `set -euo pipefail`** (a sourced lib inherits the caller's strict mode). +- **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/`; **≥4 staged code files** additionally needs a - fresh `docs/superpowers/contracts/*.json`. Validate both with - `python3 meta/scripts/validate_contract.py` / - `meta/scripts/validate_evidence.py`. -- Markdown needs `npx prettier --write` — prettier is pre-push only. + `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` and takes minutes — never edit - files while a push is running. + 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 over. -Repo-wide `jscpd` reports 2.51% 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; measure in a worktree if you need a real number. +## Definition of done (per file) -## Read these first +- 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. -1. `docs/shell-split-verification.md` — how a green split is still broken. Now - also covers the deployed-copy blocker, the `/usr/bin` bind limit, the - unstubbed `pkill`, processes the prefix cannot contain, and how to compare - when the target embeds a timestamp. -2. `docs/shell-split-recipes.md` — how to actually make a split. -3. The `4eedc17` and `26965ba` diffs — two worked examples of the entry+lib - shape. +## If you finish all four -## Definition of done +`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. -- The target and every file split out of it are **under 250 lines**. -- The before/after traces are identical once known-variable patterns are - normalized, with every written-file hash unchanged. -- Nothing moved into a lib sits in a region the trace never executed. -- Any test that greps the target's source text still passes, with no assertion - weakened. -- `pre-commit run shellcheck` clean, `shfmt` clean, zero suppressions. -- The live system is untouched by the verification runs — check mtimes of - whatever the script writes, before and after. -- Committed and pushed, with evidence (and a contract if ≥4 code files). +After that, `bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts` +(386) plus its 332-line test file. -## Ruled out — do not attempt these +## Do not attempt these (all measured, do not re-derive) -- **`install_leechblock.sh` (485)** — see the top of this file. Traces - perfectly, but is deployed as a single file and run from - `/usr/local/share/digital_wellbeing/` on every pacman invocation. -- **`setup_thorium_startup.sh` (443)** — traces to exit 0 with all 8 steps - covered and has no deployed copy, so it _is_ mechanically splittable. Do not - split it anyway: **the thing it configures is dead.** Thorium was removed from - this machine (chromium/librewolf are used now), `/usr/local/bin/thorium-browser` - is a leftover symlink to `browser-preexec-wrapper`, and the enabled - `thorium-fitatu-startup.service` fails silently at every boot: - `thorium-browser: line 33: /https://www.fitatu.com/app/planner: No such file -or directory` — while systemd still reports `status=0/SUCCESS`. Splitting a - 443-line script to keep a broken autostart under a line cap is the wrong - work. Either retarget it at chromium/librewolf or delete it and the unit; - ask the user which. That decision is worth more than the split. -- **`nvidia_troubleshoot.sh` (336).** Its trace dies at step 3 because - `backup_file` writes `/etc/profile.backup.`, a _new sibling_ in - unbound `/etc` (a file bind cannot cover siblings, and bare `/etc` is - refused). 239 of its 336 lines never execute. Keeping the unproven ones in - the entry — the rule the pacman split established — leaves the entry at - ~265, still over the cap. -- **`block_compulsive_opening.sh` (705)** — `install_all` copies the running - script into `/usr/local/bin`; same deployed-copy blocker as leechblock. -- Blocked on NAMED blockers, not line count: - `check_and_enable_services.sh` (1337), `steam_compatibility.sh` (663), - `libre_translate.sh` (488), `enforce_vbox_hosts.sh` (443). See - `refactor_claude_todo_resume.md`. +Shell targets, in the order someone would naively pick them: -**There is no easy next target.** Every remaining file under ~500 lines is now -either blocked by a deployed copy, unverifiable by trace, or configures -something dead. Pick by re-probing, not by line count, and expect the next win -to cost more than the last two. +- **`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, over the cap. Split it -when someone is doing docs work; it is not urgent and the hook is not wired. +`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. -## Known pre-existing failure (not yours) +## Context: what landed in the last session -`bash linux_configuration/tests/test_security_hardening.sh` exits 1 at HEAD -with exactly one failure, `❌ FAIL: Compulsive block wrappers installed`. It -belongs to `block_compulsive_opening.sh`. Capture it before and after your -change and confirm it is unchanged; do not try to fix it. +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) + +- `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.