feat(desktop): make the agent work viewer a peer of the thread surface - #6535
feat(desktop): make the agent work viewer a peer of the thread surface#6535baxen wants to merge 2 commits into
Conversation
The channel agent activity pane was a read-only side pane with no share of the thread surface's presentation: it could not take the focus-mode drawer, had no layout control, and kept its own back arrow as the only way out. That made the viewer feel like a subordinate inspector rather than a place you read an agent's work. Route the viewer through the same focus drawer the thread uses. One resolver (`resolveFocusDrawerSurface`) decides the drawer's occupant using the same precedence as ChannelPane's auxiliary render chain, so the drawer and the split pane cannot disagree about who is on screen. `useFocusDrawerSurfaces` owns the state that follows from that resolution — drawer presence, which surface receives dismissal, and a layout toggle per surface — keeping ChannelPane under the file-size ratchet. User-visible: the viewer gets the layout toggle and honours the shared `threadViewMode` preference (choosing a layout here chooses it for threads too), gets a centered reading column in focus mode, and is dismissed by the drawer's scrim sliver or Escape instead of a competing header arrow. The read-only boundaries are unchanged: no composer, scope label stays "Activity · #<channel>". Reading position is deliberately not preserved across a layout switch — the viewer snaps to latest. Transcript rows use `content-visibility: auto` with an over-reserved `contain-intrinsic-size`, so the scroll height shrinks as rows paint after the reflow and a restored offset slides out from under the reader (instrumented: a correct pin at scrollTop 1122 was then chased to 0 by late resize corrections). Snapping is the behaviour we can actually keep until transcript rows reserve their real heights. Signed-off-by: ss-dev-00 <a02c4e0850e5e612b4ddf95dbe2f5c56467cf27c6552203bc833ff438fb31971@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 096a31f8d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const wrapAgentSessionPanel = (panel: React.ReactNode) => | ||
| useFocusAgentDrawer ? ( | ||
| <FocusThreadDrawer |
There was a problem hiding this comment.
When a user selects the Raw activity view and then uses the new layout toggle, this conditional replaces FocusThreadDrawer with RightAuxiliaryPane/Fragment. React identity depends on element type as well as key, so the shared key does not preserve the nested AgentSessionThreadPanel; it remounts and initializes rawFeedState.show to false, unexpectedly returning the user to the transcript view. Keep the panel at a stable component position or lift its local view state across the container switch.
Useful? React with 👍 / 👎.
| <FocusThreadDrawer | ||
| ariaLabel="Agent activity" | ||
| channelName={activeChannel?.name ?? "channel"} | ||
| key={AGENT_SESSION_SURFACE_KEY} | ||
| onClose={onCloseAgentSession} |
There was a problem hiding this comment.
For a live agent in focus mode, pressing Escape while the activity settings dropdown is open closes the entire agent viewer rather than only dismissing the menu. Wrapping this panel in FocusThreadDrawer installs its unconditional window-capture Escape handler, which runs before Radix's dropdown handler can prevent the event; this bypasses the nested-control behavior that useEscapeKey normally preserves. The drawer should defer to an open nested surface before invoking onCloseAgentSession.
Useful? React with 👍 / 👎.
Give the shared layout control surface-specific accessible names and cover the viewer's centered reading measure and URL-backed history behavior. Co-authored-by: ss-quality-00 <75789fffd704a5265cd65462cfc263f44561872fbebcea3a131a2a5f511465d2@buzz.block.builderlab.xyz> Signed-off-by: ss-quality-00 <75789fffd704a5265cd65462cfc263f44561872fbebcea3a131a2a5f511465d2@buzz.block.builderlab.xyz>
1 participant
What
Makes the channel agent work viewer a first-class peer of the thread surface instead of a subordinate read-only side pane.
Before: the activity pane could not take the focus-mode drawer, had no layout control, and its header back arrow was the only way out.
After:
FocusThreadDrawerthe thread uses (aria-label="Agent activity"), dimming and inerting the channel behind it.threadViewModepreference — choosing a layout in the viewer chooses it for threads too.Read-only boundaries are unchanged by design: no composer, scope label stays
Activity · #<channel>.How
focusDrawerSurface.ts— one resolver for the drawer's occupant, with precedence deliberately mirroringChannelPane's auxiliary render chain (management → thread → agent session) so the drawer and the split pane cannot disagree about who is on screen.useFocusDrawerSurfaces.ts— owns what follows from that resolution: drawer presence, which surface receives dismissal, and a view-mode switch per surface. Extracted soChannelPanestays under the 1000-line ratchet.useThreadViewModeSwitchgains abodySelector(so a surface reads its own scroll region) and apreserveReadingPositionflag.AgentSessionThreadPanelgainscolumnMaxWidthPx,headerLeading, andisFocusMode(Escape ownership, no back arrow,enterMotion={false}so the panel's slide does not compound with the drawer's).Accepted limitation: the viewer snaps to latest on a layout switch
Reading position is not preserved across focus↔split in the viewer, and that is deliberate. Transcript rows use
content-visibility: autowithcontain-intrinsic-size: auto 200px, which over-reserves height for unpainted rows. After the layout change the content height shrinks as rows paint, and a pinned-center repin chases the shrinking coordinate: instrumented traces showed a correct pin atscrollTop 1122then a lateResizeObservercorrection ofscrollBy(0, -720)driving it to 0. The non-pinned message-anchor path drifted too.Snapping to latest is the behaviour we can actually keep until transcript rows reserve their real heights. This is a real product boundary worth naming: a future conversational surface with a composer will hit the same anchoring limits.
Verification
At
096a31f8d, rebased on074561233:pnpm exec tsc --noEmitclean;biome check src testsclean (2 pre-existing warnings elsewhere);node scripts/check-file-sizes.mjspasses.pnpm test— 5361 passed, 0 failed (includes the newfocusDrawerSurface.test.mjs).agent-viewer-focus-mode.spec.tsplusthread-focus-mode,activity-scope-label-screenshots,observer-feed-screenshots,threadpane-ultrawide— 19 passed.The new spec asserts the shared drawer + dismissal path (drawer visible and labelled, channel inert, no
agent-session-back, scope label unchanged, no composer, Escape/scrim/sidebar dismissal all reaching the viewer rather than a thread) and that narrow viewers do not offer an unavailable layout switch.