fix(ui): restore session transcript reading position

[Sun-GLiang]

Summary

Restore each Desktop Session to the same semantic reading position instead of replaying a pixel offset.

Desktop now keeps one runtime-only TranscriptReadingAnchor ({ turnId, sequence? }) per Session. A Session left while reading history reopens the saved bounded range and places that Turn at the start of the viewport, while a Session left at the tail continues following background growth. Explicit search navigation retains higher priority and its existing focus/highlight behavior.

Fixes #4413

Before / After E2E

Both GIFs run the same scenario in the real Electron/Chromium transcript fixture:

1. Open Session A while reading turn-prompt-rail-1.
2. Switch to Session B and let its transcript grow at the tail.
3. Return to Session A and inspect the visible Turn.

Before - reading position is lost	After - the same Turn is restored
Returning to A shows Turn 119/120. The saved Turn is absent from the resident DOM, so the E2E geometry probe resolves to Infinity.	Returning to A shows Turn 1 at the same viewport edge. The semantic anchor is resident and the geometry probe is within 4 px.

Result: the fix succeeds. The pre-fix control fails the regression (Infinity <= 4), while the current branch passes the same scenario and also preserves Session B's tail intent after background growth.

Root cause and design

Desktop clears the transcript synchronously during a Session switch. Restoring an absolute offset while the scroller is empty therefore clamps it to zero; once bounded transcript content changes, the old pixel no longer identifies the same Turn anyway.

The replacement uses the first visible [data-turn-id] already available after #4206. useChatScroll captures that Turn through one authority-backed reporter: existing policy/reader subscriptions call it, and a completed one-shot command flushes it once even when the browser emits no scroll event. Restore releases the pin before moving the viewport. transcript-scroll-authority.tsx remains unchanged and is still the only tail writer.

Restore priority is fixed as:

1. Explicit search target — centered, focused, and highlighted.
2. Session reading anchor — behavior: auto, block: start, without focus or highlight.
3. Tail intent — pinToTail().

Range loads retain the existing stale-Session/controller guards and message-load error path. A pending or failed restore cannot replace the saved anchor with an intermediate range, and known sequence metadata is not downgraded; a Turn-only anchor is enriched as soon as its transcript range arrives.

Verification

• npm run build --workspace @maka/ui && node --test packages/ui/dist/__tests__/use-chat-scroll.test.js
• npm run test:dist --workspace @maka/ui — 308/308 passed
• npm run build:main --workspace @maka/desktop && npm run test:dist --workspace @maka/desktop — 1907/1907 passed
• npm run e2e --workspace @maka/desktop -- transcript-scroll.spec.ts — 11/11 passed in real Chromium
• Session-switch Chromium regression: geometry stabilization 10/10 passed; locale-neutral selector run under en_US 3/3 passed
• Rebased onto upstream/main (622d6fecd); the required CI run 33581124019 passed after the upstream planner fix.
• npm run typecheck --workspace @maka/ui
• npm run typecheck --workspace @maka/desktop
• node apps/desktop/scripts/check-renderer-architecture.mjs --base 622d6fecd926a9d9f16ecf2e654c96f4f4624ff8
• npx knip --workspace apps/desktop
• npx biome check on all changed source and ledger files
• Independent AI-assisted code review completed twice: runtime edge cases were fixed, then the Conversation ownership and real imported-binding ratchet were re-reviewed with no remaining finding

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Maka produced the initial diagnosis and PR. Codex re-evaluated the Issue and review feedback, redesigned and implemented the semantic-anchor solution, added unit/controller/Chromium coverage, ran verification, and updated this description. Commits c993da1bc and 80a704696 include the required Generated-by: Codex trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed 28188e65. The bug is real and your root-cause note is right about pinToTail() being unconditional — I confirmed that on main. But the repair doesn't hold on the production path: on a real Session switch the restored offset is clamped to 0 and the remembered offset is then overwritten with that 0, so returning to a Session lands you at the top instead of where you were. In streaming that's worse than what's on main today. Detail inline.

