Skip to content

feat(components): cap mobile session-group previews at 5 rows - #323

Merged
zxch3n merged 3 commits into
mainfrom
feat/mobile-session-group-preview-cap
Sep 4, 2026
Merged

feat(components): cap mobile session-group previews at 5 rows#323
zxch3n merged 3 commits into
mainfrom
feat/mobile-session-group-preview-cap

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

The mobile home list rendered every Session in every bucket, so one project
with forty Sessions pushed every other project and worktree below the fold.
The desktop sidebar has capped its groups at MAX_VISIBLE_SESSIONS for exactly
this reason; mobile never got it.

Each bucket now previews MOBILE_CHAT_PREVIEW_MAX_ROOTS (5) top-level rows
behind a Show all (N) / Show less toggle, reusing the existing
sessions.showAll / sessions.showLess keys. It counts TOP-LEVEL rows through
countOpenedByTreeRoots and applies maxRoots after rootRank, so the
preview truncates the pinned-first / latest-activity order rather than
reshuffling it, and never splits an opener from the Sessions it opened.

1. Why 5, not the 3 that was originally suggested

A phone already fits roughly eight rows. At 3, every project becomes a two-tap
read and the toggle is back in your way on every single bucket — the cap starts
costing more attention than the clutter it removes. 5 is the point where a
project's recent work still reads as a list, while an idle project costs one
line more than its heading.

It also keeps the two platforms saying the same thing about the same workspace:
5 is the desktop MAX_VISIBLE_SESSIONS. The value is a single exported constant
(MOBILE_CHAT_PREVIEW_MAX_ROOTS) if we want to tune it later.

2. Why home only

Scoped by an explicit capGroupPreviews prop. Workspace home passes it; the
in-project list does not.

The structural reason is that the project page has almost no grouping to cap.
chat-landing.tsx:6153 force-pins chatGroupBy="none" there regardless of the
home tab's view-mode atom, so that surface has at most two buckets: Pinned,
and one unlabeled flat tail. A cap would therefore not be trimming one project
among many — it would be hiding the tail of the single list the page exists to
show.

That matters specifically for the worktree-visibility ask this change came from.
Each ConversationRow carries a per-row WorktreeIcon driven by isWorktree
(mobile-project-screen.tsx:602), so inside one project the rows are exactly
where worktree identity is visible. Capping the flat tail at 5 would hide rows
6..N and their worktree markers — the opposite of the request. On home, the same
cap does the reverse: it stops one project from burying the other projects and
worktrees.

One correction for anyone tracing this: the project page also passes
rowSecondaryField="branch", but that prop renders nothing today.
ConversationRow is single-line and explicitly discards it
(mobile-project-screen.tsx:448 / :482, void _secondaryField), with
age/branch/project meta omitted. It is retained for call-site compatibility
only, so it is not evidence for anything here — the WorktreeIcon is.

3. Why no virtualization

Two structural blockers, both already documented in mobile/AGENTS.md:

  • VList must own the scroll element, but the home screen owns it.
    Pull-to-refresh translates that whole subtree, the dock-collapse listener
    reads it, and hidden home tabs stay mounted so their scroll position survives
    a tab round-trip.
  • contain: strict on VList makes the list its own stacking context,
    which would strip the liftAboveEdgeSwipeZone escape the opener fold chevron
    depends on to clear the 48px edge-back swipe strip.

So the cap bounds the row count instead. Measured in Chromium at 393x852 with
200 Sessions across 12 projects, capped vs uncapped:

capped uncapped
rendered rows 60 200
DOM nodes in the list 555 1219
median mount-to-stable 142ms 201ms

(Median of 7 runs. The mount figure includes a fixed ~80ms frame-settling
window in the harness, so the actual work is closer to 62ms vs 121ms.) This is
the cost that mattered: atoms/doc-meta.ts flushes metadata in batches of 50
per setTimeout(0), and every batch re-renders the whole list, so first paint
was O(rows) x O(N/50). Each row is a framer-motion motion.div with layout,
so the row count is the multiplier.

Happy to revisit virtualization if the cap proves insufficient, but it would
need the scroll-ownership and stacking-context problems solved first, and this
change makes it much less urgent.

4. Out of delegated scope, kept deliberately: data-loss guard

The cap is suspended while archived multi-select is active. This was not
part of the request, and I would normally leave it out — but "select all"
operates on every id in the list, not on the rendered rows. A capped surface
would let the user confirm a permanent delete of Sessions it never showed them.
Group collapse already had this shape; adding a cap on by default would have
made it the common case. Flagging it explicitly so it gets reviewed on its own
merits rather than riding along as layout polish.

Design notes

Verified in a real browser at 393x852:

  • The label sits on the row-title x (42px) via an empty 16px leading slot.
    That column carries a row's status indicator and an opener's fold chevron, so
    a chevron there would read as one of those. Same construction as the desktop
    sidebar's "Show all".
  • 13px medium muted, min-h-11 full-width: below rows (15px foreground) and
    headings (14px semibold) in the hierarchy, while keeping a row-sized thumb
    target.
  • active:bg-muted/40 press wash, matching ConversationRow. No scale(0.96)
    — scaling a full-bleed row reads wrong.
  • The preview state joins the AnimatePresence key, so "Show less" remounts
    instead of running 35 simultaneous 0.4s height exits.
  • Collapsing calls scrollIntoView({ block: 'nearest' }) from a layout
    effect. Chrome's native scroll anchoring would cover this, but WebKit has
    never shipped overflow-anchor and iOS is the target. Verified: the toggle
    stays in view (347px -> 310px) after collapsing a 14-row bucket.
  • Confirmed by touch-tap probe that :focus-visible does not match on tap
    (boxShadow: none), so the global inset focus ring is keyboard-only. Kept.

Testing

  • pnpm check passes end to end: typecheck, lint, tests, i18n, and all three
    boundary guards.
  • 10 new tests in packages/components/tests/mobile-chat-list-preview-cap.test.tsx
    covering the cap, opt-out, root counting vs raw-row counting, opener groups
    surviving the cut, pinned-first truncation, the shared-atom isolation, the
    multi-select suspension, and the scroll anchoring. No real sleeps — the
    long-press test drives vi.useFakeTimers().
  • Storybook: GroupPreviewOverflow and GroupPreviewExpanded.

Related

Same batch, all independently reviewable and mergeable — no overlapping files
with this PR: #317 (mobile eager-sync staging), #319 (Live Activity throttling),
#320 (login white flash).

Model: claude-opus-5[1m]

The mobile home list rendered every Session in every bucket, so one project
with forty Sessions pushed every other project and worktree below the fold —
the desktop sidebar has capped its groups at MAX_VISIBLE_SESSIONS for exactly
this reason and mobile never got it.

Each bucket now previews MOBILE_CHAT_PREVIEW_MAX_ROOTS (5) top-level rows
behind a "Show all (N)" / "Show less" toggle, reusing the existing
sessions.showAll / sessions.showLess keys. It counts TOP-LEVEL rows through
countOpenedByTreeRoots and applies maxRoots AFTER rootRank, so the preview
truncates the pinned-first / latest-activity order rather than reshuffling it,
and never splits an opener from the Sessions it opened.

Scoped by an explicit capGroupPreviews prop: workspace home passes it, the
in-project list does not — you drilled into that page to read exactly that
list. The cap is also suspended while archived multi-select is active, because
"select all" operates on every id in the list and a capped surface would let
the user confirm a permanent delete of rows it never showed.

The list is deliberately not virtualized. VList must own the scroll element,
but the home screen owns it (pull-to-refresh translates that subtree, the
dock-collapse listener reads it, hidden home tabs stay mounted for scroll
position), and its contain: strict would strip the liftAboveEdgeSwipeZone
escape the opener chevron depends on. Measured on 200 Sessions across 12
projects at 393x852: 60 rendered rows instead of 200, 555 DOM nodes instead of
1219, median mount-to-stable 142ms instead of 201ms.

