Skip to content

Fix chat timeline render identity reuse#917

Merged
shanselman merged 2 commits into
openclaw:mainfrom
calebeden:agent/fix-899-chat-render
Jul 2, 2026
Merged

Fix chat timeline render identity reuse#917
shanselman merged 2 commits into
openclaw:mainfrom
calebeden:agent/fix-899-chat-render

Conversation

@calebeden

@calebeden calebeden commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Qualifies live chat timeline row keys by thread, reset generation, row kind, and entry id so reset/reorder/synthetic-row transitions cannot reuse stale visual subtrees.
  • Adds explicit non-colliding keys for the synthetic assistant thinking row.
  • Clears absent FunctionalUI modifier dependency-property values with ClearValue(...) so reused Border, TextBlock, and Control instances do not retain stale bubble layout/styling.
  • Adds Tray contract coverage for render identity, reset generation, thinking keys, and modifier cleanup.

Fixes #899.

Validation

  • ./build.ps1 - passed
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore - passed: 2686 passed, 31 skipped
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore - passed: 1455 passed
  • python ./.agents/skills/autoreview/scripts/autoreview --mode local --engine copilot - clean: no accepted/actionable findings

Codex autoreview could not run because the codex executable was not installed in this environment; the same project autoreview helper was run with the Copilot engine instead.

Real behavior proof

  • Launched the current-head packaged worktree app with:
    • winapp run ".\\src\\OpenClaw.Tray.WinUI\\bin\\Debug\\net10.0-windows10.0.22621.0\\win-arm64" --manifest ".\\src\\OpenClaw.Tray.WinUI\\Package.appxmanifest" --executable "OpenClaw.Tray.WinUI.exe" --debug-output
  • Opened chat with openclaw://chat.
  • Sent proof prompt: OpenClaw #899 render proof - please acknowledge briefly.
  • UIA proof found the full live user message text after send:
    • lbl-openclaw899rend-26bd Text "OpenClaw #899 render proof - please acknowledge briefly."
  • Screenshot captured locally at current PR head: C:\\Users\\t-edencaleb\\.copilot\\session-state\\059f92fb-1d9f-4e9c-ac3a-a6f28cb355bd\\files\\issue899-fix-chat-after-send.png.

The original #899 symptom has not been reproduced deterministically. This PR is a best-effort fix for the observed live-render failure mode: correct message data/copy/reload behavior but stale/malformed WinUI visuals. The patch targets two plausible renderer causes found in the current code: weak live row identity during reset/synthetic-row transitions and stale dependency-property values on reused FunctionalUI controls.

Here is current-head proof of the reset path: after running /new, the next sent message renders as a full user bubble and the assistant transition renders without the stale gray/empty bubble artifact. This is not proof that the flaky #899 symptom is eliminated, because we have not found a deterministic repro. It is current-head smoke proof for the highest-risk path identified in the report: /new followed by a sent user message and assistant transition. The screenshot shows that this path still renders the full user bubble and does not show the stale gray/empty bubble artifact.

image

Rubber-duck review

  • Rubber-duck review with Claude Sonnet 5 found no blocking issues.
  • Non-blocking feedback noted that source-text contracts are less strong than a behavioral render harness and that extra /new manual proof would be useful if a reliable repro path is available.

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>
@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 8:40 PM ET / 00:40 UTC.

Summary
This PR qualifies chat timeline render keys by thread/reset generation/kind/id, clears stale FunctionalUI modifier state on reused controls, prunes stale renderer caches, and adds Tray contract tests.

Reproducibility: no. deterministic current-main runtime reproduction was established. The linked issue and PR proof show the visible symptom and reset path, and source inspection shows plausible stale row-key and reused-control paths.

Review metrics: 3 noteworthy metrics.

  • Diff surface: 7 files changed, +230/-12. The fix spans chat data models, WinUI rendering, shared FunctionalUI renderer behavior, and Tray tests.
  • Shared renderer touched: 1 FunctionalUI renderer changed. Changes to reused-control cleanup can affect tray UI outside the chat timeline.
  • Contract coverage: 2 test files added. The PR adds regression contracts for row identity, reset generation flow, modifier cleanup, and cache pruning.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #899
Summary: This PR is the candidate fix for the linked post-/new native chat user-bubble misrendering report; nearby chat-rendering reports are useful context but do not supersede it.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The shared FunctionalUI ClearValue and cache-pruning changes affect reused controls beyond this one chat row path, so existing tray UI that accidentally depended on retained local values could render differently after merge.
  • [P1] The original bug remains flaky and was not deterministically reproduced, so the screenshot is strong reset-path smoke proof rather than proof that every timing path behind the report is eliminated.
  • [P1] At review time, several Build and Test jobs for head 7097207 were still in progress, so maintainers should wait for exact-head CI before merging.

Maintainer options:

  1. Land after exact-head CI (recommended)
    If maintainers accept the shared renderer cleanup semantics, merge after the current head passes the required checks.
  2. Request broader UI smoke proof
    Maintainers can ask for one more visible smoke across representative non-chat FunctionalUI controls if the shared ClearValue/cache-pruning behavior needs extra confidence.
  3. Narrow the shared renderer change
    If the compatibility surface is too broad, revise the branch to keep the chat row-key fix and only the renderer cleanup needed for the reported bubble path.

Next step before merge

  • No automated repair is needed; maintainers should wait for exact-head CI and then decide whether to accept the shared FunctionalUI compatibility risk.

Security
Cleared: The diff stays in WinUI rendering, shared FunctionalUI state cleanup, and tests; it does not change secrets, dependencies, CI, downloads, or privilege boundaries.

Review details

Best possible solution:

Land the focused renderer identity, modifier cleanup, and cache-pruning fix once exact-head CI is green and maintainers accept the shared FunctionalUI compatibility surface.

Do we have a high-confidence way to reproduce the issue?

No deterministic current-main runtime reproduction was established. The linked issue and PR proof show the visible symptom and reset path, and source inspection shows plausible stale row-key and reused-control paths.

Is this the best way to solve the issue?

Yes, this is a narrow maintainable repair for stale render identity and reused modifier state, with the maintainer follow-up addressing cache growth from generation-qualified keys. The main remaining decision is compatibility acceptance for the shared FunctionalUI cleanup.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bbd18d431766.

Label changes

Label justifications:

  • P2: This is a normal-priority user-visible native chat rendering fix with limited blast radius and no crash, security, or data-loss signal.
  • merge-risk: 🚨 compatibility: The PR changes shared FunctionalUI modifier reset and cache behavior for reused controls, which can alter existing tray UI rendering beyond the reported chat bubble path.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body includes copied UIA output and a screenshot that directly shows the /new reset path rendering a full user bubble and assistant result after the fix; private data should remain redacted in any future proof updates.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied UIA output and a screenshot that directly shows the /new reset path rendering a full user bubble and assistant result after the fix; private data should remain redacted in any future proof updates.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body includes copied UIA output and a screenshot that directly shows the /new reset path rendering a full user bubble and assistant result after the fix; private data should remain redacted in any future proof updates.
Evidence reviewed

What I checked:

Likely related people:

  • shanselman: Current-main blame for the affected timeline/reset/FunctionalUI paths routes through recent Scott Hanselman commits, and he authored the PR follow-up cache-pruning commit plus the prior merged user-bubble rendering fix. (role: recent area contributor and follow-up author; confidence: high; commits: 4166e0fd63f8, 87dc3dc79d7c, 70972073cbcf; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, src/OpenClawTray.FunctionalUI/FunctionalUI.cs)
  • Régis Brid: Local history shows Régis Brid introduced the native FunctionalUI chat experience and later worked on markdown rendering, chat text-vanish behavior, and inline exec approval in the same chat renderer family. (role: feature owner and adjacent chat contributor; confidence: medium; commits: 08cab69a038c, 0a164fc111b2, 16929b600710; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs)
  • kenehong: Local history shows earlier chat timeline layout work by kenehong in the same native chat renderer area, including continuation footer and tool-burst visual layout commits. (role: adjacent chat timeline contributor; confidence: medium; commits: 65ba516cc138, 53d6a3df4ce4; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatTimeline.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 1, 2026
@calebeden

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@calebeden calebeden marked this pull request as ready for review July 1, 2026 23:36
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 2, 2026
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 openclaw#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>
@shanselman

Copy link
Copy Markdown
Contributor

Thanks for the thoughtful fix here, Caleb. I did a two-model maintainer review because this touches the shared FunctionalUI renderer as well as the chat timeline. The row key/generation approach looks sound, and the ClearValue(...) cleanup matches the renderer's clear-then-apply-setters model.

One real issue came up during review: generation-qualified keys would otherwise mint new renderer paths on every /new, while the old _controls / _components / content flyout cache entries were not pruned. I pushed a small maintainer follow-up that adds a mark-and-sweep pass after each render so stale keyed generations do not become detached cache growth. Both reviewers rechecked that delta and found no remaining blocker.

Local validation on the patched branch:

  • ./build.ps1 passed
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore passed: 2686 passed / 31 skipped
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore passed: 1456 passed

I’m waiting for CI on the pushed commit, then I’m comfortable merging this.

@shanselman shanselman merged commit f89a88a into openclaw:main Jul 2, 2026
14 checks passed
@calebeden calebeden deleted the agent/fix-899-chat-render branch July 2, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: user chat bubble rendering

2 participants