Skip to content

feat(agent): V4 Ambient conversation redesign#769

Closed
Nikhil (shadowfax92) wants to merge 3 commits intodevfrom
feat/agent-command-center-redesign-2
Closed

feat(agent): V4 Ambient conversation redesign#769
Nikhil (shadowfax92) wants to merge 3 commits intodevfrom
feat/agent-command-center-redesign-2

Conversation

@shadowfax92
Copy link
Copy Markdown
Contributor

Summary

Restructures the BrowserOS Agent sidepanel conversation route into the V4
Ambient layout — a full-bleed editorial treatment per the user-provided spec.

  • Floating top overlay (home · state-aware status pill · new) replaces
    the fixed ChatHeader on /. The pill consolidates provider selection,
    mode toggle, and session controls into a single popover.
  • Editorial serif title (Instrument Serif, 44px / 32px narrow) renders as
    the first scroll item, above the turns, with a subtitle row showing mode,
    session hash, and turn count.
  • Floating composer pill (720px, shadow, sparkle prefix) replaces the
    in-flow ChatFooter. Compact meta row underneath retains tab attachment,
    workspace, and MCP app selectors.
  • Turn restyle — ambient user turn is an avatar + text row; reasoning is
    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 route (/history) is untouched — still uses the existing
    ChatHeader. Layout is conditional in ChatLayout.

Design

Keeps use-stick-to-bottom as the sole scroll owner; the editorial header
and streaming indicator render inside ConversationContent so auto-scroll
still works. pointer-events: none on the overlay / composer wrappers so
clicks fall through to the thread gutters. ConversationScrollButton gets
an offsetBottom prop to clear the floating composer.

Self-hosted Instrument Serif via @font-face reading
public/instrument-serif/InstrumentSerif-Regular.woff2. Until the binary is
dropped in (see public/instrument-serif/README.md), the title falls back
to 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

  • Run `bun run dev` and open the BrowserOS sidepanel.
  • Confirm the editorial title renders at the top of a new conversation.
  • Submit a prompt; confirm status pill transitions idle → Running (with
    elapsed timer) → back to idle.
  • Click the status pill; confirm the popover shows provider selector,
    mode toggle, new/history/settings/theme. Switching providers and modes
    no longer race-closes the outer popover.
  • Submit a prompt that triggers tool calls; verify the new uppercase
    "N tool calls" card renders with mono rows.
  • Submit a prompt that triggers thinking; verify the reasoning block is
    collapsible with the V4 soft muted italic box styling.
  • Attach a tab via `@`; verify the mention popover still anchors under
    the textarea without clipping against the floating composer.
  • Start voice recording; confirm the red waveform overlay renders inside
    the composer pill.
  • Trigger a tool approval flow; confirm the inline approve/deny buttons
    still appear in the tool batch.
  • Resize the sidepanel to ~400px; confirm serif title shrinks to 32px
    and composer shadow softens.
  • Navigate to `/history`; verify the old `ChatHeader` is still shown.
  • Toggle light / dark theme; verify all ambient components read correctly.

Follow-ups

  • Drop `InstrumentSerif-Regular.woff2` into `public/instrument-serif/`
    (curl command in the README).
  • Add a real step-count signal to the status pill once the agent backend
    exposes a planner-level progress stream (currently we only show elapsed).

🤖 Generated with Claude Code

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.
@shadowfax92
Copy link
Copy Markdown
Contributor Author

Greptile (@greptileai) review

@shadowfax92
Copy link
Copy Markdown
Contributor Author

Claude (@claude) review

@claude
Copy link
Copy Markdown

claude Bot commented Apr 20, 2026

Claude finished Nikhil (@shadowfax92)'s task in 2m 2s —— View job


Code Review: V4 Ambient Conversation Redesign

  • Gather context and read changed files
  • Review new ambient components
  • Review modified files
  • Check for bugs, performance issues, and accessibility concerns
  • Provide comprehensive review feedback

Overall this is a well-structured PR. The component decomposition into ambient/ is clean, the pointer-events: none overlay pattern is correct, and the variant prop on ChatMessages is a non-breaking extension. A few issues worth addressing below.


Bugs

1. Overlapping error banners [Chat.tsx:265-280]

If both agentUrlError and chatError are truthy simultaneously, two FloatingErrorBanner components render at the same absolute position (bottom-[130px]), visually overlapping each other. Even if the API guarantees they're mutually exclusive today, this is fragile.

// Consider merging into one banner, using the first non-null error:
const displayError = agentUrlError ?? chatError
{displayError && (
  <FloatingErrorBanner>
    <ChatError error={displayError} providerType={selectedProvider?.type} />
  </FloatingErrorBanner>
)}

Fix this →


2. Redundant font declaration in EditorialHeader [EditorialHeader.tsx:22-24]

The font-serif Tailwind class already resolves to font-family: var(--font-serif) (set in global.css @theme inline). The additional inline style={{ fontFamily: 'var(--font-serif)' }} is redundant and would override the class without effect. Remove the inline style.

// Before
className="font-serif text-[32px] ..."
style={{ fontFamily: 'var(--font-serif)' }}

// After
className="font-serif text-[32px] ..."

Fix this →


3. Duplicate @layer base block [global.css:413-419]

There is already a @layer base block at lines 186–195 that applies border-border outline-ring/50 and body styles. The second block at lines 413–419 is a duplicate and adds nothing.

/* 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;
  }
}

Fix this →


Minor Issues

4. queueMicrotask needs a comment [StatusPill.tsx:60]

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 FloatingErrorBanner and ChatComposerFloating [Chat.tsx:302]

bottom-[130px] in FloatingErrorBanner is implicitly coupled to the floating composer's height. If the composer grows (e.g., an attached tab row is added), the banner will overlap it. Consider a CSS variable or deriving the offset from the same source.

6. selectedTextStorage TOCTOU in dismiss handler [ChatComposerFloating.tsx:116-123]

selectedTextStorage.getValue().then((map) => {
  const { [key]: _, ...rest } = map
  selectedTextStorage.setValue(rest)
})

This read-then-write pattern is racy if the storage changes between getValue and setValue. If the API supports a functional updater (like setState(prev => ...)) that would be safer. If not, this is low-risk given the flow, but worth a note.

7. Hardcoded "U" avatar initial [AmbientUserTurn.tsx:9]

The "U" fallback is fine for an MVP, but it's worth a follow-up task to use the actual user's first initial once profile data is available in context.


Observations (No Action Required)

sm: breakpoints in a sidepanel context [ChatComposerFloating.tsx:232, ChatMessages.tsx:78]

The sm: breakpoint (768px default) will almost never fire in a Chrome extension sidepanel at normal widths. Lines like sm:inline (hint text) and sm:px-8 (padding) are effectively dead styles. They won't cause bugs, but they're unused styles in practice.

ConversationScrollButton translation [conversation.tsx:108-109]

right-[16px] translate-x-[-50%] positions the button 16px from the right, then shifts it left by half its own width. This appears intentional (to overlap the scrollbar gutter) but is non-obvious — a brief comment or a cleaner utility would help.


What's Well Done

  • Clean separation of ambient components into ambient/; the existing components are only modified with opt-in variant="ambient" conditionals — no breakage risk for /history.
  • useElapsedTimer is correct: the startRef pattern avoids stale closure issues, and the cleanup properly cancels the interval.
  • The pointer-events: none / pointer-events-auto overlay pattern is the right approach.
  • Font fallback strategy (Iowan Old Style, Georgia, serif) is well thought out and the README covers the font-drop-in clearly.
  • ConversationScrollButton's new offsetBottom prop is a minimal, non-breaking API change.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 20, 2026

✅ Tests passed — 787/790

Suite Passed Failed Skipped
agent-sdk 44/44 0 0
agent 20/20 0 0
build 7/7 0 0
eval 8/8 0 0
server-agent 261/261 0 0
server-api 127/127 0 0
server-browser 3/3 0 0
server-integration 9/9 0 0
server-root 35/38 0 3
server-sdk 11/11 0 0
server-skills 31/31 0 0
server-tools 231/231 0 0

View workflow run

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 20, 2026

Greptile Summary

This PR introduces the V4 Ambient conversation redesign for the BrowserOS sidepanel — replacing the fixed ChatHeader and in-flow ChatFooter with a floating overlay bar, editorial serif title, and floating composer pill. History and other routes are unaffected. All major features (voice input, tab mentions, tool approvals, MCP selectors, reasoning blocks) are preserved under the new layout.

Confidence Score: 5/5

Safe 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

Filename Overview
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ambient/ChatComposerFloating.tsx New floating composer pill; correctly preserves voice, tab mention, workspace, and MCP selectors from the old ChatFooter with clean pointer-events layering.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ambient/ChatOverlayBar.tsx New overlay bar with home, status pill, and new-conversation controls; straightforward and well-structured.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ambient/StatusPill.tsx New provider/mode/nav popover; uses queueMicrotask to avoid race-closing on provider selection, which is a good pattern.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ambient/useElapsedTimer.ts Clean elapsed-timer hook; correctly resets on deactivation and cleans up the interval.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/Chat.tsx Main conversation entry point refactored to V4 ambient layout; FloatingErrorBanner uses a hardcoded offset that could overlap an expanded composer.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ChatMessages.tsx Gains variant/header/emptyStateSlot props; ambient pt-10 mobile padding slightly under-clears the 42–46 px overlay bar.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/ToolBatch.tsx Redesigned to Collapsible card with mono rows; formatInputPreview exposes raw tool input values without length limit or key filtering.
packages/browseros-agent/apps/agent/entrypoints/sidepanel/layout/ChatLayout.tsx ChatHeader now conditionally shown only on non-root routes; clean and correct.

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)"]
Loading
Prompt To Fix All 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.

---

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

Comment on lines +302 to +304
<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>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant