Fix frontend stream-state, save/continue, chart redraw, and tooltip bugs - #181
Merged
Conversation
Fixes #155. - Add a stream-generation guard (streamGeneration ref) captured by sendMessage/continueMessage; every state write bails when stale, and startNewChat/loadConversation/deleteConversation(active) abort the in-flight request, so switching or clearing chats mid-stream no longer bleeds streamed text, session ids, or saves into the new conversation. - Guard loadConversation against out-of-order completion when the user switches again while an uncached conversation fetch is in flight. - Proxy: forward X-User-Id to the backend and propagate Retry-After on error responses; handle the floating pipeTo promise so aborted SSE streams no longer emit unhandled rejections. - continueMessage: snapshot and restore isComplete/stop_reason/stopped on 402/429/fetch-failure so a failed continuation no longer leaves the message stuck incomplete without its Continue affordance. - sendMessage: persist cost_gbp/stop_reason in both the done and suggestions saves so truncated turns keep Continue and cost on reload. - Generate the conversation title once per conversation (title ref plus memoized in-flight generation shared by the done/suggestions saves). - Replace the no-op scrollRef auto-scroll with a bottom-sentinel scrollIntoView gated on being near the bottom of the document. - Memoize chart scales/domains/margins/stacks so the D3 draw effects stop tearing down and redrawing on every tooltip mousemove render. - Parse Retry-After defensively (HTTP-date -> fallback 60s). - Remove the empty assistant bubble when a stream is stopped before the first token instead of leaving a dead Continue button. - Deleting the active conversation now clears messages/session state so the next send cannot resurrect it. - Shared page: treat [CHART_LOADING] as a placeholder instead of rendering the literal marker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The data-tip tooltips were hidden with opacity:0, so their boxes were always laid out; the ones hanging below the composer's bottom button row extended the document's scrollable height once the feed grew past 100dvh, leaving dead space below the page content. - Generate no tooltip box until :hover (content:none -> content:attr), with the 60ms fade as a keyframe animation since transitions cannot animate a freshly created box. - Move the attach tooltip to the button's left (new data-tip-left) and the Charts toggle tooltip to the right, so nothing hangs below the bottom row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Beta preview has been cleaned up because this PR was closed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues #164, which was auto-closed by renaming its head branch (GitHub closes a PR when its head branch is renamed; the shorter name keeps the Modal preview hostname under the 63-char DNS label limit — see #180 discussion). Same commits.
Fixes #155
Fixes #180
Single PR fixing the batch of verified frontend bugs in
frontend/. Numbering matches the issue.Changes
High
ChatPage.tsx): added astreamGenerationref.sendMessageandcontinueMessagecapture the generation at start and every state write (updateMessage/flushTarget, the drain interval, 402/429 notices,donehandling incl.sessionId.current,saveConversationcalls, abort/error handling, and thefinallyisStreaming/isWaitingresets) bails when stale.startNewChat,loadConversation, anddeleteConversation(of the active conversation) call a sharedinvalidateStream()that bumps the generation, aborts the in-flight request, and resets streaming UI state.saveConversationalso gen-guards its latesetActiveConversationIdso a save completing after a switch can't steal the active highlight.finallyblocks only clearabortRefwhen it still points at their own controller, so a newer stream's Stop button keeps working.Medium
Proxy header drops (Superseded by Frontend: forward X-User-Id through proxy, pass through backend errors, fix Continue #146, which also forwardsroute.ts): forwardsX-User-Id(withContent-Type) to the backend, and propagatesRetry-Afteron error responses so the frontend's 429 wait message is accurate through the proxy.Authorizationand passes the backend's real error body through; dropped from this PR during rebase.FloatingSuperseded by Frontend: forward X-User-Id through proxy, pass through backend errors, fix Continue #146; dropped from this PR during rebase.pipeTopromise (route.ts):.catch(() => {})on the SSE pipe so client aborts (Stop button) no longer produce unhandled rejections.continueMessagestuck-incomplete on early returns (ChatPage.tsx): snapshotsisComplete/stop_reason/stoppedbefore the optimistic clear and restores them on the 402 and 429 branches and on fetch failure, so Copy/cost/Continue survive a failed continuation attempt.sendMessagesave omitted turn metadata (ChatPage.tsx):cost_gbp/stop_reasonare hoisted out of thedonebranch and persisted in both thedonesave and the latersuggestionssave, matching whatcontinueMessagealready saved.ChatPage.tsx): aconversationTitleRef(known title) plus a memoized in-flight generation promise meanchat/titleis POSTed at most once per conversation; the suggestions-arrival save reuses the same title. Loaded conversations seed the ref from their existing title, so continuing an old conversation never regenerates it.ChatPage.tsx): the oldscrollRefcontainer was not scrollable (the document scrolls), soscrollTodid nothing. Replaced with a bottom-sentinelscrollIntoView, gated on already being within 200px of the document bottom so it never fights a user who scrolled up mid-stream; the on-donesmooth-scroll trigger is preserved with the same gate.LineChart.tsx,BarChart.tsx,ScatterChart.tsx): scales, domains, margins, categories, and stacked data are nowuseMemoized, so thesvg.selectAll("*").remove()draw effects only re-run when the spec/dimensions actually change instead of on every tooltip-mousemove-driven render.Low
Retry-AfterNaN (ChatPage.tsx): sharedparseRetryAfterSecondshelper guards withNumber.isFinite(HTTP-date headers fall back to 60s); used by both send and continue paths.Stop before first token (
ChatPage.tsx): the abort path now removes the empty assistant bubble (no text, no tools) instead of marking it stopped with a dead Continue button.Deleting the active conversation (
ChatPage.tsx): also aborts any stream, clearsmessages,sessionId.current, collapsed-working state, and the title refs, and drops the cache entry, so the next send starts fresh instead of resurrecting the deleted conversation.Shared page
[CHART_LOADING](s/[token]/page.tsx): the segment regex now matches[CHART_LOADING]like ChatPage's, so the literal marker no longer renders. Since a saved share can never finish generating the chart, it renders a static "Chart unavailable" placeholder box rather than ChatPage's live spinner.Tooltip dead space below the composer (
ChatPage.tsx, fixes Invisible tooltip pseudo-elements add dead scroll space below the composer #180): thedata-tiptooltips were hidden withopacity: 0, so their boxes were always laid out and the ones hanging below the composer’s bottom button row stretched the document’s scroll height once the feed exceeded 100dvh. Tooltips now generate no box until:hover(content: none→content: attr(...), fade via keyframe animation), and the attach/Charts tooltips open to the button’s left/right instead of below.Self-review
Reviewed the full
git diff mainonce after implementing; findings:loadConversationhad no staleness check after its ownawait, so two rapid sidebar clicks on uncached conversations could apply the older response after the newer one (and the error path could inject an error bubble into the wrong view). Fixed by capturing the generation afterinvalidateStream()and bailing (success and error paths) if it moved.402 responses routed through the proxy display the wrappedFixed on main by Frontend: forward X-User-Id through proxy, pass through backend errors, fix Continue #146, which passes the backend's error body through unwrapped."Backend error: 402"text rather than the backend's friendly copy (pre-existing: the proxy re-wraps error bodies).continueMessagedoes not re-save after a latesuggestionsevent, unlikesendMessage(pre-existing asymmetry; out of scope per the no-refactor constraint).content, which is invisible when the message hasevents(renderer prefers events; pre-existing). The restored flags make the failure recoverable regardless.scrollIntoViewrather than smooth to avoid smooth-scroll churn at the 20ms drain cadence; the on-donescroll stays smooth.Verification
There is no frontend unit-test suite; verification was
npm run build(passes: compile + lint + type-check) plus end-to-end re-reads of each changed flow (send, continue, stop, switch/new/delete mid-stream, save/title, proxy error paths, chart render/tooltip cycles).🤖 Generated with Claude Code