refactor(chat): remove the retired FunctionalUI chat surface - #1110
Open
calebeden wants to merge 5 commits into
Open
refactor(chat): remove the retired FunctionalUI chat surface#1110calebeden wants to merge 5 commits into
calebeden wants to merge 5 commits into
Conversation
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>
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: found issues before merge. Reviewed August 12, 2026, 9:17 AM ET / 13:17 UTC. ClawSweeper reviewWhat this changesThe PR removes the retired FunctionalUI native-chat implementation and related tests while retaining the Reactor chat surface. Merge readinessThe cleanup remains useful, but the current PR head is not mergeable against main and must be rebased before it can be safely reviewed for landing. Priority: P3 Review scores
Verification
How this fits togetherThe tray Chat page and popup render gateway conversation state through a native Reactor chat host. This change deletes an older FunctionalUI rendering path and moves its shared presentation inputs to the active Reactor timeline. flowchart LR
Gateway[Gateway chat state] --> Provider[Chat data provider]
Provider --> Root[Reactor chat root]
Root --> Timeline[Reactor timeline]
Timeline --> Page[Chat page]
Timeline --> Popup[Chat popup]
Legacy[Retired FunctionalUI chat] --> Removal[Deletion in this PR]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase the cleanup onto current main, preserve active Reactor behavior while applying the neutral presentation context, then validate the resolved Windows chat route. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a refactor cleanup, and the submitted Windows UI-automation smoke verifies the retained Reactor route rather than a bug reproduction. Is this the best way to solve the issue? Yes, once rebased: deleting the unreachable FunctionalUI path while retaining the Reactor owner is focused, but the dirty merge cannot establish the resulting behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4206611f2b2e. LabelsLabel 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 (23 earlier review cycles; latest 8 shown)
|
Mark the production Reactor mount contract as a source-shape guard and document when the ledger closure can be retired. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
Resolve the Reactor presentation-context migration across current-main tool activity rendering and UI proof consumers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
added
rating: 🦪 silver shellfish
Contributor
Author
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Mark the Reactor timeline ownership contract as a source-shape guard and document its retirement condition. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
added
proof: sufficient
Preserve the Reactor chat architecture documentation while adopting the repository documentation standardization changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
status: 👀 ready for maintainer look
calebeden
marked this pull request as ready for review
August 7, 2026 21:28
clawsweeper
Bot
added
merge-risk: 🚨 other
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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