refactor(chat): remove the retired FunctionalUI chat surface - #1110
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed August 20, 2026, 12:31 PM ET / 16:31 UTC. ClawSweeper reviewWhat this changesThis PR removes the retired FunctionalUI native-chat implementation and legacy-only tests, leaving Reactor as the production ChatPage and ChatWindow renderer. Merge readiness⛔ Blocked until stronger real behavior proof is added - 3 items remain Keep open for current-head proof refresh. The latest commit preserves the assistant-media callback that current main uses, but the submitted runtime evidence predates that commit. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherGateway chat state reaches the shared chat provider, then the Reactor root builds timeline inputs for the WinUI chat page and popup. The timeline uses those inputs to render messages, tool cards, and assistant media. flowchart LR
A[Gateway chat events] --> B[Chat data provider]
B --> C[Reactor chat root]
C --> D[Timeline presentation context]
D --> E[Reactor timeline]
E --> F[Assistant media cards]
E --> G[Chat page and popup]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep Reactor as the sole native-chat owner and add redacted current-head proof that an assistant image resolves and renders through the retained provider-to-timeline path. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a refactor, not a bug report; source inspection verifies the retained media callback path, while current-head runtime proof remains needed. Is this the best way to solve the issue? Yes, subject to current-head proof: moving shared inputs to the Reactor-neutral context preserves the established production owner without retaining duplicate FunctionalUI code. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3880b62dd517. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (25 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Remove the unreachable FunctionalUI chat root, timeline, composer, virtualization helpers, proof fixtures, and migration-debt tests. Keep remaining FunctionalUI consumers intact and guard production chat ownership through the Reactor mount path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf3409c-6c88-4d6c-b015-7e9cbf5e02da
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf3409c-6c88-4d6c-b015-7e9cbf5e02da
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1bf3409c-6c88-4d6c-b015-7e9cbf5e02da
2829648 to
3d3e9e5
Compare
Current-head chat E2E proofValidated commit
dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --filter "FullyQualifiedName~NativeToolIdentityScreenshotProofTests"
dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --no-build --filter "FullyQualifiedName~SessionTitleBehaviorProofTests"
dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --no-build --filter "DisplayName~ChatPage|FullyQualifiedName~ChatComposerControls_ExposeOnscreenLayoutThroughUia"
dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --no-build --filter "FullyQualifiedName~MountedReactorChatDisposalProofTests|FullyQualifiedName~ChatComposerControllerPasteFencingProofTests|FullyQualifiedName~ReactorToolActivityProofTests"Full chat surfaceExpanded activity detailFocused crop of the same current-head capture. It shows the three expanded tool rows, bounded inputs, and the sensitive command rendered as |


Related: #1115
What Problem This Solves
The native chat codebase still carried a complete FunctionalUI chat root, timeline, composer, virtualization layer, resources, and focused proof fixtures after production ChatPage and ChatWindow had moved to Reactor. This left two apparent owners for chat presentation and preserved several thousand lines of unreachable migration code.
Why This Change Was Made
This removes the retired FunctionalUI chat implementation and its chat-only virtualization infrastructure, resources, and migration-debt tests. Production chat remains owned by
OpenClawReactorChatRootandReactorChatTimeline; the presentation inputs formerly declared by the legacy timeline now live in the Reactor-neutralChatTimelinePresentationContextrecord. FunctionalUI remains available for active consumers such as Cron markdown rendering.This PR intentionally does not design or add a replacement Reactor runtime-test suite. The deleted proofs mounted the unreachable FunctionalUI renderer and contained assertions tied to
VirtualStack,ItemsRepeater, FunctionalUI control caches and reconciliation, removed XAML styles, and legacy scroll-timer mechanics. Porting the still-relevant user-facing invariants requires Reactor-specific test design rather than a mechanical test migration. That separate work is tracked in #1115 so this PR remains focused on deleting unreachable code and tests that no longer exercise production behavior.User Impact
No user-visible behavior is intended to change. Native chat continues to use the existing Reactor surface, while maintainers now have one production chat implementation and substantially less dead code to maintain.
Evidence
ReactorToolActivityProofTestsran against the chat-runtime-identical parent: 1 passed, 0 failed, 0 skipped.Chat messageslist andChatComposerInputeditor in the selected Chat route.OpenClawReactorChatRootthroughReactorChatHostExtensions.Change Type
Scope
winnodeValidation
./build.ps12829648f556e96f929fdb18de64fb3ee2c36a3aa: documentation validation, Shared, CLI, WinNode CLI, SetupEngine, and WinUI builds.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore2829648f556e96f929fdb18de64fb3ee2c36a3aa: 3,412; skipped: 32; failed: 0.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore2829648f556e96f929fdb18de64fb3ee2c36a3aa: 2,162; skipped: 0; failed: 0.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore --filter "FullyQualifiedName~ArchitectureLedgerConsistencyTests"2829648f556e96f929fdb18de64fb3ee2c36a3aatree: 6; skipped: 0; failed: 0.dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --filter "FullyQualifiedName~ReactorToolActivityProofTests"cb76e845739960ae1cfad9f7e85fd58f30dd8c82: 1; skipped: 0; failed: 0. The subsequent merge changed no production chat runtime files.Real Behavior Proof
Environment tested: Windows 11 ARM64, .NET 10 preview SDK, Windows App SDK build target
win-arm64.Current PR head validated:
2829648f556e96f929fdb18de64fb3ee2c36a3aa.Runtime chat capture commit:
cb76e845739960ae1cfad9f7e85fd58f30dd8c82.Runtime equivalence check:
git diff --name-only cb76e845..2829648f -- src/OpenClaw.Tray.WinUI/Chat src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs src/OpenClaw.Tray.WinUI/Windows/ChatWindow.xaml.csreturned no changed files.Exact production command run:
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.Executed Reactor-host proof:
dotnet test .\tests\OpenClaw.Tray.UITests\OpenClaw.Tray.UITests.csproj -r win-arm64 --no-restore --filter "FullyQualifiedName~ReactorToolActivityProofTests"passed 1 of 1 tests.Sanitized production UIA transcript:
Evidence after fix: The production Chat route mounted a native message list and composer. The connected zero-state surface rendered its welcome content and suggestions with the composer empty.
Observed result: The retained Reactor chat route is operational, and the current head contains the same production chat runtime files as the captured commit. This is a regression smoke, not a claim that runtime evidence alone proves the deleted implementation was unreachable.
Screenshot or artifact links verified? (
Yes/No/N/A): N/A. The directly copied UIA transcript and executed Reactor-host test are the proof items.Not verified or blocked: The popup ChatWindow was not captured in the runtime smoke. Its production Reactor mount remains covered by the positive ownership contract. Broader Reactor timeline runtime coverage is tracked in test(chat): add Reactor runtime coverage for timeline behavior #1115.
Security Impact
Yes/No): No.Yes/No): No.Yes/No): No.Yes/No): No.Yes/No): No.Yes, explain the risk and mitigation: N/A.Compatibility and Migration
Yes/No): Yes.Yes/No): No.Yes/No): No.Review Conversations