`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.
17 KiB
Handoff — a watch for a running program (PORTING.md Tier 1, item 5)
Written because the session was wound down for budget. The work is finished and dune test is green, run twice.
This file exists so the reasoning is not re-derived, and so the two things I would have done next are named.
The finding that reframes the task
Most of item 5 was already built before this session. The brief reads as though a watch had to be built from
nothing; it did not. What existed at 344e571:
runtime/flan_dev.c— the watch table: 64 slots, a name and rendered text per slot, a per-slot seqlock, anon/offflag so a watch call costs a load and a not-taken branch when nobody is looking, and four scalar entry points (flan_dev_watch_i64/_u64/_f64/_str).vendor/agent/flan_agent.c—watch,watch on,watch offon the agent socket.lib/dev.ml—watch_read,watch_enable, and the:op "watch"/:op "watch-enable"arms.emacs/flan-watch.el— the watch buffer and the inline ghost text, both fed from one reply.test/test_dev.ml— a block driving a real daemon and a real running program.
That is watch.clj's spy, end to end. What was missing was spy-num — the numeric accumulator for hot loops,
which is precisely the part PORTING.md calls "least obvious and most valuable". That is what this session built.
The accumulator decision
This is the expensive part to re-derive, so it is stated in full. The long-form version is in BUILT.md, "A hot loop
keeps five numbers, and the window is the editor's"; runtime/flan_dev.c carries it at the code.
What a slot keeps
Count, min, max, last, mean. Five numbers per label.
The scalar watch keeps one value per name. Sampled from a hot inner loop that is nearly useless — you see whichever
of the 91,200 cells happened to run last. watch.clj's own docstring says so and is why spy-num exists there.
Each of the five answers a question you can ask without building a query:
n— how many times the expression actually ran. This is the first thing that is wrong when a loop is wrong. A count that tracks the frame counter rather than the cell count is a loop that is not running.min/max— the range. This is the thing a single sample can never show you, and it is what you are looking for when you suspect an index or a velocity is leaving the region it should stay in.last— the one sample. Kept because it is what the scalar watch would have given you, and losing it on the way to something richer would be a regression.mean— carried as a runningsumand divided at read time. A mean accumulated as a mean drifts; a sum does not.
What it deliberately does not keep
A ring, or a history. A small ring of the last N samples was the serious alternative. It loses on the only ground
that matters here: N samples out of 91,200 is a sample of the tail of the loop, not of the loop. It answers "what
did the last few cells do" when the question is "what did the cells do". Past five numbers, every richer answer is a
UI for building a query — and a query builder is the one thing this whole design exists not to be. The ghost text
section of BUILT.md already settled the same question the same way for the scalar case ("a watch inside a loop
shows the last value written... every better answer is a UI for building a query. Settled, not open.").
The write path does no formatting
This is the feature, not an optimisation. An snprintf per sample at thousands of samples a frame is a HUD that
costs more than the game. A sample is: one relaxed load of the epoch, five compares and stores, and the slot's
seqlock. The reader — the agent's listener thread, once per editor tick — turns the five numbers into text.
watch.clj reaches the same place with a double-array per label and a render function Emacs calls.
This is also why the slot grew a num flag rather than getting a second table: the read path already copies under a
seqlock, so it copies five doubles instead of 192 bytes and renders them out of locals after the counter check.
The window is since the last reset — a deliberate divergence from watch.clj
watch.clj is cumulative until reset-spies! is called by hand. This is not, and the disagreement is on purpose.
Cumulative is the wrong default for a frame loop. A min and a max over a whole session reach the session's
extremes within a few seconds of play and then never move again — so the two most useful of the five numbers go dead
exactly when you start interacting with the thing you are debugging. This tool exists to show you a number while
you drag the mouse. So flan-watch--tick sends :reset t beside its read, and what you see is "since you last
looked": a fifth of a second, about a dozen frames, which keeps the range tracking the present.
A caller who wants cumulative numbers gets them by not resetting. The setting lives in the editor, not the runtime.
Reset is its own message, never a side effect of reading
A destructive read was the tempting shape and is wrong: it makes looking change what is there. Anything that polls
— a test's await, a second editor, a person reading twice — would silently shorten the window and come back with a
count that is noise. So watch reset is its own agent command, :reset t is a field on the read op, and dev.ml
sends the reset after the read so a tick reports the window it just closed.
This was settled before the test was written rather than after, which is the only reason the test can assert anything
about n at all.
The reader never writes the table
Reset bumps one global epoch counter and touches no slot. A slot clears itself on its next sample, when it
notices the epoch moved, and does so inside its own odd-generation window so a reader can never catch a
half-cleared slot. The game thread stays the only writer of the table — the invariant the whole watch section of
flan_dev.c rests on.
The cost: a new window begins when the program next runs, not at the instant of the reset. For a frame loop that is the only moment it could sensibly begin. This bit me in the test (see "What did not work" below) and the test now waits for it and says why.
i64 accumulates as a double
A magnitude past 2^53 loses precision in the sum and in the ends of the range. Recorded rather than designed around: a
count, a coordinate and a tile index are what this is pointed at, and a second integer accumulator for a case nobody
has would be two code paths for one tool. watch-num-i64 and watch-num-f64 are two entry points only so a program
need not cast at the call site.
A whole number prints as one
watch.clj's fmt-num does this and the reason survives the port: a watch on an array index that reads 66.0000
sends you looking for a rounding bug that is not there.
What was built, file by file
All of it is working — built, run against a real daemon and a real running program, and covered by the test. Nothing is stubbed and nothing is half-written.
| File | Change | State |
|---|---|---|
runtime/flan_dev.c |
watch_slot grew num, epoch and five doubles; watch_begin clears num; new section "A number sampled thousands of times a frame" with watch_record, flan_dev_watch_reset, flan_dev_watch_num_i64, flan_dev_watch_num_f64, watch_num_str, watch_render_num; flan_dev_watch_read renders a num slot instead of copying its (unused) text |
working |
vendor/agent/flan_agent.c |
flan_dev_watch_reset declaration; a watch reset command beside watch on/watch off |
working |
lib/dev.ml |
watch_read takes ~reset and sends watch reset after the read; the :op "watch" arm reads a :reset field the same way watch-enable reads :on |
working |
emacs/flan-watch.el |
flan-watch-ghost-call-regexp ? → *; docstring updated; flan-watch--tick sends (:op "watch" :reset t) |
working |
test/programs/dev-watch.flan |
declare-c watch-num-i64; a loop-cells hot loop sampling "cell" eight times a step at 0,3,…,21 |
working |
test/test_dev.ml |
assertions inside the existing watch block: the row exists, n > 8 (per-sample not per-step), min < max (a range, not just last), a plain read does not reset, :reset t reopens the window |
working |
BUILT.md |
new section "A hot loop keeps five numbers, and the window is the editor's", after the ghost text one | working |
PORTING.md |
item 5 struck in house style with the accumulator decision | working |
NEXT.md |
"What is left on PORTING.md's list" corrected: item 5 done, item 6 is next |
working |
Verified end to end, not just compiled
Against a real flan dev daemon and the real running program, the "cell" row read back:
n=184 min=0 max=21 last=21 mean=10.5000 (first read)
n=368 min=0 max=21 last=21 mean=10.5000 (second read — a plain read does not reset)
n=16 min=0 max=21 last=21 mean=10.5000 (after :reset t — the window reopened)
The loop is 0,3,…,21, so min, max, mean and the per-step count of 8 are each pinned by a different part of it.
Test result
dune test is green, run twice in full. test_dev.ml's first block — the separately-known-flaky socket bind race
— failed on one earlier run with "the daemon never listened" and passed on both final runs. That is exactly the
documented flake and another agent is on it; it is not related to anything here.
The ghost text path — the brief's assumption, checked
The brief said to reuse the ghost text path rather than invent a second one. It fits, and it needed one character.
The important property is in BUILT.md: ghost text anchors on the string literal in the source, not on a source
location carried in the table. flan-watch--ghost-sites scans buffers shown in a window for
(<head> "<name>" and matches the name against the rows. So a new kind of watch needs nothing new in the daemon,
nothing new in the wire, and no new display path — the accumulator's rendered value is a string like
n=368 min=0 max=21 last=21 mean=10.5 and flows through the existing :watch rows untouched.
Is it pull-based? No, and the brief's phrasing is worth correcting for whoever reads this next. The watch is
push-based and that is load-bearing: watch.clj is pull (Emacs polls a render function), and flan_dev.c's
comment explains at length why that does not port — an evaluation here compiles a module and dlopens it, so a 5Hz
poll would be hundreds of .so files a minute. So the program pushes into a table and the editor reads memory.
Ghost text is not a poller at all: it is painted from flan-watch--absorb, off the same single reply that paints the
buffer, which is what keeps the two from disagreeing and preserves the one-request-in-flight invariant. The
accumulator sits inside that unchanged.
The one thing that did not fit. flan-watch-ghost-call-regexp was watch\(?:-[[:alnum:]]+\)? — one optional
hyphenated segment. [[:alnum:]] does not match a hyphen, so on (watch-num-i64 "cell" ... the regexp matched
watch-num, then required whitespace, found -i64, and backtracked to failure. A numeric watch therefore appeared
normally in the watch buffer and got no inline value at all — a silent half-failure. * for ? is the whole
fix, verified in emacs --batch against all four head shapes.
That is worth recording as the predictable cost of the decision the ghost text section defends. Anchoring on the name
rather than on the head is what makes the head a defcustom, and a defcustom with an enumerated default is a
default that needs widening whenever the set of heads grows. Cheaper than the alternative, but not free.
What remains
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.
One item is left, and it turned into a finding rather than a test.
-
watch_render_num'sn=0branch is unreachable, so the test that was asked for cannot be written.runtime/flan_dev.c:764rendersn=0 last=<v>for a window with no samples. Nothing can present that state to the reader:s->numis set to 1 in exactly one place,watch_record(flan_dev.c:701), and that function always falls through tos->n += 1. The clear (s->n = 0on an epoch change) and the increment are both inside one odd-generation window, so a reader cannot see between them.flan_dev_watch_beginsetss->num = 0and never touchess->n, so the scalar path cannot leave a num slot at zero either.flan_dev_watch_readnever comparess->epochtowatch_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 whytest_dev.ml's reset assertion has toawaitthe 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->epochwithwatch_epochunder the seqlock and rendern=0when they differ. This makes a reset visible immediately without the reader writing the table, and would let the existingawaitin 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 ownsruntime/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.
What I tried that did not work
-
Two Flan type errors in
dev-watch.flan.(* i 3)over alet-bound0isi32, andwatch-num-i64wantsi64— fixed with an explicit(i64 …). Thenloop-cellswas declaredi64and returned thei32counter; declaredi32. Both are ordinary, but they cost two daemon round trips to find because a build error goes to the daemon's stdout, not to the client — worth knowing if you are drivingflan devby hand. -
My first probe script spoke the wrong protocol.
Wire.sendis length-prefixed ("%d\n%s"), not newline-delimited. A newline-delimited client gets the connection closed with no error text. If you write a throwaway client, readlib/wire.ml:32first. -
Two attempts at the reset assertion failed before the third stuck, and both failures were informative rather than noise:
- "the window did not reopen: 8 after 8" — one step writes eight samples, so two reads back to back leave no room under the count for a reset to show in. The test now runs the accumulator up past 32 first (which is itself the other half of the claim: every one of those polls is a plain read, and a plain read must not reset, or the count could never climb).
- "the window did not reopen: 40 after 40" — reading immediately after
:reset tstill sees the old count, because the reset bumps an epoch and the slot clears on its next sample. That is the design and not a bug; the test nowawaits the drop and the comment explains why the laziness is what keeps the reader out of the table.
-
Not attempted, deliberately: the
(watch "hp" hp)arm incheck.ml. BothPORTING.mdandflan_dev.csay it is only wanted for composites (a struct, a slice, a union), which needRender.renderpointed at theflan_dev_watch_emit_*emitters. A scalar and an accumulator both reach the runtime by plaindeclare-cand need no compiler change, which is the property that kept this change out ofcheck.mlentirely. -
One worktree note. This worktree was checked out at
2c232dd, an ancient commit, rather than atdev-loop's tip — none ofPORTING.md,NEXT.md,lib/dev.mlorruntime/existed in it.git reset --hard dev-loopon a clean tree fixed it. Also,dunefinds the parent repo's root from inside a worktree, so every build here isdune build --root ./dune test --root ..