fix: stop chat transcript blanking content above the viewport - #7207
Conversation
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS Invisible-by-design fix: dedupes virtualizer keys and guards zero-height measurements, removing dropped rows and blank transcript regions — no strings, surfaces, or flows change. [UX-REVIEWED] cdc5d90 |
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound fixes for two real, well-evidenced mechanisms — but Watch
Suggestions
[DESIGN-REVIEWED] cdc5d90 |
GPT 5.6 Review — ✅ human override acceptedHuman judgment by @CrysisDeu overrides the GPT 5.6 finding for This comment is updated in place on each push. The model was not re-run because an authorized human decision supersedes it. False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All claims verified: both production First-Principles-Verdict: PASS Both blanking mechanisms are fixed at the mechanism, the rejected cause-level alternative is named with its cost, and nothing rides along. What this change shipsIntent: stop the chat transcript showing a blank region above the viewport — a FIX.
The duplication question is settled in the diff itself: [FIRST-PRINCIPLES-REVIEWED] cdc5d90 |
Two deterministic defects in the chat virtualizer produced the reported 'earlier messages replaced by blank space' symptom: - Duplicate row keys. A single keys on msgKey alone (row-<ts>), so two rows stamped in the same clock tick collide — React silently drops a sibling and both rows share one HeightCache slot. The same collision arrives via an overlapping older page whose rows lack meta.mid. ChatPage now feeds the virtualizer a list-level deduped key array (uniqueRowKeys): first occurrence keeps the bare key (persisted heights and anchors survive), later duplicates get an occurrence suffix that is itself re-checked against natural keys. - Zero-height cache poisoning. The ResizeObserver write path cached the 0-height entries the observer reports when an ancestor goes display:none, and HeightCache persists per session — pricing the region above at ~1px/row and collapsing the top spacer. The RO path now applies the same h > 0 floor the measureRef seed path already had. Previously persisted zeros self-heal on remount (fix-forward). Because ChatPage's getKey is now index-addressed, the prepend anchor capture — the one consumer that resolves the PREVIOUS render's items at old indices — snapshots getKey alongside those items (prependPrevRef) and resolves through the snapshot; a positional-getKey test pins it. All fixes are mutation-verified. Adversarial review: 2 rounds, dual blind reviewers; round-1 Critical (stale-items getKey pairing) fixed and covered; round-2 PASS, both Optional findings adopted.
d354f39 to
cdc5d90
Compare
|
/ai-review override gpt cdc5d90: The prepend-reassignment scenario requires two rows sharing an exact-ms ts across a page boundary — rows that WITHOUT the dedupe render as duplicate React keys (one sibling silently dropped, one shared height slot); the suffix shift costs those already-broken rows one remount, which is strictly better, and removing occurrence keying as suggested reintroduces the dropped-sibling bug. Trade-off documented in the uniqueRowKeys doc comment and PR body. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
A transient "thinking" row that mounts and unmounts between rows that are
already on screen jogs the viewport, while token growth inside an existing
line is smooth. Whole-row add/remove is the difference, and the render-phase
anchor capture covered none of its shapes.
Everything below extends the ONE existing render-phase capture and its ONE
consumer -- no parallel anchor path, which would fight this one for scrollTop.
1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended`
only tested "count grew while index 0 kept its key", which a splice
satisfies too -- so it took the append path, which resolves a mounted
node's PREVIOUS-commit index through the NEW `items` and therefore anchored
on a mis-keyed row. It now keys on whether any PRE-EXISTING position
changed hands, and the splice gets TRIGGER 1's old-items,
survivor-filtered mapping.
2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the
same capture and the same consumer.
3. The estimate-vs-measured reprice, at its source. `getHeight` prices every
UNMEASURED row from the running MEAN of the measured ones, so a measurement
is never local to its own row: a transient row's height goes on pricing the
transcript after the row itself is gone.
One invariant governs the fix, in three axes:
A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the
commit that dropped the row.
COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a
commit that drops the ghost while adding output nets to growth or to zero
with its height still pricing the transcript. Departure is found by an
EXACT positional scan, so an interior replacement counts -- a boundary probe
reads an artifact card refreshed in place as a no-op. The scan is
affordable because a token append rebuilds the array while REUSING every
element object but the streaming row's, so reference equality settles those
positions with no `getKey` call and no allocation.
NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and
`handleEditResend` both snapshot the transcript, optimistically truncate
it, and dispatch the snapshot back when the server refuses the press. So
`retire` drops the height from the MEAN and keeps the entry, and
`reviveIfRetired` puts it back when a live row index resolves that key
again -- the only available signal, because an optimistic TAIL truncation
comes back as a plain append and has no commit shape of its own. Two
orderings follow, both of which had it backwards before: eviction drains
the retired set FIRST (a transient row is measured just before it leaves,
so LRU order would drop a live row instead), and revival runs as a pass
BEFORE the tree walk (mid-walk it would leave every row priced earlier in
that same walk holding the stale mean, with no later sync guaranteed).
EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not
left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an
equal-count SWAP moves none of its dependencies, so its body would not run
and the spacers would keep prices the retirement just invalidated.
Retired entries are left out of the persisted blob while retired (a reload
has no snapshot to roll back to) and persisted again once revived.
TRIGGER 6 deliberately captures NO anchor. The single consumer is invalidated
by `windowRange` and `itemCount`, and an equal-count commit moves neither, so
an anchor taken there would sit in the slot and be spent on an unrelated later
commit -- the stranded-anchor hazard the render-phase capture exists to remove.
Compensating it needs a new invalidation key in that shared consumer, which
changes the contract every trigger depends on and wants its own regression
pass: filed as #7234.
Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom
keeps following the output down.
Tests -- each red-before proven by reverting just its own production change:
- prependAnchor: splice-in above the reader drifted 100px; remove above the
reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard.
- heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67
instead of returning to 9000; a rolled-back row came back at the flat
estimate 80 instead of its own 900px measurement; an INTERIOR replacement
kept pricing the transcript at equal count (13050 instead of 9000) and
alongside an append (15100 instead of 9300); a swapped-out row left the tree
at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A
commit that only rewrites one row in place -- the token-append shape -- must
retire nothing, and does not.
- virtualizerHeightOwner: with revival inside the read, the rows the walk
reaches first kept the stale mean (2100 instead of 2500).
- HeightCache: nine cases pin the retire/revive contract -- out of the mean,
still readable, revived on re-measure, revived when its row is live again,
idempotent revive, estimate fallback when every sample is retired, not
persisted while retired but re-persisted once revived, retired evicted ahead
of an older live row, and LRU order resumed once no retired entry is left.
Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage
resolves a per-render deduped key LIST by position). Every lookup this commit
adds that reads the PREVIOUS render's items -- the positional departure scan,
the splice anchor's key resolver, and the departed-key pass -- therefore prices
them with the `getKey` captured WITH them, the contract #7207 established for
the prepend capture; the current render's closure returns the NEW list's key at
an old index. Two tests pin the interaction, both red-before: an interior
removal under a positional getKey retired nothing and kept pricing the
transcript at 14100 instead of 8700, and a splice above the reader drifted the
full 100px row because the anchor was named one row off.
Virtualizer family 202/202 green (anchorRestore, heightSyncAnchor, integration,
layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse,
spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache,
HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other
suites that import the virtualizer. No existing assertion weakened.
Fixes #6076
A transient "thinking" row that mounts and unmounts between rows that are
already on screen jogs the viewport, while token growth inside an existing
line is smooth. Whole-row add/remove is the difference, and the render-phase
anchor capture covered none of its shapes.
Everything below extends the ONE existing render-phase capture and its ONE
consumer -- no parallel anchor path, which would fight this one for scrollTop.
1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended`
only tested "count grew while index 0 kept its key", which a splice
satisfies too -- so it took the append path, which resolves a mounted
node's PREVIOUS-commit index through the NEW `items` and therefore anchored
on a mis-keyed row. It now keys on whether any PRE-EXISTING position
changed hands, and the splice gets TRIGGER 1's old-items,
survivor-filtered mapping.
2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the
same capture and the same consumer.
3. The estimate-vs-measured reprice, at its source. `getHeight` prices every
UNMEASURED row from the running MEAN of the measured ones, so a measurement
is never local to its own row: a transient row's height goes on pricing the
transcript after the row itself is gone.
One invariant governs the fix, in three axes:
A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the
commit that dropped the row.
COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a
commit that drops the ghost while adding output nets to growth or to zero
with its height still pricing the transcript. Departure is found by an
EXACT positional scan, so an interior replacement counts -- a boundary probe
reads an artifact card refreshed in place as a no-op. The scan is
affordable because a token append rebuilds the array while REUSING every
element object but the streaming row's, so reference equality settles those
positions with no `getKey` call and no allocation.
NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and
`handleEditResend` both snapshot the transcript, optimistically truncate
it, and dispatch the snapshot back when the server refuses the press. So
`retire` drops the height from the MEAN and keeps the entry, and
`reviveIfRetired` puts it back when a live row index resolves that key
again -- the only available signal, because an optimistic TAIL truncation
comes back as a plain append and has no commit shape of its own. Two
orderings follow, both of which had it backwards before: eviction drains
the retired set FIRST (a transient row is measured just before it leaves,
so LRU order would drop a live row instead), and revival runs as a pass
BEFORE the tree walk (mid-walk it would leave every row priced earlier in
that same walk holding the stale mean, with no later sync guaranteed).
EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not
left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an
equal-count SWAP moves none of its dependencies, so its body would not run
and the spacers would keep prices the retirement just invalidated.
Retired entries are left out of the persisted blob while retired (a reload
has no snapshot to roll back to) and persisted again once revived.
RETIREMENT IS GATED ON DEPARTURE, NOT ON A COUNT SHAPE. Three separate commits
reached the retirement site with a row's measurement still pricing the
transcript -- an equal-count swap, an interior replacement, and a full-session
`/clear` -- because retirement borrowed the ANCHOR's triggers, and those are
built from count arithmetic plus `frontKeyHeld`. The anchor needs both: a
renamed index 0 means the mounted nodes' indices no longer name their own rows,
and a count that does not move leaves part 2 (invalidated by `windowRange` and
`itemCount`) not running to spend what was captured. Retirement needs neither,
and the clear is what made the borrowing indefensible: emptying the list
renames index 0 exactly as paging out the head does, so the proxy read a wipe
as a page-out and carried the old conversation's heights into the next one.
The gate is now stated once, at the level the harm lives on -- A ROW LEFT THIS
SESSION -- which requires either a shrinking count or a shared index changing
hands, so the streaming commit (same rows, one more at the tail) still does no
work. Head paging is excluded on the shape of the departure itself: a trim drops
a contiguous PREFIX and leaves the rest standing, and those rows are coming
back, so their heights must keep pricing the region above. A clear departs
everything and leaves NO survivor, which is the test that separates a wipe from
a page-out -- both rename index 0, and only one is coming back.
That subsumes the equal-count trigger, which existed only to reach retirement,
so it is gone rather than left as a trigger with no anchor and no consumer. The
stranded-anchor hazard it would have carried is unchanged and still needs a new
invalidation key in the shared consumer, which wants its own regression pass:
filed as #7234.
Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom
keeps following the output down.
Tests -- each red-before proven by reverting just its own production change:
- prependAnchor: splice-in above the reader drifted 100px; remove above the
reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard.
- heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67
instead of returning to 9000; a rolled-back row came back at the flat
estimate 80 instead of its own 900px measurement; an INTERIOR replacement
kept pricing the transcript at equal count (13050 instead of 9000) and
alongside an append (15100 instead of 9300); a swapped-out row left the tree
at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A
commit that only rewrites one row in place -- the token-append shape -- must
retire nothing, and does not.
- virtualizerHeightOwner: with revival inside the read, the rows the walk
reaches first kept the stale mean (2100 instead of 2500).
- HeightCache: nine cases pin the retire/revive contract -- out of the mean,
still readable, revived on re-measure, revived when its row is live again,
idempotent revive, estimate fallback when every sample is retired, not
persisted while retired but re-persisted once revived, retired evicted ahead
of an older live row, and LRU order resumed once no retired entry is left.
Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage
resolves a per-render deduped key LIST by position). Every lookup this commit
adds that reads the PREVIOUS render's items -- the positional departure scan,
the splice anchor's key resolver, and the departed-key pass -- therefore prices
them with the `getKey` captured WITH them, the contract #7207 established for
the prepend capture; the current render's closure returns the NEW list's key at
an old index. Two tests pin the interaction, both red-before: an interior
removal under a positional getKey retired nothing and kept pricing the
transcript at 14100 instead of 8700, and a splice above the reader drifted the
full 100px row because the anchor was named one row off.
- heightSyncAnchor: a `/clear` in the same session priced the next
conversation's first row from the cleared transcript's mean (500 instead of
the flat 80 estimate), and dropping the head-paging exclusion retired a
paged-out prefix and collapsed the region above from 10000 to 1600.
Virtualizer family 204/204 green (anchorRestore, heightSyncAnchor, integration,
layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse,
spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache,
HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other
suites that import the virtualizer. No existing assertion weakened.
Fixes #6076
A transient "thinking" row that mounts and unmounts between rows that are
already on screen jogs the viewport, while token growth inside an existing
line is smooth. Whole-row add/remove is the difference, and the render-phase
anchor capture covered none of its shapes.
Everything below extends the ONE existing render-phase capture and its ONE
consumer -- no parallel anchor path, which would fight this one for scrollTop.
1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended`
only tested "count grew while index 0 kept its key", which a splice
satisfies too -- so it took the append path, which resolves a mounted
node's PREVIOUS-commit index through the NEW `items` and therefore anchored
on a mis-keyed row. It now keys on whether any PRE-EXISTING position
changed hands, and the splice gets TRIGGER 1's old-items,
survivor-filtered mapping.
2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the
same capture and the same consumer.
3. The estimate-vs-measured reprice, at its source. `getHeight` prices every
UNMEASURED row from the running MEAN of the measured ones, so a measurement
is never local to its own row: a transient row's height goes on pricing the
transcript after the row itself is gone.
One invariant governs the fix, in three axes:
A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the
commit that dropped the row.
COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a
commit that drops the ghost while adding output nets to growth or to zero
with its height still pricing the transcript. Departure is found by an
EXACT positional scan, so an interior replacement counts -- a boundary probe
reads an artifact card refreshed in place as a no-op. The scan is
affordable because a token append rebuilds the array while REUSING every
element object but the streaming row's, so reference equality settles those
positions with no `getKey` call and no allocation.
NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and
`handleEditResend` both snapshot the transcript, optimistically truncate
it, and dispatch the snapshot back when the server refuses the press. So
`retire` drops the height from the MEAN and keeps the entry, and
`reviveIfRetired` puts it back when a live row index resolves that key
again -- the only available signal, because an optimistic TAIL truncation
comes back as a plain append and has no commit shape of its own. Two
orderings follow, both of which had it backwards before: eviction drains
the retired set FIRST (a transient row is measured just before it leaves,
so LRU order would drop a live row instead), and revival runs as a pass
BEFORE the tree walk (mid-walk it would leave every row priced earlier in
that same walk holding the stale mean, with no later sync guaranteed).
EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not
left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an
equal-count SWAP moves none of its dependencies, so its body would not run
and the spacers would keep prices the retirement just invalidated.
Retired entries are left out of the persisted blob while retired (a reload
has no snapshot to roll back to) and persisted again once revived.
RETIREMENT IS GATED ON DEPARTURE, NOT ON A COUNT SHAPE. Three separate commits
reached the retirement site with a row's measurement still pricing the
transcript -- an equal-count swap, an interior replacement, and a full-session
`/clear` -- because retirement borrowed the ANCHOR's triggers, and those are
built from count arithmetic plus `frontKeyHeld`. The anchor needs both: a
renamed index 0 means the mounted nodes' indices no longer name their own rows,
and a count that does not move leaves part 2 (invalidated by `windowRange` and
`itemCount`) not running to spend what was captured. Retirement needs neither,
and the clear is what made the borrowing indefensible: emptying the list
renames index 0 exactly as paging out the head does, so the proxy read a wipe
as a page-out and carried the old conversation's heights into the next one.
The gate is now stated once, at the level the harm lives on -- A ROW LEFT THIS
SESSION -- which requires either a shrinking count or a shared index changing
hands, so the streaming commit (same rows, one more at the tail) still does no
work.
Head paging is the ONE departure that must not retire, because its rows come
back when the reader scrolls up. It is identified by all THREE of its
properties, since any two of them are also true of a departure that must retire:
the count FELL (a prepend regroup also drops a prefix row while survivors
remain, and it grows the count), the departures are a contiguous PREFIX (a tail
truncation or an interior removal leaves a survivor above a departure), and a
survivor REMAINS (a clear departs a prefix and nothing else, and is not coming
back). Every other shape retires and each is covered: interior removal by the
prefix test, equal-count swap and interior-replacement-plus-append by the count
test, tail truncation by the prefix test, clear by the survivor test.
That subsumes the equal-count trigger, which existed only to reach retirement,
so it is gone rather than left as a trigger with no anchor and no consumer. The
stranded-anchor hazard it would have carried is unchanged and still needs a new
invalidation key in the shared consumer, which wants its own regression pass:
filed as #7234.
Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom
keeps following the output down.
Tests -- each red-before proven by reverting just its own production change:
- prependAnchor: splice-in above the reader drifted 100px; remove above the
reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard.
- heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67
instead of returning to 9000; a rolled-back row came back at the flat
estimate 80 instead of its own 900px measurement; an INTERIOR replacement
kept pricing the transcript at equal count (13050 instead of 9000) and
alongside an append (15100 instead of 9300); a swapped-out row left the tree
at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A
commit that only rewrites one row in place -- the token-append shape -- must
retire nothing, and does not.
- virtualizerHeightOwner: with revival inside the read, the rows the walk
reaches first kept the stale mean (2100 instead of 2500).
- HeightCache: nine cases pin the retire/revive contract -- out of the mean,
still readable, revived on re-measure, revived when its row is live again,
idempotent revive, estimate fallback when every sample is retired, not
persisted while retired but re-persisted once revived, retired evicted ahead
of an older live row, and LRU order resumed once no retired entry is left.
Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage
resolves a per-render deduped key LIST by position). Every lookup this commit
adds that reads the PREVIOUS render's items -- the positional departure scan,
the splice anchor's key resolver, and the departed-key pass -- therefore prices
them with the `getKey` captured WITH them, the contract #7207 established for
the prepend capture; the current render's closure returns the NEW list's key at
an old index. Two tests pin the interaction, both red-before: an interior
removal under a positional getKey retired nothing and kept pricing the
transcript at 14100 instead of 8700, and a splice above the reader drifted the
full 100px row because the anchor was named one row off.
- heightSyncAnchor: a `/clear` in the same session priced the next
conversation's first row from the cleared transcript's mean (500 instead of
the flat 80 estimate); dropping the head-paging exclusion retired a paged-out
prefix and collapsed the region above from 10000 to 1600; and a prepend that
regroups the top row away kept its 900 in the mean, pricing the transcript at
19600 instead of 12000, because that departure satisfies head paging's other
two properties while growing the count.
Virtualizer family 210/210 green (anchorRestore, heightSyncAnchor, integration,
layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse,
spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache,
HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other
suites that import the virtualizer. No existing assertion weakened.
Fixes #6076
A transient "thinking" row that mounts and unmounts between rows that are
already on screen jogs the viewport, while token growth inside an existing
line is smooth. Whole-row add/remove is the difference, and the render-phase
anchor capture covered none of its shapes.
Everything below extends the ONE existing render-phase capture and its ONE
consumer -- no parallel anchor path, which would fight this one for scrollTop.
1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended`
only tested "count grew while index 0 kept its key", which a splice
satisfies too -- so it took the append path, which resolves a mounted
node's PREVIOUS-commit index through the NEW `items` and therefore anchored
on a mis-keyed row. It now keys on whether any PRE-EXISTING position
changed hands, and the splice gets TRIGGER 1's old-items,
survivor-filtered mapping.
2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the
same capture and the same consumer.
3. The estimate-vs-measured reprice, at its source. `getHeight` prices every
UNMEASURED row from the running MEAN of the measured ones, so a measurement
is never local to its own row: a transient row's height goes on pricing the
transcript after the row itself is gone.
One invariant governs the fix, in three axes:
A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the
commit that dropped the row.
COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a
commit that drops the ghost while adding output nets to growth or to zero
with its height still pricing the transcript. Departure is found by an
EXACT positional scan, so an interior replacement counts -- a boundary probe
reads an artifact card refreshed in place as a no-op. The scan is
affordable because a token append rebuilds the array while REUSING every
element object but the streaming row's, so reference equality settles those
positions with no `getKey` call and no allocation.
NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and
`handleEditResend` both snapshot the transcript, optimistically truncate
it, and dispatch the snapshot back when the server refuses the press. So
`retire` drops the height from the MEAN and keeps the entry, and
`reviveIfRetired` puts it back when a live row index resolves that key
again -- the only available signal, because an optimistic TAIL truncation
comes back as a plain append and has no commit shape of its own. Two
orderings follow, both of which had it backwards before: eviction drains
the retired set FIRST (a transient row is measured just before it leaves,
so LRU order would drop a live row instead), and revival runs as a pass
BEFORE the tree walk (mid-walk it would leave every row priced earlier in
that same walk holding the stale mean, with no later sync guaranteed).
EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not
left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an
equal-count SWAP moves none of its dependencies, so its body would not run
and the spacers would keep prices the retirement just invalidated.
Retired entries are left out of the persisted blob while retired (a reload
has no snapshot to roll back to) and persisted again once revived.
RETIREMENT IS GATED ON DEPARTURE, NOT ON A COUNT SHAPE. Three separate commits
reached the retirement site with a row's measurement still pricing the
transcript -- an equal-count swap, an interior replacement, and a full-session
`/clear` -- because retirement borrowed the ANCHOR's triggers, and those are
built from count arithmetic plus `frontKeyHeld`. The anchor needs both: a
renamed index 0 means the mounted nodes' indices no longer name their own rows,
and a count that does not move leaves part 2 (invalidated by `windowRange` and
`itemCount`) not running to spend what was captured. Retirement needs neither,
and the clear is what made the borrowing indefensible: emptying the list
renames index 0 exactly as paging out the head does, so the proxy read a wipe
as a page-out and carried the old conversation's heights into the next one.
The gate is now stated once, at the level the harm lives on -- A ROW LEFT THIS
SESSION -- which requires either a shrinking count or a shared index changing
hands, so the streaming commit (same rows, one more at the tail) still does no
work.
Head paging is the ONE departure that must not retire, because its rows come
back when the reader scrolls up. Recognising it starts from the CALLER, not from
the data: nothing pages unless the consumer asked to be told when the reader
reaches the top, so a consumer with no `onTopReached` has no page-out to exempt
and every departure it makes is final. That is what separates the transcript
(ChatPage, which wires it) from a filtered list -- the artifacts gallery does
not wire it and feeds the virtualizer a SEARCH RESULT, so narrowing the box
drops a leading run of cards and keeps later ones, which no shape test can tell
from a page-out, and those cards are not coming back. Its `sessionId` is a
shared constant, so the stale heights would price every later gallery render.
Within a paging consumer all three shape properties are still required, since
any two of them are also true of a departure that must retire: the count FELL (a
prepend regroup also drops a prefix row while survivors remain, and it grows the
count), the departures are a contiguous PREFIX (a tail truncation or an interior
removal leaves a survivor above a departure), and a survivor REMAINS (a clear
departs a prefix and nothing else, and is not coming back). Every shape is
covered: interior removal by the prefix test, equal-count swap and
interior-replacement-plus-append by the count test, tail truncation by the
prefix test, clear by the survivor test, and any departure at all in a
non-paging consumer by the capability test.
That subsumes the equal-count trigger, which existed only to reach retirement,
so it is gone rather than left as a trigger with no anchor and no consumer. The
stranded-anchor hazard it would have carried is unchanged and still needs a new
invalidation key in the shared consumer, which wants its own regression pass:
filed as #7234.
Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom
keeps following the output down.
Tests -- each red-before proven by reverting just its own production change:
- prependAnchor: splice-in above the reader drifted 100px; remove above the
reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard.
- heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67
instead of returning to 9000; a rolled-back row came back at the flat
estimate 80 instead of its own 900px measurement; an INTERIOR replacement
kept pricing the transcript at equal count (13050 instead of 9000) and
alongside an append (15100 instead of 9300); a swapped-out row left the tree
at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A
commit that only rewrites one row in place -- the token-append shape -- must
retire nothing, and does not.
- virtualizerHeightOwner: with revival inside the read, the rows the walk
reaches first kept the stale mean (2100 instead of 2500).
- HeightCache: nine cases pin the retire/revive contract -- out of the mean,
still readable, revived on re-measure, revived when its row is live again,
idempotent revive, estimate fallback when every sample is retired, not
persisted while retired but re-persisted once revived, retired evicted ahead
of an older live row, and LRU order resumed once no retired entry is left.
Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage
resolves a per-render deduped key LIST by position). Every lookup this commit
adds that reads the PREVIOUS render's items -- the positional departure scan,
the splice anchor's key resolver, and the departed-key pass -- therefore prices
them with the `getKey` captured WITH them, the contract #7207 established for
the prepend capture; the current render's closure returns the NEW list's key at
an old index. Two tests pin the interaction, both red-before: an interior
removal under a positional getKey retired nothing and kept pricing the
transcript at 14100 instead of 8700, and a splice above the reader drifted the
full 100px row because the anchor was named one row off.
- heightSyncAnchor: a `/clear` in the same session priced the next
conversation's first row from the cleared transcript's mean (500 instead of
the flat 80 estimate); dropping the head-paging exclusion retired a paged-out
prefix and collapsed the region above from 10000 to 1600; and a prepend that
regroups the top row away kept its 900 in the mean, pricing the transcript at
19600 instead of 12000, because that departure satisfies head paging's other
two properties while growing the count; and a filtered-out prefix in a
consumer that cannot page kept pricing the survivors at 10000 instead of
falling back to the flat estimate at 1600.
Virtualizer family 211/211 green (anchorRestore, heightSyncAnchor, integration,
layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse,
spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache,
HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other
suites that import the virtualizer. No existing assertion weakened.
Fixes #6076
A transient "thinking" row that mounts and unmounts between rows that are
already on screen jogs the viewport, while token growth inside an existing
line is smooth. Whole-row add/remove is the difference, and the render-phase
anchor capture covered none of its shapes.
Everything below extends the ONE existing render-phase capture and its ONE
consumer -- no parallel anchor path, which would fight this one for scrollTop.
1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended`
only tested "count grew while index 0 kept its key", which a splice
satisfies too -- so it took the append path, which resolves a mounted
node's PREVIOUS-commit index through the NEW `items` and therefore anchored
on a mis-keyed row. It now keys on whether any PRE-EXISTING position
changed hands, and the splice gets TRIGGER 1's old-items,
survivor-filtered mapping.
2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the
same capture and the same consumer.
3. The estimate-vs-measured reprice, at its source. `getHeight` prices every
UNMEASURED row from the running MEAN of the measured ones, so a measurement
is never local to its own row: a transient row's height goes on pricing the
transcript after the row itself is gone.
One invariant governs the fix, in three axes:
A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the
commit that dropped the row.
COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a
commit that drops the ghost while adding output nets to growth or to zero
with its height still pricing the transcript. Departure is found by an
EXACT positional scan, so an interior replacement counts -- a boundary probe
reads an artifact card refreshed in place as a no-op. The scan is
affordable because a token append rebuilds the array while REUSING every
element object but the streaming row's, so reference equality settles those
positions with no `getKey` call and no allocation.
NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and
`handleEditResend` both snapshot the transcript, optimistically truncate
it, and dispatch the snapshot back when the server refuses the press. So
`retire` drops the height from the MEAN and keeps the entry, and
`reviveIfRetired` puts it back when a live row index resolves that key
again -- the only available signal, because an optimistic TAIL truncation
comes back as a plain append and has no commit shape of its own. Two
orderings follow, both of which had it backwards before: eviction drains
the retired set FIRST (a transient row is measured just before it leaves,
so LRU order would drop a live row instead), and revival runs as a pass
BEFORE the tree walk (mid-walk it would leave every row priced earlier in
that same walk holding the stale mean, with no later sync guaranteed).
EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not
left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an
equal-count SWAP moves none of its dependencies, so its body would not run
and the spacers would keep prices the retirement just invalidated.
Retired entries are left out of the persisted blob while retired (a reload
has no snapshot to roll back to) and persisted again once revived.
RETIREMENT IS GATED ON DEPARTURE, NOT ON A COUNT SHAPE. Three separate commits
reached the retirement site with a row's measurement still pricing the
transcript -- an equal-count swap, an interior replacement, and a full-session
`/clear` -- because retirement borrowed the ANCHOR's triggers, and those are
built from count arithmetic plus `frontKeyHeld`. The anchor needs both: a
renamed index 0 means the mounted nodes' indices no longer name their own rows,
and a count that does not move leaves part 2 (invalidated by `windowRange` and
`itemCount`) not running to spend what was captured. Retirement needs neither,
and the clear is what made the borrowing indefensible: emptying the list
renames index 0 exactly as paging out the head does, so the proxy read a wipe
as a page-out and carried the old conversation's heights into the next one.
The gate is now stated once, at the level the harm lives on -- A ROW LEFT THIS
SESSION -- which requires either a shrinking count or a shared index changing
hands, so the streaming commit (same rows, one more at the tail) still does no
work.
Head paging is the ONE departure that must not retire, because its rows come
back when the reader scrolls up. Recognising it starts from the CALLER, not from
the data: nothing pages unless the consumer asked to be told when the reader
reaches the top, so a consumer with no `onTopReached` has no page-out to exempt
and every departure it makes is final. That is what separates the transcript
(ChatPage, which wires it) from a filtered list -- the artifacts gallery does
not wire it and feeds the virtualizer a SEARCH RESULT, so narrowing the box
drops a leading run of cards and keeps later ones, which no shape test can tell
from a page-out, and those cards are not coming back. Its `sessionId` is a
shared constant, so the stale heights would price every later gallery render.
Within a paging consumer all three shape properties are still required, since
any two of them are also true of a departure that must retire: the count FELL (a
prepend regroup also drops a prefix row while survivors remain, and it grows the
count), the departures are a contiguous PREFIX (a tail truncation or an interior
removal leaves a survivor above a departure), and a survivor REMAINS (a clear
departs a prefix and nothing else, and is not coming back). Every shape is
covered: interior removal by the prefix test, equal-count swap and
interior-replacement-plus-append by the count test, tail truncation by the
prefix test, clear by the survivor test, and any departure at all in a
non-paging consumer by the capability test.
That subsumes the equal-count trigger, which existed only to reach retirement,
so it is gone rather than left as a trigger with no anchor and no consumer. The
stranded-anchor hazard it would have carried is unchanged and still needs a new
invalidation key in the shared consumer, which wants its own regression pass:
filed as #7234.
Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom
keeps following the output down.
Tests -- each red-before proven by reverting just its own production change:
- prependAnchor: splice-in above the reader drifted 100px; remove above the
reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard.
- heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67
instead of returning to 9000; a rolled-back row came back at the flat
estimate 80 instead of its own 900px measurement; an INTERIOR replacement
kept pricing the transcript at equal count (13050 instead of 9000) and
alongside an append (15100 instead of 9300); a swapped-out row left the tree
at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A
commit that only rewrites one row in place -- the token-append shape -- must
retire nothing, and does not.
- virtualizerHeightOwner: with revival inside the read, the rows the walk
reaches first kept the stale mean (2100 instead of 2500).
- HeightCache: nine cases pin the retire/revive contract -- out of the mean,
still readable, revived on re-measure, revived when its row is live again,
idempotent revive, estimate fallback when every sample is retired, not
persisted while retired but re-persisted once revived, retired evicted ahead
of an older live row, and LRU order resumed once no retired entry is left.
Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage
resolves a per-render deduped key LIST by position). Every lookup this commit
adds that reads the PREVIOUS render's items -- the positional departure scan,
the splice anchor's key resolver, and the departed-key pass -- therefore prices
them with the `getKey` captured WITH them, the contract #7207 established for
the prepend capture; the current render's closure returns the NEW list's key at
an old index. Two tests pin the interaction, both red-before: an interior
removal under a positional getKey retired nothing and kept pricing the
transcript at 14100 instead of 8700, and a splice above the reader drifted the
full 100px row because the anchor was named one row off.
- heightSyncAnchor: a `/clear` in the same session priced the next
conversation's first row from the cleared transcript's mean (500 instead of
the flat 80 estimate); dropping the head-paging exclusion retired a paged-out
prefix and collapsed the region above from 10000 to 1600; and a prepend that
regroups the top row away kept its 900 in the mean, pricing the transcript at
19600 instead of 12000, because that departure satisfies head paging's other
two properties while growing the count; and a filtered-out prefix in a
consumer that cannot page kept pricing the survivors at 10000 instead of
falling back to the flat estimate at 1600.
Virtualizer family 211/211 green (anchorRestore, heightSyncAnchor, integration,
layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse,
spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache,
HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other
suites that import the virtualizer. No existing assertion weakened.
Fixes #6076
#7198) A transient "thinking" row that mounts and unmounts between rows that are already on screen jogs the viewport, while token growth inside an existing line is smooth. Whole-row add/remove is the difference, and the render-phase anchor capture covered none of its shapes. Everything below extends the ONE existing render-phase capture and its ONE consumer -- no parallel anchor path, which would fight this one for scrollTop. 1. A mid-list INSERT was swallowed by TRIGGER 3's predicate. `tailAppended` only tested "count grew while index 0 kept its key", which a splice satisfies too -- so it took the append path, which resolves a mounted node's PREVIOUS-commit index through the NEW `items` and therefore anchored on a mis-keyed row. It now keys on whether any PRE-EXISTING position changed hands, and the splice gets TRIGGER 1's old-items, survivor-filtered mapping. 2. A mid-list REMOVE had no trigger at all. TRIGGER 5 covers it through the same capture and the same consumer. 3. The estimate-vs-measured reprice, at its source. `getHeight` prices every UNMEASURED row from the running MEAN of the measured ones, so a measurement is never local to its own row: a transient row's height goes on pricing the transcript after the row itself is gone. One invariant governs the fix, in three axes: A retirement must be COMPLETE, NON-DESTRUCTIVE, and EFFECTIVE in the commit that dropped the row. COMPLETE. Retirement keys on KEY DEPARTURE, not on the net count falling: a commit that drops the ghost while adding output nets to growth or to zero with its height still pricing the transcript. Departure is found by an EXACT positional scan, so an interior replacement counts -- a boundary probe reads an artifact card refreshed in place as a no-op. The scan is affordable because a token append rebuilds the array while REUSING every element object but the streaming row's, so reference equality settles those positions with no `getKey` call and no allocation. NON-DESTRUCTIVE. Removal is not always permanent: `handleRegenerate` and `handleEditResend` both snapshot the transcript, optimistically truncate it, and dispatch the snapshot back when the server refuses the press. So `retire` drops the height from the MEAN and keeps the entry, and `reviveIfRetired` puts it back when a live row index resolves that key again -- the only available signal, because an optimistic TAIL truncation comes back as a plain append and has no commit shape of its own. Two orderings follow, both of which had it backwards before: eviction drains the retired set FIRST (a transient row is measured just before it leaves, so LRU order would drop a live row instead), and revival runs as a pass BEFORE the tree walk (mid-walk it would leave every row priced earlier in that same walk holding the stale mean, with no later sync guaranteed). EFFECTIVE IN THAT COMMIT. The tree is re-synced at the retirement site, not left to the `offsetIndex` memo: that memo is keyed on `itemCount`, and an equal-count SWAP moves none of its dependencies, so its body would not run and the spacers would keep prices the retirement just invalidated. Retired entries are left out of the persisted blob while retired (a reload has no snapshot to roll back to) and persisted again once revived. RETIREMENT IS GATED ON DEPARTURE, NOT ON A COUNT SHAPE. Three separate commits reached the retirement site with a row's measurement still pricing the transcript -- an equal-count swap, an interior replacement, and a full-session `/clear` -- because retirement borrowed the ANCHOR's triggers, and those are built from count arithmetic plus `frontKeyHeld`. The anchor needs both: a renamed index 0 means the mounted nodes' indices no longer name their own rows, and a count that does not move leaves part 2 (invalidated by `windowRange` and `itemCount`) not running to spend what was captured. Retirement needs neither, and the clear is what made the borrowing indefensible: emptying the list renames index 0 exactly as paging out the head does, so the proxy read a wipe as a page-out and carried the old conversation's heights into the next one. The gate is now stated once, at the level the harm lives on -- A ROW LEFT THIS SESSION -- which requires either a shrinking count or a shared index changing hands, so the streaming commit (same rows, one more at the tail) still does no work. Head paging is the ONE departure that must not retire, because its rows come back when the reader scrolls up. Recognising it starts from the CALLER, not from the data: nothing pages unless the consumer asked to be told when the reader reaches the top, so a consumer with no `onTopReached` has no page-out to exempt and every departure it makes is final. That is what separates the transcript (ChatPage, which wires it) from a filtered list -- the artifacts gallery does not wire it and feeds the virtualizer a SEARCH RESULT, so narrowing the box drops a leading run of cards and keeps later ones, which no shape test can tell from a page-out, and those cards are not coming back. Its `sessionId` is a shared constant, so the stale heights would price every later gallery render. Within a paging consumer all three shape properties are still required, since any two of them are also true of a departure that must retire: the count FELL (a prepend regroup also drops a prefix row while survivors remain, and it grows the count), the departures are a contiguous PREFIX (a tail truncation or an interior removal leaves a survivor above a departure), and a survivor REMAINS (a clear departs a prefix and nothing else, and is not coming back). Every shape is covered: interior removal by the prefix test, equal-count swap and interior-replacement-plus-append by the count test, tail truncation by the prefix test, clear by the survivor test, and any departure at all in a non-paging consumer by the capability test. That subsumes the equal-count trigger, which existed only to reach retirement, so it is gone rather than left as a trigger with no anchor and no consumer. The stranded-anchor hazard it would have carried is unchanged and still needs a new invalidation key in the shared consumer, which wants its own regression pass: filed as #7234. Every trigger stays behind the `stickRef` guard: a reader pinned to the bottom keeps following the output down. Tests -- each red-before proven by reverting just its own production change: - prependAnchor: splice-in above the reader drifted 100px; remove above the reader drifted 400px. Two pinned-to-bottom counterparts pin the stick guard. - heightSyncAnchor: after the ghost left, totalHeight stayed at 6386.67 instead of returning to 9000; a rolled-back row came back at the flat estimate 80 instead of its own 900px measurement; an INTERIOR replacement kept pricing the transcript at equal count (13050 instead of 9000) and alongside an append (15100 instead of 9300); a swapped-out row left the tree at its pre-swap 6200 instead of 9000 when the sync was left to the memo. A commit that only rewrites one row in place -- the token-append shape -- must retire nothing, and does not. - virtualizerHeightOwner: with revival inside the read, the rows the walk reaches first kept the stale mean (2100 instead of 2500). - HeightCache: nine cases pin the retire/revive contract -- out of the mean, still readable, revived on re-measure, revived when its row is live again, idempotent revive, estimate fallback when every sample is retired, not persisted while retired but re-persisted once revived, retired evicted ahead of an older live row, and LRU order resumed once no retired entry is left. Rebased onto #7207, which made a caller's `getKey` INDEX-ADDRESSED (ChatPage resolves a per-render deduped key LIST by position). Every lookup this commit adds that reads the PREVIOUS render's items -- the positional departure scan, the splice anchor's key resolver, and the departed-key pass -- therefore prices them with the `getKey` captured WITH them, the contract #7207 established for the prepend capture; the current render's closure returns the NEW list's key at an old index. Two tests pin the interaction, both red-before: an interior removal under a positional getKey retired nothing and kept pricing the transcript at 14100 instead of 8700, and a splice above the reader drifted the full 100px row because the anchor was named one row off. - heightSyncAnchor: a `/clear` in the same session priced the next conversation's first row from the cleared transcript's mean (500 instead of the flat 80 estimate); dropping the head-paging exclusion retired a paged-out prefix and collapsed the region above from 10000 to 1600; and a prepend that regroups the top row away kept its 900 in the mean, pricing the transcript at 19600 instead of 12000, because that departure satisfies head paging's other two properties while growing the count; and a filtered-out prefix in a consumer that cannot page kept pricing the survivors at 10000 instead of falling back to the flat estimate at 1600. Virtualizer family 211/211 green (anchorRestore, heightSyncAnchor, integration, layoutShrink, observerBackfill, postStreamLurch, prependAnchor, railCollapse, spacerLurch, viewportResize, UseVirtualChatCoverage, HeightCache, HeightCache.multiInstance, heightOwner, ScrollAnchorCache), plus the 13 other suites that import the virtualizer. No existing assertion weakened. Fixes #6076 Co-authored-by: Joe Guo <zejiangg@amazon.com>
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
Summary
Users intermittently see the chat transcript lose content above the viewport — a large blank region where earlier messages should be (reported with a session full of repeated
Request initialize timed out/ recovery-card rows). The blank region is the top spacer mispriced against real content. This PR fixes the two deterministic mechanisms behind it:1. Duplicate row keys (content genuinely dropped). A
singledisplay row keys onmsgKeyalone (row-<ts>); a coarse OS clock stamps two rows appended in one tick with the samets, and an overlapping older page whose rows lackmeta.midprepends duplicates. Duplicate keys make React silently drop a sibling row and make two rows share oneHeightCacheslot (spacer oscillation). ChatPage now derives a list-level deduped key array (uniqueRowKeys): the first occurrence keeps the bare key — the non-colliding common case is byte-identical, so persisted heights and scroll anchors survive — and later duplicates get an occurrence suffix, itself re-checked so it cannot collide with a natural key.2. Zero-height cache poisoning (spacer collapse). The ResizeObserver write path cached the 0-height entries the observer reports when an ancestor goes
display:none(hidden tab/panel).HeightCachepersists per session, so every row above the viewport got priced at the 1px floor, collapsingoffsetBefore. The RO path now applies the sameh > 0floor themeasureRefseed path already had. Previously-persisted zeros are fix-forward: they self-heal on remount.Because ChatPage's
getKeyis now index-addressed (a per-render key list), the prepend anchor capture — the one consumer that deliberately resolves the previous render's items at old indices — now snapshotsgetKeyalongside those items (prependPrevRef) and resolves through the snapshot. Without this, load-older would misname the reading anchor by the inserted count.Investigated but deliberately not changed here (architectural, separate work): stale key→height bindings across regroup (
refreshSlotwholesale rebuild) and the 120 ms height-sync debounce starving under continuous re-measures.Testing
ChatPage.uniqueRowKeys.test.ts— 6 unit cases incl. natural-key~dupNcollision and turn-vs-single dedupeChatPage.steerKeyIdentity.test.tsx— wiring test through the REAL getKey ChatPage hands the virtualizeruseVirtualChat.zeroHeightGuard.test.tsx— hidden-ancestor 0 reports skipped; genuine post-hide resize still appliesuseVirtualChat.prependAnchor.test.tsx— new positional-getKey prepend test (ChatPage wiring shape) + a no-surviving-anchor regression pintsc -bclean, eslint 0 errors (no new warnings), vitest 26,803 passedAdversarial review
2 rounds, blind reviewers. Round 1: dual reviewers, both independently flagged a Critical — the index-addressed getKey broke the prepend capture's stale-items contract — fixed via the
prependPrevRefgetKey snapshot and covered by the positional test. Round 2 (fresh reviewer): PASS; both Optional findings adopted (suffix/natural-key re-check, comment precision).No UI appearance change — no screenshots (behavioral fix pinned by tests).