Repository navigation
feat: Add chat history - #698
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an in-chat session history UI: new ChatHeader, ChatHistoryView, SessionActionsMenu, and ChatHistoryItem components; exposes Header and History on the Chat compound; and wires a showHistory toggle in AssistantChatContainer to switch between messages and the history view. ChangesChat Session History Navigation
Sequence DiagramsequenceDiagram
participant User
participant AssistantChatContainer
participant ChatHeader
participant ChatHistoryView
participant SessionActionsMenu
participant AIStore
User->>AssistantChatContainer: Open chat
AssistantChatContainer->>ChatHeader: Render header (when session exists)
User->>ChatHeader: Click History button
ChatHeader->>AssistantChatContainer: set showHistory = true
AssistantChatContainer->>ChatHistoryView: Render history view
User->>ChatHistoryView: Click session row
ChatHistoryView->>AIStore: switchSession(sessionId)
AIStore->>AssistantChatContainer: active session updated
User->>ChatHistoryView: Open actions on a session
ChatHistoryView->>SessionActionsMenu: trigger menu
User->>SessionActionsMenu: Select Rename or Delete
SessionActionsMenu->>AIStore: renameSession / deleteSession
AIStore->>ChatHistoryView: sessions list updated
User->>ChatHeader: Click back
ChatHeader->>AssistantChatContainer: set showHistory = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@apps/sqlrooms-cli-ui/src/components/AssistantChatContainer.tsx`:
- Line 4: The component AssistantChatContainer leaves the showHistory boolean
true when all sessions are removed, so creating a new session can mistakenly
show History; update the AssistantChatContainer to explicitly set showHistory =
false whenever the sessions list becomes empty and also clear showHistory right
before creating a new session. Concretely, add a useEffect watching the sessions
array (or sessions.length) to call setShowHistory(false) when it transitions to
zero, and also call setShowHistory(false) at the start of the createNewSession /
handleCreateSession function to ensure the new session opens in chat view.
In `@packages/ai-core/src/components/ChatHeader.tsx`:
- Around line 103-105: The icon-only "new session" Button in ChatHeader (the
Button with onClick={handleCreateSession} containing PlusIcon) lacks an
accessible name — add an explicit accessible label (e.g., aria-label="New
session" or include visually-hidden text inside the Button) so screen readers
can announce the control; ensure the label is descriptive and keep the existing
onClick and PlusIcon intact.
In `@packages/ai-core/src/components/ChatHistoryView.tsx`:
- Around line 81-102: The icon-only action buttons (the Button components
wrapping PencilIcon and TrashIcon used in the rename and delete handlers, and
the similar back-button at lines referenced) lack accessible names; update those
Button usages (the ones with onClick calling onRename/onDelete and the back
button component) to include proper accessible labeling by adding aria-label
attributes or an aria-labelledby reference (e.g., aria-label="Rename chat" and
aria-label="Delete chat" / "Back") or by including visually hidden text inside
the button; keep the existing onClick handlers (onRename/onDelete) and icon
children unchanged while ensuring screen readers receive the descriptive labels.
- Around line 49-59: The session row is rendered as a plain div, which prevents
keyboard users from focusing and activating it; update the element in
ChatHistoryView (the session row rendering that currently uses the div with
className, onClick, onMouseEnter, onMouseLeave and isActive logic) to be a
semantic interactive element—preferably a <button type="button">—so it is
focusable and keyboard-activatable, keep the existing className and event
handlers, ensure any visual focus styles remain intact, and if you cannot use a
button instead add role="button", tabIndex={0}, and key handlers (Enter/Space)
to the same component to provide equivalent keyboard accessibility.
In `@packages/ai-core/src/components/SessionActionsMenu.tsx`:
- Around line 23-25: The menu trigger Button in the SessionActionsMenu component
renders only an icon (MoreVertical) with no accessible name; update the Button
(the icon-only trigger) to include an accessible label such as adding an
aria-label="Open session actions" or aria-label prop that describes its purpose
(or include visually hidden text inside the Button) so assistive technologies
can identify the control; target the Button element that wraps MoreVertical in
SessionActionsMenu and ensure the label is concise and descriptive.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0103f29d-7a89-4093-ab5e-90d8a62946ae
📒 Files selected for processing (5)
apps/sqlrooms-cli-ui/src/components/AssistantChatContainer.tsxpackages/ai-core/src/components/Chat.tsxpackages/ai-core/src/components/ChatHeader.tsxpackages/ai-core/src/components/ChatHistoryView.tsxpackages/ai-core/src/components/SessionActionsMenu.tsx
| import {Button, SkeletonPane} from '@sqlrooms/ui'; | ||
| import {PlusIcon} from 'lucide-react'; | ||
| import React from 'react'; | ||
| import React, {useState} from 'react'; |
Contributor
There was a problem hiding this comment.
Reset showHistory when session list becomes empty (or before creating a new one).
showHistory can remain true after sessions are removed, so creating a new session from the empty-state can route users back to History instead of the chat view.
Suggested fix
-import React, {useState} from 'react';
+import React, {useEffect, useState} from 'react';
...
const [showHistory, setShowHistory] = useState(false);
+
+ useEffect(() => {
+ if (!currentSessionId) {
+ setShowHistory(false);
+ }
+ }, [currentSessionId]);
...
onClick={() => createSession()}
+ onClick={() => {
+ setShowHistory(false);
+ createSession();
+ }}Also applies to: 26-27, 31-38
🤖 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 `@apps/sqlrooms-cli-ui/src/components/AssistantChatContainer.tsx` at line 4,
The component AssistantChatContainer leaves the showHistory boolean true when
all sessions are removed, so creating a new session can mistakenly show History;
update the AssistantChatContainer to explicitly set showHistory = false whenever
the sessions list becomes empty and also clear showHistory right before creating
a new session. Concretely, add a useEffect watching the sessions array (or
sessions.length) to call setShowHistory(false) when it transitions to zero, and
also call setShowHistory(false) at the start of the createNewSession /
handleCreateSession function to ensure the new session opens in chat view.
ilyabo
approved these changes
Jun 8, 2026
Collaborator
|
maybe name the PR "Add chat history" to be more concrete? |
Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/ai-core/src/components/ChatHistoryItem.tsx`:
- Around line 41-52: The wrapper in ChatHistoryItem is a native <button> (the
group container) but the internal `@sqlrooms/ui` Button components render native
buttons by default, causing nested interactive elements; update the internal
Button usages inside ChatHistoryItem to render as non-button children by passing
asChild={true} (or the library's equivalent prop) so they don't emit a second
<button>, or alternatively change the outer wrapper to a non-button element with
role="button" and keyboard handlers; target the ChatHistoryItem component and
modify the inner Button component instances (or the wrapper element) accordingly
to eliminate nested <button> elements.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1028e88-917a-47b4-a354-d5ca5e36f139
📒 Files selected for processing (4)
packages/ai-core/src/components/ChatHeader.tsxpackages/ai-core/src/components/ChatHistoryItem.tsxpackages/ai-core/src/components/ChatHistoryView.tsxpackages/ai-core/src/components/SessionActionsMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai-core/src/components/SessionActionsMenu.tsx
- packages/ai-core/src/components/ChatHeader.tsx
dmitriy-kostianetskiy
changed the title
…ved accessibility
This was referenced Jun 13, 2026
Merged
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
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.
Summary by CodeRabbit