Skip to content

fix(desktop): stabilize prompt rail history scrolling - #5147

Open
MicroGery wants to merge 3 commits into
apache:mainfrom
MicroGery:fix/prompt-rail-history-anchor
Open

fix(desktop): stabilize prompt rail history scrolling#5147
MicroGery wants to merge 3 commits into
apache:mainfrom
MicroGery:fix/prompt-rail-history-anchor

Conversation

@MicroGery

@MicroGery MicroGery commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Prompt Rail navigation previously made the selected Turn the oldest resident Turn. The first upward gesture therefore collided with an asynchronous history prepend, while the transient gap row could also become the browser's scroll anchor. A sufficiently large trackpad gesture produced visible stalls and jumps.

This change treats the selected Turn as a reading position instead of a range boundary:

  • load bounded durable pages on both sides of a non-tail Prompt Rail target;
  • trim the combined range around the protected target using the existing byte and Turn budgets;
  • exclude the transient transcript gap row from browser scroll anchoring.

The reporter manually verified the updated development build on the affected conversation.

CI follow-up: two existing navigation fixtures returned records on the wrong side of an exclusive older-page anchor. They now return an empty page before the first record and honor direction, anchor, and through-sequence bounds. Added request/empty-page assertions preserve the original ACK and oversized-Turn checks; production boundary validation is unchanged.

Verification

Latest local verification after aligning the navigation test fixtures:

  • npm --workspace @maka/desktop run build:with-deps — passed
  • npm --workspace @maka/desktop run test:dist — 2459/2459 passed, no failures or skips
  • Navigation race, navigation regression, and transcript range store suites — 65/65 passed, including both tests that failed in CI
  • npx playwright test --config e2e/playwright.config.ts e2e/transcript-scroll-cost.spec.ts --workers=1 in apps/desktop — 4/4 passed
  • npm run typecheck in apps/desktop — passed
  • npm run check:e2e-budget in apps/desktop — passed (38 tests in 19 files)
  • npm run windows:inventory — passed (89 declarations current)
  • Biome, ASF headers, and diff checks on the two fixture changes — passed

The large upward gesture E2E previously reproduced the old boundary behavior and passed 3/3 repeated runs after the production fix. The full four-test scroll suite now also passes after the fixture corrections.

AI use

Select exactly one:

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

Tool(s) and scope: Codex diagnosed the range-boundary and browser-anchor interaction, implemented the fix, added regression coverage, and corrected the CI test fixtures under human direction. The production scroll behavior was also manually validated by the reporter.

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

Load a bounded history window on both sides of a prompt-rail target and prevent transient gap rows from becoming scroll anchors.

Generated-by: Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 10, 2026
Record the additional real-window transcript test and the Electron boundary it exercises.

Generated-by: Codex
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for chasing this down. Two notes before this can merge:

  1. CI is red on two suites this PR did not run: transcript-navigation-race.test.ts:232 and transcript-navigation-regression.test.ts:43 both fail with Desktop transcript older range crossed its anchor. Their fixtures answer every older request with the same page, so the new correlation check fires. The fixtures need to return a real older page, as you did in desktop-transcript-range-store.test.ts.
  2. The change is the right instinct (the target is a reading position, not a range boundary) but it moves the 10-Turn budget from one side of the target to the other rather than sizing it to the viewport. The test expectation going from [6..10] to [4..7] shows the newer side now pages on the first downward gesture instead. refactor(desktop): let the Renderer own the transcript window and the reading position #5163 proposes the structural version of this: the Renderer owns the window and sizes it in screens. I would rather land that than widen this one further, but if you want this in as an interim fix, fixing the two suites above is all it needs.

Astro-Han added a commit that referenced this pull request Sep 10, 2026
Main owned the presentation window: a resident range with reading
anchors, navigation versions, overlay settlement across pages and
eviction bookkeeping, while the Renderer kept its own scroll state on
top. Two owners of one window meant every reader gesture met Main's
range accounting, and the reading position existed in six
representations that had to agree.

Main now keeps a tail cache and answers page requests pass-through.
`loadBefore` / `loadAfter` return a page without touching the cache,
`loadAround` and `loadLatest` return a reset snapshot, and catch-up
evicts whole Turns from the oldest edge while always keeping the newest.
The Renderer owns the window: it extends it by pixel bandwidth, trims it
with `retain`, and re-opens the edge it trimmed as a gap. `hasOlder` /
`hasNewer` are Host page cursors on an answer; the Renderer combines them
with its own coverage, so paging reaching an end never means nothing
exists outside the window. `navigationVersion` invalidates navigation
only — Session id, replica generation and Host epoch stay independent
identity checks.

Overlay settlement follows the same rule. Main used to broadcast
`completedOverlayMessageIds`, so a window retired an overlay whether or
not it installed the durable row that replaced it. A window off the tail
declines tail growth, so it deleted the overlay and dropped the body in
the same batch and the Turn vanished from the reader's view. The catch-up
broadcast also filtered its rows through Main's own tail residency, so a
row installed and immediately evicted under budget never reached any
window at all. Retirement is now the window's own inference — installing
a durable row retires the overlay it settles — and the broadcast carries
every row the catch-up read.

The reading position is one representation: the authority publishes the
Turn crossing the top of the scrollport, the prompt rail derives its tick
from that instead of running its own observers, and the controller keeps
a bookmark to re-anchor after a replica generation change.

Behavior changes: opening a Session prefetches history until two
viewports sit above the reader; tail growth marks read while any consumer
is open, where it previously waited for the reader to have nothing newer.

Supersedes #5147, which patched these symptoms at the old window
authority.

Closes #5163

Generated-by: Claude Code
Return an empty page before the first durable record and honor exclusive anchors when selecting oversized history pages. Assert the expected read requests while preserving the ACK and selected-Turn regression checks.

Generated-by: OpenAI Codex
Astro-Han added a commit that referenced this pull request Sep 11, 2026
* refactor(desktop): give the Renderer the transcript window

Main owned the presentation window: a resident range with reading
anchors, navigation versions, overlay settlement across pages and
eviction bookkeeping, while the Renderer kept its own scroll state on
top. Two owners of one window meant every reader gesture met Main's
range accounting, and the reading position existed in six
representations that had to agree.

Main now keeps a tail cache and answers page requests pass-through.
`loadBefore` / `loadAfter` return a page without touching the cache,
`loadAround` and `loadLatest` return a reset snapshot, and catch-up
evicts whole Turns from the oldest edge while always keeping the newest.
The Renderer owns the window: it extends it by pixel bandwidth, trims it
with `retain`, and re-opens the edge it trimmed as a gap. `hasOlder` /
`hasNewer` are Host page cursors on an answer; the Renderer combines them
with its own coverage, so paging reaching an end never means nothing
exists outside the window. `navigationVersion` invalidates navigation
only — Session id, replica generation and Host epoch stay independent
identity checks.

Overlay settlement follows the same rule. Main used to broadcast
`completedOverlayMessageIds`, so a window retired an overlay whether or
not it installed the durable row that replaced it. A window off the tail
declines tail growth, so it deleted the overlay and dropped the body in
the same batch and the Turn vanished from the reader's view. The catch-up
broadcast also filtered its rows through Main's own tail residency, so a
row installed and immediately evicted under budget never reached any
window at all. Retirement is now the window's own inference — installing
a durable row retires the overlay it settles — and the broadcast carries
every row the catch-up read.

The reading position is one representation: the authority publishes the
Turn crossing the top of the scrollport, the prompt rail derives its tick
from that instead of running its own observers, and the controller keeps
a bookmark to re-anchor after a replica generation change.

Behavior changes: opening a Session prefetches history until two
viewports sit above the reader; tail growth marks read while any consumer
is open, where it previously waited for the reader to have nothing newer.

Supersedes #5147, which patched these symptoms at the old window
authority.

Closes #5163

Generated-by: Claude Code

* test(desktop): measure the reader at transcript range boundaries

The suite asserted that paging works and stays bounded, and sampled the
mounted count only once the range had settled. Neither says where the
reader ended up while a page was installing, which is the whole of what
#5163 reports. This probe reads every frame, and at each change of the
mounted range compares a Turn present on both sides: with no input
between two frames its document position must not move, so
`Delta top + Delta scrollTop` is zero unless the boundary displaced the
reader.

Measured settled-to-settled rather than across the changing frames: the
range passes through an intermediate commit that mounts far more Turns
than it keeps, and scroll anchoring corrects after layout, so a reading
taken inside the change reports a correction that never reached the
screen.

It fails on this branch. Pure trims land at 18px; every page install
displaces the reader by about 1800px.

Also stops asserting `data-search-highlight` after waiting for the jumped
Turn to mount. That highlight clears itself 2.2s after the command lands,
so the assertion fails whenever loading the page around the Turn takes
longer than the flash - a 3s pass turning into an 18s timeout under load,
reproduced 2 of 6 runs. The jump's landing place is read from the reading
position instead, which does not expire.

