Add a debug-only red `outline` ring on exactly the inline images whose row
height we reserve up front from their imeta `dim` tag — the ones the
reservation fix catches. Lets tho SEE the split in the running app: ringed
= height-reserved, un-ringed = `dim`-less natural-load thrasher (the
residual-flicker caveat made visible).
Uses `outline` (zero layout impact), never `border`, so toggling it can't
shift a single pixel. Gated behind a trivially-flippable module const
(`DEBUG_RING_DIM_IMAGES`) with a ship-then-rip comment; the ring class
applies only when `DEBUG_RING_DIM_IMAGES && reserved`. Not for shipping
styling.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
The channel intro block (the `#` avatar, channel title, "beginning of…"
line, and Create agent / Add people cards) was bottom-pinned: it lived in
a `min-h-full` flex column with `mt-auto`, which Slack-style pushes it to
the viewport bottom when the list is short. tho wants it flush at the
actual TOP of the virtualized list, since it's an in-list header — not a
floating element.
Gate the push-down on `topAlignIntro = showChannelIntro` and turn it off
for the channel-intro case in 4 spots: the outer wrapper `min-h-full`, the
SkeletonReveal `className` + `contentClassName` `min-h-full`, and the
intro div's own `mt-auto`. Natural top-down flow then places the header at
the top. The DM intro and the generic empty state keep their existing
bottom-pin behavior.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Root cause of the first-load flicker tho saw: the inline message `<img>` had no
reserved height — it painted at height 0 and grew to its intrinsic size when the
bytes arrived. In the virtualized list every late image bumps the measured row
height, shifts its neighbors, and re-measures the virtualizer: a cascade of
reflows that flickers and thrashes while a media-heavy channel loads, and fights
the first-load stick-to-bottom because the document height keeps moving.
The stick-to-bottom settle-pin (6f6d2320) held the viewport THROUGH the growth
but did not stop the per-row reflow — this kills the growth at the source.
Fix: reserve the image's rendered box up front from the imeta `dim`
(WIDTHxHEIGHT), which is already parsed and was already used for video aspect
ratios. `reservedImageSize` scales dim to the display caps (max-w-sm/max-h-64 →
384×256, object-contain fit) and the `<img>` sets those as width/height
attributes, so the row's height is stable before load. `h-auto` keeps it
responsive under the width cap while the attributes establish the aspect ratio.
Falls back to natural load when `dim` is absent (can't reserve what we don't
know). The helper lives in markdownUtils.ts (pure, exported) so it's unit-tested
directly via the DOM lane.
Main timeline only; thread pane untouched. 5 DOM tests cover the dim→box math
(within caps, width-capped, height-capped, never-exceeds, malformed fallback).
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Root cause: the init pin was one-shot. On first load the virtualizer paints
with ESTIMATED row heights and the deferred snapshot streams in over several
commits, so the true bottom keeps moving AFTER the single init scrollToBottom.
Nothing re-pinned: the new-message branch only fires on a new latest-message
key (the bottom message is already present during initial fill), so the viewport
ended up anchored where the estimated bottom was and the rest filled in below as
heights resolved — exactly the 'doesn't stay attached to the bottom' tho saw.
Fix: a settle-pin layout effect re-anchors to the bottom whenever the
virtualizer's getTotalSize() changes (estimate→measured heights, streaming rows,
container resize) — but ONLY while still pinned (stickToBottomRef) and not
chasing a deep-link, so a user who scrolled up is never yanked back down. Guarded
on a real size change (lastPinnedTotalSizeRef) so the pin's own scroll-induced
re-render can't loop; scrollToBottom records the pinned size to avoid a
redundant double-fire.
Distinct from the mid-scroll prepend retention (a8b21597) and layered cleanly on
the scrollMargin work + existing sticky-bottom autoscroll — no regression to
either. Main timeline only; thread pane untouched. Two DOM tests lock in: re-pin
on size growth while pinned, and NO re-pin after the user scrolls away.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Fixes two loading-state jank symptoms tho flagged on the main timeline, both
the same root cause.
Root cause: the scroll container holds content ABOVE the virtualized list inside
the SAME scrollable element — the pagination sentinel, the load-older spinner,
and the channel/DM intro banner. @tanstack/react-virtual positions items at
paddingStart + scrollMargin and defaults scrollMargin to 0, so it assumed row 0
sat at scrollTop 0 when it actually painted lower by the above-content height.
That offset mismatch produced:
- the header/list 'sandwich' (freshly-loaded rows wedging into the seam between
the intro/spinner and the list)
- viewport drift while rows filled above + below (the anchor math was off by a
variable above-content height)
Fix: measure the list's offset within the scroll container and feed it as the
virtualizer's scrollMargin (useVirtualScrollMargin — re-measures on a
ResizeObserver + when the intro/spinner/list visibility changes). Rows are now
positioned at virtualItem.start - scrollMargin within the spacer, which sits at
that offset, so item offsets line up with where they paint regardless of what's
above. The intro banner stays a sibling above the list and is only visible at
the very top, as intended; native key-stable prepend retention now holds steady
because the offset origin is correct.
Main timeline only; thread pane untouched. Adds a DOM test locking in the
start - scrollMargin positioning.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Phase 2 follow-on: a single self-contained TimelineDebugOverlay over the main
timeline so behavior can be watched live while the virtualizer is young.
Read-only by design: it only READS state the hook and virtualizer already
expose — rendered-rows vs total, msg/divider split, overscan, visible index
range, measured row-height min/max, totalSize, scrollTop, clientHeight,
isAtBottom, newMessageCount, and the active jump target (deep-link / find /
highlight). It never touches, wraps, or perturbs the scroll path, so there is
zero risk to sticky-bottom / prepend-retention / deep-link.
Ship-then-rip: removal is delete this one file plus its single import + single
render line in MessageTimeline.tsx. No flag-gate hook, no localStorage toggle —
nothing else references it, so it leaves zero residue. Main timeline only;
thread pane untouched.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Phase 2: render only the viewport (+overscan) of the main timeline instead of
all ~200+ rows, lifting the ~2,000-row ceiling. Main timeline ONLY — the thread
pane (and its useTimelineScrollManager) is untouched (Phase 3).
What changed:
- MessageTimeline now drives @tanstack/react-virtual off the same deferred
snapshot the rows render from, flattening day-grouped entries into
index-addressable rows via buildVirtualTimelineRows. Rows are built from the
FILTERED main-timeline entries (buildMainTimelineEntries), so dropped thread
replies never desync the row->entry mapping.
- useVirtualTimelineScroll REPLACES the bespoke 427-line useTimelineScrollManager
for the main timeline. The virtualizer owns the scroll container and all
measurement/anchoring; this hook is a thin wrapper keeping sticky-bottom
autoscroll, accent smooth-scroll, the newMessageCount pill, isAtBottom, and
deep-link + find-in-page jumps (via findVirtualRowIndexForMessage ->
scrollToIndex). No scrollTop locking, no ResizeObserver re-pinning.
- useLoadOlderOnScroll is gutted to a trigger: the double-requestAnimationFrame
scrollTop correction and restoreScrollPosition plumbing are DELETED. Stable
per-row keys (getItemKey) let the virtualizer hold scroll position on prepend
natively — the band-aid is gone at the root.
- cmd+F find drives scroll-to-row through the same scrollToIndex bridge; a
render-all-while-searching escape hatch exists as an off-by-default fallback
(the in-app path is the default, keeping the perf win).
- The per-entry render (3 row variants + video-review context) is extracted to
renderTimelineEntry + useVideoReviewContextById, a single source of truth
injected into VirtualizedTimelineList. The old nested TimelineMessageList is
deleted (orphaned by the rewire).
Must-keeps preserved: sticky-bottom autoscroll, day dividers (first-class
variable-height flat rows), jump-to-message deep links, no-tearing (same
deferredMessages snapshot drives both rows and scroll logic).
Coverage: 8 jsdom/testing-library DOM tests cover row dispatch (divider vs
message, flat-index->entry mapping), init autoscroll, accent smooth-scroll, and
deep-link scrollToIndex. jsdom has no layout engine, so react-virtual's pixel
measurement/scroll math stays on a manual/visual verification pass.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Phase 2 (b)-lite, slice 1: establish a thin DOM test lane so the upcoming
virtualized scroll rewrite ships with automated coverage, not just manual checks.
The repo's test runner is `node --test` over `*.test.mjs` with no jsdom and no
JSX transform, so component rendering wasn't testable. This adds a second lane
that coexists with the existing pure-logic lane:
- esbuild `load` hook transforms .ts/.tsx (TS + JSX, automatic React runtime)
- global-jsdom installs DOM globals via `--import`
- @testing-library/react + /dom for render/query
- `pnpm test` now runs both lanes (test:unit + test:dom)
- DOM tests are `*.dom.test.tsx`
Deliberately thin: jsdom has no layout engine, so it does not cover
react-virtual's real measurement/scroll math — those stay on the
manual/visual verification pass (see FEASIBILITY.md). One smoke test proves
the net is wired.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Read-only Phase 2 feasibility spike (NOT ship-ready). Assesses whether
@tanstack/react-virtual can cleanly own the main timeline, replacing the
bespoke scrollTop-locking scroll manager and the ~2,000-row render ceiling.
Artifacts:
- buildVirtualTimelineRows.ts: pure helper flattening day-grouped entries
into a flat indexed row list (day-divider + message kinds) the virtualizer
can measure/key; includes findVirtualRowIndexForMessage for deep-link/find.
- buildVirtualTimelineRows.test.mjs: 9 tests covering dividers, renderKey
keying, prepend key stability (native position-retention contract), and
flat-index lookup.
- __spike__/VirtualizedTimelinePoc.tsx: thin non-wired PoC showing the
react-virtual integration shape (autoscroll, prepend retention via
getItemKey, scrollToIndex for find/deep-link).
- __spike__/FEASIBILITY.md: honest verdict on the three scoped points —
sticky-bottom autoscroll, scroll-up pagination with native position
retention, and cmd+F find-in-page plan.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
