feat(platform): polish chat streaming UX and improve resilience#512
Conversation
- Refine thinking animation to only show tool details before text appears and filter out completed tool parts - Add streaming skeleton to next-steps follow-up section - Skip thread loading in search dialog when closed - Make paginationOpts optional to handle Convex reconnection edge cases - Wrap ChatHeader in error boundary for graceful failure - Add tests for ThinkingAnimation and NextStepsSection
Greptile SummaryThis PR polishes the chat streaming UX across several dimensions: the
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| services/platform/app/features/chat/components/thinking-animation.tsx | Refactored to filter out completed tool parts and only show detailed tool status before text streaming begins. Contains a pre-existing no-op ternary in query slicing logic. |
| services/platform/app/features/chat/components/structured-message/section-renderers.tsx | Added isStreaming prop to NextStepsSection for showing a skeleton placeholder while streaming, with proper memo comparison and accessibility attributes. |
| services/platform/app/features/chat/components/structured-message/structured-message.tsx | Passes isStreaming={isActiveSection} to NextStepsSection; minimal one-line change, correct usage. |
| services/platform/app/features/chat/components/chat-search-dialog.tsx | Added skip: !isOpen to useThreads to avoid loading all threads when the search dialog is closed. Clean performance optimization. |
| services/platform/app/features/chat/hooks/queries.ts | Added skip parameter to useThreads and a type assertion to work around Convex type mismatch from making paginationOpts optional. The type cast bypasses compile-time safety. |
| services/platform/convex/threads/queries.ts | Made paginationOpts optional with a fallback default to handle Convex WebSocket reconnection edge cases. Hardcoded fallback numItems (20) duplicates THREADS_PAGE_SIZE constant. |
| services/platform/app/routes/dashboard/$id/chat.tsx | Wrapped ChatHeader in LayoutErrorBoundary for graceful failure handling. Clean, minimal change consistent with existing patterns in the file. |
| services/platform/app/features/chat/components/tests/thinking-animation.test.tsx | New test suite for ThinkingAnimation covering default state, active/completed tools, text streaming fallback, and tool-invocation filtering. Good coverage. |
| services/platform/app/features/chat/components/structured-message/tests/section-renderers.test.tsx | New test suite for NextStepsSection covering rendering, empty state, streaming skeleton visibility. Clean and thorough. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Chat Message Stream] --> B{Has tool parts?}
B -->|Yes| C{Tool state?}
B -->|No| D[Show 'Thinking...']
C -->|input-streaming / input-available| E{Text already streaming?}
C -->|output-available / other| F[Filter out completed tool]
F --> B
E -->|No| G[Show detailed tool status]
E -->|Yes| D
D --> H[Render ThinkingAnimation]
G --> H
I[NextStepsSection] --> J{Has items OR isStreaming?}
J -->|No| K[Return null]
J -->|Yes| L[Render follow-up buttons]
L --> M{isStreaming?}
M -->|Yes| N[Show skeleton placeholder]
M -->|No| O[No skeleton]
P[ChatSearchDialog] --> Q{isOpen?}
Q -->|Yes| R["useThreads({ skip: false })"]
Q -->|No| S["useThreads({ skip: true })"]
S --> T[Skip Convex query]
Last reviewed commit: 57e10ea
📝 WalkthroughWalkthroughThis PR refactors chat component streaming behavior and data loading patterns. Changes include reworking ThinkingAnimation to filter and display only active tool states (state: 'input-streaming' or 'input-available'), adding streaming-state awareness to NextStepsSection via a new isStreaming prop, making thread data loading conditional in ChatSearchDialog through a skip parameter, and making backend paginationOpts optional. Accompanying test files provide coverage for ThinkingAnimation and NextStepsSection components, and ChatHeader is wrapped with an error boundary. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/chat/components/__tests__/thinking-animation.test.tsx`:
- Around line 61-73: The test fixture makeStreamingMessage should validate the
base object against the UIMessage type at compile time: create a const base
message object with the fixed fields (id, key, order, role, status, text,
_creationTime, parts) and use the TypeScript "satisfies UIMessage" assertion on
that base, then return { ...base, ...overrides } so overrides can change fields
but the base is checked; update the makeStreamingMessage function to build a
satisfies-checked base and spread overrides into the returned object
(referencing makeStreamingMessage and UIMessage).
In
`@services/platform/app/features/chat/components/structured-message/__tests__/section-renderers.test.tsx`:
- Around line 28-33: Add a unit test in the existing section-renderers.test.tsx
suite that verifies NextStepsSection returns null when onSendFollowUp is not
provided: render <NextStepsSection content="Option A" /> (no onSendFollowUp
prop) and assert container.innerHTML is an empty string; place this alongside
the existing "returns null when no items and not streaming" test to cover the
missing-callback edge case.
In `@services/platform/app/features/chat/components/thinking-animation.tsx`:
- Around line 138-144: The current fragile prefix extraction in
thinking-animation.tsx—creating searchingPrefix by calling
t('thinking.searching', { query: '' }).replace('""','')—should be replaced with
a robust approach: either define the searching prefix explicitly (e.g., add a
dedicated translation key like 'thinking.searching_prefix') or change the
translation to include a stable placeholder token (e.g., '{{QUERY}}') and
compute the prefix by calling t('thinking.searching') then removing that token
(or using a regex to split on the token) so uniqueDisplayTexts.every(text =>
text.startsWith(searchingPrefix)) reliably detects search messages; update
references to searchingPrefix and the startsWith check accordingly.
- Around line 147-152: The slice end index in the mapping that builds queries is
using a no-op conditional (text.endsWith('"') ? text.length : text.length);
update the logic in the uniqueDisplayTexts.map callback (the queries
computation) to actually strip a trailing quote when present by using
text.length - 1 as the end index when text.endsWith('"'), otherwise use
text.length; keep the start index as searchingPrefix.length and ensure the slice
removes only the trailing quote if it exists.
- Around line 164-168: The branch in thinking-animation.tsx using
uniqueDisplayTexts.length <= 2 is incorrect because it assumes two items; change
that condition to uniqueDisplayTexts.length === 2 and add an explicit branch to
handle the single-unique-tool case (uniqueDisplayTexts.length === 1) so
displayText is set using only uniqueDisplayTexts[0] (e.g., a single-tool
translation key or a plain display) before falling back to the existing
multi-tool and default logic; update references to uniqueDisplayTexts and
displayText accordingly.
In `@services/platform/convex/threads/queries.ts`:
- Around line 26-29: The default page size 20 is a magic number — extract it to
a named constant (e.g., THREADS_PAGE_SIZE) and use it when building the default
paginationOpts in the call to listThreadsHelper; if there is an existing shared
constant file import THREADS_PAGE_SIZE there, otherwise declare and export
THREADS_PAGE_SIZE in this module and replace the hardcoded 20 in
threads/queries.ts so the call that passes paginationOpts (used with
args.paginationOpts ?? { cursor: null, numItems: THREADS_PAGE_SIZE }) references
the constant.
- Extract query reference to own line so oxlint-disable covers the type assertion for optional paginationOpts reconnection workaround - Add missing toolCallId and stepOrder to test fixtures for UIMessage type compliance
Summary
useThreads({ skip: !isOpen }))paginationOptsoptional inlistThreadsquery to handle edge cases where Convex replays subscriptions with empty args during WebSocket reconnectionChatHeaderinLayoutErrorBoundaryfor graceful failureThinkingAnimationandNextStepsSectionTest plan
ThinkingAnimation(default state, active tools, completed tools, text streaming fallback)NextStepsSection(rendering items, empty state, streaming skeleton)🤖 Generated with Claude Code
Summary by CodeRabbit