The fact the analysis is missing. setActiveId clears the transcript synchronously in the same update that changes the Session — session-workspace-actions.ts:198-199 does messagesRef.current = []; setMessagesState([]), and app-shell.tsx:1287 notes the same thing ("Session switches clear the transcript projection before its async read"). So when this effect runs, the scroller has no overflow yet. Everything else follows from that.

On "without adding another scroll-position authority" — half right. The write side is fine: releasePin() then a one-shot scrollTop write is the shape the authority's own header documents, and there are already two commands in that shape on main. The state side is where it breaks. The authority's whole design is that the answer is a single height-independent boolean, which is why it can self-correct when content changes. This adds a second representation of the same question — an absolute pixel offset — whose meaning is invalidated by the very event it's keyed on. The pinned field in the Map is also a hand-synced copy of the authority's boolean, kept current by two separate listeners.

Grading note. I considered the restore failure a merge blocker on severity and stepped it down to P2: the consequence is a viewport position — in-memory, no persisted or externally visible state, recovered by one scroll gesture. What makes me say don't merge isn't the severity, it's that the PR doesn't achieve what it sets out to do on the path it targets.

A smaller repair is available, and it's already in the tree. #4206 added the "first visible [data-turn-id]" primitive to this very file, and the input.target = {turnId, nonce} effect is already a complete restore channel — release pin, wait for the DOM node, scrollIntoView. Remembering a turnId and reusing that channel is smaller than a new pixel path, survives content changing while you were away, and dissolves the timing problem in P2-1 for free, since the target effect already re-runs on input.messages until the Turn appears. Worth rebasing first — this branch's base predates #4206 landing in use-chat-scroll.ts.

Evidence boundary: I read use-chat-scroll.ts and transcript-scroll-authority.tsx on this head against main, plus the Session-switch and transcript-open paths in session-workspace-actions.ts and app-shell-effects.ts. The behaviour in P2-1 was reproduced by driving the compiled useChatScroll through a real TranscriptScrollAuthorityProvider with production ordering (collapse scrollHeight on switch, then deliver content via the resize signal); I did not run it inside a real Electron renderer, so the exact clamp target for an empty scroller is inferred — though the conclusion holds whether it clamps to 0 or to some small positive value. Your note about composer-running-attachments checks out as unrelated: it asserts data-maka-file-drop-target and shares no code with either file.

---

AI-assisted review: drafted with Maka; I verified the synchronous transcript clear, the authority's unpinned contract, and the overwrite ordering against the branch source myself.

[Astro-Han]

P2 — this write is clamped to 0, and the line below then saves the 0 over the offset you wanted.

By the time this effect runs, setActiveId has already emptied the transcript synchronously (session-workspace-actions.ts:198-199), so the scroller has no overflow and the browser clamps scrollTop = 900 to ~0. The remember() at :98 — commented "Record the browser-clamped position immediately" — then writes {scrollTop: 0, pinned: false} for this Session, destroying the real offset. When the transcript arrives and scrollHeight grows, the authority is unpinned, and its contract is explicit about that case (transcript-scroll-authority.tsx:29: !pinned → nothing here writes scrollTop, ever). Nothing corrects the position, and every later return to this Session starts from the saved 0.

Driving the compiled hook through a real TranscriptScrollAuthorityProvider, changing only the ordering to match production (collapse scrollHeight on switch, then deliver content through the resize signal):

	after restore	after content arrives	second return
this head	0	0	0
main	0	2400 (tail)	2400

So it replaces "jumps to tail" with "jumps to top", and main is better in the streaming case: its pin is height-independent, so the resize observer re-writes to the new tail. Here the reader is parked at the top while replies keep appending below.

Two things need to change together. The restore can't happen in the sessionId effect — it has to wait until this Session's transcript is actually present. And remember() must never write back a clamped result; that stores a failed write as if it were intent.

