Skip to content

perf(chat): bound the per-pane history hydrate in the session grid - #3240

Merged
bolichen97 merged 1 commit into
kirodotdev:mainfrom
rnoack1:fix/bound-grid-pane-hydrate
Aug 22, 2026
Merged

perf(chat): bound the per-pane history hydrate in the session grid#3240
bolichen97 merged 1 commit into
kirodotdev:mainfrom
rnoack1:fix/bound-grid-pane-hydrate

Conversation

@rnoack1

@rnoack1 rnoack1 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

The session grid mounts one ChatPane per session, and each pane hydrates its own slot with its own request:

const { data: slotDetail } = useQuery({
  queryKey: ['slot-messages', slotKey],
  queryFn: () => api.chatSlotDetail(slotKey),   // no limit
  staleTime: Infinity,
})

GET /api/chat/slots/{slot} with no limit takes its documented "return ALL messages" path — it returns the whole conversation, reading the full chained history from disk when the slot has older sessions behind it (_disk_older_count > 0, chat_handlers.py:1067) and serving from memory when it does not (:1081). So opening a grid of N sessions issues N concurrent full-history fetches, parses N full histories, and pushes them all into the store. The cost scales with the number of panes on screen multiplied by how long those conversations are, and none of it is visible: a pane shows the tail.

Why it matters

The cost is paid on every grid mount and it is invisible, which is why it has gone unnoticed. A pane only ever shows the tail, so nothing on screen reflects that the full history was fetched, parsed, and pushed into the store — N times over for N panes. That makes the worst case the heaviest user: someone with a grid of long-running sessions, whose panes each carry the most history to re-read. There is no user-facing error to report, so the symptom surfaces as the grid simply being slow to become usable.

What changed (motivation → approach → change)

The handler already supports the bound, so this is a call-site change only. That keeps the change small, but it is also why the exemption below exists rather than a cleaner fix -- see "Honest limitations":

# chat_handlers.py, api_chat_slot_detail
start = max(0, end - limit)
messages = all_msgs[start:end]

With limit and no before, end is total, so the slice is the most recent limit messages — exactly what a pane renders. limit is clamped to 1..500 and a value below 1 is a 400, so 50 is in range.

The focused session is not affected. hydrateSlotMessages returns early for the active slot:

if (slot === state.activeSlot) return

The active session is fetched by ChatPage on its own and reaches the rest of its history through loadOlderMessages, which is unchanged here.

PANE_HYDRATE_LIMIT is also added to the React Query queryKey, so the cached entry is keyed to the bound it was fetched with. 'slot-messages' has exactly one usage in website/src/ and nothing calls invalidateQueries / setQueryData / getQueryData against it, so no exact-key consumer is affected.

Tests

New website/src/test/ChatPane.hydrateBound.test.tsx (7 tests) asserts the pane's hydrate call carries a numeric limit in 1..500, that the pane hydrates once so the bound is what caps a multi-pane grid, and that the earlier-messages row appears, is suppressed, and hands off exactly where it should.

  • New suite: 7 passed.
  • Negative controls, one per guard, each run against the whole file: reverting the call site to api.chatSlotDetail(slotKey) fails the bound tests with expected undefined to be type of 'number'; removing has_more from the row's condition fails the absence test; removing the slotKey !== activeSlot term fails the suppression test. All three fail for the intended reason.
  • Two of those guards were vacuous when first written, which is worth stating because the shapes look correct: a shared mock queued with mockResolvedValueOnce drifts payloads between tests, and await waitFor(() => expect(...).toHaveLength(1)) passes on its first poll while a two-pane count is transiently 1 and so can never observe it reaching 2. The suite now sets each mock per test and settles before asserting counts synchronously.
  • tsc -b: clean.
  • eslint on both changed files: 0 errors, and zero warning deltaChatPane.tsx reports 6 warnings before and after, SessionGridView.tsx 3 before and after, measured by linting each file's merge-base version and the changed version in turn.
  • catalogParity.test.ts: 77 passed. scripts/i18n-check.mjs: clean.
  • ChatPane.dirSend + ChatPanel + the new suite together: 15 passed.
  • chatSlice.warmSlotCacheBound.test.ts: 57 passed, including four added this revision for the two removal-class merges described below — two reproducing each finding, and two opposite-direction guards that fail if either fix is made broader than the defect it closes. Whole chatSlice scope: 736 passed.

Honest limitations

  • 50 is a judgement, not a measurement. It comfortably covers what a background pane renders, but it is not derived from a p99 of pane scroll depth. If a pane is later given its own scrollback, it will need paging rather than a larger constant.
  • Background pane scrollback really does shrink, and an earlier draft of this description got that wrong. It claimed "nothing in the pane UI currently reaches past that, so this is not a regression in reachable content". That is false: splitPaneMessages only partitions by role and never slices, so a background pane rendered the entire store array and scrolling up did reach the full history. A freshly mounted pane still ends at the cut, which is a real reduction in reachable content by scrolling — which is why the row is worded to name where the rest lives and what opening it costs. What no longer happens is the pane losing history it already held: a later round stopped the warm shrinking an array that holds more than it returned (below). Reaching past the cut still takes leaving the split, and the grid has no in-pane paging to offer instead.
  • A grid of RUNNING sessions still issues one unbounded fetch per pane -- the harm the title names, unfixed for that case. The bound has to exempt a streaming slot because the handler slices RAW rows (chat_handlers.py:1110) and only collapses chunk runs afterwards (:1114), so bounding a streaming slot returns the tail of its in-flight response instead of the last 50 messages. Exempting it client-side is a patch around that ordering, not a fix for it: the cause-level change is to collapse and then bound inside the handler, which would delete both the limitRef latch in ChatPane and the running branch in warmSlotCache. That is deferred rather than dismissed, and deliberately so -- next_before is documented as a raw-index cursor and loadOlderMessages (chatSlice.ts:1498) depends on that meaning, so changing the slice point is a paging-contract change and not a one-line move.
  • A pane could be left holding the bounded page when the slot record lost the race. This is now FIXED (chatSlice.ts, this revision): the hydrate accepts exactly one bounded-to-unbounded upgrade, so the pane commits immediately -- never deadlocking -- while a corrective unbounded fetch is still allowed through. The earlier call-site gate that deadlocked the hydrate is described, with its negative control, under "A page fetched before the slot record resolves".
  • Scoped to the pane hydrate. Scroll-position preservation when older messages are prepended is a separate concern and is not addressed here.

Marking the cut (review follow-up)

The pane hydrates a bounded tail, so its scroll container previously ended at message N−50 with nothing marking the cut — a user scrolling up in a long session reached a clean-looking beginning and could reasonably conclude the earlier messages were gone. The response already carries has_more (the limited path computes it as start > 0), so when it is true the pane renders one muted top row pointing at the full session, where the whole history is reachable.

Two details of that row came out of review, and both are cases where the obvious implementation is silently wrong.

The row hands the exit to its caller rather than performing it. Dispatching switchSlot and calling navigate('/chat') stays on the same /chat/:slug? route, so ChatPage never remounts and the local splitMode state that puts the grid on screen survives — the click would change the focused slot without ever leaving the grid. The row therefore calls a new onOpenFull?: (slot: string) => void prop, and SessionGridView wires it to the collapse-to-single-chat handler it already had. The test asserts the handover, not a route change.

The row is suppressed on the active slot, where it would be false. selectSlotMessages returns state.chat.messages for the active slot, so that pane renders the full history from the store while its own bounded query still reports has_more: true; hydrateSlotMessages also returns early for the active slot, so the pane's bounded fetch is discarded outright. Rendering the row there would tell a user that messages are missing from a pane that is already showing all of them.

The row reads "Earlier messages — open full session (closes split)". UX review noted that the earlier wording, "open session to view", did not say where it opens, and that the sibling string load_earlier loads in place — so a first-time reader could reasonably expect an in-pane load rather than losing the split. The label now names both the destination and the cost. The string components.chatPane.earlier_messages_open_session is added to en.manual.json, the regenerated en-XA.json, and all eleven translated catalogs. src/i18n/catalogParity.test.ts requires that: it fails any locale missing an English key and has no allowlist, so an English-only key is not an option here. The non-English values are machine-generated and worth a native-speaker glance; each uses its own catalog's existing term for a session (Sitzung, sessione, сессия, 会话, セッション).

Two clarifications on that in-place comparison, because both halves of it shifted while this change was in review. First, the sibling load_earlier string belongs to a different embedded app: it resolves to apps.mochi.chat.load_earlier in website/src/apps/mochi/src/renderer/ChatPanel.tsx, not to the dashboard chat. So when the comparison was first drawn it was across two apps, not an inconsistency within one surface. Second, that is no longer the whole picture. #2822 has since merged to main and adds in-place paging to the dashboard chat itself, through an EarlierMessagesBar driven by a handleTopReached callback when the transcript is scrolled to the top. That bar does not reach the cut this change creates, though: it renders only on the main single-pane transcript, it is absent from SessionGridView entirely, and its render condition requires the paging cursor to belong to the active slot. A background pane in split view therefore still has no in-place option, and the only affordance at its cut is the one that closes the split. The inconsistency is now inside the dashboard, between the single-pane view and the split panes, rather than between two apps. That is a sharper version of the same point, not a weaker one, and the cause-level fix is tracked in #4306.

