fix: stop the chat transcript moving on its own

[buluoray]

A reader on a phone saw history load itself on a session switch, a saved reading
position land somewhere different each time, and a session holding 300 messages
come back holding 6,265. This is that whole family, plus the tool that found it.

One shape underneath all of it: a decision computed for one state, applied in
another. Each cause below was reproduced on a real device and named by the
writer the inspector caught doing it -- not inferred from reading the code, which
produced self-consistent stories rather than answers for several rounds.

Why a dev tool ships with the fix

Four of these are structurally invisible to the test suite. jsdom's
getBoundingClientRect returns zeros, so the settle loop that re-lands a
restored reading position has a body that never executes there: a tolerance
finer than the device pixel grid, a frame budget that expired before measurement
began, a cancelled requestAnimationFrame, and a hydration deadline that got
harder to meet the more was loaded. All four were found by putting a live readout
on the phone.

The inspector overlays the transcript with its geometry and a rolling log of the
decisions that move the reader, plus two sticky lines naming how the last LEAVE
and the last ENTRY resolved the saved position -- those happen at the start of a
switch and are the first to scroll out of an 8-line window, which on a phone
means they were never visible. Off is zero cost: a module-level flag is read
first by every entry point, so disabled means no element, no interval, nothing
retained.

The turning point was giving each of the fourteen code paths that write this
scroller a name in the log. A position landing somewhere nobody intended had been
unattributable; one capture then read:

WRITE restore  3021->965
WRITE reprice2  965->20211

Same decisecond. Culprit, direction and magnitude, in two lines.

History nobody asked for

• The switch window ratcheted. It asked for cached + one page as coverage
  headroom, but the server window is anchored at the NEWEST row and extends
  backward, so every spare row is a row of older history -- and the next revisit
  measured the cache it had just grown, one page per switch to the handler
  ceiling. Coverage is verified AFTER the response, so the headroom bought
  nothing the retry did not already cover.
• A slot mid-turn was exempt from that bound, on the same pre-purchase
  argument, which applied the unbounded shape to the commonest switch there is:
  303 loaded messages became 6,265 in one step, ~293,000px of transcript, and
  eventually an OOM-killed tab. Run state is no longer an input to the window at
  all -- an exemption that can be re-expressed by passing a flag is one that
  grows back.
• The retained server count refused every RUNNING response, which
  manufactured the absence its own coverage check then read as "assume a hole",
  so a slot that streams for most of its life took the unbounded path forever.
  Only an UNBOUNDED read counts raw rows; a bounded one is collapsed by the
  handler before it slices, so its count is comparable and is kept. The retry
  carries the bounded count forward instead of discarding the only comparable one
  it had.
• The coverage check counted rows the server never had. queued, streaming,
  thinking and permission rows exist only in this client, so a bounded window
  cannot contain one however wide it is asked to be -- and a cached row the window
  can never hold is not a hole that a bigger read closes. It read as a shortfall
  that never goes away, so every switch into a slot holding a queued message or a
  permission card refetched the whole transcript. The check now reads the same
  isDurableRow predicate two other consumers in that file already share; a
  narrower home-grown test caught streaming and missed the other three.
• Entry no longer inherits authorization. The walk poll's authorization was a
  latch that could only turn ON -- one touch authorized it for the rest of the
  mount -- and its page budget was an effect-local variable reissued on every
  re-creation. It reads the same expiring window the sentinel door does. The
  pinned-jump walk incremented a counter it never compared to anything.

The reading position

• The persisted anchor used the wrong vocabulary. It stored a per-render key,
  which ChatPage documents as valid only inside the render that produced it. It
  stores the index-free stable id, in all four places -- including the settle
  comparison that aborted at frame 0 on the mismatch.
• Neither end of a row is stable alone. Appends rename its tail; an older
  page landing regroups messages into its head and renames its lead. A switch
  back into a live turn does both at once, which is why one identity always
  missed. Both are persisted and either resolves.
• Convergence was unreachable during a live turn. It required the whole
  transcript to stop growing, but the cause of the corrections is height arriving
  ABOVE the anchor, which appends below it never touch. Every restore burned its
  entire budget with the skeleton up, however early it had really landed.
• Four mechanisms wrote the scroller during one landing, each undoing part of
  the last (+261px of drift). They hold a row where it was, which is the settle's
  job, so they stand down while it is ACTIVELY correcting -- and take over when it
  goes blind, because a settle that cannot see its row corrects nothing while
  still holding its gate.
• An automatic bottom pin decided on pre-restore geometry and applied its
  write a frame later, landing a reader who had left 24,600px from the end at the
  end. It re-checks when it applies, not only when it is decided.
• evaluateAutoPin's idle branch released follow without asking who moved, so
  a late image growing the transcript under a reader parked at the bottom made
  the rule that exists to catch a scroll-up release follow instead.

Automatic actions no longer authorize themselves: the walk reads an expiring
window, the anchor save revokes the gesture a restore borrowed, and the settle
aborts on hard input rather than on the scroll events its own writes produce.

The placeholder that covers all of it

One placeholder now covers both waits. Fetching a slot showed a centred spinner
and restoring a position showed grey bars, so two readings of the same fact --
the transcript is not ready -- looked like different events. It is shaped like a
transcript (assistant blocks with a short last line, alternating with narrower
right-aligned user bubbles, the sweep staggered per line so it reads as one wave
travelling down them), because six equal full-width bars preview a table.

.skeleton also pointed at @keyframes shimmer, which translates its TARGET --
right for .animate-shimmer, wrong here: every skeleton bar in the app slid
sideways by its own width instead of passing a highlight across itself. It has
its own keyframes now, moving a highlight pseudo-element, so a sweep costs a
compositor transform rather than a gradient repaint every frame -- which matters
most exactly here, since a skeleton appears when the main thread is busiest
measuring the rows it stands in for. Every other skeleton in the app is fixed by
the same change.

Device verification

Same session, scrolled up mid-history, switched away, switched back:

	before leaving	after returning
scroll position	y=8972	y=8972
loaded messages	200 / 7,417	200 / 7,417

Byte-identical, not merely close, and repeatable across sessions. The settle
converges at frame 1--2 with a 0.0--0.9px residual during a live turn, where it
previously ran 34 frames and gave up.

Tests

Every fix is pinned, and every guard was mutation-verified -- the source mutated,
the run confirmed red, the mutation reverted. Two invariants have no runtime
surface jsdom can reach (a restore must drop the capture it supersedes; the
bottom pin must re-check ownership at APPLY time, not only when decided), so
those are guarded against the source text, the way FollowController.test.ts
already reads its own source.

One mutation stayed green and is reported as such rather than papered over:
devWatchScroller's gate is belt-and-braces behind ensureHost()'s own, and its
only unique effect has no observable surface.

Ran: tsc -b (0), targeted eslint --max-warnings 0 (0), the 15 affected test
files (185 passing), lint:i18n and i18n:check (both 0). The full backend
suite was not run -- this change is frontend-only.

Not in scope, recorded

Scrolling up through never-measured history accelerates in places, because
unmeasured rows are priced at one running mean while the content is bimodal (the
height index's own comment: a code-fenced row is often 5--30x the mean). That is
pre-existing, needs a better per-row estimate, and belongs in the measurement
path rather than stacked on this.

Screenshots

The new surface: Developer → Debug tools, behind Developer Mode. Off is the
default, and off means the module holds no element, no interval and nothing
retained -- the two states are shown because that contract is the reason it can
ship enabled-by-toggle rather than behind a build flag.

Scroll inspector toggled on

Captured on an isolated pod built from this branch (kirocrew pod up), not the
live gateway; the pod's HOME was verified gone afterwards. The overlay's own
output is quoted throughout this description -- those readouts are what this tool
produces on a real phone, which is where every defect above was found.

Removed outright: the idle history prefetch

ChatPage.tsx had an Idle history prefetch (feeds the measure farm) effect. While the
reader was idle and everything loaded was measured, it pulled the next older page so the
measure farm had geometry to price, repeating until the whole session was measured.

It is deleted, not adapted, and the reason is the rule this PR is built on: it issued
loadOlderMessages with no reader gesture behind it. That is the definition of history
loading itself, and it cannot be reconciled with the expiring-gesture window the sentinel
and the walk now share -- an idle prefetch fires precisely when the reader has expressed
no intent, so authorizing it by a real gesture is a contradiction rather than a tightening.

The trade-off, stated: a first back-scroll now crosses estimate territory that the prefetch
used to have measured. That is the same acceleration symptom recorded under "Not in scope"
below, and it is accepted here rather than paid for with unasked loading. The real fix is
to price unmeasured rows better, not to load them early.

Pattern harvest

Rule candidate: semgrep (frontend)
Pattern: a scroll-position correction applied without re-reading the state it was computed against

Every defect in this PR is one shape, seventeen times over: a decision computed
for one state and applied in another. Named concretely so it is checkable:

• A window sized from cached + headroom where the window's own geometry makes
  headroom mean "older history".
• An authorization latch that can only turn ON (sawRealInputRef), so a gesture
  authorizes every later event.
• A budget held in an effect-local variable, reissued whenever the effect
  re-creates.
• An anchor persisted in a per-render vocabulary and resolved in a later render.
• A compensation captured before a positioning write and consumed after it.
• A gate evaluated on pre-write geometry whose write is deferred to a rAF.

One behaviour change worth naming, because it is not a bug fix

Replacing the top sentinel's sawInput: !vGetFollowRef.current() with the expiring
real-gesture window narrowed who can authorize automatic older history: !follow
was true after ANY scroll-up, our own included, which is what let the ratchet run —
but it also meant a keyboard reader (PgUp/Home/space) or a scrollbar-drag reader
authorized it for free. The window is fed by real input events, and had those been
wheel/touchmove alone, exactly those readers would have lost automatic history
and been left with the manual Earlier bar.

So the vocabulary is four events, bound to the SCROLLER: wheel, touchmove,
keydown, pointerdown. None of the four can be produced by writing scrollTop,
so the premise — an automatic scroll cannot authorize itself — is unchanged, while
keyboard navigation keeps working. keydown is deliberately not on document:
typing in the composer is not an intent to read history.

The generalizable half is narrower than "be careful": a correction that reads
geometry must re-read it at the moment it writes, and a permission that can be
granted must be able to expire. Both are mechanically detectable in this
codebase's shape -- a writeScrollTop inside a requestAnimationFrame whose
guard was evaluated outside it, and a Ref that is only ever assigned true.

A third, weaker candidate is a lint for a @keyframes name shared by rules that
animate different properties: .skeleton pointed at keyframes written to
translate their target, which silently slid every placeholder bar sideways for as
long as it existed.

Not generalizable, and left as one-offs: the pinned-jump counter that was
incremented and never compared, and the tolerance chosen finer than the device
pixel grid.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Behavioral scroll fixes plus a shape-true skeleton and a properly gated Developer debug tab; every new surface matches product patterns and states are bounded with graceful give-ups.

Suggestions

• ChatTranscriptSkeleton.tsx promises "the caller carries aria-busy on the container", but no aria-busy exists in ChatPage.tsx; with the transcript visibility: hidden during restoreGate and the skeleton aria-hidden, a screen-reader user meets a silently empty chat on every session restore. Add aria-busy={slotLoading || virt.restoreGate} on the swapping container as the component's own contract states.

[UX-REVIEWED] c6fc3e4

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All key surfaces check out: the windowing redesign (coverage verified after the response instead of pre-purchased headroom, streaming exemption retired only after retainServerTotal makes its baseline comparable), the gesture-authorization model (expiring window fed by four events none of which a scrollTop write can produce), the i18n gate exemptions (callee- and file-scoped with the exact-path precedent standard), the docs update in the same commit, and temp-screenshots/ being an existing repo convention rather than new pollution. The deliberate trade-offs (idle-prefetch deletion, walk page size) are disclosed with the follow-up recorded, and every non-trivial hunk is accounted for by the description.

Design-Verdict: PASS

One coherent root cause (state-skew in scroll decisions) fixed at the cause, every trade-off disclosed, dev tooling gated to zero cost when off.

[DESIGN-REVIEWED] c6fc3e4

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] c6fc3e4

False positive or not applicable? A repository writer can comment:
/ai-review override gpt c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

Every fix sits at cause level with a measured harm; the one soft spot is a new plural-named "Debug tools" tab shipping exactly one toggle.

What this change ships

Intent: stop the chat transcript loading, growing, and losing the reader's place on its own — a FIX, with one declared instrument riding along.

1. A session switch no longer grows loaded history (window = what the tab holds; ratchet removed) — justified, cause-level
2. Switching into a mid-turn session no longer reloads the whole transcript (streaming exemption deleted) — justified
3. Queued/permission cards no longer force full refetches; coverage counts real rows via the shared isDurableRow — justified, replaces a proxy with the definition
4. A saved reading position now resolves after a switch (stable id + lead-message alt; pre-v3 anchors orphaned once) — justified
5. The restored position lands and holds (time-budgeted settle, one owner of scroll writes at a time) — justified
6. Auto bottom-pin re-checks at apply; idle release never reads its own write as consent — justified
7. Automatic history fetches now require a recent real gesture (expiring window, per-session, keyboard/scrollbar count) and are bounded to 2 pages of 100 — justified; three defaults change quietly
8. Idle history prefetch deleted outright — justified deletion (it was a self-issuing door)
9. Transcript-shaped skeleton replaces the spinner and covers the restore wait; app-wide skeleton sweep corrected — declared, rides along
10. Scroll inspector overlay + Developer → Debug tools tab — declared rider ("the tool that found it"); tab is a one-entry container

Watch

The Debug tools tab is a generalized container with one entry: 1 toggle ships in it (DebugToolsTab.tsx), at the cost of a permanent 11th ?tab= value, a feature-map row, and 2 keys across 14 locale files. The author's comment rules out Settings (search indexing) and Feature Previews (instrument, not preview) — the first reason is derived, the second is taxonomy; the existing pages/developer/FeaturePreviewsTab.tsx already provides exactly this surface (per-device localStorage toggle on the Developer page, unindexed).

Subtractions

Defer the debug-tools tab: render the inspector toggle as a card on the existing Developer page tab and add the tab when a second tool exists — removes the tab key, the feature-map row, and the debugTools.* catalog entries.

[FIRST-PRINCIPLES-REVIEWED] c6fc3e4

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6 — this comment is updated in place on each push.

Review details

Both candidates are low-confidence by the discovery pass's own admission, and each dissolves under falsification.

Candidate 2 (chatSlice.ts:3206): requires a durable, mid-carrying cached row whose ts is unreadable AND that is not a live tail row (so older history sits above it). The code's own invariant — an unstamped row is a live tail row that a newest-N window necessarily reaches — is the guard, and a server-minted row (one bearing meta.mid) carries a server timestamp, so the triggering combination (mid present, ts unparseable, non-tail) is not shown to occur. The candidate concedes this ("may not occur in practice"). (a) fails: no concrete input that occurs in practice. Dropped.

Candidate 1 (useVirtualChat.ts:1789): the deadline is renewed only while itemCount strictly increases, and any pause in row additions longer than RESTORE_HYDRATE_WAIT_MS (1200ms) lets the armed expiry timer fire and lift the skeleton via the giveup path. Sustained sub-1200ms row-count growth (not content streaming into an existing row, which does not raise the count) with the anchored older row never landing, on a cold load whose bounded window excludes that row, is a compound of conditions none of which is established to hold together in practice. The observable "transcript hidden for the whole run" therefore comes out as "could," not a re-derivable outcome. Dropped.

No new grounded defect at the 80+ bar surfaced while falsifying these in the two changed files.

No findings.

[OPUS-REVIEWED] c6fc3e4

Verdict parsed from the review's SHA-scoped output markers for commit c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6.

False positive or not applicable? A repository writer can comment:
/ai-review override fable c6fc3e483f49184b8a7ea3ffb4d54767e0c6f0a6: <one-sentence reason>

[buluoray]

Disposition on f65cd780b — GPT 5.6 ×3, Design Review ×2

FIXED — GPT BLOCKING: a mutable field beside a stable id forces an unbounded reload

coverageRowIdentity keyed a mid-bearing row as [mid, role, instant]. Verified against
source, and the mechanism is worse than the finding states:

• sseChatMessage (chatSlice.ts:3775) contains
  if (message.ts && bubble.ts && message.ts !== bubble.ts) — the code explicitly
  handles the server ts replacing an optimistic client ts, stashing the old one as
  meta.clientTs because the change would otherwise remount the row.
• mergePreservedClientTs's own docstring records that finalization flips the role
  streaming → assistant.

So a cached row and its window copy can share a mid while differing in ts and in role.
Under the old key that is two rows → a false shortfall → an unbounded reload of the whole
transcript, after nothing more exotic than sending a message and switching slots. That is
the defect this branch exists to remove, re-entering through my own identity function.

A mid is now matched alone. The mid is the server-minted row id; everything else on a
row mutates, so pairing any of it with a stable id defeats the id. deduplicateByMid does
pair mid with role and ts — for a different job: it COLLAPSES rows in the rendered
transcript, so it must not let a crafted mid hide a legitimate one. Coverage cannot be
fooled that way because it counts: two cached rows carrying one mid still need two
window rows carrying it. That is pinned, and mutating the key to re-add ts or role each
turns exactly one test red.

Why this is not a fourth patch on the same boundary. Rounds 1–3 were one mistake in
three places — inferring set membership from an ordering — and that proxy is gone, replaced
by the multiset difference. This round is a different defect in a different function: an
over-specified identity. The tell is the direction of harm, which is the opposite of rounds
2–3: those dropped rows, this one reloads everything.

DEFERRED — GPT BLOCKING: dual anchors fail when both ends change

Correct, and already documented: anchorDualIdentity.test.ts pins it as
misses only when BOTH ends were renamed. The consequence is a lost reading position,
not lost data — the anchor does not resolve, so entry falls back to the pre-PR behaviour
rather than landing somewhere wrong. Two identities are strictly better than the one this
branch started from.

