PR #508 follow-ups: DrawTicks has no assertion, and three probe-doc drifts nf-2v69 ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/521 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #508. All are below the gate's severity floor — #508 is CONVERGED and none of these should hold it up. Filed unclaimed.

1. DrawTicks has no assertion anywhere — the harness cannot express a delta

PianoRoll.DrawTicks (games/tonapse/ui/PianoRoll.cs:1339) is the one instrument that survives Aedan's *"maybe the probe is causing enough of an update for the animation to show for u"* objection, because reading a property does not QueueRedraw. But expect_prop (shared/VisualProbe.cs:712-723) compares one read to a literal, so "grew by ≥ N across this gap" is inexpressible — tools/probes/playhead-song.json asserts PlayheadShown and PlayheadAdvancing ten times and DrawTicks zero times. It is a manual-probing instrument only.

An expect_prop_delta step (read, wait_ms, read, assert the difference) would make repaint rate gate-able. #517 is the first thing that needs it — a frozen roll is exactly a DrawTicks delta of ~0 — so this probably lands with that work rather than alone.

2. The hidden-roll path of PlayheadShown is unpinned

PlayheadShown returns hidden when !IsVisibleInTree() (PianoRoll.cs:1372-1373), added because VisualProbe.FindPropertyInTree falls back to matches[0] when no carrier is visible (VisualProbe.cs:828-829) and would otherwise hand a probe lit Song for a screen with no roll on it.

Mutation-wise the guard is half-live: inverting it turns the probe red, removing it survives, because nothing in the sweep exercises a hidden roll. #508 states this limit honestly and routes it to #517 (one-screen layout is that issue's subject). Recording it here so it is findable from the PR rather than only from a comment.

3. shot: "playhead-song-03-ghosted" names the behaviour it proves absent

tools/probes/playhead-song.json — that shot sits directly under expect_prop PlayheadShown is "off Song". The 22% ghost was tried and rejected (*"I believe this is the wrong fix entirely"*) and reverted in c150d9c; the filename is the last artifact on the branch still naming it, and it lands in probe_out/ on every gate sweep. playhead-song-03-off or -dark.

4. The probe's _intent says "WAIT_MS, NEVER WAIT" and the same file uses wait three times

tools/probes/playhead-song.json _intent: *"WAIT_MS, NEVER WAIT (#508 F1)"* — while the steps carry wait: 45 (startup settle), wait: 3 and wait: 4 (post-focus settles). Those three are correct as written: none of them gates a wall-clock assertion, and the ms offsets are all measured from ui_accept, which comes after them. But the rule as stated is absolute, and the _intent is deliberately written as durable guidance for the next person — who will either be confused or "fix" three harmless settles. One clause: frame waits for settling, wait_ms for anything asserting a wall-clock fact.

Explicitly dropped, not worth doing

Related: #508, #496, #517, #513.

Dependencies

None.

Comments

No comments.

Add a comment