diff --git a/src/OpenClaw.Shared/Markdown/ChatMarkdownAst.cs b/src/OpenClaw.Shared/Markdown/ChatMarkdownAst.cs index b2d94c6d2..138661d4f 100644 --- a/src/OpenClaw.Shared/Markdown/ChatMarkdownAst.cs +++ b/src/OpenClaw.Shared/Markdown/ChatMarkdownAst.cs @@ -68,6 +68,24 @@ public sealed record MdTableRow(IReadOnlyList Cells); public sealed record MdTableCell(IReadOnlyList Inlines); +/// +/// Base type for all inline AST nodes. +/// +/// IMPORTANT — cache invariant: ChatMarkdownRenderer uses +/// IReadOnlyList<MdInline>.SequenceEqual (record value-equality) +/// to short-circuit rebuilding TextBlock.Inlines on re-render, which +/// is what keeps a user's text selection alive across pointer-enter/leave +/// and streaming token updates. That short-circuit is sound ONLY while every +/// concrete MdInline subtype contains exclusively value-comparable +/// members (primitives, string, enums). If a future subtype adds a +/// reference-typed member (e.g. IReadOnlyList<MdInline> Children +/// for links), the auto-generated record Equals will compare those +/// members BY REFERENCE — silently breaking cache correctness and wiping +/// selection again on every re-render. Update the renderer's equality +/// strategy (and MdInlineEqualityTests) before introducing such a +/// member. +/// +/// public abstract record MdInline; /// diff --git a/src/OpenClaw.Tray.WinUI/Chat/Markdown/ChatMarkdownRenderer.cs b/src/OpenClaw.Tray.WinUI/Chat/Markdown/ChatMarkdownRenderer.cs index 6ab6e3e0f..5daa2e4b8 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/Markdown/ChatMarkdownRenderer.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/Markdown/ChatMarkdownRenderer.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Documents; @@ -479,15 +481,43 @@ private static int MaxCells(IReadOnlyList a, IReadOnlyList> + s_inlinesCache = new(); + private static TextBlockElement InlinesTextBlock(IReadOnlyList inlines) => TextBlock(string.Empty).Set(tb => { tb.TextWrapping = TextWrapping.Wrap; tb.IsTextSelectionEnabled = true; - tb.Inlines.Clear(); - AppendInlines(tb.Inlines, inlines); + ApplyInlines(tb, inlines); }); + private static void ApplyInlines(TextBlock tb, IReadOnlyList inlines) + { + // Only honour the cache when the previously-built inlines are still + // present. ConfigureTextBlock may set Text="" before this setter + // runs, which clears the Inlines collection out from under us; in + // that case we must rebuild even if the source list is unchanged. + if (tb.Inlines.Count > 0 + && s_inlinesCache.TryGetValue(tb, out var cached) + && (ReferenceEquals(cached, inlines) || cached.SequenceEqual(inlines))) + { + return; + } + s_inlinesCache.AddOrUpdate(tb, inlines); + tb.Inlines.Clear(); + AppendInlines(tb.Inlines, inlines); + } + private static void AppendInlines(InlineCollection sink, IReadOnlyList inlines) { foreach (var inline in inlines) diff --git a/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs b/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs index f64818db2..7c6615d35 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs @@ -131,6 +131,42 @@ private static Element SafeMarkdownText(string? text) private static readonly System.Runtime.CompilerServices.ConditionalWeakTable s_plainCache = new(); + // FontFamily instances are immutable, but `new FontFamily(...)` per + // render allocates a fresh CLR object whose reference does not equal + // the previous one. Reassigning a referentially-different FontFamily + // to a TextBlock invalidates its inline runs even when the source + // string is identical, which (in the tool-output panel) makes + // multi-line wrapped text vanish during a pointer-exit re-render. + // Caching sidesteps both the GC pressure and the invalidation. + // + // FontFamily is a DependencyObject with thread affinity, so a single + // process-wide singleton would crash with RPC_E_WRONG_THREAD if a + // second window on a different dispatcher ever tried to read it. + // Keying by DispatcherQueue mirrors the brush cache above: one + // shared instance per window, collected with its dispatcher. + // Off-dispatcher callers (tests, design-time) get a one-shot + // uncached instance — correct, just not reused. + private const string MonoFontFamilySource = "Cascadia Code, Cascadia Mono, Consolas"; + private static readonly System.Runtime.CompilerServices.ConditionalWeakTable< + Microsoft.UI.Dispatching.DispatcherQueue, FontFamily> s_monoFontByDispatcher = new(); + private static FontFamily s_monoFontFamily + { + get + { + var dispatcher = Microsoft.UI.Dispatching.DispatcherQueue.GetForCurrentThread(); + if (dispatcher is null) + { + return new FontFamily(MonoFontFamilySource); + } + if (!s_monoFontByDispatcher.TryGetValue(dispatcher, out var family)) + { + family = new FontFamily(MonoFontFamilySource); + s_monoFontByDispatcher.Add(dispatcher, family); + } + return family; + } + } + // Per-DispatcherQueue selection-highlight brushes for the user // bubble. The bubble background is the user's chosen system accent // (which may be red, green, purple, …), so a hardcoded color would @@ -1433,7 +1469,7 @@ Element BuildRow(ChatTimelineItem entry, bool isFirst, bool isLast, string? step Caption(labelText).Foreground(SecondaryText) .Set(t => { - t.FontFamily = new FontFamily("Cascadia Code, Cascadia Mono, Consolas"); + t.FontFamily = s_monoFontFamily; t.FontWeight = Microsoft.UI.Text.FontWeights.SemiBold; }) .VAlign(VerticalAlignment.Center) @@ -1472,6 +1508,7 @@ Element BuildRow(ChatTimelineItem entry, bool isFirst, bool isLast, string? step ).Set(b => b.MinHeight = 32); Element body = Empty(); + bool hasExpandedBody = false; if (isExpanded) { var sections = new System.Collections.Generic.List(); @@ -1507,14 +1544,31 @@ Element PhantomChevron() => Caption("▸") var codeBlock = Border( ScrollView( - TextBlock(displayText) + // Use Text="" + Inlines populated by + // ApplyPlainSelectableInlines so that the + // ConditionalWeakTable cache short-circuits + // re-population on hover-out re-renders. + // Setting Text directly (even value-guarded) + // leaves the rendered run unanchored, which + // lets WinUI drop the text glyphs during a + // pointer-exit re-render while keeping any + // active selection rectangles around. This + // mirrors the working markdown-bubble path. + TextBlock("") .Set(t => { - t.FontFamily = new FontFamily("Cascadia Code, Cascadia Mono, Consolas"); + // Hoisted static FontFamily is the + // critical bit — reassigning the same + // reference is a DP-equality no-op, + // so this setter is safe to re-run on + // every render without invalidating + // the selection. + t.FontFamily = s_monoFontFamily; t.FontSize = 11; t.TextWrapping = TextWrapping.Wrap; t.IsTextSelectionEnabled = true; t.LineHeight = 16; + ApplyPlainSelectableInlines(t, displayText); }) .Foreground(SecondaryText) .Padding(11, 8, 11, 10) @@ -1558,7 +1612,25 @@ Element PhantomChevron() => Caption("▸") sections.Add(BuildSection(outLabel, entry.ToolOutput!)); } - body = Border(VStack(0, sections.ToArray())).Background(blockHeaderBg); + hasExpandedBody = sections.Count > 0; + if (hasExpandedBody) + { + var bodyBorder = Border(VStack(0, sections.ToArray())).Background(blockHeaderBg); + // When this row is the last in the card AND expanded + // with actual content, the body sits at the bottom + // edge — it must own the bottom rounded corners so + // the row blends into the card instead of showing a + // square edge clipped by the card's rounding. + if (isLast) + { + bodyBorder = bodyBorder.Set(b => b.CornerRadius = new CornerRadius(0, 0, 8, 8)); + } + body = bodyBorder; + } + // sections.Count == 0 leaves `body` as the default + // Empty() above so no phantom bordered strip stacks under + // the header. The header keeps its own bottom rounding + // via hasExpandedBody=false. } Action toggle = () => @@ -1569,32 +1641,26 @@ Element PhantomChevron() => Caption("▸") expandedToolChips.Set(next); }; - // 1px separator between rows (skipped on the first row). The - // separator lives inside the row so collapsed/expanded heights - // both keep continuity with the next row above. - var rowContent = VStack(0, headerRow, body); - var rowWithSeparator = isFirst - ? (Element)rowContent - : Border(rowContent) - .Set(b => - { - b.BorderThickness = new Thickness(0, 1, 0, 0); - b.BorderBrush = toolCardBorderBrush; - }); - - return Button(rowWithSeparator, toggle) + // Scope the toggle Button to the header only — the body + // contains selectable TextBlocks (TOOL OUTPUT / CALL), and + // wrapping body in the Button caused unhandled PointerReleased + // events inside the body's padding/whitespace to bubble up + // and fire Click → collapse the section while the user was + // trying to select text. + var headerButton = Button(headerRow, toggle) .Set(b => { b.HorizontalAlignment = HorizontalAlignment.Stretch; b.HorizontalContentAlignment = HorizontalAlignment.Stretch; b.Padding = new Thickness(0); - // Round only the outer corners so rows blend into the - // wrapping card without leaving gaps. + // Top corners follow isFirst. Bottom corners follow + // isLast unless the expanded body below owns them + // (i.e., expanded AND has at least one section). b.CornerRadius = new CornerRadius( isFirst ? 8 : 0, isFirst ? 8 : 0, - isLast ? 8 : 0, - isLast ? 8 : 0); + (isLast && !hasExpandedBody) ? 8 : 0, + (isLast && !hasExpandedBody) ? 8 : 0); }) .Resources(r => r .Set("ButtonBackground", new SolidColorBrush(Colors.Transparent)) @@ -1609,6 +1675,21 @@ Element PhantomChevron() => Caption("▸") .Set("ButtonBorderBrush", new SolidColorBrush(Colors.Transparent)) .Set("ButtonBorderBrushPointerOver", new SolidColorBrush(Colors.Transparent)) .Set("ButtonBorderBrushPressed", new SolidColorBrush(Colors.Transparent))); + + // 1px separator between rows (skipped on the first row). The + // separator lives inside the row so collapsed/expanded heights + // both keep continuity with the next row above. + var rowStack = VStack(0, headerButton, body); + var rowWithSeparator = isFirst + ? (Element)rowStack + : Border(rowStack) + .Set(b => + { + b.BorderThickness = new Thickness(0, 1, 0, 0); + b.BorderBrush = toolCardBorderBrush; + }); + + return rowWithSeparator; } // ── Style-aware composition ────────────────────────────── @@ -2307,7 +2388,7 @@ Element RenderPermissionEntry(ChatTimelineItem entry) { t.FontSize = 12; t.TextWrapping = TextWrapping.Wrap; - t.FontFamily = new FontFamily("Cascadia Code, Cascadia Mono, Consolas"); + t.FontFamily = s_monoFontFamily; }) .Foreground(TertiaryText) .Padding(0, 4, 0, 4), diff --git a/tests/OpenClaw.Shared.Tests/Markdown/MdInlineEqualityTests.cs b/tests/OpenClaw.Shared.Tests/Markdown/MdInlineEqualityTests.cs new file mode 100644 index 000000000..870f27205 --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/Markdown/MdInlineEqualityTests.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using OpenClaw.Shared.Markdown; +using Xunit; + +namespace OpenClaw.Shared.Tests.Markdown; + +/// +/// Guard tests for the equality contract that +/// ChatMarkdownRenderer's inline cache depends on. +/// +/// +/// The renderer short-circuits rebuilding TextBlock.Inlines when the +/// new inline list is value-equal to the previously rendered one (via +/// record-generated Equals + SequenceEqual). That short-circuit +/// is what preserves the user's text selection across pointer events and +/// streaming token updates. It is sound only while every concrete +/// subtype contains exclusively value-comparable +/// members (primitives, string, enums) — record-generated equality +/// compares reference-typed members by reference, which would silently +/// break cache correctness. +/// +/// +/// +/// If a new MdInline subtype is introduced (or an existing one gains +/// a member) that violates this invariant, these tests fail and the +/// renderer's equality strategy must be updated before the change ships. +/// +/// +public class MdInlineEqualityTests +{ + private static readonly HashSet AllowedMemberTypes = new() + { + typeof(string), + typeof(bool), + typeof(int), + typeof(long), + typeof(double), + typeof(MdColumnAlignment), + }; + + private static IEnumerable ConcreteMdInlineTypes() + => typeof(MdInline).Assembly + .GetTypes() + .Where(t => !t.IsAbstract && typeof(MdInline).IsAssignableFrom(t)); + + [Fact] + public void KnownConcreteSubtypes_AreExactlyTheExpectedSet() + { + var actual = ConcreteMdInlineTypes() + .Select(t => t.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToArray(); + + var expected = new[] { nameof(MdInlineLineBreak), nameof(MdInlineText) }; + + Assert.Equal(expected, actual); + } + + [Fact] + public void EveryMdInlineSubtype_OnlyHasValueComparableMembers() + { + foreach (var type in ConcreteMdInlineTypes()) + { + var properties = type + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.DeclaringType != typeof(MdInline) + && p.Name != "EqualityContract"); + + foreach (var prop in properties) + { + var pt = prop.PropertyType; + var underlying = Nullable.GetUnderlyingType(pt) ?? pt; + + var ok = AllowedMemberTypes.Contains(underlying) + || underlying.IsEnum + || underlying.IsPrimitive; + + Assert.True( + ok, + $"{type.Name}.{prop.Name} has type {pt.FullName}, which is not " + + "value-comparable for record equality. ChatMarkdownRenderer's " + + "inline cache relies on deep value equality of MdInline lists — " + + "adding a reference-typed member silently breaks selection " + + "preservation. Update the renderer's equality strategy before " + + "extending MdInline this way."); + } + } + } + + [Fact] + public void SequenceEqual_OnEquivalentFreshLists_ReturnsTrue() + { + IReadOnlyList a = new MdInline[] + { + new MdInlineText("hello", IsStrong: true), + new MdInlineLineBreak(IsHard: false), + new MdInlineText(" world", IsEmphasis: true), + }; + + IReadOnlyList b = new MdInline[] + { + new MdInlineText("hello", IsStrong: true), + new MdInlineLineBreak(IsHard: false), + new MdInlineText(" world", IsEmphasis: true), + }; + + Assert.False(ReferenceEquals(a, b)); + Assert.True(a.SequenceEqual(b)); + } + + [Fact] + public void SequenceEqual_OnDifferingContent_ReturnsFalse() + { + IReadOnlyList a = new MdInline[] + { + new MdInlineText("hello"), + }; + + IReadOnlyList b = new MdInline[] + { + new MdInlineText("hello!"), + }; + + Assert.False(a.SequenceEqual(b)); + } +}