The pixel offset is also the wrong thing to remember, independent of the timing. app-shell-effects.ts:413 constructs a new DesktopTranscriptRangeStore(activeId) and reopens the default bounded range on every activation, so earlier history the reader had paged in is not restored — 900px in the new Turn set points at unrelated content. Anchoring to a turnId and reusing the existing target channel avoids both problems, and the "first visible [data-turn-id]" helper #4206 added to this file is already the primitive for capturing it. The description doesn't say why pixels were chosen over a Turn anchor; if there's a reason, it's worth writing down.

[Sun-GLiang]

Resolved in the current head (ae5bd5995). The pixel snapshot path and copied pinned state are gone.

useChatScroll now captures the first visible [data-turn-id]. When a Session has a saved anchor, activation releases the pin and the restore effect retries as messages change until that Turn exists, then uses scrollIntoView({ behavior: 'auto', block: 'start' }). An empty/intermediate range is not reported back, so the pre-fill clamp cannot erase the saved intent.

Desktop enriches the anchor to { turnId, sequence }; if the bounded range does not contain the Turn, it calls the existing loadAround(sequence) path before replacing messages. Explicit search remains higher priority.

Fresh verification on this head:

• hook regression: 1/1 passed
• Desktop tests: 1882/1882 passed
• real Chromium transcript spec: 11/11 passed, including the A/B Session switch and bounded-range restore

The Chromium assertion observes the saved Turn at the scroller start within 4 px; the pre-fix control produces Infinity because that Turn is absent from the resident DOM.

[Astro-Han]

P2 — the fixture establishes a precondition the production caller guarantees is false, so the suite is green on behaviour that doesn't happen.

Each renderSession() is preceded by setting scrollHeight to that Session's final height, i.e. "the content is already at full height at the moment of the switch". Production is the opposite: the transcript is cleared synchronously and arrives asynchronously afterwards. That's why these 133 lines pass while the behaviour in the other comment goes unnoticed, and it's why CI is green.

Credit where it's due — this does drive the real useChatScroll inside a real TranscriptScrollAuthorityProvider rather than poking internal state, and "fails without it" is true (on main the last return pins to 2400 and the assert.equal(900) fails). One test with four assertions is the right size, not a matrix.

To make it mean something: collapse scrollHeight to clientHeight at switch time, deliver the content through a triggerable resize stub, and then assert the restored position. That ordering turns this red against the current implementation. Worth also asserting the Session comes back unpinned, which #4413 asks for explicitly and no assertion currently covers.

[Sun-GLiang]

The fixture has been rewritten around the production ordering you described.

It now explicitly:

1. collapses the transcript to clientHeight and clears its Turns,
2. switches the Session,
3. installs the transcript asynchronously,
4. delivers the resize signal,
5. returns through another empty transcript before filling the saved range.

It asserts both the restored Turn geometry and authority.getSnapshot().pinned === false. The same test keeps Session B pinned, grows its transcript while away, and verifies it returns at the latest tail.

There is also a real Electron/Chromium regression in transcript-scroll.spec.ts that performs the full A → B → A switch, forces A out of the latest bounded window, asserts A's saved Turn returns within 4 px of the scroller start, and asserts B remains at tail after background growth.

Fresh results on the current head: hook regression 1/1, Desktop 1882/1882, Chromium transcript spec 11/11.

[Astro-Han]

P3 — this Map's lifetime is incidental, and the repo already has an owner for per-Session view state.

As a ref inside useChatScroll, it lives exactly as long as ChatView stays mounted. But ChatMessageSurface is mounted conditionally on navSelection.section === 'sessions', and activating WorkHub swaps the whole ChatSurfaceLayout. So every remembered position is silently dropped when the user visits Settings or WorkHub and comes back — a rule nobody declared, and one a reader of this hook can't see. There's also no cleanup when a Session is deleted or closed.

