improve(chat): make prose and simple lists one selectable text block - #997
Conversation
…selection Chat prose was rendered as many separate TextBlocks, so a user could not drag-select text across paragraphs of one message. Add a poolable RichTextBlock primitive to the in-house FunctionalUI reconciler and route both assistant and user message text through one RichTextBlock per message (a Paragraph per block), giving each message a single continuous, selection-friendly text scope while keeping chat virtualizable. - FunctionalUI: RichTextBlockElement record, RichTextBlock() factory, Set(Action<RichTextBlock>), reconciler arm, ConfigureRichTextBlock (leaves Blocks to the setter), ApplyModifiers case. - ChatMarkdownRenderer: coalesce consecutive paragraph/heading blocks into one RichTextBlock; lone text blocks stay TextBlocks; lists/code/tables/etc stay separate selectable siblings. Structural block-run cache preserves active selection across re-renders independent of AST-cache eviction. Inert posture preserved (AppendInlines only). - OpenClawChatTimeline: user bubble renders as a single-Paragraph RichTextBlock preserving font, foreground, wrap and SelectionHighlightColor. - Tests: FunctionalUI primitive tests, renderer coalescing tests, updated user-bubble contract + virtualization proof text collection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extend the assistant-bubble text coalescing so simple (text-only, possibly nested) bullet/ordered/task lists flow into the same per-message RichTextBlock as surrounding prose, making paragraphs + headings + list items one continuous drag-selection scope. Lists whose items carry a code block, table, blockquote, thematic break, or raw block stay as their own Grid-based selectable island (RenderList), preserving that chrome. List items render as hanging-indent Paragraphs inside the shared RichTextBlock: a per-item marker (task box, ordered number, bullet), fixed hanging indent so wrapped lines / continuation paragraphs align at the content column, per-level indentation for nesting, and heading formatting preserved for heading list items. Extend the selection-preservation cache (BlockEqual) with structural ListEqual so the bounded AST cache evicting and re-parsing an unchanged message does not force a Blocks rebuild that would wipe the active selection. Update MarkdownRendererCoalesceTests / MarkdownRendererListTests: simple lists now assert RichTextBlock coalescing; the issue openclaw#636 Grid wrap guards retarget to complex (blockquote-bearing) lists that still use the Grid island path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 3:25 PM ET / 19:25 UTC. Summary Reproducibility: not applicable. as a bug reproduction: this PR proposes a new selection-scope capability. The submitted live WinRT UI-thread fixture provides a high-confidence verification path for the intended renderer structure and island boundaries. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Accept or reject the narrowly scoped FunctionalUI extension explicitly; if accepted, merge with the current renderer and live-UI contract tests intact, leaving whole-transcript selection and any broader Reactor migration to their linked follow-ups. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this PR proposes a new selection-scope capability. The submitted live WinRT UI-thread fixture provides a high-confidence verification path for the intended renderer structure and island boundaries. Is this the best way to solve the issue? Unclear pending maintainer direction. The renderer approach is narrowly implemented and covered, but adding a reusable primitive to FunctionalUI must be weighed against the open Reactor/architecture follow-up. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 081f307a4561. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
Thanks for the careful read. I traced this on its merits and it does not apply here, because the premise -- one pooled Every timeline row is emitted with Two additional backstops make the handoff unreachable even under recycling:
Given that, unifying or cross-invalidating the two caches would add complexity to guard a state that isn't reachable, so I'm intentionally not making that change. If a maintainer wants the invariant pinned down defensively, I'm happy to add an alternating user/assistant regression test that asserts the two paths never share a control instance. On the real-behavior-proof ask: the PR body documents honestly that live drag-selection video/screenshot was not captured in this automated session (the isolated app was relaunched for manual verification). The behavior is covered by the record-level renderer tests (prose plus simple list coalesce into one |
The code review was favorable. In particular, the reported cross-writer cache contamination does not appear reachable: Please add a current-head screenshot or short recording that visibly shows an active drag selection spanning:
all as one continuous selection. Please also show that a fenced code block or table remains a separate selection island, because preserving those boundaries is part of the implementation contract. A useful self-test recipe for the PR body:
If reproducing that message through a live gateway is inconvenient, please provide the exact prompt/session fixture you used. A maintainer can then open the PR branch in an isolated session and run the same steps locally. A small deterministic FunctionalUI/demo fixture would also be acceptable if it exercises the shipping renderer rather than a mock rendering path. The fixed hanging indent for very large ordered markers and reduced screen-reader item granularity are reasonable follow-up concerns, not blockers for this focused change. The visible current-head selection proof is the item needed now. |
Add a deterministic UI-thread proof that mounts the shipping OpenClawChatTimeline through the real FunctionalUI reconciler and renders one assistant message mixing prose paragraphs, a bullet list, a trailing paragraph, a fenced code block, and a table. It asserts prose + list + trailing coalesce into a single RichTextBlock (one continuous selection scope) while the code block and table remain their own island TextBlocks. This closes the record-level gap where headless WinRT could not activate FontFamily for code/table islands, and serves as the automated stand-in for the interactive native drag-select recording requested in review. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Thanks for confirming the cross-writer cache path is unreachable, and for offering the deterministic fixture as an acceptable substitute. The live native drag-select recording is an interactive pointer gesture that this automated session cannot capture, so I have taken you up on the deterministic option. New proof (current head This is not a mock rendering path. It mounts the real Deterministic message fixture (NATO tokens so the tree-walk is unambiguous):
|
Related: #995
Related: #996
What Problem This Solves
In the tray chat timeline, a message's text was fragmented into many separate text controls: each paragraph, heading, and list item was its own box. A user could not drag-select a whole assistant reply in one gesture, which was awkward and annoying when trying to copy part of an answer.
Why This Change Was Made
Each chat message now renders through a single per-message
RichTextBlock, and consecutive prose (paragraphs + headings) plus simple text-only lists (bullet / ordered / task, including nested) coalesce into that one control, so the whole run is one continuous drag-selection scope.Blocks that need their own chrome stay as separate selectable islands: code blocks, tables, block quotes, thematic breaks, and any list whose items contain one of those. This split is a WinUI constraint, not a preference:
InlineUIContaineris excluded fromSelectedTextand breaks selection at its boundary, and there is no built-in way to run one selection across multiple controls. So chrome-bearing content cannot be embedded inline without losing selection.Supporting pieces:
RichTextBlockprimitive added to the in-houseOpenClawTray.FunctionalUIlayer (factory,Set, modifiers).RichTextBlock(per-item marker, fixed hanging indent so wrapped lines align at the content column, per-level nesting, heading formatting preserved).The larger follow-ups are tracked separately: a custom cross-control selection engine for whole-transcript selection (#995), and porting FunctionalUI to Reactor to stop hand-maintaining bespoke primitives (#996).
User Impact
Users can drag-select an assistant reply's prose and bullet / numbered lists as one continuous selection instead of fighting per-line boxes. Code blocks and tables stay distinctly styled and individually selectable. No change to how messages are authored or sent.
Evidence
Record-level renderer tests assert the Element shape directly: prose + a simple list collapses to one
RichTextBlockElement, while a list containing a non-text block stays its own island. A live-runtime UI-thread test (ChatBubbleSelectionScopeProofTests) additionally proves the shipping renderer on a real WinRT thread. Full required suites plus focused renderer tests pass (counts below).Change Type
Scope
winnodeValidation
All run on branch head
d9e2b995(ARM64 host,-p:Platform=x64for WinUI/UITests):./build.ps1-> all 5 projects built successfullydotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj-> Passed 2754, Skipped 31, Failed 0dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj-> Passed 1705, Failed 0dotnet test ./tests/OpenClawTray.FunctionalUI.Tests/...-> Passed 14, Failed 0dotnet test ./tests/OpenClaw.Tray.UITests/...-> Passed 107, Failed 0 (adds the deterministic selection-scope proof on headd9e2b995)--filter FullyQualifiedName~MarkdownRenderer-> Passed 12, Failed 0Real Behavior Proof
Deterministic renderer proof (head
d9e2b995).tests/OpenClaw.Tray.UITests/ChatBubbleSelectionScopeProofTests.csmounts the realOpenClawChatTimelinethrough the shipping FunctionalUI reconciler on a live WinRT UI thread and renders one assistant message (two prose paragraphs, a three-item bullet list, a trailing paragraph, a fenced code block, and a table). It asserts: prose + list + trailing paragraph land in ONERichTextBlock(one continuous selection scope); the code and table text are absent from thatRichTextBlock; and each renders in its own separateTextBlockisland. Run:dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -p:Platform=x64 --filter FullyQualifiedName~ChatBubbleSelectionScopeProof-> Passed 1/1. This is the automated stand-in for the interactive native drag recording, which is a human-captured artifact.OpenClawGatewayWSL distro; focused renderer tests headless; deterministic proof on a live WinRT UI-thread fixture.d9e2b995dotnet test --filter FullyQualifiedName~MarkdownRenderer -p:Platform=x64; (2)dotnet test --filter FullyQualifiedName~ChatBubbleSelectionScopeProof -p:Platform=x64; (3) relaunched the isolated app (run-app-local.ps1 -NoBuild -Isolated -AllowNonMain) to drag-select a message containing prose + a bullet list + a fenced code block.MarkdownRendererCoalesceTests/MarkdownRendererListTestsassert prose + simple list -> oneRichTextBlockElement, lone simple list ->RichTextBlockElement, and a list containing a block quote -> separateStackElementisland (12/12).ChatBubbleSelectionScopeProofTestsproves the same contract end-to-end through the shipping renderer on a real UI thread including code/table islands (1/1).RichTextBlock; code blocks and tables stay their own styled islands; Copy button grabs the whole message.autoreviewcould not run becausecodex/claude/piCLIs are not installed in this environment; a rubber-duck review did run and both findings (heading-in-list formatting, true hanging indent) were fixed.Security Impact
Hyperlink/BitmapImageintroduced,AppendInlinesunchanged, rendering stays non-interactive.Compatibility and Migration
Review Conversations