The suggested remedy — persist an identity from a constituent message that survives
simultaneous head and tail growth — is the right next step and is a new mechanism, not
a boundary fix: it needs the anchor to persist a message-level id, which changes what is
written to storage and what a legacy anchor means on read. That belongs in its own change
with its own device verification, not appended to a branch already carrying eleven fixes.

REBUTTED (second time) — font:9px below a 10px minimum

No such rule exists in this repository. Checked website/AGENTS.md, AGENTS.md,
docs/system-specs/common/code-style.md, website/docs/, docs/ and AUTOSDE.yaml:
zero matches for a font-size floor. WCAG sets none either. 9px is shipped precedent in
three places, including apps/issue-radar/WelcomeCarousel.tsx and
apps/issue-radar/views/GraphView.tsx. The inspector is a developer overlay in a fixed
350px box holding eight lines of diagnostics beside the transcript it measures, and it
renders nothing a user reads.

FIXED — Design CONCERNS: the idle history prefetch was deleted without disclosure

A fair catch. The effect pulled loadOlderMessages while the reader was idle so the
measure farm had geometry to price. It is deleted, and the PR body now says so with the
trade-off, because it cannot be adapted the way the sentinel and the walk were: it issued a
fetch with no reader gesture behind it, and an idle prefetch authorized by a real
gesture is a contradiction, not a tightening. The cost — a first back-scroll crossing
estimate territory — is the same acceleration symptom already recorded as out of scope, and
is accepted rather than paid for with unasked loading.

FIXED — Design suggestion: slotSwitchNeedsUnboundedRetry is dead

Confirmed: no production caller after the multiset replacement, only a stale prose
reference and a direct unit test. Function and its describe block removed, and the
comment that cited it now points at slotCoverageShortfall. The five
slotSwitchFetchLimit cases in that file are untouched.

Gates

tsc -b clean, eslint clean on every changed file, full frontend suite green: 1,853 files
/ 29,092 tests. Prior art in the PR body is untouched.

[buluoray]

First Principles disposition on f65cd780b

Advisory CONCERNS, one concrete subtraction: drop the one-entry debug-tools tab and move
its card into FeaturePreviewsTab.tsx. I verified the counts, and one of my own earlier
objections turned out to be wrong — recording both.

Verified, and the review is right

claim	measured
DebugToolsTab.tsx holds one SettingsToggle	1 — correct
FeaturePreviewsTab.tsx has 3 cards on the same SettingsCard/SettingsToggle shape	3 — correct
debugTools.* keys span 14 locale files	14 — correct
scrollInspector.ts is statically imported by 4 product modules besides the tab	chatSlice.ts, useVirtualChat.ts, ScrollAnchorCache.ts, ChatPage.tsx — correct

Correcting myself: I first grepped FeaturePreviewsTab.tsx for safeGetItem/safeSetItem,
got zero, and was about to argue the localStorage shape is not shared. That grep was too
narrow. The persistence is one module away in utils/previewFlags.ts, which does import
safeGetItem/safeSetItem, and it already fires a window CustomEvent
(PREVIEW_FLAG_EVENT = 'mc-preview-flag-changed') so a listener updates in the same tick —
which is exactly the mechanism the inspector needs, since the overlay lives outside React by
design. So the review's mechanical claim holds: mechanically, a card is a card.

What still argues against the move, from that module's own docstring

previewFlags.ts documents a narrower contract than "a place for client-only toggles":

Preview flags — local, per-device opt-ins for surfaces that ship in the bundle but are
NOT ready to be released.

Retiring a flag is the goal, not an afterthought: when the surface is polished, delete its
previewFlag from the registry entry and its card from Developer > Feature Previews.

The scroll inspector is neither unfinished nor destined for release. It is a permanent
diagnostic that should stay off for every normal user forever, so the registry's retirement
rule — the thing that keeps that list from growing without bound — would never apply to it,
and the nav rail's preview-surface handling would be reasoning about a developer overlay as a
product surface awaiting launch.

So the trade is: one tab + 14 locale key groups + a feature-map row for a single toggle
against a permanent instrument living in a registry whose contract is "temporary gate on an
unreleased surface". Neither side dominates, and the review is right that the
preview-vs-instrument distinction is semantic — it is a contract, not a mechanism.

Disposition

Left as-is on this SHA, and handed to the repository owner rather than decided here. Two
reasons, both procedural rather than a defence of the current shape:

1. CONCERNS is advisory, and this is a scope/taxonomy judgment with a real cost either way —
   the category that belongs to a human, not to a bot round.
2. A push voids every SHA-scoped verdict. UX, Design and Opus 4.8 all read clean on
   f65cd780b and GPT 5.6 is mid-run on it; spending that on an advisory restructuring —
   which would also touch 14 locale files, the exact surface with a zero-tolerance i18n
   gate — is a poor trade to make unasked.

If the owner prefers the subtraction, it is a contained change: delete DebugToolsTab.tsx,
drop the debug-tools row from buildTabs() in DeveloperPage.tsx, remove the feature-map
row, delete the pages.developerPage.tabs.debugTools.* keys from all 14 locales, and add one
card to FeaturePreviewsTab.tsx — either as a real PREVIEW_* registry entry (accepting the
contract stretch) or keeping its own key and event (accepting the first non-registry card in
that tab). I would take the second, so the registry's retirement rule stays true of every
entry in it.

[buluoray]

GPT 5.6 disposition on f65cd780b — and a stop

Both blocking findings verified against source before answering. One is real and is my own
incomplete fix; the other is largely closed by fixing the first. This is the fifth consecutive
round on the same function, so I am stopping rather than pushing a fifth revision — reasoning
at the bottom.

BLOCKING 2 — client-only rows trigger an unbounded fetch — REAL, accepted

This one is correct, cause-level, and the remedy is not a guess: it names this file's own
predicate.

chatSlice.ts:1720:

const CLIENT_ONLY_ROLES: ReadonlySet<string> = new Set(['queued', 'streaming', 'thinking', 'permission'])
function isDurableRow(m: ChatMessage): boolean { return !CLIENT_ONLY_ROLES.has(m.role) }

And the file already documents this exact hazard at chatSlice.ts:2613, for a different
consumer:

isDurableRow is load-bearing here, not decoration. A client-only row can carry a mid too
… The limit reaches a handler that slices DISK, and disk has no client-only rows, so only
durable ones may be counted against it.

slotCoverageShortfall filters on transcriptTsMs(r.ts) === null instead — a proxy for
"is this row on the server". In an earlier round I found this same harm through a streaming
row (the probe read badCached=["streaming:partial:x"]) and fixed it with the ts test.
streaming is one of four client-only roles. A queued or permission row that carries a
readable ts passes my filter, is counted as outside the window, and — since disk has no
client-only rows — the server window can never contain it. That is a permanent false
shortfall for any slot holding one: an unbounded refetch on every switch into it, the same
harm class as the 303 → 6,265 row measurement in the description.

So my ts filter covers one of the four roles it needed to. isDurableRow is strictly stronger
and is the definition the rest of the file already agrees on.

BLOCKING 1 — reused mids can falsely prove coverage — ordinary-operation path closes with the fix above

The realistic way two rows share a mid is the one that same comment names: a client-only row
can carry a mid too. Filter client-only rows off both sides and that source is gone.

What remains is duplicate mids among durable rows — the server stamping two persisted
rows with one id. I cannot produce that from the client, and the suggested remedy (match bare
ids only when unique in both inputs, else corroborate) would be the fourth revision of
this identity rule.

The two non-blocking findings

• font:9px — rebutted twice already, same evidence, unchanged: no minimum-text-size rule
  exists in website/AGENTS.md, AGENTS.md, docs/system-specs/common/code-style.md,
  website/docs/, docs/, or AUTOSDE.yaml (zero matches), WCAG sets no minimum, and 9px is
  shipped precedent in three places including apps/issue-radar/WelcomeCarousel.tsx.
• Color literals ignore the active theme — new, and answered rather than dismissed: the
  inspector is a dev overlay that assigns its own cssText and is deliberately
  theme-independent, so it stays legible above any theme including a user's custom pack. A
  diagnostic that becomes unreadable under the theme you are debugging cannot do its job. It is
  off by default and off for every user who does not enable it.

Why I am stopping here instead of pushing

slotCoverageShortfall has now been found subtly wrong five rounds running — timestamps
compared as strings, then ties, then set-vs-multiset, then mid paired with mutating fields,
now a hand-rolled durability proxy. Four of those five were fixes I wrote. Each was real. That
pattern is itself the finding: this one judgement keeps being wrong in a way that a review
round catches and I do not, and a fifth revision written by the same hand in the same loop is
not evidence of convergence.

The position fixes (Thread B) are a separate, device-verified body of work that has been stable
for several rounds — y=8972 → y=8972 byte-identical, no WRITE bottom after RESTORE.OK.
They should not keep paying for this function's rounds.

Handing the decision to the repository owner with three real paths, rather than choosing one
unilaterally:

1. Fix blocking 2 only — swap the ts proxy for isDurableRow on both sides of the
   coverage comparison (one predicate, the file's own), decline the fourth identity revision,
   and override the duplicate-durable-mid residual as unreachable from the client with an
   issue tracking it.
2. Split the coverage work out — keep the reading-position fixes and the dev tool in this
   PR, move slotCoverageShortfall and the switch-window changes to their own PR where five
   rounds of scrutiny cost nothing else.
3. Override both and track the residual, if the measured device evidence is judged to
   outweigh two unreproduced-from-the-client paths.

I recommend 1 or 2. Not 3 — blocking 2 is real and I would not ship it knowingly.

Unrelated red lane

Backend Tests (Windows) (3) is not from this diff: the PR touches 52 files and zero
Python, and that lane is Python-only. Same lane was a confirmed pre-existing xdist flake on
#8516. A same-SHA rerun was refused while sibling jobs are still in flight; it will be rerun
once the run settles, rather than assumed either way.

[buluoray]

Three of the five red lanes are pre-existing main breakage, not this PR

Recording the attribution so the next reader does not spend the round I just spent, and
correcting something I said earlier in this thread.

What actually fails

Backend Tests (3.12, 3) — 9 failed, 21943 passed. All nine are in two classes, and all
nine assert on the git-push security deny-regex:

• test/test_push_branch_gate.py::TestUnrecognisedOptionsReadProtectively (6)
• test/test_security.py::TestGitPublishSubshellGluing (3)

e.g. 'git push origin @(main)' was not read as a wildcard shape, and
assert 'git-publish-push-bare' in frozenset().

Backend Tests (Windows) (3) is the same shard failing the same way. Coverage Gate's failing
step is "Require upstream coverage jobs to have succeeded" — a downstream aggregator failing
closed because the backend job failed, not an independent finding.

Proof it is not this diff, by tree identity rather than inference

This PR changes 52 files and zero Python; git diff over test/ and src/kiro_crew/ between
the merge base and this branch is empty. CI runs the PR merged with current main, so the
decisive check is what that merge's Python tree actually is:

merged-tree = 953f358a0951503d9d012eb6e83911b3edc4c93b

test            main=8272386b9368a9820bcd55c980ac94b1fa2fd7ca  merged=8272386b93…  IDENTICAL
src/kiro_crew   main=2fc6cdc8f3d0cf0e51126da333da40e6e43061d9  merged=2fc6cdc8f3…  IDENTICAL

The merged tree's Python is byte-identical to origin/main's. This lane is running main's
Python, so the nine failures are main's.

Where main's breakage came from

Two commits landed on origin/main after this branch's merge base (159a9fbe), both on exactly
this code:

• eaa8a45bb fix(security): model publish option arity so the floor tag holds (#7808)
• 166ff5acc fix(security): extract shell payloads glued to a -c cluster (#8197) (#8491)

Two independently developed fixes to the same gate, and they do not agree. Five open PRs are
already reconciling it — #8719 names the collision outright (reconcile #7356 and #7808 in the
git-publish floor), alongside #8721, #8727, #8712 and #8672. Nothing for this PR to do; fixing
a Python security gate from a frontend scroll PR would be the wrong blast radius.

Two open PRs (#8730, #8728) still show this lane green — the ordinary stale-green artifact: a
PR's checks are not re-run when main advances, so anything that finished before the breaking
merge keeps its green until its author pushes again.

Correcting myself

Earlier in this thread I called the Windows red "a confirmed pre-existing xdist flake, same lane
as #8516." The conclusion (not from this diff) was right; the mechanism I named was wrong —
this is deterministic, not a flake. The tell was there and I read past it: the same shard
number failed on both Linux and Windows with the same assertions, which is not how a
concurrency flake behaves. I had also been unable to read the log at the time and leaned on a
prior-PR memory instead of waiting for the evidence. The reread is above.

Where that leaves the PR

Of five red lanes on f65cd780b: three are the one main breakage above, and two — GPT 5.6 Review and the PR Readiness gate that follows it — are the coverage-judgement decision handed
to the repository owner in the previous comment. No code pushed for either.

[buluoray]

GPT 5.6 blocking findings — fixed on 2cfe53d13

Both adjudicated. One was real and is fixed at the cause; the other's reachable half closes
with that same fix. Also rebased onto origin/main (c791f0f1d), which clears the three
backend lanes that were inherited from main's push-gate breakage.

BLOCKING — client-only rows trigger an unbounded fetch — fixed

Correct, and the remedy was this file's own predicate rather than anything new.
slotCoverageShortfall decided which rows it could compare with
transcriptTsMs(r.ts) === null, a home-grown stand-in for "is this row on the server".
chatSlice.ts already answers that question properly, fifteen lines below and two consumers
deep:

const CLIENT_ONLY_ROLES: ReadonlySet<string> = new Set(['queued', 'streaming', 'thinking', 'permission'])
function isDurableRow(m: { role?: string }): boolean { return !CLIENT_ONLY_ROLES.has(m.role ?? '') }

The timestamp stand-in caught streaming — the one role the original device probe surfaced —
and missed queued, thinking and permission. Any of those with a readable ts was counted
as outside the window, and because the server never writes them, no wider read could ever
bring one back: a permanent shortfall, so every switch into a slot holding a queued
message or a permission card refetched the entire transcript. Same harm class as the 303 →
6,265 measurement in the description.

The comparison now filters on isDurableRow and a readable ts, kept as two independent
tests because they answer different questions — can the window contain this row at all, versus
can the row be placed so its identity key is whole. isDurableRow's parameter was widened from
ChatMessage to { role?: string } so the coverage row shape can ask the same predicate;
behaviour for its existing callers is unchanged, since ChatMessage.role is always a string.

Fixing that exposed a second, smaller defect in the same function, which the mutation run found
rather than review: the empty-window decline returned cached.length, billing the reader for
rows the server was never holding. It returns the comparable count.

BLOCKING — reused message ids can falsely prove coverage — reachable half closed

The realistic way two rows share a mid is the one this file already documents at
chatSlice.ts:2613 — "a client-only row can carry a mid too". With client-only rows filtered
off both sides, that source is gone.

What remains is duplicate mids among durable rows: the server stamping two persisted rows
with one identifier. That is not reachable from this client — the identifier is the server's own
per-row stamp — and closing it would be the fourth revision of this identity rule in five
rounds. Recorded as declined-with-reason rather than fixed. If this verdict re-raises only that
residual on the new SHA, it will be answered with a human /ai-review override rather than a
fifth revision, per the escalation already agreed on this PR.

The two non-blocking findings

• font:9px — unchanged rebuttal, third time, same evidence: no minimum-text-size rule
  exists in website/AGENTS.md, AGENTS.md, docs/system-specs/common/code-style.md,
  website/docs/, docs/ or AUTOSDE.yaml (zero matches), WCAG sets no minimum, and 9px is
  shipped precedent in three places including apps/issue-radar/WelcomeCarousel.tsx.
• Colour literals ignore the active theme — declined with reason. The inspector is a dev
  overlay that assigns its own cssText and is deliberately theme-independent so it stays
  legible above any theme, including a custom pack. A diagnostic that becomes unreadable under
  the theme being debugged cannot do its job. Off by default, and off for everyone who does not
  enable it.

Verification

• npx tsc -b — 0 errors.

• eslint --max-warnings 0 on both changed files — 0 findings.

• 32 test files / 922 tests green, run after the rebase, covering chatSlice.*, src/store/
  and ChatPane.hydrateBound (the other consumer of the widened predicate).

• Mutation-verified, each reverted immediately:

  mutation	result
  drop isDurableRow from the comparison	6 tests red
  decline over cached.length instead of the comparable count	1 test red
  remove permission from CLIENT_ONLY_ROLES	2 tests red

  The first probe of this reported all three GREEN. That was the probe lying, not the guards
  failing — it parsed vitest's summary with a regex that did not survive the ANSI escapes, so a
  red run read as no match. Re-run with the escapes stripped, and each mutation reds exactly the
  cases it should. Worth stating because a green mutation is normally a signal that a test is
  worthless, and here it was a signal that the measuring instrument was.

[buluoray]

GPT 5.6 on bf867e004 — the new blocking finding is fixed

The previous round's coverage finding stayed fixed; this round raised a different
mechanism, in a different file, and it was right.

BLOCKING — clearing restore ownership early can erase the saved anchor — fixed

Verified against source before touching anything, and the source is unambiguous. Two facts
had to line up:

1. Both writers of the persisted anchor gated on pendingRestoreRef.current — the debounced
   save, and the leave flush. That ref goes false at useVirtualChat.ts:3446, the moment the
   anchored row is located, which is before restoreAnchor writes the scroller and long
   before the settle stops correcting. In that window the geometry is ours, not the reader's,
   and neither writer was refusing it.
2. The at-bottom → clear branch sits above the intent gate
   (sinceHard > ANCHOR_SAVE_INTENT_WINDOW_MS). So the gesture revocation restoreAnchor
   performs — which its own comment says exists so "our own placement cannot be persisted as
   if it were a reading position" — protects the SAVE path and never reaches the CLEAR path.
   That is deliberate and pinned by an existing test (clearing at the bottom stays
   unconditional), so the remedy could not go there.

Together: a restore that lands at or near the end clears the anchor it had just restored, and
the next entry, finding none, takes the default pin to the bottom. This is the same invariant
class as the rest of this PR — an automatic action must not authorize itself — and it is
the fourth site in this file where our own write was being read as the reader's intent.

restoreOwnsPosition() (settleGate || pendingRestore) already existed for exactly this
question and was being asked only by the auto-pin path. Both anchor writers now ask it too, and
the predicate moved up beside the two refs it reads so the leave flush — the earliest caller —
can reach it; a caller that cannot reach it reaches for pendingRestore instead, which is the
defect. Its docstring said "must refuse an automatic bottom pin", which is how two
anchor-writing sites came to use the narrower ref; it now states that every persisted-anchor
write asks the same question.

This may also be what the device readout in this thread was showing: LEFT STORE.CLEAR,
then ENTER STORE.load … ABSENT — a saved anchor cleared, and the next entry opening at the
bottom.

BLOCKING — duplicate message ids — unchanged, and this is the residual already dispositioned

The reachable half closed with the previous round's isDurableRow fix: the realistic way two
rows share a mid is the one this file documents at chatSlice.ts:2613, "a client-only row
can carry a mid too", and client-only rows are now filtered off both sides.

What remains is the server stamping two persisted rows with one identifier, which is not
reachable from this client. The suggested remedy this round (require timestamp corroboration
for mid matches) is the same remedy declined last round for the same reason, and adopting it
would reintroduce a defect this PR already fixed: the server rewrites ts between the two
reads, so pairing mid with a timestamp is precisely what made one message look like two.
That was measured, not reasoned — it is why the match is on mid alone.

Per the escalation agreed earlier on this PR, this residual is answered with a human
/ai-review override rather than a fifth revision of the identity rule, once this verdict
re-reads the new SHA.

Verification

• npx tsc -b — 0 errors (it caught the declaration move: const does not hoist, so the
  ordering is compiler-enforced rather than test-enforced).
• eslint --max-warnings 0 on both changed files — 0 findings.
• 27 test files / 303 tests green across useVirtualChat.*, anchorDualIdentity,
  FollowController, ScrollAnchorCache, pinnedPrompt.jumpAnchor and the scroll-shell
  render test.
• Mutation-verified, each reverted immediately: reverting the debounced save to
  pendingRestoreRef.current reds 1 test; reverting the leave flush reds 1 test.

The guard is asserted against the source, deliberately: the gap is a frame-ordering one that
the settle crosses in rAF callbacks jsdom does not run, so a behavioural test in this harness
would pass against the defect. A candidate second test — that the declaration precedes its
earliest caller — was written and then removed: its failure mode is a compile error, so it
would only have restated what tsc already refuses.

[buluoray]

/ai-review override gpt bf867e0: a wire-supplied meta.mid cannot reach a stored row -- the only ingress strips it and the server mints its own -- so the crafted-duplicate premise is unreachable, and the suggested corroboration reintroduces a measured defect this PR fixes.

Recording the evidence for the override, having traced the finding's premise through the backend rather than answering it from the frontend comment I cited last round (which said the mid is "the server's own per-row stamp" -- true in effect, but not because the field is unwritable).

mid is minted server-side, and a supplied one is only honoured from a trusted caller.
dashboard/state.py:4267 mints mint_row_mid() for any row that arrives without one. A row
that already carries meta.mid keeps it — so the question is whether an untrusted caller can
construct that row.

Every meta that reaches slot.append is built with server-chosen keys. The steer path
(chat_delivery.py:497) builds {"steer": True, "steerState": …} and stores the one
client-minted value it accepts under sendId, length-bounded (chat_delivery.py:66) — not
under mid.

The single path where a whole meta dict arrives off the wire strips mid explicitly, and
its comment gives the same reason this finding turns on — remote_relay.py:387:

Keep the durable tool correlation (tool name, input, output, call id) the peer stored, but
DROP its mid: that is a per-gateway row delivery id, and adopting the peer's would collide
with the local mid space — slot.append mints a fresh local one when none is supplied.

So the id-space collision this finding needs is already refused at the boundary, by design,
before any coverage comparison sees a row.

And the suggested remedy would reintroduce a defect this PR fixes. "Corroborate IDs with
role and timestamp" is what the identity rule used to do. The server rewrites ts between the
two reads (chatSlice.ts:3775 handles that transition explicitly) and the role moves
streaming → assistant when a reply finalises, so pairing mid with either made ONE message
look like two — a false shortfall, and an unbounded refetch. That was measured on a device, not
reasoned: 303 → 6,265 rows. It is why the match is on mid alone, and why the reachable half
of this finding was closed instead by filtering client-only rows off both sides
(isDurableRow), which is what the previous round shipped.

The residual — a server stamping two persisted rows with one id — is a backend invariant, not
something this comparison can defend against, and it is tracked as the known limitation already
recorded in this PR's thread rather than patched here. Fifth revision of this identity rule
declined per the escalation agreed earlier on this PR.

The non-blocking font:9px finding is unchanged and rebutted on the same evidence as the three
previous rounds: no minimum-text-size rule exists in website/AGENTS.md, AGENTS.md,
docs/system-specs/common/code-style.md, website/docs/, docs/ or AUTOSDE.yaml (zero
matches), WCAG sets no minimum, and 9px is shipped precedent in three places.

[github-actions]

Human judgment recorded

@buluoray marked the gpt AI finding as false positive, not applicable, or explicitly accepted for bf867e004355e71b0c50159b89e94d4207f120e2.

a wire-supplied meta.mid cannot reach a stored row -- the only ingress strips it and the server mints its own -- so the crafted-duplicate premise is unreachable, and the suggested corroboration reintroduces a measured defect this PR fixes.

This decision applies only to this commit. A new push requires a new judgment.

[buluoray]

GPT 5.6 on ff23c7d87 — the alt-identity de-dupe finding is fixed

The duplicate-mid residual is gone from GPT's list on the previous SHA, so that is settled.
This round raised a different, real defect — and it is the second time in this PR that a
hand-rolled copy of an existing predicate silently outranked the real one, so the fix removes
the copy rather than teaching it about one more field.

BLOCKING — a save whose only change is alt was discarded — fixed

There are two de-dupe layers, and only one of them knew about alt.

ScrollAnchorCache.anchorWriteChangesState — the storage layer — reads it correctly:

if ((prev.alt ?? '') !== (next.alt ?? '')) return true

But the debounced save never got that far. The in-memory layer in useVirtualChat.ts kept a
formatted string and compared that:

const state = `${a.key}@${Math.round(a.top)}`
if (saved?.session === scheduledSession && saved.state === state) return

alt is not in it. So a write that changed only the lead identity returned early and
saveScrollAnchor was never called — the storage layer's correct comparison never ran.

The harm path is the exact case the dual identity exists for, and this PR's own description
states the premise: neither end of a row is stable alone. An older-page prepend regroups the
row and renames its lead while the tail id and the offset stay put → that write is
swallowed → the stored alt is now stale → a later append renames the tail → neither
identity resolves → entry falls back to the bottom pin. Carrying two identities buys nothing if
a change to one of them cannot be persisted.

anchorSavedStateRef now holds the last ScrollAnchor (with null meaning cleared) and asks
anchorWriteChangesState, so both layers answer the question the same way by construction.
That also removes a quieter disagreement: the string used Math.round(top), which flips on a
0.5px move, while the storage layer uses a 1.5px epsilon — the two layers had different ideas of
"unchanged" as well.

The pattern, stated plainly

This is the same shape as the isDurableRow round earlier in this PR: the correct predicate
already existed, a second hand-rolled spelling guarded a different call site, and the weaker one
was the one that ran first. Both are now single-definition. The restoreOwnsPosition round was a
third instance of the same class — a narrower ref consulted where the real predicate belonged.
Worth naming because it is a review-visible, author-invisible defect class: each copy looks
locally correct.

Verification

• npx tsc -b — 0 errors. eslint --max-warnings 0 on both changed files — 0 findings.
• 28 test files / 326 tests green across useVirtualChat.*, ScrollAnchorCache,
  anchorDualIdentity, FollowController, DebugToolsTab, scrollInspector and
  pinnedPrompt.jumpAnchor.
• The new guard is behavioural, not a source assertion: it drives the real hook through a
  scroll settle, changes only getAltId, settles again, and requires the second alt to reach
  storage. Mutation-verified — restoring the key@top string comparison reds exactly that test
  and nothing else.

Coverage Gate went green on the previous SHA with the tests added for it (ScrollAnchorCache.ts
72.2% → 96.2%, DebugToolsTab.tsx 62.5% → 100%), so that lane is settled too.

[buluoray]

c6fc3e483 — GPT clear; Design's accessibility finding fixed, with one correction to its framing

GPT 5.6 is ✅ no blocking findings on ff23c7d87. The alt-identity de-dupe fix cleared it
on its own, so no override was needed and the duplicate-mid residual is off its list. Both
blocking lanes are now clean without a human override.

Design CONCERNS — real, and it is this PR's regression — fixed

The finding: "real gesture" now means wheel/touchmove only, so a reader who reaches the
top by scrollbar drag, PgUp/Home or space never authorizes automatic older history and is left
with the manual Earlier bar. Verified at the registration site — the vocabulary was exactly
two events, and keydown/pointerdown appeared nowhere.

One correction to the framing, because the split changes who owns which half. Against the
merge base c791f0f1d:

path	at merge base	on ff23c7d87
top sentinel	sawInput: !vGetFollowRef.current() — any scroll-up authorized it	expiring gesture window
the walk	one-way sawRealInputRef, fed by the same two events only	same window

So the sentinel half is this PR's regression, as stated. The walk half was already
keyboard-blind at the base — the latch it used had the identical two-event vocabulary. Design
attributes the whole thing to this PR; half of it predates it. That does not reduce the fix,
which repairs both halves at once, but the record should be accurate.

The remedy is the first one Design offers, not the second. Restoring !follow is not
available: !follow is true after any scroll-up including our own, and that is precisely the
self-authorization this PR removes. Nor is "state the manual-bar fallback as the intended
contract" acceptable — keyboard navigation is the accessibility path, and quietly demoting it to
a manual button is a functional loss, not a contract.

So the vocabulary widens to four events, bound to the scroller:

el?.addEventListener('wheel', noteInput, { passive: true })
el?.addEventListener('touchmove', noteInput, { passive: true })
el?.addEventListener('keydown', noteInput, { passive: true })
el?.addEventListener('pointerdown', noteInput, { passive: true })

This does not loosen the window's premise by even a little. The premise is that writing
scrollTop cannot authorize the next fetch, and a scrollTop write fires none of these four —
only scroll, which is still excluded and still pinned by the existing assertion. What changes
is only the set of humans the window can hear.

keydown is bound to el, deliberately not to document: a document-level keydown would let
typing in the composer authorize a history fetch, which is the same category of mistake as
reading our own scroll write as consent. Teardown removes both new listeners alongside the
original two.

Verification

• tsc -b 0 errors; eslint --max-warnings 0 on both changed files, 0 findings.
• 90 test files / 913 tests green across ChatPage.*, olderHistory*, pagination*,
  useVirtualChat.*, ScrollAnchorCache and chatPins.
• The existing design-rationale test was extended, not rewritten: its property (the stamp is
  refreshed by real input, never by scroll) is untouched and still asserted; the vocabulary it
  pins now requires all four events, with the reason recorded inline. Its 400-char scan window
  was widened because the new registrations sit past it.
• Two mutations, each reddening only what it should: deleting the keydown registration reds 2
  tests; re-binding keydown to document reds exactly the scope test.

Disclosed in the PR body under One behaviour change worth naming — it is a deliberate
narrowing of who may authorize, not a bug fix, and it belongs in the description rather than
only in a comment.

Still the user's call, unchanged

• First Principles CONCERNS — the one-toggle debug-tools tab (dispositioned in
  #5551253575): keep
  the tab, or move the card into FeaturePreviewsTab.tsx. The mechanical claim holds; what
  argues against the move is that module's own retirement contract, which a permanent diagnostic
  never satisfies.
• The forcepin 2 Hz no-op, proven pre-existing at c791f0f1d.