Repository navigation
Conversation
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This change replaces hand-wired Preview/Diff integration with new shared registry and host abstractions across ChatView, panel launchers, and panel lifecycles. Although tests cover the refactor and visible behavior is intended to remain unchanged, the cross-cutting production restructuring warrants human review. You can add or adjust custom eligibility rules. Learn more. |
saphid
force-pushed
the
stack/02-panel-host
branch
3 times, most recently
from
October 6, 2026 16:03
ea5c743 to
bce155d
Compare
Contributor
Author
|
Review requested
Logged so this PR shows when a maintainer was asked to review it. |
saphid
force-pushed
the
stack/02-panel-host
branch
2 times, most recently
from
October 7, 2026 08:51
8d5196e to
02a5811
Compare
saphid
force-pushed
the
stack/02-panel-host
branch
from
October 10, 2026 02:13
897efdf to
a60d8ba
Compare
saphid
force-pushed
the
stack/02-panel-host
branch
from
October 10, 2026 04:05
a60d8ba to
f8c9fce
Compare
Preview becomes the second panel on the side-panel registry that Diff started. Each definition now also carries the panel's title, icon, launcher letter, client support and unavailable copy, so the tabs, the empty launcher and the add menu read one ordered list instead of three hand-kept ones. Labels, letters, order and copy are unchanged. Panel props are inferred from each lazily loaded body, and the caller is a closed union, so another panel's props, unknown ids and widened ids do not compile. ChatView lends the rendered panel a small host (thread, right panel visibility, composer draft target, workspace mutation id and the annotation send) instead of drilling the same props into each body; the annotation send keeps the per-render closure it had before, and PreviewView still drops a pick that settles after a thread switch. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ChatView built a new PanelHost on every render, so every usePanelHost consumer re-rendered even when no host field changed. Memoize it on its fields and send annotations through onSendRef so the sender stays stable. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Plain server threads reuse one ChatView, so the memoized panel host's sender could resolve to the next thread's composer when a pick settled after a switch. The latest sender now carries its thread key, and each host forwards only to a sender for its own thread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ChatView replaced the panel host's annotation sender while rendering. If React threw that render away, an in-flight preview pick could still call its onSend, for example one that edits a queued message instead of sending a turn. Update the sender in a layout effect so only committed renders lend it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
saphid
force-pushed
the
stack/02-panel-host
branch
from
October 10, 2026 07:06
f8c9fce to
16877e3
Compare
This branch has not been deployed
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
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.
Stacked on #15010. Review only the top 4 commits: 16877e3.
Problem
After #15010, Diff mounts through the side-panel registry, but Preview (the Browser panel) still has its own hand-written lazy import and prop wiring in
ChatView. The tab strip, the empty launcher and the + menu also each keep their own hand-written list of surfaces, letters and "unavailable" copy, so Browser and Diff are described in four places. Adding or changing a panel means keeping all of them in step. This PR moves Preview onto the registry and lets the registered panels describe themselves once, with no visible change.Why this qualifies
This is the second of the two PRs proposed in Ideas discussion #14938 (internal panel host, following #1377). No maintainer has agreed to that direction yet. It is stacked on #15010 and depends on it; review that first. If #14938 is declined, close both.
Fix
panels/panelRegistry.ts: a definition now also carriestitle,icon,launcherKey, optionalisSupported, and the short and long unavailable copy. Each panel's props are inferred from its lazily imported body, so there is no hand-written props map.get(id)returns the samelazy()component for every lookup and is typed with that id's props. It throws for an unregistered id.panels/bundledPanels.tsxregisters Diff and Preview.<RegisteredSidePanel>takes a closed union, so another panel's props, an unknown id or a widened id do not compile. It owns the<Suspense fallback={null}>thatChatViewused to write around each panel.components/preview/PreviewPanel.tsxmoves topanels/preview/PreviewSidePanel.tsx.ChatViewonly ever usedmode="embedded", so themodeprop is removed. The desktop-only copy and the optionaltabIdhandling are unchanged.panels/panelHost.ts:ChatViewlends the rendered panel{ threadRef, visible, composerDraftTarget, workspaceMutationId, sendAnnotation }through context. That is exactly what Diff and Preview were given as props before, and each field has a consumer here (Diff reads the draft target and workspace mutation id; Preview reads the thread, visibility and annotation send).sendAnnotationis the same per-render closure overonSendas before, so an element pick that settles after navigating away still sends from the thread where it started.RightPanelTabs.tsx:rightPanelSurfaceActions()builds one ordered list (B T F D P L M) for both the empty launcher and the + menu, replacing two inline lists and two copy maps. Browser and Diff take their label, icon, letter, availability and copy from their definitions. The Diff tab title and icon and the Browser fallback title and icon read the same source. Browser's availability is stillisPreviewSupportedInRuntime(), now throughisSupported, combined with the thread's own availability. The launcher callsonOpen()without arguments, so a click event can never be read as a browser profile id.diff.toggle,preview.toggle), storage keys, launcher letters, order and all copy are unchanged. Stores, queries, server, contracts, desktop IPC/CSP and mobile are untouched.Size: 14 files, +959 / −322. Production code is +351 / −313 (+38 net); the rest is tests.
Evidence
Environment: macOS 26.5.2 arm64. Before is #15010 rebased on main (5981e74cdf); after is this head (8b92a7f06b). Each revision ran on its own fresh, isolated T3 home with the same disposable fixtures: a Git repo on
feature/panelwith one commit and one uncommitted edit, a non-Git folder, and a local static page. Web: Vite dev client in headless Chromium at 1440×1000. Desktop: each revision's built Electron app (vp run build:desktop) at 1440×1000. The theme is set through Settings → Appearance. The providers were not signed in, so the annotation turn was stopped as soon as the message landed.Reproduction (same steps on both):
#hero, add a note and send it. Then start another pick in A, switch to Thread B, send a late click to A's page, and switch back to A.Observed before and after:
#heroannotation lands as a user message in Thread A only, and Thread B stays empty. Switching to B cancels the pending pick, and a late click sent to A's retained page adds nothing to either thread. Back in A, there is still exactly one annotation. The behaviour is the same on both revisions.greet.tsand README), closes, reopens with ⌘D, and shows README only (+2 −0) in Uncommitted. All of these pairs are identical on web and desktop.PreviewViewor the in-page annotation tool. The ninth pair (light, empty pull-requests page) differs by 19 pixels on the Sort icon and has no panel.Before/after images are stacked below. Click a heading for the set.
Recording: Browser open via launcher → close → reopen, profile chooser → Incognito (desktop, dark, real time)
Before (#15010):

After (this PR):

MP4: before · after
Recording: annotation sent from Thread A, then a pick started in A and abandoned by switching to Thread B (desktop, dark, real time)
Before (#15010):

After (this PR):

MP4: before · after
Recording: Diff open → close → reopen with ⌘D (web, dark, real time)
Before (#15010):

After (this PR):

MP4: before · after
Empty launcher, Git thread (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Empty launcher, Browser unavailable on web
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Empty launcher, non-Git thread: Diff unavailable hint (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

+ menu (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

+ menu: Browser tooltip on web
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

+ menu, non-Git thread: Diff tooltip (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Launcher Browser profile chooser (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

+ menu Browser profile submenu (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Incognito Browser tab (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Browser loading fallback title and icon (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Browser with the fixture page loaded (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Annotation: #hero picked in Thread A (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Annotation sent: message in Thread A (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Late pick: Thread B after switching away from a pick in A (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Late pick: back in Thread A, still one annotation (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Diff open on branch changes (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Diff closed (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Diff reopened with ⌘D (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Diff, Uncommitted scope (desktop)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Pull-requests page: + menu with Browser and Diff unavailable (web)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Pull-requests page: Diff tooltip (web)
Before (#15010), light:

After (this PR), light:

Before (#15010), dark:

After (this PR), dark:

Pixel-diff images for the 9 non-zero desktop pairs are published next to these captures as
diff-electron-<theme>-<state>.png.Checks at this head (
8b92a7f06b), re-run 2026-10-05 (CI=true, all exit 0):vp test run apps/web/src/components/RightPanelTabs.test.tsx apps/web/src/components/RightPanelTabs.browserProfile.test.tsx apps/web/src/panels apps/web/src/components/preview/PreviewView.test.tsx: 6 files, 40 tests pass.panels/preview/PreviewSidePanel.test.tsxdrives the realRegisteredSidePanel→PreviewSidePanel→PreviewViewpath, stubbing only the desktop bridge, stores and leaf chrome. Re-rendering with a new host for the same thread keeps an in-flight element pick and does not remount Preview. Switching to another thread also keeps the one mount, and a pick that resolves after the switch still goes to the original thread's sender and draft.RightPanelTabs.browserProfile.test.tsxclicks the empty launcher's Browser row, which opens the default profile, then picks Incognito from the profile chooser, which opens that profile.panelRegistry.test.tsx: duplicate ids rejected without loading; nothing loads until first render, one mount across lookups; two panels with different props load and dispose independently and keep stable lazy identities.bundledPanels.test.tsx: the real registry loads only the selected body (Preview, not Diff) and lends it the host.rightPanelSurfaceActions, thepanelHostmodule, and thepanels.previewlauncher prop. The Preview lifecycle and late-pick behaviour is not new. The parent already had it throughPreviewPanel, and these tests show it survives the move.vp run --filter @t3tools/web typecheckpasses. Recorded during development: the compile-only fixtures (@ts-expect-error: Preview props on Diff,threadRefpassed as a prop, wrong input type, unknown id, widened id with Preview props, missing and foreign props on the registry) produce 18 type errors against the parent.vp lint --report-unused-disable-directivesandvp fmt --checkon the touched files: pass. Lint warnings match the parent for the same files, exceptRightPanelTabs.tsxgoes from 1 to 0.vp run knip:checkpasses.vp run --filter @t3tools/web build(emits separateDiffSidePanelandPreviewSidePanelchunks),vp run build:desktopandnode scripts/release-smoke.tspassed on the previous revision of this PR, which differs only in tests, and all three passed again at the top of this stack (the example plugins PR), which contains this change.Surfaces
apps/web.Not verified
--sharepass.apps/web.Claude Opus 5.5 (build), GPT-6.1 Sol (review) and GPT-6 Astra (captures) via T3 Code
🤖 Generated with Claude Code