Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions STAGING-v0.0.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
60 changes: 60 additions & 0 deletions docs/manual-tests/chat-autoscroll-pinning.md
Original file line number Diff line number Diff line change
@@ -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).
52 changes: 49 additions & 3 deletions frontend/src/app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,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
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -237,6 +242,17 @@ function HomePageContent() {
const [htmlArtifacts, setHtmlArtifacts] = useState<Map<string, HtmlArtifact>>(() => new Map());

const bottomRef = useRef<HTMLDivElement>(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<HTMLDivElement>(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<EventSource | null>(null);
const inputRef = useRef<MentionInputHandle | null>(null);
const prevExperimentIdRef = useRef<string | null>(null);
Expand Down Expand Up @@ -295,11 +311,33 @@ 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'` (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(() => {
bottomRef.current?.scrollIntoView({ behavior: 'smooth' });
if (!pinnedToBottomRef.current) return;
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 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;
}, []);
Comment thread
greptile-apps[bot] marked this conversation as resolved.

// Seed the chat input with the suggested prompt handed off by the
// sample-dataset gallery (first-run flow). Consumed exactly once, and
// never clobbers something the user already typed.
Expand All @@ -316,6 +354,9 @@ function HomePageContent() {
// ---------------------------------------------------------------------------

const addItem = useCallback((item: Omit<ChatItem, 'id' | 'timestamp'>) => {
// 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() },
Expand Down Expand Up @@ -964,6 +1005,7 @@ function HomePageContent() {
setDraft([]);
setIsRunning(false);
streamingItemIdRef.current = null;
pinnedToBottomRef.current = true;
setSessionState('created');
workspacePanelRef.current?.collapse();
setCanvasContent('');
Expand Down Expand Up @@ -2064,7 +2106,11 @@ function HomePageContent() {
{/* Chat panel */}
<Panel defaultSize={canvasOpen ? 30 : 100} minSize={20}>
<div className="h-full flex flex-col min-w-0">
<div className="flex-1 overflow-y-auto px-4 py-4">
<div
ref={chatScrollRef}
onScroll={handleChatScroll}
className="flex-1 overflow-y-auto px-4 py-4"
>
<div
className={`mx-auto w-full space-y-4 ${canvasOpen ? 'max-w-3xl' : 'max-w-5xl'}`}
>
Expand Down