app-shell-session-ui-state.ts already holds the per-Session renderer state in its *BySession registries and already has clearSessionUiState(sessionId) wired into the Session lifecycle (session-workspace-actions.ts:219). Viewport intent belongs there, reaching ChatView through the existing scrollTargetTurn prop, rather than as a new unmanaged Map inside a general-purpose @maka/ui hook — which also keeps @maka/ui free of Session lifetime concerns.

Not reporting growth as an issue: the entries are a number, a boolean and an id, so even hundreds of Sessions are negligible. The problem is ownership, not size.

[Sun-GLiang]

The per-Session state now lives in the Desktop Session UI controller, not in useChatScroll.

The controller owns a runtime-only TranscriptReadingAnchor { turnId, sequence? } registry. It is intentionally non-observable, so ordinary scrolling does not notify render subscribers; clearSessionUiState(sessionId) clears the anchor alongside the rest of that Session's UI lifetime. @maka/ui only reports/carries a semantic Turn target and has no Session registry.

The controller test verifies:

• anchors are isolated by Session,
• anchor writes do not notify subscribers,
• sequence metadata is not downgraded,
• clearing/deleting a Session removes its anchor without touching others.

Fresh Desktop verification on the current head passed 1882/1882.

[me2seeks]

Reviewed the restore path end to end: anchor capture (captureTranscriptReadingAnchor), runtime-only registry lifecycle (cleared on clearSessionUiState), range restore (restoreSessionTranscriptRange with search-target priority and disposal guards), and the scroll authority interplay (pin release before async fill, reader-scroll re-pin at tail, geometry-change discrimination). The semantic-anchor design is sound and the E2E matrix is strong. One boundary finding:

[P2] A restore target that never renders leaves the session permanently unpinned and freezes anchor capture for the app's lifetime.

The command effect in use-chat-scroll.ts waits for [data-turn-id="<id>"] with no bound. If the element never appears, three things stick:

