diff --git a/DRAG-PERF-ANALYSIS.md b/DRAG-PERF-ANALYSIS.md new file mode 100644 index 0000000..e365e37 --- /dev/null +++ b/DRAG-PERF-ANALYSIS.md @@ -0,0 +1,94 @@ +# Drag/Drop Smoothness Analysis + +Where the drag experience actually spends its time, which pre-checks earn their place on the hot path and which don't, and where optimistic allowances are worth making. Written 2026-08-06 from a full read of the pipeline (`BoardDrops.swift`, `DragSession.swift`, `DragAutoScroller.swift`, `DropSlotMath.swift`, the commit path in `BoardStore.swift`, the watcher in `FolderWatcher.swift`) against the model in DRAG-REORDER.md and the measurements in RENDER-INSTRUMENTATION.md. This is an analysis document: findings are prioritized and sequenced but nothing here is implemented yet. + +The headline, stated up front: **the mid-drag hot path is already in good shape, and the felt latency lives almost entirely in the release.** The per-sample arithmetic is microseconds on any realistic board; the pause between mouse-up and the card being real is a 200ms debounce plus a whole-strip render pass, and one honesty bug can stretch a *failed* drop into a 1.5-second freeze. The biggest wins are not in doing less checking mid-drag — they are in making the echo arrive fast, making the landing frame cheap, and making the handoff structurally incapable of blinking. + +## Anatomy of a drag, as the profiler sees it + +**Pickup** (once per gesture): the handle's `.onDrag` closure guards `!isReadOnly, !isEditingInline`, resolves the dragged run's lanes with one O(board) scan, JSON-encodes the payload, and calls `DragSession.begin` — which freezes `cardHeights`/`laneUnits` (the only frozen geometry), clears the `RestingLayoutCache`, and arms the watchdog and the modifier-flip monitor. All synchronous, all trivial. The expensive thing that *also* happens at pickup is not the drag's fault: click-select precedes the drag, and a selection change re-runs **every card body on the board** (RENDER-INSTRUMENTATION.md — "Selection is O(board) in card bodies"). + +**Per sample** (every `dropUpdated`, and again on every autoscroll frame that actually scrolled): `revalidateProposal()` — an O(lanes) `.contains(where:)` — then the shared retarget: cursor conversion, a `RestingLayoutCache` lookup (hit = a struct-equality key check), `DropSlotMath` zone arithmetic over one masonry column, and a `propose()` that early-outs when the slot didn't change. The strip-side retargets (`retargetLanes`, `retargetCardsFromStrip`) allocate a filtered lane array and a display-units array per sample — the one piece of per-sample work the cache does not cover. On a ≤12-lane board this whole chain is single-digit microseconds; at 120Hz it is not where frames go. + +**Release**: `commitDrop()` re-runs the guard ladder once (revalidate, this-board check, survivors, mixed-kind, the `TrashDrop.accepts` re-ask), dispatches to the store writer — a synchronous `performWrite` whose disk work is one `order`-field rewrite or one folder rename — and arms the **committed-overlay hold**: the board keeps drawing the proposed arrangement until the echo snapshot lands or `CommittedHold.timeout` (1.5s) gives up. + +**Echo**: `performWrite`'s bracket closes → `FolderWatcher.endBracket()` schedules the mandatory post-bracket reload → **200ms trailing debounce** → off-main tree walk (cheap; `ParseMemo` re-parses only the touched files) → main-actor `land()` with the animated snapshot assignment → `BoardView`'s `snapshotGeneration` watch calls `DragSession.handOff`, dissolving the hold. + +So the release pause = write (~1–10ms) + **200ms debounce (dominant)** + walk (a few ms warm) + apply/render. The `drop-release-pause` signpost measures exactly this span, and its healthy outcome today is bounded below by the debounce. + +## Findings, prioritized + +### P1 — Expedite the app-mediated echo (~180ms off every drop) + +The 200ms trailing debounce exists to coalesce foreign FSEvents bursts. But a drop commit is not a burst the watcher has to wait out — the store *knows* it just wrote, the bracket already owes exactly one delivery, and the user is staring at the gap. Add a surgical `FolderWatcher.expedite()`: if `bracketDepth == 0 && pendingOrigin != nil`, cancel the armed debounce and fire the owed delivery now. Expose it through the store and call it from `commitDrop()` right after the writer returns. + +This is deliberately **not** a shorter global debounce — `endBracket` serves every `performWrite` (card saves, inline edits, heals), and dropping the settled 200ms figure everywhere would turn write bursts into per-write walks. It is also **not** a violation of one-way flow: nothing mutates the snapshot from the write path; the delivery the bracket already owed just fires earlier, and the store still learns the new order by walking disk. + +Races, analyzed: our own FSEvents still in kernel flight arrive after the fast reload and schedule a `.foreign` delivery → one redundant memoized walk → value-equal → assignment skipped, `snapshotGeneration` unmoved, nothing visible. A foreign write landing between our write and the fast reload folds under the `.appMediated` label — the same accepted blur `WatchOrigin.merged` already documents. POSIX guarantees the walk sees the completed writes, because `performWrite`'s FileManager work returned before `expedite` was called. All benign; the cost is one extra no-op walk per drop. + +Expected result: `drop-release-pause` outcome `echo` at ~10–50ms instead of ~250–400ms. This also all but closes the rapid-successive-drag window (see P5). Tests: FolderWatcherTests additions (expedite fires the owed delivery once, respects open brackets, no-ops with nothing pending). + +### P2 — Honest failure path: don't arm a hold for a write that didn't happen + +`commitDrop()` arms `session.commit(into: store)` unconditionally (`BoardDrops.swift:978`), but the drop-path store methods return `Void` and swallow failure via `try? performWrite`. A *failed* write therefore posts its banner immediately — and then leaves the dropped arrangement frozen on screen for the full 1.5s `CommittedHold.timeout` before animating back. By this repo's own definition (`drop-release-pause` outcome `timeout` is "a bug, not a slow frame") and 03-board-ui.md § Motion's promise ("a failed write discards the proposal and the board animates back"), this is a bug, not a design. + +The fix is plumbing a fact the writers already compute: make `moveCards`, `copyCards`, `moveLanes`, `restoreLanes`, `receiveCards`, `receiveLanes`, `deleteByDrag`, `deleteLanesByDrag` return `@discardableResult Bool`; arm the hold only on `true`, else `cancelDrop()` — the immediate animated snap-back, banner already posted. A refused no-op arrangement also stops arming a pointless 1.5s hold, which de-noises the signpost and makes `CommittedHold`'s own doc comment ("every drop path refuses a no-op arrangement before it opens a write bracket") true end-to-end. + +### P3 — The headerInk hoist: make the landing frame cheap + +Every echo reload — the one *inside* the release window — currently re-runs every lane body on the board, because `LaneView.body → headerInk` reads `store.snapshot.background` and Observation tracks whole properties (RENDER-INSTRUMENTATION.md — "the lane gate is never asked on a snapshot change"). The fix is already designed there: resolve the ink once in `BoardView` and pass it down as a compared parameter, the way `slotWidth` and `columns` already are. The tripwire is armed: `theLaneCostFollowsTheBoard` fails in the good direction when this lands, and the container budget in `aOneCardEditIsNotAWholeBoardRebuild` drops from `laneCount` to the pathfinder's 4. + +This matters more once P1 lands: with the debounce gone, apply/render becomes the dominant share of the pause, and this is the cheapest way to shrink it. It also smooths the *other* moment the drag model cares about — a foreign reload landing mid-flight, where a whole-strip body storm currently rides the re-grounding. + +### P4 — Shadow identity handoff: the echo becomes a content swap + +Today the hold renders `DragShadow`s keyed `"shadow:\(index)"`, and the echo swaps a shadow-identity ForEach element for a card-identity element — an insert/remove, exactly the shape the create placeholder deliberately avoids ("a committed placeholder is already keyed by the arriving card's identity… the ForEach element is neither inserted nor removed — only its content changes", `LaneView.swift` at the placeholder). While the session is settled (`hold != nil`), key each held shadow slot by its member's `ItemID` — the session knows `members` in flatten order. The handoff then swaps content in place, structurally incapable of running the appear transition or a one-frame blink, under Reduce Motion or not. + +With P1 this largely answers 03-board-ui.md § Motion's reopened release-presentation question with the cheapest possible answer: **nothing moves at all** — the card face materializes in its slot ~30–60ms after mouse-up, under AppKit's own drag-image fade. (Shortening that fade is not reachable: SwiftUI's `.onDrag` never exposes the `NSDraggingSession`.) A brief landed-highlight pulse in the selection-wash vocabulary remains available as optional polish, but prototype it only after P1+P4 — the fast echo may make any additional presentation unnecessary. + +### P5 — Rapid successive drags: dissolved by P1, document only + +Drag #2's `begin()` clears drag #1's hold, and until the echo lands the board regresses to the stale arrangement — the just-dropped card visibly snaps back, then jumps forward mid-drag when the echo re-grounds the zones. Today that window is ~250–400ms and reachable by a fast user; after P1 it is ~30–60ms and effectively unreachable. `handOff` itself is race-free (all main-actor, root+generation-guarded hold, `expire` self-checks, `begin` cancels the timeout). No mechanism needed; note the residual micro-window in DRAG-REORDER.md when P1 lands. + +### M1 — Selection-gated card bodies: the pickup frame + +`CardFaceView.body` reads `store.selection`, so selecting one card re-runs all 180 faces — and pickup *is* a selection change, so this O(board) body storm lands on the exact frame the pickup lift starts. The fix is the design change RENDER-INSTRUMENTATION.md already names: each face takes its own selected-ness (and the selection count, for the badge) as compared parameters through `CardFaceView.==`, making a selection change cost only the faces whose state flipped. Preserve the counter-invariant (`selectionStillRepaints`: a selected card must still repaint) and update `ViewEquatableTests`' comparison list. This is the likeliest source of a *pickup* hitch on large boards — the O(board) payload scan is not (see the rejected list). + +### M2 — File-drop importable-count cache + +The Finder-drop path runs `FinderDrop.importableCount` **twice per sample** (`acceptsFileDrop`, then the shadow count), each doing UTType-database conformance checks per provider — the only genuinely non-trivial per-sample system call in any drag mode. Cache the count on `DragSession` beside `fileTarget`, computed on the first sample of a hover, cleared with the file target and the file watchdog, keyed defensively on provider count. The commit is unaffected (it counts resolved URLs, not providers). + +### M3 — Strip-side caches (allocation hygiene, lowest yield) + +`retargetLanes` and `retargetCardsFromStrip` allocate a filtered lane array and a display-units array per sample, and `revalidateProposal` scans lanes per callback. A sibling cache to `RestingLayoutCache` — same keying discipline, same session lifecycle — holding the display-units array, its hidden-filtered variant, and a lane-ID `Set` (making revalidation O(1)) removes all of it. Honest sizing: at ≤12 lanes this is microseconds; the value is allocation pressure and pattern consistency, not visible frames. Do it last, or not at all if profiling says done. The reactive alternative — invalidating the proposal from the `snapshotGeneration` watch instead of polling per sample — is not recommended: it trades a provably-cheap check for an ordering dependency, and the re-grounding contract ("at the top of every callback and again at release") is pinned in prose and behavior. + +### Wishlist — pre-flush the git seam at pickup + +`GitAutoCommitter.noteWillWrite()` is rare (only when the window holds an uncommitted foreign change) but is the single worst possible release stall when it fires: a synchronous stage/commit plus HEAD materialize plus loader walks, all before the drop's write. The optimistic allowance: when a drag **begins** on a board whose window `holdsForeignChanges`, kick the flush then — mid-drag reloads are already a designed-for scenario (the re-grounding trio), and the release then finds nothing to flush. Medium complexity; keep as a wishlist note until the stall is ever observed in a trace. + +## Rejected, with reasons + +These were analyzed and turned down; recorded so they aren't re-litigated. + +- **Full optimistic snapshot mutation at commit.** The `CommittedHold` already renders the proposed arrangement — members lifted, siblings reflowed, slot held — so the only perceptual delta versus mutating the snapshot is card-face-vs-shadow at the slot, which P1+P4 close for ~50ms of exposure. Breaking the one-way-flow invariant would buy that sliver at the price of reconcile-on-echo logic, snapshot rollback on write failure (the current failure story is trivially honest *because* the snapshot never lies), and re-deriving `EchoLedger`/`BoardDiff` semantics. The invariant stays. +- **Incremental/targeted reload.** `ParseMemo` already makes the echo walk parse ~2 files; what remains is directory enumeration, deliberately never memoized because attachments and loose files don't touch `index.md`. A touched-lanes re-parse would fork "the snapshot is rebuilt purely from disk" into two code paths to save single-digit milliseconds. P1 removes 200ms; this would remove ~5. +- **Async off-main `BoardWriter`.** Same-volume APFS renames and one-file order rewrites are sub-millisecond metadata ops; async buys nothing and costs the failure-after-hold problem — a banner about a drop the user watched succeed. If the one unbounded case (cross-board copy of a lane with heavy attachment trees) ever shows in a trace, handle that operation with a progress affordance, not the drop architecture. +- **Collapsing the commit-time guard ladder.** It runs once per release — not on any frame path — and the `TrashDrop.accepts` re-ask exists because ⌥ can change after the proposal stood with no callback reporting it; removing it converts a promised copy into a delete. `ModifierFlipTests` pins this. Zero smoothness gain, real correctness risk. +- **Dropping the commit no-op guard.** The `DropSlotMath.applied` recompute is once per release, ~30 comparisons, and is what keeps a drag that ends where it started from stamping `modified` and minting a git commit — a disk/history invariant, not a UI one. +- **Indexing the pickup payload scan.** ≤360 iterations once per gesture, microseconds. A cardID→laneID index invalidated per snapshot buys nothing measurable; the pickup hitch, if felt, is M1's selection storm plus replica rendering. +- **Caching `TrashDrop.accepts` per hover.** Six boolean clauses, no allocation, deliberately uncached for modifier freshness. Already free. +- **Allocation-free `DropSlotMath` / caching `MasonryPlacement.frames`.** The frames depend on `placement.origin`, which moves with every autoscroll step — the registry's live placement is deliberately uncached ("derived at event time", four stores and a divide). Caching origin-relative frames and translating per sample is the same O(n) with more machinery, churning the most heavily tested pure math in the app. +- **Gating the autoscroll retarget.** Already correct: `step()` returns before `didScroll` unless the clamped target moved > 0.01pt; frames where nothing scrolls cost one cursor conversion and an engagement test. + +## Validation + +No new instrumentation is needed; the existing instruments were built for exactly these claims. + +- **P1/P2**: the `drop-release-pause` signpost — healthy drops move from ~250–400ms `echo` to ~10–50ms; failed drops stop producing `timeout` outcomes at all. +- **P3**: `theLaneCostFollowsTheBoard` fails in the good direction and gets rewritten to pin the fixed behavior; the container budget in `aOneCardEditIsNotAWholeBoardRebuild` tightens from `laneCount` to 4. +- **M1**: `selectionStillRepaints` keeps its non-zero floor; a new budget pins selection cost to the flipped faces. +- **M2/M3**: `RestingLayoutCacheTests`-style build/reuse counters on the new caches; `DropSlotMathTests`, `DragAutoScrollMathTests`, `ModifierFlipTests`, `DragWriteTests` all continue to pin the behavior none of this may change. + +## Sequencing + +P2 first (small, fixes a real bug, de-noises the signpost) → P1 (the latency win; validate with the signpost) → P3 (rides the same window; flips the tripwire) → P4 (handoff identity) — then reassess the open release-presentation question with the fast echo in hand before designing any pulse. M1 next if pickup hitches are felt on large boards; M2 with any file-drop work; M3 only if profiling still shows the strip allocations after everything above.