Both hydrate paths, not one (review follow-up)

Design review found the bound covered only half the problem, and the half it missed also falsified the new row. warmSlotCache runs on every background chat_done and called fetchSlotDetail(key) with no limit — the same "return ALL messages from disk" path this PR exists to avoid — replacing the pane's store array with the complete history. The marker keys off a staleTime: Infinity bounded query, so once that landed a background pane rendered the whole transcript while still showing a row claiming earlier messages were missing. Exactly the false row this PR suppresses on the active slot, arriving through state drift instead.

fetchSlotDetail now takes an optional limit and warmSlotCache passes PANE_HYDRATE_LIMIT, which is exported from the store so the pane's query and the warm path cannot drift apart. That fixes both symptoms at their shared root: the store stops re-accumulating a full history per completing background turn, and has_more once again describes what the pane actually holds, so the row is true when it appears.

A later review round found a third path that fills the same array and wrote neither flag — see "A third hydrate path" below. The active-slot paths are deliberately left unbounded. switchSlot and refreshSlot both serve the focused session, which renders the full transcript and pages through it — bounding those would truncate it. A test pins all three: the background warm is bounded, an already-active slot does not fetch at all, and both active-slot thunks still fetch with no limit. Reverting just the bound fails only the first, with expected [ 'background-slot', 50 ].

A third hydrate path, and two UX gaps (review follow-up)

Design review found the invariant this design rests on — the flag is "written with the cache it describes" — still had a hole, and UX found two ways the row's promise broke. All three are fixed here.

The flag had two writers and a third path that wrote neither. switchSlot.pending caches the outgoing active slot's full array into slotMessages[key] and never touched slotPaneHasMore, so re-entering split could render a complete transcript under a row claiming earlier messages were missing — the same false row this PR exists to remove, arriving by a third route. Two changes close the class rather than the instance. hydrateSlotMessages now carries hasMore and writes the flag beside the array it describes, which retires the warmHasMore ?? slotDetail?.has_more dual source and leaves the store as the only owner. And the switch-away cache writes the active slot's own slotHasMore rather than a constant — the active view pages its own older history, so hardcoding false there would have hidden a row a pane legitimately needs.

A bounded warm deleted scrollback under a reader. warmSlotCache replaced a background pane's array with the 50-message tail on every chat_done, so a reader scrolled up in a pane watched that history vanish the moment its slot finished a turn. The warm now keeps whatever older head the pane already holds, matched on the warm's oldest row IDENTITY (meta.mid), and leaves the flag untouched when it does: an array that is no longer the bounded warm is not described by the warm's has_more. Identity matters rather than ts because two rows can legitimately share a timestamp -- a coarse clock stamping one tick, or a channel replay -- so a ts-keyed cut matches the EARLIER row and slices a distinct message out of the middle while the result still looks like a retained head. A bounded hydrate can split a response that is still streaming. The server's limit slices RAW rows, and a streaming reply exists as many chunk rows that only collapse into one message afterwards (_prepare_messages) -- so bounding a running slot could keep just the tail of its in-flight response and drop the messages before it. Both bounded callers now take the unbounded path while the slot is streaming: the pane captures that at mount so it cannot refetch mid-stream, and warmSlotCache reads the slot's own run state. Chunk rows are in-memory only, so a completed response is a single message on disk and stays safely bounded.

A row with no mid has no identity, so the merge declines to cut -- and when the pane's own array is LONGER than the warm it keeps that array rather than replacing it. Replacing was the first shape and it was wrong: legacy rows predate meta.mid entirely, so a pane that had already paged in older history lost it on the next chat_done. Keeping the longer side means nothing is dropped in either direction, and the marker is left alone because the result is then not the bounded warm. All four writers of slotMessages now go through one writeSlotPage(state, key, messages, hasMore) so the array and its marker cannot disagree -- switchSlot.fulfilled was writing the array and no marker at all. Whichever side that branch keeps, the pane's rows are first routed through the same hydrateQueuedBubbles path the warm itself uses, so a branch that keeps prior rows cannot also keep a queued bubble the server has since started running.

The row promised earlier messages and delivered the newest one. Earlier messages — open full session handed the slot over, and the full session opens pinned to the bottom — so a reader hunting a turn 60 messages back paid the split teardown and then paged from the newest message anyway. Every click hit this. The handoff now carries the pane's oldest held ts and reuses the existing pinned-jump machinery, which pages older history until the anchor loads and scrolls to it. switchSlot.pending sets activeSlot synchronously, so the jump effect sees the right slot on the next render.

Four new tests plus one updated, each negative-controlled against the unfixed code: reverting the switch-away write gives expected true to be false, reverting the flag write gives expected undefined to be true, reverting the warm merge gives expected [ 'warm oldest' ] to include 'older head', and the handoff test pins the anchor as the pane's oldest ts rather than absent. 973 tests across 63 files pass, tsc clean, eslint 0 errors.

A running background slot, and a full cache (review follow-up)

A background slot that was already running still got the bounded fetch. The limit slices RAW rows and a chunk run only collapses after, so bounding a streaming slot leaves its in-flight response as the tail alone. The first guard read the pane's own stream state, which reports idle for a background slot until an SSE frame arrives -- so an already-running slot fetched bounded and the pane showed only the tail of its response. The slot record is the signal that is true on arrival, so the limit now reads paneSlot.running as well, and latches once the record resolves so a later turn cannot re-fetch the whole transcript mid-conversation. It starts bounded and can only upgrade to unbounded once, which means the query always fires: a gate that waited for the record would deadlock a pane whose slot list is empty. A later round tried to add exactly such a gate and CI proved that sentence correct -- see "A page fetched before the slot record resolves" below.

A bounded page prepended onto an already-loaded transcript reordered it. hydrateSlotMessages fills the same array a full page write can already have filled -- switch away from a fully loaded slot, then open it as a split pane, and the bounded tail was prepended to the front of the complete history. That both duplicated rows and put the newest messages above the oldest, and it carried the bounded query's has_more onto an array that was never the bounded page. Only a page write records a marker, so the marker's presence is the proof that the array is a loaded transcript, and the hydrate now declines outright in that case. Where the array holds live frames and no marker, the prepend still runs and passes undefined so the existing marker is left alone.

Screenshot evidence