Model: claude-opus-5[1m]
Ablated every mechanism this branch added and kept only what a test or a
measurement defends.

The ablations that found something:

- Deriving the cap from `groupBy !== 'none'` instead of the explicit
  `capGroupPreviews` prop passed the whole suite. The prop stays — the project
  page is `none` for heading reasons, not cap reasons, so the derivation couples
  two unrelated decisions and fails silently in both directions. That was a test
  hole rather than dead code, so the opt-out test now also pins that the cap is
  independent of grouping, and it fails under the derivation.
- The `scrollIntoView` layout effect looked redundant until the scenario was
  right: with the toggle near the document end, scrollTop clamping alone keeps
  it on screen. With content below it and `overflow-anchor: none` (Chromium
  emulating Safari, which ships no scroll anchoring) removing the effect drops
  the toggle from y=328 to y=-68. Kept, comment now cites the measurement.
- The `AnimatePresence` key does what it claims: with the key carrying only
  `archived`, collapsing a 14-row bucket still reports 14 rows a frame later,
  animating out for 400ms. Comment now states the measured 14, not an
  extrapolated 35.

Tests 10 -> 9. The exact-at-cap boundary folded into the main preview test as a
third bucket, so one list now covers over / exactly at / under the cap. The
scroll test asserts which element the list asks to keep on screen instead of
tallying calls to a patched prototype. Dropped an unreachable `if (toggle)`
guard in the expanded story.

Nothing was removed for being uncovered: the multi-select suspension and the
overflow gate each fail tests when ablated, and the shared-accessors constant
stays as drift prevention, matching the desktop `SESSION_ROW_OPENED_BY_TREE_ACCESSORS`.

Model: claude-opus-5[1m]

zxch3n commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Ablation results

Every mechanism this branch adds was removed one at a time and re-checked.
Gate per ablation: @lody/components typecheck + full vitest suite (421 files /
3028 tests) + lint:i18n. Scroll and animation behaviour cannot be observed in
jsdom, so those two were measured in Chromium at 393x700 against the
GroupPreviewOverflow story.

# Ablated Result Verdict Reason
A capGroupPreviews prop, derived groupBy !== 'none' instead suite passed kept + test added Equivalent today, but only by coincidence — see below
B scrollIntoView layout effect passed suite; failed in browser kept Toggle drops to y=-68 without it once scroll anchoring is off
C preview state in the AnimatePresence key passed suite; failed in browser kept 14 rows still in the DOM a frame after collapse, animating out
D overflowsPreview gate (always render the toggle) 4 tests failed kept Load-bearing, well covered
E CHAT_OPENED_BY_TREE_ACCESSORS constant (inlined both copies) passed kept Drift prevention, not dead code — see below
F multi-select suspension (coverage probe only) 1 test failed kept Data-loss guard, coverage confirmed adequate
G GroupPreviewExpanded story n/a kept Distinct requested state; its if (toggle) guard was unreachable and is gone

A — the one that found a real hole

Replacing the prop with groupBy !== 'none' passed the entire suite. That is a
signal, not a verdict, so: is it redundant, or load-bearing and untested?

