From b8ec20cef43f94707fdc1cbb97b8ad581612741b Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Sat, 18 Jul 2026 23:27:49 -0300 Subject: [PATCH 1/3] fix(frontend): stop auto-scroll from hijacking scrollback during streaming bottomRef.scrollIntoView({behavior:'smooth'}) fired on every chatItems change, including every agent_token during streaming, yanking the view back to the bottom and making it impossible to read earlier output. Track whether the user is pinned near the bottom via a scroll listener + threshold and only auto-scroll while pinned; use behavior:'auto' while a bubble is actively streaming, and re-pin when the user sends a message. Closes #99 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4 --- frontend/src/app/page.tsx | 46 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/page.tsx b/frontend/src/app/page.tsx index 779a80e..2a70772 100644 --- a/frontend/src/app/page.tsx +++ b/frontend/src/app/page.tsx @@ -235,6 +235,17 @@ function HomePageContent() { const [htmlArtifacts, setHtmlArtifacts] = useState>(() => new Map()); const bottomRef = useRef(null); + // The actual scrollable chat pane (the `overflow-y-auto` div `bottomRef` + // sits at the bottom of). Used to measure scroll position for the + // pinned-to-bottom tracking below. + const chatScrollRef = useRef(null); + // Whether the user is scrolled near the bottom of the chat pane. The + // auto-scroll effect below only fires while this stays true — otherwise a + // user who scrolls up to read earlier output during a long streaming + // response gets yanked back to the bottom on every token. A ref (not + // state) because the scroll handler runs on every native scroll event and + // we don't want that to trigger a re-render. + const pinnedToBottomRef = useRef(true); const sseRef = useRef(null); const inputRef = useRef(null); const prevExperimentIdRef = useRef(null); @@ -290,16 +301,40 @@ function HomePageContent() { fileNames: string[]; } | null>(null); - // Auto-scroll on new chat items + // Auto-scroll on new chat items — but only while the user is pinned near + // the bottom of the pane. `chatItems` changes many times per second while + // an assistant reply streams token-by-token; without the pin gate, + // `scrollIntoView` fired on every single one of those changes and + // hijacked the scroll position, making it impossible to scroll up and + // read earlier output. `behavior: 'auto'` (no animation) while a bubble is + // actively streaming avoids stacking up smooth-scroll animations that + // fight each other; once streaming settles we go back to a smooth nudge. useEffect(() => { - bottomRef.current?.scrollIntoView({ behavior: 'smooth' }); + if (!pinnedToBottomRef.current) return; + bottomRef.current?.scrollIntoView({ + behavior: streamingItemIdRef.current ? 'auto' : 'smooth', + }); }, [chatItems]); + // Track whether the user is pinned near the bottom of the chat pane via a + // scroll listener + threshold, rather than assuming every render should + // snap back down. + const AUTO_SCROLL_PIN_THRESHOLD_PX = 96; + const handleChatScroll = useCallback(() => { + const el = chatScrollRef.current; + if (!el) return; + const distanceFromBottom = el.scrollHeight - el.scrollTop - el.clientHeight; + pinnedToBottomRef.current = distanceFromBottom <= AUTO_SCROLL_PIN_THRESHOLD_PX; + }, []); + // --------------------------------------------------------------------------- // addItem helper // --------------------------------------------------------------------------- const addItem = useCallback((item: Omit) => { + // The user just sent something — they're at the input, not mid-read of + // scrollback, so re-pin to bottom even if they'd scrolled up earlier. + if (item.type === 'user') pinnedToBottomRef.current = true; setChatItems((prev) => [ ...prev, { ...item, id: `${Date.now()}-${Math.random()}`, timestamp: Date.now() }, @@ -931,6 +966,7 @@ function HomePageContent() { setDraft([]); setIsRunning(false); streamingItemIdRef.current = null; + pinnedToBottomRef.current = true; setSessionState('created'); workspacePanelRef.current?.collapse(); setCanvasContent(''); @@ -2027,7 +2063,11 @@ function HomePageContent() { {/* Chat panel */}
-
+
From 05cb5397387dd6812a6744ce79f9aafa0696031c Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Mon, 20 Jul 2026 09:53:11 -0300 Subject: [PATCH 2/3] fix(review): address Greptile findings on #152 - P1: use behavior 'auto' unconditionally in the auto-scroll effect. A smooth scroll after send re-pinned and then animated through intermediate positions whose scroll events un-pinned the view mid-animation, so fast first tokens could leave the stream unfollowed. - P2: hoist AUTO_SCROLL_PIN_THRESHOLD_PX to module level so the constant is created once and is stable for the scroll-handler closure. - Add a manual test tutorial for the scroll-pinning behavior (no frontend unit-test harness exists, and jsdom cannot reproduce smooth-scroll animation events anyway). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4 --- docs/manual-tests/chat-autoscroll-pinning.md | 60 ++++++++++++++++++++ frontend/src/app/page.tsx | 20 ++++--- 2 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 docs/manual-tests/chat-autoscroll-pinning.md diff --git a/docs/manual-tests/chat-autoscroll-pinning.md b/docs/manual-tests/chat-autoscroll-pinning.md new file mode 100644 index 0000000..b862ecf --- /dev/null +++ b/docs/manual-tests/chat-autoscroll-pinning.md @@ -0,0 +1,60 @@ +# Manual test tutorial — chat auto-scroll pinning (PR #152 / issue #99) + +The frontend has no unit-test harness, and the behavior under test (browser +smooth-scroll animation, `scroll` events, streaming SSE tokens) cannot be +reproduced in jsdom anyway. Verify the scroll-pinning behavior manually with +this checklist in a real browser. + +## Setup + +1. Start the app (`docker compose up` or backend + `cd frontend && npm run dev`). +2. Open the chat page and start a session. +3. Send a few prompts that produce **long streaming replies** (e.g. + "Explain gradient descent in 2000 words") until the chat pane has enough + history to scroll (scrollbar visible). + +## Test 1 — pinned follow (baseline) + +1. Scroll to the very bottom of the chat pane. +2. Send a prompt with a long streaming reply. +3. **Expected:** the view stays glued to the bottom for the whole stream; + every new token is visible without touching the scrollbar. + +## Test 2 — scrollback is not hijacked (the original #99 bug) + +1. Send a prompt with a long streaming reply. +2. While tokens are still streaming, scroll **up** several screens and stop. +3. **Expected:** the view stays exactly where you put it; it does NOT jump + back to the bottom on each token. You can read old messages undisturbed + until the stream finishes. + +## Test 3 — re-pin by scrolling back down + +1. Continue from Test 2 (still streaming, scrolled up). +2. Scroll back down to the bottom (within ~96 px of it). +3. **Expected:** auto-follow resumes; the view sticks to the bottom for the + rest of the stream. + +## Test 4 — sending a message re-pins instantly (the Greptile P1 fix) + +1. With plenty of history, scroll far **up** in the pane (several screens). +2. Type a new prompt and hit send. +3. **Expected:** + - The view jumps **instantly** to the bottom (no smooth animation + gliding down — a smooth animation is exactly what used to un-pin the + view mid-flight). + - Your new user bubble is visible at the bottom. + - When the assistant's first tokens arrive (even if they arrive + immediately), the view **follows the stream to the end**. It must not + get stuck at an intermediate scroll position. + +Repeat Test 4 a few times with a fast-responding model; the failure mode it +guards against was a race between the smooth-scroll animation and +first-token latency, so it was timing-dependent. + +## Test 5 — threshold sanity + +1. During a stream, scroll up just a tiny nudge (well under ~96 px from the + bottom). **Expected:** still pinned; the view keeps following. +2. Scroll up more than ~96 px. **Expected:** un-pinned; the view stops + following (Test 2 behavior). diff --git a/frontend/src/app/page.tsx b/frontend/src/app/page.tsx index 2a70772..0f49ee9 100644 --- a/frontend/src/app/page.tsx +++ b/frontend/src/app/page.tsx @@ -159,6 +159,11 @@ const SUGGESTIONS = [ }, ]; +// How close (px) to the bottom of the chat pane the user must be for +// auto-scroll to stay "pinned". Module-level so the binding is created once +// and is unambiguously stable for the scroll-handler closure. +const AUTO_SCROLL_PIN_THRESHOLD_PX = 96; + // --------------------------------------------------------------------------- // Main page component // --------------------------------------------------------------------------- @@ -306,20 +311,21 @@ function HomePageContent() { // an assistant reply streams token-by-token; without the pin gate, // `scrollIntoView` fired on every single one of those changes and // hijacked the scroll position, making it impossible to scroll up and - // read earlier output. `behavior: 'auto'` (no animation) while a bubble is - // actively streaming avoids stacking up smooth-scroll animations that - // fight each other; once streaming settles we go back to a smooth nudge. + // read earlier output. `behavior: 'auto'` (instant, no animation) is used + // unconditionally: a smooth scroll animates through intermediate positions, + // and each intermediate `scroll` event would make `handleChatScroll` see + // `distanceFromBottom > threshold` and un-pin mid-animation — so if the + // first streaming tokens arrived before the animation landed, auto-scroll + // silently stopped. An instant jump fires a single scroll event already at + // the bottom, which keeps the pin state consistent. useEffect(() => { if (!pinnedToBottomRef.current) return; - bottomRef.current?.scrollIntoView({ - behavior: streamingItemIdRef.current ? 'auto' : 'smooth', - }); + bottomRef.current?.scrollIntoView({ behavior: 'auto' }); }, [chatItems]); // Track whether the user is pinned near the bottom of the chat pane via a // scroll listener + threshold, rather than assuming every render should // snap back down. - const AUTO_SCROLL_PIN_THRESHOLD_PX = 96; const handleChatScroll = useCallback(() => { const el = chatScrollRef.current; if (!el) return; From caf4423a680e7861f023db88cfef549a2dc6ad68 Mon Sep 17 00:00:00 2001 From: Lucas Tonon Date: Wed, 29 Jul 2026 15:28:46 -0300 Subject: [PATCH 3/3] =?UTF-8?q?chore(staging):=20ledger=20row=20=E2=80=94?= =?UTF-8?q?=20PR=20#150?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- STAGING-v0.0.5.md | 1 + 1 file changed, 1 insertion(+) diff --git a/STAGING-v0.0.5.md b/STAGING-v0.0.5.md index 6a44fae..f1e810d 100644 --- a/STAGING-v0.0.5.md +++ b/STAGING-v0.0.5.md @@ -49,6 +49,7 @@ branch → run relevant tests → record below. | 29 | #165 | fix/107-cost-budget | #107 | P1 (non-BudgetExceededError from `check_budget` landing session in `failed`) already fixed by author's fixup `1bfed27` (`_check_budget_failopen` + 2 tests); P2 (two-SELECT atomicity in `get_budget_status`) dismissed by author with reasoning (AsyncSession autobegin shares one transaction; race direction is conservative; re-checked after every usage event) | branch repaired in triage worktree `f165`: staging merged in, 8 conflict files resolved preserving BOTH features (training controls AND budget enforcement) — `ProjectSettingsModal.onSave` signature combined to `(config, budgetUsd, training)`; hand-rolled `ALTER TABLE projects ADD COLUMN budget_usd` converted to Alembic revision `8e4b2d6f1a35_projects_budget_usd` (chained on `3f7a1c9e2d64`); alembic tests extended to assert `budget_usd` | branch: full suite **463 passed, 8 skipped**, ruff check + format --check clean (168 files), tsc/prettier/vitest 4/4 clean; post-merge staging: **463 passed, 8 skipped**; alembic chain verified linear, single head `8e4b2d6f1a35`; CLI `alembic upgrade head` fresh SQLite → both new columns present | conflicts kept both: `models.py`, `routers/projects.py`, `schemas.py`, `runner.py` (budget pre-check + wall-clock clamp), `ProjectSettingsModal.tsx` (Budget + Training Controls sections), `Sidebar.tsx`, `api.ts`, `types.ts`; pushed `712e37f` verified as merge parent | | 30 | #87 | feat/workspace-download | #79 | rounds 1–2 already fixed by author (`7dc0d1b`, `8cfc952`): log_table rows iterator, cap-exceeded full file reads, yield-in-finally async generator, unused `READ_CHUNK_BYTES`, 404-on-empty-sessions convention, partial-read zip entries marked in `__truncated.txt`; round-3 P1 (`log_confusion_matrix` exhausts `y_true`/`y_pred` iterators → silent all-zero matrix) outstanding → fixed on PR branch in `1f6ad6d` (materialize both to lists once up front; +2 regression tests in new `backend/tests/test_trainable_runtime.py`) | ruff UP031 (branch predates pinned config): 2 `%`-format raises → f-strings (`f80d7f4`); `frontend/node_modules` worktree symlink accidentally committed via `git add -A` — dropped in `f313653` (branch) and merged to staging as a follow-up merge; main checkout's real `node_modules` had been replaced by that symlink during the merge → reinstalled via `npm ci` | branch: full suite **477 passed, 8 skipped**, ruff check + format --check clean (174 files), tsc/prettier/vitest 4/4 clean; post-merge staging: **477 passed, 8 skipped** | no `db.py`/`models.py` changes → no Alembic revision needed; conflicts: `backend/main.py` router includes (kept both `download` + `samples`); `sandbox.py` auto-merged — #87's refactor (inline SDK preamble → shared `services/trainable_runtime.py`) intact, staging's pre-sandbox volume bootstrap preserved; `conftest.py` auto-merged (MockVolume chunked-read + error injection); `page.tsx`/`Sidebar.tsx` auto-merged; pushed `f313653` (contains merge `f80d7f4` + P1 fix `1f6ad6d`) verified as merge parent | | 31 | #148 | fix/100-dedupe-eventsource | #100 | round-1 findings (P2 second-context vs lib/AGENTS.md; P2 cross-session event bleed) already fixed by author's fixup `3f73570` (session-scoped `publish` + documented stateless bus context + 5 vitest tests); round-2 P1 (throwing bus listener silently kills `connectSSE`'s UI switch — `publish` ran inside the try/catch before the switch) outstanding → fixed on PR branch in `cfbfb40` (per-listener try/catch in `SSEStreamContext.publish`, logs and continues) | branch repaired against staging in `8f74aab`: conflict only in `frontend/package-lock.json` (lockfile churn) — took staging's via `--theirs`, regenerated with `npm install` against merged package.json; `frontend/node_modules` verified real directory (not symlink) | branch: vitest **9/9** (4 api + 5 bus/notebook), tsc clean, prettier clean; post-merge staging: vitest **9/9**, tsc clean, prettier clean | `page.tsx` auto-merged — verified #148's `SSEStreamProvider`/`publish(sid, event)` AND #169's `takeSuggestedPrompt`/`Sparkles` wiring AND #87's `/api/sessions/{id}/download` UI all present; pushed `8f74aab` (contains P1 fix `cfbfb40`) verified as merge parent | +| 32 | #150 | fix/98-memoize-chat | #98 | both P2 findings (shared `streamingItemId` string prop breaks `memo` bailout for ALL items on stream start/end) already fixed by author's fixup `b37907e` (per-item `isStreaming` boolean + manual-test tutorial `docs/manual-tests/chat-memoization.md`) | branch repaired against staging in `3e48748` — ort auto-merged CLEAN (no conflicts; branch was stacked on #148's original commit, staging already carried #148's fixup + P1 repair) | branch: vitest 9/9, tsc clean, prettier clean; post-merge staging: vitest 9/9, tsc clean, prettier clean | `page.tsx` merge kept memoized `ChatItemView` (`isStreaming={cur.id === streamingItemId}` verified) + #148 bus + #169/#87 wiring; pushed `3e48748` verified as merge parent | ## Deferred / not merged