Per hermes-sweeper review on PR #67005:
- Remove `broadcastSessionsChanged` import (unused)
- Remove `setSessionTodos` import (unused)
- Keep `clearActiveSessionTodos` and other active imports
No behavioral change, just dead code removal.
Cherry-pick of b4d5ba3e onto current main. Original commit's target file
apps/desktop/src/app/session/hooks/use-message-stream.ts has since been
moved into apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts.
The fix is unchanged: replace clearSessionSubagents() with
pruneFinishedSessionSubagents() at the message.start boundary so that
still-running subagent rows survive new turns.
Per @teknium1's review of #64038 on 2026-07-16: 'mergeable_state=dirty'
because the dispatcher moved. Resolved by retargeting the call.
Tests: 3 new cases in subagents.test.ts (unchanged from original PR).
Failing-first verified in original PR (3/3 fail on unfixed, 3/3 pass on fix).
- artifact card title/meta/open-hint now use --conversation-text-font-size /
--conversation-tool-font-size like CodeCard and the rest of the transcript,
instead of hardcoded rem literals
- drop no-op hover:border-border (border is border-border at rest)
- comment the deliberate raw bg-white + colorScheme:light on the sandboxed
iframe so it doesn't read as an untokenized literal
Substantial generated content (full HTML documents, large SVGs, long code)
now promotes out of the transcript into versioned, openable artifacts:
- lib/artifact-detect.ts: pure detection over fenced blocks — html docs,
large standalone svg, and 48+ line / 3k+ char code fences become
artifacts; prose/terminal/mermaid fences and small snippets never do.
Titles derive from <title>/<h1>, filename comments, or declarations.
- store/artifacts.ts: per-session registry with content-hash dedupe and
version history (same kind+title = one artifact the model iterates on),
persisted to localStorage with per-session/per-artifact/byte caps.
- ArtifactCard (transcript): compact openable card replaces the wall of
code; streaming shows shimmer + line count; versions accumulate
automatically on completion but the rail only opens on click
(offer, don't hijack). Reasoning scratchpads never register.
- ArtifactPane (right rail): artifact: tabs beside preview/file tabs with
version stepper (v2 of 3 / Latest), PREVIEW/SOURCE toggle, copy,
download (kind-aware extension), open-in-browser for HTML.
- Rendering: HTML runs in a sandbox=allow-scripts iframe (opaque origin,
no network to the app, no top-nav); SVG is DOMPurify-sanitized with the
same profile as the inline embed; source view reuses the windowed Shiki
renderer so 5k-line artifacts scroll smoothly.
- Rail integration: artifact tabs participate in tab order, close-others/
close-to-right, ⌘W, pane visibility, and reveal; gateway switches close
open artifact tabs; a persisted artifact: active-tab id reconciles to
preview on boot (artifact tabs are ephemeral).
- i18n: en/zh/zh-hant/ja/ar strings for card + pane.
- session-ref-open.test.tsx: mock now spreads the real module — the
artifact card imports session-view, which needs $sessionStates.
Tests: artifact-detect (15), artifacts store (12), markdown-pipeline
integration (3); full desktop suite 3128 passed, electron project 751
passed, tsc no new errors.
SessionTabMenu subscribed to $sessions + $projectTree for values it
never rendered (the row was re-read imperatively), so every tab of every
tile re-rendered its menu wrapper on any session-list or project-tree
churn — for a context menu that is almost never open. Same class as the
TreeGroup fix (#72245): derive the three scalars the menu actually shows
(pinId, title, profile) behind a keyed bail-out, so the wrapper only
re-renders when one of them changes.
#72163 narrowed $focusedSessionState but left two whole-store reads in
the same hook paying the same price:
- $subagentsBySession: only two COUNTS are rendered, but the whole-map
subscription re-ran the hook (rebuilding all ~9 statusbar items) on
every subagent progress tick in any session. Select the two scalars.
- $sessions: only one row's started_at is read, but any session-list
write (title update, poll refresh, archive) re-ran the hook. Select
the one scalar.
$statusItemsBySession rebuilt its whole output map — fresh arrays,
fresh item objects — on every recompute, and its inputs churn constantly
(subagent ticks, 5s background polls, todo updates, in ANY session). A
whole-map useStore in ComposerStatusStack then re-rendered every mounted
stack — one per open tile — on all of it, and the fresh item objects
defeated row memoization downstream.
Two halves, per the documented slice contract (use-session-slice.ts):
- producer: stabilize $statusItemsBySession per key — an unchanged
session keeps its previous array and item objects, and a fully
unchanged map keeps its previous reference so computed skips the
notify entirely ('preserve reference identity on no-ops').
- consumer: the stack subscribes to its OWN session's slice via
useSessionSlice instead of the whole map.
ModelPickerOverlay subscribed to $focusedSessionState whole — a
projection of $sessionStates, republished on every message delta — to
read two fields that essentially never change (model, provider). The
overlay is mounted app-wide and unconditionally renders the un-memoized
ModelPickerDialog (closed), so the focused session's stream re-ran the
dialog's full hook body ~30x/s.
Same defect class and same fix as the statusbar (#72163): select each
scalar through useStoreSelector so unchanged values bail out.
The salvaged no-op-publish guard moved the compression-rotation signal
into ensureSessionState, where storedSessionId is string|null. A cleared
stored id is a detach, not a rotation — firing the event with a null
next id would send the route-follow effect chasing nothing (and tsc
rejects it). Guard on a real next id.
The component map Thread passes to the virtualizer listed the
onBranchInNewChat / onCancel callbacks as useMemo deps. Whenever a parent
re-render handed down a fresh callback identity, the memo rebuilt the map
and produced new component *types*, so React unmounted and remounted every
visible message. Async-rendered parts (shiki code blocks) collapsed and
re-expanded on each remount, making the whole thread visibly jump.
That is exactly what shipped in v0.15.1: the desktop controller passed an
inline arrow for onBranchInNewChat, and the 15s status-snapshot poll
re-rendered the controller, so threads with code blocks jumped every 15
seconds (layout-shift scores of 0.39 + 0.47 per cycle, measured via CDP).
arrow away from regressing.
Route the callbacks through a ref so the component types survive any
parent re-render; only the callbacks' definedness stays a dep, because it
gates UI (the user-message Stop button). Add a regression test that fails
on the old code by asserting message DOM nodes keep their identity when
callback props change identity.
Tested on macOS arm64 (vitest + rebuilt app, CDP layout-shift
instrumentation confirms zero shifts over multiple poll cycles).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The renderer OOMs every ~60s because periodic ~1/s session.info
heartbeats churn the entire $sessionStates store on every tick even
when nothing changed. Each heartbeat:
1. Called updateSessionState with the running-test updater, which
always returned a new spread object — even when busy state hadn't
changed — because the updater param was already a fresh spread.
2. publishSessionState then spread the full $sessionStates record
and set it, firing every computed atom ($workingSessionIds,
$attentionSessionIds) and their subscribers on every heartbeat.
Over 60s × ~1/s heartbeat this continuous store churn creates
millions of short-lived objects, amplifies React re-renders, and
starves the GC, sending the renderer working set from ~350 MB to
1.2-5 GB before the OOM crash.
Fix (two changes):
1. updateSessionState: pass the raw previous state (not a spread) to
the updater so it can return the same reference on no-op. Skip the
store write, publishSessionState, and syncSessionStateToView when
the updater returned the same reference. The rotation signal from
storedSessionId changes is now emitted directly from
ensureSessionState since publishSessionState (and thus
handleTransition) is skipped on no-op.
2. publishSessionState: guard with `prev === state` reference check
(belt-and-suspenders for any other caller).
Fixes#69016
Captures medians of 5 runs for multitab and render-churn so tonight's
wins can't silently regress.
idle-cost is deliberately NOT gated. Its render attribution and idle
commit rate are trustworthy and are what the scenario exists for, but the
drag fps it reports (~0.6fps, p95 814ms) contradicts a direct
single-clock probe of the same gesture on the same build (57fps). I ruled
out sash selection, tile setup, render-counter residue, and a 20s soak,
and could not explain the gap — so the metric ships as a report, not a
gate. Gating CI on a number I can't defend would either fire on a phantom
or mask a real stall.
tier: 'report' is outside GATED ('ci','cold'), so the scenario still runs
and prints but neither compares nor writes a baseline.
withFrames ran its own requestAnimationFrame ticker while the gesture body
independently awaited rAF per step. Two rAF consumers, so the observer's
deltas counted the driver's frames as well as the app's — it reported
~3fps for a drag that a single-clock probe measures at ~23fps, and it
never moved no matter what got fixed underneath.
Timing now comes from the same callbacks the body drives (__MARK__).
This also fixes a silent false-negative on the typing pass: it paced on
setTimeout, so the independent ticker was mostly sampling idle waits
between keystrokes and reported a flat 61fps. On the driving clock the
same interaction reports ~30fps with 27 of 40 frames over 33ms — which
matches the 'typing feels slow' symptom I previously could not reproduce.
TYPE now records __TYPE_TARGET__ and the runner throws when no composer is
found, so a pass that measures nothing fails loudly instead of scoring a
perfect 0 deficit — same guard DRAG already had.
Every `Tip` carried its own `TooltipProvider`, and there are ~107 call
sites. Each is a subtree that re-renders when anything above it does, so
they dominated unrelated interactions: 52,784 TooltipProvider renders and
18.3s of component time in a single sash drag.
Radix's provider holds only refs and stable callbacks (no reactive state)
— hoisting one to the app root is what it is designed for. `Tooltip`
still reads delayDuration/disableHoverableContent from context, and the
per-Tip overrides are preserved.
`Tip` keeps a local provider as a FALLBACK, chosen by context: a
component rendered in isolation has no root provider and Radix throws
"`Tooltip` must be used within `TooltipProvider`". Without this, 20 unit
tests that render a single control fail. Inside the app the flag is
always true, so the common path is a bare Tooltip.
This is the shape the earlier lazy-mount attempt should have taken. That
one deferred the Radix subtree until hover, which moved
data-slot="tooltip-trigger" off the mounted DOM and broke 18 tests
encoding that contract. Hoisting keeps the contract intact — every one of
those tests passes unchanged.
Measured on the same drag:
TooltipProvider 52,784 renders / 18.3s -> gone from the table
Primitive.div 40.5s -> 13.4s
Popper 10.5s -> 2.7s
Tooltip 15.7s -> 4.4s
A CDP trace of one sash drag settled what the render counters could not.
I had assumed the remaining cost was layout/paint; it was not:
script 6770ms | style 1866ms | layout 71ms
Layout was never the problem. The top attributable callsite in our own
code was use-resize-observer.ts at 977ms.
Counting the callbacks named the mechanism exactly: 8,620 ResizeObserver
instances constructed, and during a 40-move drag 2,600 callbacks each
carrying exactly ONE entry — 65 separate callbacks per pointermove. Every
consumer owned a private observer, so N elements resizing under a common
ancestor meant N trips through the observer machinery instead of one
batched delivery. With five mounted tiles that is ~100 user bubbles, each
with its own observer, all woken by a width change.
One shared observer with a WeakMap of target -> handlers. Callers keep
their exact contract: a handler observing several elements is still
invoked once with all of its entries, and unobserve happens when the last
handler for an element goes away.
Verified by trace, before -> after:
use-resize-observer 977ms -> 42.5ms (-96%)
style recalc 1866ms -> 1145ms (-39%)
total script 6770ms -> 3929ms (-42%)
Callback count 2,600 -> 43: one delivery per frame instead of 65.
Adds the two probes that found it. diag-drag-trace.mjs takes a real
timeline trace and prints the style/layout/script split plus the top
script callsites — that split is what disproved the layout theory.
diag-ro-storm.mjs counts RO callbacks vs entries, which is what
distinguished 'a few expensive calls' from 'very many cheap ones'.
The status bar shipped every affordance it had, so approvals, the terminal
toggle, agents, cron and webhooks sat there permanently for users who never
touched them.
Those five now start hidden and the bar owns a context menu that turns them
back on, persisted per install. Items opt in by naming themselves with
`toggleLabel`, so a plugin contribution that doesn't opt in always shows;
the system icon and the version/update pills are listed but locked on,
since hiding the way back into settings strands the user.
Preferences store the hidden set rather than the visible one, so an item
added to the bar in a later version appears for existing users instead of
staying silently off.
Electron's findInPage selection is per-webContents, not per-route: without
an explicit teardown, the highlight overlay (and a stale match counter)
from one chat survived navigating to another session or a settings page.
The FindBar now closes itself via effect cleanup keyed on the router
pathname — the first render never fires it, a route change tears down the
previous route's search, and unmount (the session/profile switch paths
that remount the global overlays) gets the same teardown. closeFindBar is
already idempotent, so a closed bar never re-enters the bridge, and a
navigation with the bar closed makes zero bridge calls.
Tests (find-bar.test.tsx 42 -> 48):
- navigation closes the bar, resets the store, and calls
stopFindInPage exactly once (highlight cleanup on session switch)
- navigation with the bar closed never reaches into the bridge
- keybind registration pins: view.findInPage is mod+f in the view
category; mod+f passes comboAllowedInInput (so ⌘F fires from the
composer instead of typing 'f'); the find-next/find-previous pair
ships unbound while view.toggleReview keeps mod+g; all three actions
have en + zh labels for the keybinds panel
The component tests now render inside a MemoryRouter (FindBar reads
useLocation), with a navigation harness that captures useNavigate.
PR #53891 (cherry-picked in the three preceding commits) landed the Electron
bridge, the store, the find bar overlay, and Cmd/Ctrl+F to open. It stopped
short of the rest of the accelerator set and had two lifecycle leaks. This
completes the surface to match the platform convention that Chrome, Safari,
VS Code, and Claude Desktop's own findInPage bundle all ship.
Accelerators now wired:
- Cmd/Ctrl+F open the find bar (view.findInPage, from the PR)
- Cmd/Ctrl+G find next (new)
- Cmd/Ctrl+Shift+G find previous (new)
- Enter / Shift+Enter step from the input (from the PR)
- Escape close + stopFindInPage('clearSelection') (from the PR)
Keyboard ownership (the substantive fix)
Cmd+G was already bound to `view.toggleReview` and Escape to
`composer.cancel`. The find bar's own capture-phase window listener cannot
win those keys by calling stopPropagation: the keybind dispatcher's listener
sits on the SAME window target in the SAME phase, and propagation control
does not suppress sibling listeners on one target. Left alone, Cmd+G would
step a match AND toggle the review pane, and Escape would dismiss the bar AND
abort a running turn.
So ownership is decided by the dispatcher, which AGENTS.md already names the
single owner of combo dispatch: `findBarClaimsCombo` is consulted in
use-keybinds before the registry lookup, and yields mod+g / mod+shift+g /
escape to the bar only while it is open. Closing the bar hands every one of
them straight back. That is the "keyboard ownership follows focus / one cancel
gesture does exactly one thing" invariant.
`view.findNext` / `view.findPrevious` are registered with EMPTY defaults on
purpose — shipping mod+g as a second default would flag a permanent conflict
in the keybinds panel against view.toggleReview. The entries document the pair
and let a user bind a dedicated chord; stepping is a no-op unless the bar is
open with a query, so a bound key can never search invisibly.
Listener-leak fixes
- The found-in-page bridge subscription is now refcounted in the store, so a
remount (the connection re-home path remounts the global overlays) cannot
stack duplicate subscribers that each re-dispatch the same result and
outlive their component. The subscription is deliberately mount-scoped, not
active-scoped: results for an in-flight search must still land if the bar
just closed.
- `setFindQuery` now refuses to search a closed bar. The component clears its
debounce on close, but a 200ms timer that already fired would re-issue a
find and re-highlight the page after the user pressed Escape. Caught by the
test, fixed in the store rather than papered over in the component.
- `closeFindBar` is idempotent — Escape is a shared gesture, so a second close
must not reach into Electron again.
Pure logic extracted for testing (no source regexing)
- `src/lib/find-in-page.ts`: `formatMatchLabel` (three distinct counter
states: hidden with no query, explicit 0/0, ordinal/count; clamps the
ordinal and never emits NaN — Electron legitimately reports ordinal 0 on a
non-final update), `findBarKeyAction` (the keybinding matcher, DOM-free),
and `findBarClaimsCombo` (the ownership predicate above).
Also: match counter and buttons get accessible names and the counter is
aria-live, the hardcoded English "Previous"/"Next"/"Close" tooltips move to
i18n (en + zh) alongside the new keybind labels, and the input gets an
aria-label so the bar is reachable by role.
Tests: apps/desktop/src/components/find-bar.test.tsx — 42 cases over the
pure helpers, the store (open/close, next/prev dispatch shape, escape clears,
refcount, double-release), and the component (focus on open, debounce
coalescing, Cmd+G from outside the input, unmount releases both the bridge
subscription and the window listener).
cd apps/desktop && npx vitest run src/components/find-bar.test.tsx \
electron/find-in-page.test.ts
-> 62 passed (42 new + 20 from the PR)
Adjacent suites (src/lib/keybinds, src/i18n, src/store): 487 passed.
`tsc -p tsconfig.electron.json --noEmit` clean; `tsc -p .` has 114 pre-existing
errors vs 120 on the merge base (all @assistant-ui / bippy / composer), none in
the touched files. eslint clean on every touched file.
Co-authored-by: David Metcalfe <DavidMetcalfe@users.noreply.github.com>
Vitest coverage for apps/desktop/electron/find-in-page.ts.
The helpers and the IPC handlers in main.ts are the only
consumers, so the tests pin the wire shape (match counter
shape, options defaults, no-throw-on-destroyed) and the
multi-window correctness that the original CJS PR missed:
- formatFoundInPage:
- Maps { activeMatchOrdinal, matches } → wire payload.
- Coerces missing fields to zero (the renderer never
sees NaN).
- Tolerates null / undefined inputs.
- performFind:
- Forwards query + options to webContents.findInPage.
- Defaults forward=true and findNext=false when omitted.
- Treats null / non-object options as "all defaults".
- Coerces a non-string query to string (defensive against
a misbehaving renderer).
- Is a no-op on null webContents.
- Is a no-op on destroyed webContents (does not throw
across the IPC boundary).
- stopFind:
- Calls stopFindInPage with the default action
('clearSelection').
- Honors an explicit action argument.
- Is a no-op on null or destroyed webContents.
- installFoundInPageForwarder:
- Forwards 'found-in-page' to the sender as a formatted
payload.
- Handles missing fields without throwing.
- Skips send when webContents is destroyed at fire time.
- Returned uninstall removes the listener.
- Returned uninstall on null/destroyed webContents is a
safe no-op.
- Regression: two forwarders installed on distinct
webContents do not cross-fire. This is the bug the
original PR shipped — the global mainWindow listener
routed results to the primary regardless of which
renderer invoked findInPage. Pinning this here keeps
the per-sender routing from regressing.
Run with:
cd apps/desktop && npx vitest run electron/find-in-page.test.ts --project electron
Brings forward the renderer-side changes from PR #53891,
adapted to the current main branch (where app-shell.tsx
has been replaced by apps/desktop/src/app/contrib/wiring.tsx
and keybinds/actions.ts has gained new view.* entries):
- apps/desktop/src/store/find-in-page.ts (new): nanostores
atom + actions for the find bar (openFindBar, closeFindBar,
setFindQuery, findNext, findPrevious, updateFindResults,
initFindInPageListener). openFindBar is dispatched by the
view.findInPage keybind handler in use-keybinds.
- apps/desktop/src/components/find-bar.tsx (new): the find
bar overlay (top-right, below the titlebar). Debounces
input 200ms before issuing findInPage, focuses on open,
supports Enter (next) / Shift+Enter (previous) / Escape
(close), shows a "3/12" match counter from the
'hermes:found-in-page' stream. Global capture-phase
Escape listener so the bar closes regardless of focus.
- apps/desktop/src/lib/keybinds/actions.ts: adds
view.findInPage with default combo 'mod+f'. The keybinds
runtime already routes any mod+ / ctrl+ combo through
editable-focus contexts (see comboAllowedInInput in
lib/keybinds/combo.ts:193), so ⌘F focuses the find bar
instead of typing 'f' into a textarea — matches browser
behavior.
- apps/desktop/src/app/hooks/use-keybinds.ts: wires
view.findInPage → openFindBar in the global handler map.
- apps/desktop/src/app/contrib/wiring.tsx: mounts <FindBar />
alongside the other global overlays (CommandPalette,
SessionSwitcher, etc.).
- apps/desktop/src/i18n/{en,zh}.ts: labels
'view.findInPage' for the keybinds panel.
Closes#46169
The original PR targeted the CJS Electron files
(apps/desktop/electron/main.cjs and preload.cjs), but commit
39d09453f "feat(desktop): ts-ify everything" renamed them to
main.ts and preload.ts on current main. The PR's diff therefore
targeted files that no longer exist on main.
Brings the bridge forward to the current TypeScript Electron
files and extracts the IPC bridge helpers into a focused
pure-helpers module:
- apps/desktop/electron/find-in-page.ts (new):
- performFind(webContents, query, options) — wraps
webContents.findInPage with default-coercing options.
- stopFind(webContents, action) — clears highlights.
- formatFoundInPage(result) — pure projection of
Electron's FoundInPageResult onto the wire payload shape
({ activeMatchOrdinal, count }).
- installFoundInPageForwarder(webContents) — wires a
sender-scoped 'found-in-page' forwarder; returns an
uninstall function. Returns a no-op uninstall for null
or destroyed webContents so callers don't need guards.
- apps/desktop/electron/main.ts:
- ipcMain.handle('hermes:find-in-page', event => ...)
resolves the requesting window via
BrowserWindow.fromWebContents(event.sender) and routes
the search to THAT window, not the global primary. This
fixes a multi-window bug where Cmd+F pressed in a
secondary session window (one per chat, spawned via
hermes🪟openSession) searched the primary window
instead of the focused surface.
- ipcMain.handle('hermes:stop-find-in-page', event => ...)
routes stopFind through the requesting window for
multi-window correctness.
- A per-sender lazy forwarder registry
(foundInPageForwarders: Map<webContentsId, () => void>)
installs installFoundInPageForwarder on first
findInPage call, scoped to the sender's webContents.
Cleans up automatically via webContents.once('destroyed',
...). The forwarder sends results back to the SAME
renderer that initiated the search, never the global
primary — so a secondary session window's Cmd+F shows
matches from THAT window and the match counter reports
matches from THAT window's DOM.
- apps/desktop/electron/preload.ts:
- hermesDesktop.findInPage(query, options) — invokes
the IPC handler.
- hermesDesktop.stopFindInPage() — invokes the IPC
handler.
- hermesDesktop.onFoundInPage(callback) — subscribes to
'hermes:found-in-page' results from the sender;
returns an unsubscribe function so the FindBar can
clean up on unmount.
- apps/desktop/src/global.d.ts:
- Three new hermesDesktop method declarations:
findInPage, stopFindInPage, onFoundInPage. The new
forwarder install registers a 'found-in-page' listener
bound to the sender's webContents and emits
'hermes:found-in-page' results back to the sender.
The multi-window fix is part of the same port — the old
PR's behavior (Cmd+F in a secondary session window searched
the global primary) was a bug present in the CJS files,
not a design constraint we wanted to preserve. The new
helper module uses event.sender by design, so the
multi-window correctness lands with the TS port.
Fixes#46169
Follow-ups on the salvaged goal-status display (#63527):
- Seed the goal store from the /goal dispatch notice ("⊙ Goal set …") and
from /goal status|pause|resume|clear exec output in slash.ts. The backend
only emits status.update kind:"goal" after the first turn's post-turn
judge, so without this the indicator stayed empty while the kickoff turn
ran (sweeper review finding on #63527).
- Add the missing ja / zh-hant statusStack goal copy — desktop ships four
locales, not two (sweeper review finding on #55651).
- Add a component-level vitest for the composer goal indicator rendering
from store states: none / active / paused / detail line / other-session.
Co-authored-by: HaisamAbbas <95044189+HaisamAbbas@users.noreply.github.com>
Assisted-by: Claude Fable 5 via Hermes Agent
redirectPrompt inserts its correction as a second user row just before the
live reply, so one turn can own a contiguous run of user rows. Three recovery
paths each assumed a turn has exactly one, and all three kept the correction
and discarded the prompt that started the turn:
- recoverableTail walked back to the nearest user row, so the crash journal
never stored the original.
- preserveLocalPendingTurnMessages kept only the newest optimistic user row.
Widened to the contiguous run — rows separated by an assistant reply are
still dropped, which is the stale-post-compression case that rule exists for.
- appendLiveSessionProjection had no way to render corrections; it now projects
them after the prompt, deduped against the transcript's latest user run.
Losing a row also shifted every later role:ordinal pairing in the reconcile,
which is why the thread looked like it compacted rather than just missing one
bubble. Reproducible on a reconnect and on a dev hot update, which remounts the
session cache while the gateway socket survives.
The sidebar labelled sections and workspace lanes `loaded/total`, which
read as a progress bar people expected to fill up rather than a count of
loaded rows. Pricing that label cost a COUNT(*) per profile database on
every sidebar refresh, purely so the numerator and denominator could
differ.
Pagination only needs to know whether another page exists, and that comes
free from the rows the query already returned: a window that comes back
full means more remain on disk. Sections now show the loaded count alone,
and the backend reports per-profile `profiles_truncated` flags in place of
`total` / `profile_totals`.
Investigating the missing-spinner report turned up no defect in the seeding
path: the active_list poll already lights a row for a turn the renderer never
saw start, holds it across polls, and follows a recycled runtime id onto its
new stored session. Pin all three so the reap change can't silently regress
turn-start while fixing turn-end.
Two boundaries worth naming rather than rediscovering:
- `starting` is deliberately NOT working. It means agent_build_started without
agent_ready, and _start_agent_build runs on any incidental RPC that needs the
agent — not just a prompt — so treating it as a turn would spin the row on
merely opening a session.
- $workingSessionIds is keyed by STORED id and drops entries whose
storedSessionId is null, while message.start flips busy without carrying one.
A runtime that was never seeded with a stored id therefore goes busy
invisibly. That is the remaining path by which a spinner can go missing.
A run of 3+ adjacent tool calls collapses into the `.tool-group-scroll`
window, but `shouldBoundToolGroup` took `hasUnboundable` as a run-level
veto: a single exempt row anywhere in the range disabled collapsing for
the entire run. The exempt set was clarify, image_generate, execute_code,
read_file and every file-edit tool — which is most of what a coding
session does, so in practice runs never collapsed. Replaying a real
session's transcript: 84 consecutive calls, 30 of them veto-triggering,
zero windows.
The code-body entries were never needed. Everything ToolEntry renders
carries `data-tool-row`, and the `:has([data-tool-row][data-tool-open])`
rule already lifts the cap and the mask. A diff row mounts open, so it
frees the group the moment it appears; a collapsed row is a one-line
status whose body is not in the DOM at all, so there is nothing to clip.
Collapsing the row drops the group back to a compact window, which the
JS veto could not do.
Narrow the opt-out to the two components that bypass ToolEntry and so
can never emit `data-tool-row`: clarify and image_generate.
preserveReasoningParts was gated on exact text equality with the cached row.
Mid-turn the authoritative text has advanced by a delta or two, so the guard
fails and the row is rebuilt from the gateway's inflight projection — which
is text-only. The renderer's cache is the sole carrier of a running turn's
structure, so switching away and back stripped the reasoning and every tool
call, leaving the turn looking inert.
Carry tool calls alongside reasoning, dedupe them on toolCallId, and match on
same-turn (identical text, or authoritative text extending the cached text)
rather than strict equality. Attachment refs and image re-appending stay on
the strict path: those reconcile a settled row, and a growing row is by
definition not settled.
session.active_list is authoritative about absence, but the renderer only
read the rows it returned. A turn that ends while the websocket is degraded
— a remote gateway on a flaky link, a reconnect, a profile swap — drops out
of the gateway's _sessions without Desktop ever seeing the running=false
edge, so the row spins forever and the busy->idle transition that paints the
green unread dot never fires.
Track live runtime ids per gateway profile and settle anything that
disappears between polls through publishSessionState so the real transition
fires. Profile scoping is load-bearing: background profiles are served by
other gateways and never appear in this profile's snapshot, so an unscoped
reap would dark out every other profile's running rows.
The edit composer pastes through the same `insertComposerContentsAtCaret` the
main composer uses, so it had the identical bug: the Range-based insert never
reaches Chromium's undo stack and Cmd+Z skipped past the paste to destroy the
edit before it. Its inline-ref and trigger-chip inserts mutate the DOM directly
too, with the same result.
Both surfaces now share `useComposerUndo`. The hook already keys its
document-level `beforeinput` claim off `document.activeElement`, so the two
mounted instances stay independent — only the focused editor's stack responds.
Undo/redo is handled ahead of Escape here, since a stray Cmd+Z falling through
would cancel the whole edit rather than step back one change.
`insertRefStrings` banks through `withUndoPoint` rather than recording after the
insert, which would have snapshotted the state it was meant to restore.
The rich editor mutates its DOM through `Range` rather than the browser's
editing commands, because `execCommand('insertText')` is ~O(n²) on large
multiline blobs and froze the composer for seconds on a big paste (#45812).
Those mutations never reach Chromium's undo stack, so a paste was invisible to
it — Cmd+Z skipped straight past the pasted text and undid whatever edit came
before it, leaving the paste stranded and the earlier edit destroyed.
Owning the stack outright is the only coherent fix; a half-owned one interleaves
our snapshots with Chromium's own typing entries and undoes them out of order.
Every edit path now banks its pre-edit state, and the editor claims Cmd+Z /
Cmd+Shift+Z (plus Ctrl+Y) instead of letting the native command run. Snapshots
are plain text + a caret offset rather than DOM, since the editor already
round-trips losslessly through composerPlainText/renderComposerContents.
Consecutive keystrokes coalesce inside a 600ms window, so undo steps back by a
typing burst the way a native editor does rather than one character at a time.
Electron's Edit menu `{ role: 'undo' }` fires the native command without a
keystroke the renderer can see, so a capture-phase `beforeinput` listener claims
historyUndo/historyRedo too and keeps the menu item and the shortcut in
agreement. Switching drafts resets the history — undoing into another
conversation's text is worse than having none.
Co-authored-by: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com>
Reverts the tooltip half of 4798994dc; keeps the idle-cost scenario.
Lazily mounting Radix on first hover measured well (105k -> 26k
TooltipProvider renders per drag) but broke 18 tests across 12 files.
Those tests are not incidental: the repo has an established convention of
asserting `[data-slot="tooltip-trigger"]` at mount to prove a control
carries a tooltip, and deferring the mount invalidates all of them at
once. There is also a real behavior risk the convention was protecting —
`asChild` puts the slot on the button element itself, so arming REPLACES
the node, which is exactly the kind of identity change that breaks focus
restoration and ref-holding call sites.
A 4x cut in tooltip churn is not worth reworking every tooltip assertion
in the app plus taking that risk, on a component with ~107 call sites.
If it's worth revisiting, the right shape is probably making
TooltipProvider itself cheap (one app-level provider) rather than
deferring the mount per call site — that preserves the DOM contract these
tests encode.
The genuine win in this branch stands on its own: the $layoutTree
subscription fix (commits 83 -> 12 on a sash drag) is unaffected.
`plainTextInRange` serialized the caret's preceding content through a bare
<div>, but `composerPlainText` appends "\n" to any block element that isn't the
editor slot. So `beforeText` always looked like it ended in whitespace and the
separating space was never inserted — dragging a file in after a word produced
`review@file:...` glued together.
Marking the scratch container with RICH_INPUT_SLOT makes it serialize in the
same coordinates as the editor. Same fix lands in the new `caretOffsetInEditor`,
which measures caret offsets the same way.
