diff --git a/HANDOFF-f3.md b/HANDOFF-f3.md index 9e35de7..a1015ab 100644 --- a/HANDOFF-f3.md +++ b/HANDOFF-f3.md @@ -179,25 +179,43 @@ default that needs widening whenever the set of heads grows. Cheaper than the al --- -## What remains, in the order a fresh session should do them +## What remains -Item 5 is complete. These are the two gaps I would close next, both small. +Item 5 is complete, and so is the fixture the previous list asked for: `emacs/test-flan-watch.el` now has a block +"Every head the default regexp claims to cover" pinning `watch-num-i64` and `watch-num-f64` beside the one-segment +heads. It was verified by mutation — restoring the old `?\=` in `flan-watch-ghost-call-regexp` makes exactly those +two checks fail and nothing else — so a regression in that regexp is now caught. -1. **`emacs/test-flan-watch.el` — add a `watch-num-i64` fixture site.** The regexp fix is covered by *nothing* - automated; I verified it by hand in `emacs --batch`. Add a fixture whose buffer text contains - `(watch-num-i64 "cell" x)` and assert `flan-watch--ghost-sites` returns `("cell" . …)`, alongside the existing - `watch-i64` fixtures. No daemon needed — the file is already in the suite via `test-flan-cider.el` and, per - `BUILT.md`, ghost text is a pure function from rows plus buffer text to overlays. **This is the highest-value - remaining item**: it is the one change in this commit with no regression test behind it. +One item is left, and it turned into a finding rather than a test. -2. **`test/test_dev.ml` — assert the `n=0` render.** `watch_render_num` in `runtime/flan_dev.c` has a branch for a - window with no samples that emits `n=0 last=` (the range and the mean have nothing behind them, but `last` is - still the last value the name ever had). No test reaches it: the program in `dev-watch.flan` samples every step, - so a window is never empty in practice. Reaching it needs a reset followed by a read inside one step, or a - program with a watch call that runs only sometimes. +1. **`watch_render_num`'s `n=0` branch is unreachable, so the test that was asked for cannot be written.** + `runtime/flan_dev.c:764` renders `n=0 last=` for a window with no samples. Nothing can present that state to + the reader: -Neither blocks anything. Item 6 (frame rollback as a worked example) is the next real piece of work and `NEXT.md` now -points at it. + - `s->num` is set to 1 in exactly one place, `watch_record` (flan_dev.c:701), and that function always falls + through to `s->n += 1`. The clear (`s->n = 0` on an epoch change) and the increment are both inside one + odd-generation window, so a reader cannot see between them. + - `flan_dev_watch_begin` sets `s->num = 0` and never touches `s->n`, so the scalar path cannot leave a num slot + at zero either. + - `flan_dev_watch_read` never compares `s->epoch` to `watch_epoch`. A reset therefore does not make the slot read + as empty. + + The user-visible consequence is worth stating on its own, because it is the thing the branch was presumably + written for: **after `:reset t`, a read taken before the program's next sample reports the *previous* window's + five numbers, not an empty one.** That is why `test_dev.ml`'s reset assertion has to `await` the count dropping + rather than read once — the test is already working around this. + + Two ways out, and both are decisions rather than test work, which is why neither was taken here (the lane was + scoped to tests): + + - **Delete the branch.** Honest about what the design says: a window begins when the program next runs, and until + then the last window is what there is to show. + - **Make the read epoch-aware** — compare `s->epoch` with `watch_epoch` under the seqlock and render `n=0` when + they differ. This makes a reset visible immediately without the reader writing the table, and would let the + existing `await` in the test become a single read. It is a behaviour change to what an editor sees in the tick + after a reset, so it belongs to whoever owns `runtime/flan_dev.c`, not to a test pass. + +Item 6 (frame rollback as a worked example) is the next real piece of work and `NEXT.md` points at it. --- diff --git a/emacs/test-flan-watch.el b/emacs/test-flan-watch.el index 6e7a21c..5b33c68 100644 --- a/emacs/test-flan-watch.el +++ b/emacs/test-flan-watch.el @@ -67,6 +67,37 @@ what bounds its cost; a `with-temp-buffer' would be scanned by nothing." "and a call written inside a string is not one of them" (not (member "fake" (mapcar #'car sites))))))) +;; --- Every head the default regexp claims to cover ------------------------ +;; +;; `flan-watch-ghost-call-regexp' allows *any number* of hyphenated segments, +;; and the accumulator is why. With one optional segment — the `?\=' this +;; default used to have — `watch-num-i64' matched as far as `watch-num', then +;; required whitespace, found `-i64', and backtracked to failure. The failure +;; is silent and looks like nothing: the row is in the watch buffer as usual +;; and the call site simply never gets an inline value. So the two-segment +;; heads are pinned here beside the one-segment ones, or the next widening of +;; this default has nothing to fail against. + +(let ((buf (test-flan-watch--buffer + (concat "(defn step [] i64\n" + " (watch \"bare\" x)\n" + " (watch-i64 \"ticks\" ticks)\n" + " (watch-f64 \"hp\" hp)\n" + " (watch-str \"label\" label)\n" + " (watch-num-i64 \"cell\" (at grid i))\n" + " (watch-num-f64 \"dt\" dt))\n")))) + (with-current-buffer buf + (let ((sites (flan-watch--ghost-sites))) + (test-flan--check + "a two-segment head is a site: the accumulator gets inline text too" + (equal (assoc "cell" sites) + (cons "cell" (save-excursion (goto-char (point-min)) + (line-end-position 6))))) + (test-flan--check + "and every head the default documents is found, in buffer order" + (equal (mapcar #'car sites) + '("bare" "ticks" "hp" "label" "cell" "dt")))))) + ;; --- Matching a row to a site --------------------------------------------- ;; ;; The runtime holds 31 bytes of a name, so a longer one in the source can