Retarget the next-session brief at kcd2_dice_solver
Some checks are pending
Pre-commit checks / pre-commit (push) Waiting to run
Shell tests / shell-tests (push) Waiting to run
Shell tests / arch-tests (push) Waiting to run

The shell-installer seam is close to exhausted: of the four candidates probed
last session, one was blocked by a deployed single-file copy, one was
unverifiable (239 of 336 lines never execute under trace), and one configured a
feature that had been dead for months. What is left there is expensive.

kcd2_dice_solver has four files over the cap and a far stronger verifier than
anything the shell harness produces: 288 vitest tests in ~4s, 100% branch
coverage enforced by config thresholds, and tsc+eslint. Verified all three
green at 5dce82f before writing this.

The brief front-loads the two things most likely to go wrong: that the project
has NO CI and is not in pre-commit, so the gate is local-only and a broken
split would push clean; and that splits should re-export from the original file
rather than rewrite imports, so the test files stay untouched and the
verification stays honest.

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 08:20:13 +02:00
parent 5dce82f2c7
commit bb9b162afb

View File

@ -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 '<target-basename>' linux_configuration/ --include='*.sh' \
| grep -v '<the target itself>:'
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/<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.
## 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 '<basename>' linux_configuration/ --include='*.sh' | grep -v '<target>:'
# 2. Baseline at HEAD, BEFORE editing. Fresh, non-existent prefix.
meta/scripts/trace_shell_split.sh <target> --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/<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` 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.<stamp>`, 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.<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, 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.