1. commandTarget.current stays set, so the reporter's guard (commandTarget.current && handledTarget.current !== commandTarget.current) suppresses anchor reporting forever — the stale anchor can never be updated or cleared, and every re-activation of the session re-attempts the same restore (activation resets on sessionId change, re-reading the same stored anchor).
2. The activation effect releases the pin on every visit, so the session opens not-following-growth each time; only a manual scroll to the tail re-engages the pin (the authority's reader-scroll path), and even then the anchor stays frozen because of (1).

The trigger does not require turn deletion (though revision/compaction would do it): captureTranscriptReadingAnchor can persist a { turnId } anchor without a sequence when the range store can't resolve it, and restoreSessionTranscriptRange then returns early (target.sequence === undefined) without ever calling loadAround — the turn exists in durable history but is never loaded, so the element never renders. The restore-side enrichment retry only helps if the controller's range happens to be loaded for that session at activation time.

A bounded fallback would close it: after ready() + loadAround settle (or after a fixed number of transcript updates), if the target turn still isn't in the store, treat the restore as landed-but-absent — mark the command handled, clear or re-anchor to the first visible turn, and let the pin/reporting machinery resume.

[P3] Restore-side enrichment doesn't verify range ownership. In restoreSessionTranscriptRange, the enrichment controller.store.sequenceForTurn(turnId) — unlike captureTranscriptReadingAnchor, which checks range().sessionId === sessionId first — reads the store without confirming the range belongs to the session being restored. Turn ids make a cross-session hit practically impossible, so this is only about consistency with the capture-side guard, but the asymmetry is what allows the unenriched-anchor path above to survive activation.

Everything else checked out: search navigation correctly preempts and clears the restore, restore uses block: 'start' without stealing focus/highlight, the empty-range guard prevents a pre-fill empty transcript from erasing the anchor, and runtime-only storage means no persistence-format concerns.

[Sun-GLiang]

Addressed this review in 13891e13d, after rebasing the branch onto Apache main at 92fa52819.

P2 — bounded missing-target fallback

• restoreSessionTranscriptRange now waits for the range controller to become ready, attempts loadAround(sequence) when sequence metadata is available, and then verifies that the target Turn is actually resident.
• If the Turn is still absent, Desktop clears the stale reading anchor and publishes a Session-owned, observable restore-unavailable marker.
• useChatScroll consumes that marker as a terminal command state. It marks the command handled, then re-anchors to the first visible Turn or returns an empty transcript to tail intent. Anchor reporting is therefore no longer suppressed indefinitely, and revisiting the Session does not repeatedly leave it unpinned.
• Pending and unavailable phases use distinct handled keys, and handled state is reset per Session activation, so the same semantic target can be handled correctly on a later activation.

P3 — range ownership

Sequence enrichment now requires controller.store.range()?.sessionId === sessionId before reading sequenceForTurn or loading around the result. A stale range from another Session cannot enrich or drive the restore.

Regression coverage now includes:

• foreign-Session range ownership;
• Turn-only targets absent after ready();
• known-sequence targets still absent after loadAround();
• unavailable-marker publish/consume lifecycle;
• reactivation with the same target identity;
• first-visible fallback and empty-transcript tail fallback.

Fresh verification on the rebased head:

• UI: 309/309
• Desktop: 1961/1961
• Session-switch Chromium regression: 3/3
• full workspace build; UI/Desktop typecheck
• Biome, knip, and renderer architecture check against 92fa52819

[Astro-Han]

Approving 13891e13. Both points from last round are closed for real. The restore now has three converging exits (resident after ready(), no sequence, still absent after loadAround), the unavailable marker is a renderer-local per-Session map with one writer and two clearers, handledTarget is set before the anchor is cleared so reporting resumes within the same activation, and the pending/unavailable keys plus the per-activation reset stop a later visit from being swallowed by an old success. The range-ownership guard sits at all three reads of sequenceForTurn and loadAround. All six regressions go through the production owners, @maka/ui 309/309 and the desktop suites pass locally, merge-tree is clean.

Three small things, none blocking:

• controller.ready() resolves on the open handshake, which the main process answers after the first delivery batch; the store's own range().ready is the signal for the whole window. A target Turn in a second batch reads as absent right after ready() and the anchor is dropped for a first-visible fallback. Cheap loss, one scroll recovers it, but the right authority already exists: judge absence only once range().ready is true.
• app-shell-session-ui-state.ts and task-readiness-notice.ts are now pure re-export shims while app-shell.tsx imports from ./features/conversation directly, and a few files still go through the shims. Point them at the feature and delete the shims in this PR; the move created the redundancy, so the move should remove it.
• The glue in app-shell.tsx between the marker and restoreTargetTurn, and the clear in handleTranscriptReadingAnchorChange, has tests on either side but none across it; the UI test re-creates that shape by hand. A missing-target E2E would close that.

Evidence boundary: static read of 13891e13 against main 92fa5281; @maka/ui and desktop main suites run locally, typecheck, Biome, knip and the renderer architecture check green; Playwright not run, the 3/3 Chromium result is the author's.

AI-assisted review: drafted with Maka; I verified the restore exits, the ready() implementation and the shim imports myself.

简体中文

批准合并。上轮两条都真正关了：restore 三条出口收敛，marker 是 renderer 本地状态、单写两清，handledTarget 先于 anchor 清除所以同一 activation 内 reporting 一定恢复，ownership 守卫覆盖三处读。三条不阻塞：controller.ready() 只等 open 握手，多批投递时目标 Turn 在第二批会被误判缺失，应以 range().ready 为准；两个纯转发 shim 应在本 PR 删掉；marker 到 restoreTargetTurn 的胶水没有跨层测试。

[Astro-Han]

controller.ready() is the open handshake, answered after the first delivery batch. If the target Turn arrives in a later batch this reads null and the anchor is abandoned. store.range().ready is the whole-window signal; gate the absence verdict on it.