Load-bearing and untested. The project page is groupBy="none" only because
chat-landing.tsx:6153 pins it so a single-bucket list does not draw a
redundant heading — a presentation decision with nothing to do with capping.
Deriving from it couples the two through a shared variable and fails silently in
both directions: give the project page a date mode and the cap switches on and
starts hiding worktree rows (the opposite of this PR's purpose); give home a
flat mode and the cap vanishes. groupBy also defaults to none, so any new
caller silently opts out.

So the prop stays, the rationale is now in the code, and the opt-out test was
extended to pin that the cap is not a function of grouping. Under the
derivation that test now fails. Previously nothing did.

B — measured, because the first measurement was wrong

My first attempt showed the effect doing nothing (347 -> 310 with and without
it). That scenario was degenerate: the toggle sat near the end of the document,
so the browser clamped scrollTop on shrink and that alone kept it on screen.
With filler below it — which any real home list has — the picture changes:

Scroll anchoring Effect Toggle top after collapse In view
none (Safari/iOS) removed -68 no
none (Safari/iOS) present 0 yes
Chrome default removed 310 yes

This also confirms the code comment: Chrome's native anchoring does cover it, so
the effect is precisely the WebKit fallback. It is not subsumed by C — C was
present in every run above.

C — verified, and the number corrected

With the key carrying only archived, collapsing a 14-row bucket reports 14
rows one frame later (all animating out, settling to 5 after ~400ms). With the
preview state in the key, the same frame already reports 5. The comment now
states this measured 14 rather than the extrapolated "35" it claimed before.

E — kept, but not because it is load-bearing

Inlining the accessors changes no behaviour and breaks no test. It is not dead
code either: the render pass and the overflow count must agree on what a "root"
is, or the toggle appears for a bucket that does not overflow. One definition
makes that drift impossible, and the desktop does the same thing with
SESSION_ROW_OPENED_BY_TREE_ACCESSORS (session-list.tsx:231). No test is
owed — "someone might edit one of two copies" is not testable.

Test pruning: 10 -> 9

Change Why
Folded leaves a bucket exactly at the cap untouched into the main preview test One list now spans over / exactly at / under the cap. The boundary still has its own assertion, so an off-by-one in the > is still caught
Rewrote the scroll test Was tallying calls to a patched Element.prototype.scrollIntoView. Now asserts which element the list asks the browser to keep on screen, plus that expanding moves nothing
Extended the opt-out test Now also asserts the cap ignores groupBy — closes the ablation-A hole
Removed if (toggle) in the expanded story findAllByRole already throws; the guard was unreachable and would have let the story silently render the wrong state

Nothing was deleted for being uncovered. The remaining 9 each fail under a
distinct ablation. I deliberately did not cut further to hit a number: the
two root-counting tests look similar but are not substitutable — with
chats.length > 5 instead of countOpenedByTreeRoots(...) > 5, previews five rows... still passes and counts top-level rows... fails, so that one is the
only thing distinguishing root-counting from row-counting.

The scroll test is the one judgment call against the letter of "no mock
assertions": jsdom has no layout, so the DOM node handed to the browser is the
only observable, and ablation B proved the mechanism real. Happy to drop it if
you would rather that live only in the PR record.

Cleanup

git diff origin/main...HEAD re-read in full. No console.*, no /tmp paths,
no performance.now instrumentation, no commented-out experiments, no
screenshots, no .tmp.mjs probes. The perf probe, the touch-focus probe and the
Playwright scripts were all scratch files outside the diff and are gone; working
tree is clean. pnpm check and pnpm format pass.

`overflowsPreview` depended on `preview`, which `MobileChatList` rebuilds as an
object literal for every bucket on every render, so the memo never hit. Fixing
that dependency alone does not help: measured with a counter around
`countOpenedByTreeRoots`, a state-only re-render (tapping one bucket's toggle)
still recomputed both buckets, because `groupChats` runs in the parent's render
body and hands every card a freshly built `chats` array too.

A memo keyed on `chats` therefore cannot hit in this component at all, so the
wrapper is removed rather than repaired — a cache that caches nothing costs a
deps array and a comparison on top of the scan it fails to avoid, and reads as
a guarantee that is not there. `previewEnabled` replaces the two `preview !=
null` reads so nothing depends on that literal's identity.

Behaviour is unchanged and the scan is O(rows) per bucket, dwarfed by the row
renders the cap removes. No test added: the only assertion available would be a
call tally, which is the shape deleted in the previous commit.

The pre-existing `treeNodes` memo above misses for the same upstream reason.
Left alone here: memoizing `groupChats` is not a one-liner because it takes
`nowMs = getServerNow()` and buckets by date, so caching it would freeze
Today/Yesterday headings until the session list changes.

Model: claude-opus-5[1m]
@zxch3n
zxch3n merged commit 7e33466 into main Sep 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant