Restore Branch Head to Commit 488b47c #456
Conversation
Restored the branch to the expected head at commit 488b47c. This recovers several missing features and architectural improvements: - Integrated HistoryToggleProvider and HistorySidebar. - Integrated UsageToggleProvider and the new UsageView component. - Removed legacy usage-sidebar.tsx. - Restored pricing and feature updates in PurchaseCreditsPopup. - Fixed layout structure in app/layout.tsx to support these new global state providers. Resolved previous merge conflicts and incorrect force-push state. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The biggest concern is overlay coordination: history, usage, and profile can now be opened independently, likely leading to conflicting UI states and focus/scroll issues. UsageView currently looks stateful but is effectively static mock data, which should be simplified to reduce confusion. There are also a couple of maintainability concerns (duplicated Stripe URL and brittle onOpenChange={setHistoryOpen} handler) that will make future changes riskier.
Additional notes (4)
- Maintainability |
components/history-sidebar.tsx:19-32
TheSheetContentwidth is fixed tow-64with no explicit side. Depending on yourSheetimplementation defaults, this may slide in from an unexpected side and can be cramped on larger screens.
More importantly, there’s no coordination logic preventing history from opening while usage/settings are open (you added this coordination for usage vs profile). That can create overlapping panels and confusing UX. If multiple overlays can exist, consider enforcing a single active overlay at a time.
-
Readability |
components/chat.tsx:110-114
This nested ternary is starting to get hard to read/extend. It also encodes UI precedence (usage > settings > map) in two places (mobile + desktop), which invites drift later. -
Maintainability |
components/header.tsx:27-27
Headeropens the purchase popup unconditionally on mount (setIsPurchaseOpen(true)inuseEffect). That forces a modal on every app load, which is a major UX regression unless this is strictly intended for a limited scenario (e.g., credit-exhausted users).
If this modal is meant to appear only when the user actually hits a credit limit, it should be triggered from that condition rather than on mount.
- Maintainability |
components/purchase-credits-popup.tsx:23-24
handlePurchaseand the Upgrade button inUsageViewboth open a Stripe URL viawindow.open. This duplicates the URL in multiple places, making future changes error-prone and risking divergence (especially across environments).
Summary of changes
What this PR changes
-
Restores global UI state providers in
app/layout.tsx- Wraps the app in new
HistoryToggleProviderandUsageToggleProvideralongside existing providers. - Mounts a new global
HistorySidebarnext to the existingSidebar.
- Wraps the app in new
-
Replaces legacy usage sidebar behavior with an integrated view
- Deletes
components/usage-sidebar.tsx. - Adds
components/usage-view.tsxandcomponents/usage-toggle-context.tsx. - Updates
components/chat.tsxto render<UsageView />when usage is toggled open (both mobile and desktop right pane).
- Deletes
-
Adds history sidebar + toggle context
- Adds
components/history-sidebar.tsxandcomponents/history-toggle-context.tsx. - Refactors
components/history.tsxto act as a simple toggle button (no longer owns theSheet). - Updates
components/header.tsxso clicking the logo toggles history and introduces a coordinated usage toggle.
- Adds
-
UI/UX tweaks
components/conditional-lottie.tsx: hides the loading animation when either settings/profile view or usage view is open.components/purchase-credits-popup.tsx: expands plan details to show a yearly price and feature checklist.components/profile-toggle-context.tsxandcomponents/profile-toggle.tsx: addscloseProfileView()and coordinates closing usage when opening profile sections.
| export function HistorySidebar() { | ||
| const { isHistoryOpen, setHistoryOpen } = useHistoryToggle() | ||
|
|
||
| return ( | ||
| <Sheet open={isHistoryOpen} onOpenChange={setHistoryOpen}> | ||
| <SheetContent className="w-64 rounded-tl-xl rounded-bl-xl" data-testid="history-panel"> | ||
| <SheetHeader> |
There was a problem hiding this comment.
<Sheet open={isHistoryOpen} onOpenChange={setHistoryOpen}> passes setHistoryOpen directly as the handler. Many sheet/dialog components call onOpenChange(open: boolean) but some call with additional args or an event-like payload. Passing a raw setter is brittle and makes it harder to add side-effects later (e.g., closing other overlays when history opens).
Suggestion
Wrap the handler to lock the contract to a boolean and to give yourself a single place to coordinate overlays later.
<Sheet
open={isHistoryOpen}
onOpenChange={(open) => setHistoryOpen(open)}
>If you want coordination (recommended), this is also where you can close Usage / Profile when open === true. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| 'use client' | ||
|
|
||
| import React, { useEffect, useState } from 'react' | ||
| import { Button } from '@/components/ui/button' | ||
| import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/table' | ||
| import { Zap, RefreshCw, LayoutPanelLeft, Minus } from 'lucide-react' | ||
| import { useUsageToggle } from './usage-toggle-context' | ||
|
|
||
| export function UsageView() { | ||
| const [usage] = useState([ | ||
| { details: 'Efficiently Fix Pull Request ...', date: '2026-01-17 08:05', change: -418 }, | ||
| { details: 'Fix Build and Add Parallel S...', date: '2026-01-16 06:10', change: -482 }, | ||
| { details: 'How to Add a Feature to a ...', date: '2026-01-14 10:42', change: -300 }, | ||
| ]) | ||
| const [credits] = useState(0) | ||
| const { toggleUsage } = useUsageToggle() | ||
|
|
There was a problem hiding this comment.
UsageView uses useState for constant mock data (usage, credits) and imports useEffect but does not use it. Even if ESLint passes, this is still unnecessary runtime work and makes future wiring to real data harder to reason about (because it looks stateful but isn't).
Suggestion
Make the mock data constants (or module-level constants) and remove the unused useEffect import.
import React from 'react'
const MOCK_USAGE = [
{ details: 'Efficiently Fix Pull Request ...', date: '2026-01-17 08:05', change: -418 },
// ...
]
const MOCK_CREDITS = 0
export function UsageView() {
const usage = MOCK_USAGE
const credits = MOCK_CREDITS
const { toggleUsage } = useUsageToggle()
// ...
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this refactor.
| import React, { useEffect, useState } from 'react' | ||
| import { Button } from '@/components/ui/button' | ||
| import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/table' | ||
| import { Zap, RefreshCw, LayoutPanelLeft, Minus } from 'lucide-react' | ||
| import { useUsageToggle } from './usage-toggle-context' | ||
|
|
||
| export function UsageView() { | ||
| const [usage] = useState([ | ||
| { details: 'Efficiently Fix Pull Request ...', date: '2026-01-17 08:05', change: -418 }, | ||
| { details: 'Fix Build and Add Parallel S...', date: '2026-01-16 06:10', change: -482 }, | ||
| { details: 'How to Add a Feature to a ...', date: '2026-01-14 10:42', change: -300 }, | ||
| ]) | ||
| const [credits] = useState(0) | ||
| const { toggleUsage } = useUsageToggle() |
There was a problem hiding this comment.
UsageView stores static mock data in useState and even imports useEffect but doesn’t use it. Since the values are constant and not updated, using state here adds unnecessary React state machinery and re-render semantics, and it makes it harder to later swap in real data fetching logic.
Also, the table rows use key={i}. If this ever becomes real dynamic data, index keys will cause subtle UI bugs on insert/reorder (rows reusing wrong DOM/state).
Suggestion
Use plain constants for the mock data (or fetch via an effect), remove the unused useEffect import, and use a stable key (e.g., derive an id or combine date+details).
import React from 'react'
const usage = [
{ details: 'Efficiently Fix Pull Request ...', date: '2026-01-17 08:05', change: -418 },
// ...
]
const credits = 0
// ...
{usage.map((item) => (
<TableRow key={`${item.date}:${item.details}`}> {/* replace with real id when available */}
{/* ... */}
</TableRow>
))}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| <Button variant="ghost" size="icon" onClick={toggleUsage}> | ||
| <Minus className="h-6 w-6" /> | ||
| <span className="sr-only">Close usage</span> | ||
| </Button> |
There was a problem hiding this comment.
The usage panel close button uses toggleUsage(). If usage can be opened/closed from multiple entry points, a toggle-based close can accidentally open usage when state gets out of sync (e.g., double-clicks, concurrent state updates, or future code paths that call this handler when already closed).
A close affordance should be idempotent and call a dedicated closeUsage() instead.
Suggestion
Use closeUsage() for the close button so the action is deterministic.
const { closeUsage } = useUsageToggle()
<Button variant="ghost" size="icon" onClick={closeUsage}>
<Minus className="h-6 w-6" />
<span className="sr-only">Close usage</span>
</Button>Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const { toggleUsage, isUsageOpen } = useUsageToggle() | ||
| const { activeView, closeProfileView } = useProfileToggle() | ||
| const { toggleHistory } = useHistoryToggle() | ||
|
|
||
| const handleUsageToggle = () => { | ||
| // If we're about to open usage and profile is open, close profile first | ||
| if (!isUsageOpen && activeView) { | ||
| closeProfileView() | ||
| } | ||
| toggleUsage() | ||
| } |
There was a problem hiding this comment.
handleUsageToggle closes the profile view when opening usage, but the inverse coordination is handled elsewhere (ProfileToggle closes usage when opening a profile section). History is now also a global overlay, but it isn't coordinated with either usage or profile. This will likely allow multiple overlays to be open at once (history sheet + usage view), which can produce confusing input focus/scroll behavior.
Suggestion
Centralize overlay coordination so only one of {history, usage, profile} can be open at a time. A minimal improvement is to close history when opening usage and close usage when opening history.
Example (in handleUsageToggle):
if (!isUsageOpen) {
closeProfileView()
setHistoryOpen(false) // or a `closeHistory()` helper
}
toggleUsage()You may want to add closeHistory() to HistoryToggleContextType for symmetry. Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing this coordination.
| }; | ||
|
|
||
| const features = [ | ||
| 'Internet Search', |
There was a problem hiding this comment.
Dialog's onOpenChange callback receives the next open boolean, but this component ignores it and always calls onClose(). That means opening the dialog via internal triggers (or re-opening due to state changes) can immediately call onClose() and fight state, and it also prevents supporting controlled open/close semantics cleanly.
This is a correctness issue with Radix/Shadcn controlled dialogs: you should close only when open becomes false.
Suggestion
Update onOpenChange to respect the boolean and only close when it transitions to false.
<Dialog
open={isOpen}
onOpenChange={(open) => {
if (!open) onClose()
}}
>Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
User description
I have recovered the missing changes by resetting the branch to the specific commit you identified (488b47c). This restoration includes the full history sidebar integration, the new integrated usage view, and various UI enhancements that were lost during the previous conflict resolution. The codebase is now exactly at the state you expected, with no remaining conflicts.
PR created automatically by Jules for task 16847885310673003402 started by @ngoiyaeric
PR Type
Enhancement
Description
Restore lost architectural improvements with integrated global state providers
Implement new
UsageViewcomponent replacing legacyUsageSidebarfor better UXAdd
HistorySidebarwith dedicated toggle context for history managementEnhance UI interactions with coordinated profile/usage/history view toggles
Update pricing popup with feature list and improved visual presentation
Diagram Walkthrough
File Walkthrough
13 files
Add global state providers for history and usageNew context for managing history sidebar stateNew sidebar component for chat history displayRefactor to use global history toggle contextNew context for managing usage view stateNew integrated usage view componentRemove legacy usage sidebar componentIntegrate usage view with conditional renderingAdd coordinated toggle handlers for viewsAdd closeProfileView method for view coordinationImplement view coordination when opening profileUpdate visibility logic for usage view stateAdd feature list and improve pricing display