fix: the new conversation title isn't displayed in the sidebar#1662
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new hook, Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Chat UI / useChat
participant Sync as useNewChatSidebarSync
participant Credits as Credits Service
participant Conv as Conversations Provider
Note over UI,Sync: Chat lifecycle events
UI->>Sync: onFinish({ isError, isAbort })
Sync->>Credits: refetchCredits() -- conditional on success/non-abort
Sync->>Conv: refetchConversations() -- conditional for new-chat, single-latch
alt first assistant stream on new-chat
UI->>Sync: maybeRefreshOnFirstStream(status="streaming")
Sync->>Conv: refetchConversations() -- one-time trigger
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR modifies core logic within the useVercelChat hook to handle state synchronization, which falls under business logic changes requiring human review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2aebd0e8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| !isAbort && | ||
| !hasSyncedRecentChatsAfterFirstAssistantRef.current && | ||
| finishedMessages.filter((m) => m.role === "assistant").length === 1; |
There was a problem hiding this comment.
This guard can permanently skip refetchConversations() for a brand-new chat when the first generation is aborted after partial output: !isAbort blocks the initial sync, and on the next successful turn the prior assistant message makes the assistant count > 1, so the === 1 check never passes. In that flow the server can still create and title the conversation on a later successful turn, but the sidebar remains on the optimistic “New Chat” entry until a manual refresh.
Useful? React with 👍 / 👎.
luxapientia
changed the title
sweetmantech
left a comment
sweetmantech
left a comment
There was a problem hiding this comment.
DRY / KISS - This solution looks overcomplicated. Look at how normal chat enables the chat title to appear instantly in the sidebar.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useVercelChat.ts`:
- Around line 197-214: The code currently sets
hasSyncedRecentChatsAfterFirstAssistantRef.current before awaiting
refetchConversations(), which can falsely mark the chat as synced if the refresh
fails; update the onFinish handlers (the async onFinish function and the similar
block at the later occurrence) to await refetchConversations() first and only
set hasSyncedRecentChatsAfterFirstAssistantRef.current = true after
refetchConversations() resolves successfully (handle errors by not toggling the
ref and allowing subsequent successful finishes to retry); keep refetchCredits()
as before but sequence the refetchConversations() await so the latch is set only
on success.
- Around line 211-214: refetchCredits is being called unconditionally, causing
unnecessary requests on aborted/errored generations; change the Promise.all
array to only include refetchCredits when the same success gate is true (the
same condition used for refetchConversations). Concretely, replace the current
array [refetchCredits(), ...(shouldRefreshRecentChats ? [refetchConversations()]
: [])] with a single conditional spread so both refetchCredits() and
refetchConversations() are only added when shouldRefreshRecentChats is truthy
(e.g., ...(shouldRefreshRecentChats ? [refetchCredits(), refetchConversations()]
: [])), ensuring credits are only refreshed on successful generations.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6a5a9b5-5f51-477a-ace3-9f108c315f24
📒 Files selected for processing (1)
hooks/useVercelChat.ts
There was a problem hiding this comment.
4 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useVercelChat.ts">
<violation number="1" location="hooks/useVercelChat.ts:205">
P3: Removing the `finishedMessages.filter((m) => m.role === 'assistant').length === 1` guard means `onFinish` will now refetch conversations on every first completion for existing conversations too (the `useEffect` only covers `!roomId`). Consider re-adding a guard for existing conversations, e.g., `!roomId` or restoring the assistant-count check in the `onFinish` condition.</violation>
<violation number="2" location="hooks/useVercelChat.ts:218">
P1: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
`useVercelChat.ts` remains far over the 100-line cap (412 LOC) and this PR adds more hook logic instead of splitting responsibilities.</violation>
<violation number="3" location="hooks/useVercelChat.ts:226">
P1: The `useEffect` sets the sync ref to `true` during streaming, which suppresses the `onFinish` refetch for new conversations. If the conversation title isn't generated server-side until after the response completes, this early refetch will return stale data (e.g., "New Chat") and the post-completion refetch that would catch the real title is skipped.
Consider using a separate ref for the streaming-phase sync, or clearing the ref before the `onFinish` check so both refetches can fire.</violation>
<violation number="4" location="hooks/useVercelChat.ts:226">
P2: Set the sync flag *after* `refetchConversations()` resolves, not before. If the refetch request fails (network error, timeout, etc.), the flag is already `true` and neither this effect nor the `onFinish` handler will ever retry — leaving the sidebar stale for this conversation with no recovery path.
```ts
// Instead of:
hasSyncedRecentChatsAfterFirstAssistantRef.current = true;
refetchConversations();
// Do:
refetchConversations().then(() => {
hasSyncedRecentChatsAfterFirstAssistantRef.current = true;
});
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| }, | ||
| }); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
P1: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
useVercelChat.ts remains far over the 100-line cap (412 LOC) and this PR adds more hook logic instead of splitting responsibilities.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useVercelChat.ts, line 218:
<comment>`useVercelChat.ts` remains far over the 100-line cap (412 LOC) and this PR adds more hook logic instead of splitting responsibilities.</comment>
<file context>
@@ -216,6 +215,18 @@ export function useVercelChat({
},
});
+ useEffect(() => {
+ const shouldRefreshRecentChatsOnFirstStream =
+ !roomId &&
</file context>
luxapientia
force-pushed
the
trongtruongpham192/myc-4598-chat-the-new-conversation-title-isnt-displayed-in-the
branch
from
e6a70cc to
d180832
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
hooks/useVercelChat.ts (1)
219-231:⚠️ Potential issue | 🟡 MinorSkip credit refetches for aborted and failed generations.
refetchCredits()still runs whenisErrororisAbortis true, so non-success paths keep issuing an extra request even though the recent-chat refresh is correctly gated.Suggested fix
onFinish: async ({ isError, isAbort, }) => { - const shouldRefreshRecentChats = - !isError && - !isAbort && - !hasSyncedRecentChatsAfterFirstAssistantRef.current; + if (isError || isAbort) return; + + const shouldRefreshRecentChats = + !hasSyncedRecentChatsAfterFirstAssistantRef.current; await Promise.all([ refetchCredits(), ...(shouldRefreshRecentChats ? [refreshRecentChatsOnce()] : []), ]); },As per coding guidelines,
hooks/**/*.ts: "Handle edge cases and errors".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useVercelChat.ts` around lines 219 - 231, The onFinish handler always calls refetchCredits(), causing extra requests on aborted or failed generations; change the logic in the onFinish closure (symbols: onFinish, refetchCredits, shouldRefreshRecentChats, refreshRecentChatsOnce, hasSyncedRecentChatsAfterFirstAssistantRef.current) so refetchCredits() is only added to the Promise.all task list when the run succeeded (i.e., !isError && !isAbort), similar to how refreshRecentChatsOnce() is conditionally included — build the tasks array conditionally and then await Promise.all(tasks) to avoid calling refetchCredits on error/abort paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useVercelChat.ts`:
- Around line 184-207: The sync latch is currently global and can be set by an
earlier refetch resolving after id changes; inside refreshRecentChatsOnce
capture the current id (the hook's id) into a local const (e.g., const
startingId = id) before awaiting refetchConversations(), and only set
hasSyncedRecentChatsAfterFirstAssistantRef.current = true if startingId === id;
keep isRefreshingRecentChatsRef toggling as-is in try/finally and reference
refetchConversations, id, refreshRecentChatsOnce,
hasSyncedRecentChatsAfterFirstAssistantRef, and isRefreshingRecentChatsRef to
locate where to add the startingId check.
---
Duplicate comments:
In `@hooks/useVercelChat.ts`:
- Around line 219-231: The onFinish handler always calls refetchCredits(),
causing extra requests on aborted or failed generations; change the logic in the
onFinish closure (symbols: onFinish, refetchCredits, shouldRefreshRecentChats,
refreshRecentChatsOnce, hasSyncedRecentChatsAfterFirstAssistantRef.current) so
refetchCredits() is only added to the Promise.all task list when the run
succeeded (i.e., !isError && !isAbort), similar to how refreshRecentChatsOnce()
is conditionally included — build the tasks array conditionally and then await
Promise.all(tasks) to avoid calling refetchCredits on error/abort paths.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25d5f8bc-ae9b-4174-ba6c-69d7dfd88e55
📒 Files selected for processing (1)
hooks/useVercelChat.ts
luxapientia
force-pushed
the
trongtruongpham192/myc-4598-chat-the-new-conversation-title-isnt-displayed-in-the
branch
from
d180832 to
5429f0a
Compare
This feature wasn't implemented before. No chat title appeared in the sidebar when a chat was started. |
| useEffect(() => { | ||
| hasSyncedRecentChatsAfterFirstAssistantRef.current = false; | ||
| isRefreshingRecentChatsRef.current = false; | ||
| }, [id]); | ||
|
|
||
| const refreshRecentChatsOnce = useCallback(async () => { | ||
| const startedForChatId = chatIdRef.current; | ||
|
|
||
| if (hasSyncedRecentChatsAfterFirstAssistantRef.current) return false; | ||
| if (isRefreshingRecentChatsRef.current) return false; | ||
|
|
||
| isRefreshingRecentChatsRef.current = true; | ||
| try { | ||
| const result = await refetchConversations(); | ||
| if ( | ||
| result.isSuccess && | ||
| chatIdRef.current === startedForChatId | ||
| ) { | ||
| hasSyncedRecentChatsAfterFirstAssistantRef.current = true; | ||
| return true; | ||
| } | ||
| return false; | ||
| } catch (error) { | ||
| console.error("Failed to refresh recent chats:", error); | ||
| return false; | ||
| } finally { | ||
| if (chatIdRef.current === startedForChatId) { | ||
| isRefreshingRecentChatsRef.current = false; | ||
| } | ||
| } | ||
| }, [refetchConversations]); |
There was a problem hiding this comment.
OCP / KISS Principles
- actual: adding a ton of new code to the existing useVercelChat hook.
- required: new hook file for this net new code.
- Ideally, you follow DRY and reuse existing code for setting / displaying the new chat item.
luxapientia
force-pushed
the
trongtruongpham192/myc-4598-chat-the-new-conversation-title-isnt-displayed-in-the
branch
from
5429f0a to
76aab38
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useNewChatSidebarSync.ts`:
- Around line 41-43: The early-return in refreshRecentChatsOnce (checking
hasSyncedRecentChatsAfterFirstAssistantRef.current and
isRefreshingRecentChatsRef.current) can skip the finish-time sync; change the
logic so that when an in-flight refresh is detected
(isRefreshingRecentChatsRef), refreshRecentChatsOnce does not return false
immediately but instead awaits the shared in-flight promise (or attaches to it)
and only returns after that promise resolves/rejects, and update the onFinish
path to await that same in-flight refresh (or retry the refresh on failure) so
the completion flow always waits for or retries the running refresh; locate the
code around refreshRecentChatsOnce and onFinish and use
isRefreshingRecentChatsRef, hasSyncedRecentChatsAfterFirstAssistantRef, and the
shared in-flight refresh promise to implement this await/attach 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 00837084-e64c-4b6d-8f67-f80936075836
📒 Files selected for processing (2)
hooks/useNewChatSidebarSync.tshooks/useVercelChat.ts
luxapientia
force-pushed
the
trongtruongpham192/myc-4598-chat-the-new-conversation-title-isnt-displayed-in-the
branch
from
76aab38 to
c47283a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
hooks/useNewChatSidebarSync.ts (1)
55-57:⚠️ Potential issue | 🟠 MajorDon’t let the streaming refresh suppress the post-finish title sync.
Line 55 marks the chat as “synced” after any successful conversations refetch, including the
"streaming"path at Line 113. If that first refetch lands before the backend has persisted/generated the real title, Line 88-Line 91 skips the later finish-time refresh and the sidebar can stay on the placeholder entry.A safer shape here is to keep the stream-time refresh as a best-effort warm-up only, and reserve the sync latch for one successful post-finish refresh on the active new-chat flow.
Possible direction
- const refreshRecentChatsOnce = useCallback(async (): Promise<boolean> => { + const refreshRecentChats = useCallback( + async ({ latchOnSuccess }: { latchOnSuccess: boolean }): Promise<boolean> => { if (hasSyncedRecentChatsAfterFirstAssistantRef.current) return false; ... const result = await refetchConversations(); - if (result.isSuccess && chatIdRef.current === startedForChatId) { + if (result.isSuccess && chatIdRef.current === startedForChatId) { + if (latchOnSuccess) { hasSyncedRecentChatsAfterFirstAssistantRef.current = true; + } return true; } return false; - }, [refetchConversations]); + }, + [refetchConversations], + );Then call it like:
- stream path →
refreshRecentChats({ latchOnSuccess: false })- finish path →
refreshRecentChats({ latchOnSuccess: true })As per coding guidelines,
hooks/**/*.ts: "Handle edge cases and errors".Also applies to: 88-91, 113-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useNewChatSidebarSync.ts` around lines 55 - 57, The current logic in useNewChatSidebarSync sets hasSyncedRecentChatsAfterFirstAssistantRef.current on any successful refresh, which lets the streaming warm-up block the later post-finish title sync; modify refreshRecentChats to accept an options object (e.g., { latchOnSuccess?: boolean }) and change its success handling so it only sets hasSyncedRecentChatsAfterFirstAssistantRef.current = true when latchOnSuccess is true; then call refreshRecentChats({ latchOnSuccess: false }) from the streaming path (the earlier call around the "streaming" branch) and refreshRecentChats({ latchOnSuccess: true }) from the finish/post-completion path so only the post-finish refresh latches the "synced" flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@hooks/useNewChatSidebarSync.ts`:
- Around line 55-57: The current logic in useNewChatSidebarSync sets
hasSyncedRecentChatsAfterFirstAssistantRef.current on any successful refresh,
which lets the streaming warm-up block the later post-finish title sync; modify
refreshRecentChats to accept an options object (e.g., { latchOnSuccess?: boolean
}) and change its success handling so it only sets
hasSyncedRecentChatsAfterFirstAssistantRef.current = true when latchOnSuccess is
true; then call refreshRecentChats({ latchOnSuccess: false }) from the streaming
path (the earlier call around the "streaming" branch) and refreshRecentChats({
latchOnSuccess: true }) from the finish/post-completion path so only the
post-finish refresh latches the "synced" flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b79de431-c884-4c5f-9d8d-ad7dc5ba680c
📒 Files selected for processing (2)
hooks/useNewChatSidebarSync.tshooks/useVercelChat.ts
feat: implement useNewChatSidebarSync for improved chat synchronization - Added a new hook, useNewChatSidebarSync, to manage synchronization of recent chats in the new chat sidebar. - Integrated the new hook into useVercelChat, replacing previous synchronization logic. - Enhanced the onFinish handler to conditionally refresh credits and recent chats based on chat status.
luxapientia
force-pushed
the
trongtruongpham192/myc-4598-chat-the-new-conversation-title-isnt-displayed-in-the
branch
from
c47283a to
da90832
Compare
sweetmantech
left a comment
sweetmantech
left a comment
There was a problem hiding this comment.
KISS principle
- actual: multiple hundred of lines of code to fix a bug I am unable to reproduce.
- required: if this is a required change, make the code simpler and document the bug so it is easier to reproduce.
2 participants
Enhanced chat synchronization and conversation refetching
Summary by cubic
Fixes the missing new conversation title in the sidebar by centralizing “new chat” sync: refresh recent chats once on first stream and after a successful finish when there’s no
roomIdor a pending new thread. Addresses Linear MYC-4598 and avoids duplicate refreshes, cross-chat races, and stale lists.Bug Fixes
idchanges.streamingon the new-chat route.Refactors
useNewChatSidebarSyncand integrated it intouseVercelChat, replacing prior sync logic.Written for commit da90832. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes