fix(chat): drop the streaming cursor that detached onto its own line - #4507
Merged
Conversation
The cursor rendered as a sibling of the markdown container. That placement was chosen deliberately — it is the only way to show a caret without injecting into the sanitized HTML string — but the container is a block-level <div>, so the span could never trail the last rendered line and instead wrapped onto a row of its own. What the reader saw was a detached grey bar sitting between the message and whatever followed it, after every streaming message that had rendered markdown. Reported from the running app. Restoring a caret would mean appending inside the last inline descendant of the sanitized HTML, which is exactly the injection the sibling placement existed to avoid. Progress is already legible from the text growing, so the affordance goes rather than gets moved. The plain-text fallback cursor goes with it: it only shows for the window before the first render lands, so keeping it would flash a caret for a moment and then lose it for the rest of the stream. `pending` still drives progressive rendering; only the glyph is gone. The two source-text assertions that required the cursor now assert its absence at both sites, so it cannot creep back without the block-container problem being solved first.
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the streaming cursor glyph from MessageBubble markdown/plain-text rendering to eliminate a detached “grey bar” artifact caused by rendering the cursor as a sibling of a block-level markdown container.
Changes:
- Remove the pending-gated cursor glyph from both the plain-text fallback and sanitized-HTML markdown render paths in
MarkdownContent. - Update markdown-source tests to assert the cursor glyph is absent and prevent reintroduction without solving the layout constraint.
- Update session-debug assertions/comments to reflect that progressive rendering remains, but the cursor glyph is gone.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/components/message-bubble.tsx |
Removes the cursor glyph rendering in MarkdownContent; adds explanatory inline comment about why it’s removed. |
src/components/message-bubble-markdown.test.ts |
Updates source-text assertions to enforce absence of the cursor glyph and sibling pending-gated cursor span. |
src/lib/session-debug.test.ts |
Updates pinned description/assertion text to remove cursor-glyph wording while keeping progressive-render behavior checks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two comments still described the streaming cursor as if it existed — the
`segments` prop doc ("progressive markdown + cursor") and the assistant
segment comment ("the cursor shows on at most one span"). Both would have
sent the next reader looking for a glyph that is no longer rendered.
Also shortens the block comment at the removal site. The reasoning that
matters is why it cannot simply be re-added as a sibling; the rest belonged
in the PR description, where it already is.
Reviewer flagged both on #4507.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from the running app: a small grey bar sitting on its own line between
a message and whatever followed it.
What it was
The streaming cursor (
▌) rendered as a sibling of the markdown container:That placement was chosen deliberately — it is the only way to show a caret
without injecting into the sanitized HTML string — but
.cave-mdis ablock-level
<div>, so the span can never sit at the end of the last renderedline. It necessarily wraps onto a row of its own.
So this was not specific to one message. It appeared after every streaming
message that had rendered markdown, and read as a rendering artifact rather
than as a caret.
Why remove rather than reposition
Putting a caret back at the end of the text means appending inside the last
inline descendant of the sanitized HTML — exactly the injection the sibling
placement existed to avoid. Progress is already legible from the text growing,
so the affordance goes rather than gets moved.
The plain-text fallback cursor goes with it. It only shows for the window
before the first render lands, so keeping it would flash a caret for a moment
and then lose it for the rest of the stream — a flicker, not an affordance.
pendingstill drives progressive rendering; only the glyph is gone.Tests
Two source-text assertions previously required the cursor and would have
failed this change. They now assert its absence at both sites, so it cannot
creep back without the block-container problem being solved first. A third
assertion in
session-debug.test.tsstill pins that only the last text spanstreams — that behaviour is unchanged; only its wording mentioned the glyph.
Verified:
pnpm test:app(1161 files),pnpm lint,pnpm typecheck.Closes cave-1yslk.