feat(chat): typeable input while bootstrap prepares (#1767)#1773
feat(chat): typeable input while bootstrap prepares (#1767)#1773ahmednahima0-beep wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR centralizes chat-session provisioning in a new provider, threads an optional ChangesDraft Input and Provisioning Flow
Sequence Diagram(s)sequenceDiagram
participant NewChatBootstrap
participant ChatSessionProvisionProvider
participant useProvisionChatSession
participant useChatSessionProvision
participant NewChatPreparingShell
participant Chat
participant useVercelChat
NewChatBootstrap->>useChatSessionProvision: read provisioning state
alt provisioning pending
NewChatBootstrap->>NewChatPreparingShell: render(input=draftInput)
NewChatPreparingShell-->>NewChatBootstrap: onInputChange(newDraft)
else provisioning ready
NewChatBootstrap->>Chat: render(initialInput=draftInput)
Chat->>VercelChatProvider: forward initialInput
VercelChatProvider->>useVercelChat: initialize(input = initialInput ?? "")
useVercelChat->>ChatSessionProvisionProvider: markProvisionConsumed(chatId) on first send
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7dbb21c7d
ℹ️ 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".
| <PaymentProvider>{children}</PaymentProvider> | ||
| </ConversationsProvider> | ||
| </SidebarExpansionProvider> | ||
| <ChatSessionProvisionProvider> |
There was a problem hiding this comment.
Reset provisioning before showing another new chat
Because this provider is now mounted under the root app providers, its useProvisionChatSession mutation state survives route changes. After a user sends the first new chat and the URL is replaced with /sessions/.../chats/..., navigating back to /chat or / with the same artist/org will still expose the previous ready sessionId/chatId (the provision hook bails on identical inputs once successful), so the supposedly new composer mounts on and appends to the prior conversation instead of minting a fresh chat. Scope this state back to the new-chat page or reset/refetch it after the prepared chat is consumed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/VercelChat/NewChatPreparingShell.tsx (1)
28-69: ⚡ Quick winDuplicated layout scaffolding risks drift from the “ready” UI.
The outer scaffold here (gradient bar,
flex-1spacers,max-w-3xlwrapper,ChatGreeting) is copy-pasted fromChatContentincomponents/VercelChat/chat.tsx(the centered-greeting branch). Since the whole point is a seamless preparing→ready handoff, any future tweak to one layout will silently desync the two and produce a visible jump. Extracting a small presentational wrapper (e.g.NewChatCenteredLayout) that both screens compose keeps them in lockstep (DRY/single source of truth for this layout).🤖 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 `@components/VercelChat/NewChatPreparingShell.tsx` around lines 28 - 69, This file duplicates the outer layout scaffold (gradient bar, flex-1 spacers, max-w-3xl wrapper and ChatGreeting) that ChatContent in components/VercelChat/chat.tsx uses; extract that shared presentational wrapper (e.g. NewChatCenteredLayout) and have both NewChatPreparingShell and ChatContent compose it to avoid layout drift. Create a stateless component (NewChatCenteredLayout) that renders the gradient bar, top/bottom flex spacers, the max-width container and ChatGreeting slot/children, then replace the duplicated markup inside NewChatPreparingShell and the corresponding area in ChatContent to render <NewChatCenteredLayout>{/* inner prompt/prompt input */}</NewChatCenteredLayout>.
🤖 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 `@components/VercelChat/NewChatPreparingShell.tsx`:
- Around line 53-56: The "Preparing…" status in NewChatPreparingShell is only
visual; make it accessible by turning the span inside PromptInputToolbar into a
polite live region so screen readers announce changes. Update the element
rendering "Preparing…" (the span) to include attributes for assistive tech
(e.g., aria-live="polite" and aria-atomic="true") or wrap it in an equivalent
accessible live-region component, ensuring the text remains visually the same
while being announced to screen readers.
---
Nitpick comments:
In `@components/VercelChat/NewChatPreparingShell.tsx`:
- Around line 28-69: This file duplicates the outer layout scaffold (gradient
bar, flex-1 spacers, max-w-3xl wrapper and ChatGreeting) that ChatContent in
components/VercelChat/chat.tsx uses; extract that shared presentational wrapper
(e.g. NewChatCenteredLayout) and have both NewChatPreparingShell and ChatContent
compose it to avoid layout drift. Create a stateless component
(NewChatCenteredLayout) that renders the gradient bar, top/bottom flex spacers,
the max-width container and ChatGreeting slot/children, then replace the
duplicated markup inside NewChatPreparingShell and the corresponding area in
ChatContent to render <NewChatCenteredLayout>{/* inner prompt/prompt input
*/}</NewChatCenteredLayout>.
🪄 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: 61f9ee66-92ff-4741-8b7c-4b4a0e061c53
📒 Files selected for processing (8)
components/VercelChat/NewChatBootstrap.tsxcomponents/VercelChat/NewChatPreparingShell.tsxcomponents/VercelChat/chat.tsxhooks/useNewChatBootstrap.tshooks/useVercelChat.tsproviders/ChatSessionProvisionProvider.tsxproviders/Providers.tsxproviders/VercelChatProvider.tsx
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 4/5
- This PR looks safe to merge with minimal risk: the reported issue is moderate-low severity (4/10) and appears localized rather than broadly destabilizing.
- In
components/VercelChat/NewChatBootstrap.tsx,draftInputcan persist across re-bootstrap cycles, which may cause stale text to reappear and create confusing UX during provisioning retries. - Pay close attention to
components/VercelChat/NewChatBootstrap.tsx- ensuredraftInputis cleared or reset appropriately between bootstrap runs.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…trap logic This commit refactors the NewChatBootstrap component to utilize the ChatSessionProvisionProvider, allowing for improved management of chat session states. It introduces new hooks for resetting provisions and marking chat IDs as consumed. Additionally, it enhances the bootstrap logic to clear draft inputs based on session status changes. The NewChatPreparingShell component is updated to improve accessibility with aria-live attributes. Overall, these changes streamline chat initialization and enhance user experience.
This commit refines the NewChatBootstrap component by introducing a ref to track the reset state, ensuring that the chat session is reset only once when returning to the chat after a prior interaction. The logic for determining when to reset the provision has been streamlined, and the useEffect dependencies have been updated for better clarity. Additionally, the useProvisionChatSession hook has been modified to include a mint generation state, allowing for more controlled session resets. These changes improve the overall reliability and user experience of the chat initialization process.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@components/VercelChat/NewChatBootstrap.tsx`:
- Around line 22-39: The component currently lets <Chat> mount before the
useEffect resets a stale provision; before rendering <Chat> add a guard that
checks the same stale-ready condition (state.status === "ready" &&
consumedChatId !== null && state.chatId === consumedChatId &&
!staleResetStartedRef.current) and, when true, call resetProvision() (or set a
local "isResetting" flag) and return a placeholder (null or loader) so <Chat> is
not rendered until the stale reset completes; update both places that render
<Chat> to use this pre-render gate and reuse staleResetStartedRef, state,
consumedChatId, and resetProvision to locate the logic.
In `@hooks/useVercelChat.ts`:
- Around line 316-320: The code currently calls markProvisionConsumed(id) (and
updates URL/sidebar) before the send is confirmed, which can rotate provision or
update UI for a chat that never sends; fix by deferring provision consumption
and URL/sidebar updates until after the send is proven to start/succeed: in the
new-chat branches that call addOptimisticConversation("New Chat", id, sessionId,
messageContent) and silentlyUpdateUrl(), remove/hold markProvisionConsumed(id)
there and instead call it only after awaiting the actual send flow (await
handleSubmit(event) in handleSendMessage and after getHeaders() and the send
promise resolves in handleSendQueryMessages). Also mirror this change for the
other new-chat branch (lines ~324-341) so both paths only call
markProvisionConsumed(id) post-success.
🪄 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: 966abf85-4293-4d57-8bf5-28afd148f9a5
📒 Files selected for processing (6)
components/VercelChat/NewChatBootstrap.tsxcomponents/VercelChat/NewChatPreparingShell.tsxhooks/sessions/useProvisionChatSession.tshooks/useNewChatBootstrap.tshooks/useVercelChat.tsproviders/ChatSessionProvisionProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- providers/ChatSessionProvisionProvider.tsx
- hooks/useNewChatBootstrap.ts
- components/VercelChat/NewChatPreparingShell.tsx
| const { state, consumedChatId, resetProvision } = useChatSessionProvision(); | ||
| const [draftInput, setDraftInput] = useState(""); | ||
| const staleResetStartedRef = useRef(false); | ||
|
|
||
| // Returning to `/` or `/chat` after a prior first-send still has a | ||
| // successful mutation in the root provider — reset once on mount so | ||
| // we mint a fresh session instead of appending to the old chat. | ||
| useEffect(() => { | ||
| if (staleResetStartedRef.current) return; | ||
| if ( | ||
| state.status === "ready" && | ||
| consumedChatId !== null && | ||
| state.chatId === consumedChatId | ||
| ) { | ||
| staleResetStartedRef.current = true; | ||
| resetProvision(); | ||
| } | ||
| }, [state, consumedChatId, resetProvision]); |
There was a problem hiding this comment.
Gate stale ready state before rendering <Chat>.
This reset runs too late to prevent the old provision from mounting. On a return to / or /chat with state.status === "ready" and state.chatId === consumedChatId, the component still renders <Chat> once and only resets it in the effect afterward. That can briefly show the previous chat and lets child mount effects run against the stale session.
Suggested fix
export default function NewChatBootstrap({
initialMessages,
}: NewChatBootstrapProps) {
const { state, consumedChatId, resetProvision } = useChatSessionProvision();
const [draftInput, setDraftInput] = useState("");
const staleResetStartedRef = useRef(false);
+ const hasStaleReadyProvision =
+ state.status === "ready" &&
+ consumedChatId !== null &&
+ state.chatId === consumedChatId;
@@
useEffect(() => {
if (staleResetStartedRef.current) return;
- if (
- state.status === "ready" &&
- consumedChatId !== null &&
- state.chatId === consumedChatId
- ) {
+ if (hasStaleReadyProvision) {
staleResetStartedRef.current = true;
resetProvision();
}
- }, [state, consumedChatId, resetProvision]);
+ }, [hasStaleReadyProvision, resetProvision]);
@@
- if (state.status === "ready") {
+ if (state.status === "ready" && !hasStaleReadyProvision) {
return (
<Chat
id={state.chatId}
sessionId={state.sessionId}
initialInput={draftInput}Also applies to: 48-57
🤖 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 `@components/VercelChat/NewChatBootstrap.tsx` around lines 22 - 39, The
component currently lets <Chat> mount before the useEffect resets a stale
provision; before rendering <Chat> add a guard that checks the same stale-ready
condition (state.status === "ready" && consumedChatId !== null && state.chatId
=== consumedChatId && !staleResetStartedRef.current) and, when true, call
resetProvision() (or set a local "isResetting" flag) and return a placeholder
(null or loader) so <Chat> is not rendered until the stale reset completes;
update both places that render <Chat> to use this pre-render gate and reuse
staleResetStartedRef, state, consumedChatId, and resetProvision to locate the
logic.
| if (!chatId) { | ||
| // New chat from `/` or `/chat` — sidebar + URL update on first send. | ||
| addOptimisticConversation("New Chat", id, sessionId, messageContent); | ||
| markProvisionConsumed(id); | ||
| silentlyUpdateUrl(); |
There was a problem hiding this comment.
Only consume the provision after the first send path succeeds.
Both new-chat paths advance provisioning before the send is confirmed to start. In handleSendMessage, handleSubmit(event) is not awaited; in handleSendQueryMessages, markProvisionConsumed(id) runs before getHeaders(). If either path fails, the provider still rotates to the next provision and the URL/sidebar can move to a chat that never actually sent its first message.
Suggested fix
const handleSendMessage = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
@@
- // Submit the message
- handleSubmit(event);
+ // Submit the message
+ await handleSubmit(event);
if (!chatId) {
// New chat from `/` or `/chat` — sidebar + URL update on first send.
addOptimisticConversation("New Chat", id, sessionId, messageContent);
markProvisionConsumed(id);
silentlyUpdateUrl();
}
};
const handleSendQueryMessages = useCallback(
async (initialMessage: UIMessage) => {
- if (!chatId) {
- markProvisionConsumed(id);
- }
- silentlyUpdateUrl();
const headers = await getHeaders();
sendMessage(initialMessage, { body: chatRequestBody, headers });
+ if (!chatId) {
+ markProvisionConsumed(id);
+ silentlyUpdateUrl();
+ }
},Also applies to: 324-341
🤖 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 `@hooks/useVercelChat.ts` around lines 316 - 320, The code currently calls
markProvisionConsumed(id) (and updates URL/sidebar) before the send is
confirmed, which can rotate provision or update UI for a chat that never sends;
fix by deferring provision consumption and URL/sidebar updates until after the
send is proven to start/succeed: in the new-chat branches that call
addOptimisticConversation("New Chat", id, sessionId, messageContent) and
silentlyUpdateUrl(), remove/hold markProvisionConsumed(id) there and instead
call it only after awaiting the actual send flow (await handleSubmit(event) in
handleSendMessage and after getHeaders() and the send promise resolves in
handleSendQueryMessages). Also mirror this change for the other new-chat branch
(lines ~324-341) so both paths only call markProvisionConsumed(id) post-success.
There was a problem hiding this comment.
2 issues found across 6 files (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="components/VercelChat/NewChatBootstrap.tsx">
<violation number="1" location="components/VercelChat/NewChatBootstrap.tsx:22">
P3: Unstable `state` object in the stale-reset effect dependency array — depending on the whole object instead of the primitives used in the condition causes unnecessary effect re-execution on every render when the reference changes.</violation>
</file>
<file name="hooks/sessions/useProvisionChatSession.ts">
<violation number="1" location="hooks/sessions/useProvisionChatSession.ts:33">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
File exceeds the 100-line code limit after this change (103 lines), violating the code-structure size rule.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| orgId: string | undefined; | ||
| } | ||
|
|
||
| export interface UseProvisionChatSessionResult { |
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
File exceeds the 100-line code limit after this change (103 lines), violating the code-structure size rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/sessions/useProvisionChatSession.ts, line 33:
<comment>File exceeds the 100-line code limit after this change (103 lines), violating the code-structure size rule.</comment>
<file context>
@@ -30,6 +30,12 @@ interface UseProvisionChatSessionInput {
orgId: string | undefined;
}
+export interface UseProvisionChatSessionResult {
+ state: ProvisionChatSessionState;
+ /** Clears the mutation so the next enabled cycle mints a fresh session. */
</file context>
| initialMessages, | ||
| }: NewChatBootstrapProps) { | ||
| const state = useNewChatBootstrap(); | ||
| const { state, consumedChatId, resetProvision } = useChatSessionProvision(); |
There was a problem hiding this comment.
P3: Unstable state object in the stale-reset effect dependency array — depending on the whole object instead of the primitives used in the condition causes unnecessary effect re-execution on every render when the reference changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/VercelChat/NewChatBootstrap.tsx, line 22:
<comment>Unstable `state` object in the stale-reset effect dependency array — depending on the whole object instead of the primitives used in the condition causes unnecessary effect re-execution on every render when the reference changes.</comment>
<file context>
@@ -19,8 +19,31 @@ interface NewChatBootstrapProps {
initialMessages,
}: NewChatBootstrapProps) {
- const state = useNewChatBootstrap();
+ const { state, consumedChatId, resetProvision } = useChatSessionProvision();
const [draftInput, setDraftInput] = useState("");
+ const staleResetStartedRef = useRef(false);
</file context>
sweetmantech
left a comment
There was a problem hiding this comment.
Took a close pass focused on the new-chat preparing flow. Two correctness/UX regressions that I think block (#1 spinner flash at handoff, #2 stale consumed-session render), one infra/cost decision to make consciously (#3 eager sandbox per login), and two quality items (#4 focus seam + duplicated input, #5 hook coupling). Details inline. Overall direction is solid — keeping sessionId required and provisioning early is the right call; these are about the handoff edges.
| * `chatId` its validator requires (recoupable/chat#1747). | ||
| * New-chat entry for `/` and `/chat`. Provisioning runs at app level | ||
| * (`ChatSessionProvisionProvider`); while ids are pending this shows a | ||
| * typeable input instead of a full-screen spinner. `<Chat>` mounts only |
There was a problem hiding this comment.
The “typeable input instead of a full-screen spinner” goal regresses at the shell→Chat handoff.
useVercelChat still calls the message loader unchanged (hooks/useVercelChat.ts:284-286):
useMessageLoader(sessionId, messages.length === 0 ? id : undefined, ...)When <Chat> mounts on ready with the brand-new chat id, useMessageLoader initializes useState(!!chatId) → true, so ChatContentMemoized hits if (isLoading) return <Loader/> and shows a full-screen spinner while it fetches history for a chat that has none. Net UX:
shell (with typed text) → full spinner (text vanishes) → empty Chat with text restored
Fix: don’t load history for a bootstrap-created chat — gate the loader on the route chatId so it only runs on the canonical /sessions/[sessionId]/chats/[chatId] route:
const { chatId: routeChatId } = useParams<{ chatId?: string }>();
useMessageLoader(
sessionId,
messages.length === 0 && routeChatId ? id : undefined,
userId,
setMessages,
);| } | ||
| }, [state.status]); | ||
|
|
||
| if (state.status === "ready") { |
There was a problem hiding this comment.
Stale consumed-session renders for a frame when returning here.
The reset runs in a useEffect, which fires after the first paint. So when the user clicks “New Chat” after a prior send (the app-level provider still holds ready with chatId === consumedChatId), this first render returns <Chat id={consumedChatId}>. That mounts the already-used chat and fires a message fetch for it — briefly surfacing the previous conversation — before the reset effect swaps to a fresh session.
Fix: treat “ready but consumed” as not-ready during render so the stale Chat never mounts; keep the effect to trigger the reset:
const isConsumed =
state.status === "ready" && consumedChatId === state.chatId;
if (state.status === "ready" && !isConsumed) {
return <Chat id={state.chatId} sessionId={state.sessionId} initialInput={draftInput} .../>;
}
// otherwise fall through to <NewChatPreparingShell/>; the reset effect still fires| <PaymentProvider>{children}</PaymentProvider> | ||
| </ConversationsProvider> | ||
| </SidebarExpansionProvider> | ||
| <ChatSessionProvisionProvider> |
There was a problem hiding this comment.
Mounting this above the whole tree provisions a sandbox on every login for every user.
POST /api/sessions + POST /api/sandbox now fire as soon as auth + artist roster are ready — including for users who go straight to an existing chat, files, or settings and never start a new chat. The sandbox is the expensive resource: measured POST /api/sandbox ≈ 10.9s cold plus compute, so this eagerly burns a sandbox per login that often goes unused.
Worth a deliberate decision. Options:
- (a) scope provisioning to a new-chat surface rather than app-wide, or
- (b) eagerly create the session row but defer
createSandboxuntil the new-chat view actually mounts.
If we keep it app-wide, let’s at least confirm idle sandboxes hibernate/expire cheaply.
| "shadow-sm", | ||
| )} | ||
| > | ||
| <FileMentionsInput |
There was a problem hiding this comment.
Focus seam + duplicated/reduced input.
- The shell’s textarea is a different DOM node from the real
ChatInput, so focus drops at the shell→Chat swap (and a keystroke in the commit/unmount window can be lost). At minimum, autofocus on handoff. - This shell omits the attachments button, model select, and @-mention resolution that
ChatInputhas, and duplicates its markup. ReusingChatInputin a “send-disabled” mode (e.g. anisBootstrapPreparingflag on the chat context) would remove the duplication and the focus seam at once, while keeping full input capability during preparing.
| const artistId = selectedArtist?.account_id; | ||
| const messagesLengthRef = useRef<number>(); | ||
| const { addOptimisticConversation } = useConversationsProvider(); | ||
| const { markProvisionConsumed } = useChatSessionProvision(); |
There was a problem hiding this comment.
This couples the core chat hook to ChatSessionProvisionProvider.
useChatSessionProvision() throws if <Chat> is ever rendered outside that provider (unit tests, Storybook, future embeds). Consider making the context optional — return a no-op markProvisionConsumed when the provider is absent — or pass markProvisionConsumed down as an optional prop so useVercelChat stays decoupled.
Summary by cubic
Users can type while a new chat session provisions. Provisioning runs at the app level; typed text is restored on mount; the next session is queued after first send; and stale provisions reset once when returning.
New Features
NewChatPreparingShellwith a typeable input and “Preparing…” state; Send is disabled and announced via aria-live until ready.initialInputthroughChat→VercelChatProvider→useVercelChat.NewChatBootstrapwith the preparing shell.Refactors
ChatSessionProvisionProvider;NewChatBootstrapresets a stale provision once on mount after a prior send and clears draft input on idle/bootstrapping;useNewChatBootstrapis read-only.useProvisionChatSessionnow returns{ state, reset }and uses a mint-generation counter to force fresh sessions on reset;useVercelChatmarks a provision as consumed on first send (including query flow); updatedProvidersto include the new provider.Written for commit c560e93. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements