-
Notifications
You must be signed in to change notification settings - Fork 1
Fix streaming transition and inline composer layout #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ecc7786
4c1a5dd
7cfefe7
ab965f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,11 +25,98 @@ import { | |
| faArrowDown, | ||
| } from "@fortawesome/pro-regular-svg-icons"; | ||
| import { useState, useEffect, useRef, useCallback, type FC } from "react"; | ||
| import Markdown from "react-markdown"; | ||
| import remarkGfm from "remark-gfm"; | ||
| import { cn } from "@/lib/utils"; | ||
|
|
||
| // Animation constants (like lasker-app) | ||
| // Animation constants | ||
| const CHARS_PER_TICK = 2; | ||
| const TICK_MS = 16; // ~60fps | ||
|
|
||
| // Markdown components matching MarkdownText for seamless streaming→complete swap. | ||
| // Keep in sync with defaultComponents in src/components/assistant-ui/markdown-text.tsx | ||
| /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars */ | ||
| const streamingMarkdownComponents = { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Duplicated markdown component definitions will drift out of sync The comment on line 38 says "Keep in sync with defaultComponents in markdown-text.tsx", but this is a manual invariant with no enforcement. The streaming components are already missing the
This partially undermines the stated goal of "seamless renderer swap". Recommendation: Extract the shared component definitions into a single source of truth. For example, export a |
||
| h1: ({ node: _n, className, ...props }: any) => ( | ||
| <h1 | ||
| className={cn( | ||
| "mb-8 scroll-m-20 text-4xl font-extrabold tracking-tight last:mb-0", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ), | ||
| h2: ({ node: _n, className, ...props }: any) => ( | ||
| <h2 | ||
| className={cn( | ||
| "mt-8 mb-4 scroll-m-20 text-3xl font-semibold tracking-tight first:mt-0 last:mb-0", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ), | ||
| h3: ({ node: _n, className, ...props }: any) => ( | ||
| <h3 | ||
| className={cn( | ||
| "mt-6 mb-4 scroll-m-20 text-2xl font-semibold tracking-tight first:mt-0 last:mb-0", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ), | ||
| p: ({ node: _n, className, ...props }: any) => ( | ||
| <p | ||
| className={cn("mt-5 mb-5 leading-7 first:mt-0 last:mb-0", className)} | ||
| {...props} | ||
| /> | ||
| ), | ||
| a: ({ node: _n, className, ...props }: any) => ( | ||
| <a | ||
| className={cn( | ||
| "text-primary font-medium underline underline-offset-4", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ), | ||
| ul: ({ node: _n, className, ...props }: any) => ( | ||
| <ul | ||
| className={cn("my-5 ml-6 list-disc [&>li]:mt-2", className)} | ||
| {...props} | ||
| /> | ||
| ), | ||
| ol: ({ node: _n, className, ...props }: any) => ( | ||
| <ol | ||
| className={cn("my-5 ml-6 list-decimal [&>li]:mt-2", className)} | ||
| {...props} | ||
| /> | ||
| ), | ||
| pre: ({ node: _n, className, ...props }: any) => ( | ||
| <pre | ||
| className={cn( | ||
| "overflow-x-auto rounded-t-none! rounded-b-lg bg-black p-4 text-white", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ), | ||
| code: ({ node: _n, className, ...props }: any) => { | ||
| // Detect code block: react-markdown adds language-* class for fenced blocks | ||
| const isCodeBlock = | ||
| typeof className === "string" && /language-/.test(className); | ||
| return ( | ||
| <code | ||
| className={cn( | ||
| !isCodeBlock && "bg-muted rounded border px-1 font-semibold", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ); | ||
| }, | ||
| }; | ||
| /* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unused-vars */ | ||
|
|
||
| const ScrollToBottomButton: FC<{ visible: boolean; onClick: () => void }> = ({ | ||
| visible, | ||
| onClick, | ||
|
|
@@ -98,6 +185,7 @@ export const Thread: FC = () => { | |
| <ThreadPrimitive.Viewport | ||
| ref={viewportRef} | ||
| onScroll={handleScroll} | ||
| autoScroll | ||
| className="flex grow flex-col overflow-y-auto overscroll-none px-4 pt-8" | ||
| > | ||
| <ThreadPrimitive.Messages components={{ Message: ChatMessage }} /> | ||
|
|
@@ -113,7 +201,7 @@ export const Thread: FC = () => { | |
| /> | ||
| <Composer /> | ||
| <p className="mt-2 text-center font-sans text-xs text-[#9a9893]"> | ||
| BT Servant Web v1.3.0 | ||
| BT Servant Web v1.3.1 | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [LOW] Version string is hardcoded in four locations The version Recommendation: Import the version from // src/lib/constants.ts
import pkg from '../../package.json';
export const APP_VERSION = pkg.version;Then use |
||
| </p> | ||
| </div> | ||
| </AssistantIf> | ||
|
|
@@ -150,7 +238,7 @@ const ThreadWelcome: FC = () => { | |
| <div className="flex w-full max-w-3xl flex-col items-center"> | ||
| {/* Welcome message */} | ||
| <div className="mb-8"> | ||
| <p className="text-center text-lg text-[#1a1a18] sm:text-2xl dark:text-[#eee]"> | ||
| <p className="text-center text-2xl text-[#1a1a18] sm:text-3xl dark:text-[#eee]"> | ||
| Hello, I'm BT Servant. How can I serve you today? | ||
| </p> | ||
| </div> | ||
|
|
@@ -196,7 +284,7 @@ const ThreadWelcome: FC = () => { | |
| {/* Footer */} | ||
| <div className="shrink-0 pb-4"> | ||
| <p className="text-center font-sans text-xs text-[#9a9893]"> | ||
| BT Servant Web v1.3.0 | ||
| BT Servant Web v1.3.1 | ||
| </p> | ||
| </div> | ||
| </div> | ||
|
|
@@ -227,30 +315,28 @@ const Composer: FC = () => { | |
| return ( | ||
| <div className="mx-auto w-full max-w-3xl"> | ||
| <ComposerPrimitive.Root className="flex w-full flex-col rounded-2xl border border-transparent bg-white p-0.5 shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.035),0_0_0_0.5px_rgba(0,0,0,0.08)] transition-shadow duration-200 focus-within:shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.075),0_0_0_0.5px_rgba(0,0,0,0.15)] hover:shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.05),0_0_0_0.5px_rgba(0,0,0,0.12)] dark:bg-[#1f1e1b] dark:shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.4),0_0_0_0.5px_rgba(108,106,96,0.15)] dark:focus-within:shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.5),0_0_0_0.5px_rgba(108,106,96,0.3)] dark:hover:shadow-[0_0.25rem_1.25rem_rgba(0,0,0,0.4),0_0_0_0.5px_rgba(108,106,96,0.3)]"> | ||
| <div className="m-3.5 flex flex-col gap-3.5"> | ||
| <div className="relative"> | ||
| <div className="m-3.5 flex items-center gap-3"> | ||
| <div className="relative min-w-0 flex-1"> | ||
| <div className="max-h-96 w-full overflow-y-auto"> | ||
| <ComposerPrimitive.Input | ||
| placeholder="How can I help you today?" | ||
| className="block min-h-6 w-full resize-none bg-transparent font-sans text-[#1a1a18] outline-none placeholder:text-[#9a9893] dark:text-[#eee] dark:placeholder:text-[#9a9893]" | ||
| className="block min-h-8 w-full resize-none bg-transparent font-sans text-lg text-[#1a1a18] outline-none placeholder:text-lg placeholder:text-[#9a9893] dark:text-[#eee] dark:placeholder:text-[#9a9893]" | ||
| /> | ||
| </div> | ||
| </div> | ||
| <div className="flex w-full items-center gap-2"> | ||
| <div className="relative flex min-w-0 flex-1 shrink items-center gap-2"> | ||
| {/* Voice button - hidden when audio disabled */} | ||
| {AUDIO_ENABLED && voiceRecorder.isSupported && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setShowVoiceRecorder(true)} | ||
| disabled={isLoading} | ||
| className="flex h-8 min-w-8 items-center justify-center overflow-hidden rounded-lg border border-[#00000015] bg-transparent px-1.5 text-[#6b6a68] transition-all hover:bg-[#f5f5f0] hover:text-[#1a1a18] active:scale-[0.98] disabled:opacity-50 dark:border-[#6c6a6040] dark:text-[#9a9893] dark:hover:bg-[#393937] dark:hover:text-[#eee]" | ||
| aria-label="Voice message" | ||
| > | ||
| <MicIcon width={16} height={16} /> | ||
| </button> | ||
| )} | ||
| </div> | ||
| <div className="flex shrink-0 items-center gap-2"> | ||
| {/* Voice button - hidden when audio disabled */} | ||
| {AUDIO_ENABLED && voiceRecorder.isSupported && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setShowVoiceRecorder(true)} | ||
| disabled={isLoading} | ||
| className="flex h-8 min-w-8 items-center justify-center overflow-hidden rounded-lg border border-[#00000015] bg-transparent px-1.5 text-[#6b6a68] transition-all hover:bg-[#f5f5f0] hover:text-[#1a1a18] active:scale-[0.98] disabled:opacity-50 dark:border-[#6c6a6040] dark:text-[#9a9893] dark:hover:bg-[#393937] dark:hover:text-[#eee]" | ||
| aria-label="Voice message" | ||
| > | ||
| <MicIcon width={16} height={16} /> | ||
| </button> | ||
| )} | ||
| <ComposerPrimitive.Send className="flex h-8 w-8 items-center justify-center rounded-lg bg-[#ae5630] transition-colors hover:bg-[#c4633a] active:scale-95 disabled:pointer-events-none disabled:opacity-50 dark:bg-[#ae5630] dark:hover:bg-[#c4633a]"> | ||
| <ArrowUpIcon width={16} height={16} className="text-white" /> | ||
| </ComposerPrimitive.Send> | ||
|
|
@@ -300,34 +386,41 @@ const UserMessage: FC = () => { | |
| }; | ||
|
|
||
| // Animated text hook for streaming - handles character-by-character reveal | ||
| function useAnimatedText(text: string): [string, boolean] { | ||
| function useAnimatedText( | ||
| text: string, | ||
| charsPerTick: number = CHARS_PER_TICK | ||
| ): [string, boolean] { | ||
| const [displayedLength, setDisplayedLength] = useState(text.length); | ||
| // Track previous text to detect resets | ||
| const [prevText, setPrevText] = useState(text); | ||
|
|
||
| // Detect text reset and update state together | ||
| if (text !== prevText) { | ||
| setPrevText(text); | ||
| // If text was cleared, reset displayed length | ||
| if ( | ||
| text.length === 0 || | ||
| (prevText.length > 0 && !text.startsWith(prevText)) | ||
| ) { | ||
| if (text.length === 0) { | ||
| // Text cleared (new message) — reset animation | ||
| setDisplayedLength(0); | ||
| } else if (displayedLength > text.length) { | ||
| // New text is shorter than what we've shown — clamp to end | ||
| setDisplayedLength(text.length); | ||
| } else if (!text.startsWith(prevText.slice(0, displayedLength))) { | ||
| // Content diverged from what was displayed — reset to avoid garbled text | ||
| setDisplayedLength(0); | ||
| } | ||
| // Otherwise text grew or was replaced with compatible prefix — keep position | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| // Animate to catch up to text length | ||
| if (displayedLength < text.length) { | ||
| const interval = setInterval(() => { | ||
| setDisplayedLength((prev) => | ||
| Math.min(prev + CHARS_PER_TICK, text.length) | ||
| Math.min(prev + charsPerTick, text.length) | ||
| ); | ||
| }, TICK_MS); | ||
| return () => clearInterval(interval); | ||
| } | ||
| }, [text.length, displayedLength]); | ||
| }, [text.length, displayedLength, charsPerTick]); | ||
|
|
||
| const isAnimationDone = displayedLength >= text.length; | ||
| return [ | ||
|
|
@@ -342,25 +435,33 @@ const AnimatedText: FC<{ | |
| isCompleting: boolean; | ||
| onAnimationCaughtUp: () => void; | ||
| }> = ({ text, isCompleting, onAnimationCaughtUp }) => { | ||
| const [displayedText, isAnimationDone] = useAnimatedText(text); | ||
| const [displayedText, isAnimationDone] = useAnimatedText( | ||
| text, | ||
| CHARS_PER_TICK | ||
| ); | ||
| const calledRef = useRef(false); | ||
|
|
||
| // Reset the called flag when isCompleting transitions to true | ||
| useEffect(() => { | ||
| if (isCompleting) { | ||
| calledRef.current = false; | ||
| } | ||
| }, [isCompleting]); | ||
|
|
||
| // Fire callback when completing AND animation has caught up | ||
| useEffect(() => { | ||
| if (isCompleting && isAnimationDone && !calledRef.current) { | ||
| calledRef.current = true; | ||
| onAnimationCaughtUp(); | ||
| } | ||
| }, [isCompleting, isAnimationDone, onAnimationCaughtUp]); | ||
|
|
||
| return <span className="whitespace-pre-wrap">{displayedText}</span>; | ||
| return ( | ||
| <Markdown | ||
| remarkPlugins={[remarkGfm]} | ||
| components={streamingMarkdownComponents} | ||
| > | ||
| {displayedText} | ||
| </Markdown> | ||
| ); | ||
| }; | ||
|
|
||
| const AssistantMessage: FC = () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -140,39 +140,41 @@ export function useChatRuntime() { | |
| if (!pending) return; | ||
|
|
||
| pendingCompleteRef.current = null; | ||
| // React 18+ auto-batches these into a single render | ||
| setIsCompleting(false); | ||
| setMessages((prev) => [...prev, pending.message]); | ||
| setIsLoading(false); | ||
| setStatusMessage(null); | ||
| setMessages((prev) => [...prev, pending.message]); | ||
| setStreamingText(""); | ||
| }, []); | ||
|
|
||
| // Define handlers before sendMessage so they can be in the dependency array | ||
| const handleComplete = useCallback((data: ChatResponse) => { | ||
| const joinedResponse = data.responses.join("\n\n"); | ||
| const currentStreaming = streamingTextRef.current; | ||
|
|
||
| const assistantMessage = createMessage( | ||
| `assistant-${Date.now()}`, | ||
| "assistant", | ||
| joinedResponse, | ||
| data.voice_audio_base64 || undefined | ||
| ); | ||
|
|
||
| const currentStreaming = streamingTextRef.current; | ||
|
|
||
| // If no streaming text was accumulated, or complete text diverges from | ||
| // what was streamed, swap immediately (no animation to wait for) | ||
| if (!currentStreaming || !joinedResponse.startsWith(currentStreaming)) { | ||
| // If no streaming text was accumulated, swap immediately | ||
| if (!currentStreaming) { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MEDIUM] Removing the prefix-divergence guard may cause garbled animation on corrected completions Previously, the code checked Now with that guard removed, if the backend's final Is the backend guaranteed to always produce a Recommendation: Either (a) keep a guard that falls back to immediate swap when the complete text does not start with the currently displayed text, or (b) document explicitly that the backend contract guarantees prefix consistency. A safe middle ground: if (!currentStreaming || !joinedResponse.startsWith(currentStreaming)) {
// Immediate swap -- no animation needed or safe
setMessages((prev) => [...prev, assistantMessage]);
setIsLoading(false);
setStatusMessage(null);
setStreamingText("");
return;
} |
||
| setMessages((prev) => [...prev, assistantMessage]); | ||
| setIsLoading(false); | ||
| setStatusMessage(null); | ||
| setStreamingText(""); | ||
| return; | ||
| } | ||
|
|
||
| // Defer swap: store pending data and set full text so animation finishes | ||
| // Defer swap: update streaming text to the full response so AnimatedText | ||
| // can animate the remaining characters, then finalizeComplete swaps in | ||
| // the permanent message once the animation catches up. | ||
| pendingCompleteRef.current = { message: assistantMessage }; | ||
| setIsCompleting(true); | ||
| setStreamingText(joinedResponse); | ||
| setIsCompleting(true); | ||
| setStatusMessage(null); | ||
| }, []); | ||
|
|
||
|
|
@@ -313,7 +315,6 @@ export function useChatRuntime() { | |
| setStatusMessage(null); | ||
| return; | ||
| } | ||
| console.error("Chat error:", error); | ||
| handleError("Sorry, I encountered an error. Please try again."); | ||
| } finally { | ||
| abortControllerRef.current = null; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test comment