56 lines
8.0 KiB
Markdown
56 lines
8.0 KiB
Markdown
---
|
|
name: monolith-split-under-dist-pin
|
|
description: "\"Use when cutting a monolithic source file into modules behind a committed-bundle pin (dist must stay byte-identical to src); one verified commit per cut, red-proof against the previous bundle. Not for feature work or doc-only edits.\""
|
|
---
|
|
|
|
Procedure that landed a 1354-line `index.ts` as 9 modules + 301 lines of wiring across 9 commits with zero red commits; the same loop then split a 487-line `utterance.ts` in two cuts. Assumes: black-box test suite (tests drive the built bundle, not internals) and a pin test asserting the committed `dist/` equals a fresh build of `src/`.
|
|
|
|
## Per-cut loop (one commit per cut, always in this order)
|
|
|
|
1. `bun run build` (or project equivalent) **before** staging — commit `dist/` in the same commit as the moved `src/`, otherwise the pin test fails ("committed dist is byte-identical to a fresh build") and you must amend. Amending is fine only while the branch is unpushed. Expect this to be the ONLY red test before the commit lands: it is `git diff --name-only -- dist` being non-empty, not a broken build.
|
|
2. `tsc --noEmit`, then run the full suite async (`bats tests/ > /tmp/x.log 2>&1; grep -E '^1\.\.'`).
|
|
3. **Launch the async suite with no deadline.** The default background deadline (~300 s) kills a minutes-long suite mid-run; the log then ends on a plausible `ok` line and reads like a partial pass. Likewise never edit `src/` while it runs.
|
|
4. **Never write to `src/` while the suite runs.** `setup()` rebuilds the bundle from `src/`, so a concurrent edit attributes someone else's half-written state to your run. Read-only prep (planning the next cut's exact line ranges) — or writing a NEW module file that nothing imports yet — is the correct thing to do while waiting: an unimported file cannot change the bundle.
|
|
5. Gate green → next cut. Gate red → fix, rebuild, amend, re-gate.
|
|
|
|
## Cutting mechanics
|
|
|
|
- Cut leaf/pure modules first (grammar, transcript rules, URL resolution, pure segmenter), then stateful ones (config, IO, pipeline, UI). Never let a module import `index` — if the new module needs something from `index`, that thing is a leaf and must move too (this is what forced `segment.ts` out early).
|
|
- Extract in **verbatim** bodies: read the exact ranges, write them into the new file, then delete the same ranges. Only module docs, export keywords, and injected dependencies change. Keeps each commit reviewable as a move.
|
|
- Edit tools that require "displayed lines" from the latest read snapshot: re-read the exact ranges under the current tag, then issue one edit call with all ops listed **bottom-up** (original line numbers throughout). Adjacent ranges merge into one `CUT a=b`; leaving a doubled blank line is the usual scar — one extra `CUT` next turn.
|
|
- A class member's JSDoc keeps the class's own indent (` /**` then ` * ` at member-level+1): a `PUT` body that starts at column 0 silently de-indents the whole block and the next `PUT` on those lines gets rejected for "restating the line below".
|
|
- Mutable config must reach a long-lived object as **getters**, not a construction-time snapshot: menu handlers mutate `cfg` after construction, and a snapshot freezes the settings the UI exists to change. Reading the getters once per unit of work (instead of per inner loop step) is a behavior improvement worth noting in the commit message.
|
|
- After moving code, grep the old file for the moved names: stale call sites compile under bundlers that skip typechecking (`stop()` vs `speaker.stop()` was caught only by `tsc`, not by `bun build`).
|
|
- A worktree may have no `node_modules` at all: the build fails on the natives loader with a version that doesn't match the primary checkout. `ln -sfn <primary>/node_modules <worktree>/node_modules` when `.gitignore` already anticipates the symlink (`git status` stays clean).
|
|
- Don't invent a third module to hold state the class already owns. If `queue.ts` would own no fields and need the pipeline injected as a callback, the queue stays in the class; say so in the report rather than shipping callback soup.
|
|
|
|
## Proving a new assertion is red-capable (before trusting it green)
|
|
|
|
Build the old bundle and point the test harness at it — no stash, no worktree juggling:
|
|
|
|
```bash
|
|
git show HEAD:dist/index.js > /tmp/old-dist.js # bundle is self-contained
|
|
# rebuild the throwaway harness by extracting the driver the suite writes
|
|
sed -n "<heredoc-body-range>p" tests/behavior.bats > "$W/driver.ts"
|
|
DV_EXT=/tmp/old-dist.js <same env as the test> bun "$W/driver.ts" <scenario> # expect RED
|
|
DV_EXT="$PWD/dist/index.js" <same env> bun "$W/driver.ts" <scenario> # expect GREEN
|
|
```
|
|
|
|
A pre-fix bundle copied to `/tmp` can fail to load its natives (`Cannot find module …/pi-natives@X/loader-state.js`) because module resolution follows the *bundle's* directory, and it reports every probe as zero — not red, just dead. The robust alternative in a private worktree: `git show HEAD:src/<file>.ts > src/<file>.ts`, rebuild, run the filtered bats tests (they rebuild from `src` themselves), then copy your version back and rebuild. Keep the copy in `/tmp` before overwriting.
|
|
|
|
Copy the project's real player/server stub verbatim — a simplified stub (e.g. one that skips writing the log file the driver counts) prints a false `PLAYS 0` and looks like a failing fix. Pin the scratch dir to a fixed path; `ls -dt /tmp/tmp.*` picks up another session's directory.
|
|
|
|
## Black-box test hygiene learned the hard way
|
|
|
|
- A test that counts artifacts in the **shared** temp directory observes every other process on the box (live agent sessions synthesize into the same `dankvoice-*` dirs). Give the run a private `TMPDIR="$WORK/scratch-tmp"` and assert there; the assertion's sensitivity is proven by the fact it caught a real leftover while scoped globally.
|
|
- Let preseeds reference the ephemeral fixture port with a placeholder (`__PORT__`) substituted by the driver, so a test can preseed a URL pointing at its own fixture server.
|
|
- Count what the user **heard**, not what was requested. Synthesis counts double-count legitimately whenever a turn is cut off and the remainder is re-synthesized, so an assertion like "the intro appears once" fails on correct code. Stamp each clip with the arrival index of the request that made it (the existing tag byte), print the played tags from the player stub, and emit one `SPOKEN <text>` line per played tag by joining tags back to request payloads — then count hearings.
|
|
- Pinning an assertion to a timing window: drive both knobs from env (when the cut lands, how long a clip plays) and choose a window wider than the poll interval, so the test can't go red from a race it isn't about.
|
|
- Refactor-only commits get **no new test**: re-asserting unchanged labels/order after a commit whose point is that they are unchanged is a tautology; the existing suite is the gate. New tests belong to behavior changes, red-first.
|
|
- Tests must observe **output**, never internals: assert `PLAYS`/`CALLS`/`CALL <url>`/`NOTIFY <text>`, not that a variable was set.
|
|
- When a behavior fix turns a cluster of pre-existing tests red, check whether the cluster was pinning the *delivery path* rather than the feature: scenarios that drove a "only-unsolicited-narration" knob through a typed command were accidentally pinning the retired behavior. Move the scenario to the real path (deliver through the harness event) and leave the assertions word-for-word; strengthen any test whose fixture never answered (its assertion was absence, not evidence). Report the move explicitly — it looks like nine changed tests.
|
|
|
|
## Docs after a split (same branch, own commit)
|
|
|
|
Move each explanation to the file that now owns it (knob list → the module that reads env; state-machine prose → the module that implements it) and leave the entry point a one-line-per-module map. Then grep for pointers into the old file — a README saying "see header of `src/index.ts`" becomes a lie the moment the knobs move. Also re-check the laws table's test pointers: a law that claimed "pinned by scenarios asserting X" while those scenarios never armed the feature is a false citation, found by running the suite once.
|