From f6105d4389a3522cf87b2ddcbe10371e28a26faf Mon Sep 17 00:00:00 2001 From: rzen Date: Sat, 1 Aug 2026 19:14:11 -0400 Subject: [PATCH] =?UTF-8?q?The=20marquee=20registry=20stops=20being=20obse?= =?UTF-8?q?rvable=20=E2=80=94=20reflow=20writes=20cost=20a=20dictionary=20?= =?UTF-8?q?store=20and=20nothing=20else?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops @Observable from MarqueeTargetRegistry (MarqueeSession keeps it — its rect genuinely renders the band). The audit found no body read anywhere: the begin guard and sample loop read from inside the drag gesture, the arrows from inside a key handler, so nothing ever needed invalidating when a frame moved — while every make-room reflow had each sliding face re-firing onGeometryChange per display frame, each write paying Observation registrar bookkeeping on top of the reflow's own render work (the confirmed A/B culprit of 2026-07-31). Write-gating on drag-active was rejected: a suppressed write never replays, leaving the band and arrows navigating stale rectangles. A tripwire test pins the registry against anyone re-adding the macro. Drag-perf confirmed culprit, card eb7b75ce. Claude-Session: https://claude.ai/code/session_01CqjXB7ASoWtbyoGod68k97 --- Kanban/UI/Board/MarqueeSession.swift | 20 +++++++- KanbanTests/KeyboardGrammarTests.swift | 65 ++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/Kanban/UI/Board/MarqueeSession.swift b/Kanban/UI/Board/MarqueeSession.swift index 7823acc..135d365 100644 --- a/Kanban/UI/Board/MarqueeSession.swift +++ b/Kanban/UI/Board/MarqueeSession.swift @@ -93,8 +93,26 @@ final class MarqueeSession { /// It is also the **begin guard's** universe: a drag that starts inside a registered frame belongs /// to that item's own gesture (a card drag, a drag out of the trash), never to the band. Deciding /// that geometrically rather than by gesture priority is what keeps the two from fighting. +/// +/// **Deliberately not `@Observable`** — `LaneDropRegistry`'s rule, for the same reason and with the +/// same proof: nothing renders off it. Every reader asks at *event* time — the band's begin guard and +/// its sample loop from inside a drag gesture (`MarqueeControl.gesture(in:)`), the arrows from inside +/// a key handler (`BoardView.step`, `.extend`, over `NavigationMath`) — so no view body ever reads a +/// frame from here and none needs invalidating when one moves. +/// +/// What observing it cost is the whole reason the rule is stated: every card face registers here +/// through `onGeometryChange`, and a drag's make-room reflow *animates positions*, so for the ~0.18 s +/// of every proposal change each sliding face re-fires its observer once per display frame. Observed, +/// each of those writes was an `@Observable` mutation with registrar bookkeeping — dozens of cards at +/// the display's refresh rate, invalidating the strip on top of the reflow's own render work (an A/B +/// on 2026-07-31 confirmed it: removing the card-face registration alone made dragging visibly +/// smoother). Unobserved, a registration is a dictionary store and nothing else. +/// +/// The alternative — suppressing the writes while a drag is in flight — was rejected: a suppressed +/// write never replays if that card's geometry does not change again after the drag ends, which +/// leaves the band and the arrows navigating by stale rectangles. The frames must stay live; only +/// their observation had to go. @MainActor -@Observable final class MarqueeTargetRegistry { private(set) var targets: [ItemID: MarqueeTarget] = [:] diff --git a/KanbanTests/KeyboardGrammarTests.swift b/KanbanTests/KeyboardGrammarTests.swift index d1e3cf8..a5455d8 100644 --- a/KanbanTests/KeyboardGrammarTests.swift +++ b/KanbanTests/KeyboardGrammarTests.swift @@ -1,5 +1,6 @@ import CoreGraphics import Foundation +import Observation import Testing @testable import Kanban @@ -184,6 +185,70 @@ struct NavigationMathTests { } } +// MARK: - The registry the arrows and the band read + +/// The registry is the one piece of the marquee/arrow pair that is *written* from a view — every card +/// face keeps its drawn frame in it — and it is deliberately unobserved so those writes invalidate +/// nothing (`MarqueeTargetRegistry`, `LaneDropRegistry`'s rule). That is a property of the type rather +/// than of any call site, so it is pinned here: a drag's make-room reflow animates positions, and an +/// observed registry turns each sliding face into a stream of view invalidations at the display's +/// refresh rate for as long as the reflow runs. +@Suite("MarqueeTargetRegistry ▸ registering a frame invalidates nothing") +@MainActor +struct MarqueeTargetRegistryTests { + + /// A box the tracking callback can flip — `withObservationTracking`'s `onChange` is `@Sendable`, + /// and Observation calls it from wherever the mutation happened. + private final class Tripwire: @unchecked Sendable { + var fired = false + } + + @Test("Every reader the band and the arrows use registers no observation") + func writesAreNotObserved() { + let registry = MarqueeTargetRegistry() + registry.update(target(card1, x: 0, y: 0)) + let tripwire = Tripwire() + + withObservationTracking { + // Exactly the three reads the event-time callers make: the sample loop's `all` + // (`MarqueeControl.gesture`), the arrows' keyed lookup (`BoardView.step`/`.extend`), and + // the band's begin guard. + _ = registry.all + _ = registry.targets[card1] + _ = registry.contains(.zero) + } onChange: { + tripwire.fired = true + } + + // A reflow's worth of re-registration: the same card at a new frame, a new card, a removal. + registry.update(target(card1, x: 0, y: 40)) + registry.update(target(card2, x: 0, y: 200)) + registry.remove(card2) + + #expect(tripwire.fired == false, "MarqueeTargetRegistry must not be @Observable") + } + + @Test("The frames it hands back are still live after those writes") + func readsStayCorrect() { + let registry = MarqueeTargetRegistry() + registry.update(target(card1, x: 0, y: 0)) + registry.update(target(card2, x: 0, y: 120)) + + registry.update(target(card1, x: 0, y: 40)) + #expect(registry.targets[card1]?.frame.minY == 40) + #expect(registry.all.count == 2) + // Inside the moved card1 (y 40…140), and above where it now starts. + #expect(registry.contains(CGPoint(x: 10, y: 50))) + #expect(!registry.contains(CGPoint(x: 10, y: 20))) + // Inside card2 (y 120…220) and nothing else. + #expect(registry.contains(CGPoint(x: 10, y: 200))) + + registry.remove(card2) + #expect(registry.targets[card2] == nil) + #expect(!registry.contains(CGPoint(x: 10, y: 200))) + } +} + // MARK: - SortMath @Suite("SortMath ▸ within-lane sort")