fix(chat): preserve conversation root and transcript layout state during temporary overlay transitions#47
Conversation
…ing temporary overlay transitions Root cause: The `ChatConversationSurfaceProjectionCoordinator` mapped `ShouldLoadActiveConversationRoot` and `ShouldLoadTranscriptSurface` directly from the `ShouldShow...` state of `ChatConversationSurfaceStatePresenter`. As a result, when an activation overlay (like the session switch mask) became visible, the loading state regressed to `false`, effectively destroying the visual tree via Uno `x:Load`. This broke the layout-loading state's expected monotonically and caused the UI skeleton/content to completely unload instead of merely being hidden behind the overlay. Safe architectural fix: Introduced boolean latches (`_hasLoadedActiveConversationRoot` and `_hasLoadedTranscriptSurface`) inside the `ChatConversationSurfaceProjectionCoordinator` that permanently lock to `true` once their respective visibility properties are activated for the first time. This ensures that the layout surface remains loaded even during temporary overlays, adhering to monotonic lifecycle requirements and preserving layout state for a warm resume. Verification: - `dotnet build SalmonEgg.sln --configuration Debug` - `dotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restore` (All passed cleanly)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Closing during PR triage: this is a stale or duplicate layout-loading monotonicity attempt. #49 is the clean green candidate for this behavior; this PR is superseded, failing/conflicting, or mixed-scope and should not be merged as-is. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
The Observed Problem
During the daily lifecycle review of the chat page activation, it was discovered that the
ShouldLoadActiveConversationRootandShouldLoadTranscriptSurfaceproperties were incorrectly mapped to their visual visibility counterparts (ShouldShowActiveConversationRootandShouldShowTranscriptSurface). Consequently, when an overlay, such as the session-switch preview or connection lifecycle overlay, appeared (making the surface visibilityfalse), the underlyingx:Loadlayout state would regress tofalse. This destroyed the loaded UI skeleton and transcript visual trees unnecessarily.The Root Cause
In
ChatConversationSurfaceProjectionCoordinator.cs, the coordinator simply forwarded the boolean output ofShouldShowActiveConversationRootandShouldShowTranscriptSurfaceto theShouldLoad...properties. The load state was not monotonic.Why the Fix is Safe Architecturally
The fix correctly introduces a simple one-way latch inside the
ChatConversationSurfaceProjectionCoordinatorclass (_hasLoadedActiveConversationRootand_hasLoadedTranscriptSurface). By keeping this state derivation encapsulated entirely within the Presentation.Core projection layer, no business rules were leaked into the View or code-behind. This maintains pure Uno/WinUI cross-platform compatibility and honors the rule that the skeleton must not regress to an earlier stage after reaching a later one.Exact Verification Commands Run
dotnet build SalmonEgg.sln --configuration Debug(to ensure the core and platform projects compile cleanly)dotnet test tests/SalmonEgg.Presentation.Core.Tests/SalmonEgg.Presentation.Core.Tests.csproj --configuration Debug --no-restore(to guarantee no regressions in Core Mvux tests, explicitly checking the updated ChatViewModelTests and ChatConversationSurfaceProjectionCoordinatorTests assertions)Any Remaining Unverified GUI-only Risks
None identified. The changes exclusively affect view models and UI presentation bindings in the core module, which are entirely covered by the modified unit tests.
PR created automatically by Jules for task 6084835674123996412 started by @YoungSx