Restores the band-check guard removed in the previous commit. A
controlled comparison over 6 runs each puts the flake at 2 of 6 without
it and 3 of 6 with it, so the ablation that removed it rested on a single
passing run and the guard is not what that flake was about.

Generated-by: Claude Code

* fix(desktop): scope transcript window invalidation to what each read assumed

The window has five writers — command answers, replica replacements, tail
broadcasts, band trims and automatic fills — but only command answers were
invalidatable, and the automatic fill borrowed the reader's own navigation
channel. Five reported defects fall out of those two gaps.

A read is answerable only while what it assumed still holds, and the two kinds
of read assume different things. An extension splices rows onto one edge, so
any replacement of that edge — navigating away, or the band trimming it out —
makes its answer unable to reach what is left; installing it would open a hole
the contiguous-window model cannot express and no edge cursor can name. A
replacement discards the edges, so only a newer navigation makes it stale, and
a replica replacement that answers nothing this window asked for is admitted
whole rather than misfiltered as a superseded command.

Filling an edge is not navigating to it: it gets its own channel, so it no
longer consumes the reader's outstanding jump, and it answers whether the
window moved so a read that changed nothing is not reissued in its own
callback, forever.

Generated-by: Claude Code

* feat(desktop)!: drop the transcript range boundary notice

The notice told the reader that the window they are reading does not hold the
whole transcript, and gave them a button to extend it. Neither is theirs to
care about: the window is a memory budget, the band already fills the edge a
reader approaches, and every arrival and departure of the notice is a height
change above or below them with no content behind it.

It arrived in #4560 without a design decision — the merge's own before/after
images do not show it — and #5147 has since had to exclude it from browser
scroll anchoring, because a row that is not content was becoming the anchor the
reader's position is measured from. Removing it retires that exclusion, the
row-projection module whose only job was placing it, its copy in three locales,
and the whole explicit history-load path it was the only entry point to: the
one navigation a reader still makes is returning to the tail.

Removing it also exposed a bug it had been hiding. Returning to the tail
cancelled an outstanding bookmark frame only because the notice's pending state
forced a render; without that render the queued frame scrolled the reader back
to the bookmark they had just left. An explicit pin now outranks a queued
restore, which is what the existing regression always claimed to check.

Generated-by: Claude Code

* chore: drop the retired transcript history pending state from the hook inventory

Removing the range boundary notice removed the `useState` that held its pending
target; the gate counts call sites, so the inventory has to say 11.

Generated-by: Claude Code

* chore(desktop): refresh the renderer architecture ledger

The removals in this branch move every recorded number down, except
`app-shell-effects.ts`, which grew with the reading-position work.

The retain callback's parameter was also named `window`, which shadows the
global and which the scanner reads as an environment capability the shell does
not use. Naming it for what it is keeps a phantom capability out of the ledger.

Generated-by: Claude Code

* refactor(desktop): let the subscription's lifetime say when a transcript apply is live

`applyTranscript` and `applyReadError` each took an `isDisposed` closure so a
stable event handler could re-check a flag its caller already knew. Both callers
know it locally: the store subscription is torn down in the effect's cleanup, so
anything it publishes is live by construction, and the two error paths either
sit behind the batch callback's own early return or check the flag where they
are.

Removing the indirection takes the file below the debt the ledger recorded for
it before this branch, so the architecture ratchet passes again.

Generated-by: Claude Code

* fix(desktop): keep a navigation answerable across a trim of the window under it

The window has two lifetimes, and the split stopped at the Renderer's
`accepts()`. Both leaks let a navigation the reader asked for be dropped.

A replacement admitted its reset batch under the navigation epoch and then
failed its own continuation fragments against the window epoch a trim had
minted meanwhile. The reset now takes the window's identity back to the epoch
the navigation was issued under — it discards the edges those trims were
protecting — so the rest of its fragments splice on. Minting moves to a counter
kept apart from that identity, so a number still names one window forever and a
page in flight under the trimmed one stays refusable.

Main ordered by a single epoch and read any newer number as "the Renderer left
that window", which an extension issued after a trim is not: it advanced the
consumer and silently discarded the outstanding replacement, even a one-batch
one. Only a replacing command now moves the consumer; an extension naming a
newer window is admitted without moving it, and a read stays current while its
epoch is at least the standing replacement's.

Also refreshes the astryx surface inventory, stale since chat-view stopped
reaching for HStack and Text with the range boundary notice.

Generated-by: Claude Code

* fix(desktop): release the delivery budget a stranded page answer holds

Letting an extension name a newer window without moving the consumer left the
queued answers of the window it left behind in place: they held their bytes in
the delivery budget until they were sent and refused, and the opening ramp
mints an epoch per trim. A consumer could reach its delivery capacity on a
transcript nothing was wrong with.

Queued answers are dropped again, now by what the window can still take rather
than by whichever number is newest: an extension splices onto the window it
named, so any newer one strands it; a replacement installs its own window, so
only a newer replacement does. The same question decides what the delivery loop
sends, so a page cannot be kept and then discarded unsent.

Sizes the scroll suite's settle wait against the opening ramp rather than the
10s default, which the suite documents as sized for UI already on screen. How
many pages the ramp reads is how tall the viewport happens to be against the
fixture; this machine finishes it in under two seconds and a loaded CI runner
measured past ten.

Generated-by: Claude Code

* fix(desktop): give the transcript window one lifecycle and two invariants

Three rounds of epoch patches had left the window with no statable rule about
when a read may be spliced onto it. Adversarial review found two ways to break
it, both of them mine, and both dissolve once the lifecycle is named.

The window is in one of two states. Settled: its rows are the window its
identity names, so an edge can be read from and its answer spliced back.
Navigating: a replacement has claimed the window but its rows have not arrived,
so the edges on screen belong to the window being left.

I1, the window is always contiguous. A read is answerable only while the edge
it was anchored on is still the window's edge. An extension may therefore only
be issued while settled, and carries the window's identity; a replacement
carries its own, and all of its fragments belong to it however many epochs the
band mints underneath. Installing a replacement names the window afresh, which
refuses every read that was anchored on an edge it threw away. Filling while
navigating anchored on the window being left and spliced onto the one that
replaced it — a hole no edge cursor can name and no later page can fill. Now
there is nothing to anchor on, so nothing is asked.

I2, a fill does not spin. A read that left an edge exactly where it found it
answers the same way again, so the range keeps where each edge stood and
refuses to re-ask until something moves it. This replaces the `moved` answer
`prefetchHistory` reported back to the scroll hook, which asked the geometry to
decide whether a read was worth issuing: it read the store's snapshot identity,
which a Session streaming into its tail changes on its own, so the guard was
blind exactly when it was needed. Deciding whether a read is worth issuing
belongs to the layer holding the window.

Main keeps a single number again. Which window the Renderer holds is not
derivable from the epochs Main sees and does not need to be — the Renderer
refuses what it cannot splice. The monotone `newestWindowEpoch` of the previous
commit claimed to derive it and got it wrong: once a navigation's answer landed
it stranded every later fill, leaving the older edge unable to load for the rest
of the Session.

Also drops the gap-row assertion the perf suite still made against a row deleted
with the range boundary notice, and the two dead gap measurements beside it.

Generated-by: Claude Code

* test(desktop): tell the displacement probe when the reader's hand is on the wheel

The probe skipped frames within 250ms of the last wheel event, on the theory
that a reading which catches a tick cannot tell the reader's own scrolling from
a page that moved them. The gesture and the rAF that reads it land in the same
frame in an order nothing here controls, so on a loaded runner the reading went
first and the guard was still holding the previous gesture's timestamp: eight
boundaries reported displacement of exactly 120px, one wheel tick, where this
machine reported none.

A time window cannot fix that, because the quantity it is inferring — whether
the reader is mid-gesture — is something the test already knows. It now says
so: the measurement window closes around each gesture and opens for the quiet
that follows, which is where a page lands and where a reader would see it jump.

Generated-by: Claude Code

* chore(desktop): keep hand-written e2e drivers out of the tree

Throwaway drivers for investigating a scenario by hand have to sit beside the
suite to resolve `@playwright/test`, and four of them reached a commit here,
where the ASF header audit correctly refused them.

Generated-by: Claude Code

* fix(desktop): drop the retired fill answer from the app-shell stories

Two stories still answered `onPrefetchHistory` with whether the window moved,
which the range controller now decides for itself.

Generated-by: Claude Code

* refactor(desktop)!: make the transcript window an anchored immutable value

The Renderer owned the window but kept Main's mutable structure without
the serial queue that had made it consistent. Four writers — jump, fill,
trim and tail broadcast — mutated it in place across processes and rAF
frames, and a monotone window epoch stood in for the transaction the
structure never had. Three holes followed from that shape, each found
independently by review and by Totoro-qaq's probes: a tail change lost
in Main's delivery loop left the window believing it still reached the
tail, so the next row spliced onto a gap; `#reset()` forgot to mint and
left `#navigating` set; and a jump snapshot carried the whole overlay
into a parked window, whose settlement then installed the durable row
far from anything it held.

Durable sequences advance by a stride, so adjacency cannot be read off
the numbers. The only proof that rows are contiguous is the Host read
that produced them, anchored on a known row or watermark. The window is
now an immutable value and every answer carries its anchor: a page names
the edge it was read from (`extends`), a tail change names the watermark
it read forward from (`coversFrom`), and a reset names the navigation it
answers. Rows land only where the anchor is still the window's edge;
watermark and overlay retirement land regardless, because they are facts
about the tail, not the window. Batches assemble off-window and install
once at `ready`, so the screen never shows half an answer and no
navigating phase or fill barrier is needed.

`coversFrom` has three states: a number or `null` (read from the start
of the transcript) is a contiguity claim; absent is none. Main's merge
drops rows and clears the claim when consecutive changes do not meet,
and the delivery loop no longer cuts an answer short mid-stream. A
dropped increment therefore degrades to a watermark the window cannot
join, which sets `hasNewer` and lets the band read forward from its own
edge — Main's navigation number is now only the cancellation hint the
old comment claimed it was.

The overlay leaves the window. It is shown only while the window reaches
the tail, which retires both entrances the escape hatch had: a jump
snapshot's overlay in a parked window, and a trim that opened the newer
edge without dropping it. The parked-window settlement test encoded that
hatch and is rewritten to the new contract; Totoro-qaq's probes are
adopted as regressions.

Fills report whether they issued a read, and the scroll hook re-checks
only when one did: the previous "cannot spin" claim was false, since a
refused fill resolved synchronously and re-entered the check.

Ablation considered and rejected: pull-only tail delivery (watermark
without rows) would flip `hasNewer` on every Turn completion and hide the
streaming overlay for a round trip each time. The push stays, verified.

Generated-by: Claude Code

* chore(desktop): refresh the renderer architecture ledger

`app-shell.tsx` gained one token: the prefetch fallback now resolves to
`false` so the scroll hook knows no read was issued.

Generated-by: Claude Code

* fix(desktop): move the tail watermark with every catch-up page

`#catchUp` installed rows page by page but only set `#durableThrough` at
the end, so a window opening between pages took a snapshot whose rows
reached further than the watermark it named. The next tail change was
anchored on the real watermark, which no longer matched the window's,
so the anchored window refused it and reported `hasNewer`. The main
shell reads forward and recovers; WorkHub offers no newer fill, so its
view stayed behind, the queued follow-up it was waiting to observe never
arrived, and a second follow-up was refused as a retry
(`workhub-layout.spec.ts`, "keeps the submitted prompt visible"). The
epoch model had masked this by splicing tail rows whenever the window
believed it was at the tail.

Generated-by: Claude Code

* fix(desktop): bring the app-shell stories onto the Renderer-owned window

The storybook smoke had never run against the new window model: every CI
run on this branch had failed earlier in the same job. Five stories were
still written against the contract the window refactor replaced.

Two encoded the old rule that the tail never asks for history on its own.
The band now fills two viewports above the reader when a Session opens,
so `TailFollowDoesNotAskForHistory` becomes
`TailPrefetchesHistoryUntilTheBandIsFull`, and
`AWheelTheScrollerCannotActOnAsksForHistory` goes: a transcript short
enough for the wheel to be the only signal no longer exists, the band has
already asked before the wheel arrives.

Two rail stories relied on the old rail aiming its own jump. The rail now
derives its tick from the scroll authority and a rail jump lands through
the scroll target that `createSessionOpenCommand` publishes, so the story
harnesses gain that half of the production seam. The rail itself learns
one rule the stories already asserted: pinned to the tail, the active tick
is the newest Turn, not whichever Turn crosses the top of the scrollport.

`PartialHistoryNotice` crashed React with `Invalid array length`: its fill
answered `true` for a window it had already read and resolved before
layout, so the band's chained re-check ran in the same microtask, saw the
same geometry, and asked again until React's update queue could not grow.
The harness now answers the way the Host's range controller does — `false`
for a window it has read, and a frame later otherwise.

Generated-by: Claude Code

* test(ui): scroll the rail reading-position probe as a reader

84b1f05 made the rail's active tick the newest Turn while the scroll
authority is pinned to the tail. This probe moved `scrollTop` with no
reader gesture, so the authority never released the pin and the rail,
correctly, stayed on the newest Turn. The probe now asserts that rule at
the tail, then wheels before it scrolls, which is what a reader leaving
the tail looks like to the authority.

Generated-by: Claude Code

* fix(ui): re-check the transcript band when the viewport resizes

The band is measured in screens of the scroller's own height, but its
check ran only on reader input and message changes. A resize with the
reader and the messages standing still redefines the band and reached
nothing: enlarging the viewport left a fillable edge unfilled and
shrinking it left a trimmable range retained until the next gesture.
The authority is not a substitute signal — it publishes only when its
snapshot changes, and a resize that keeps the pin and the reading Turn
changes nothing it publishes — so the band observes the root's size
itself.

Two regressions drive a resize alone: growing the scroller issues one
older fill, shrinking it retains the band around the reader.

Generated-by: Claude Code

* fix(desktop): mark the tail read only when the Renderer window reaches it

The window refactor deleted Main's window authority but left one of its
consumers behind. The read marker used to advance on `!change.hasNewer`,
a fact Main derived from the window it owned; with the window gone Main
advanced it whenever a consumer existed, so a reader parked in history
had every completing Turn marked read without having received a row of
it. Restoring the old condition is not possible — Main no longer knows
where the window ends — and guessing from delivery is unsafe: a change
whose `coversFrom` meets the last acknowledged watermark still fails to
join if a trim lands between the two.

The owner reports the fact instead. The Renderer window acknowledges
each new watermark it reaches with `hasNewer` false, once per value and
never while parked or serving a cached generation; Main records it per
consumer and marks read only when the acknowledged watermark has reached
the replica's own. Opening a Session at the tail acknowledges through the
same path, so opening still marks read; delivery and open no longer do so
on their own.

Regressions: a consumer parked off the tail sees the durable transcript
advance and the marker stays; acknowledging the current watermark moves
it. The controller acknowledges once per watermark and nothing while
`hasNewer`.

Generated-by: Claude Code

* fix(desktop): read the tail back before answering return-to-latest

`loadLatest` answered from Main's tail cache. Global memory reclaim can
trim that cache to nothing while the Session stays open, so returning
to the latest messages could install an empty window with `hasNewer`
false — a blank conversation, and no affordance left, until the band's
older prefetch refilled it. On `main` the follow-tail command read a
fresh page from the Host.

The tail is read back before it is replayed: when the cache holds fewer
Turns than it should and older rows exist, `loadTranscriptLatest` reads
the newest page from the Host into the cache, then answers through the
existing reset path with the watermark the cache already names. A cache
that covers the whole transcript is not short, so a small Session still
answers without a read. Reclaim keeps trimming; it is a performance
event again, not a correctness one.

Generated-by: Claude Code

* fix(desktop): keep the reader's place across a Host epoch change

Sequences name the same rows only within one Host epoch, so the
generation-change re-anchor stopped at an epoch change and a reader
parked in history was dropped at the tail when the Runtime Host
restarted. On `main` the registry rewrote the outstanding navigation by
Turn id. The Renderer now does the equivalent with what it already has:
after an epoch change it refreshes the Turn landmark index, resolves the
bookmarked Turn id to its new sequence and navigates there; a Turn the
new index does not know leaves the reader at the tail, quietly.

A read still in flight across that change was rejected by Main as a
stale epoch, and the rejection reached the conversation as a load error
for a condition the reset that follows resolves by itself. The
recovering controller now raises that rejection as a superseded read;
the reading-position layer treats it as neither an error nor an
unavailable bookmark, so the banner does not appear and the bookmark
survives for the re-anchor above. Every other error is still raised.

Generated-by: Claude Code

* fix(desktop): give the WorkHub transcript the pixel band

WorkHub mounts the same ChatView as the chat surface but was wired
without the band: no automatic fill, no trim, an explicit "older"
button instead. Two consequences of the Renderer-owned window followed.
Every page the button loaded was retained forever, where Main used to
bound every consumer's window. And a window behind the tail never
followed it — Main used to splice tail growth into whatever window it
held — so a queued follow-up's own row never arrived, the observation
check never satisfied, and the next follow-up was refused as a retry.

The WorkHub transcript port now carries `prefetchHistory` and `retain`,
delegated to the same range controller the chat surface uses, and the
ChatView gets the band's callbacks. The "older" button goes with it.
Queuing a follow-up returns the window to the tail before admission, on
every admission outcome, so the queued row lands and the guard clears.
A trim publishes the trimmed snapshot: it is the one window change with
no batch behind it.

Generated-by: Claude Code

* refactor(desktop): drop transcript window state nothing reads

Three mirrors of state that already lives somewhere else, each written
and never read back.

`PendingTranscriptPage.replaces` was queued alongside every page answer.
Its strongest case was that it names the same thing as
`#admitTranscriptNavigation(request, targetId, replaces)`, so it reads
like that concept persisted; but that argument is an input consumed at
the call site, and the delivery loop only ever looks at `navigation` and
`generation`.

`DesktopTranscriptReplicaSnapshot.navigation` was a Renderer concept on
a Main type the replica never sets: the only writers were the observer's
two `{ ...snapshot, navigation }` spreads. It looked load-bearing because
`encodeDesktopTranscriptSnapshot` needs a navigation, but what the
encoder needs is `TranscriptBatchIdentity`'s field, which a second
parameter supplies. The field also reached the local transcript cache,
which stored it permanently undefined.

The controller's `lastNavigation` was identical to the store's
`#navigations` by construction — same initial value, incremented only in
`navigate()`. The extension path needs "the navigation the window sits
on" once a pending answer has landed, but that is the store's own state;
it now answers for it through a one-line `navigation()` getter.

Generated-by: Claude Code

* refactor(desktop): publish WorkHub transcript snapshots from the store's commit

WorkHub delivered window changes over a second, hand-maintained path: a
`handler(store.snapshot())` inside the batch callback, plus a second one
inside `retain` guarded by a `trimmed` flag, with a comment explaining
that a trim is the one window change nothing else delivers. That comment
was the tell — the store already has a single channel that covers both
entry points, `#commit()` behind `subscribe()`, which is how app-shell
has always been wired. Two hosts of the same store were kept consistent
by hand instead of by construction.

`store.subscribe(() => handler(store.snapshot()))` puts the callback
shape the port wants on top of that one channel, so the trim special
case and its `cancellation.aborted` re-check go with it. The
subscription is torn down both on `close()` and on the cancellation
signal, which is what made the extra guard redundant. It is also
strictly safer: the old `|| batch.ready` branch could call
`store.snapshot()` before any reset had installed a range, where
`range()` throws.

The two `if (!store.accepts(batch)) return;` guards in front of
`store.accept(batch)` went too, and `accepts` is now private. `accept()`
calls it first and returns false without decoding anything, so the guard
only ran the same test twice. It did not even cover the one case where
the two differ — a reset for another Session, which `accept()` throws on
and `accepts` admits.

Generated-by: Claude Code

* refactor(desktop): let the read-marker gate stand alone

`consumer.acknowledgedThrough` mirrored, per consumer, a deduplication
the Renderer already does: the window reports a watermark once, and only
while it is actually at the tail. The case for keeping it was that the
Renderer is not trusted, so a repeated acknowledgement would re-write the
read marker.

It does not hold. The write is idempotent twice over — `#markTranscriptRead`
goes to `setSessionReadMarker`, and the Host's catalog coordinator
declines a marker equal to the one it already holds. The field was
per consumer, so two windows on one Session each acknowledged and each
triggered a marker anyway; it never bought "once per Session". And a
watermark that goes backwards was never its job: `request.through <
durableThrough` rejects that, and stays.

Generated-by: Claude Code

* refactor(desktop): name the transcript epoch-changed rejection once

Main threw `'Desktop transcript host epoch changed; reopen the
transcript'` and the Renderer recognised it with
`message.includes('Desktop transcript host epoch changed')` — the same
literal written twice, on either side of `ipcRenderer.invoke`, where
editing the prose on one side silently reclassifies the rejection on the
other.

The reason given for leaving it as prose is that invoke carries only a
message and no type channel. True, and already solved elsewhere in this
app: `SESSION_WORKSPACE_UNAVAILABLE_CODE` is an exported constant that
both the thrower and the matcher name, matched as a `CODE:` message
prefix. `DESKTOP_TRANSCRIPT_HOST_EPOCH_CHANGED_CODE` in
`preload/transcript-contract.ts` does the same for this one; both sides
already import that module.

