Repository navigation
refactor(ui): move section routing out of ChatView - #831
Merged
Merged
Conversation
Astro-Han
added a commit
that referenced
this pull request
Jul 12, 2026
#831 lifted section routing out of ChatView: leaving chat now unmounts the component together with its scroll DOM, and the mode prop is gone. The warm-up effect no longer needs (or can have) a props.mode dependency — returning to a session re-runs it naturally against the rebuilt turn nodes. The E2E mode-switch regression test still locks the user-visible invariant end-to-end.
Astro-Han
added a commit
that referenced
this pull request
Jul 12, 2026
…y turns (#828) * fix(ui): warm up content-visibility turns to stop endless upward scroll Never-rendered turns in a remounted long session occupy only the 250px contain-intrinsic-size placeholder, so scrolling up inflates the document turn by turn while scroll anchoring keeps compensating — the top keeps receding. Verified against the live app: a seeded 24-turn session mounted at scrollHeight 14640 vs 31872 real and bounced through 51 anchor-corrected steps to reach the top; Chromium never renders the skipped turns on its own. Add a chunked, bottom-up idle-time warm-up (createTurnSizeWarmup) that forces each turn through one rendering opportunity so the browser records its last remembered size, then releases it back to content-visibility:auto. After warm-up the same session reports exact geometry after ~2 idle chunks and scrolls to the top with constant height and monotonic scrollTop. * fix(ui): keep pinned-to-bottom through render-skipped turn inflation Opening a long session pins the viewport against placeholder geometry (content-visibility turns still at their 250px estimate). The turns then inflate to real heights with no DOM mutation and no scroll event, so the one-shot pin plus the mutation-only follower left the viewport stranded mid-document — verified live at scrollTop 5408 of an eventual 31872. Give createPinnedBottomFollower a second channel: a ResizeObserver on the content's element children (re-synced when mutations add turns), so any height growth re-pins while the user is pinned. Verified live: a remounted 24-turn session mounts at distanceFromBottom 0, stays 0 while warm-up grows the document 23256 to 31872, and an immediate user scroll-up is respected (distance preserved exactly, no yank-back). * fix(ui): record warm-up sizes from the final layout The scroll-geometry E2E contract caught the warm-up remembering sizes from a transient mount-time layout: until the lazy markdown-body chunk commits, every bubble is the plain-text Suspense fallback (~7.5px taller per turn), so each turn scrolled into view corrected its height and the scrollbar drifted through the whole climb (24 x 7.5px). Awaiting the chunk import is not enough — the Suspense retry is a low-priority render that can commit after an idle callback — so the walk now starts when the DOM holds no .maka-markdown-pending fallback (bounded poll, degrades to warming anyway) and document.fonts.ready has resolved. Warm-up also now forces 'contain: layout style paint' alongside content-visibility: visible, matching the containment content-visibility: auto applies to on-screen turns, so sizes are measured under the same containment they will have when the turn scrolls into view. * test(e2e): lock chat scroll geometry with a long-transcript fixture Add a visual-smoke long-transcript scenario (24 turns, ~1300px each, opened as the active session on boot) so E2E can mount a session whose above-viewport turns are render-skipped placeholders — previously impossible outside fixture mode because sessions:readMessages projects from the AgentRun/RuntimeEvent ledger, so store-seeded messages read back empty, and building tall turns through the fake backend streams for minutes. The scroll-geometry spec locks both user-visible invariants: the session opens pinned to bottom and stays pinned while the warm-up grows the document, and scrolling a settled session to the top observes a single scrollHeight in the honest number of steps. Probes read only scroller metrics; per-turn getBoundingClientRect would force-render skipped turns and mask the regression. Verified RED on a build with both fixes reverted (mid-document mount; 23 distinct heights while climbing). * test(e2e): gate scroll-geometry settling on warmed-scale height Two consecutive scrollHeight reads agreeing is not proof the warm-up finished: on a slow machine the fonts/lazy-markdown gates can delay the walk past the first reads, and two agreements on the PLACEHOLDER height would declare the geometry settled before it ever grew. Require the fixture's warmed scale (24 turns x >800px real vs 250px placeholder) before the stability check, and extract the settle/climb helpers. * fix(ui): rewalk turn warm-up when a mode switch rebuilds the transcript Leaving chat for skills/automations/daily-review unmounts the scroll DOM; returning rebuilds every .maka-turn with no remembered size, so the placeholder geometry (and the endless upward scroll) came back. The pinned-bottom follower effect already re-runs on props.mode for exactly this reason — the warm-up effect missed the dependency. Found by external review (P1). RED-verified: the new E2E test fails without the dependency (settle times out at placeholder height). * fix(ui): drop the warm-anyway deadline from the markdown settle gate If the lazy markdown chunk commits after the 5s cap, the walk records the plain-text fallback layout and re-creates the exact per-arrival drift the warm-up exists to remove. Poll until the pending markers are gone instead: warming a transient layout is never right, and if the chunk truly never loads, placeholder scroll geometry is the least of that session's problems. Found by external review (P2). * test(e2e): treat an in-flight warm-up walk as not settled A stalled compositor freezes mid-walk geometry, and frozen reads are equal reads — on CI the settle check declared a stuck warm-up "settled" and the pinned test passed vacuously. The walk's forced inline content-visibility is visible DOM state: any remaining forced turn means the walk is still in flight, so require its absence before the stability check. * test(e2e): make the climb hang-proof and self-diagnosing Both climb tests died on CI as bare 60s timeouts with the evaluate still in flight — zero information twice in a row. Give the climb an in-page deadline (30s, under the test timeout), a 2s watchdog per frame wait, and return frame statistics (frames seen, max frame gap, elapsed); assert on them so a stalled or crawling compositor fails with numbers instead of 'Target page ... closed'. * fix(desktop): show E2E windows on CI so the compositor runs at full rate E2E windows launch hidden so local runs never steal focus. On Linux CI a hidden window's compositor only produces ~1fps BeginFrames — measured by the instrumented climb: 38 frames over 31s, single stable scrollHeight (the geometry itself was already correct). The warm-up's handful of frames squeaked through; the climb's ~106 frames cannot. Under xvfb there is no focus to steal, so the E2E harness sets MAKA_E2E_SHOW_WINDOW=1 when CI is set and main.ts lets it override startHidden. Local runs and visual-smoke capture stay hidden. Replaces the backgroundThrottling attempt, which the instrumented run refuted (the throttle applies to BeginFrame production for unmapped windows, not renderer-side timer policy). * fix(ui): adapt turn warm-up to chat-only ChatView (#831) #831 lifted section routing out of ChatView: leaving chat now unmounts the component together with its scroll DOM, and the mode prop is gone. The warm-up effect no longer needs (or can have) a props.mode dependency — returning to a session re-runs it naturally against the rebuilt turn nodes. The E2E mode-switch regression test still locks the user-visible invariant end-to-end.
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
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.
Summary
@maka/uipage components selected by app-shell.ChatViewchat-only by removing section routing,mode, and all non-chat module props.Why
Stage 1 of #829.
navSelectionbelongs to app-shell, butChatViewpreviously survived section switches while its scroll DOM did not. That split lifecycle forced every scroll effect to depend on navigation state individually and caused the defect class found in #828. MountingChatViewonly for sessions makes its component and DOM lifecycles coincide.Refs #829.
Scope
This PR changes page ownership only. Turn rendering and scroll-effect extraction remain for Stages 2 and 3. It does not redesign module pages, chat behavior, or visuals.
Verification
npm --workspace @maka/ui test— 105 passed.npm --workspace @maka/desktop test— 2,351 passed, including renderer source contracts and accessibility/copy checks.npm run build— production builds passed; Skills, Automations, and Daily Review remain separate lazy chunks.scrollTop; it only retained stalepinned=false, so the proposed keep-alive would restore the lifecycle defect this stage removes.Impact
No user-visible, storage, API, migration, documentation, or release-note impact. The internal
@maka/uisurface gains three page-level exports;ChatViewdrops non-chat props.Reviewer notes
Returning from a module now remounts a fresh transcript DOM pinned to the latest message. The old implementation did not preserve scroll position because its early return also destroyed that DOM; retaining the old
pinned=falsestate merely left the rebuilt transcript at the top.Ready for review
Verificationexplains why it is not applicable