Prompt History and PR Pane -> Primary - #1070
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe chat UI now supports session-scoped prompt history. Users can navigate and restore drafts, jump to matching transcript prompts, and preview selections in the minimap. Floating PR-pane inset measurement and left-side transcript reservation were removed. ChangesChat prompt history
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/chat/chatPromptHistory.test.ts (1)
49-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding key-precedence coverage.
The current test only exercises the
steerIdbranch ofpromptHistoryEventKey. ThemessageIdprecedence and theturnId/textfallbacks carry the jump identity for older transcripts and are untested. Add one table-driven case per branch to lock the precedence order.🤖 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/desktop/src/renderer/components/chat/chatPromptHistory.test.ts` around lines 49 - 70, Extend the tests for promptHistoryEventKey with table-driven cases covering each identity branch: messageId precedence, turnId fallback, and text fallback. Keep the existing metadata-decoration assertion, and verify the expected key for each branch so the precedence order is locked down.apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx (1)
1991-2000: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePrefer
useEffectfor the history re-anchor.This effect only writes refs and calls
clearPromptHistory. It reads no layout and produces no synchronous visual result, so it does not need to block paint.useEffectis the cheaper choice on a hot input path.🤖 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/desktop/src/renderer/components/chat/AgentChatComposer.tsx` around lines 1991 - 2000, Replace useLayoutEffect with useEffect for the prompt history re-anchor around promptHistorySelectedKeyRef and promptHistoryIndexRef, preserving the existing dependencies and callback behavior while allowing paint before this non-layout synchronization runs.
🤖 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/desktop/src/renderer/components/chat/AgentChatComposer.test.tsx`:
- Around line 1326-1334: Update the three prompt-stash tests around the
window.ade stub to preserve test isolation: capture the existing window.ade
value before each test and restore it in afterEach, or use a shared
stubPromptStashes helper that retains the other agentChat and app namespaces.
Replace the inline assignments with the helper while keeping each test’s list,
create, and delete behavior unchanged.
- Around line 1219-1251: Extend the AgentChatComposer tests with a named
regression case for a multiline new draft while promptHistoryIndexRef is null.
Render a draft containing multiple lines, place the caret on the last line,
press ArrowUp, and assert onDraftChange is not called, preserving native caret
movement instead of entering prompt history.
In `@apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx`:
- Around line 3937-3963: The prompt-history entry handling in
AgentChatComposer’s currentIndex === null branch must respect line boundaries:
update atFirstLine so cursorOffset === 0 counts as the first line, and return
false when !atFirstLine before applying history. In
apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx lines 3937-3963,
make these behavior changes; in
apps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsx lines
1219-1251, add a named test rendering a multiline draft with the caret on the
last line, pressing ArrowUp, and asserting onDraftChange was not called.
- Around line 1954-1966: Update the window listeners in the useEffect containing
cancelIfActive to use passive capture options for wheel and touchstart,
including matching options in cleanup; preserve pointerdown behavior and ensure
cancelIfActive remains free of preventDefault calls.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 12740-12747: Update the prFloating overlay container to be
rendered only at sufficiently wide viewport widths, or reserve layout space
below that minimum width so it cannot overlap the transcript. Preserve the
existing positioning, sizing, and animation behavior when the width threshold is
met.
---
Nitpick comments:
In `@apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx`:
- Around line 1991-2000: Replace useLayoutEffect with useEffect for the prompt
history re-anchor around promptHistorySelectedKeyRef and promptHistoryIndexRef,
preserving the existing dependencies and callback behavior while allowing paint
before this non-layout synchronization runs.
In `@apps/desktop/src/renderer/components/chat/chatPromptHistory.test.ts`:
- Around line 49-70: Extend the tests for promptHistoryEventKey with
table-driven cases covering each identity branch: messageId precedence, turnId
fallback, and text fallback. Keep the existing metadata-decoration assertion,
and verify the expected key for each branch so the precedence order is locked
down.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ffadfc1-104d-4341-b7ce-1135c6d5a0ae
⛔ Files ignored due to path filters (1)
docs/features/chat/composer-and-ui.mdis excluded by!docs/**
📒 Files selected for processing (14)
apps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatMessageList.test.tsxapps/desktop/src/renderer/components/chat/AgentChatMessageList.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/chat/ChatUserMinimap.test.tsxapps/desktop/src/renderer/components/chat/ChatUserMinimap.tsxapps/desktop/src/renderer/components/chat/ComposerPromptStash.tsxapps/desktop/src/renderer/components/chat/chatPrPaneInset.tsapps/desktop/src/renderer/components/chat/chatPromptHistory.test.tsapps/desktop/src/renderer/components/chat/chatPromptHistory.tsapps/desktop/src/renderer/components/chat/chatUserMinimap.logic.test.tsapps/desktop/src/renderer/components/chat/chatUserMinimap.logic.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/components/chat/chatPrPaneInset.ts
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
The PR adds session-scoped prompt-history navigation and transcript synchronization while making floating PR panes independent of transcript and minimap layout.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
Reviews (4): Last reviewed commit: "fix: honor multiline prompt navigation b..." | Re-trigger Greptile
Context used: