From 3dff79a0ec4e2c6d8766b3b7432891923acc9978 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 14 Aug 2026 17:50:54 +0200 Subject: [PATCH] fix(panel): D2 was broken on the real sequence, and Q#BP-R3 is ruled Two of review's findings: the framing blocker, and a live defect in the commit before this one. The remaining six are queued. **I REPORTED M-D2 AS BITING AND IT DID NOT.** `presented()` filters on `frame.geometry_epoch == self.panel.geometry_epoch`, and a geometry change advances that field FIRST --- so by the time the matching frame arrives, `presented()` already answers `None` and my `is_some_and` predicate skipped the reset entirely. The shipped D2 did nothing on the production sequence. The witness could not see it because it invented a higher-epoch frame without driving `next_geometry_declaration`, leaving `self.panel.geometry_epoch` untouched so `presented()` still matched. A test that skips the step which breaks the code cannot fail on it. The predicate now compares against the RETAINED frame (`self.panel.frame`), which survives the epoch advance, and the witness drives `GeometryTrigger::Metrics` for real --- asserting along the way that `presented()` IS `None` in that window, so the trap is pinned rather than merely avoided. Restoring the `presented()` predicate now fails the row. **M-D3 WAS ALSO UNCONSTRAINED**, for a smaller reason: arming clears `last_pointer_cell`, and the leg only armed, so the field was already `None` before the replacement and deleting its reset changed nothing. The arm helper now seeds the baseline with one accepted motion --- what a real gesture would have produced --- and after replacement the row requires `panel_motion_is_new` at that same cell to return true. Deleting only that line now fails. **Q#BP-R3 IS RULED: current-state hit semantics, narrowly, with the token named as follow-up.** `PanelPointer` carries epochs and a cell but nothing identifying the frame CONTENT the user saw, and `panel_epoch` is stable across ordinary frames by design. So a document wheel moves `view_top` daemon-side, and a click sent before the new frame lands is inverted through the NEW `view_top` --- selecting a row the user never saw, with every validation passing. Closing it properly needs a per-frame token on `PanelFrame` echoed by `PanelPointer`: a WIRE CHANGE, and this lane is non-protocol-bearing with 1b blocked behind it. A daemon-only mitigation was considered and does not work --- inverting against the last EMITTED frame still cannot tell which frame the user SAW, and the failing window is identical. So the lane accepts current-state semantics and says so: the window is narrow and self-inflicted (the same frontend must move the view and then click within one round trip), the magnitude is bounded by `SCROLL_LINES`, and the TUI is structurally unaffected. The token is recorded as a named follow-up for the next protocol-bearing slice, so it is inherited rather than rediscovered. Overrule stated explicitly: the trade is a narrow same-frontend mis-hit now, against serializing this lane and 1b behind a v25 wire change. Gates: all nine green under `env -u TMPDIR`, log 20260814T154611Z. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/bottom-panel-framing.md | 62 ++++++++++++++++++++++++++++++++++++ pmacs-gpu/src/main.rs | 62 +++++++++++++++++++++++++++++------- 2 files changed, 112 insertions(+), 12 deletions(-) diff --git a/docs/bottom-panel-framing.md b/docs/bottom-panel-framing.md index 48e04b4..e45bdff 100644 --- a/docs/bottom-panel-framing.md +++ b/docs/bottom-panel-framing.md @@ -2031,6 +2031,68 @@ follows acceptance 48's wording — it names row *selection* — and keeps document navigation from becoming an incidental consequence of wiring replay. +#### Q#BP-R3 — a panel cell has no frame-content provenance **RULED: current-state hit semantics, narrowly, with the token named as follow-up** + +**The hole.** `PanelPointer` carries `geometry_epoch`, `panel_epoch`, +`buffer_id` and a `coord` — **and nothing identifying the frame CONTENT +the user was looking at** (`pmacs-protocol/src/message.rs:500`). +`panel_epoch` is deliberately *"stable across ordinary frames of one +continuously present window/buffer"* (`panel.rs:61`), so an ordinary +repaint changes no epoch at all. + +**The race that follows is real.** A document-panel wheel moves +`view_top` **daemon-side**. The daemon repaints and emits a new frame. +Before that frame reaches the GPU, the user clicks. Every validation +passes — same buffer, same epochs — and the daemon inverts the cell +through its **current** `view_top`, selecting a row the user never saw. +Off by up to `SCROLL_LINES` (3). **No existing gate can reject it**, +because nothing about the event is stale by any test the ladder +applies. + +**Why the epochs cannot be stretched to cover it.** Moving +`panel_epoch` on content change would invalidate a gesture on every +repaint, which breaks drags outright — the field is stable *by design*, +and that design is what makes selection possible. + +**Why a fix is not free.** Closing it properly needs a **per-frame +token** on `PanelFrame`, echoed by `PanelPointer` — **a wire change**, +which makes it a protocol-bearing slice. This lane is explicitly +non-protocol-bearing, and GUI arc 1b is blocked behind it. + +**A daemon-only mitigation was considered and does not work.** Having +the daemon invert against the `view_top` it used for its **last emitted +frame** sounds like it removes the wire dependency, but it does not: +the daemon still cannot know **which** emitted frame the user saw, and +the failing window — frames emitted after the one on screen — is +exactly the same one. Without a token echoed back, the information does +not exist on the receiving side. + +**Ruling: this lane accepts CURRENT-STATE hit semantics**, and says so +rather than leaving it undiscovered: + +- A panel cell is resolved against the daemon's state **at the moment + the event is processed**, not against the frame the frontend painted. +- **The window is narrow and self-inflicted**: it requires the same + frontend to change the panel's view and then click inside one + round-trip. It cannot arise from another frontend's activity, because + a foreign edit that moves `view_top` is not a thing panels do. +- **The magnitude is bounded** by whatever moved the view — one wheel + step, `SCROLL_LINES` rows. +- **The TUI is unaffected.** It has no round trip; the hazard is + structural to a remote frontend inverting cells against mutable + daemon state. + +**Named follow-up, not a shrug: `PanelFrame` gains a content token and +`PanelPointer` echoes it, in the next protocol-bearing slice.** The +daemon then drops a gesture whose token no longer matches — the same +shape as the epoch ladder, one level finer. It is recorded here so the +next wire slice inherits it rather than rediscovering the race. + +**Overrule this if you would rather block the lane on a protocol +slice.** The trade is explicit: a narrow, bounded, same-frontend +mis-hit now, against serializing this lane and 1b behind a v25 wire +change. + ### The four replay edges — none of which "activation ordering" covers §5a's first draft concluded that the activation rule made the replay diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index b06a7b1..cbc7a7c 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -7085,7 +7085,14 @@ impl State { // Only on a CHANGE of identity. A panel repaints constantly // during a drag, and resetting on every frame would make // selection impossible. - let identity_changed = self.panel.presented().is_some_and(|current| { + // + // Compared against the RETAINED frame, never `presented()`: + // that accessor filters on `geometry_epoch == self.panel + // .geometry_epoch`, and a geometry change advances the field + // FIRST, so by the time the matching frame arrives + // `presented()` is already `None` and an `is_some_and` + // predicate skips the reset — exactly the case D2 covers. + let identity_changed = self.panel.frame.as_ref().is_some_and(|current| { current.panel_epoch != frame.panel_epoch || current.geometry_epoch != frame.geometry_epoch }); @@ -20105,9 +20112,20 @@ mod tests { }; let frame = present_panel(&mut state, 4); + let live = pmacs_protocol::CellCoord::new(1, 1); let arm = |state: &mut State| { state.set_panel_pointer_held(true); - state.panel.gesture_last_content_cell = Some(pmacs_protocol::CellCoord::new(1, 1)); + state.panel.gesture_last_content_cell = Some(live); + // SEED THE DEDUPE BASELINE TOO. Arming clears it, so a leg that + // only arms leaves `last_pointer_cell` already `None` and its + // reset is unconstrained — deleting that line stays green. One + // accepted motion is what a real gesture would have produced by + // the time a replacement arrives. + assert!( + state.panel_motion_is_new(live), + "the first motion after a press is never a duplicate" + ); + assert_eq!(state.panel.last_pointer_cell, Some(live)); }; // D4, the NEGATIVE leg, first: a CHANGED frame at the SAME identity @@ -20137,23 +20155,43 @@ mod tests { "a replacement panel never saw the press" ); assert_eq!(state.panel.gesture_last_content_cell, None); - assert_eq!( - state.panel.last_pointer_cell, None, - "and the dedupe baseline goes too, or the successor's first \ - same-cell motion is silently suppressed as a duplicate" + assert_eq!(state.panel.last_pointer_cell, None); + assert!( + state.panel_motion_is_new(live), + "the dedupe baseline goes too: the successor's first motion at \ + the predecessor's cell must reach the daemon, not be suppressed \ + as a duplicate of a gesture that belonged to another panel" ); // D2 — geometry identity, with panel identity UNCHANGED. This is the // font/scale case: the gesture would otherwise resume under a new // grid carrying epochs that are current and perfectly valid. - let base = state - .panel - .presented() - .cloned() - .expect("a frame is present"); + // + // **Driven through the real declaration path.** Inventing a + // higher-epoch frame is not the production sequence: a font or + // scale change advances `self.panel.geometry_epoch` FIRST, and only + // then does the matching frame arrive. That ordering is what broke + // the first implementation — `presented()` filters on the epoch, so + // it answers `None` in exactly this window — and a witness that + // skips the declaration cannot see it. + let base = state.panel.frame.clone().expect("a frame is retained"); arm(&mut state); + let (next_epoch, total) = state + .next_geometry_declaration(GeometryTrigger::Metrics) + .expect("a metrics change re-declares geometry"); + assert_ne!(next_epoch, base.geometry_epoch, "the declaration advanced"); + assert!( + state.panel.presented().is_none(), + "and the retained frame no longer answers `presented()` — the \ + window in which a `presented()`-based reset silently skips" + ); let mut regeometried = base.clone(); - regeometried.geometry_epoch = base.geometry_epoch + 1; + regeometried.geometry_epoch = next_epoch; + regeometried.size = CellSize::new(base.size.rows, total.cols.max(1)); + regeometried.cells = vec![ + pmacs_protocol::Cell::default(); + (regeometried.size.rows * regeometried.size.cols) as usize + ]; assert_eq!( regeometried.panel_epoch, base.panel_epoch, "the point of this leg is that PANEL identity holds"