A numeric watch has a site, and an empty window has none
`flan-watch-ghost-call-regexp' allowed one hyphenated segment until this lane's sibling widened it, so `watch-num-i64' matched as far as `watch-num', required whitespace, found `-i64' and backtracked to failure: the row appeared in the watch buffer as usual and the call site got no inline value at all. Nothing automated covered the widening. It does now — a block pinning both accumulator heads beside the four one-segment ones, verified by putting the `?' back and watching exactly those two checks fail. The other half of that handoff asked for a test of `watch_render_num''s `n=0' branch. It cannot be written: `s->num' is set in one place, `watch_record', which always falls through to `s->n += 1', and the clear and the increment sit inside one odd-generation window. The reader never compares the slot's epoch to the global one, so a reset does not make a slot read as empty either. The branch is dead, and the live consequence is that a read taken after `:reset t' and before the program's next sample reports the previous window — which is why the existing test waits for the count to drop rather than reading once. Recorded in HANDOFF-f3.md with the two ways out, both of which belong to whoever owns runtime/flan_dev.c.
This commit is contained in:
parent
a016c3f226
commit
6e7b53b988
@ -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=<v>` (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=<v>` 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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user