Captured from the real built SPA by website/scripts/capture-pane-earlier-marker.mjs, following the pattern the other capture scripts in that folder use: website/dist behind scripts/lib/serve-dist.mjs, every /api/** answered from fixtures via scripts/lib/stub-dashboard-api.mjs, Playwright at deviceScaleFactor: 2. Split view is reached the way a user reaches it — a two-session layout persisted under mc-split-layouts and anchored at a session leaf, which auto-enters split on load.

One frame carries the row and its suppression. Both sessions in the layout are longer than the hydrate bound, so the server answers has_more: true for both. The upper pane is the active slot, so the row is correctly absent even though its own query reported more history; the lower background pane earns the row and shows it. A frame proving only the presence of the row would not distinguish the fix from a row that renders unconditionally.

The harness asserts a bounded limit was actually sent, that exactly one marker exists across both panes, and that the marker is inside the viewport — Playwright's visible is a layout predicate, not an in-viewport one, so without that last check the capture passed while the marker sat above the fold. Getting the marker into frame needs scrollIntoViewIfNeeded on the element itself: an earlier version picked the scroll container by looking for one containing a button, which matches the first pane because message rows carry their own buttons, so every assertion passed while the captured pixels omitted the row.

Split view: the background pane marks earlier messages, the active pane does not

The marker itself:

The muted earlier-messages row

A page fetched before the slot record resolves (review follow-up)

Design review asked whether "starts bounded, upgrades to unbounded once" is safe when the bounded fetch lands before the slot record does. The finding is real. It is now fixed in the reducer -- see "The one-shot is now upgradeable" below. A first attempt fixed it at the call site instead, was measured to be worse than the problem, and was reverted. This section keeps that measurement, because it is the reason the fix ended up where it did.

The gap. When the pane has no slot record yet, neither latch branch runs: the slot is not known to be running, and paneSlot !== undefined is false. So the bounded request goes out. The store hydrate is one-shot -- hydrateSlotMessages returns early on an already-hydrated slot, and that flag is cleared only when the session is deleted -- so whichever page arrives first is the only one that pane ever accepts, and a later corrective fetch is discarded silently. The record can lose that race: it arrives over a WebSocket frame or a separate HTTP call, while the pane's history fetch is an independent request, and the grid mounts panes from the persisted layout without waiting on the slot list.

Why the obvious fix is wrong, measured. The attempted fix withheld the commit until the latch settled (settling it on the slot list having loaded, as proof no record was coming). That deadlocks the hydrate whenever the slot list never resolves. website/integration/helpers.tsx seeds no dashboard.slots and no slotsLoaded, and the integration store receives them only over a transport the harness never provides -- so the gate never released, the queued cards never committed, and ChatPaneQueueReorder.integration.test.tsx failed on a waitFor for the queue stack's expand control. A negative control isolated it on an unchanged tree: with the gate the test fails, with the gate removed it passes. That is not a harness artifact to paper over -- it is the same condition a real client hits whenever the slots list never arrives, and the pane would render empty indefinitely.

Where the fix belongs. Not at the call site. The pane has no local signal that distinguishes "the record is still coming" from "no record will ever come", so any pane-side gate must either deadlock or commit a guess. The cause-level change is in the reducer: let hydrateSlotMessages accept exactly one bounded-to-unbounded upgrade and replace the page, so the pane can commit immediately (never deadlocking) while a corrective unbounded fetch is still allowed through. That touches the identity-merge branches review has already flagged as a watch item, so it wants its own change with its own tests rather than riding along here.

Net effect on this PR: the interleaving no longer strands a pane. What remains true from this section is the diagnosis -- a pane-side gate cannot work -- which is why the fix is in the reducer.

A turn that starts mid-hydrate (review follow-up)

GPT 5.6 Review blocked this revision on ChatPane.tsx:152: an idle pane mounts, the user
sends before the bounded fetch is served, and the pane commits the tail of the in-flight
response. Verified at source and fixed here.

The latch previously settled as soon as the slot record resolved, including the idle case, so
a turn starting afterwards could not upgrade the limit. The fetch had already been issued with
a limit, the server slices RAW rows before collapsing chunk runs, and by serve time the slot
was streaming -- so the page was the tail of that response rather than the last N messages. The
latch now settles only when it goes unbounded, so an idle pane stays upgradable: a turn flips
the query key and supersedes the in-flight bounded request. Nothing is withheld, so this cannot
reintroduce the hydrate deadlock an earlier attempt at a commit-gate caused.

Correction to an earlier claim in this description. A previous revision argued the marker
would still appear, because no code path writes has_more = false. That was too strong. Once a
live frame had seeded the pane array, the hydrate passed undefined and
writeSlotPage returned early (chatSlice.ts:1076), so slotPaneHasMore was left UNSET -- and the
row reads slotPaneHasMore?.[slotKey] (ChatPane.tsx:89), where unset is falsy. No marker
renders. Unset is user-visibly identical to false, so the reviewer's "suppresses hasMore" was
right and the earlier rebuttal was wrong.

Cost of the change: a pane whose slot starts a turn after the bounded page already landed now
issues one additional unbounded fetch, whose result the one-shot hydrate discards. That is one
fetch per pane, on turn start, and it cannot corrupt the store -- not the N-concurrent-at-mount
cost this PR exists to remove.

Test: an idle-mounted pane with a never-resolving detail fetch, then the slot flips to running;
the assertion is that a call with no limit follows. Negative control: restoring the idle-latch
fails it with expected false to be true. Integration suites 304 passed, unit 76, tsc and
eslint clean.

The one-shot is now upgradeable, and a pruned slot leaves no marker (review follow-up)

GPT 5.6 Review blocked this revision with two findings. Both were verified at source before any code changed, and both are fixed here.

1. A bounded page could defeat its own unbounded upgrade. An idle pane fetches a bounded page. If the slot starts a turn before that page lands, the previous revision issued the corrective unbounded fetch but the store threw the result away: hydrateSlotMessages returned early for any already-hydrated slot, so whichever page arrived first was the only one the pane ever accepted. The reviewer's own description of the mechanism was accurate. The reducer now allows exactly one upgrade (chatSlice.ts:2291): a bounded page may be superseded by an unbounded one, the reverse is refused, and the record that permits it is deleted on use so a slot cannot upgrade twice. The replacement keeps the rows the bounded page never fetched -- the bounded write is [page, ...priorRows], so everything past its recorded length is a live tail (chatSlice.ts:2295, length recorded at :2307). The pane tells the two apart by passing the limit it used (ChatPane.tsx:166). Nothing is withheld, so this cannot reintroduce the hydrate deadlock the earlier call-site gate caused.

2. A stale marker suppressed hydration after slot reuse. This one is worse than truncation -- it renders an EMPTY pane. When a slot disappears from a slots push, the sseSlots prune deletes its transcript and its hydrate flag but did not delete its "has older history" marker. If a slot with the same key was then recreated, hydration passed the flag check and hit the marker check instead, returning without writing anything at all. The prune now clears the marker and its bounded-length peer alongside the transcript (chatSlice.ts:3370), which is the smaller of the two fixes the reviewer offered. Note the marker is only ever written for a page whose has_more is known (chatSlice.ts:1076) and is read as slotPaneHasMore?.[slotKey] (ChatPane.tsx:89), where a surviving true is indistinguishable from a real one.

Tests, each negative-controlled so it fails for its own reason. Three new reducer-level tests. The upgrade test asserts the full history replaces the bounded page and the live tail survives; reverting the upgrade branch fails it with expected [ 'b-1', 'b-2', 'live-1' ] to deeply equal [ 'a-0', 'b-1', 'b-2', 'live-1' ], and with the preceding assertion removed the marker assertion fails independently with expected true to be false. The once-only test is controlled by dropping the delete, which lets a second page re-upgrade and fails it with expected [ 'x-9', 'b-1' ] to deeply equal [ 'a-0', 'b-1' ]. The slot-reuse test is controlled by reverting the prune, which fails first on the surviving marker and then, with that assertion removed, on the recreated slot's transcript being undefined -- the empty pane, reproduced.

Verification. Full unit suite 1312 files / 20824 tests pass (2 expected-fail, 3 skipped); website/integration/ 30 files / 304 tests pass, which is the no-seed harness that caught the earlier call-site gate; tsc --noEmit 0 errors; eslint 0 errors.

The bounded-length record belongs to the writer, not to its callers (review follow-up)

GPT 5.6 Review blocked the previous revision on the record added for the upgrade above: a bounded hydrate records how many leading rows came from the bounded page, and switchSlot.fulfilled could then replace the array while leaving that record in place, so a later unbounded hydrate sliced at a stale offset and re-appended rows the new page already carried. Verified at source, and it is real -- but narrower as stated than it actually is.

Three writers, not one. writeSlotPage is the sole writer of the pane array, and it has five call sites. Three of them replace the array wholesale: switchSlot.pending writing the active transcript back to the cache when the user switches away (chatSlice.ts:3407), switchSlot.fulfilled (:3528, the one the reviewer named), and the warmSlotCache merge (:3520, which can write an array of a different length again). So the reviewer's suggested fix -- delete the record in switchSlot.fulfilled -- closes one of three doors, and the remaining two fail the same way. The most reachable one is in fact switchSlot.pending, because switching away is exactly when the slot becomes a background pane again.

So the fix is in the writer. The record is an INDEX INTO the array being written, which makes its lifetime a property of the write rather than of any caller. writeSlotPage now takes the bounded length as a parameter and either sets or clears the record on every call (chatSlice.ts:1074), decided on that call's own argument and never on what the key already holds. The three replacing writers pass nothing and therefore clear it; the bounded hydrate passes a length. Nothing outside the writer touches the record any more, so a fourth writer added later cannot leave it stale by omission -- it has to opt in. This is the consolidation the Design lane asked for, applied one dimension further than the marker.

One placement detail that is load-bearing. The record is handled BEFORE writeSlotPage's hasMore === undefined early return (chatSlice.ts:1076). Two real callers pass no marker -- the warm merge when it keeps prior rows, and the hydrate when a live frame already seeded the array -- so a clear placed after that return would silently skip exactly those writes. A test pins this directly.

Tests, each negative-controlled for its own reason. Two new reducer tests. The duplicate-append test drives the real switchSlot.pending reducer over a bounded-hydrated pane and then upgrades; reverting the writer to set-only (never clearing, i.e. the previous revision's behaviour) fails it with expected 3 to be 4, and with that assertion removed the next one shows the duplicate directly: expected [ 'a-0', 'b-1', 'b-2', 'b-2' ] to deeply equal [ 'a-0', 'b-1', 'b-2' ]. That same control also fails the once-only upgrade test, because the clear now does the job the explicit delete used to. The placement test is controlled by moving the record block after the early return, which fails it with expected undefined to be 1. The mid handoff is controlled by dropping the mid argument, which fails the handoff assertion.

Verification. Full unit suite 1312 files / 20826 tests pass (2 expected-fail, 3 skipped); website/integration/ 30 files / 304 tests pass; tsc --noEmit 0 errors; eslint 0 errors with the warning count unchanged; comment lint 0 errors.

Two copies of one row have to be recognised as one row (review follow-up)

GPT 5.6 Review blocked the previous revision on the reconciliation itself, in two separate paths. Both verified reachable at source, and both fixed here. They share a cause: the merges were POSITIONAL (slice at a recorded offset, or keep the longer array) while the problem is one of IDENTITY, and a position cannot tell whether two arrays hold the same row twice.

The identity is populated, which is what makes a dedupe real rather than a no-op. This was the first thing checked, because a dedupe keyed on a field that is absent in the failing window would read as a fix and do nothing. A row the user just sent carries only the one-shot meta.sendId its send generated (ChatPane.tsx:283) until the server echo lands, at which point the echo's meta.mid is merged in and the sendId is stripped from the local copy (chatSlice.ts:125). The server stores the client meta opaquely and then stamps its own id (chat_handlers' slot store, state.py:1755), so the SERVER's copy of that row carries BOTH. So there is a usable key in each window: sendId before the echo, mid after it.

That asymmetry is a trap, and a test caught it rather than reasoning. The first implementation returned one preferred identity per row -- mid when present, else sendId. Under that rule the local pre-echo row is known as send:… while the server copy is known as mid:…, so the two copies of ONE row are compared on keys that cannot agree and the dedupe silently does nothing. The test failed with expected [ 'a-0', 'b-1', 's-1', 's-1' ] to deeply equal [ 'a-0', 'b-1', 's-1' ]. The helper now returns EVERY identity a row carries (chatSlice.ts:1022) and a row counts as present if any of them matches.

Path 1 -- the unbounded upgrade duplicated a just-sent row. The upgrade kept the live tail past the recorded bounded length, but the wider page is a fresh server snapshot and a sent row is persisted before its send is acked, so the page could already carry the row the tail held. The tail is now filtered against the page by identity (chatSlice.ts:2295).

Path 2 -- a warm snapshot dropped a row sent while it was in flight. The warm merge replaced the array with the server page, and a row sent after that fetch went out sits after the page's newest row, so it was lost. Rows strictly after the warm's newest row are now preserved, and only when that row is locatable in the prior array -- decline rather than guess, the same rule the merge's existing cut already follows (chatSlice.ts:3630). When such rows are appended the array is no longer exactly the server page, so the write no longer claims the page's has_more.

A row with NO identity is kept, deliberately. Dropping a local row on the strength of a guess is the failure this exists to prevent, so an unidentified row survives even at the cost of a possible duplicate. A control that inverts this -- dropping unidentified rows -- fails with expected [ 'a-0', 'b-1' ] to deeply equal [ 'a-0', 'b-1', 'legacy' ].

Also fixed, from the Design lane: the record now dies on an explicit delete too. deleteSlot.fulfilled cleared the marker but not the bounded-length record (chatSlice.ts:3707), so the stated invariant had two lifecycles and only the sseSlots prune applied it -- a slot-key reuse after an explicit delete re-entered the stale-index class the previous revision closed. One line, and the lane was right to call it out.

Controls, one per behaviour. Removing the upgrade dedupe fails BOTH windows -- [ 'a-0', 'b-1', 's-1', 's-1' ] pre-echo and [ 'a-0', 'b-1', 'sent', 'sent' ] post-echo. Removing the warm preservation fails with expected [ 'm-1', 'm-2' ] to deeply equal [ 'm-1', 'm-2', 's-2' ]. Dropping unidentified rows fails as above. Reverting the delete clear fails with expected 1 to be undefined.

Verification. Full unit suite 1312 files / 20831 tests pass (2 expected-fail, 3 skipped); website/integration/ 30 files / 304 tests pass; tsc --noEmit 0 errors; eslint 0 errors with the warning count unchanged; comment lint 0 errors.

Advisory lanes on this revision (answered; one now coded)

Newly raised this round, answered here:

  • Design -- deleteSlot.fulfilled did not clear the bounded-length record. FIXED (see the
    section above). The lane was right that the invariant had two lifecycles with only one applying
    it.
  • Design -- the warm merge's "keep the longer array" fallback prefers stale rows when the server
    legitimately SHRINKS a history.
    Real, and DEFERRED with a reason rather than patched. The
    payload does carry total, so a total-aware guard looks like a one-expression change -- but it
    is not sound as written: prior legitimately holds rows the server total does not count, namely
    an optimistic row whose send has not been acked and hydrated queued bubbles. So
    prior.length <= total is false in ordinary operation, and adding it would flip the fallback off
    and delete loaded scrollback in the COMMON case to fix an uncommon one. A correct version has to
    count only server-identified rows against total, which is a design step, not a guard. Recorded
    here so the next attempt starts from that, rather than from the version that looks cheap.
  • UX -- the only affordance at the cut tears down the split arrangement, and the return path is an
    unmentioned badge.
    Correct. Both fixes are out of reach of this revision: appending reassurance
    to the string is a new key in all 13 catalogues under the i18n parity test, and paging older
    history inline is the same handler-side change (collapse-then-bound) already scoped as the
    cause-level follow-up -- inline paging on a bounded pane is exactly what the raw-row slice
    prevents. Deferred, not dismissed.
  • UX -- absence of the row is ambiguous: a pane holding everything looks the same as one that is
    cut.
    Correct, and the asymmetry is deliberate rather than accidental -- the active pane renders
    the full history, so a recovery row there would fetch nothing, which is why it is suppressed and
    pinned by a test. But the lane is right that a reader who has learned the row exists will read its
    absence as "nothing earlier". The suggested terminus ("Beginning of session" on panes that truly
    hold everything) is the correct shape and is deferred: it is another new catalogue key, and it
    wants the same treatment as the two strings already deferred so they land together rather than one
    at a time.
  • UX -- the row does not look clickable (muted 12px caption, accent only on hover). Accepted as
    a real affordance gap. Correcting an earlier reason given here: this was grouped with the
    deferred i18n work, which was wrong -- link styling needs no new catalogue key at all, it is a
    one-line class change on the row's className (ChatPane.tsx:423), so the i18n parity gate is
    not what holds it. It is held only because the branch currently conflicts with main, so no
    check can run against a new revision until that is resolved; it rides the same revision as that
    resolution rather than spending a separate one on a presentation tweak nothing can validate. A
    chevron icon is a larger change (an import plus markup) and is not what is proposed here.
  • UX -- when the handoff callback is absent but there IS older history, the row is hidden and the
    transcript top poses as the conversation start.
    Correct and worth fixing; deferred for the same
    reason, since inert replacement text is another new catalogue key. Note this configuration does
    not arise on the surface this PR adds -- the grid always supplies the callback -- so the exposure
    is to other embedders rather than to the panes shipped here.

UX, Design and First Principles are all CONCERNS and none blocks merge. Explicit decision: none
of them is folded into this amend.
The amend carries two correctness fixes to the hydrate path;
the remaining UX items are a new i18n key across every catalogue under the parity test and a
change to the handoff payload, and neither belongs in the same commit as a store-correctness fix.
Each is recorded below as a tracked follow-up rather than waved through.

  • The anchor jump could resolve against the bounded page. VERIFIED AT SOURCE AND FIXED in this
    revision
    -- this supersedes an earlier statement in this description that it "stands
    unrefuted", which was an admission of not having checked rather than a defence. The chain:
    switchSlot.pending restores the pane cache into the active view (chatSlice.ts:3430), so for a
    background pane the active transcript IS the bounded page; the jump effect resolves the anchor
    against it and, on success, clears the pending jump (ChatPage.tsx:5429); switchSlot.fulfilled
    then replaces the transcript, and the effect cannot re-apply because it returns early on a null
    pending jump. So the anchor was resolved against the wrong array and never re-resolved.
    The reviewer's suggested mitigation -- wait for slotLoading to clear -- would have been dead
    code
    : switchSlot.pending sets slotLoading to FALSE whenever a cache exists, which is
    exactly the failing case. The signal that does discriminate is the bounded-length record, which
    is present only while the view is that provisional page and is cleared by
    switchSlot.fulfilled's own write (chatSlice.ts:3528). The jump now still fires immediately
    for feedback but keeps the pending jump alive while the view is provisional, so it re-resolves
    once the full transcript lands. It withholds nothing and cannot hang: if the fetch never
    completes, the user keeps the early jump, which is today's behaviour.
    Coverage, stated honestly: the new test pins the RECORD's lifecycle -- present after
    switchSlot.pending restores a bounded cache with slotLoading false, absent after
    switchSlot.fulfilled -- and its control (making fulfilled retain the record) fails with
    expected 2 to be undefined. The effect's consumption of that signal is verified by reading the
    code, NOT by a rendered test: no test in this repo currently exercises the pinned-jump effect at
    all, so there was no harness to extend and building one for this page was out of scope for a
    single amend. That gap is real and named rather than implied away.
  • UX -- the failure path speaks pin vocabulary. A handoff that mentioned no pin can surface
    "This pinned message is no longer available". Verified correct. The fix is a dedicated
    earlier_messages_unavailable string, which is a new key in all 13 catalogues under the i18n parity test. NO LONGER DEFERRED -- it is implemented; see "The earlier-messages row no longer speaks pin vocabulary" below for the reachability chain, the fix, and its negative controls. An earlier revision of this description deferred it on the reasoning that a 13-file mechanical i18n change riding in the same commit as a store-correctness fix risks one missed catalogue failing the parity gate and costing the whole revision. That risk was tested rather than assumed: every i18n gate passes, including diff-scoped against the base.
  • UX -- pass meta.mid alongside ts in the handoff. DONE in this revision. The consumer
    already preferred a mid and fell back to ts (ChatPage.tsx:5390), and the pending-jump state
    already carried an optional mid -- this one producer was the only path dropping it. So the fix is
    additive plumbing, not new behaviour: the row now hands over messages[0].meta.mid beside the ts,
    and the handoff test asserts both. Folded in because it is the same identity argument the merge
    logic in this PR already makes -- two rows can share a ts, so resolving a jump by ts alone can
    land on the wrong row.
  • Design / First Principles -- the latch and both streaming exemptions are epicycles around the
    handler slicing raw rows before collapse.
    Agreed, and this is a scope judgement rather than a
    defect, so it is answered here rather than actioned. The reducer one-shot upgrade they named is
    now done (above). The other one is not: the cause-level change is to collapse chunk runs and
    then bound inside the handler, which would delete the limitRef latch and the running branch
    in warmSlotCache outright. It is deferred because next_before is documented as a raw-index
    cursor and loadOlderMessages (chatSlice.ts:1498) depends on that meaning, so moving the
    slice point changes the paging contract and needs its own change with its own tests. The
    mount-path bound ships now because it removes the cost this PR exists to remove -- N concurrent
    full-history fetches when a grid of idle panes mounts -- and does so without touching that
    contract. First Principles' proposal to subtract the bounded warm path instead would give up
    that saving, which is why it is recorded rather than taken. Both lanes ask for that follow-up to be
    tracked "with teeth" rather than deferred in prose, and they are right that a paragraph in a
    description is not a tracker. This PR cannot create that tracker on its own -- filing the issue is
    the maintainers' call, not something to slip in from a fork branch -- so it is named here
    explicitly for them to pick up: collapse-then-bound in the handler, which deletes the limitRef
    latch, the warmSlotCache running branch, and the paging-contract change to next_before that
    makes it non-trivial.
    First Principles also proposes a genuinely smaller alternative worth recording -- leave
    warmSlotCache unbounded and have it write the marker false, which would delete the
    olderHead/keptPrior merge branches and the ts-collision decline logic along with their tests.

Rapid switching corrupted the bounded pane's metadata (this revision)

GPT 5.6 Review blocked on chatSlice.ts:3407, the switchSlot.pending write that caches the
active transcript before switching away. Verified at source and FIXED. It is a real corruption, and
the reason is an ordering nobody sees from the call site.

setPagingCursor is the ONLY writer of state.slotHasMore (chatSlice.ts:995), and it refuses to
write while a switch to the active slot is still in flight. So between switchSlot.pending and
switchSlot.fulfilled, slotHasMore still describes the chat the user LEFT. In that window
switchSlot.pending restored the pane's cached page into the active view (chatSlice.ts:3430), so
for a bounded pane the visible transcript IS its 50-row page. Switch away again before the fetch
lands and the write fired with the previous chat's slotHasMore, and with no bounded length -- which
writeSlotPage treats as "clear the record" (chatSlice.ts:1074). The pane kept 50 rows while
losing both its bounded record and its marker, so a truncated transcript then posed as a complete
one. switchSlot.fulfilled cannot repair it because it returns early once the user has moved on.

The fix preserves both when, and only when, the pane's own switch has not landed
(chatSlice.ts:3392, :3407). The condition is not "does a bounded record exist" -- that reads true
for a stale record left on a slot whose view is already the full transcript, and preserving there
would keep a length that no longer describes the array. It is the same in-flight test
setPagingCursor already uses, so the two now agree on when slotHasMore may be trusted.

Controls, one per half. Reverting to the unconditional write fails the new test with
expected false to be true -- the stale false overwriting a live marker. Keeping that half but
passing no bounded length fails with expected undefined to be 2 -- the record deletion, a distinct
symptom. Both pre-existing switch-away tests still pass under either revert, which is what shows the
change is scoped to the in-flight window and leaves settled switches alone.

The UX lane's "cut can render with no marker at all" is the same boundary, and is also FIXED --
but it needed the opposite edit, so it is two changes, not one.
At chatSlice.ts:2307 the hydrate
passed hasMore as undefined whenever a live frame had already seeded the pane array, and
undefined means "leave the marker alone", so the marker was never recorded at all. Seeded frames
are NEWER rows appended after the page, so the page's own has_more still correctly describes what
precedes it; it is now passed through. At :3407 the correct value is undefined (preserve what the
pane had); at :2307 the correct value is the real has_more (record it). One helper, one field,
opposite directions -- a single shared edit would have fixed one and re-broken the other.

A pre-existing test asserted the marker stayed unset on that path, which pinned the defect rather
than an invariant. Its real invariant -- that the bounded length is handled BEFORE the
hasMore === undefined early return -- is preserved, now reached the way it is still reachable, by a
caller that omits has_more entirely.

Verified on this tree: full unit suite 1345 files / 21177 passing, website/integration/ 30 files /
304 passing, tsc 0 errors, eslint 0 errors, comment lint 0 errors.

Two lanes disagreed about the missing marker; the source settles it (UX accepted)

UX called the absent earlier-history marker a defect. Opus examined the same path and judged it
DELIBERATE, pinned by the test "still prepends history in front of live frames that arrived first",
reasoning that a live frame implies the slot is running, so the pane's latch re-fetches unbounded and
the marker arrives anyway. That second claim is the substantive one, and it is WRONG. The fix stands.

The pane un-latches to an unbounded fetch only when running || paneSlot?.running
(ChatPane.tsx:155), and running is streamState !== 'idle' (:81). So the question is whether a
frame that seeds the pane array also moves the slot's run state. It does not always:
applyNonActiveFrame sets run.state = 'streaming' in the chunk branch only
(chatSlice.ts:793), and two earlier branches return before reaching it. The stop_event branch
PUSHES a row into the pane array and returns (chatSlice.ts:785, :786); _segment returns at
:790.

So a stop_event -- a TURN-ENDING frame, exactly when the slot is no longer running -- seeds the
array while leaving the run state idle. The latch never fires, the pane stays on its bounded page,
and under the old argument the marker was never recorded at all. Not transient: permanent for that
pane. That is UX's claim, reproduced from the source rather than from either lane's summary.

On the test Opus cites: it does assert the marker stays unset, but its name and its primary
assertion are about ORDERING, which this change preserves untouched, and no comment there justifies
the marker expectation. Its sibling in ChatPane.hydrateBound.test.tsx is the one carrying a real
invariant -- that the bounded length is handled BEFORE writeSlotPage's hasMore === undefined
early return -- and that invariant is preserved, now reached through a caller that omits has_more
instead of through a seeded frame.

Recording the disagreement rather than quietly siding with one lane: if a maintainer prefers Opus's
reading, the revert is one argument at chatSlice.ts:2307 and the two marker assertions that go with
it.

What the bound does and does not remove server-side (Design lane, this revision)

Every chat_handlers.py line number in this description is relative to this PR's merge base, not to current main — the handler has since shifted by roughly fourteen lines upstream, so #4306 restates the same coordinates against a named main commit for anyone reading it there.

An earlier version of the Problem section above said the unbounded call "reads the full chained history across gateway restarts" with no qualification. The Design lane showed that is too strong, and it is now corrected in place. The accounting, read at source: the unbounded path reads disk only when the slot has older sessions behind it (if slot._disk_older_count > 0 and state.conversation_log, chat_handlers.py:1067) and otherwise serves the in-memory list (:1081); the bounded path carries no such condition and always calls read_messages_chained (:1093-1095), exactly as its own comment states ("Always reads from chained disk history", :1089).

So for a slot with no older sessions on disk, the bound replaces a memory read with a full disk read and parse — a server-side regression for that case. What the bound does remove is transfer size, client-side parse, and store growth, which is where the N-panes cost this PR targets is actually paid. It does not reduce server-side history reading, and for a non-restart slot it increases it. That sharpens rather than weakens the case for the deferred collapse-then-bound handler change described under "Honest limitations", which is now TRACKED as #4306, which states the ordering problem, both of its consequences, and why the raw-index cursor makes it a paging-contract change rather than a reordering.

The Design lane also notes that writeSlotPage being the single writer of slotMessages and its two records is enforced by convention only: nothing stops a future reducer from assigning state.slotMessages[k] directly and reintroducing the stale-marker class this PR closed. That is accurate and also deferred — a lint rule or a reducer-level accessor would be the fix, and neither is in this diff.

The earlier-messages row no longer speaks pin vocabulary (UX lane)

Status: shipped in this commit. The locale key, the required origin field on the pending-jump state, and the hover-token fix are all present in the current head.

The row's handoff reuses the pinned-jump machinery, and that machinery's failure branches rendered pages.chat.pins.message_unavailable ("This pinned message is no longer available in the loaded history") to a user who clicked "Earlier messages" and never touched a pin. Verified reachable end to end before changing anything: the button (ChatPane.tsx:447) calls onOpenFull, SessionGridView.tsx:136 passes that straight through as onCollapse, and ChatPage.tsx then sets the same pendingPinnedJump state the pin path uses — which carried {slotKey, messageTs, mid} and no field saying where the jump came from, so all three failure branches inside the shared effect reached for pin wording.

Fixed by giving that state a required origin: 'pin' | 'earlier' field. Required rather than optional is the point: a future caller that forgot it would otherwise silently inherit pin copy, and now it cannot compile. The effect resolves the notice once per run from that field, so the three branches (exhausted history, a page that returns nothing, a rejected page) all report the entry point the jump actually came from. The fourth site that still names the pin string directly is the pin-only path in handleJumpToPinnedMessage, which no earlier-messages click can reach.

The new string is components.chatPane.earlier_messages_unavailable, registered the same way as this PR's own sibling key earlier_messages_open_session: hand-authored in en.manual.json plus the eleven translated catalogues, with en-XA regenerated by npm run i18n:pseudo. en.json is deliberately untouched because it is regenerated wholesale from source literals, so a hand-added key there would be dropped on the next codemod run. All i18n gates pass, including diff-scoped against the base and the catalog key-reference gate.

Also from the lane's Suggestions: the row's button carried hover:text-accent over a text-accent base, so it looked identical at rest and on hover and the transition-colors animated nothing. It now uses hover:text-accent-hover, a token defined in every theme and already used in fifteen places in this codebase.

One honest note on coverage. The existing test for this area asserts against the source text of ChatPage.tsx rather than rendering it, and I kept that idiom rather than introducing a harness. That means the new assertions pin the wiring, not the rendered behaviour. A real harness for this effect is the follow-up the Design lane itself recommends as the best value here, and it is not in this diff. Both assertions were negative-controlled: run against the pre-fix file they fail, one on the missing origin tag and one on the notice selection.

A background warm could drop a message sent while it was in flight (automated review, this revision)

An automated review flagged the newer-tail rescue in the background warm as able to discard a just-sent message. It was right, and it is fixed here.

The rescue looks for the warm page's own NEWEST row inside the array the pane already holds, then appends whatever sits after it. That only works when the newest row can be identified. A completed stream row can come back from the server carrying neither meta.mid nor meta.sendId, and rowIdentities returns nothing for such a row, so the search was skipped altogether and the array was replaced by the warm page. A message sent while the warm was in flight sat past the end of that page, so it was deleted.

Reproduced before anything was changed. A pane holding one server row plus an optimistic send, warmed by a three-row page whose newest row carries no identity, ends up holding ["history", "warm mid", "completed stream"] -- the sent row is gone.

The fix anchors on the newest row the pane already holds that the warm page still represents, rather than on the warm page's own newest row. Rows after that anchor are newer than the page, so they are appended.

The review also proposed a broader remedy: keep the prior array whenever the newest row cannot be located. That is deliberately not adopted. It would suppress replacement in every unlocatable case, including a genuinely stale short array that should be replaced -- and stranding stale scrollback is the failure the warm path exists to fix, so the two mistakes are opposite and both silent. The narrower anchor avoids the trade: a row with no identity can never become the anchor, so a stale array of unidentifiable rows is still replaced.

Both directions are pinned. The regression test fails without the fix (expected [ 'history', 'warm mid', ... ] to include 'just sent') and passes with it. A counter-case test asserts a stale short array is still replaced, and it passes both with and without the fix, which makes it a control on the narrowing rather than a restatement of it.

One residual gap, stated rather than fixed. removeQueuedMessage promotes a queued bubble into a user row carrying no meta at all, so that row has no identity either and this mechanism cannot tell it apart from a stale row. Closing that needs an identity on the promoted row, which means changing a different reducer this change does not touch. Before this change the warm replaced the array wholesale, so such a row was dropped as well -- the gap is narrowed here, not introduced.

A warm resolve stopped a background pane from ever widening to full history (automated review, this revision)

An automated review flagged the marker write at the end of the background warm. It was right, and it is fixed here. Its own header said it found nothing blocking, so this was only visible by reading the details body.

writeSlotPage treats its two trailing arguments oppositely when they are omitted. Leaving out hasMore leaves the existing value alone, but leaving out the bounded length deletes the marker. The warm passed only four arguments, so every warm resolve deleted slotPaneBounded for that pane. hydrateSlotMessages then takes an early return when that marker is absent, which means the one bounded-to-full widen the code deliberately allows can never happen afterwards. The pane stays on the narrow page while still advertising that earlier messages exist.

Measured before changing anything, with a control. A pane holding a two-row page plus one live tail row, warmed by a truncated page, then handed the full history: the marker went from 2 to absent and the array stayed ["p1","p2","t1"] -- the older rows were discarded. The same fixture with no warm in between upgraded correctly to ["older A","older B","p1","p2","t1"], so the refusal is caused by the warm and not by the fixture.

The fix restates the bounded length, but only while the warm page is still the merged array's prefix. That is the condition the marker actually means: the first N rows are the page and everything after is a live tail.

The review's own suggestion was to retain the length whenever the warm is still truncated. That is deliberately not adopted, because the merge can legitimately grow past the page -- older rows can be prepended, or a longer prior array kept -- and restating a length over those arrays would claim the pane is narrow when it is not. That is the stale-marker class this change spent several rounds closing, so the two mistakes are opposite and both silent.

All three directions are pinned. The regression test fails without the fix (expected undefined to be 2) and passes with it. Two counter-case tests assert no marker is claimed when older rows were prepended and when a longer prior array was kept; both pass with and without the fix, which makes them controls on the scoping rather than restatements of it.

A failed fetch borrowed the copy for history that is genuinely gone (automated review, this revision)

An automated review flagged the pending-jump .catch branch. It was right, and it is fixed here.

The effect built one notice and used it for every way a jump can fail to land. Two of those are genuine absences: the history holds no older page, or the fetch returned none. The third is a fetch that errored. Because all three shared a string, a one-off network failure after clicking "Earlier messages — open full session" told the reader their earlier messages were no longer available in this session's history -- a permanent claim, in a banner that self-dismisses after eight seconds and offers only a close button, with nothing to retry. The sibling bar for the very same operation already says "Couldn't load earlier messages — retry".

The two genuinely-gone branches keep the existing wording, which is correct for them. Only the error branch changes, and it gets a new string: "Couldn't load earlier messages. Try again." The new key is scoped to the earlier-messages entry point; the pinned-message entry point has no paging-error string of its own, so it keeps its existing wording rather than gaining an untranslated one. Each translation is built from that locale's own established phrasing for this failure, taken from the retry bar's string rather than invented.

Two things worth recording because they shaped the change.

The retry bar's own copy was not reused. It ends in "— retry", which promises a control; the notice has no retry control, only a dismiss. Borrowing it would have replaced a false permanence claim with a false affordance claim.

The selection was briefly extracted into a small pure module so it could be unit-tested directly. That was reverted after measuring the consequence: the i18n key gate resolves literal, const and as const map arguments, so passing a helper's return value into the translator turned both strings into unresolvable dynamic sites -- pages/ChatPage.tsx: 0 -> 2, static references down from 12,284 to 12,282, and the gate states plainly that such a site "is exempt from every check above" while still exiting 0. Two user-facing strings would have silently lost verification behind a green gate. With the literals back at the call site the gate reports 12,285 static references and 40 dynamic sites, which is the pre-change baseline.

On testing: the branch assertion is against the page's source rather than a render, which is the convention this repo documents for page-level wiring -- the transcript renders through a virtualizer that "mounts an empty window with no layout engine", and no harness here can reach this effect, since the earlier-messages origin is only set by a click inside split view. Those assertions do fail without the fix: reverting only the page produces three failures, including the guard-ordering check and the count that pins two not-found notices against one error notice. The half a source grep cannot cover -- whether the keys exist at all -- is covered by execution instead, reading the real catalogues; removing the key from one file fails that test with the file named.

Neighbouring timing-fragile test: fixed on mainline, not by this revision

An earlier revision of this branch fixed website/src/test/PierreWorkspaceTreeImpl.test.tsx, which belongs to the workspace file tree rather than to chat. Mainline has since carried that fix itself, so it is absent from this revision: the file is not among its changed files. The sharding account below is retained because it is why a workspace-tree test was ever touched from a chat change.

What happened is a sharding effect. The frontend suite is split into four shards by hashing each test file's path, sorting by that hash, and slicing the sorted list into four. The slice boundaries depend on how many test files exist, so adding a file can change which files land in a shard. This revision added one test file for the locale keys described above, and that file hashes into the fourth shard. Recomputing the split both ways shows the fourth shard gained exactly that one file and lost nothing, and that the workspace-tree test was already in the fourth shard before and after. So this PR did not move that test into a new shard; it changed what else runs alongside it, and that was enough to tip an assertion that was already fragile.

The assertion was fragile because it read state synchronously. The test makes the data layer refetch, then asserted that the tree model was reset a second time. The refetched data is delivered on a later turn of the event loop, so the assertion could run before the component had seen it, and it then read only the first reset. Mainline's version now waits for the second reset instead of assuming it has already landed, and its own comment records the same two-ticks reasoning. The values asserted there are unchanged and still exact, so the test still fails if that reset never happens; it only tolerates the reset arriving a moment later.

The evidence that this is pre-existing rather than caused by this branch: the same test passes on the base commit this branch merges against, on all four shards; it passed on every earlier revision of this branch; and it fails intermittently in isolation, with no code change at all. Reproduced once locally with a byte-identical failure, then passed twelve consecutive runs unmodified. With the fix it passes, and the whole file's tests run no slower, which shows the wait resolves immediately rather than sitting out the ten-second poll interval.

This revision is also rebased onto current mainline, so the commit's parent is the branch tip it merged into. That rebase is what brought mainline's own version of the test into the tree, and it closes a gap of several dozen commits that had opened up while this PR was in review.

One other job failed in the same run and is not addressed here: a backend Python test that asserts an audit call happens once observed it happening twice, with the second call plainly belonging to a different test that leaked into its mock. This diff contains no Python at all, that test file is byte-identical on both sides, and the backend suite is split by its own committed timing data, so nothing in this change can alter which backend tests run, in what order, or what they contain. It passes on the base commit. Flagged rather than patched, since guessing at another suite's cross-test leakage from inside a frontend change would be a worse outcome than naming it.

Rebased onto the new single-owner slot teardown (conflict resolution, this revision)

This revision is rebased onto current mainline to clear a merge conflict. Exactly one file conflicted, website/src/store/chatSlice.ts, and the conflict is a real design change rather than a textual collision, so it is worth stating what happened to it.

When this branch was written there were two separate places that tore down a closed slot: the reconcile against the authoritative slot list, and the local delete. Each carried its own hand-written list of the per-slot maps to clear, and this branch added its two new maps to both of those lists. Mainline has since given teardown a single owner: both paths now route through one helper, and one registry names every map that is keyed per slot. That registry is the thing this branch has to join, so both of its hand-written entries were dropped and the two maps are registered once instead. The resolution keeps mainline's side of both conflict regions in full, including the new snapshot-seen guard on the live-frame path, and moves this branch's contribution into the registry.

That is not a cosmetic reconciliation. Registering the maps is load-bearing: with the registration removed and everything else unchanged, the existing completeness test fails because the deleted slot's entry survives in both new maps. Left unregistered, the rebase would silently have reintroduced exactly the leak that mainline's change exists to prevent.

The completeness and parity tests enumerate the per-slot maps from a list held in the test rather than from the registry, so they would have passed without ever touching the two new maps. Both maps are now seeded and listed there, which is what makes the check above meaningful rather than vacuous.

Nothing else conflicted. The thirteen locale catalogues auto-merged, and both sides' keys were verified to survive rather than assumed: no key mainline added is missing, and the only key present before and absent now is one mainline deleted deliberately in a separate change. The two screenshot files are unchanged, byte for byte.

"Try again" named an action the surface no longer offers (UX lane, this revision)

The transient copy added earlier ended in "Try again", which was accurate about the failure being retryable but named nothing the reader could actually do. By the time the notice appears, the marker's own action has already switched to the full session and closed the split, so the row that was clicked is gone.

The copy now points at the retry path that does exist on the surface the reader is looking at. Scrolling to the top of the full conversation re-triggers the fetch, and the top of the transcript also carries an explicit control whose own label, in the failed state, offers the retry directly. Both were checked against the code rather than assumed: a rejected older-messages fetch records the error but leaves the has-more flag alone, so the control's render condition still holds after the failure, and neither the scroll trigger nor the button consults the error flag before allowing another attempt. The string changed in all thirteen catalogues, each built from that locale's own existing wording for this failure, with the pseudolocale regenerated rather than hand-edited.

A retained live tail left the pane marker unrecorded, and a late hydrate then duplicated rows (blocking review finding, this revision)

Two automated reviewers read the same expression and disagreed about it, so this is worth stating plainly: one called it correct, the other called it a data-corruption path. The second was right, and the disagreement turned on a detail of the helper rather than on the expression itself.

The warm writes the pane's array and, alongside it, two facts: whether older messages exist beyond what the array holds, and how many leading rows are the bounded page. In the helper those two arguments behave OPPOSITELY when omitted. Omitting the page length DELETES that record. Omitting the has-more flag does not clear anything — it returns early and leaves whatever was there before. So declining to pass the flag is not the neutral act it reads as: when nothing was there before, nothing gets recorded at all.

That matters because the flag's presence is doing a second job. The hydrate reducer treats it as the signal that the slot already holds a loaded transcript, and refuses to prepend a page in front of one. Its own comment says so. When the flag is absent that refusal does not happen, and the code path it falls through to concatenates the incoming page ahead of the existing array with no overlap check.

The two conditions had drifted apart. The page length was recorded whenever the warm's rows were the start of the merged array, but the has-more flag only when the merged array was EXACTLY the warm's rows. The gap between those is the case where the warm keeps a newer local tail — rows that arrived live while the fetch was in flight. In that case the array still begins with the warm's rows, so the flag would have been truthful, and it was withheld anyway.

That case is reachable without any unusual timing. Live frames append to a background slot's array without writing either record, so a warm that finds an anchor among them and appends the newer ones is the first writer of any marker. Both conditions now key on the same predicate: the flag and the page length are recorded together whenever the warm's rows are the array's prefix, which is exactly when both are true of it.

Confirmed by execution rather than by argument. With the tests present and only this condition reverted, one fails because no marker is recorded at all, and the other fails with the transcript holding the overlapping row twice — the duplication the finding predicted. The two counter-cases that keep the fix from being over-broad still pass unchanged: neither an array that gained older rows at the front nor one that kept a longer previous array claims to be a bounded page.

Formatter-baseline line: pruned on mainline, not by this revision

An earlier revision of this branch deleted one line from the repository's black baseline, src/kiro_crew/mcp_gateway/preflight.py. Mainline has since pruned that entry itself, so the deletion is absent from this revision: .github/black-baseline.txt is not among its changed files. The rebase described below is still load-bearing, which is why this section is retained rather than dropped.

The baseline lists the files the formatter is allowed to skip, and it may only shrink. That file became formatter-clean on mainline while its entry stayed behind, so the gate failed on every open pull request at once, on a file none of them touched -- the graduation half of that check scans the whole baseline rather than the files a change touches. With mainline now carrying the prune, this branch no longer needs to, and re-adding the line here is barred.

The rebase serves the same gate rather than housekeeping. The other half of the check is scoped to mainline's commits since this branch's base, so an older base widens that scope until it captures unrelated files mainline left unformatted. Rebasing narrows it back to this change's own files, which is what the check is meant to judge. This revision is rebased onto current mainline with nothing behind.

Two removal-class merges in the background warm (blocking review findings, this revision)

Both sit in the same reconciliation and both lose messages, in opposite directions, so they are described together. Each was reproduced by execution before anything was changed.

An unordered writer erased the ordering the warm had recorded. The retained per-slot server count has a companion field recording WHICH warm last set it, and the staleness check reads that field to separate a genuinely shorter history from a late-arriving earlier response. Only the background warm supplies that ordering token; every other writer of the count — the two focused-slot fetches and the pane hydrate — has none. The retainer treated the absence of a token as a reason to CLEAR the recorded order, which destroyed the one field the staleness check depends on. An unbounded pane hydrate landing between two concurrent warms was therefore enough on its own: the earlier warm's older, lower count then read as a truncation and the rescue that keeps the newer completed turn was suppressed. Before the fix the newer turn is present once the second warm lands and gone once the first arrives.

Only an ordered response now moves the recorded order. The opposite direction is pinned by the same test rather than reasoned about: an unordered response is still the newest view of the COUNT and has to land, so the count still updates while the recorded order is left alone. The alternative — refusing an unordered count while an order exists — was rejected because it freezes the baseline outright if the ordering token ever stops being emitted, which loses messages later rather than not at all.

A confirmed removal was still allowed to restore removed history. Clearing a conversation empties only the focused session, so a background pane keeps its cached array and the turn-end warm is the first thing to see the shorter history. The reconciliation already computes whether the server's own count fell, and already consults it to suppress the live-tail rescue — but the choice of merge base ignored it, and two of that choice's branches keep the whole previous array. Both put deleted messages back on screen, and one of them also discards the confirmation the server had just written. Both branches were reproduced independently.

The removal check now governs that choice too. It is deliberately NOT placed above every branch. The branch that retains older scrollback above the page is reached only when the warm's own oldest row was FOUND in the previous array, which the two restoring branches require to have been MISSED, so those cases cannot both apply; ordering the check between them closes the removal case while a rewind that removed only newer rows keeps its contiguous scrollback. The broader placement was applied first as a control and measured rather than argued down: it deletes that scrollback, which is the failure the retained head exists to prevent. That measurement is why the fix is narrower than the remedy as first stated.

@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 14b6bac845d320e219b186431217d34e0e1b4681 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 14b6bac

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 14b6bac845d320e219b186431217d34e0e1b4681 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Reviewed the full diff (ChatPane row, ChatPage jump/notice wiring, SessionGridView handoff, all 13 locale catalogs, the capture harness, and the reducer surface that decides when the row appears), traced each new string's handler against its label, and checked the base-tree siblings for consistency. The added screenshots are not materialized in the base tree, so the visual evidence was reviewed via the capture harness's own assertions (bounded limit sent, exactly one marker, marker in viewport).

What I verified, lens by lens: the row's label "Earlier messages — open full session (closes split)" names outcome, destination, and cost, and the handler keeps that promise (switchSlot + split teardown + anchored jump to the pane's oldest message). The two failure notices split transient from permanent correctly — "Couldn't load earlier messages. Scroll to the top of the session to try again." offers a real action, and the pin-vocabulary leak is closed with a required origin field. The false-row states (active pane, complete transcript, warm-truncated cache) are each suppressed and pinned by tests. The row is a real button (keyboard reachable), link-styled at rest, and reads through the theme tokens. The remaining gaps — no in-pane paging, absence-of-row ambiguity, split teardown as the only affordance — are named in the PR description with a tracked cause-level follow-up (#4306), and the label's "(closes split)" cost warning is the shipped mitigation.

UX-Verdict: PASS

The row says exactly what it does and what it costs, every false-row state is suppressed, and both failure notices offer a truthful next step.

[UX-REVIEWED] 14b6bac

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 14b6bac845d320e219b186431217d34e0e1b4681 via the fork AI-review pipeline; updated in place on each push.

Review details

The single candidate fails falsification. Its own trigger — "another client both rewinds one older row and adds one turn" between two warms — is a "could" scenario I cannot ground: for sameCountRewrite to drop row R, R must be absent from the fresh server page (rescuable requires a post-anchor prior row not in warmed), yet the candidate stipulates "server has R," in which case R is in warmed, rescuable excludes it, and it survives in base. The two premises are mutually exclusive. Where R genuinely is not on the server, this is precisely the interleaving the slotServerTotal doc comment names as an accepted, uncovered residual, and the harm is a transient omission in a background pane's cache that self-heals on the next unbounded refetch — not data loss, no server-side loss, no crash. Confidence well below 80.

No new grounded defect surfaced under Step 2.

No findings.

[OPUS-REVIEWED] 14b6bac

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 14b6bac845d320e219b186431217d34e0e1b4681 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

The mount bound is right; bounding the warm path too bought a hand-rolled cache-coherence protocol in the reducer that #4306 will only partly retire.

Watch

  • Complexity is concentrated where the PR's own root-cause analysis says it shouldn't live. The stated cost ("N concurrent full-history fetches when a grid mounts") is removed by the pane-query bound alone; bounding warmSlotCache as well is what forced four new per-slot maps (slotPaneHasMore, slotPaneBounded, slotServerTotal, slotServerTotalSeq), a warm sequence counter, identity-based merges, and count-fall rewind detection into chatSlice. The description's own recorded alternative — unbounded warm, marker written false — deletes the merge branches at the cost of one full fetch per background chat_done (far rarer than grid mounts). Five consecutive review rounds each found a new silent-corruption path in this machinery, which is evidence its state space sits at the edge of what review can hold; the handler-side collapse-then-bound fix (Collapse chat rows before applying limit in the slot detail handler #4306) deletes the latch and the running branch but not most of the merge engine.
  • Truncation detection is heuristic-on-heuristic: server-total deltas, "keep the longer array", parsed-ts ordering, and same-count-rewrite length comparison stand in for a signal the server could state authoritatively. The description itself records one known-wrong case (legitimate shrink vs. longer stale prior) as deferred without a tracker.
  • Description ↔ diff: the section claiming a PierreWorkspaceTreeImpl.test.tsx flake fix "in this revision" has no backing hunk in this patch; if mainline absorbed it, the section should be retracted the way the black-baseline one was.

Suggestions

[DESIGN-REVIEWED] 14b6bac

@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from cb1e761 to 60deb06 Compare August 13, 2026 13:19
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 13, 2026
@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from 60deb06 to df4b51e Compare August 13, 2026 14:14
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from df4b51e to 940a50f Compare August 13, 2026 14:36
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: action required A blocking check or review needs attention labels Aug 13, 2026
@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from 940a50f to 930c6f0 Compare August 13, 2026 16:18
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention labels Aug 13, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 14, 2026
@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from 062f05a to 0644c15 Compare August 14, 2026 21:31
@github-actions github-actions Bot added readiness: checking Automated validation is still running merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: action required A blocking check or review needs attention labels Aug 14, 2026
@rnoack1
rnoack1 force-pushed the fix/bound-grid-pane-hydrate branch from 0644c15 to 4aa4e0e Compare August 15, 2026 12:11
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 15, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 15, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

4 similar comments
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

  • ## Problem / Motivation
  • ## Why it matters
  • ## What changed
  • ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

@rnoack1

rnoack1 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Dispositions for the two advisory lanes. The full reasoning and source evidence live in the PR description; this is the short version plus the tracker pointer.

Design — the server-side harm claim (accepted; description corrected). Verified at source. The unbounded path reads disk only when the slot has older sessions behind it, and otherwise serves the in-memory list. The limit path calls read_messages_chained unconditionally, as its own comment says. So bounding shrinks transfer size, client-side parse and store growth, but leaves server-side history reading unchanged — and for a slot with no older sessions on disk it replaces a memory read with a full disk read and parse. My original description claim was too strong and now states this plainly, including that one case regresses.

One correction to the review coordinate, written so it does not decay: chat_handlers.py:1093 does not point at the limit path. A bare line number is not much use here — main moved three times while this was being checked — so here are the anchors to search for instead. The unbounded branch opens with if limit_raw is None and before_raw is None: and reads disk only under slot._disk_older_count > 0 and state.conversation_log. The limit branch carries the comment # Always reads from chained disk history; no in-memory offset math. immediately above its own unconditional read_messages_chained call. On main at d6ac047e8d those are lines 1155, 1157, 1179 and 1184; line 1093 there is an unrelated dictionary entry. The review's verdict was right and only its line number was off. #4306 records the same evidence against a named commit.

Design — complexity pending the cause-level fix (tracked). Filed as #4306. It covers collapsing rows before applying limit, and the next_before raw-index cursor question that makes this a paging-contract change rather than a simple reordering of two operations.

Design — the single-writer invariant is convention-only (accepted; not fixed here). Accurate as stated. A lint rule or a reducer-level accessor would make it structural, and neither is in this diff. I did not add one, because I have not proven a check that actually fails when a second writer is introduced, and a gate that cannot fail is worse than none.

UX — recovery exits split view (accepted trade-off; tracked). The affordance names its cost on purpose. Paging in place depends on the same handler change, so it is covered by #4306 rather than reworked here.

UX — pin vocabulary on the earlier-messages path (accepted; real). Verified reachable end to end: the row's button hands off through onOpenFull to onCollapse, which sets the same pending-jump state the pinned-jump path uses. That state carried no field recording which entry point the jump came from, so three of its failure branches rendered the pinned-message string to someone who had only clicked "Earlier messages". The fix is a dedicated earlier_messages_unavailable key across the catalogues plus a required origin field on the pending-jump state, with the notice resolved once per effect run. That fix is now shipped in this PR — the locale key, the required origin field, and the hover-token fix are all present in the current head.

No new commit was spent on these dispositions; everything above is either a description correction or a filed issue.

The session grid mounts one pane per session and each hydrated its full transcript, so the cost scaled with panes on screen; each pane now hydrates a bounded page and carries a marker for whether older history exists.

That marker is written by one writeSlotPage helper alongside the array it describes, so the two cannot disagree, and the warm merge keeps a pane's existing older head by matching row identity (meta.mid) rather than timestamp -- two rows can share a ts, so a ts-keyed cut sliced a distinct message out of the middle.

A bounded page is a snapshot, so the merge also has to tell a row another client
rewound away from one the page was simply built too early to carry. It reads the
server's own row count for that: a fall between fetches means history was
truncated, so the pane's forward turns are not rescued. The pane's own hydrate
now carries and retains that count, because it is the first thing that learns it
for a background slot -- the other two retainers both sit behind an active-slot
guard, so neither can ever seed one, which the tests establish by execution.
This does not widen any fetch: the count describes what the server holds and is
returned with every page regardless of limit.

Rewritten deliberately: the "decline, not guess" case in
chatSlice.warmSlotCacheBound.test.ts asserted that a pane with no retained count
keeps the rescue, and used a fixture standing in for the pane path. That path now
has a count, so the fixture no longer represents it. The principle is unchanged
and still correct for a pane that genuinely has none, so the test is narrowed to
one seeded by live frames alone rather than deleted. A negative control pins the
opposite direction, since suppressing the rescue wrongly drops a live row the
server does hold.

Known residual, stated rather than implied: a rewind coinciding with a local send
the server has not yet acked can suppress that unacked row. That is a property of
the count comparison itself, not of this hydrate, and it now reaches the
hydrate-seeded panes too.

A count taken while the turn is RUNNING is refused rather than retained. The
server counts raw rows, so a streaming response is inflated by rows that collapse
when the turn ends, and retaining it made the next warm read that ordinary
collapse as a truncation -- suppressing the rescue and dropping a live row, the
opposite direction to the re-append the baseline exists to prevent. The guard sits
in the single retainer, so all four call sites are covered and cannot drift, and
it leaves no baseline rather than a wrong one. A settled hydrate still seeds the
count, which is what keeps a genuine rewind detectable; a test pins that boundary
so the guard cannot quietly widen into "never retain".

Composed with kirodotdev#4578 rather than choosing a side. That change routes this reducer's
array write through mergePreservedThinking, because reasoning is broadcast-only and
never persisted, so a warm rebuilt from server history dropped every block of a
slot the user switched away from mid-turn. Its concern and this reconciliation are
orthogonal: the engine reconciles SERVER rows, and reasoning is not one. So the
thinking rows are held out of the engine's inputs -- they carry no identity, and
the rescue would otherwise keep them under "decline, not guess" and append a second
copy -- and restored by that helper onto the reconciled list, which appends any
block it cannot anchor, so holding them out cannot lose one. The bounded marker is
an index into the array written, and reviving inserts rows above it, so it is
re-derived against the revived array rather than taken as the warm's length.

Measured, not assumed: without the helper the interleaved block is lost outright
(a test reproduces it), and kirodotdev#4578's own tests plus AssistantMessage stay green, so
a structural revert of either side would be caught.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants