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.
parseMarkdownIntoBlocksCached bypassed its cache for text under 1024
chars, on the theory that re-lexing a short message is cheap. The lex is
cheap; what it returns is not free. `parseMarkdownIntoBlocks` builds a
fresh array every call (verified in streamdown's dist: `let r=[]` ...
`return r`), and Streamdown mirrors the block list into useState — so a
new array identity for UNCHANGED text re-renders Streamdown and every
Block beneath it.
Most messages are short, so most of the transcript was on the uncached
path. Caching every length cuts the idle cost of five mounted tiles:
Streamdown 5.2ms -> 2.6ms
Block 128ms -> 85ms
Ct 122ms -> 81ms
Cache bumped 64 -> 256 entries to cover the now-larger key space.
This does NOT reduce Streamdown's 105 idle self-renders — array identity
turned out not to be what drives those, and I verified the cache returns
a stable identity, so that root is still open. This is a cost win, not
the churn fix.
TreeGroup called useStore($layoutTree) to build its right-click menu's
move/split directions. That subscribes every zone — and therefore every
mounted pane and its entire transcript — to the whole layout tree. A sash
drag rewrites the tree once per frame, so dragging the sidebar re-rendered
all five tiles' message lists on every pointermove, for a context menu
nobody had open.
The directions are only read when the menu renders, so read the tree there
with .get() instead. Same lazy shape the neighbouring `closable` prop
already uses.
Measured over one 60px sash drag with five busy tiles:
commits 83 -> 12
ChatView 150 -> 10 (4465ms -> 353ms)
AuiProvider 9450 -> 630 (9868ms -> 774ms)
TreeGroup 180 -> 12
TreeSplit 90 -> 6
Also fixes an observer effect in the harness: idle-cost recorded render
attribution *during* the timed gesture, and the counter walks the fiber
tree on every commit. That was large enough to hide this 15x reduction
behind an unchanged fps, so timing and attribution are separate passes now
and `record` defaults off.
Adds scripts/diag-drag-churn.mjs — the probe that found this. It reports
the transcript chain (who above the messages re-rendered) plus every atom
that notified, which is what named TreeGroup instead of leaving it to be
guessed at. Notably the atom list came back EMPTY: this was never store
churn, so the render-attribution path was the only thing that could have
found it.
The synthetic gesture oscillated +/-3px, which nets to zero displacement
and can clamp to a no-op — so it reported a confident fps number for a
drag that never moved the sash. Sweeps monotonically now, dispatches
pointer events React's synthetic system accepts (isPrimary/button/buttons),
and records dragTarget + dragMoved so a drag that silently did nothing is
visible in the output rather than passing as a measurement.
Verified: dragMoved now reports 60px where it previously reported 0.
Adds an `idle-cost` scenario for the symptom Brooklyn reported: with a
thread spinning, resizing the sidebar feels slow. It holds N tiles busy,
pushes NO tokens, and measures the renderer's self-inflicted commit rate
plus fps while dragging the splitter and while typing.
It reproduces immediately. Five busy tiles, nothing streaming:
idle commits 17.7/sec (should be 0 — nothing is happening)
drag 1.4 fps p95 812ms, worst frame 1.9s
typing 61 fps (fine — this is specific to resize)
Attributing the drag window showed 105,385 TooltipProvider renders and
~15s of component time across a 60-frame gesture. Cause: `Tip` mounts a
full Radix provider + Tooltip per call site, and there are ~107 of them.
Radix's Tooltip holds real state and Popper subscribes to layout, so an
unrelated interaction re-rendered all of them.
Mounts the machinery lazily instead, on first hover/focus. Tooltip churn
drops ~4x (105k -> 26k) and drag doubles to 3fps.
Note `defaultOpen` on the armed Tooltip is load-bearing: the pointerenter
that armed it has already fired, so Radix never sees it and the tip mounts
silently closed. A test caught exactly that, and now guards it.
3fps is still bad — the remaining cost is the whole transcript
re-rendering per resize frame (MessagePrimitive.Parts 12,600 renders /
10.5s, Block/Ct 24,300 each, all 100% wasted). Separate fix.
Repair signalled "reinstall me" by deleting the bootstrap marker. That was
already destructive -- repair is reachable from a transient backend error on a
fully working install -- and it stranded users in first-run setup with no way
back short of hand-writing the marker file.
Carry the intent in an explicit flag instead. Repair forces the next resolve
through the installer and clears itself once the reinstall starts, so a forced
reinstall still works without destroying provenance about how the install was
created.
Closes#72166
Marker presence was the launch gate, but the marker is provenance about who
ran the install -- not proof the runtime works. A CLI-installed repo+venv, or
a healthy install whose marker a repair deleted, both read as "never
installed" and dropped the user into first-run bootstrap on every launch.
Split the two questions: classifyActiveRuntime() reports marker validity and
runtime usability separately, and the resolver launches whenever the runtime
is usable, logging when it proceeds without a marker. An unusable runtime
still falls through to bootstrap even with a valid marker, so an interrupted
install can't spawn a dead backend.
Drops isBootstrapComplete(), which had no callers left once the gate moved.
Co-authored-by: iveywest <iveywest@users.noreply.github.com>
Co-authored-by: lihengming <lihengming@users.noreply.github.com>
The expanded terminal row printed the same string as the title, as the
`$` transcript, and again as detail. shellCommand preferred the
backend's display preview over the real `command` arg, so the transcript
showed a summary of what ran rather than what ran; and a terminal call
with no output fell through to the generic fallback, which echoes
args.context under a transcript already showing it.
Two problems, both found by distrusting the harness's own numbers.
1. The scenario slept a fixed 1s after mounting tabs, then recorded. Boot
and session hydration are not reliably done by then, so a variable
amount of unrelated work landed inside the measurement window. Three
back-to-back runs on identical code spread 2.2x on total_renders and
3.8x on wasted_renders — wide enough that a single-run before/after
delta could be mostly noise. Replaced with a quiesce gate that waits
for commits to hold still before recording, and reports 'quiet:N' or
'timeout:...' so a contaminated run is visible instead of silent.
2. The counter attributed a context-driven re-render as 'wasted', which
pointed at memo() as the fix when memo cannot block context at all.
Adds contextChanged via the fiber's context dependency list, and
excludes it from wasted.
The gate also turned up a finding worth more than the fix: with five busy
tiles and NO driver running, the renderer still commits ~18x/sec. The
report now names the cascade roots (own state changed, props did not)
rather than leaving them to be guessed at — Streamdown re-renders itself
105 times while idle, which is what drives Block/Ct.
A pasted link went in as raw URL text, wrapping across the composer and
staying inert. It now becomes the same `@url:` reference the "+ → Add URL"
dialog inserts — parsed in place, so a link mid-sentence keeps its position
and the punctuation that ended the sentence stays outside the chip. Typing
one and pressing space commits it the same way.
Both rich-editor surfaces get it: the composer and the message-edit box,
whose paste went through `execCommand` and could not produce a chip at all.
`insertPlainTextAtCaret` dropped its text in verbatim, so a caller with
directives in hand had no way to land them as chips. It becomes
`insertComposerContentsAtCaret`, sharing the parse `renderComposerContents`
already uses, and gains a sibling `replaceBeforeCaret` for swapping a
just-typed token for a chip.
The composer labeled a `@url:` chip with `refLabel`, which takes the last
path segment — three PR links all read as their number. The transcript had
its own better labeler that stopped at the hostname, so the same reference
read differently before and after send.
One `refChipLabel` now serves both: host without `www.`, path riding along
for the chip's existing truncate to cut, and the full value on the chip's
title so a cut-off link is still readable on hover.
The statusbar subscribed to `$focusedSessionState` — a projection of
`$sessionStates`, which is republished on every message delta — but reads
only three fields off it. Every token therefore re-ran useStatusbarItems
and rebuilt all ~9 item objects, and since StatusbarItemView was not
memoized, one changed item (the running timer) re-rendered the whole bar.
Adds `useStoreSelector` beside the existing `useSessionSlice`, the same
narrowing idea for a scalar instead of a keyed array: subscribe to the
store, but bail out unless the selected value changes. Applies it to the
three fields the statusbar actually reads, and memoizes StatusbarItemView.
Measured over five concurrent streaming tabs (`render-churn`):
total renders 78,385 -> 21,701 (-72%)
wasted renders 10,432 -> 6,112 (-41%)
TooltipContent 2,304 -> 143 (-94%)
StatusbarItemView 2,174 -> 0
Drives the same synthetic multi-tab streaming workload as `multitab`
(publishSessionState per session per flush, no backend, no credits) but
reports render attribution instead of frame pacing — sidebar_renders,
wasted_renders, and wasted_notifies.
Answers 'does the sidebar re-render while an agent is typing' directly.
Adds two dev-only counters that attribute re-renders and store
notifications during an interaction, so a perf claim can be answered with
a number instead of a hunch:
window.__RENDER_COUNTS__ what re-rendered, and why (props/state/parent)
window.__ATOM_CHURN__ which store published it, and whether it mattered
The `wasted` column in each is the fix list — components that re-rendered
with no changed input, and stores that published a value equal to the
last one. Both are inert until start(), so idle cost is one branch per
commit and per notify.
React 19.2 removed injectProfilingHooks from react-dom, so the mark*
profiling family is unavailable and onCommitFiberRoot is the only channel
left. <Profiler> can't answer the question either: React invokes onRender
for every Profiler in a committed tree including subtrees that bailed
out, and a bailed-out subtree still reports nonzero actualDuration. This
uses bippy's didFiberRender instead. bippy over react-scan because
react-scan/lite is a thin wrapper over it while the package pulls ~217
transitive deps and floats two on latest.
main.tsx imports the entry statically above react-dom because react-dom
captures the devtools hook at module init — a late install reports
renderers=0 and observes zero commits. Production exclusion is handled by
a build-time alias to a no-op module rather than tree-shaking, since a
static side-effect import can't be eliminated.
While the workspace pane shows a full page (Artifacts, Skills, Messaging, a
plugin route), a sidebar click on the ACTIVE session did nothing: onResumeSession
took focusOpenSession's `true` for the main-session branch as "already on
screen" and skipped the navigate, but fronting the workspace tab doesn't put the
chat back — the page is still routed. The user had to click some other session
and then the active one to get back.
focusOpenSession now reports WHICH surface it fronted ('main' | 'tile' | null),
and focusedSessionNeedsRoute decides: a tile never needs a route (its pane
renders the chat regardless), a main hit does while a page covers the workspace.
`/work` typed into a fresh Cmd+T tab loaded the skill in that tab and
printed "⚡ loading skill: work" there, then fired the skill's kickoff
prompt as a user message into whatever conversation was on screen.
The dispatcher resolves its target once, through resolveTargetSessionId,
and every other consumer of that answer already honors it: the output
writer binds to the target's stored id, and the busy gate reads the
target's own state. The send did not — `submitPromptText(message)` passed
no target at all, so submit fell back to `activeSessionIdRef`, which
names the foreground chat. #71805 fixed the two sibling leaks in this
same function; this is the third and the one that actually moved the
user's prompt.
Forward the resolved pair instead. Every target the dispatcher serves —
a tile, a background queue drain, a session this very call created —
was hitting the same fallback, so the fix covers the class rather than
the tab case that surfaced it.
Technical mode rendered the raw payload two different ways — a bare
block for most rows, a native `<details>` for file edits, whose
browser-drawn marker matches nothing else in the app. Both are now one
collapsed chevron disclosure at a smaller type size, with even padding
against the row body.
The strip's rule is an inset shadow painted in the container's last pixel
row, and full-height tabs covered it — so each tab read as overhanging
the bar by 1px. Inactive tabs compensated with their own border, stacking
a second translucent line that darkened the seam.
Inactive tabs now stop 1px short and draw no bottom border, leaving the
container as the sole owner of one continuous rule; the active tab keeps
full height so it alone cuts through. Hover also darkens rather than
lightens, since lightening moved a hovered tab toward the active
surface's look.
The arc reduced to a few faint dots on session rows. Two causes: the ring
was outset by 2px into a scroller that clips horizontally, losing its
left and right runs; and its tail color defaults to the chrome
background, which is invisible against the sidebar, leaving only the
bright stop of each gradient pass.
An `arc-row` variant sits flush and ties the tail back to the ring
color. The ring's radius is now derived from its standoff (r_host + gap)
rather than inherited, which keeps any outset host concentric instead of
pinched.
Content links read as a small primary-tinted chip instead of an
underline, dropping the trailing external-link arrow. The tint is
currentColor-relative, so one class carries text and fill in the same
hue across every theme, and `box-decoration-break: clone` gives a
wrapped link a chip per line fragment.
The sidebar "+" now stacks a tab instead of replacing the surface, so the
prior session stays mounted and several chat surfaces can be on the page at
once. Helpers that waited for the old transcript to disappear from the page
timed out, and `.first()` locators / bare `document.querySelector` calls
started resolving against the wrong session (CI's "resolved to 2 elements"
strict-mode violation).
Target the most recently mounted `[data-composer-target]` surface instead,
and assert the NEW surface is empty rather than waiting for the old text to
vanish.
