feat(extension): collapse long assistant code blocks - #4750
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryReviewed the assistant-message code-block collapse feature (pure helpers, Files Reviewed (14 files)
Reviewed by claude-sonnet-5 · Input: 32 · Output: 11.7K · Cached: 1.1M Review guidance: REVIEW.md from base branch |
pandemicsyn
left a comment
There was a problem hiding this comment.
lgtm - couple nits inline it looks like - feel free to ignore.
| {expanded ? null : ( | ||
| <div | ||
| aria-hidden="true" | ||
| className="pointer-events-none absolute inset-x-0 bottom-0 h-10 rounded-b bg-gradient-to-t from-[rgb(9_9_11)] to-transparent" |
There was a problem hiding this comment.
Neon review finding (nit): The fade-out gradient uses an opaque rgb(9 9 11) end color, but the actual <pre> background (from .agent-message-markdown :where(pre)) is rgb(9 9 11 / 70%) layered over the assistant bubble's bg-zinc-900. The two colors are close but not identical, so the fade can show a faint seam against the real background instead of blending perfectly.
Suggested fix: Use the same semi-transparent background value (or a CSS variable shared with the pre rule) for the gradient's terminal color so the fade blends exactly with the block's actual background.
Generated by Neon. Edit or delete before submitting the review.
| /> | ||
| )} | ||
| </div> | ||
| <button |
There was a problem hiding this comment.
Neon review finding (nit): The toggle button has aria-expanded but no aria-controls/id association with the code region it discloses, which is a minor accessibility gap for a disclosure widget (screen readers can't programmatically link the control to the controlled content).
Suggested fix: Give the <pre>/wrapper an id and add aria-controls on the button pointing to it, consistent with standard disclosure-widget patterns.
Generated by Neon. Edit or delete before submitting the review.
Summary
Long code blocks (>15 lines) in assistant chat messages in the Kilo browser extension side panel now render collapsed once the message finishes streaming: an 8-line preview with a fade-out and a
Show more (N lines)toggle. Tool panels, thinking blocks, and user messages are unchanged.Product decisions
preblocks only (fenced and indented uniformly; inline code untouched)Show more (N lines)/Show lesswitharia-expandedImplementation
CollapsibleCodeBlockcomponent (entrypoints/sidepanel/collapsible-code-block.ts(x)); unconditional hooks so the streaming→collapsed handoff keeps hook orderMessageEventoverrides ReactMarkdown'sprerenderer for the assistant branch only, with a defensive fallback to the default renderer on unexpected child shapesstreamingMessageIdAtomFamily+ coreonAssistantStreamingstart/end callback (start at the first content delta; end guaranteed bytry/finallyon success, abort, and error), wired through both turn runners; three clear points (core end signal, runfinally,abortConversationRun) so stop/close/delete mid-stream never leaks a stale idclearPerConversationAtomsevicts the new family (covers a conversation whose only entry is a mid-first-run streaming id)Test plan
tests/e2e/collapsible-code-blocks.test.ts): scripted-SSE assistant message with 20-line + 18-line + 3-line fenced blocks; asserts collapsed previews, exact toggle labels, per-block independent expand/collapse witharia-expanded, short block plain, no horizontal overflowfirefox-selenium-e2e.ts(scroll assertions omitted — no helper idiom in that suite)tests/e2e/kilo-api-fixture.tsmockKiloApiharness — a real model call cannot deterministically produce an exact >15-line fenced block with a stable assertable line count and timing; every existing extension rendering E2E uses this harness perapps/extension/AGENTS.md.Gate
verify(typecheck + lint + format:check + 243 unit tests),build,build:firefox,e2e:chrome(53 passed),e2e:firefox(26/26),pnpm format— all green.