fix(platform): keep cursor visible during typewriter buffer drain#551
Conversation
The cursor was tied to isStreaming, which flips to false the moment the server marks the message as complete — even though the typewriter buffer still has seconds of text left to reveal at 50 CPS. Fix: tie showCursor to isTyping from useStreamBuffer so the cursor stays visible until the last character is revealed. Also add isCurrentlyTyping check to IncrementalMarkdown's isLastElement logic to handle mid-block reveal (endOffset > revealedLen), and add tests for the partial-reveal cursor case that was previously untested.
Greptile SummaryFixed cursor visibility during typewriter buffer drain by switching from
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| services/platform/app/features/chat/components/typewriter-text.tsx | Changed showCursor to use isTyping from useStreamBuffer instead of isStreaming, ensuring cursor stays visible during buffer drain |
| services/platform/app/features/chat/components/incremental-markdown.tsx | Added isCurrentlyTyping check to handle mid-block cursor placement when parser hasn't closed the element yet |
| services/platform/app/features/chat/components/tests/incremental-markdown.test.tsx | Added comprehensive test suite for cursor behavior during partial reveal (mid-paragraph, post-anchor, mid-list, mid-heading scenarios) |
Last reviewed commit: 1c89fea
📝 WalkthroughWalkthroughThis PR addresses cursor visibility issues during the buffer-drain phase of streaming responses. It modifies IncrementalMarkdown to detect when text is being actively typed mid-block by considering both start and end offsets of node positions, introducing Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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
`@services/platform/app/features/chat/components/__tests__/incremental-markdown.test.tsx`:
- Around line 153-247: Tests never exercise the new isCurrentlyTyping branch
because StreamingMarkdown slices content before parsing (revealedContent =
content.slice(0, revealedLength)), so parser AST node endOffset can never be >
revealedLen; to fix add a targeted test that triggers isCurrentlyTyping either
by (A) testing the lower-level helper that checks isCurrentlyTyping/endOffset
directly (call the function that uses endOffset and revealedLen) or (B) mocking
the markdown parser used by StreamingMarkdown to return an AST node with
endOffset > revealedLen (so isCurrentlyTyping becomes true), referencing
StreamingMarkdown, revealedContent/revealedLen, endOffset and isCurrentlyTyping
to locate the code under test.
In `@services/platform/app/features/chat/components/incremental-markdown.tsx`:
- Around line 221-241: Remove the unreachable mid-block guard and its use:
delete the isCurrentlyTyping computation (the const isCurrentlyTyping = ...
block) and update the isLastElement expression in incremental-markdown.tsx to no
longer reference isCurrentlyTyping, relying instead on the existing endOffset
=== revealedLen and the trim-based check; ensure revealedLenRef/current and
hasCursorEligibleChild logic remain unchanged and run the existing cursor tests
to confirm behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
services/platform/app/features/chat/components/__tests__/incremental-markdown.test.tsxservices/platform/app/features/chat/components/incremental-markdown.tsxservices/platform/app/features/chat/components/typewriter-text.tsx
- Hide ThinkingAnimation when text is streaming with no active tools; typewriter text already signals progress, showing both is redundant - Expose isDraining from useStreamBuffer so cursor stays visible during the post-stream drain phase, not just while the server is sending - Skip cursor injection into empty elements (e.g. trailing <p> from \n) to prevent a stray cursor appearing on a blank line after content - Update thinking-animation test to assert the new null-return behavior
Closes #550
Summary
showCursorintypewriter-text.tsxwas tied toisStreaming, which flips tofalsethe moment the server marks the message complete — even though the buffer still has seconds of text left to drain at 50 CPS. Fixed by tyingshowCursortoisTypingfromuseStreamBufferinstead, so the cursor stays visible until the last character is revealed.isCurrentlyTypingcheck toIncrementalMarkdown'sisLastElementlogic to handle the mid-block case whereendOffset > revealedLen(parser hasn't closed the element yet), which previously caused no cursor to be placed."cursor during partial reveal (mid-stream)"test suite covering mid-paragraph, post-anchor, mid-list, mid-heading, and multi-reveal cases — all previously untested.Test plan
npm run test:ui --workspace=@tale/platform— all 16 tests passSummary by CodeRabbit
Release Notes
Bug Fixes
Tests