feat(agent): V4 Ambient conversation redesign#769
feat(agent): V4 Ambient conversation redesign#769Nikhil (shadowfax92) wants to merge 3 commits intodevfrom
Conversation
Restructures the sidepanel conversation route into a full-bleed editorial layout per the V4 Ambient spec: floating top overlay, editorial serif title that scrolls with content, centered 760px thread, and a floating pill composer. The `/history` route keeps its existing chrome header. - New `ambient/ChatOverlayBar` replaces the fixed ChatHeader on `/` with a floating strip (home, state-aware status pill, new). - New `ambient/StatusPill` absorbs provider + mode selectors and nav actions into a single state-aware popover (idle/running/error), with an elapsed timer while the agent is streaming. - New `ambient/ChatComposerFloating` replaces the in-flow ChatFooter with a 720px floating pill containing the sparkle icon, input, and a compact tabs/workspace/apps meta row. - New `ambient/EditorialHeader` renders the agent name in Instrument Serif (44px / 32px narrow) above the first turn, scrolling with content. - Self-hosted Instrument Serif via `@font-face` in `public/instrument-serif/`; falls back to `Iowan Old Style, Georgia, serif` if the woff2 binary isn't present in the bundle (see `public/instrument-serif/README.md`). - Restyled ToolBatch to the uppercase "N tool calls" card with mono tool rows and orange check icons. - Restyled Reasoning content to a soft muted italic box. - `ConversationScrollButton` gains an `offsetBottom` prop so the scroll-down button clears the floating composer. No data-layer or streaming changes. Voice input, @ mention, tool approvals, selected-text pills, JTBD popup, feedback actions all preserved.
- StatusPill: control outer popover open/close so selecting a provider or mode doesn't dismiss through nested Radix popovers. - Chat: replace useMemo with useState lazy-init for the display session hash so it's stable under strict-mode/concurrent rerenders. - ChatComposerFloating: skip the focus-refocus effect when any Radix popover is open to avoid stealing focus mid-interaction. - ChatMessages: give the ambient streaming indicator a reserved min-height so toggling it doesn't jump the scroll anchor; drop the dead <div /> sentinel; replace the Fragment around AmbientUserTurn with a direct key. - Chat: extract the duplicated floating-error-banner wrapper to a helper.
|
Greptile (@greptileai) review |
|
Claude (@claude) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 2m 2s —— View job Code Review: V4 Ambient Conversation Redesign
Overall this is a well-structured PR. The component decomposition into Bugs1. Overlapping error banners [ If both // Consider merging into one banner, using the first non-null error:
const displayError = agentUrlError ?? chatError
{displayError && (
<FloatingErrorBanner>
<ChatError error={displayError} providerType={selectedProvider?.type} />
</FloatingErrorBanner>
)}2. Redundant font declaration in The // Before
className="font-serif text-[32px] ..."
style={{ fontFamily: 'var(--font-serif)' }}
// After
className="font-serif text-[32px] ..."3. Duplicate There is already a /* Lines 413-419 — safe to delete, fully covered by lines 186-195 */
@layer base {
* {
@apply border-border outline-ring/50;
}
body {
@apply bg-background text-foreground;
}
}Minor Issues4. This is a subtle correctness fix for the popover race condition mentioned in the PR description, but there's no in-code explanation. A future reader will likely remove it thinking it's unnecessary. const handleSelectProvider = (p: Provider) => {
onSelectProvider(p)
// Defer close by one microtask so the provider selector's own onOpenChange
// fires first — prevents the outer Popover from closing prematurely.
queueMicrotask(close)
}5. Magic number coupling between
6. selectedTextStorage.getValue().then((map) => {
const { [key]: _, ...rest } = map
selectedTextStorage.setValue(rest)
})This read-then-write pattern is racy if the storage changes between 7. Hardcoded The Observations (No Action Required)
The
What's Well Done
|
✅ Tests passed — 787/790
|
Greptile SummaryThis PR introduces the V4 Ambient conversation redesign for the BrowserOS sidepanel — replacing the fixed Confidence Score: 5/5Safe to merge; all findings are P2 style/layout polish items with no data-loss or runtime-breaking risk. No P0 or P1 issues found. The three P2 items (error banner offset, mobile padding, formatInputPreview) are cosmetic and edge-case layout concerns that don't break any primary user path. The refactor correctly gates the new layout behind the route check and preserves all existing feature hooks. ToolBatch.tsx (formatInputPreview raw input exposure) and Chat.tsx (FloatingErrorBanner bottom offset) could use a quick pass before the next release. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
ChatLayout -->|pathname === '/'| Chat
ChatLayout -->|pathname !== '/'| ChatHeader
Chat --> ChatOverlayBar
Chat --> main
Chat --> ChatComposerFloating
ChatOverlayBar --> HomeBtn["Home button"]
ChatOverlayBar --> StatusPill
ChatOverlayBar --> NewBtn["New conversation button"]
StatusPill --> Popover["Popover: Provider · Mode · History · Settings · Theme"]
main --> ChatMessages
ChatMessages --> ConversationContent
ConversationContent --> EditorialHeader
ConversationContent --> AmbientEmptyStateSuggestions
ConversationContent --> AmbientUserTurn
ConversationContent --> MessageResponse["MessageResponse (assistant text)"]
ConversationContent --> ReasoningContent["ReasoningContent (collapsible italic block)"]
ConversationContent --> ToolBatch["ToolBatch (uppercase card + mono rows)"]
ConversationContent --> AmbientStreamingIndicator
Chat --> FloatingErrorBanner["FloatingErrorBanner (absolute, bottom-130px)"]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/Chat.tsx
Line: 302-304
Comment:
**Hardcoded error banner offset may overlap the composer**
`bottom-[130px]` is computed assuming the composer pill is roughly 110px tall. When a selected-text card or multiple attached-tab rows are visible, the composer pill grows taller and the error banner will render on top of it, making both unreadable. Consider using a larger, safer offset or measuring the composer height dynamically.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ChatMessages.tsx
Line: 77-79
Comment:
**Top padding may slightly clip below overlay bar on mobile**
The ambient `ConversationContent` starts with `pt-10` (40 px) on mobile, but `ChatOverlayBar` is absolutely positioned at `top-4` (16 px) with button elements roughly 26–30 px tall — putting its bottom edge at ~42–46 px. The first scroll item will be partially hidden under the overlay. On `sm` breakpoints the explicit `sm:pt-[72px]` clears it correctly. Consider bumping the mobile value by a few pixels to `pt-14`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ToolBatch.tsx
Line: 558-565
Comment:
**Raw tool input values may expose sensitive data in UI**
`formatInputPreview` returns the first string value of the tool's input object as-is. Some tools pass credentials, auth tokens, or full file contents as string parameters. These will now appear in plain text in the mono row visible to anyone looking at the sidepanel. Consider truncating at a safe length or allowlisting which input keys to preview (e.g., `url`, `query`).
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/Chat.tsx
Line: 319
Comment:
**Suggestion button React key relies on display text**
`s.display` is a user-facing label that could be duplicated across mode variants, causing React to silently reuse the wrong node. A stable identifier (e.g. the index) is safer here.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(agent): address review comments for ..." | Re-trigger Greptile |
| <div className="pointer-events-none absolute inset-x-0 bottom-[130px] z-20 flex justify-center px-6"> | ||
| <div className="pointer-events-auto w-full max-w-[720px]">{children}</div> | ||
| </div> |
There was a problem hiding this comment.
Hardcoded error banner offset may overlap the composer
bottom-[130px] is computed assuming the composer pill is roughly 110px tall. When a selected-text card or multiple attached-tab rows are visible, the composer pill grows taller and the error banner will render on top of it, making both unreadable. Consider using a larger, safer offset or measuring the composer height dynamically.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/Chat.tsx
Line: 302-304
Comment:
**Hardcoded error banner offset may overlap the composer**
`bottom-[130px]` is computed assuming the composer pill is roughly 110px tall. When a selected-text card or multiple attached-tab rows are visible, the composer pill grows taller and the error banner will render on top of it, making both unreadable. Consider using a larger, safer offset or measuring the composer height dynamically.
How can I resolve this? If you propose a fix, please make it concise.- FloatingErrorBanner: lift to bottom-[220px] so the error stays above the composer when attached-tab or selected-text rows expand the pill height. - ChatMessages: bump ambient mobile top padding to pt-14 so the first scroll item clears the overlay bar bottom edge (~46px). - ToolBatch.formatInputPreview: allowlist safe-to-show keys (url, path, query, name, etc.) and truncate at 80 chars so credentials or page text passed through arbitrary tool inputs don't leak into the UI preview. - Chat: key suggestion buttons by prompt (more unique) instead of display.
Summary
Restructures the BrowserOS Agent sidepanel conversation route into the V4
Ambient layout — a full-bleed editorial treatment per the user-provided spec.
the fixed
ChatHeaderon/. The pill consolidates provider selection,mode toggle, and session controls into a single popover.
the first scroll item, above the turns, with a subtitle row showing mode,
session hash, and turn count.
in-flow
ChatFooter. Compact meta row underneath retains tab attachment,workspace, and MCP app selectors.
a muted italic soft-bg block; tool batches are an uppercase "N tool calls"
card with mono rows and orange check icons; assistant text keeps the
existing Streamdown rendering at 14.5px / 1.65 line-height.
/history) is untouched — still uses the existingChatHeader. Layout is conditional inChatLayout.Design
Keeps
use-stick-to-bottomas the sole scroll owner; the editorial headerand streaming indicator render inside
ConversationContentso auto-scrollstill works.
pointer-events: noneon the overlay / composer wrappers soclicks fall through to the thread gutters.
ConversationScrollButtongetsan
offsetBottomprop to clear the floating composer.Self-hosted Instrument Serif via
@font-facereadingpublic/instrument-serif/InstrumentSerif-Regular.woff2. Until the binary isdropped in (see
public/instrument-serif/README.md), the title falls backto
Iowan Old Style, Georgia, serif— visually acceptable and non-blocking.No data-layer, streaming, or session-storage changes. Voice input, @ mention,
tool approvals, selected-text pills, JTBD popup, and feedback actions are all
preserved.
Test plan
elapsed timer) → back to idle.
mode toggle, new/history/settings/theme. Switching providers and modes
no longer race-closes the outer popover.
"N tool calls" card renders with mono rows.
collapsible with the V4 soft muted italic box styling.
the textarea without clipping against the floating composer.
the composer pill.
still appear in the tool batch.
and composer shadow softens.
Follow-ups
(curl command in the README).
exposes a planner-level progress stream (currently we only show elapsed).
🤖 Generated with Claude Code