From 4e29b958a21f009da06864805af630eb9884c346 Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Wed, 14 Oct 2026 15:10:49 +0200 Subject: [PATCH] fix: going live restores immediately instead of after a few trigger toggles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The symptom was "view a recorded event, click ``, then toggle the trigger a few times before it starts streaming again". The mechanism was the restore's own gate: `restore_live_state()` waits for the *stream's* channel list, because the trigger's stored channel is a **name** and a file view that renamed its channels (`square (A)`) must be gone before the name means the right signal — and that wait was gated on `generation()` having moved since *Go live* was pressed. That gate could never be satisfied in the common case. A **single** event keeps the stream's own channel names, and the restarted client's CONFIG is identical — which the model treats as *no change on purpose* (`same_stream_signals()`: "the honest recovery is nothing") — so nothing bumped the generation and the restore sat there. Toggling the trigger *is* a structural change and moved it, which is exactly the workaround the report describes. The gate is now the question it was actually asking: is the snapshot's channel name resolvable against the live channels (or did the snapshot name none)? For a single event that is true as soon as the client has a channel list at all, so going live is immediate; for a group the names are renamed, so it waits for the stream's own CONFIG exactly as before — which is what the gate was for. `live_restore_generation` is still captured and printed (`gen=a/b`) as information, not as the gate. Verified against the documented reading, both ways into a file view, from a session with a window and a trigger so the paired fields have something to disagree with: [events] live restored (after replay): window=0.500000 (want 0.500000) trigger=1 (want 1) channel='square' (want 'square') level=0 (want 0) enabled=1 armed=1 pinned=0 held=0 mode=2 pending=0 gen=6/3 [events] live restored (after a group): window=0.500000 (want 0.500000) trigger=1 (want 1) channel='square' (want 'square') level=0 (want 0) enabled=1 armed=1 pinned=0 held=0 mode=2 pending=0 gen=16/12 Every want satisfied, `pending=0` in both, `make test-ui` 14 files, zero `Gtk-CRITICAL`. Docs: the invariants' bullet rewritten around what the restore waits for and why the generation was the wrong question, and `probe-readings.md` gaining the thing that cost me two runs to find — those four paired fields are the probe's capture of the live view, so a bare first run reads the application's defaults and is not a defect; reproduce the documented line from a session that has a window and a trigger. --- docs/claude/invariants-trigger-and-pinning.md | 19 ++++++++++------ docs/claude/probe-readings.md | 21 +++++++++++++----- src/ui.cpp | 22 +++++++++++++++---- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/docs/claude/invariants-trigger-and-pinning.md b/docs/claude/invariants-trigger-and-pinning.md index 25ec0ae..f43d389 100644 --- a/docs/claude/invariants-trigger-and-pinning.md +++ b/docs/claude/invariants-trigger-and-pinning.md @@ -224,13 +224,18 @@ is the complaint this answers. - `rebuild_trigger_signals()`, finding a `channel` of `-1` after the rebuild renumbered the channels, silently settles on the first signal *and* re-defaults the level -- which is why `trigger_level_touched` is restored with the configuration. -- **It waits for the stream's own channel list, gated on `generation()` having moved.** The restored - name has to be resolved against the live channels, and those do not exist until the first CONFIG of - the restarted client lands on the producer thread. `App::live_restore_generation` is the generation - read when *Go live* was pressed; `live_restore_pending` stays set until it differs **and** - `channels()` is non-empty. A `pending=1` that never clears is therefore a reading about the - *stream*, not about the restore -- see `invariants-session-and-export.md` for the save that made it - stick at `channels=0`. +- **It waits for the trigger's channel *name* to resolve again, not for `generation()` to move.** The + restored name has to be resolved against the live channels, and a file view that renamed its channels + (`square (A)`) must be gone first or the name selects the wrong signal. `App::live_restore_pending` + stays set until `channels()` is non-empty **and** a channel carries the snapshot's name (or the + snapshot named none). Gating that on the generation was wrong in the common case: a *single* event + keeps the stream's own channel names, and the restarted client's CONFIG is identical, which the model + treats as **no change on purpose** (`same_stream_signals()`) -- so the generation never moved and the + restore waited for something that could not happen. What shook it loose was toggling the trigger, + which *is* a structural change: "click live, then toggle the trigger a couple of times" was the + symptom. A `pending=1` that never clears is a reading about the *stream* (no channels yet), not about + the restore -- see `invariants-session-and-export.md` for the save that made it stick at + `channels=0`. - **Only the two controls a file view actually moved are restated** -- the signal dropdown and the level spin. The rest of the trigger panel was never touched, and writing to a control fires its handler, so the ones with nothing to put back are left alone. Both writes are inside diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 50460a6..ce037e8 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1079,11 +1079,22 @@ the event is opened precisely so the pair can disagree — with the event's own nothing to tell "restored" from "untouched". Both lines are read a *later* step, not in the handler that pressed *Live*: the restore waits for the -restarted client's first CONFIG, so it is performed by a tick ~900 ms on. `pending=0` is what says it -happened; `gen=a/b` is the generation now against the one *Go live* read. A **stuck `pending=1`** is -not a reading about the restore but about the stream behind it — the generation never moved, or the -channel list came back empty, which is how a session file poisoned by a save taken during a group -produced `pending=1 channels=0` and a live view with nothing on it. +restarted client's channel list being usable again — the restored trigger's channel *name* has to +resolve — so it is performed by a tick ~900 ms on. `pending=0` is what says it happened; `gen=a/b` is the +generation now against the one *Go live* read, and it is informational: the restore no longer waits for it +to move, because a restarted client whose CONFIG is identical is deliberately *no change*, so a single +event — which keeps the stream's own channel names — never moved it and the restore waited until the +trigger was toggled. A **stuck `pending=1`** is not a reading about the restore but about the stream +behind it: the channel list came back empty, which is how a session file poisoned by a save taken during a +group produced `pending=1 channels=0` and a live view with nothing on it. + +**Run this one against a *configured* live view.** The four paired fields are the probe's own capture of +what the live view held when the file view opened over it, so a bare first run legitimately reads +`window=2.000000 (want 0.000000) trigger=1 (want 0) channel='' (want '')` — the wants are the empty +snapshot and the values are the application's defaults, which is not a defect. The documented line above +is reproduced from a session that has a window and a trigger (`window_s = 0.5`, `trigger_enabled = true`, +`trigger_signal = "square"`); the probe sets its own window to 0.5 s before opening the event, and a +session that then disagrees with it takes the pair apart. | field | value it must read | the defect a wrong reading names | | --- | --- | --- | diff --git a/src/ui.cpp b/src/ui.cpp index eb9a06b..3d38954 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -3369,10 +3369,24 @@ gboolean tick(gpointer user) { // window the session file holds, so a restore done before it would be overwritten by the saved // value. The generation is the other half of the wait -- the stream's first CONFIG, which is what // gives the stored trigger channel a list to be found in. - if (app->live_restore_pending && !app->replaying && !model.channels().empty() && - model.generation() != app->live_restore_generation) { - app->live_restore_pending = false; - restore_live_state(*app); + // The wait is for the stream's own channel list, and the question it is really asking is whether the + // trigger's channel *name* can be resolved again: a file view that renamed its channels (`square (A)`) + // has to be gone before the name means the signal it meant. Asking that directly, rather than watching + // `generation()`, is what makes going live immediate in the common case -- a single event keeps the + // stream's own channel names, so its channel list is already the right one, and the generation of a + // restarted client whose CONFIG is identical is *deliberately* unchanged (`same_stream_signals()`), so + // the restore used to wait for something that was never going to happen. What it took to shake loose + // was the trigger toggle, which is a structural change and moved the generation -- the "toggle it a few + // times" this fixes. + if (app->live_restore_pending && !app->replaying && !model.channels().empty()) { + bool resolvable = app->live_state.trigger_channel.empty(); + for (const ChannelInfo& c : model.channels()) { + if (c.name == app->live_state.trigger_channel) resolvable = true; + } + if (resolvable) { + app->live_restore_pending = false; + restore_live_state(*app); + } } // What the display shows, decided once for the whole frame: the producer can open or