fix(chat): load older history only when the reader is at rest

[buluoray]

Problem / Motivation

Reading back through a long chat transcript on a phone, two things went wrong.

History arrived unasked: pages loaded while the reader was still climbing, and the
"Load earlier messages" control counted as visible while it was still two screens
above the viewport — so the reader never saw the thing that offered the load.

And each landing moved the transcript under them. The position always ended up
right (read back after the write, the anchor row lands within a pixel of where it
was captured) while the motion was wrong, so every attempt to make the
correction more accurate failed: accuracy was never what was broken.

Why it matters

This is the reading path for every long conversation, and the failure is on the
surface a reader spends the most time on. Losing your place mid-sentence while
scrolling back is not a cosmetic defect — it makes long history effectively
unreadable on a phone, and it fires on every page, not occasionally.

What changed (motivation → approach → change)

Symptom → cause → change, for each of three defects.

1. History loaded while the reader was moving

The affordance check was passed a 2000px lead, so the bar counted as on screen two
screens early. The timing gate required a scroll event within the last 1500ms —
i.e. it deliberately loaded during the climb.

A programmatic scrollTop write during an iOS fling perturbs a curve the
compositor owns, not the main thread, so the write cannot be made invisible while
the surface is moving. Rest is the only state where a landing cannot be felt — the
clearest evidence being that at the very top of the transcript loading never
bounces, and the very top is exactly where a fling cannot persist.

Lead is now zero, and the gate is inverted: 100ms of quiet. Motion also aborts a
page already in flight, because the gate is checked when a fetch begins — a
reader at rest who flings during the round trip would otherwise take the landing
mid-gesture. One half refuses to start, the other refuses to finish.

100ms is matrix-react-sdk's number for the same problem, reached independently.
Every estimate-based virtualizer that ships a chat list lands here: react-virtuoso
carries an iOS-only glitch report for attempting it live, virtua's README states
the user must release the scroll, TanStack Virtual has a code path named for
deferring the adjustment on iOS.

2. The anchor correction created the displacement it repairs

Three distinct defects, each found from a device reading rather than from code
review.

It corrected rows that had not moved. The write is now gated on the anchor
row's measured on-screen displacement, not only on the derived delta. On one
entry frame the displacement was 0px both times while the delta asked for −1016
and then +1448, and both writes were painted.

It built on an abandoned basis. A restore that gives up re-arms follow and
takes the default placement, which moves scrollTop — so a capture straddling it
is priced against a layout that no longer exists. That capture is now dropped, and
the flag is consumed by the first consume rather than latched, because an
authorization that can only turn on is not an authorization.

It credited its own writes to the reader. The correction isolates the content
term by adding back the scroll drift since capture, on the premise that the drift
is a finger. One frame carried repriced 3078px in 7 writes inside that window,
so the drift added back was our own reprice and the same content was compensated
twice — a 1px displacement authorising a 267px write. The drift is now split, with
our own writes accumulated at the one chokepoint every write already passes
through, and read as a difference against the capture rather than as an absolute
(a cumulative counter used absolutely drifts further off the longer a session
stays open, which no short test would surface).

Deliberately not clamping the write to the measured displacement, which looks
equivalent and is not: with content up 8360 and the reader down 500, writing 8360
preserves their scroll and clamping to 7860 erases it.

3. One percent of rows carried 69% of a page's bytes — now #9156, not this PR

A file-change before/after pair ships both full texts, so a large edited file
travels twice at full size. That shrink is real and measured (5.50MB → 1.44MB per
page) but it is not in this diff — it moved out on Design Review's
recommendation, and the section below records the split. No backend file, no
_unified_patch, and no new pytest module is present here; grep either name at
this head and you get nothing.

Kept in this PR is only the diff-row behaviour that shares files with the scroll
work: an open diff row survives a warm swap instead of collapsing and losing the
reader's place.

Tests

tsc -b clean, eslint clean on every changed source file, 1904 frontend test
files / 30,014 tests green (full suite, because this PR touches shared components
where a local run has already proved insufficient). No backend gate is claimed:
this PR contains no Python.

Every new guard was mutation-verified — the fix removed, inverted, or degraded,
and the test confirmed red before restoring. What each locks in:

• anchorCorrection.test.ts (5 → 9 cases): that the scroll drift is split between
  the reader and our own writes, and that the write total is read as a difference
  rather than an absolute. Mutations: not subtracting our writes → 3 red; wrong
  sign → 3 red; absolute instead of difference → 2 red; dropping the reader term →
  6 red. The five pre-existing cases now pass capturedWriteSum: 0 explicitly,
  because "nothing else wrote the scroller in this window" is an assumption each
  reading was taken under and a default would hide it.
• olderHistoryDistanceTrigger.test.ts: zero affordance lead, stillness as the
  permission, a floor on the quiet threshold, and that motion aborts an in-flight
  page.
• scrollInspectorWiring.test.ts: that the write accumulator is fed at the one
  chokepoint and before the write, and the two conditions that stop the corrector
  moving a reader who has not moved.
• scrollInspector.test.ts: that the residual is sourced from the corrector rather
  than from the spacer, and that no live line exceeds the width the device shows.

Two of these are source-level pins rather than behavioural tests, and that is a
measured limit rather than a shortcut: a test cannot move the scroller
between an anchor's capture and its consume, because act(() => rerender(...))
flushes layout effects synchronously. Two earlier attempts to test it through the
harness passed with the correction removed and with its sign inverted. The gap is
recorded in the test file.

One i18n key (pages.chatPage.loading_earlier_messages) is deleted from all 12
catalogues rather than left dead, so the dead-key ratchet holds at 28 instead of
being raised to 29.

Manual verification

Every number in this description was read off an iPhone against a dev overlay, not
inferred — including the ones that refuted my own explanations. Three separate
readings each overturned a hypothesis I had already written down:

• hold=yes on the device disproved "iOS has no scroll anchoring". WebKit has
  shipped it (bug 307734), so the browser is now a second controller here.
• A frame with spacer 0->0 while 8,360px of content arrived proved the residual
  instrument was reporting a fake defect on a pixel-perfect landing.
• off = −S, not off = G − Δ: an identity I got wrong, which had me claim two
  writes were "pure error" when the reading only showed scrollTop drifting.

Desktop behaviour is recorded below, on a pod running this branch.

Screenshots / video

Desktop, 1440x900, 27s. The pauses are the feature, not padding: the gate wants
the affordance genuinely on screen and the scroller quiet, so a clip that
scrolled continuously would show nothing loading at all.

Three pages land. Measured by the scenario itself, from the scroller:

	transcript height	scrollTop
start	11,624px	10,907
after page 1	22,672px	11,164
after page 2	34,368px	11,548
after page 3	45,428px	11,243

The reader reaches the top each time, roughly 11k of content arrives above
them, and scrollTop advances to match — so the height grows 3.9× while their
position reading barely moves. That is the compensation doing its job.

Recorded against an isolated pod running this branch, seeded with 500 synthetic
messages of deliberately uneven height: short replies, walls of prose, code
blocks, lists. A transcript of uniform rows is the one case a height-estimating
virtualizer gets right by accident — every estimate is correct, so no correction
is ever needed and a recording of it proves nothing.

Scrolling is driven by mouse.wheel, not by assigning scrollTop: an assignment
would bypass the very event path the gate is written against.

recorded from b464e39a450448b246720d68f9312eab6df44f8f · fix/older-history-loads-at-rest · pod kirocrew-wt-diffrow · real server and real paging path, no fixtures or mocks

Related Issues

N/A — found and reproduced from device use rather than from a filed report.

Pattern harvest

Rule candidate: review-prompt

Pattern: a derived figure corroborated only by another figure computed from the
same capture is not corroborated.

This defect class produced three of the failures above and cost most of the time.
delta and owed disagreed by only 246px, so they read as confirming each other
while both were priced against an abandoned capture — and res, the difference
between them, looked small, which is exactly why watching the residual could
never have found it. The same shape appeared in the instrument: a residual derived
from the top spacer agreed with itself across landings while reporting 8,360px of
defect on a landing the corrector measured at zero, because the spacer is a proxy
for "content above the reader" that stops moving once the window reaches the start
of the list.

The rule that falls out: when a correction checks itself, the check must come from
an independent source — for a position, that means a readback from the DOM, not
a second arithmetic path over the same inputs. Worth a review prompt because it is
invisible in a diff (both figures look carefully computed) and it fails toward
confidence rather than toward an error.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — the findings, the device readings they came from, and the fix each points at are recorded in useVirtualChat.ts next to the code they govern
• No secrets, credentials, or internal references in the diff

Not in this PR

The above-fold reprice still converges rather than corrects on the landings where
it runs — a run of writes measured at +8, +5, +4, +3, +2, +1, +1 inside 100ms,
which is two controllers settling against each other now that WebKit ships scroll
anchoring and the browser is doing this too. It reads as already-loaded content
shifting slightly instead of standing still. The finding, the readings, and the fix
it points at are recorded in useVirtualChat.ts: one mechanism corrects the
position at a time, and changing which one deserves its own change with its own
device pass.

Disclosure: relandConvergedAnchor

Added after First Principles Review correctly noted this ships undeclared.

A restored reader was put at a stated target — row X, top px below the viewport
edge — so when rows above it are repriced, scrollTop += delta is the wrong answer:
that keeps a reader still relative to content, which is what someone who
scrolled there themselves wants, not someone placed on an absolute mark. This
re-solves the absolute placement instead: read where the row is now, correct by the
difference.

On the "one mechanism corrects the position at a time" invariant this PR leans on —
it upholds it rather than adding a fourth competitor. It returns true only when it
owns the correction, and the caller's relative compensation stands down for that
batch; the two are mutually exclusive by construction, and it applies once per batch
rather than on a loop, which is what keeps it out of a tug-of-war with the
compensation it replaces. It declines whenever it cannot be sure — no memory, a
different session, the reader has taken over, the row is unmounted, a degenerate
rect — and the existing behaviour stands.

Fair hit that the description omitted it: a mechanism that writes scrollTop should
be named in a PR whose whole argument is about who is allowed to write scrollTop.

Split: the payload shrink moved out

On Design Review's recommendation, the oversized-diff-pair change is now
#9156 instead of riding here.
Design was right that it shares no file with the scroll work, so this PR's
"cannot be split" rationale never covered it — and bundling them meant reverting a
scroll regression would also revert the transcript wire shape.

The three blocking defects GPT 5.6 found in that component are fixed there, not
deferred: the symlink window in the snapshot read, the unbounded read feeding
difflib, and the gate that let a change past the cap disappear from the UI.

What stays here is the scroll work and the diff-row behaviour it shares files
with: a row keeps the contents it opened with when the payload is replaced
underneath it, survives a warm swap instead of collapsing, and withholds its
disclosure control when there is nothing to disclose.

Riding along, declared

First Principles is right that these were in the diff without being described. They
are named here rather than removed, because each is in a file the scroll work
already changes and each was found while measuring it:

• Tapping a disclosure no longer shoves the tapped header off screen. Same
  anchor machinery as the scroll fix, same measurement rig.
• An expanded Pierre diff skips WarmSwap. This changes every
  PierreFilePair consumer, not only chat — the widest-blast-radius item in the
  PR. It removes the perceived expand animation, which is a known open
  regression, tracked and deliberately not fixed here: restoring WarmSwap
  trades the snap for a blank row, because inside the warm box the surface is
  absolute inset-0 … invisible and windows against a hidden parent-sized
  viewport, so it renders no rows and nothing invalidates the measurement on
  reveal. PierreCode already carries the same exemption for whole-file surfaces.
• Open rows survive a slot-list refresh, and ~400 lines of dev-only overlay
  instruments, off by default and asserted to cost nothing while off, carry the
  device readings every number in this description comes from.
• relandConvergedAnchor writes scrollTop. In a PR whose own comments say
  one mechanism corrects the position at a time, this is a third writer and needs
  saying: it is mutually exclusive with the relative compensation it replaces —
  the converged path runs instead of that write, not alongside it — so it does not
  make a fourth concurrent mechanism.

Two deletions the coverage gate forced

• src/lib/scrollQuiet.ts is deleted, not retained. This PR removed its last
  production consumer, which dropped the file to 29.4% and below the per-file
  floor; the gate's own message says to add tests rather than extend the baseline,
  and adding tests for a module nothing calls is the wrong answer. Its
  device-measured rationale — the hold was 1.95s of a 2.1s wait against a 0.16s
  request — now sits next to the mechanism that took the job over, on the trigger
  side. The feeder call that stamped a timestamp on every user scroll goes with it.
• A stale duplicate comment in chatSlice.ts that still described the landing
  buffer as present, stacked directly above the one explaining its removal.

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

All evidence gathered. The blind read couldn't open the sole artifact (4MB GIF, same limit I hit), the diff's user-visible changes are the silent-at-rest history load, the removed loading overlay, the withheld chevron on no-op diff rows, and the declared expand-animation regression. Composing the review.

UX-Verdict: CONCERNS

Solid scroll-reading fixes, but the only visual evidence is a 4MB GIF nobody's blind read could open, and one declared animation regression ships app-wide.

Watch

• Every expanded diff row across the app now snaps open with no transition (PierreFilePair returns impl directly, skipping WarmSwap) — declared as a "known open regression" with a stated tradeoff (blank row otherwise). High frequency × minor friction × every open; a human should explicitly accept shipping it.
• The manual "Load earlier messages" click now gets dim-only feedback: the overlay spinner is deleted and the bar's busy state is opacity-50 with the label unchanged, so a slow page reads as a disabled button, not work in progress. Smallest fix: swap the button label to the just-deleted "Loading earlier messages…" string while loading.

Evidence gaps

• The blind read did not run: shot-01.gif exceeded the reader's size limit ("This image would not open — the file is too large"). Re-commit a smaller GIF/webm plus static PNG frames so the cold-read lane can adjudicate.
• The withheld chevron (row where fc.before === fc.after renders a spacer instead of a toggle) appears in no screenshot — one PNG of a no-op diff row beside a normal one would close it.
• The diff-row open handoff (fallbackHeader bridge, and the expand snap) has no recording; the committed GIF covers only the older-history landing.

[UX-REVIEWED] c829837

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] c829837

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Root-cause fix with device-measured evidence: stillness gates the fetch at both ends, drift accounting stops self-compensation, and every trade-off is declared and argued.

The riskiest pieces were checked and hold up: the removed abort-on-scroll reappears at the new single gate chain (start refuses while moving, motion kills in-flight), the untestable capture-consume race is answered by extracting the arithmetic into a directly-tested pure function rather than by weakened harness tests, the WarmSwap removal is behaviorally coupled to this PR's press-rooted reprice rule (not scope creep), and the payload change that genuinely didn't belong was already split to #9156. Everything here is frontend behavior that reverts cleanly; the acknowledged residual risks (landing-buffer removal re-verified only on device, expand-animation snap) are declared in the PR with their fallback fixes recorded next to the code.

[DESIGN-REVIEWED] c829837

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of c829837c9faf10deb692378098fd332f092594a0 — 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.

All verification is done: the temp-screenshots/ GIF follows a documented repo convention (PR template + ux-review workflow consume it), EarlierMessagesBar really does carry its own loading state, _unified_patch is absent (defect 3 genuinely moved to #9156), and earlierAffordanceInView has exactly one live caller — which this PR changes to bypass the module's own documented default.

First-Principles-Verdict: CONCERNS

The zero-lead gate ships while pagination.ts still documents, defaults, and tests the lead it bypasses — two pins now record opposite decisions.

Not justified as shipped

9. no-chevron on identical before/after — rides along: not in the description's declared scope ("an open diff row survives a warm swap"); harm is named in the test.
10. dev-inspector instruments — rides along: ~250 source lines of off-by-default diagnostics in a PR framed as a fix.

What this change ships

Intent: make reading back through long chat history on a phone stop jumping and stop loading pages the reader never asked for — a FIX.

1. Older pages load only after 100ms of scroll stillness; motion aborts a page in flight — justified
2. Auto-paging requires "Load earlier messages" actually on screen; trigger distance is 2000px absolute — justified
3. The landing hold (scrollQuiet.ts) and the measure-farm gate are deleted; pages land immediately, including for a parked reader — justified
4. A tap no longer refills the history page budget; a gesture stream coalesces to one refill — justified
5. The transcript-level older-pages spinner is gone (key deleted from 12 catalogues); the manual bar keeps its in-place state — justified
6. Landing corrections compensate content only, gated on measured displacement; stand down for overscroll, restores, abandoned captures — justified
7. Opening a disclosure no longer walks the tapped header off screen — justified
8. An open diff row keeps its contents and header across payload swaps and slot-list refreshes — justified
9. A file row with identical before/after offers no disclosure chevron — rides along (undeclared)
10. Dev scroll-inspector gains ~10 off-by-default instruments — rides along
    (The change has more than 10 items; these are the 10 a person would notice.)

Watch

• earlierAffordanceInView has ONE live caller (grepped earlierAffordanceInView|EARLIER_ADMISSION_LEAD_PX: ChatPage.tsx:4820), which now passes 0 — leaving the leadPx default, EARLIER_ADMISSION_LEAD_PX = 600, and its rationale ("without a lead … the prepend happens IN VIEW … a real regression even with perfect compensation") with zero live consumers, while pagination.earlierAdmission.test.ts:78 still pins the constant > 0. The repo now maintains two contradictory spellings of one admission policy.
  Clears when: the default, constant, comment and the >0 pin are deleted or rewritten to the shipped zero-lead decision.

Subtractions

• Delete EARLIER_ADMISSION_LEAD_PX and the leadPx default (pagination.ts:154, 180) — 0 live consumers; make the sole caller's 0 the function's behavior and retire the stale pins in pagination.earlierAdmission.test.ts.
• Drop devOlderSpans/olderSpans (scrollInspector.ts) — its own doc says the second span it split against is gone, and devOlderLatency already reports the request half; one dev number, two spellings (1 feed each: chatSlice.ts, ChatPage.tsx).

[FIRST-PRINCIPLES-REVIEWED] c829837

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

The removals are clean — spinnerNearTop has no dangling references, loadingOlder is still correctly threaded to its remaining consumers, and the removed i18n key had only the one (deleted) reference.

The candidate list was empty. Working through the diff, the production-affecting changes (FileChangeChips content-pinning and nothingToShow gating, repriceAboveFoldDelta press exemption, the contentShiftFor correction, the overscroll/reland/giveup guards, the older-history trigger rewrite, the whenScrollQuiet removal, the Pierre fallbackHeader, and the disclosure-reset skip-on-null) are each accompanied by tests, and the bulk of the new surface (scrollInspector instrumentation) is gated behind inspectorOn()/enabled and never runs in production. I cannot ground a concrete input → call path → observable wrong outcome at the required bar on any changed line, and nothing meets the BLOCKING classes.

No findings.

[OPUS-REVIEWED] c829837

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

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

[buluoray]

Both GPT 5.6 findings are about code this PR does not contain. They belong to
c8d63c669 — feat(apps): app session controls (#7573) — which landed on main
after this branch's merge base, so it appears in a two-dot main..HEAD diff as
if it were mine.

Evidence, from this branch's worktree:

claim	check	result
ChatPage.tsx:5092 defines sessionControlKey	grep -c sessionControlKey website/src/pages/ChatPage.tsx	0 — the symbol does not exist in this branch; lines 5090–5094 are handleAddToContext
manifest.py:507 adds MAX_SESSION_CONTROLS_PER_APP	grep -c MAX_SESSION_CONTROLS_PER_APP src/kiro_crew/apps/manifest.py	0
the feature is in this PR's diff	git diff origin/main...HEAD | grep -c 'sessionControlKey|MAX_SESSION_CONTROLS_PER_APP'	0
apps/manifest.py is a changed file	gh pr view 9130 --json files	not present — 46 files, none under apps/

git rev-parse --short $(git merge-base origin/main HEAD) → a7bfe44c7, and
main is 6 commits ahead of that point. The two-dot diff therefore shows main's
additions as removals on this branch, which is where 86 files and "half a megabyte
of #7573" come from; the three-dot diff GitHub actually serves for this PR is 46
files and matches gh pr view --json files exactly.

So this is a diff-scope artifact rather than a security regression: the session
identity handling GPT describes is #7573's, already reviewed and merged, and
nothing in this PR reads, writes, or guards a session-control path.

The underlying cause is real and worth fixing anyway — a stale base makes every
reviewer, human and bot, read the wrong set of changes, and First Principles Review
independently flagged the same artifact (its item 10) while UX Review's blind read
landed on #7573's screenshots instead of this PR's. Rebasing onto current main
removes the artifact for all of them, and it is queued together with the squash
that PR Hygiene requires (this PR has 4 commits against a cap of 2). Both need a
force-push, so they are waiting on the author's go-ahead rather than being done
silently.

/ai-review override gpt 6a976bb: both findings cite #7573 code absent from this branch (grep returns 0 for sessionControlKey and MAX_SESSION_CONTROLS_PER_APP; apps/manifest.py is not among the PR's 46 files) — a stale-base two-dot diff artifact, and a rebase onto current main is queued.

[buluoray]

Disposition for the three CONCERNS verdicts. Each finding was checked against
source before being accepted; three of them are right and name defects I
introduced, and I have said so rather than arguing.

Accepted — real, and mine

1. Deleting the spinner left a press with no feedback. (UX)

Correct, and worse than the review states: I also deleted the string it used, so
the only remaining cue on a 2.6s page is the bar's 50% dim. The fix is the one UX
suggests — put pages.chatPage.loading_earlier_messages back and swap the bar's
label to it while loading. That also resolves it more cleanly than my deletion
did: the key stops being dead because it is used again, so the dead-key ratchet
holds at 28 without removing a string a reader needs.

2. A capped-patch row opened from a minimal chip degrades to a plain file view.
(Design)

Verified at website/src/components/FileChangeChips.tsx:639:

<MinimalChip … onClick={() => onOpenDiff?.(fc.path, fc.after ?? '', fc.before ?? '')} />

A patch-only row has no before/after, so this passes '' , '' and the diff
panel has nothing to diff. This is a regression my own change introduced — the
shrink removed the pair without following it into the open-diff path. Design's
suggestion (route fc.patch there and render PierrePatch) is the right shape.

3. whenScrollQuiet has zero production consumers while still being fed.
(First Principles)

Verified: grep whenScrollQuiet src/ | grep -v /test/ returns only its own
definition, yet noteUserScrollActivity() still runs at
website/src/hooks/virtualizer/useVirtualChat.ts:2569 on every user scroll. So
every scroll stamps a timestamp nothing reads — a real per-scroll cost for nothing.
The feeder call goes. The module itself is retained deliberately for its
device-measured rationale, and I will say so at the definition rather than leaving
the next reader to rediscover that it is intentional.

4. leadPx / EARLIER_ADMISSION_LEAD_PX now argue against the shipped
behaviour. (First Principles)

Verified: one production caller (ChatPage.tsx:6345), and it passes 0. The 600px
default and its rationale describe the coupling this PR removed, so they are not
merely unused — they are misleading. Both go.

5. relandConvergedAnchor is undeclared. (First Principles)

Fair. It is a position-writing mechanism in a PR whose own comments say one
mechanism corrects the position at a time, and the description never mentions it.
Disclosure fix, no code change — it will be named in "What changed" with what it
does and why it does not race the others.

Accepted — cause identified, needs a force-push

6. Half the diff is #7573. (First Principles item 10, and the root cause of both
GPT 5.6 findings and of UX's blind read landing on #7573's screenshots.)

A stale base. merge-base is a7bfe44c7 and main is 6 commits past it, so a
two-dot diff shows main's additions as this branch's removals. The three-dot diff
GitHub serves is 46 files and contains none of it, but the bots' evidence-gathering
clearly reads the two-dot view, and a base that misleads every reviewer is a defect
regardless of which diff is technically correct.

A rebase onto current main fixes it, and PR Hygiene independently requires a
squash (4 commits against a cap of 2). Both need a force-push, so both are prepared
and waiting on the author rather than done silently. The squash is verified to be
content-identical: git diff <current-head> <squashed-head> is empty.

Needs a decision, not a guess

7. The wire-format change does not share the "cannot be split" rationale.
(Design)

This is correct and I will not argue it. chat_runner.py, chat_utils.py,
FileChangeChips.tsx and pierre/index.tsx share no file with ChatPage.tsx or
useVirtualChat.ts, so the description's justification covers the scroll and
diff-card work but not the payload shrink. Reverting the scroll gating after a
device regression would currently also revert the transcript wire shape.

Splitting it out is a scope decision for the author, so it is in front of them
rather than decided here. Noting the tradeoff honestly: the shrink is independently
verified (5.50MB → 1.44MB, 16 new tests) and was written first, so a split costs a
second PR and a rebase but buys an independently revertable wire change.

8. Is 8KB the intended UX cut? (Design)

Design is right that _PATCH_INSTEAD_OF_PAIR_BYTES = 8_192 combined reaches past
the "~1% of rows" tail the description claims — a ~100-line file's edit crosses it.
The 1% figure was measured on real transcripts, but "1% of rows" and "any 100-line
file" are different claims and only the first is in the description. That is a
product call about where the cut belongs, not a bug, so it goes to the author with
the numbers rather than being tuned quietly.

Not accepted

The two GPT 5.6 blocking findings, rebutted with evidence in the comment above:
both cite #7573 code that returns 0 occurrences in this branch.

[buluoray]

The three new GPT 5.6 findings are accepted — all real, all mine, and the
sharpest part is that a sibling function in the same file already does both things
correctly, so there is no defensible reason mine does not.

F3 — symlink TOCTOU in the snapshot read

_safe_read_snapshot_raw (chat_runner.py:1373-1384) validates then reads:

validated = validate_file_path(path)
...
return p.read_text(encoding="utf-8", errors="replace")

_reconstruct_str_replace_before, ~60 lines below at :1447-1454, reads the same
class of agent-written file through the right primitive, and its own comment names
exactly the window I left open:

Read through hooks.safe_read_file — the symlink-safe chokepoint (re-checks the
RESOLVED target + O_NOFOLLOW open, closing the validate→read TOCTOU window;
AWS-33/AWS-62).

safe_read_file is already imported (:196). Accepted.

F2 — unbounded read and patch generation on the event loop

Same asymmetry. The sibling gates on size before reading and re-checks after
(:1440-1449, _MAX_RECONSTRUCT_BYTES = 2_000_000, with a comment explaining that
stat() races a growing writer). _safe_read_snapshot_raw has no cap at all, and
_unified_patch then runs difflib over both full bodies — on the turn coroutine,
one line above a save_slot_off_loop that exists precisely because this work does
not belong inline. Accepted, both halves: bound the read, and take the read and the
diff off-loop.

F1 — a change past the cap becomes invisible

The gate is entry["before"] == entry["after"]:

if (before_full is not None and after_full is not None
        and entry["before"] == entry["after"]      # capped prefixes identical
        and before_full != after_full):
    entry["patch"] = _unified_patch(...)

My own comment above it claims this is "computed only for exactly that case" —
which is the bug, not the justification. A large file edited both inside and
past _MAX_SNAPSHOT has differing prefixes, so no patch is computed, and the wire
carries a capped pair that does not contain the later edit. The reader sees the
first change and never learns the second exists.

The condition was written from the wrong question. It asks "are the prefixes
identical?" when the thing that matters is "can the capped pair represent every
change?" — which is false whenever either body was truncated at all. Accepted.

Where these get fixed is a scope question, not a technical one

All three are in the payload-shrink component, which Design Review independently
recommends splitting out of this PR on the grounds that it shares no file with the
scroll work and so is not covered by the "cannot be split" rationale. Design was
right about that, and these three findings make the case stronger rather than
weaker: the scroll changes are device-verified and clean, while the shrink now has
two security/availability defects and one display-loss defect to fix properly.

So the author decides whether they are fixed here or in the split-out PR, and I am
not making that call by starting to push code into a component that may leave. What
I will not do is leave the findings unanswered: they are verified above with the
sibling code that shows the correct pattern, and the fix for each is named.

No override is requested for any of these three.

[buluoray]

Two backend failures here are inherited from main, not produced by this PR

Both are outside this PR's diff, and one reproduces on a pristine origin/main
checkout with nothing of mine in it. Leaving them red rather than reaching outside
this PR to patch them — flagging for a maintainer instead.

1. test_security.py::TestIsSensitiveBashCommand::test_chained_cd_expansions_do_not_blow_up_the_gate

AttributeError: <module 'kiro_crew.security'> does not have the attribute '_dir_holds_sensitive_leaf'

main removed the symbol and left the test monkeypatching it:

ref	security.py defines _dir_holds_sensitive_leaf	test_security.py references it
origin/main (d4cb9afc7)	0	2

d4cb9afc7 — refactor(security): move the path fence to the layer that can hold it (#9089).

Reproduced on a detached worktree at origin/main, no branch of mine involved:

$ git worktree add --detach /tmp/probe origin/main
$ python -m pytest test/test_security.py
1 failed, 1500 passed, 1 skipped
FAILED test/test_security.py::TestIsSensitiveBashCommand::
       test_chained_cd_expansions_do_not_blow_up_the_gate - AttributeError

This PR's diff touches zero files matching security or hooks.py
(git diff --name-only origin/main...HEAD → 0 matches), and since a PR is tested
as a merge commit, the breakage arrives through the merge.

2. test_terminal_handler.py::TestTerminalWsIntegration::test_ws_bash_restores_an_inherited_prompt_command@pty_integration

assert tail.index(b"PREV_RAN") < tail.index(b"PC''NOW")
E   assert 93 < 58

An ordering assertion over pty output. test_terminal_handler.py passes in full on
a pristine origin/main worktree locally (340 passed, 1 skipped), and the case
carries an @pty_integration marker, so it only executes where a real pty is
allocated. This PR touches no terminal, pty, or websocket file. Not rerun — the
authority I am operating under forbids rerunning a job to dodge a failure, and
whether this is order-dependence in CI or a genuine main defect needs a
maintainer's read rather than a green retry.

Coverage Gate is a cascade, not a third failure

It fails closed because an upstream backend job did not succeed
(::error::backend-test=... -- failing closed). The per-file frontend floor it was
previously failing on — src/lib/scrollQuiet.ts at 29.4% — is resolved: that module
is deleted in the current head, since this PR removed its last production consumer
and the gate's own message says to add tests rather than extend the baseline. Its
device-measured rationale moved next to the mechanism that took over the job.

[buluoray]

Disposition for the BLOCK

The blocker is right, and it is the sharpest kind of finding: the description
claimed work that is not here.

Blocker — accepted, fixed in the description

Defect 3 was claimed as shipped and is not in this diff. Correct. That change is
#9156 — split out on Design
Review's recommendation earlier in this PR's life — and the description kept
describing it as delivered while also carrying a section explaining that it moved.
Two contradictory claims, and the review read the one that would have gone into the
merge record.

Worse than stated, in one respect the review could not check: the Tests section
claimed flake8, mypy --platform linux, isort, the black gate and "two new
pytest modules (16 tests)" for a PR containing no Python at all. Those gates
did run — on #9156. Reporting them here would have credited this PR with a green
backend it never had.

Both are corrected: section 3 now states plainly that the shrink is not in this
diff and points at #9156, and the Tests section says no backend gate is claimed.
The frontend numbers are refreshed to the current run (1,904 files / 30,014 tests).

Subtractions — both accepted, both verified first

devOlderSpans's holdMs. Verified: one production call site
(chatSlice.ts), constant 0. The parameter existed to tell the request apart
from the landing hold, and this PR deleted the hold — so it is not an under-used
reading, it is a number that can no longer be anything but zero. Removed, along
with the hold= span it rendered.

One caveat worth recording for the next reader: the overlay has two hold=
readings, and only one of them is this. env … hold=<yes|no> is the WebKit
scroll-anchoring behavioural probe and is untouched.

The test that pinned this was rewritten rather than dropped, because its intent
still holds — an instrument going silent already cost a round on this PR, so "no
hold" must stay distinguishable from "the instrument broke". It now pins the call
itself and asserts the second argument is absent, so re-adding a hold is a
visible edit:

expect(thunkBody()).toMatch(/devOlderSpans\(netMs\)/)
expect(thunkBody()).not.toMatch(/devOlderSpans\(netMs,/)

Mutation-verified: restoring devOlderSpans(netMs, 0) reddens it.

The HeightIndex.setMeasured comment naming a "content-aware estimator".
Verified: one hit in virtualizer/, the comment itself. It is the residue of a
calibrated estimator that was built, wired, and then fully reverted under this
branch's no-estimates rule — a height is computed or the code waits for it. A
comment asserting that every measurement teaches an estimator that does not exist
is worse than no comment, so it is deleted.

Watch items — now declared rather than removed

Items 7–10 were real fixes riding undeclared, and that was a defect in the
description, not in the code: each is in a file the scroll work already changes.
They now have their own section. The widest one is named as such — an expanded
Pierre diff skipping WarmSwap changes every PierreFilePair consumer, not just
chat, and it removes the perceived expand animation, which is an open regression
recorded with its three candidate fixes rather than quietly left out.

relandConvergedAnchor is declared too, with the part that matters: it is
mutually exclusive with the relative compensation it replaces, so it is not a
fourth concurrent position writer.

Also in this push

src/lib/scrollQuiet.ts is now deleted, which reverses what I said in the
previous round's disposition — I said the module would be retained deliberately for
its rationale and only its feeder call removed. The coverage gate then failed it at
29.4%, below the per-file floor, with its own message saying to add tests rather
than extend the baseline; writing tests for a module nothing calls is the wrong
answer, so the module goes and its device measurement moves next to the mechanism
that took the job over. First Principles item 3 was more right than I allowed.

Not addressed here

Two backend checks are red from main, not from this PR — one reproduces on a
pristine origin/main worktree. Analysis is in the preceding comment; leaving them
red rather than reaching outside this PR's scope to patch a subsystem it does not
touch.

[buluoray]

The three remaining red checks are one main defect and its cascade

Confirmed across both PRs this cycle, which is what settles it: this PR contains
no Python at all, and it fails the same test as the backend PR does.

Check	Cause
Backend Tests (3.12, 4)	test_security.py::test_chained_cd_expansions_do_not_blow_up_the_gate
Backend Tests (Windows) (3)	the same test — Windows shards split differently, so it lands in shard 3
Coverage Gate	cascade: ::error::backend-test=failure -- failing closed

The defect is in main, not in either branch. d4cb9afc7 — refactor(security):
move the path fence to the layer that can hold it (#9089) — removed
_dir_holds_sensitive_leaf from security.py and left the test monkeypatching it:

ref	security.py defines it	test_security.py references it
origin/main	0	2

Reproduced on a detached worktree at origin/main with nothing of mine present:

$ git worktree add --detach /tmp/probe origin/main
$ python -m pytest test/test_security.py
1 failed, 1500 passed, 1 skipped
FAILED …::test_chained_cd_expansions_do_not_blow_up_the_gate - AttributeError

And the cross-PR check, which is the part a single-PR view cannot give you:

• fix(chat): load older history only when the reader is at rest #9130 (scroll + diff-row) — git diff --name-only origin/main...HEAD matching
  security|hooks.py: 0 files. It contains no Python whatsoever.
• perf(chat): send an oversized diff pair as a capped patch #9156 (payload shrink) — same test, different shard number.

Two unrelated branches cannot independently break one security test they never
touch. Since a PR is built as a merge commit, both inherit it from the base.

Not rerun and not patched: rerunning would only re-import the same missing symbol,
and fixing it means editing security.py or its test, which is outside both PRs'
scope and belongs to whoever landed #9089. Flagging for a maintainer.

The Coverage Gate entry needs no separate action — it fails closed on the backend
job rather than on a coverage floor, and the per-file floor it was previously
failing (src/lib/scrollQuiet.ts, 29.4%) is resolved in the current head.

[bolichen97]

Rebased onto main 6fbb06bc by a maintainer as part of the 2026-09-08 open-PR audit. Clean rebase, no conflicts, no content changes to the PR.

Gates run locally in a scratch worktree:

• npx tsc --noEmit -p website/tsconfig.json - pass
• vitest on the 14 test files this PR touches - 231 passed, 1 skipped
• vitest on src/i18n (this PR edits locale catalogs) - 664 passed; the single failure in src/i18n/navLabels.test.tsx is a missing yaml dependency in my local node_modules, unrelated to this PR

Please review the rebase result. Because a maintainer pushed last, the repo's last-push rule means a second approver is needed. Reply here if anything looks wrong.

[bolichen97]

@buluoray This PR and #9156 overlap on website/src/components/FileChangeChips.tsx (audited at b464e39).

All three of that file's edits appear in both PRs in near-identical text: CollapsedRowHeader's optional onToggle? with the aria-hidden 16px chevron spacer, ExpandedRow's nothingToShow gate feeding renderPierre, and the render-time pinRef pin of before/after. origin/main has none of them, so this is duplicated work, not one PR inheriting the other. #9156's copy is a strict superset: each hunk is extended for its new fc.patch field (nothingToShow = !fc.patch && fc.before === fc.after, aria-expanded, a pin that also handles an absent pair). The two will conflict textually in both components. The split both bodies describe moved the backend wire change out; the diff-row behaviour did not split.

Suggested order: land this PR first, since #9156 still has an open blocking review finding awaiting your design decision. When #9156 rebases, drop this PR's copy of the three hunks and keep its patch-aware version. The fallbackHeader prop this PR adds to PierreFilePair is exclusive to it and #9156 does not touch it, so that stays here.

Two things independent of the overlap. Please rebase onto current main; the branch is 99 commits behind, and that stale base is what produced the two rebutted GPT findings about #7573 code missing from the branch. Also, the diff commits a 4,037,906-byte GIF under temp-screenshots/, which the body does not mention; please confirm that binary is meant to land in the repository.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.