Scroll longer recent-session lists instead of capping at five - #191
Scroll longer recent-session lists instead of capping at five#191leoncheng57 wants to merge 2 commits into
Conversation
"Recently opened" and "Recently active" were capped at five rows, which is too few to be useful once a few projects are in play. The cap lived in three coordinated places, so raising any one of them silently did nothing: the client view helper clamped with Math.min, the BFF clamped the window it returned, and the client's own request default asked for five to begin with. Raise the bound to 100 and let each column scroll inside a fixed-height container, so the section keeps roughly the height it already had. Both clamps keep their Math.min shape, so neither side can widen the other, and api.recentSessions now derives its request default from MAX_VISIBLE_RECENT_SESSIONS rather than repeating the number a third time. Raising the server bound costs no extra upstream requests: the per-directory pool is already fetched at RECENT_SESSION_CONTEXT_LIMIT and the limit only slices that existing pool. MAX_STORED_RECENT_SESSIONS stays at 50 — that is localStorage retention for this browser's open-history, a different concern. The columns are grid children of an overflow-hidden grid, so they need min-h-0; without it min-height: auto lets the list push the section taller instead of scrolling.
…ssions # Conflicts: # tests/e2e/mock-opencode.ts
|
Merged Conflict resolution: only Re-verification on the merged tree: typecheck clean, Note on an environmental failure, not a code failure: the first post-merge e2e run reported three workspace failures. The cause was a corrupted shared fixture — The two flaky specs ( |
Interactive PR previewDeployed from The simulator contains deterministic fixture data only. It has no OpenCode process, AI provider key, GitHub token, repository secret, or access to a contributor filesystem. |
PR screenshots
Source: Captured from the production app against deterministic mock OpenCode fixtures only. |
1 participant
What changed and why
"Recently opened" and "Recently active" were capped at five rows. Five is too few
once more than one or two projects are in play, and the issue asks for a much
larger bound while keeping the vertical space the section already occupies.
The cap lived in three coordinated places, which is why nobody could raise it
by changing one number — the other two silently put it back:
client/lib/recentSessions.ts—MAX_VISIBLE_RECENT_SESSIONS, applied by theMath.minclamp invisibleLimit()server/routes/recents.ts—RECENT_SESSION_LIMIT, bounding the merged "recently active" windowclient/lib/api.ts—recentSessions(..., limit = 5), the number actually requestedMAX_VISIBLE_RECENT_SESSIONSBoth clamps keep their
Math.minshape, so a caller still cannot widen the boundby asking for more — the two halves are enforced independently and the browser's
number is not trusted by the BFF. The third site is now derived rather than
repeated:
api.recentSessionsimports the client constant, so the requestedwindow and the rendered window cannot drift. (That import is safe:
recentSessions.tsonly imports a type fromapi.ts, so the cycle is erased atbuild time.)
The section keeps its height by scrolling internally. Each column became
flex flex-col, and the list sits in athin-scrollbar max-h-60 min-h-0 flex-1 overflow-y-auto overscroll-containcontainer with a
${testId}-scrolltestid.min-h-0is load-bearing: thesecolumns are grid children of an
overflow-hiddengrid and flex children of thecolumn, and both default to
min-height: auto, which would let the list push thesection taller instead of scrolling.
max-h-60(240px) is about five 44px rows —the height this section already occupied back when five was also the hard cap —
with a sliver of the sixth row visible as a scroll affordance. Both columns share
the bound, so the section cannot become lopsided.
Not changed, deliberately:
MAX_STORED_RECENT_SESSIONSstays 50. That is localStorage retention for thisbrowser's own open-history, a different concern; 50 stored feeding a 100-row
window is coherent.
RECENT_SESSION_CONTEXT_LIMITalready fetches 100sessions per directory and
limitonly slices that existing pool, so raisingthe server bound is free.
RECENT_DIRECTORY_LIMIT(40) is untouched.AGENTS.md:340).AGENTS.mdgains decision 12a recording the row-budget framing, the two-clamprule and the
min-h-0trap.Verification actually run
All commands run in this worktree, on the assigned ports, with the full e2e suite
under the shared
/tmp/custom-dca-opencode-e2e.lock.npm run typechecknpm test— baseline before editingnpm test— afternpm run buildtsc -p tsconfig.server.json)CI=1 PORT=3533 MOCK_OPENCODE_PORT=4736 MOCK_PREVIEW_PORT=4737 npm run test:e2eNotes on the e2e run:
workflows.ui.spec.ts:206 "keeps the picker attachment out of an ordinary send". It passed on retry, is unrelated to recents, andtouches none of the files in this diff.
screenshots.ui.spec.ts, which self-skips unless thescreenshot runner supplies a request file.
New coverage:
tests/recent-sessions.test.ts— the assertion that hard-codedtoHaveLength(5)for a 10-session pool now expects all 10, plus two new cases:the clamp still refuses
Number.MAX_SAFE_INTEGERand a 10 000 limit (returningexactly
MAX_VISIBLE_RECENT_SESSIONS), non-finite limits still return nothing,and twenty sessions render as twenty rows.
tests/recent-fanout.test.ts— the route's limit clamp was previously inlineand untested. It is now
recentSessionLimit()(pure, exported, same behaviour)with four new cases: the bound is ≥100, it clamps down and never up, negatives
and fractions and junk are handled without collapsing the panel to zero rows,
and the maximum stays injectable.
tests/e2e/recents-capacity.ui.spec.ts— 7 new browser tests, described below.tests/e2e/mock-opencode.ts— a new/tmp/mock-recents-projectfixture withtwelve flat
ses_recents_*sessions. PerAGENTS.md's shared-state ownershiprule this directory is owned by the new spec file alone; adding the sessions to
MOCK_DIRECTORYwould have moved the exact row countssmoke.ui.spec.tsasserts. The new spec also intercepts
/api/recent-sessionsto filter the poolto its own fixtures, because the BFF unions requested directories with the
shared pin file and a concurrent spec's pin would otherwise add rows.
tests/e2e/smoke.ui.spec.ts—toHaveCount(5)in "merges recently activeacross projects newest first" now reads
toHaveCount(pinnedFixtures.length)with a comment saying it is a fixture-pool assertion, not a cap assertion.
(It did in fact still pass unchanged, because
pinRecentsTofilters theresponse to exactly five ids — but leaving a bare
5there would havemisdescribed the system.
pinRecentsToitself is untouched and still working.)The new browser tests assert: both panels render 12 rows (>5); rows are ordered
newest-first and a row past the fifth is really in the DOM; the section stays
shorter than
rows × rowHeightwhile each scroll container overflows; each columnscrolls independently and moving one does not move the other or the page; a
focused row scrolls itself into view inside the container and
Enternavigates;clicking the seventh row navigates to that session; the empty state still renders
inside the scroll wrapper; and at 390px there is no horizontal overflow and rows
keep their 44px minimum height.
Screenshots
Read these two routes together, and note what each does and does not prove.
/?directory=/tmp/mock-recents-projectis the new fixture project and is thecapacity proof: "Recently active" holds twelve sessions, the column is clipped
mid-row with the sixth peeking below the fifth, and the section is still about
the height it was at five rows.
/?directory=/tmp/mock-projectis the pre-existing deterministic fixture anddoes not demonstrate the new capacity: that project only has four
non-archived sessions, so it renders four rows either way. It is included to
show the unchanged appearance at low row counts, not the new bound.
In both routes "Recently opened" is empty. That panel is driven by
localStorage, which the screenshot runner does not seed, so no captured imagecan show the opened column populated. Its capacity is proven by
recents-capacity.ui.spec.tsinstead, which seeds the history and asserts twelverows there too. A screenshot is not proof of interaction — nothing below the
"Human verification" heading is established by these images.
Captured locally with
npm run screenshots:local(pointed at the request fileabove) before opening this PR; both routes validated.
Human verification
Run against a real
opencode servewith a project that has more than fivesessions, plus at least one second project.
Expect: "Recently active" lists more than five rows.
Failure signal: exactly five rows and no more, which means one of the three
caps was missed — check the BFF response in DevTools before blaming the UI.
/api/recent-sessions?limit=…response for that load.Expect:
limit=100in the request URL, and asessionsarray longer thanfive.
Expected failure signal:
limit=5in the URL (the client default did notmove) or a five-element array despite
limit=100(the server clamp did notmove).
Hub between each.
Expect: "Recently opened" grows past five rows, newest first.
Failure signal: the list stops growing at five, or reorders unpredictably.
main(same window, same zoom) — measure from the top border to the bottomborder.
Expect: roughly unchanged, ~240px of list per column plus the heading.
Failure signal: the section is now hundreds of pixels tall and pushes
"Choose a project" below the fold. That is
min-h-0failing to apply.mouse wheel.
Expect: only that column scrolls; the page behind it does not move, and
"Recently opened" stays where it was.
Failure signal: the whole page scrolls instead, or both columns move
together.
scrolling.
Expect: the page does not start scrolling (
overscroll-contain).Failure signal: scroll chaining — the page jumps once the column bottoms out.
Expect: navigation to that exact session's conversation, in that session's
own project directory (check the
?directory=in the address bar).Failure signal: the wrong session opens, or the URL carries the currently
selected project instead of the row's own project.
and keep going past the visible rows.
Expect: every row is reachable, and the column scrolls the focused row into
view; the focus ring is never hidden behind the column edge.
Failure signal: focus disappears into a clipped region, or Tab skips the
rows below the fold entirely.
Expect: the same navigation as clicking it.
Failure signal: nothing happens.
localStorage(opencode.recentSessions.v1) and reload.Expect: "Recently opened" shows "Open a session to keep it handy here." and
the section does not collapse or become lopsided next to a populated
"Recently active".
Failure signal: a blank column with no message, or the two columns at
visibly different heights.
Expect: the columns stack, each still scrolls within its own budget, rows
stay at least 44px tall, and there is no horizontal scrollbar.
Failure signal: a horizontal scrollbar, or the stacked section consuming
most of the screen height.
Expect: the column renders only those rows with no internal scrollbar, and
looks as it did before this change.
Failure signal: a scrollbar on a short list, or dead space forcing the
section taller than its content.
Classification
VERIFIED (executed in this session, output above):
313 passed / 1 flaky (pre-existing, unrelated) / 1 skipped.
client helper and the extracted server helper.
height bounded below
rows × rowHeight, both containers overflowing, columnsscrolling independently without moving the page, focus scrolling a row into view
inside the container, Enter and click both navigating to the correct session,
the empty state rendering inside the wrapper, and no horizontal overflow at
390px with a full list.
the
/tmp/mock-recents-projectimage.UNVERIFIED (not reachable from this environment):
opencode servewith real sessions. All evidencehere is the deterministic mock; no live agent was contacted.
overscroll-containchaining(human steps 5, 6, 11). The e2e sets
scrollTopprogrammatically, which provesthe container is a scroller and is independent per column, but not that a
trackpad gesture behaves correctly or that scroll chaining is suppressed.
asserts the container scrolled; it does not walk the tab sequence, so "every row
is reachable by Tab" is asserted by inspection only.
main(human step 4). Asserted here as an absolute bound(≤
max-h-60+ heading), not as a before/after diff againstmain.the 100 bound itself is only proven by the clamp unit tests.
prefers-reduced-motion/ assistive tech. No screen-reader pass was made onthe new scroll container.
FAILED: none.
Disposition: partially verified — ready for review, with the browser-gesture
and live-server items above left for a human.
Everything mechanically checkable is green and the change is proven end-to-end
against the mock. What remains genuinely requires a human at a device: real wheel
and touch scrolling, tab traversal by hand, and a live agent server.
Judgement calls
api.recentSessionsderives its default instead of hard-coding100. Theprompt named three places to change. Leaving three independent literals is what
created this bug, so the client's two were collapsed into one constant. The
server keeps its own number — a BFF must not import client code, and it must
clamp independently of what the browser claims.
recentSessionLimit(). Unit testsrun under
environment: "node"with no route harness, and there was no existingtest that mounted
recentRoutes. Extracting the pure function was the smallestway to get real assertions on the server half of the cap. Behaviour is
byte-identical, including the "unparseable falls back to the default, not zero"
branch.
max-h-60(240px), notmax-h-72(288px) as the project list below uses.The brief says keep the section's current height; 240px is ~five 44px rows,
which is what it occupied before. 288px would have grown it.
smoke.ui.spec.ts.AGENTS.md's shared-state rule: adding sessions toMOCK_DIRECTORYwould move the exact row counts other specs assert. The newspec also filters the recents response to its own fixtures, because the pin file
is shared across parallel spec files.
smoke.ui.spec.ts:297was reworded rather than left alone. It still passedunchanged —
pinRecentsToconstrains the pool to five ids, so the count is fiveeither way — but a bare
5in a recents test now reads as a cap assertion thatno longer exists.
main. This branch is cut from88ac654as briefedand is now 9 commits behind, because sibling agents merged while this ran.
Rebasing onto a moving target mid-flight would invalidate the verification above
without a chance to re-run it. The base is stated here so review can decide.
AGENTS.mddecision 12a was added. The repository records non-obviousdecisions there, and "the cap is enforced twice, both with
Math.min, andmin-h-0is what makes it scroll" is exactly the kind of thing the next personre-derives painfully.
Deliberately not done (follow-ups)
current/unknown ownership on these exact surfaces and Surface interrupted sessions in session lists #101 owns surfacing
interrupted sessions.
Hub.tsx:38-40) — unrelated to capacity.SessionTreeListor the sub-session disclosure — shared with themain session list; a capacity change must not restyle it.
MAX_STORED_RECENT_SESSIONSpast 50 — different concern, and the briefruled it out.
Closes #44.