diff --git a/docs/superpowers/specs/2026-10-08-resequencing-and-relative-timing-design.md b/docs/superpowers/specs/2026-10-08-resequencing-and-relative-timing-design.md new file mode 100644 index 0000000..aa00eef --- /dev/null +++ b/docs/superpowers/specs/2026-10-08-resequencing-and-relative-timing-design.md @@ -0,0 +1,319 @@ + + + +# A late packet leaves a hole it could have filled, and the clock it would be filed under is the wrong one + +Two requests, and they are one design. + +The first is that a reordered packet currently reads as permanent damage. `udps_client.c:504-514` +orders the stream by the wrapping `uint32` counter and discards anything that arrives behind the +newest counter — `stats.stale_packets++; return 0` — *before* spending anything on the payload. +The samples are gone, the span they should have filled stays empty, and udpscope never learns +there was a span at all. The asked-for remedy is to leave a hole of +` × `, mark it invalid, and fill it when the +missing packets arrive. + +The second is `todo.md` item 2: a relative-time mode in which the plot's x comes only from the +declared sample rate and the packet number, with absolute stream time kept for export. + +They are one design because **the counter is what makes a hole addressable**. A reserved hole has +to be a known number of samples at a known position, and a late packet has to compute which +position it belongs to. Absolute stamps cannot do either — a lost packet's stamps were never +received, so its extent in time is a guess, and the guess is what the glitch bridge at +`src/acquisition.cpp:274-292` is already making. The counter gives the extent exactly. Relative +timing is not a cosmetic preference that happens to be requested at the same time; it is the +coordinate system in which resequencing is arithmetic rather than inference. + +Two facts from the existing code shape almost every decision below, and both say the work is +smaller than it looks: + +- **`frame->counter` and `frame->lost` are read nowhere in udpscope.** `frame.lost` is set by the + client at `udps_client.c:560` and appears in `src/acquisition.cpp` only inside comments + (`:309`, `:331`, `:336`). The client has been reporting the exact gap width for the life of the + project and nothing has ever consumed it. Neither input needs inventing. +- **`clamp_deadline()` already exists** (`udps_client.c:1102`), documented as *"Clamps `budget` so + a pending deadline is not slept through"* and already used for the keepalive and the silence + timeout. The reorder deadline is a third call to it. + +## What was decided, and the constraint that decided it + +The latency budget is **20 ms**, maximum. That one number settles the shape of the whole receive +path, because it is a *time* budget and not a packet count: at 1000 pkt/s it is twenty packet +spans of reorder depth, at 10 pkt/s it is a fifth of one. A hold policy counted in packets would +either be useless at high rates or add 400 ms at low ones. A policy counted in wall time works at +every rate, because network reorder is measured in milliseconds regardless of how fast the +producer sends. + +It also means a *contiguous* arrival must be released immediately. The 20 ms is spent only when +there is a gap to wait on, never on the common case. + +Past the window, a committed hole is still **backfilled for values only**. Math channels and the +trigger are not replayed. A math channel takes NaN in and gives NaN out, so after a repair the +raw trace is whole and the math trace derived from it keeps its gap. That is a deliberate, +documented state, not an oversight: the display then says *"this region arrived too late to be +part of the computation"*, which is true and is cheaper than any mechanism that could make it +false. + +Resequencing and hole reservation run in **both** time modes. In relative mode a hole's stamps +are exact; in stream mode they are the declared cadence continued from the last good stamp, which +is the existing bridge mechanism reused. Relative timing is therefore an independent toggle a user +flips for jitter, not a prerequisite for recovery. + +A hole is surfaced two ways: **status-bar counters** and a **shaded band** over the invalid span. +A NaN run already breaks the polyline, but a one-packet hole at a 10 s window is sub-pixel, and a +gap nobody can see is not a report. + +## Relative timing + +`build_stamps()` today picks its clock *by inspection*: `time_source_` latches 1 for the declared +time signal or 2 for the arrival clock and never flips back (`src/acquisition.cpp:357-367`). +Relative timing is not a third thing to infer. It is a user-selected mode that bypasses the +inference: `--relative-time` on the command line, a toggle in the UI, persisted in the session +file. + +### The stamp + +``` +stamps_[k] = double(slot_base_ + k) / rate +``` + +`slot_base_` is a running `u64` of samples seen. Each packet the counter reports missing advances +it by `batch`, **before** the frame's own samples are stamped; the delivered frame then advances it +by its own `n`. The order is what puts the hole ahead of the data that revealed it. + +This is the requested `counter × batch + sample_index` with the origin left at counter zero, but +**accumulated rather than multiplied**, and the difference earns three things: + +- `batch` is then only ever needed to size what is *absent*. That is exactly the + `` of the original formula, and it is the only place the value is load + bearing. +- The `uint32` wrap costs nothing. The counter is consumed only as a signed delta — the same + arithmetic `udps_client.c:505` already performs — so no unwrapping bookkeeping is needed. +- A frame whose `n` disagrees with the established `batch` cannot silently shift the whole + timeline, because the timeline is built from the `n` values actually seen. + +`batch` is established from the first DATA frame and held for the life of the stream. A later +frame disagreeing is a genuine inconsistency in the producer, not a loss, so it warns once rather +than re-deriving. + +### The rate is the declared one, and only the declared one + +`build_stamps()` currently overrides a declared cadence with the arrival-measured one when the two +disagree by more than ±25% (`src/acquisition.cpp:152-158`), because a CONFIG declaring 1 MS/s +while delivering a hundred samples every fifth of a second reads as a staircase in the x column. +Relative mode must **not** do this. Discarding arrival timing is the entire point of the mode. The +override stays live in stream mode and is skipped in relative mode. + +### The constant offset is already absorbed + +`src/acquisition.cpp:555` stamps `rel = stamps_[k] - epoch`, and the `t0 =` caption names the +epoch. A counter that happens to read 500 000 on connect therefore shows up as an epoch value, not +as a 50 000 s axis. No origin subtraction is needed in the formula itself. + +### The x axis needs no change at all + +`RowSet::update_time_labels()` already goes to a relative label form on the tick step against the +label resolution at the axis' magnitude, with the `t0 =` caption naming the absolute anchor +(`docs/claude/invariants-time-axis-and-window.md:34`). "Counting backwards, like it is now" is +what that already does. Relative mode changes *where a stamp comes from* and nothing in the label +layer. + +### What relative mode makes unreachable + +The degeneracy guard, the implausible-jump test, `kFailureLimit`, the bridge and the monotonic +clamp are all dead in relative mode: the stamps are monotone by construction, so there is no bad +stamp to detect and nothing to clamp. (`kGlitchPackets` itself survives — not as the threshold of +that test, which is unreachable, but as the reservation cap below, where its value and its +reasoning carry over unchanged.) + +That is not tidiness, it is a fix. Measured last session: a loss burst past `kGlitchPackets` (256) +is misread as a producer clock glitch; each bridge then advances the anchor by *one* packet span +while the true timeline is 300 ahead, so the next four packets fail too, `kFailureLimit` trips, +and the whole stream is thrown onto the arrival clock — the exact failure the bridge was added to +prevent (`--drop-burst 300` reads +`jumped implausibly far ahead ... (1.299..1.29999 against a previous 0.99899)` then +`timing samples by arrival instead`). In relative mode a 300-packet gap is a known quantity and +cannot be misdiagnosed. + +### Absolute time for export + +`todo.md` requires absolute stream time to remain available for export. There is exactly one f64 +time ring per `BufferStore`, shared by every channel of that buffer, and relative mode takes it. +Export therefore needs a **second** f64 ring carrying the absolute stamp under the same group-mean +rule, written in relative mode only and read by export and by nothing else — eight bytes per +stored slot. + +This cost is paid rather than avoided. Without it, selecting relative mode silently degrades the +one output `todo.md` says must keep absolute time. + +## The receive path + +### In the client: a reorder ring of reassembled payloads + +Every `udps_frame_t` sets `frame.signals = c->sigs; frame.values = c->vals` +(`udps_client.c:566-567`) — one client-owned decode buffer, reused for every packet. A frame is +therefore valid only inside the callback, and holding one requires a copy *wherever* the holding +happens. The client has no advantage there. + +It has a different one: it still holds the **raw datagram**. Resequencing undecoded wire bytes is +strictly cheaper than resequencing decoded doubles — an `f32` or `i16` payload is two to four +times smaller than what `c->valbuf` would hold — and the bytes exist nowhere else. That is why +this sits in the client and not in `Acquisition`. + +The hook is the `decode_data()` call site at `udps_client.c:599`: after fragment reassembly, which +already tolerates arrival order through the slot table, and before decode. Each held entry carries +the payload bytes, their length, the counter and `recv_time`. + +The policy, against `next_counter = last_delivered + 1`: + +| arrival | action | added latency | +| --- | --- | --- | +| `== next_counter` | decode and deliver now, then drain held entries that became contiguous | **none** | +| `> next_counter` | copy into the ring; set the gap deadline to `now + 20 ms` if not already set | the newer packet waits | +| `< next_counter` | the gap is already committed — decode and deliver, flagged **late** | n/a | +| deadline fires | give up on the missing counters, advance `next_counter` to the lowest held entry, drain in order | capped at 20 ms | + +The last row is where `stats.stale_packets++; return 0` at `:506-509` is replaced: a late packet +is delivered flagged instead of discarded, which is the single change that makes backfill possible +at all. + +The deadline is served by a third `clamp_deadline()` call on the `select()` budget +(`udps_client.c:1145-1152`). Without it, a held packet on a quiet stream would wait for the next +arrival — 100 ms at 10 pkt/s — and blow the budget. + +Two bounds on the ring, because 20 ms is a different number of packets at every rate: a fixed +entry count and a fixed byte budget. Overflow commits the gap early rather than growing, and +increments its own counter, so an undersized window is visible rather than silent. + +The window itself is `udps_config_t::reorder_window_s`, defaulting to `0.020`, along +`recv_buffer_bytes` and `silence_timeout_s` (`udps_client.c:982`). Zero disables resequencing and +restores today's discard-on-late behaviour exactly, which is what makes the change bisectable +against a reported regression. The policy value is udpscope's; the mechanism is the client's. + +### In `Acquisition`: reservation + +On a frame with `lost > 0`, before pushing the frame's own samples, run `lost × batch` NaN samples +through the **existing** per-sample loop at `src/acquisition.cpp:553`. + +That reuse is the design. `tick_` advances, so `tick % g == g - 1` keeps firing on the same phase +and the decimation grid does not shift; the group mean still lands on the mean stamp; and +`MinMaxPyramid::append` sees NaN, which its existing non-finite contract already handles as +*advances the positional count, excluded from every aggregate*. Nothing bespoke is needed for +decimation, stamping or the pyramid. + +Stamps across the hole come from the active time source — exact in relative mode, the declared +cadence continued from the last good stamp in stream mode, which is the bridge at `:274-292`. + +Math channels take NaN and give NaN, so a math trace keeps its gap. The trigger engine's +comparisons are false on NaN, so it cannot latch inside a hole; this is asserted by the probe +rather than assumed. + +**Reservation is capped at 256 missing packets**, reusing `kGlitchPackets` and its existing +reasoning verbatim — *256 packet spans in one go is an outage, not loss*. Past the cap there is no +reservation: log once and continue as today. That cap is also what stops a ten-minute outage from +pushing billions of NaN samples through the ring and wiping all history to make room for a gap. + +## Backfill + +`plot::data::RingBuffer` and `MinMaxPyramid` are both strictly append-only today. Backfill needs +an amend in each, and the two belong on different threads. + +`plot_host.hpp:183` calls `acquire(ScopeModel&)` from the frame path, so the **pyramid is mutated +only on the GTK main thread**, once per frame: the producer pushes into the ring and the main +thread folds what was pushed into the pyramid. That split carries straight over to the amend. + +- **Ring amend — producer thread.** `RingBuffer::amend(absolute, v)`, bounds-checked against + `[produced_ - capacity, produced_)`, a single relaxed store. The same race class as a push, and + an `f32` cannot tear. Measurements and export read the ring directly, so they see a repair + immediately. +- **Pyramid amend — main thread, inside `acquire()`.** The producer enqueues `(slot, value)` on an + SPSC queue; `ChannelBuffer::acquire()` drains it **after** its append pass, so every amended + index is already published. + +The thread split is not a stylistic choice. A bucket is 32 bytes and is not atomic, so amending it +from the producer would let a reader see a half-updated bucket whose `sum` does not match its +`count` — a wrong mean from a structure whose readers trust `count == 0` to mean *absent*. On the +main thread that cannot happen and no seqlock is needed. + +`MinMaxPyramid::amend(index, v)` folds `v` into every covering bucket: at `count == 0`, +`min = max = v`, `sum = v`, `sumsq = v²`, `count = 1`; otherwise widen `min`/`max` and accumulate +`sum`/`sumsq`, `++count`. Its documented precondition is **non-finite → finite only**, which is +what makes it purely additive and O(log n) with no rebuild. An index already dropped by +`drop_before` is a no-op. + +### The time ring is never amended + +The hole was pre-stamped at reservation, so a backfill changes values only. `neighbours()` keeps +the monotonic array its binary search requires and `window_x0_` stays valid. This is the reason +reservation pre-stamps rather than leaving the ring short. + +### Addressing a late packet + +Reservation records `(counter_first, counter_last, tick_start, slot_start, slot_count)` in a +bounded side table. A late frame's counter indexes straight into it: + +``` +tick = tick_start + (counter - counter_first) * batch +``` + +Only slots lying **wholly** inside the arrived packet's tick range are amended. A slot straddling +a hole edge went NaN when its accumulator took a NaN, and its real members were never retained, so +it stays NaN. A displayed hole is therefore at most one slot wider per edge than the true loss — +invisible at any hole size worth repairing, and it makes a boundary slot need no special case. + +The side table is bounded by the 256-packet reservation cap and a maximum entry count; it is also +the source the shaded band reads. + +## Surfacing + +`StreamStats` gains `late_resequenced`, `late_committed` (gave up past 20 ms, or overflowed the +reorder ring) and `late_backfilled`, and finally exposes `stale_packets` — which the client has +counted since the beginning and which `src/status_text.cpp:70` has never shown, so an inversion +today has to be inferred from `packets` and `frames` diverging. All four sit in the status bar's +second tier. + +The band is a new `Plot::set_invalid_spans()` in `deps/plotter`, drawn as a tinted quad behind the +trace. Unlike the `t0` caption this *is* data-anchored — it must pan and zoom with the data it +marks — so it is annotation-class, not chrome. The invariant it must respect is that it reserves +no layout space and cannot be picked, so a hole cannot move the layout the labels were measured +into. + +## Verification + +udpscope has no unit test suite; the only automated check is the application's own headless +self-check (`CLAUDE.md`). This adds exit-dump lines and a `UDPSCOPE_PROBE_RESEQ_MS` probe, +asserting: a reorder inside the window is resequenced with no hole; a reorder past the window +leaves a hole and then backfills it; the trigger does not latch inside a hole; a 300-packet burst +hits the cap and does not throw the stream onto the arrival clock; and relative mode's stamps are +monotone with the declared spacing. + +Runs are serial and short, against the two standing baselines: four build warnings on a clean +build (two on an incremental build) and zero `Gtk-CRITICAL`. Batching several `xvfb` runs back to +back starves the mock past its 2 s `silence_timeout_s` and reads as a reconnect, which is a +measurement artefact and not a defect (`docs/claude/verification.md`). + +**One gap in the mock has to be closed first.** `--reorder-every` releases the held packet +immediately after the one that overtook it, so it is ~0 ms late and would always land inside the +20 ms window. That exercises resequencing but leaves the **backfill path untestable**. The mock +needs `--reorder-delay-ms` so a packet can be released deliberately past the window. + +## Documentation this lands against + +- `docs/claude/invariants-data-path.md` — append-only stops being the whole truth; the amend's + precondition and its thread belong here. +- `docs/claude/invariants-plot-and-layout.md` — the band, and the layout space it must not take. +- `docs/claude/invariants-trigger-and-pinning.md` — the trigger does not latch inside a hole. +- `docs/claude/verification.md` — the new probe and dump lines, and `--reorder-delay-ms`. +- `docs/claude/build-and-run.md` — the vendored `udpsclient` diff: the reorder ring, the late + delivery replacing the discard, and the third `clamp_deadline()`. +- `README.md` and a `SPECS.md` deviation note for relative-time mode. + +## Deliberately not in scope + +- **Replaying math channels or the trigger over a backfilled region.** Decided against above. +- **Interpolating across an unrepaired hole.** A hole is reported, not invented. There is no + interpolation anywhere in the data path today and this does not add the first. +- **Making relative mode the default.** It is a toggle. The default stays the declared time + signal, so no existing session or run changes what it shows. +- **The `(min, max, mean, rms)` gather mode** that would fix the separate solid-band degeneration + at wide time windows. Related to the pyramid, unrelated to this; still open.