Skip to content

Commit f89a88a

Browse files
calebedenCopilotshanselman
authored
Fix chat timeline render identity reuse (#917)
* Fix chat timeline render identity reuse Qualify live chat row keys by thread, generation, kind, and entry id so reset or synthetic-row transitions cannot reuse stale visual subtrees. Clear stale FunctionalUI modifier dependency properties on reused controls to avoid leaking old bubble styling/layout into new rows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prune stale FunctionalUI render cache paths Prevent keyed chat timeline generations from leaving old renderer controls, components, and content flyouts cached after they disappear from the rendered tree. This keeps the #917 row-identity fix from trading stale visual reuse for unbounded detached UI cache growth. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Scott Hanselman <scott@hanselman.com>
1 parent bbd18d4 commit f89a88a

7 files changed

Lines changed: 230 additions & 12 deletions

File tree

src/OpenClaw.Chat/ChatModels.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ public record ChatDataSnapshot(
194194
ChatComposeTarget ComposeTarget,
195195
IReadOnlyList<ChatModelChoice>? ModelChoices = null,
196196
IReadOnlyList<OpenClaw.Shared.GatewayCommand>? AvailableCommands = null,
197-
bool CommandsSupported = true);
197+
bool CommandsSupported = true,
198+
IReadOnlyDictionary<string, long>? TimelineGenerations = null);
198199

199200
/// <summary>
200201
/// Describes where the UI may send the next chat message. Distinct from

src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4136,6 +4136,7 @@ private ChatDataSnapshot BuildSnapshotLocked()
41364136

41374137
// Snapshot a defensive copy of the timeline dict.
41384138
var timelinesCopy = new Dictionary<string, ChatTimelineState>(_timelines);
4139+
var timelineGenerationsCopy = new Dictionary<string, long>(_resetVersions);
41394140

41404141
var defaultThreadId = ResolveDefaultThreadIdLocked();
41414142

@@ -4171,7 +4172,8 @@ private ChatDataSnapshot BuildSnapshotLocked()
41714172
// distinguish "loading" from "loaded but empty". IsSupported=false
41724173
// surfaces the unsupported state.
41734174
AvailableCommands: _commandCatalog?.Commands,
4174-
CommandsSupported: _commandCatalog?.IsSupported ?? true);
4175+
CommandsSupported: _commandCatalog?.IsSupported ?? true,
4176+
TimelineGenerations: timelineGenerationsCopy);
41754177
}
41764178

41774179
private string? ResolveDefaultThreadIdLocked()

src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,13 @@ Element BuildLoadingElement()
286286
var timeline = effectiveThread is not null && snapshot.Timelines.TryGetValue(effectiveThread.Id, out var tl)
287287
? tl
288288
: ChatTimelineState.Initial();
289+
var timelineGeneration = 0L;
290+
if (effectiveThread is not null
291+
&& snapshot.TimelineGenerations is { } generations
292+
&& generations.TryGetValue(effectiveThread.Id, out var generation))
293+
{
294+
timelineGeneration = generation;
295+
}
289296

290297
var entries = (IReadOnlyList<ChatTimelineItem>)timeline.Entries;
291298
var connectedRaw = snapshot.ConnectionStatus;
@@ -479,6 +486,7 @@ Element BuildLoadingElement()
479486
HasMoreHistory: false,
480487
OnLoadMoreHistory: null,
481488
EntryMetadata: entryMeta,
489+
TimelineGeneration: timelineGeneration,
482490
UserSenderLabel: "OpenClaw Windows Tray",
483491
AssistantSenderLabel: assistantSenderLabel,
484492
DefaultModel: effectiveThread.Model,

src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public record OpenClawChatTimelineProps(
4444
bool HasMoreHistory,
4545
Action? OnLoadMoreHistory,
4646
IReadOnlyDictionary<string, ChatEntryMetadata>? EntryMetadata = null,
47+
long TimelineGeneration = 0,
4748
string UserSenderLabel = "OpenClaw Windows Tray",
4849
string AssistantSenderLabel = "Field",
4950
string? DefaultModel = null,
@@ -788,6 +789,12 @@ static string FormatTime(DateTimeOffset? ts) =>
788789
ChatEntryMetadata? MetaFor(string id) =>
789790
meta is not null && meta.TryGetValue(id, out var m) ? m : null;
790791

792+
string RowKey(ChatTimelineItem entry) =>
793+
$"thread:{Props.SessionId ?? "none"}|generation:{Props.TimelineGeneration}|kind:{entry.Kind}|id:{entry.Id}";
794+
795+
string SyntheticRowKey(string id, ChatTimelineItemKind kind) =>
796+
$"thread:{Props.SessionId ?? "none"}|generation:{Props.TimelineGeneration}|kind:{kind}|synthetic:{id}";
797+
791798
// Hover-revealed action icon (copy / read aloud / trash). Opacity 0
792799
// and not hit-testable until the entry is hovered, then fades in
793800
// and becomes clickable. Soft pill radius + Light weight glyph so
@@ -2307,19 +2314,19 @@ bool BurstIsNestable(System.Collections.Generic.List<ChatTimelineItem> b)
23072314
{
23082315
if (!showToolCalls)
23092316
{
2310-
renderedEntries[k] = Empty().WithKey(entry.Id);
2317+
renderedEntries[k] = Empty().WithKey(RowKey(entry));
23112318
continue;
23122319
}
23132320
if (!startsBurst)
23142321
{
2315-
renderedEntries[k] = Empty().WithKey(entry.Id);
2322+
renderedEntries[k] = Empty().WithKey(RowKey(entry));
23162323
continue;
23172324
}
23182325
if (nestedConsumed.Contains(k))
23192326
{
23202327
// The assistant bubble above already rendered this burst
23212328
// inline as a child element — emit nothing here.
2322-
renderedEntries[k] = Empty().WithKey(entry.Id);
2329+
renderedEntries[k] = Empty().WithKey(RowKey(entry));
23232330
continue;
23242331
}
23252332
var burst = new System.Collections.Generic.List<ChatTimelineItem> { entry };
@@ -2329,7 +2336,7 @@ bool BurstIsNestable(System.Collections.Generic.List<ChatTimelineItem> b)
23292336
burst.Add(Props.Entries[orderedIdx[kj]]);
23302337
kj++;
23312338
}
2332-
renderedEntries[k] = RenderToolBurst(burst, showAvatar, currentBubbleSlot).WithKey(entry.Id);
2339+
renderedEntries[k] = RenderToolBurst(burst, showAvatar, currentBubbleSlot).WithKey(RowKey(entry));
23332340
continue;
23342341
}
23352342

@@ -2365,11 +2372,11 @@ bool BurstIsNestable(System.Collections.Generic.List<ChatTimelineItem> b)
23652372
}
23662373
}
23672374

2368-
renderedEntries[k] = RenderAssistantEntry(entry, startsBurst, endsBurst, showAvatar, currentBubbleSlot, nestedTool).WithKey(entry.Id);
2375+
renderedEntries[k] = RenderAssistantEntry(entry, startsBurst, endsBurst, showAvatar, currentBubbleSlot, nestedTool).WithKey(RowKey(entry));
23692376
continue;
23702377
}
23712378

2372-
renderedEntries[k] = RenderEntry(entry, startsBurst, endsBurst, showAvatar).WithKey(entry.Id);
2379+
renderedEntries[k] = RenderEntry(entry, startsBurst, endsBurst, showAvatar).WithKey(RowKey(entry));
23732380
}
23742381

23752382
var thinkingNestedConsumed = new System.Collections.Generic.HashSet<int>();
@@ -2436,7 +2443,8 @@ bool BurstIsNestable(System.Collections.Generic.List<ChatTimelineItem> b)
24362443
nestedTool: thinkingNestedTool,
24372444
suppressFooter: true,
24382445
forceVisible: true)
2439-
.LiveRegion(Microsoft.UI.Xaml.Automation.Peers.AutomationLiveSetting.Polite);
2446+
.LiveRegion(Microsoft.UI.Xaml.Automation.Peers.AutomationLiveSetting.Polite)
2447+
.WithKey(SyntheticRowKey("__thinking__", ChatTimelineItemKind.Assistant));
24402448
}
24412449

24422450
// Build the final element list, splicing the thinking indicator

src/OpenClawTray.FunctionalUI/FunctionalUI.cs

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -913,10 +913,19 @@ internal sealed class UiRenderer(Action requestRender)
913913
private readonly Dictionary<string, Component> _components = new();
914914
private readonly Dictionary<string, Flyout> _contentFlyouts = new();
915915
private readonly HashSet<string> _mountedPaths = new();
916+
private readonly HashSet<string> _visitedControlPaths = new();
917+
private readonly HashSet<string> _visitedComponentKeys = new();
918+
private readonly HashSet<string> _visitedContentFlyoutPaths = new();
916919

917920
public UIElement Render(Element element, string path, List<Action> effects)
918921
{
919-
return RenderElement(element, path, effects);
922+
_visitedControlPaths.Clear();
923+
_visitedComponentKeys.Clear();
924+
_visitedContentFlyoutPaths.Clear();
925+
926+
var rendered = RenderElement(element, path, effects);
927+
PruneUnvisitedPaths();
928+
return rendered;
920929
}
921930

922931
public void Dispose()
@@ -993,6 +1002,8 @@ private UIElement RenderNavigationHost(INavigationHostElement element, string pa
9931002

9941003
private T GetOrCreate<T>(string path) where T : UIElement, new()
9951004
{
1005+
_visitedControlPaths.Add(path);
1006+
9961007
if (_controls.TryGetValue(path, out var existing) && existing is T typed)
9971008
return typed;
9981009

@@ -1020,6 +1031,8 @@ private UIElement RenderComponent(ComponentElement element, string path, List<Ac
10201031
{
10211032
var componentKey = GetComponentKey(element.ComponentType);
10221033
var key = path + ":" + componentKey;
1034+
_visitedComponentKeys.Add(key);
1035+
10231036
if (!_components.TryGetValue(key, out var component))
10241037
{
10251038
component = (Component)Activator.CreateInstance(element.ComponentType)!;
@@ -1378,6 +1391,8 @@ private FlyoutBase CreateFlyout(FlyoutElement element, string path, List<Action>
13781391

13791392
private Flyout CreateContentFlyout(ContentFlyoutElement element, string path, List<Action> effects)
13801393
{
1394+
_visitedContentFlyoutPaths.Add(path);
1395+
13811396
// Cache the Flyout instance per path so its identity is STABLE across
13821397
// re-renders. ConfigureButton reassigns control.Flyout on every render,
13831398
// and a full-root re-render fires on every state change (including the
@@ -1403,6 +1418,39 @@ private Flyout CreateContentFlyout(ContentFlyoutElement element, string path, Li
14031418
return flyout;
14041419
}
14051420

1421+
private void PruneUnvisitedPaths()
1422+
{
1423+
foreach (var (key, component) in _components.ToArray())
1424+
{
1425+
if (_visitedComponentKeys.Contains(key))
1426+
continue;
1427+
1428+
component.Context.RunEffectCleanups();
1429+
_components.Remove(key);
1430+
}
1431+
1432+
foreach (var (path, flyout) in _contentFlyouts.ToArray())
1433+
{
1434+
if (_visitedContentFlyoutPaths.Contains(path))
1435+
continue;
1436+
1437+
flyout.Hide();
1438+
flyout.Content = null;
1439+
_contentFlyouts.Remove(path);
1440+
}
1441+
1442+
foreach (var (path, control) in _controls.ToArray())
1443+
{
1444+
if (_visitedControlPaths.Contains(path))
1445+
continue;
1446+
1447+
_mountedPaths.Remove(path);
1448+
DetachChildren(control);
1449+
RemoveFromParent(control);
1450+
_controls.Remove(path);
1451+
}
1452+
}
1453+
14061454
private static MenuFlyout CreateMenuFlyout(MenuFlyoutContentElement element)
14071455
{
14081456
var flyout = new MenuFlyout { Placement = element.Placement };
@@ -1620,20 +1668,37 @@ private static void ApplyModifiers(FrameworkElement control, Element element)
16201668
var m = element.Modifiers;
16211669
control.Tag = element;
16221670
if (m.Margin is { } margin) control.Margin = margin;
1671+
else control.ClearValue(FrameworkElement.MarginProperty);
16231672
if (m.Width is { } width) control.Width = width;
1673+
else control.ClearValue(FrameworkElement.WidthProperty);
16241674
if (m.Height is { } height) control.Height = height;
1675+
else control.ClearValue(FrameworkElement.HeightProperty);
16251676
if (m.MinWidth is { } minWidth) control.MinWidth = minWidth;
1677+
else control.ClearValue(FrameworkElement.MinWidthProperty);
16261678
if (m.MaxWidth is { } maxWidth) control.MaxWidth = maxWidth;
1679+
else control.ClearValue(FrameworkElement.MaxWidthProperty);
16271680
if (m.MinHeight is { } minHeight) control.MinHeight = minHeight;
1681+
else control.ClearValue(FrameworkElement.MinHeightProperty);
16281682
if (m.MaxHeight is { } maxHeight) control.MaxHeight = maxHeight;
1683+
else control.ClearValue(FrameworkElement.MaxHeightProperty);
16291684
if (m.HorizontalAlignment is { } hAlign) control.HorizontalAlignment = hAlign;
1685+
else control.ClearValue(FrameworkElement.HorizontalAlignmentProperty);
16301686
if (m.VerticalAlignment is { } vAlign) control.VerticalAlignment = vAlign;
1687+
else control.ClearValue(FrameworkElement.VerticalAlignmentProperty);
16311688
if (m.Opacity is { } opacity) control.Opacity = opacity;
1689+
else control.ClearValue(UIElement.OpacityProperty);
16321690
if (m.AutomationName is { } automationName) AutomationProperties.SetName(control, automationName);
1691+
else control.ClearValue(AutomationProperties.NameProperty);
16331692
if (m.LiveRegion is { } liveRegion) AutomationProperties.SetLiveSetting(control, liveRegion);
1693+
else control.ClearValue(AutomationProperties.LiveSettingProperty);
16341694
ApplyResourceOverrides(control, m.ResourceOverrides);
1635-
if (m.Disabled is { } disabled && control is Control disabledControl)
1636-
disabledControl.IsEnabled = !disabled;
1695+
if (control is Control disabledControl)
1696+
{
1697+
if (m.Disabled is { } disabled)
1698+
disabledControl.IsEnabled = !disabled;
1699+
else
1700+
disabledControl.ClearValue(Control.IsEnabledProperty);
1701+
}
16371702

16381703
control.KeyDown -= ElementKeyDown;
16391704
if (m.KeyDown is not null) control.KeyDown += ElementKeyDown;
@@ -1646,37 +1711,59 @@ private static void ApplyModifiers(FrameworkElement control, Element element)
16461711
{
16471712
case TextBlock tb:
16481713
if (m.FontSize is { } textSize) tb.FontSize = textSize;
1714+
else tb.ClearValue(TextBlock.FontSizeProperty);
16491715
if (m.FontWeight is { } textWeight) tb.FontWeight = textWeight;
1716+
else tb.ClearValue(TextBlock.FontWeightProperty);
16501717
if (m.FontFamily is { } textFamily) tb.FontFamily = textFamily;
1718+
else tb.ClearValue(TextBlock.FontFamilyProperty);
16511719
if (m.TextWrapping is { } wrapping) tb.TextWrapping = wrapping;
1720+
else tb.ClearValue(TextBlock.TextWrappingProperty);
16521721
if (m.Padding is { } textPadding) tb.Padding = textPadding;
1722+
else tb.ClearValue(TextBlock.PaddingProperty);
16531723
if (m.ForegroundResourceKey is { } textFgResource) tb.Foreground = ThemeResources.ResolveBrush(textFgResource);
16541724
else if (m.Foreground is { } textFg) tb.Foreground = textFg;
1725+
else tb.ClearValue(TextBlock.ForegroundProperty);
1726+
tb.ClearValue(TextBlock.TextTrimmingProperty);
1727+
tb.ClearValue(TextBlock.MaxLinesProperty);
1728+
tb.ClearValue(TextBlock.LineHeightProperty);
1729+
tb.ClearValue(TextBlock.CharacterSpacingProperty);
16551730
break;
16561731
case Control c:
16571732
if (m.Padding is { } controlPadding) c.Padding = controlPadding;
1733+
else c.ClearValue(Control.PaddingProperty);
16581734
if (m.FontSize is { } controlSize) c.FontSize = controlSize;
1735+
else c.ClearValue(Control.FontSizeProperty);
16591736
if (m.FontWeight is { } controlWeight) c.FontWeight = controlWeight;
1737+
else c.ClearValue(Control.FontWeightProperty);
16601738
if (m.FontFamily is { } controlFamily) c.FontFamily = controlFamily;
1739+
else c.ClearValue(Control.FontFamilyProperty);
16611740
if (m.ForegroundResourceKey is { } controlFgResource) c.Foreground = ThemeResources.ResolveBrush(controlFgResource);
16621741
else if (m.Foreground is { } controlFg) c.Foreground = controlFg;
1742+
else c.ClearValue(Control.ForegroundProperty);
16631743
if (m.BorderBrushResourceKey is { } controlBorderResource) c.BorderBrush = ThemeResources.ResolveBrush(controlBorderResource);
16641744
else if (m.BorderBrush is { } controlBorder) c.BorderBrush = controlBorder;
1745+
else c.ClearValue(Control.BorderBrushProperty);
16651746
if (m.BorderThickness is { } controlThickness) c.BorderThickness = controlThickness;
1747+
else c.ClearValue(Control.BorderThicknessProperty);
16661748
break;
16671749
case Border b:
16681750
if (m.Padding is { } borderPadding) b.Padding = borderPadding;
1751+
else b.ClearValue(Border.PaddingProperty);
16691752
if (m.BackgroundResourceKey is { } backgroundResourceKey)
16701753
b.Background = ThemeResources.ResolveBrush(backgroundResourceKey);
16711754
else if (m.Background is { } bg)
16721755
b.Background = bg;
1756+
else b.ClearValue(Border.BackgroundProperty);
16731757
if (m.BorderBrushResourceKey is { } borderResourceKey)
16741758
b.BorderBrush = ThemeResources.ResolveBrush(borderResourceKey);
16751759
else if (m.BorderBrush is { } borderBrush)
16761760
b.BorderBrush = borderBrush;
1761+
else b.ClearValue(Border.BorderBrushProperty);
16771762
if (m.BorderThickness is { } borderThickness)
16781763
b.BorderThickness = borderThickness;
1764+
else b.ClearValue(Border.BorderThicknessProperty);
16791765
if (m.CornerRadius is { } radius) b.CornerRadius = radius;
1766+
else b.ClearValue(Border.CornerRadiusProperty);
16801767
break;
16811768
}
16821769
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
using System.Text.RegularExpressions;
2+
3+
namespace OpenClaw.Tray.Tests;
4+
5+
public sealed class ChatTimelineRenderIdentityContractTests
6+
{
7+
[Fact]
8+
public void TimelineRows_UseGenerationQualifiedKindedKeys()
9+
{
10+
var timeline = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatTimeline.cs");
11+
12+
Assert.Contains("long TimelineGeneration = 0", timeline);
13+
Assert.Contains("string RowKey(ChatTimelineItem entry)", timeline);
14+
Assert.Contains("Props.TimelineGeneration", timeline);
15+
Assert.Contains("entry.Kind", timeline);
16+
Assert.Contains("entry.Id", timeline);
17+
Assert.DoesNotContain(".WithKey(entry.Id)", timeline);
18+
Assert.Matches(
19+
new Regex(@"WithKey\(RowKey\(entry\)\)"),
20+
timeline);
21+
}
22+
23+
[Fact]
24+
public void ThinkingIndicator_UsesSyntheticGenerationQualifiedKey()
25+
{
26+
var timeline = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatTimeline.cs");
27+
28+
Assert.Contains("string SyntheticRowKey(string id, ChatTimelineItemKind kind)", timeline);
29+
Assert.Contains("SyntheticRowKey(\"__thinking__\", ChatTimelineItemKind.Assistant)", timeline);
30+
}
31+
32+
[Fact]
33+
public void TimelineGeneration_FlowsFromProviderSnapshotToTimelineProps()
34+
{
35+
var models = Read("src", "OpenClaw.Chat", "ChatModels.cs");
36+
var provider = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatDataProvider.cs");
37+
var root = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatRoot.cs");
38+
39+
Assert.Contains("IReadOnlyDictionary<string, long>? TimelineGenerations = null", models);
40+
Assert.Contains("new Dictionary<string, long>(_resetVersions)", provider);
41+
Assert.Contains("TimelineGenerations: timelineGenerationsCopy", provider);
42+
Assert.Contains("snapshot.TimelineGenerations", root);
43+
Assert.Contains("TimelineGeneration: timelineGeneration", root);
44+
}
45+
46+
[Fact]
47+
public void ResetClearPath_BumpsTimelineGenerationBeforeReusingEntryIds()
48+
{
49+
var provider = Read("src", "OpenClaw.Tray.WinUI", "Chat", "OpenClawChatDataProvider.cs");
50+
51+
Assert.Matches(
52+
new Regex(@"private\s+ResetClearPersistence\s+ClearThreadHistoryAfterResetLocked\(string\s+threadId\)[\s\S]*_resetVersions\[threadId\]\s*=\s*GetResetVersionLocked\(threadId\)\s*\+\s*1;[\s\S]*_timelines\[threadId\]\s*=\s*ChatTimelineState\.Initial\(\)\s*with\s*\{\s*HistoryLoaded\s*=\s*true\s*\};"),
53+
provider);
54+
}
55+
56+
private static string Read(params string[] parts)
57+
=> File.ReadAllText(Path.Combine(new[] { TestRepositoryPaths.GetRepositoryRoot() }.Concat(parts).ToArray()));
58+
}

0 commit comments

Comments
 (0)