Fetch diff-card context on demand; virtualize and cap the file preview (G2, G3) - #1901
Merged
SawyerHood merged 4 commits intoAug 19, 2026
Conversation
SawyerHood
changed the title
SawyerHood
force-pushed
the
bb/mobile-perf/diff-and-file-preview
branch
from
August 19, 2026 07:42
08eb170 to
8b892df
Compare
SawyerHood
force-pushed
the
bb/mobile-perf/diff-and-file-preview
branch
from
August 19, 2026 07:50
8b892df to
7870d5e
Compare
SawyerHood
marked this pull request as ready for review
August 19, 2026 08:04
This was referenced Aug 19, 2026
SawyerHood
force-pushed
the
bb/mobile-perf/diff-and-file-preview
branch
from
August 19, 2026 15:46
7870d5e to
ea3f547
Compare
SawyerHood
force-pushed
the
bb/mobile-perf/diff-and-file-preview
branch
from
August 19, 2026 15:59
ea3f547 to
d0dad9d
Compare
Diff-tab text cards fetched the full old and new file contents as soon as the card scrolled within 200 px of the viewport, only so @pierre/diffs could offer expand-context buttons. That cost two whole-file reads per modified card plus a second tokenize/render pass on the enriched diff, which phones cannot afford. Text cards now render from the patch alone. An app-owned "Expand context" row under the diff requests the contents on demand (with a retry on failure); fine-pointer devices keep the zero-click experience by scheduling the same request during idle time. Image and SVG cards, which cannot render without the contents, still fetch on viewport entry. Added and deleted files never offer the row since their patch already carries every line. Co-Authored-By: Claude <noreply@anthropic.com>
… for text The right-panel file preview handed pierre the whole file un-virtualized, remounted the renderer when the worker highlight cache arrived (throwing away its DOM and scroll position for nothing since pierre repaints the highlighted AST in place), had no size ceiling, and built a base64 data: URL for every workspace file read — including text files, where the URL was only ever used as part of a cache key that then embedded the whole encoded file. - The code view now owns its own scroll container registered as pierre's virtualizer root, so only rows near the viewport are rendered. Deep-link scrolling nudges the viewport toward the target's estimated offset until the row is realized (exact in scroll mode, converging in wrap mode). - Files over 5,000 lines or 512 KB render a leading prefix with a "Load full file" action; a line link past the prefix implicitly loads the whole file. Loading the full file remounts pierre (its virtualized instance keeps its hydrated contents) and preserves the scroll offset. - Pierre mounts once per file; the plain-to-highlighted transition no longer remounts it. - Workspace previews build a data: URL only for image/video kinds; text previews use the /diff/file route URL as their identity. - Stories cover the capped view, a deep link past the cap, and a code preview inside a content-sized scroller (skill detail). Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Gives the on-demand context affordance a visual surface: a mid-file hunk with a delayed contents fetcher, so the coarse-pointer Expand context / Loading states and the fine-pointer idle auto-expansion can both be checked in Ladle. Co-Authored-By: Claude <noreply@anthropic.com>
SawyerHood
force-pushed
the
bb/mobile-perf/diff-and-file-preview
branch
from
August 19, 2026 16:13
d0dad9d to
df059f6
Compare
SawyerHood
added a commit
that referenced
this pull request
Aug 19, 2026
…routes, composer retention (E1, E6, E7, E10, D3, J3, J4, I6, B28) (#1900) ## What was wrong On phones (iOS Safari, compact viewports) several app-side patterns made every page and every thread heavier than it needed to be: - Sidebar rows wrapped each row in a modal Radix ContextMenu whose 700 ms long-press set aria-hidden on #root, registered a non-passive touchmove and flipped body pointer-events with the timeline mounted behind the drawer (E1). - dnd-kit's TouchSensor kept a permanent non-passive window touchmove listener from sidebar DndContexts mounted at boot, and the SidebarInset swipe-open registered another one for every content touch. Both make the first move of every scroll wait for the main thread (E6, E7). - Settings/Tools routes swapped the whole AppSidebar out; returning remounted ProjectList in the closed drawer (E10). - A pending permission/question swapped FollowUpPromptBox out of the tree, rebuilding TipTap per approval (D3). - The collapsed compact composer built the full TipTap editor during thread mount (D5 (reverted, see below)). - Fixed-panel storage was scanned and zod-parsed on every navigation and re-written on mount (J3). - Heavy per-thread caches used the default 5-minute gcTime (J4). - PR check icons and the GitHub logo fetched light+dark PNGs from github.githubassets.com (I6). - Root compose showed only "Loading…" until the sidebar bootstrap settled (B28). ## What changed - E1: `CompactLongPressMenu` (long-press/right-click detector, nothing mounted until first open) opens the existing responsive drawer with the same DropdownMenu items; desktop keeps the context menu. - E6: `SidebarTouchSensor` installs dnd-kit's listener only while the compact drawer shows (external store written by SidebarProvider, no context subscription); tab strip wires TouchSensor only when open with 2+ tabs. - E7: only edge-zone touches (24-72 px) get the non-passive swipe path; deeper touches keep the recognizer on a passive listener without preventDefault. - E10: on compact one persistent `<Sidebar>` panel hosts the AppSidebar body (hidden while Settings/Tools body shows); `mobileHosted` mode on AppSidebar/SectionSidebar; 220 ms close hold preserved. - D3: `pendingInteraction` prop on FollowUpPromptBox keeps the editor shell mounted and hidden, interaction as last stack item, footer pickers read-only. - D5 (reverted, see below): static 48 px compact row on compact + coarse pointer; editor realizes after paint (idle/timeout, transition) or at first tap (pointerdown mounts under an overlay, click focuses via `focusEndForTap`); Stop/voice/drafted submit work from the row; #1771 handoff untouched. - J3: prune once per page load from idle, lastUsedAt checked before schema parse, no-op storage writes skipped. - J4: `HEAVY_PAYLOAD_QUERY_POLICY` (60 s gc) on turn-summary details and file previews; diff patches use a reader lease with a 60 s post-unmount eviction (observer-less entries no longer gc while shown). - I6: bundled GitHub glyph + theme-token status dot; logo via the shared icon. - B28: composer renders immediately with a loading project picker; projectId-keyed queries gate on the settled bootstrap. ## How you verified - `pnpm exec turbo run typecheck --filter=@bb/app`: pass. - `pnpm exec turbo run lint --filter=@bb/app`: 0 errors (147 pre-existing warnings, unchanged count). - `pnpm exec turbo run test --filter=@bb/app --force`: 364 files, 2906 passed, 3 skipped (one registry boundary test fixed in the last commit and re-run green). - New/updated tests: compact-long-press-menu.test.tsx, useSidebarReorderDnd.test.tsx (sensor install/remove), sidebar.test.tsx (passive vs non-passive registration), AppLayoutSidebar.test.tsx (single panel + single mount across app -> settings -> tools -> app), FollowUpPromptBox.test.tsx (editor DOM identity across pending interaction; deferred compact editor lifecycle), ThreadDetailPromptArea.test.tsx (composer retained + stack ordering), fixed-panel-tabs-sync.test.ts (no rewrite, prune decisions), use-environment-diff-patches.test.tsx (retention lease), PluginNewThreadComposer.test.tsx (root composer before settle, gated queries). Fixes: mobile perf sweep findings E1, E6, E7, E10, D3, D5 (reverted, see below), J3, J4, I6, B28. ## Update (2026-08-19) The deferred compact follow-up editor (D5: static stand-in row + pointerdown/click focus handoff) was removed from this PR after a device test on iPhone showed the caret rendering above the composer after the first tap and a ~11 px footer shift when the real editor replaced the stand-in. The compact composer keeps the previously verified always-mounted behavior (#1263, #1381, #1771). D5 stays open as a follow-up once it can be verified on iOS. Fixes: part of the mobile / iOS Safari performance program (verified sweep report in the bb thread; no single issue). ## Stack context Layer 21 of 22 in the `bb/mobile-perf/*` stack (bottom → top: quick wins first, big rocks last). - Prerequisite (layer below): `bb/mobile-perf/markdown-streaming-and-containment` (#1899). - Next layer: `bb/mobile-perf/diff-and-file-preview` (#1901). - Audit findings addressed: see title IDs. Review: approved-with-fixes; 5 review fix commit(s). - Deliberately not done here: D5: reverted after an iPhone test showed a mispositioned caret and a footer shift; needs a device-verified handoff before it returns - Reviewer notes / follow-ups: Device/iOS Simulator verification still needed for: D5 tap handoff (pointerdown-cancel + click focus with real TipTap; jsdom mocks PromptBoxInternal), E7 deep-content passive swipe | E1: ThreadActionsContextMenu/ProjectActionsContextMenu now branch on useIsCompactViewport, so crossing the compact breakpoint (tablet rotation, window resize) remounts every row's | J4: diff patches now refetch after >60 s away from a thread; turn-summary details/file previews gc after 60 s without observers (by design per audit). - Wire/contract: None. No server/daemon wire, protocol, CLI, plugin API, or DB changes; HOST_DAEMON_PROTOCOL_VERSION untouched. All changes are in apps/app. > AGENT GENERATED: by Claude Code (claude-mangosteen-eap) --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
Diff-tab text cards fetched the full old and new file for every modified card as soon as it came within 200 px of the viewport, only so pierre could show expand-context buttons; the enriched diff was then tokenized and rendered a second time. The file preview rendered whole files un-virtualized with no size cap, remounted pierre when the highlighted AST arrived (throwing away DOM and scroll position), and re-encoded every text file to a base64
data:URL that only ever fed a cache key.What changed
data:URL only for image/video; text previews carry the/diff/fileroute URL.How you verified
DiffFileCard.contextExpansion.test.tsx(coarse pointer: no fetch on reveal, fetch on click, affordance retires; fine pointer: idle auto-fetch; error -> Retry; added file never fetches — 3 of 4 fail before),FilePreview.test.tsx(single mount on highlight, 5,000-line cap + Load full file +:headcacheKey, 512 KB cap, line link past the cap shows whole file, target-line scroll on the virtualized viewport),environment-queries.test.tsx(text preview has no data URL; image does).pnpm exec turbo run typecheck --filter=@bb/app,pnpm exec turbo run lint --filter=@bb/app(pre-existing warnings only),pnpm exec turbo run test --filter=@bb/app -- src/components/secondary-panel src/components/git-diff src/hooks/queries src/lib/file-preview.test.ts(39 files, 256 tests pass).Fixes: part of the mobile / iOS Safari performance program (verified sweep report in the bb thread; no single issue).
Stack context
Layer 22 of 22 in the
bb/mobile-perf/*stack (bottom → top: quick wins first, big rocks last).bb/mobile-perf/mobile-interactions(Phone interactions: long-press menu, touch listeners, sidebar across routes, composer retention (E1, E6, E7, E10, D3, J3, J4, I6, B28) #1900).maxBytesparam on /environments/:id/diff/file: not added. The route was not touched (the fix is on-demand fetching in the client), and a real capGitDiffCard(timeline/thread surface) never passespatchTexttouseGitDiffCardBody, so its text cards never get context expansion (before this bra | Non-blocking: on fine pointers, when a text card's patch identity changes while contents are already loaded, the fetch effect briefly starts and cancels one fetch before the idle r | Not done by design (per implementer): themaxBytesroute param from G2 was skipped because it needs a daemon change; no wire changes on this branch, so no HOST_DAEMON_PROTOCOL_VE | Small UX change on desktop worth knowing: code previews now own their scroller (usesFullHeightLayout), so the file header stays fixed while code scrolls, matching iframe/CSV prev