feat(setbuilder): mobile chat view via Agent FAB + full-viewport overlay (#402)#452
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hydration-safe matchMedia hook (defaults desktop on first render) that gates the mobile agent chat surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…idebar (#402) Split the agent-chat logic into a reusable useAgentChat hook and a presentational ChatPanelBody (with a shared PersonaToggle) so the desktop sidebar and the upcoming mobile overlay can share one implementation. ChatSidebar is now a thin desktop shell; its existing test suite stays green as the behavior-preservation guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A floating Agent FAB surfaces the critique grade and opens a full-viewport overlay reusing useAgentChat + ChatPanelBody. History loads lazily on open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
page.tsx renders EITHER the desktop ChatSidebar OR the MobileAgentOverlay based on useIsMobile, so only one useAgentChat mounts and fetches. CSS adds the FAB + full-viewport overlay (>=44px touch targets, overlay-scoped so the desktop composer stays compact) and drops the now-empty chat grid column on narrow viewports. The pool/timeline workspace remains a documented non-responsive gap, out of scope here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughExtracts all agent-chat state into a new ChangesMobile Agent Chat — Issue
Sequence Diagram(s)sequenceDiagram
participant BuilderPage
participant useIsMobile
participant ChatSidebar
participant MobileAgentOverlay
participant useAgentChat
participant api
BuilderPage->>useIsMobile: matchMedia("max-width: 720px")
useIsMobile-->>BuilderPage: isMobile = true / false
alt Desktop (isMobile = false)
BuilderPage->>ChatSidebar: render with setId, refreshToken
ChatSidebar->>useAgentChat: open=true, setId, refreshToken
else Mobile (isMobile = true)
BuilderPage->>MobileAgentOverlay: render with setId, refreshToken
MobileAgentOverlay->>useAgentChat: open=false initially
Note over MobileAgentOverlay: FAB shows overall_grade
MobileAgentOverlay->>useAgentChat: open=true on FAB tap
end
useAgentChat->>api: critiqueSet(setId)
api-->>useAgentChat: critique
useAgentChat->>api: getSetAgentHistory(setId)
api-->>useAgentChat: history entries
useAgentChat->>api: chatWithSetAgent(setId, message) on send
api-->>useAgentChat: user + assistant entries
useAgentChat->>BuilderPage: onMutationApplied() if mutating tools
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
dashboard/app/(dj)/setbuilder/components/useAgentChat.ts (2)
50-54: ⚡ Quick winDocument explicit agent capabilities and limitations in this file.
The comment explains purpose, but it doesn’t explicitly enumerate limitations/behavioral boundaries (e.g., history only when
open, one-send-at-a-time semantics, stale-request handling constraints). Add a concise capabilities/limitations section near the hook docs.
As per coding guidelines: “Document agent capabilities and limitations in agent implementation files” .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/app/`(dj)/setbuilder/components/useAgentChat.ts around lines 50 - 54, The useAgentChat hook's documentation comment lacks explicit enumeration of its agent capabilities and behavioral limitations. Expand the existing comment block (before the useAgentChat function definition) to include a concise "Capabilities and Limitations" section that documents specific behavioral boundaries such as history loading only when the agent is open, one-send-at-a-time semantics for message sending, and stale-request handling constraints to help developers understand the hook's boundaries and expected behavior.Source: Coding guidelines
77-171: ⚡ Quick winAdd structured logging for agent actions and state transitions.
The hook currently performs critique/history fetches and send lifecycle transitions without logging, which makes runtime diagnosis harder in production incidents.
As per coding guidelines: “Use logging for agent actions and state changes to aid debugging and monitoring” .🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/app/`(dj)/setbuilder/components/useAgentChat.ts around lines 77 - 171, The hook lacks structured logging for agent actions and state transitions, making production debugging difficult. Add logging calls to track: the start and completion of the critiqueSet() API call and any errors in the first useEffect, the start and completion of the getSetAgentHistory() API call in the second useEffect, and the execution flow within the send() function including when chatWithSetAgent() is called, when entries are updated, and when mutations are applied. Use consistent logging levels (info for actions, error for failures) to provide visibility into the hook's runtime behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dashboard/app/`(dj)/setbuilder/[setId]/page.tsx:
- Line 71: The useIsMobile hook initially returns false on mobile clients before
hydration completes, causing ChatSidebar to mount first on mobile, then switch
to MobileAgentOverlay at lines 432-450, which triggers useAgentChat twice. To
fix this, add a conditional render guard in the ChatSidebar section (around
lines 432-450) to only render ChatSidebar when isMobile is definitively false,
either by wrapping the ChatSidebar component in a condition that checks
isMobile, or by adding a skip flag to prevent the desktop chat component from
mounting until hydration confirms the actual device type. This prevents the
duplicate agent initialization during first paint.
In
`@dashboard/app/`(dj)/setbuilder/components/__tests__/MobileAgentOverlay.test.tsx:
- Around line 16-88: The test suite for MobileAgentOverlay currently lacks
coverage for the persona switching functionality (handled by PersonaToggle
component) and error scenarios from the useAgentChat hook. Add two new test
cases: one that verifies the PersonaToggle behavior when a different persona is
selected and that the appropriate API calls or state changes occur, and another
that mocks the chatWithSetAgent API to reject or return an error state, then
asserts that the error fallback UI from ChatPanelBody is rendered correctly
(such as an error message or retry affordance).
In `@dashboard/app/`(dj)/setbuilder/components/MobileAgentOverlay.tsx:
- Around line 24-47: Add logging telemetry when the agent overlay open state
changes in the MobileAgentOverlay component. Specifically, add a log statement
before or after the setOpen(true) call in the first button's onClick handler to
indicate the overlay is opening, and similarly add a log statement before or
after the setOpen(false) call in the close button's onClick handler to indicate
the overlay is closing. This will ensure agent-session state transitions are
tracked for production debugging and monitoring purposes.
- Around line 42-57: The MobileAgentOverlay component sets up proper dialog
semantics with role="dialog" and aria-modal="true", but does not manage focus
lifecycle. Add focus management to move focus into the dialog when it opens
(consider using a ref on the styles.agentOverlay container or the close button)
and restore focus to the previously focused element when the dialog closes via
setOpen(false). Use useEffect with the open state to handle focus on mount and
cleanup on unmount, ensuring keyboard users remain within the dialog context.
In `@dashboard/app/`(dj)/setbuilder/components/useAgentChat.ts:
- Around line 55-186: The useAgentChat hook contains critical functionality
without corresponding unit or integration tests. Create tests that cover the key
flows: the critique loading effect triggered by setId/refreshToken changes, the
history loading effect with stale-request guards using historyRequestIdRef, the
send function which creates pending entries and handles the API response with
mutation detection via onMutationApplied callback, error handling in both
useEffect blocks using setError and historyErrorRef, and the error normalization
through formatAgentError. Ensure tests validate that cancelled requests are
properly ignored and that pending entries are correctly replaced after
successful sends.
- Around line 132-170: The `send()` function has a race condition where the
`busy` guard check happens before `setBusy(true)` commits to state, allowing
rapid consecutive calls in the same tick to both pass the guard and trigger
duplicate API requests. Create a ref (such as `busyRef`) to synchronously track
the busy state, set `busyRef.current = true` immediately at the start of the
`send()` function right after the message validation check, and update the guard
clause to check `busyRef.current` instead of the state-based `busy` variable.
This ensures subsequent calls in the same tick will see the updated state before
proceeding.
---
Nitpick comments:
In `@dashboard/app/`(dj)/setbuilder/components/useAgentChat.ts:
- Around line 50-54: The useAgentChat hook's documentation comment lacks
explicit enumeration of its agent capabilities and behavioral limitations.
Expand the existing comment block (before the useAgentChat function definition)
to include a concise "Capabilities and Limitations" section that documents
specific behavioral boundaries such as history loading only when the agent is
open, one-send-at-a-time semantics for message sending, and stale-request
handling constraints to help developers understand the hook's boundaries and
expected behavior.
- Around line 77-171: The hook lacks structured logging for agent actions and
state transitions, making production debugging difficult. Add logging calls to
track: the start and completion of the critiqueSet() API call and any errors in
the first useEffect, the start and completion of the getSetAgentHistory() API
call in the second useEffect, and the execution flow within the send() function
including when chatWithSetAgent() is called, when entries are updated, and when
mutations are applied. Use consistent logging levels (info for actions, error
for failures) to provide visibility into the hook's runtime behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bae564c5-def1-409d-9546-6486a7c0e241
📒 Files selected for processing (12)
dashboard/app/(dj)/setbuilder/[setId]/page.tsxdashboard/app/(dj)/setbuilder/components/ChatPanelBody.tsxdashboard/app/(dj)/setbuilder/components/ChatSidebar.tsxdashboard/app/(dj)/setbuilder/components/MobileAgentOverlay.tsxdashboard/app/(dj)/setbuilder/components/__tests__/ChatPanelBody.test.tsxdashboard/app/(dj)/setbuilder/components/__tests__/MobileAgentOverlay.test.tsxdashboard/app/(dj)/setbuilder/components/__tests__/useIsMobile.test.tsxdashboard/app/(dj)/setbuilder/components/useAgentChat.tsdashboard/app/(dj)/setbuilder/components/useIsMobile.tsdashboard/app/(dj)/setbuilder/setbuilder.module.cssdocs/superpowers/plans/2026-06-17-issue-402-mobile-chat.mddocs/superpowers/specs/2026-06-17-issue-402-design.md
Address CodeRabbit review on PR #452: - page.tsx: gate both chat surfaces behind a post-hydration `chatSurfaceReady` flag so a mobile client never briefly mounts the desktop sidebar (and its duplicate agent fetches) before `useIsMobile` resolves. - useAgentChat: add a synchronous `sendInFlightRef` guard so two `send()` calls in the same tick cannot both pass the `busy` check and fire duplicate requests; document the hook's capabilities/limitations. - MobileAgentOverlay: manage dialog focus lifecycle — move focus to the close button on open, restore it to the FAB on close, and close on Escape. - Add useAgentChat hook tests (critique/history effects, optimistic send, same-tick double-send guard, mutation callback, error normalization) and MobileAgentOverlay tests (persona switch, error fallback, focus + Escape). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
CodeRabbit body nitpicks —
|
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.
Why
The WrzDJSet builder is a hard 3-column desktop grid (
pool | timeline | chat)with no mobile treatment, so the agent chat column is unusable on a phone. This
gives the agent a first-class mobile surface without attempting to make the
whole workspace responsive (a larger, separate effort).
What
On viewports ≤720px the desktop chat column is replaced by a floating Agent
FAB — carrying the live critique grade — that opens a full-viewport overlay
with the complete agent experience (critique card, tool-call cards, suggestion
chips, persona toggle, composer).
To avoid duplicating ~390 LOC of chat logic,
ChatSidebarwas refactored:useAgentChat— hook holding all chat state + actions (critique/historyload, send, persona, error handling).
ChatPanelBody— presentational body (messages, tool cards, critique,suggestion chips, composer) plus a shared
PersonaToggle.ChatSidebar— now a thin desktop shell wrappingChatPanelBody.MobileAgentOverlay— the FAB + overlay, also wrappingChatPanelBody.useIsMobile— hydration-guardedmatchMedia(720px)hook sopage.tsxrenders either the desktop sidebar or the mobile overlay — never both,
so only one
useAgentChatmounts and fetches.Touch targets are ≥44px and the overlay stacks single-column (consistent with
the #438 mobile-reorder work); the bumps are scoped to
.agentOverlayso thedesktop composer is byte-for-byte unchanged.
Out of scope (documented known gap): the pool/timeline/curve workspace
itself is still not responsive.
Testing
npm test -- --run→ 1272 passed)npm test -- --run --coverage→ exit 0; 78.75% stmts/ 69.89% branch / 72.75% funcs / 81.13% lines)
npm run lint) + TypeScript strict (npx tsc --noEmit)ChatSidebar.test.tsx(11 tests) stays green as thebehavior-preservation guard
grade → tap opens overlay → critique/tool cards/chips legible → send works
→ close returns to the builder; desktop sidebar unchanged
🤖 Co-authored by Claude Opus 4.8. Closes #402.
Summary by CodeRabbit
Release Notes
New Features
Documentation