Skip to content

fix(sessions): keep conversation pinned to bottom incl. footer#2500

Open
adamleithp wants to merge 1 commit into
mainfrom
scroll-to-bottom-fix
Open

fix(sessions): keep conversation pinned to bottom incl. footer#2500
adamleithp wants to merge 1 commit into
mainfrom
scroll-to-bottom-fix

Conversation

@adamleithp
Copy link
Copy Markdown
Contributor

Problem

The conversation list wasn't sticking to the bottom — the footer (context-usage chart) sat below the fold, the scroll-to-bottom button flickered, and during streaming the view drifted off the latest content.

Root cause: the footer was rendered as a fake virtual row with a constant key. tanstack's followOnAppend only fires when the last virtual key changes on append (virtual-core didEdgeKeysChange), so a perpetual constant-keyed last item silently disabled it. A hand-rolled [items] effect papered over appends but never covered the footer's own height changes (generating→done, compacting), stranding it below the DOM bottom. anchorTo:"end" also pinned to the virtual end (last message), excluding the non-virtual footer + pb-16 cushion.

Fix — make it tanstack-native

  • Take the footer out of the virtual count; reserve its measured height as paddingEnd so virtual-end == DOM-end.
  • anchorTo:"end" now handles in-place token growth, followOnAppend handles appends, and isAtEnd/scrollToEnd line up with the footer bottom — all native.
  • The one thing tanstack can't see (the footer is not a virtual item) is its own resize → covered by a ResizeObserverpaddingEnd + a single re-pin effect.
  • Scroll-to-bottom button gets reporting hysteresis so the append → measure-taller → re-pin cycle doesn't flicker it, plus a far-drift (>400px) escape hatch so follow can't get silently stuck. Button visibility is pure UI state; tanstack still drives all scrolling.

RawLogsView (the other consumer) passes no footer → paddingEnd=0, behaviour unchanged.

Test plan

  • Open a session — lands at the footer (true bottom), no button.
  • Stream a turn — view tracks tokens, button stays hidden.
  • Let the turn finish (footer swaps to "Generated in…") — stays pinned.
  • Scroll up — button appears; click — settles at footer; new content follows again.

🤖 Generated with Claude Code

The footer (context chart) was a fake virtual row with a constant key,
which permanently killed tanstack's followOnAppend (it only fires when
the last virtual key changes on append), so new messages never followed.
The hand-rolled [items] effect that papered over it didn't cover the
footer's own height changes, leaving it stranded below the fold.

Make stick-to-bottom tanstack-native: take the footer out of the count
and reserve its measured height as paddingEnd, so virtual-end == DOM-end.
anchorTo='end' then handles in-place token growth, followOnAppend handles
appends, and isAtEnd/scrollToEnd line up with the footer bottom. The only
non-native piece is the footer's own resize (not a virtual item), covered
by a ResizeObserver -> paddingEnd + single re-pin.

Scroll-to-bottom button gets reporting hysteresis so the append ->
measure-taller -> re-pin cycle doesn't flicker it, with a far-drift
escape hatch so follow can't get silently stuck.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@adamleithp adamleithp added the Stamphog This will request an autostamp by stamphog on small changes label Jun 5, 2026
@adamleithp adamleithp enabled auto-merge (squash) June 5, 2026 15:28
Copy link
Copy Markdown

@stamphog stamphog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review agent failed after 3 attempts — needs human review.

@stamphog stamphog Bot removed the Stamphog This will request an autostamp by stamphog on small changes label Jun 5, 2026
@adamleithp adamleithp requested a review from jonathanlab June 5, 2026 15:30
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 5, 2026

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/code/src/renderer/features/sessions/components/VirtualizedList.tsx:32-34
The far-drift threshold of `400` is the only magic number not extracted into a named constant alongside `AT_BOTTOM_THRESHOLD`, `ESTIMATED_ROW_SIZE`, and `OVERSCAN`. Naming it makes the intent explicit and keeps all tuning knobs in one place.

```suggestion
const AT_BOTTOM_THRESHOLD = 50;
const ESTIMATED_ROW_SIZE = 80;
const OVERSCAN = 6;
const FAR_DRIFT_THRESHOLD = 400;
```

### Issue 2 of 2
apps/code/src/renderer/features/sessions/components/VirtualizedList.tsx:186-188
Use the new named constant here so the threshold value is defined in exactly one place.

```suggestion
    const farFromEnd = el
      ? el.scrollHeight - el.clientHeight - scrollTop > FAR_DRIFT_THRESHOLD
      : false;
```

Reviews (1): Last reviewed commit: "fix(sessions): keep conversation pinned ..." | Re-trigger Greptile

Comment on lines 32 to 34
const AT_BOTTOM_THRESHOLD = 50;
const ESTIMATED_ROW_SIZE = 80;
const OVERSCAN = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The far-drift threshold of 400 is the only magic number not extracted into a named constant alongside AT_BOTTOM_THRESHOLD, ESTIMATED_ROW_SIZE, and OVERSCAN. Naming it makes the intent explicit and keeps all tuning knobs in one place.

Suggested change
const AT_BOTTOM_THRESHOLD = 50;
const ESTIMATED_ROW_SIZE = 80;
const OVERSCAN = 6;
const AT_BOTTOM_THRESHOLD = 50;
const ESTIMATED_ROW_SIZE = 80;
const OVERSCAN = 6;
const FAR_DRIFT_THRESHOLD = 400;
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sessions/components/VirtualizedList.tsx
Line: 32-34

Comment:
The far-drift threshold of `400` is the only magic number not extracted into a named constant alongside `AT_BOTTOM_THRESHOLD`, `ESTIMATED_ROW_SIZE`, and `OVERSCAN`. Naming it makes the intent explicit and keeps all tuning knobs in one place.

```suggestion
const AT_BOTTOM_THRESHOLD = 50;
const ESTIMATED_ROW_SIZE = 80;
const OVERSCAN = 6;
const FAR_DRIFT_THRESHOLD = 400;
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +186 to +188
const farFromEnd = el
? el.scrollHeight - el.clientHeight - scrollTop > 400
: false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Use the new named constant here so the threshold value is defined in exactly one place.

Suggested change
const farFromEnd = el
? el.scrollHeight - el.clientHeight - scrollTop > 400
: false;
const farFromEnd = el
? el.scrollHeight - el.clientHeight - scrollTop > FAR_DRIFT_THRESHOLD
: false;
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/sessions/components/VirtualizedList.tsx
Line: 186-188

Comment:
Use the new named constant here so the threshold value is defined in exactly one place.

```suggestion
    const farFromEnd = el
      ? el.scrollHeight - el.clientHeight - scrollTop > FAR_DRIFT_THRESHOLD
      : false;
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant