Skip to content

A pass skips finished run files without parsing them - #54

Merged
tomkaczocha merged 5 commits into
mainfrom
t3/placeholder-2
Oct 16, 2026
Merged

tomkaczocha merged 5 commits into
mainfrom
t3/placeholder-2

Conversation

@rawphp

@rawphp rawphp commented Oct 16, 2026

Copy link
Copy Markdown

Closes #49

Contract

  • done: listUnfinishedRuns skips a run whose file's last complete line is a finished event by reading at most 8192 bytes from the end of the file, never the whole file. Unfinished runs are reported exactly as before (id, state, confirmed, unreadable, session) in the same order. A file whose last line is cut off, whose last complete line is not JSON or not a valid entry, or whose tail cannot decide (a line longer than the tail, a tail of only blank lines) is read in full as before. No index file or archive directory; the runs/ format is unchanged.
  • out: the ledger writer, the runs/ layout, readProgress, and the other callers of readRun (gate.ts, start.ts).
  • risk: none
  • test: vp test run apps/server/src/factory/ledger/listRuns.test.ts → pass

Acceptance criteria

  • A finished run is skipped by reading only the end of its file; a large finished file is never read whole, shown by a spy on the full read — listRuns.test.ts "never reads a large finished run whole, and still lists an unfinished sibling" (a > 64 KiB finished file; vi.mock("node:fs") records readFileSync calls), plus "skips a small finished run, also after a trailing blank line or CRLF".
  • An unfinished run is reported exactly as today and in the same order; the existing listRuns.test.ts cases pass unchanged — every pre-existing case is untouched and green.
  • A file whose last line is cut off or not JSON is treated as unfinished and read in full — "reads a file whose last line is cut off in full and lists it as unfinished", "reads a file whose last complete line is not JSON in full and lists it as unfinished", "reads a file in full when its last line names finished but is not a valid entry", "reads a file in full when a cut-off fragment follows the finished line", "a finished line longer than the tail is still skipped, after a full read", "lists a run file of only blank lines as unfinished, as readRun reads it", "lists a large run file whose whole tail is blank lines, with its state".
  • No new index file or archive directory — the change is a bounded readSync at the tail of each run file (read.ts lastLineIsFinished); nothing is written.

Gate

  • vp test run apps/server/src/factory → exit 0 (31 files, 952 tests)
  • vp run --filter t3 typecheck → exit 0

Models

  • maker: claude-sonnet-5-5 (no models block in .dw/config.yml, so the roster default was used)
  • checker: claude-opus-5-5
  • orchestrator: this session (Fable 5.1, not the roster's Opus 5.5)

Review

  • cycles: 2
  • cycle 1 (correctness, reliability; both read-only): one blocker, found by both. The blank-line step-back in lastLineIsFinished called Buffer.lastIndexOf with offset -1 when the walk reached the start of the tail; a negative offset counts from the end, so a file of only "\n" (or a tail of only blank lines) looped forever and would have hung the pass. Fixed test-first in 7d264025c + baee5a7da.
  • cycle 2 (testing, mutating, 8 named mutants in a detached tree): 6 killed, 2 survived, both coverage gaps with no behavior change. Added as tests in cd28e906c: a last line that names finished but is not a valid entry, and a cut-off fragment after a complete finished line, both proven to force the full read.
  • red/green: the first test: commit was run in a detached tree at ff4348f00 (2 failed, 15 passed) and the tip is green. The red for 7d264025c was a hang, reproduced by both reviewers and the maker. cd28e906c pins existing behavior and had no red phase.
  • leftovers: none

Isolate

  • The worktree was behind origin/main with a clean tree; fast-forwarded e4bcce9f6 → 9e8095616 before provisioning.
  • Labels: all seven factory:* labels already existed; none created.

Lessons

  • Check the worktree's real branch name before the first push. Symptom: the first git push -u origin <branch> failed with "src refspec does not match any". Cause: the launch context named the worktree's directory suffix as the branch, but the checked-out branch had a different name. Default: at Isolate run git branch --show-current and push that name; never take the branch from the directory or the launch message.
  • Remove the detached review tree before the gate. Symptom: a test command scoped to a path reported twice the test files. Cause: the detached review worktree lived under .factory/, and the test runner's path filter is a substring match, so it collected the copies too. Default: git worktree remove the review tree as soon as the mutating reviewer reports, before any gate or scoped test run from the main worktree.
  • A hang-mutant needs a process group, not an alarm. Symptom: perl -e 'alarm 60; exec ...' did not stop a hung vitest run; only the parent died and the forks lived on. Default: start the run with setpgrp in its own group and bound it with a wait loop that kills that group by its pid (kill -TERM -- -$pid), which only touches processes the reviewer started.

Commits

  • ff4348f00 test: finished runs are skipped by reading only the file's tail — Make slice (red)
  • a264b7b5c feat: listUnfinishedRuns skips a finished run by its tail — Make slice (green)
  • 7d264025c test: a run file of only blank lines is listed as unfinished — Check cycle 1 (red: hang)
  • baee5a7da fix: lastLineIsFinished stops at a blank line that starts the tail — Check cycle 1 (green)
  • cd28e906c test: a finished-named invalid line and a cut-off fragment after finished are read in full — Check cycle 2

Files

  • apps/server/src/factory/ledger/read.ts — new exported lastLineIsFinished(path): lstat guard, one bounded readSync at the tail, parseLines-compatible decision; readRun unchanged.
  • apps/server/src/factory/ledger/listRuns.ts — calls it before readRun and skips the run when true, inside the existing try/catch so IO failures still list the run as unreadable; doc comment updated.
  • apps/server/src/factory/ledger/listRuns.test.ts — the spy and nine new cases; existing cases untouched.

Two cases fail: a large finished run and small finished runs are still
read whole by listUnfinishedRuns.

Issue: #49
A run whose last complete line is a finished event is skipped after one
bounded read from the end of its file. A cut-off or invalid last line,
or a finished line longer than the tail, falls back to the full read.

Issue: #49
A run file holding only blank lines, or a large file whose last 8192 bytes are all blank, must be listed as unfinished exactly as readRun reads it (state null, or the state of its first line). Red: before the fix the run hung node (the vitest worker printed no result and had to be killed after the 60s alarm did not stop it).

Issue: #49
The blank-line step-back called lastIndexOf with a byteOffset of -1 once it reached the first byte, which Buffer reads as counting from the end, so the walk looped forever. With no earlier line it now returns false and the file is read in full.

Issue: #49
…shed are read in full

Both tests pin existing behavior, so there was no red phase. They kill two mutation survivors: a last line naming finished that is not a valid entry, and a cut-off fragment after a complete finished line, must each force a full read.

Issue: #49
@rawphp
rawphp marked this pull request as ready for review October 16, 2026 03:49
@rawphp

rawphp commented Oct 16, 2026

Copy link
Copy Markdown
Author

Factory gate: passed (attempt 1, run by the factory in a fresh checkout of the pull request's head cd28e90 merged with the base branch tip 9e80956)

Command Exit Time
factory/scripts/check.sh 0 35.6 s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A pass skips finished run files without parsing them

2 participants