`TranscriptReadSupersededError` stays. Making Main silently no-op
instead is not equivalent: `restoreSessionTranscriptRange` reads
"succeeded but the Turn is not in the window" as a dead bookmark and
clears the anchor, which would break re-anchoring across an epoch
change. Superseded has to remain a distinguishable outcome.

Generated-by: Claude Code

* refactor(ui): report reader scroll by phase only

`subscribeToReaderScroll` broadcast `(direction, phase)`. Before this
branch the direction had a real consumer: `use-chat-scroll` decided
whether to request history with `canLoad(direction) &&
nearEdge(direction)`. Moving that decision into the pixel band's own
geometry left both production subscribers ignoring the parameter — one
takes only `phase`, the other takes nothing — with the argument surviving
in the contract, in `reportReader`, and in a story.

The authority does still compute a direction, which is the case for
keeping it. But that direction serves the pin decision at scrollend; it
is not something a subscriber was using, and a subscriber that wanted
one could recompute it from geometry, as the band now does.

Generated-by: Claude Code

* test(desktop): give the teardown fakes the transcript window contract

The transcript window work added `acknowledgeTranscriptTail` and
`loadTranscriptLatest` to the source `openTranscript` requires, so the
teardown fakes landed on main with five of the seven methods. Against this
branch `requireTranscriptSource` rejects them, `openTranscript` throws before
the fake ever runs, and the `started` deferred these tests await never
resolves — the file hangs to the 900s timeout instead of failing.

Generated-by: Claude Code

* fix(desktop): release a transcript registration whose source is unavailable

openTranscript registers the consumer, then called requireTranscriptSource
outside the try. A source missing any transcript method therefore threw past
the cleanup every other failure takes: the registration stayed in the map and
its `ready` promise never settled, so the consumer id was permanently burned
and anything awaiting readiness hung. #restoreTranscript already made the
check inside its try; this makes the open path agree.

Regression: `releases a transcript registration whose source lacks the window
contract` attaches an observe-only source and asserts both that the open
rejects and that the registration is gone — reusing the consumer id reaches
the same failure instead of the duplicate-identity guard. Verified to fail
with the check moved back out.

Generated-by: Claude Code

* refactor(desktop): drop the navigation wrapper and the surface's pass-through props

A one-field interface around a sequence number bought nothing: the preload
unwrapped it on the IPC boundary and both the wire request and the batch
payload already declared `navigation` bare, so the wrapper existed only to
be built and taken apart again. Pass the number.

ChatMessageSurface re-declared and re-passed four ChatView props that its
own props type already inherits and `...chatViewRest` already forwards.
Removing the duplication keeps them required by naming them through
`Required<Pick<…>>` rather than relying on the surface's own optionality.

Generated-by: Claude Code

* fix(desktop): acknowledge the transcript tail only for a visible reader

The transcript range controller wired tail acknowledgement to every
consumer that reached the tail. readDelegatedTurnResult() opens one only
to compute a bounded WorkHub result preview and never renders it, so a
delegated Turn with later durable rows had its Session marked read —
returning the target preview while clearing unread state nobody saw.

Acknowledgement is now an explicit property of a reader that renders the
transcript: the plain controller stays silent unless asked, and the
recovering controller — which every visible reader uses and no projection
does — is the single place that asks. The projection cannot opt in by
accident.

Regression: `delegation feedback does not advance the target Session read
marker` drives delegationFeedback against a handle that records
acknowledgeTail, with a later Turn beyond the delegated one.

Generated-by: Claude Code

* chore: trim comments and test scaffolding the transcript window PR added

Comments that restate the code they sit on, pin a number nothing
re-verifies, or repeat a sentence the contract already carries do not
survive the next edit intact, and a stale one is worse than none. Keep
each statement where the contract lives and delete the copies.

The parked-completion probe asserted only what the range-store suite
already proves; its live overlay row now carries an assertion of its own —
verified to fail both when the coversFrom check is dropped and when the
overlay settle is.

Also: e2e assertions against selectors no product code emits, a local
`deferred` the shared test-only helper replaces, a private method that only
forwarded to a generic one, and a probe value threaded through ten sites to
feed an assertion that proved nothing.

Generated-by: Claude Code
@Astro-Han

Copy link
Copy Markdown
Contributor

#5170 has merged (89f3839) and covers what this PR patched at the old window authority: the boundary notice and its browser-anchoring exclusion are gone, and the rail's reading position is derived from the scroll authority. If nothing here is left uncovered, this can be closed — thanks for the groundwork, it shaped the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants