From dcdcf6e4f376d0cc7d506fa67a60503118d6e586 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 10:03:16 +0200 Subject: [PATCH 01/10] perf(buddy): stop a keystroke from re-rendering the whole conversation (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing in the buddy composer grew sluggish as a conversation grew: in a 50-message thread the box lagged for tens to hundreds of milliseconds per character. The composer was not the cost — everything the words were attached to was. Both the draft and the transcript were owned by `useBuddyConversation`, whose entire return value is published through one context. `setDraft` on every keystroke therefore produced a new session value, and React re-rendered every reader of it: on `/buddy` the page itself, and with it the whole thread — each reply re-run through `ReactMarkdown` with `remarkGfm` (a full AST parse per reply per character), each bubble re-running its animation hooks, plus the dock and the widget for the same session. This commit takes the draft out of that path: * `BuddyDraftProvider` (new) owns the words, and sits *below* the session provider. The page, the dock and the widget are its `children`, whose elements never change identity, so a keystroke now re-renders the box and nothing else. * Two contexts, not one, because the halves age differently. The value (`draft`/`setDraft`/`handleSubmit`) changes per keystroke and only the composer reads it; the write-only `BuddyDraftActions` never changes identity, so the surfaces that merely *fill* the box — the suggestion chips, the dock's hand-off to `/buddy` — stay off the per-keystroke path entirely. * `handleSubmit` keeps its exact contract — an egg phrase plays its effect, is swallowed, and returns `false` so the composer keeps the caret — only its home moved. The caret tests pin that. * `useBuddyConversation` no longer owns `draft`, `setDraft` or `handleSubmit`. What it still owes the composer is the news that the thread was replaced: a fresh visit and a project switch bump `draftResetToken`, and the provider clears the box in the same pass that notices it (the documented "adjust state when a prop changes" pattern rather than an effect, which would paint one frame of a draft belonging to a conversation that is gone, and pay a second render to correct it). * `useBuddy` merges the session with the draft again, so the widget and the tests that type-then-submit keep one API. * `dismissAction` reads the transcript through a ref instead of through `messages` state — same behaviour, but the callback stays the same function for the session's lifetime, which is what the memoisation in the next commit compares. The props the transcript will be compared on are also held in one identity across `BuddyPage`, `BuddyDock` and `BuddyWidget` (`renderReplyAction`, `renderQuestionAction`, `lastMessageFooter`, `onRetryOpen`, `onStartFreshVisit`, `onHideSuggestions`): a memo can only skip a re-render it is not asked for. Nothing else changes: the same words, the same submit contract, the same clearing on a fresh visit, the same one composer shared by the dock and the page. --- src/features/buddy/BuddyDraftProvider.tsx | 93 +++++++++++++++++++ src/features/buddy/BuddyProvider.tsx | 10 +- src/features/buddy/buddyDraftContext.ts | 71 ++++++++++++++ .../buddy/components/BuddyComposer.tsx | 18 ++-- .../buddy/components/BuddyConversation.tsx | 31 +++---- src/features/buddy/components/BuddyDock.tsx | 54 ++++++----- src/features/buddy/components/BuddyWidget.tsx | 55 ++++++----- src/features/buddy/hooks/useBuddy.ts | 10 +- .../buddy/hooks/useBuddyConversation.ts | 74 +++++++-------- src/pages/BuddyPage.tsx | 76 +++++++++------ .../buddy/BuddyComposerCaret.test.tsx | 53 ++++++----- .../features/buddy/BuddyConversation.test.tsx | 26 +++--- tests/unit/features/buddy/BuddyDock.test.tsx | 42 +++++---- .../features/buddy/useBuddyTeamMode.test.tsx | 14 +-- .../features/buddy/useHandedOffDraft.test.tsx | 90 +++++++++++------- 15 files changed, 476 insertions(+), 241 deletions(-) create mode 100644 src/features/buddy/BuddyDraftProvider.tsx create mode 100644 src/features/buddy/buddyDraftContext.ts diff --git a/src/features/buddy/BuddyDraftProvider.tsx b/src/features/buddy/BuddyDraftProvider.tsx new file mode 100644 index 00000000..9b638753 --- /dev/null +++ b/src/features/buddy/BuddyDraftProvider.tsx @@ -0,0 +1,93 @@ +import { useCallback, useMemo, useState } from "react"; +import type { FormEvent, ReactNode } from "react"; +import { matchEggPhrase } from "../easter-eggs/lib/eggPhrases"; +import { playEggEffect } from "../easter-eggs/eggEffectBus"; +import { BuddyDraftActionsContext, BuddyDraftContext } from "./buddyDraftContext"; +import type { BuddyDraft, BuddyDraftActions } from "./buddyDraftContext"; +import { useBuddySession } from "./buddySessionContext"; + +/** + * Owns the composer's words — one box for both buddy surfaces. + * + * **Why it is a provider of its own, *under* the session's.** The draft used to be a piece of + * `useBuddyConversation`, so typing in it re-rendered every reader of the session: on `/buddy` + * that is the whole page, and with it every reply in the thread, each one re-parsed by + * `ReactMarkdown` (issue #236 — the composer grew sluggish as a conversation grew). Down here + * the state is a step further from everything else: a keystroke re-renders this provider and the + * composer that reads it, and nothing else. The page, the dock and the widget are all `children`, + * whose elements never change identity, so React leaves them where they are. + * + * The words still outlive the dock — close it mid-sentence and they are there next time — because + * this provider lives as long as the app's one session, not as long as any one surface. + * + * Two contexts, not one: the value changes per keystroke by design, and only the composer should + * follow it. A surface that merely *fills* the box reads `BuddyDraftActions` and never re-renders + * for a keystroke — see that type for why the distinction is load-bearing. + */ +export function BuddyDraftProvider({ children }: { children: ReactNode }) { + const { sendMessage, draftResetToken } = useBuddySession(); + const [draft, setDraft] = useState(""); + + /** + * A fresh visit or a project switch replaced the conversation on screen, so the words in the + * box went with it — they were a question about the thread that is gone. + * + * Told, not pulled: the session sits *above* this provider and cannot reach into its state, so + * it bumps a token and the clearing happens on the way through. React's documented "adjust + * state when a prop changes" pattern — the same shape `BuddyPage` uses for its rail — rather + * than an effect, because an effect here would paint one frame of a draft belonging to a + * conversation that no longer exists, and cost a second render to fix it. + */ + const [seenResetToken, setSeenResetToken] = useState(draftResetToken); + if (seenResetToken !== draftResetToken) { + setSeenResetToken(draftResetToken); + setDraft(""); + } + + /** + * The one way a message leaves the box. The contract is unchanged from when this lived in the + * session: an egg phrase plays its effect and is swallowed, anything else is cleared and sent, + * and the return value says whether a turn actually started (the composer keeps the caret when + * one did not). + */ + const handleSubmit = useCallback( + (event: FormEvent) => { + event.preventDefault(); + + // Easter-egg phrases are intercepted before anything is sent: the effect plays app-wide + // (EggEffectsLayer) and the message is swallowed silently — no reply, no request. Same + // contract as the AI chat. + const eggEffect = matchEggPhrase(draft); + if (eggEffect) { + setDraft(""); + playEggEffect(eggEffect); + return false; + } + + const text = draft; + if (!text.trim()) return false; + + setDraft(""); + void sendMessage(text); + return true; + }, + [draft, sendMessage], + ); + + const value = useMemo( + // `setDraft` is a state setter — stable for this provider's lifetime — so it is deliberately + // not a dependency. The value is new exactly when the words, or the submit behaviour, are. + () => ({ draft, setDraft, handleSubmit }), + [draft, handleSubmit], + ); + + // Built once and never again: the setter it carries never changes, which is the whole point of + // handing surfaces this half instead of the value. + const actions = useMemo(() => ({ setDraft }), []); + + return ( + + {children} + + ); +} diff --git a/src/features/buddy/BuddyProvider.tsx b/src/features/buddy/BuddyProvider.tsx index f5edb9d7..4f1a4bdf 100644 --- a/src/features/buddy/BuddyProvider.tsx +++ b/src/features/buddy/BuddyProvider.tsx @@ -1,5 +1,6 @@ import type { ReactNode } from "react"; import { useToast } from "../../context/useToast"; +import { BuddyDraftProvider } from "./BuddyDraftProvider"; import { BuddySessionContext } from "./buddySessionContext"; import { useBuddyConversation } from "./hooks/useBuddyConversation"; import { useProjectContext } from "../projects/useProjectContext"; @@ -52,5 +53,12 @@ export function BuddyProvider({ children }: { children: ReactNode }) { ), ); - return {children}; + // The composer's words get a provider of their own, under the session's — one keystroke then + // costs one render of one box instead of a render of every reader of the conversation. See + // `BuddyDraftProvider` for what the split is worth and why it sits below rather than inside. + return ( + + {children} + + ); } diff --git a/src/features/buddy/buddyDraftContext.ts b/src/features/buddy/buddyDraftContext.ts new file mode 100644 index 00000000..f519e14a --- /dev/null +++ b/src/features/buddy/buddyDraftContext.ts @@ -0,0 +1,71 @@ +import { createContext, useContext } from "react"; +import type { Dispatch, FormEvent, SetStateAction } from "react"; + +/** + * Everything the composer is: the words in it, the way they are filled in, and the one way they + * leave it. + * + * Split out of the buddy session because the two age at completely different rates. The session + * changes when the conversation does — a message, a turn, a switch — while this changes on every + * keystroke. While they shared one context value, every keypress handed every consumer of the + * conversation a new value, so the whole thread re-parsed itself per character typed. See + * [BuddyDraftProvider](./BuddyDraftProvider.tsx) for the shape that fixes it. + */ +export type BuddyDraft = { + /** The words currently in the box — one composer, shared by the dock and the page. */ + draft: string; + setDraft: Dispatch>; + /** + * Submits the composer. Returns whether a turn was actually started — `false` when the + * submission was swallowed (an easter-egg phrase, an empty draft), which is what keeps the + * caret in the box instead of handing it off on a send that never happened. + */ + handleSubmit: (event: FormEvent) => boolean; +}; + +/** + * The write-only half of the composer: what a surface needs in order to *fill* the box, without + * the box's value. + * + * Its value never changes, so a reader can hold the setter for the session's lifetime and never + * re-render for a keystroke — which is exactly what the surfaces that seed a draft (the + * suggestion chips, the dock's hand-off to `/buddy`) have to do. Reading [BuddyDraft] for the + * setter instead would put them back on the per-keystroke path this split exists to leave. + */ +export type BuddyDraftActions = { setDraft: Dispatch> }; + +export const BuddyDraftContext = createContext(null); + +export const BuddyDraftActionsContext = createContext(null); + +/** + * The composer this session has — the value *and* the way to fill it. + * + * Deliberately no fallback, like `useBuddySession`: a hook that quietly made its own draft would + * put the dock and the page back on two composers that merely share a name. + */ +export function useBuddyDraft(): BuddyDraft { + const draft = useContext(BuddyDraftContext); + + if (!draft) { + throw new Error("useBuddyDraft must be used inside a BuddyDraftProvider"); + } + + return draft; +} + +/** + * The setter without the value — see [BuddyDraftActions] for why the two are apart. + * + * This is the one to reach for whenever a surface only *writes* to the composer: nothing it + * reads here ever changes, so the surface keeps its sub over a keystroke-free value. + */ +export function useBuddyDraftActions(): BuddyDraftActions { + const actions = useContext(BuddyDraftActionsContext); + + if (!actions) { + throw new Error("useBuddyDraftActions must be used inside a BuddyDraftProvider"); + } + + return actions; +} diff --git a/src/features/buddy/components/BuddyComposer.tsx b/src/features/buddy/components/BuddyComposer.tsx index 8568d68d..69e386b6 100644 --- a/src/features/buddy/components/BuddyComposer.tsx +++ b/src/features/buddy/components/BuddyComposer.tsx @@ -3,16 +3,9 @@ import type { KeyboardEvent } from "react"; import { Send } from "lucide-react"; import { Button } from "../../../components/ui/Button"; import { useAutoResize } from "../../../components/ui/useAutoResize"; +import { useBuddyDraft } from "../buddyDraftContext"; type BuddyComposerProps = { - draft: string; - setDraft: (value: string) => void; - /** - * Submits the box. Returns whether a turn was actually started: `false` when the caller - * swallowed the submission (an easter-egg phrase, an empty draft), which is what keeps the - * caret here instead of handing it off on a send that never happened — see `submit`. - */ - handleSubmit: (event: React.FormEvent) => boolean; /** Composer placeholder — "Type your answer…" while the buddy is intaking. */ placeholder?: string; /** Drops the keyboard hint under the box, for the dock where the room is better spent. */ @@ -56,17 +49,20 @@ type BuddyComposerProps = { * It draws no band of its own — no border, no background, no page padding. Each surface frames * it: the page's card gives it a bottom band, the dock hands it to `SidePanel`'s footer. Owning * the frame here is what previously put two `border-t`s across the dock. + * + * It reads the words from the shared composer (`useBuddyDraft`) rather than taking them as + * props — the box is the one component a keystroke is *allowed* to re-render, and reading the + * draft directly is what keeps that true: a surface passing `draft` down would be re-rendering + * for every character it forwarded. See `BuddyDraftProvider`. */ export function BuddyComposer({ - draft, - setDraft, - handleSubmit, placeholder = "Ask your buddy anything...", compact = false, focusOnMount = false, busy = false, gameActive = false, }: BuddyComposerProps) { + const { draft, setDraft, handleSubmit } = useBuddyDraft(); const fieldRef = useRef(null); // Set when *this* composer gave up the caret on a send, so the refocus diff --git a/src/features/buddy/components/BuddyConversation.tsx b/src/features/buddy/components/BuddyConversation.tsx index 5c7f1a1c..03276137 100644 --- a/src/features/buddy/components/BuddyConversation.tsx +++ b/src/features/buddy/components/BuddyConversation.tsx @@ -1,3 +1,4 @@ +import { useCallback } from "react"; import type { ReactNode } from "react"; import type { BuddyMessageView, ProposedAction } from "../types"; import { BuddyComposer } from "./BuddyComposer"; @@ -14,13 +15,6 @@ type BuddyConversationProps = { isThinking: boolean; /** The tool the buddy is running right now, if any — becomes "Checking your progress…". */ activeTool: string | null; - draft: string; - setDraft: (value: string) => void; - /** - * Submits the composer. Returns whether a turn started — an egg phrase comes back `false`, - * because nothing was sent for it. - */ - handleSubmit: (event: React.FormEvent) => boolean; /** Confirms a buddy-proposed action (the only path that mutates). */ confirmAction: (messageId: string, action: ProposedAction) => void; /** Declines a proposed action; nothing changes. */ @@ -95,9 +89,6 @@ export function BuddyConversation({ messages, isThinking, activeTool, - draft, - setDraft, - handleSubmit, confirmAction, dismissAction, placeholder, @@ -117,6 +108,19 @@ export function BuddyConversation({ }: BuddyConversationProps) { const { containerRef, onScroll } = useStickToBottom(messages); + /** + * The row under every reply, held in one identity for the life of this component. + * + * `BuddyThread` is memoised — that is what keeps a keystroke out of the thread — and a + * callback created inline would hand it a new prop on every render, defeating exactly that. + */ + const renderReplyAction = useCallback( + (reply: string, message: BuddyMessageView) => ( + + ), + [], + ); + return ( <>
( - - )} + renderReplyAction={renderReplyAction} messages={messages} isThinking={isThinking} isStreaming={isStreaming} @@ -191,9 +193,6 @@ export function BuddyConversation({ {aboveComposer &&
{aboveComposer}
} (null); const { containerRef, onScroll } = useStickToBottom(messages); + // The chips fill the composer through the write-only half, so this window does not follow + // every character typed into the box — see `useBuddyDraftActions`. + const { setDraft } = useBuddyDraftActions(); + + /** + * The callbacks `BuddyThread` is handed, each held in one identity. + * + * The thread is memoised — that is what keeps a keystroke (or a token) from re-rendering the + * whole conversation — and a callback built inline here would hand it a new prop on every + * render of this window, which is exactly the dance the memo exists to avoid. + */ + const renderReplyAction = useCallback( + (reply: string, message: BuddyMessageView) => ( + + ), + [], + ); + const renderQuestionAction = useCallback( + (question: string) => + teamProjectId === null ? : undefined, + [teamProjectId], + ); + const startFresh = useCallback(() => void startFreshVisit(), [startFreshVisit]); + // Escape closes it, the way every other dismissible surface in the app behaves. Bound to the // document rather than the panel so it works while the hire is reading the page behind it. useEffect(() => { @@ -327,9 +347,7 @@ export function BuddyDock({ className="min-h-0 flex-1 overflow-x-hidden overflow-y-auto px-4 py-4" > ( - - )} + renderReplyAction={renderReplyAction} compact messages={messages} isThinking={isThinking} @@ -340,14 +358,12 @@ export function BuddyDock({ dismissAction={dismissAction} // Hire-flow only: "Send this to your PM" escalates the hire's own question, and a // team-mode conversation is not one — the offer must not even render there. - renderQuestionAction={(question) => - teamProjectId === null ? : undefined - } + renderQuestionAction={renderQuestionAction} openError={openError} onRetryOpen={onRetryOpen} dinoGameActive={dinoGameActive} onDinoGameExit={onDinoGameExit} - onStartFreshVisit={() => void startFreshVisit()} + onStartFreshVisit={startFresh} />
@@ -389,15 +405,7 @@ export function BuddyDock({ `focusOnMount` rather than a bare `focus()`. A focused textarea with a value in it starts the caret at position 0, so "Ask your buddy about this" used to hand over a question the hire then typed in front of. */} - + diff --git a/src/features/buddy/components/BuddyWidget.tsx b/src/features/buddy/components/BuddyWidget.tsx index bd794db8..0b6f2eb4 100644 --- a/src/features/buddy/components/BuddyWidget.tsx +++ b/src/features/buddy/components/BuddyWidget.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { AnimatePresence, animate, motion, useMotionValue, useReducedMotion } from "framer-motion"; import { useLocation, useNavigate } from "react-router-dom"; import { Sparkles } from "lucide-react"; @@ -67,8 +67,6 @@ export function BuddyWidget() { isOpen, toggleOpen, draft, - setDraft, - handleSubmit, confirmAction, dismissAction, suggestions, @@ -193,6 +191,33 @@ export function BuddyWidget() { void navigate(BUDDY_PAGE, { state: { draft } }); }, [draft, navigate]); + /** + * The props the dock's memoised thread compares, each held in one identity. + * + * The thread — and every row in it — is memoised, which is what keeps a keystroke and every + * streamed token out of the conversation's re-render path (issue #236). Any of these built + * inline here would hand it a new prop on every render of the widget and put them all back. + */ + const hasUserMessage = messages.some((message) => message.role === "USER"); + // The greeting's one suggested next step, which only `/buddy` used to offer. + const lastMessageFooter = useMemo( + () => + openerAction && !greeting.isRevealing && !hasUserMessage ? ( + + ) : undefined, + [openerAction, greeting.isRevealing, hasUserMessage, sendMessage], + ); + const retryOpenAction = useCallback(() => void retryOpen(), [retryOpen]); + const hideSuggestions = useCallback(() => setSuggestionsHidden(true), []); + /** * Grows the open dock into the page — one gesture instead of a cut. * @@ -294,25 +319,11 @@ export function BuddyWidget() { // `isOpening` too, the way `/buddy` passes it: a dock opened while the greeting is // still being written showed an empty window instead of the buddy typing. isThinking={isThinking || isOpening || greeting.isThinking} - // The greeting's one suggested next step, which only `/buddy` used to offer. - lastMessageFooter={ - openerAction && !greeting.isRevealing && !messages.some((m) => m.role === "USER") ? ( - - ) : undefined - } + // Held in one identity above, with the reason written there — the thread's memo + // compares it. + lastMessageFooter={lastMessageFooter} isStreaming={isStreaming} activeTool={activeTool} - draft={draft} - setDraft={setDraft} - handleSubmit={handleSubmit} confirmAction={confirmAction} dismissAction={dismissAction} suggestions={suggestions} @@ -323,11 +334,11 @@ export function BuddyWidget() { isDeciding={isDeciding} teamProjectId={teamProjectId} openError={openError} - onRetryOpen={() => void retryOpen()} + onRetryOpen={retryOpenAction} onClose={toggleOpen} onOpenFull={openFull} suggestionsHidden={suggestionsHidden} - onHideSuggestions={() => setSuggestionsHidden(true)} + onHideSuggestions={hideSuggestions} // Hire conversation ↔ team conversations, in the header beside the title. The // switcher only *offers* the switch; the restore audit lives in the session // (`useBuddyConversation` / `BuddyProvider`). diff --git a/src/features/buddy/hooks/useBuddy.ts b/src/features/buddy/hooks/useBuddy.ts index 76d45005..9cf0e8eb 100644 --- a/src/features/buddy/hooks/useBuddy.ts +++ b/src/features/buddy/hooks/useBuddy.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useState } from "react"; import { onOpenAiBuddy } from "../aiBuddyBus"; +import { useBuddyDraft } from "../buddyDraftContext"; import { useBuddySession } from "../buddySessionContext"; import { useBuddySuggestions } from "./useBuddySuggestions"; @@ -18,7 +19,11 @@ import { useBuddySuggestions } from "./useBuddySuggestions"; */ export function useBuddy() { const conversation = useBuddySession(); - const { ensureOpened, setDraft, teamProjectId } = conversation; + // The composer's half lives in its own provider now — see `BuddyDraftProvider`. It is merged + // back in here because every surface that drives the dock wants "the buddy" as one thing, and + // the hand-off to `/buddy` has to read the words the hire was mid-way through typing. + const { draft, setDraft, handleSubmit } = useBuddyDraft(); + const { ensureOpened, teamProjectId } = conversation; const [isOpen, setIsOpen] = useState(false); @@ -68,6 +73,9 @@ export function useBuddy() { return { ...conversation, + draft, + setDraft, + handleSubmit, isOpen, toggleOpen, closeDock, diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 8be71944..a9edaec2 100644 --- a/src/features/buddy/hooks/useBuddyConversation.ts +++ b/src/features/buddy/hooks/useBuddyConversation.ts @@ -1,7 +1,5 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useDinoUnlocked, useSpaceOpensDino } from "../../easter-eggs/hooks/useDinoWaitingGame"; -import { matchEggPhrase } from "../../easter-eggs/lib/eggPhrases"; -import { playEggEffect } from "../../easter-eggs/eggEffectBus"; import { getMessages, streamOpenBuddy, @@ -143,7 +141,15 @@ export function useBuddyConversation( // Set when the conversation could not be brought on screen at all -- distinct from a turn that // failed, which carries its own reason. Nothing is on screen to hang that on, so it is state. const [openError, setOpenError] = useState(null); - const [draft, setDraft] = useState(""); + /** + * Bumped whenever the conversation on screen is replaced — a fresh visit, a project switch. + * + * The composer's words are not this hook's any more (see `BuddyDraftProvider`), so the one + * thing this session still owes them is the news that they belonged to a thread that is gone. + * Told as a token rather than a call: the composer's state lives *below* this hook's provider, + * and a parent cannot reach into a child's setter. + */ + const [draftResetToken, setDraftResetToken] = useState(0); /** * The last greeting a surface has actually put in front of the hire — either watched while it * streamed, or revealed by `useGreetingReveal`. Held here, not per surface, so a greeting the @@ -489,7 +495,9 @@ export function useBuddyConversation( setMessages([]); setOpenerAction(null); setOpenError(null); - setDraft(""); + // The box is emptied through the token: a question typed about the conversation being + // cleared is about a thread that no longer exists. See `draftResetToken`. + setDraftResetToken((token) => token + 1); setIsOpening(true); try { await greet(); @@ -778,6 +786,20 @@ export function useBuddyConversation( [beginDecision, endDecision, patchAction], ); + /** + * The transcript as of the last render, for `dismissAction` to read. + * + * A dismissal has to find the *action object* (a stored proposal is declined by id at the + * backend, and only the object says which kind it is), but reading it from `messages` state + * would rebuild this callback every time a token lands — and every memoised row would then + * re-render for a token that belongs to one of them. A click always happens after the render + * that recorded it, so the ref is never behind what is on screen. + */ + const messagesRef = useRef(messages); + useEffect(() => { + messagesRef.current = messages; + }, [messages]); + /** * Declines a proposed action — nothing changes; the conversation simply continues. * @@ -788,7 +810,7 @@ export function useBuddyConversation( */ const dismissAction = useCallback( (messageId: string, actionId: string) => { - const action = messages + const action = messagesRef.current .find((m) => m.id === messageId) ?.actions?.find((a) => a.id === actionId); @@ -840,7 +862,7 @@ export function useBuddyConversation( } })(); }, - [beginDecision, endDecision, messages, patchAction], + [beginDecision, endDecision, patchAction], ); /** @@ -873,7 +895,8 @@ export function useBuddyConversation( setMessages([]); setOpenerAction(null); setOpenError(null); - setDraft(""); + // Same rule as a fresh visit: the words belonged to the conversation that just went away. + setDraftResetToken((token) => token + 1); setActiveTool(null); setIsThinking(false); setIsStreaming(false); @@ -934,35 +957,6 @@ export function useBuddyConversation( [isThinking, isStreaming, isOpening, isGreeting, isDeciding, selection, setTeamMode], ); - /** - * Handles a composer submission: an egg phrase plays its effect and is swallowed, anything - * else is sent. Returns whether a turn was started — `false` means the submission went - * nowhere, which the composer uses to decide whether the caret should be handed off. - */ - const handleSubmit = useCallback( - (event: React.FormEvent) => { - event.preventDefault(); - - // Easter-egg phrases are intercepted before anything is sent: the - // effect plays app-wide (EggEffectsLayer) and the message is swallowed - // silently — no reply, no request. Same contract as the AI chat. - const eggEffect = matchEggPhrase(draft); - if (eggEffect) { - setDraft(""); - playEggEffect(eggEffect); - return false; - } - - const text = draft; - if (!text.trim()) return false; - - setDraft(""); - void sendMessage(text); - return true; - }, - [draft, sendMessage, setDraft], - ); - return { messages, isThinking, @@ -983,10 +977,12 @@ export function useBuddyConversation( isDeciding, switchTeamProject, - draft, - setDraft, + // The composer's words live in `BuddyDraftProvider`, below this provider; this is the only + // thing about them the session still owns — the news that the thread they belonged to is + // gone. See `draftResetToken`. + draftResetToken, + sendMessage, - handleSubmit, confirmAction, dismissAction, diff --git a/src/pages/BuddyPage.tsx b/src/pages/BuddyPage.tsx index 62ffb11f..31a2a2c8 100644 --- a/src/pages/BuddyPage.tsx +++ b/src/pages/BuddyPage.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from "react"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { useLocation } from "react-router-dom"; import { Inbox, Sparkles, Users } from "lucide-react"; import { Button } from "../components/ui/Button"; @@ -12,6 +12,7 @@ import { import { useMediaQuery } from "../hooks/useMediaQuery"; import { useRailOverlayGuard } from "../hooks/useRailOverlayGuard"; import { useBuddySession } from "../features/buddy/buddySessionContext"; +import { useBuddyDraftActions } from "../features/buddy/buddyDraftContext"; import { useProjectContext } from "../features/projects/useProjectContext"; import { useBuddySuggestions } from "../features/buddy/hooks/useBuddySuggestions"; import { BuddyModeSwitcher } from "../features/buddy/components/BuddyModeSwitcher"; @@ -168,10 +169,7 @@ function BuddyMentorHome() { isOpening, activeTool, openerAction, - draft, - setDraft, sendMessage, - handleSubmit, confirmAction, dismissAction, openError, @@ -189,6 +187,12 @@ function BuddyMentorHome() { isDeciding, } = useBuddySession(); + // The page fills the composer (the chips, the hand-off from the dock) but never reads it — so + // it takes the write-only half, which never changes, rather than the per-keystroke value. That + // is what keeps a character typed into the box from re-rendering this page at all. See + // `BuddyDraftProvider`. + const { setDraft } = useBuddyDraftActions(); + // The page shows the thread, so Space may open the dino game here; leaving the page releases // it (and closes a game still running) — see `registerDinoSurface`. useEffect(() => registerDinoSurface(), [registerDinoSurface]); @@ -302,6 +306,40 @@ function BuddyMentorHome() { // that arrives while the buddy is answering. const startFresh = useCallback(() => void startFreshVisit(), [startFreshVisit]); + /** + * The props the transcript's memo compares, each held in one identity. + * + * `BuddyThread` and its rows are memoised — that is what keeps a keystroke, and each streamed + * token, from re-rendering every reply in the conversation — and any of these built inline + * would hand the thread a new prop on every render of this page, which is exactly the dance + * the memo exists to avoid. Each one's dependencies are what it is actually made of. + */ + const renderQuestionAction = useCallback( + (question: string) => (isHireMode ? : undefined), + [isHireMode], + ); + const retryOpenAction = useCallback(() => void retryOpen(), [retryOpen]); + // Escalating hangs off the hire's own question now, not off the buddy's answer — see + // `BuddyQuestionActions`. What is left here is the greeting's own next step, offered where a + // messenger offers a quick reply: right under the message that suggested it. It sends on one + // click, unlike the chips, because accepting something the mentor just offered is not composing + // a question of your own. + const lastMessageFooter = useMemo( + () => + !hasUserMessage && openerAction && !greeting.isRevealing ? ( + + ) : undefined, + [hasUserMessage, openerAction, greeting.isRevealing, sendMessage], + ); + // The keyboard half of the control in the visit divider. Gated the same way that control is: // a visit nobody has spoken in is already the fresh one, and re-opening it would only replay // the greeting — and, like the chat's, only while this is the half on screen, since the shell @@ -374,38 +412,18 @@ function BuddyMentorHome() { isThinking={isThinking || isOpening || greeting.isThinking} isStreaming={isStreaming} activeTool={activeTool} - draft={draft} - setDraft={setDraft} - handleSubmit={handleSubmit} confirmAction={confirmAction} dismissAction={dismissAction} dinoGameActive={dinoGameActive} onDinoGameExit={closeDinoGame} - // Escalating hangs off the hire's own question now, not off the buddy's answer — see - // `BuddyQuestionActions`. What is left here is the greeting's own next step, offered - // where a messenger offers a quick reply: right under the message that suggested it. It - // sends on one click, unlike the chips, because accepting something the mentor just - // offered is not composing a question of your own. - lastMessageFooter={ - !hasUserMessage && openerAction && !greeting.isRevealing ? ( - - ) : undefined - } + // Both held in one identity above, with the reasons written there — the thread's memo + // compares them. + lastMessageFooter={lastMessageFooter} // Hire-flow only: "Send this to your PM" escalates the hire's own question, and a // team-mode conversation is not one — the offer must not even render there. - renderQuestionAction={(question) => - isHireMode ? : undefined - } + renderQuestionAction={renderQuestionAction} openError={openError} - onRetryOpen={() => void retryOpen()} + onRetryOpen={retryOpenAction} onStartFreshVisit={canStartFresh ? startFresh : undefined} // This page is the one that binds it — see `useNewConversationShortcut` above. freshVisitShortcut={NEW_CONVERSATION_CHORD} diff --git a/tests/unit/features/buddy/BuddyComposerCaret.test.tsx b/tests/unit/features/buddy/BuddyComposerCaret.test.tsx index d475846e..a0ea7695 100644 --- a/tests/unit/features/buddy/BuddyComposerCaret.test.tsx +++ b/tests/unit/features/buddy/BuddyComposerCaret.test.tsx @@ -1,13 +1,36 @@ import { describe, expect, it, vi } from "vitest"; import { fireEvent, render, screen } from "@testing-library/react"; import { BuddyComposer } from "../../../../src/features/buddy/components/BuddyComposer"; +import { BuddyDraftContext } from "../../../../src/features/buddy/buddyDraftContext"; // The caret contract of the composer: a send hands the caret to the page (so // Space can open the dino game), a swallowed submission keeps it where the // hire is typing. See `submit` in the component. +// +// The words come from the shared composer context now (`BuddyDraftProvider`) rather than from +// props, so every render goes through `Harness` — which is also the only way to re-render the +// box with a different draft, since the component no longer takes one. describe("BuddyComposer caret handoff", () => { + function Harness({ + draft = "", + handleSubmit = () => true, + busy = false, + gameActive = false, + }: { + draft?: string; + handleSubmit?: (event: React.FormEvent) => boolean; + busy?: boolean; + gameActive?: boolean; + }) { + return ( + + + + ); + } + const submitWith = (handleSubmit: (event: React.FormEvent) => boolean) => { - render(); + render(); const field = screen.getByLabelText("Message"); field.focus(); fireEvent.submit(field.closest("form")!); @@ -26,40 +49,20 @@ describe("BuddyComposer caret handoff", () => { it("does not steal focus back while the dino game is open, and returns it when the game closes", () => { const handleSubmit = () => true; - const { rerender } = render( - , - ); + const { rerender } = render(); const field = screen.getByLabelText("Message"); field.focus(); fireEvent.submit(field.closest("form")!); expect(document.activeElement).not.toBe(field); // Turn in flight, game opened. - rerender( - , - ); + rerender(); // Reply lands mid-game: the composer must not grab the keys the game is using. - rerender( - , - ); + rerender(); expect(document.activeElement).not.toBe(field); // Game closed: the caret comes back to the composer. - rerender( - , - ); + rerender(); expect(document.activeElement).toBe(field); }); }); diff --git a/tests/unit/features/buddy/BuddyConversation.test.tsx b/tests/unit/features/buddy/BuddyConversation.test.tsx index dae0d0b8..1a98be3f 100644 --- a/tests/unit/features/buddy/BuddyConversation.test.tsx +++ b/tests/unit/features/buddy/BuddyConversation.test.tsx @@ -2,6 +2,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi } from "vitest"; import { BuddyConversation } from "../../../../src/features/buddy/components/BuddyConversation"; +import { BuddyDraftContext } from "../../../../src/features/buddy/buddyDraftContext"; import type { BuddyMessageView } from "../../../../src/features/buddy/types"; /** @@ -36,18 +37,19 @@ function renderConversation(overrides: { onRetryOpen?: () => void; }) { return render( - , + // The composer inside reads the shared draft from its own provider now — see + // `BuddyDraftProvider`. The conversation itself never touches it. + + + , ); } diff --git a/tests/unit/features/buddy/BuddyDock.test.tsx b/tests/unit/features/buddy/BuddyDock.test.tsx index 6f60a11c..63d10ddd 100644 --- a/tests/unit/features/buddy/BuddyDock.test.tsx +++ b/tests/unit/features/buddy/BuddyDock.test.tsx @@ -2,6 +2,10 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi } from "vitest"; import { BuddyDock } from "../../../../src/features/buddy/components/BuddyDock"; +import { + BuddyDraftActionsContext, + BuddyDraftContext, +} from "../../../../src/features/buddy/buddyDraftContext"; import type { BuddyMessageView } from "../../../../src/features/buddy/types"; import type { BuddySuggestion } from "../../../../src/services/buddyService"; @@ -42,23 +46,27 @@ function renderDock( } = {}, ) { return render( - , + // The words come from the shared composer now (`BuddyDraftProvider`): the chips fill the box + // through the write-only half, and the box inside reads the value. The dock's own props no + // longer carry either. + + + + + , ); } diff --git a/tests/unit/features/buddy/useBuddyTeamMode.test.tsx b/tests/unit/features/buddy/useBuddyTeamMode.test.tsx index 36b43c96..c2a7fcd7 100644 --- a/tests/unit/features/buddy/useBuddyTeamMode.test.tsx +++ b/tests/unit/features/buddy/useBuddyTeamMode.test.tsx @@ -340,11 +340,8 @@ describe("useBuddyConversation — team mode", () => { }); await waitFor(() => expect(result.current.messages.length).toBeGreaterThan(0)); - act(() => { - result.current.setDraft("shift Task 0"); - }); - act(() => { - result.current.handleSubmit({ preventDefault: vi.fn() } as unknown as React.FormEvent); + await act(async () => { + await result.current.sendMessage("shift Task 0"); }); await waitFor(() => { expect(result.current.messages.some((m) => m.actions?.length)).toBe(true); @@ -386,11 +383,8 @@ describe("useBuddyConversation — team mode", () => { await result.current.ensureOpened(); }); await waitFor(() => expect(result.current.messages.length).toBeGreaterThan(0)); - act(() => { - result.current.setDraft("shift Task 0"); - }); - act(() => { - result.current.handleSubmit({ preventDefault: vi.fn() } as unknown as React.FormEvent); + await act(async () => { + await result.current.sendMessage("shift Task 0"); }); await waitFor(() => { expect(result.current.messages.some((m) => m.actions?.length)).toBe(true); diff --git a/tests/unit/features/buddy/useHandedOffDraft.test.tsx b/tests/unit/features/buddy/useHandedOffDraft.test.tsx index 59926d63..7b92f681 100644 --- a/tests/unit/features/buddy/useHandedOffDraft.test.tsx +++ b/tests/unit/features/buddy/useHandedOffDraft.test.tsx @@ -2,8 +2,13 @@ import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi } from "vitest"; import { useState } from "react"; +import type { ReactNode } from "react"; import { MemoryRouter, Route, Routes, useNavigate } from "react-router-dom"; import { useHandedOffDraft } from "../../../../src/features/buddy/useHandedOffDraft"; +import { + BuddyDraftActionsContext, + BuddyDraftContext, +} from "../../../../src/features/buddy/buddyDraftContext"; vi.mock("../../../../src/context/useAuth", () => ({ useAuth: () => ({ @@ -119,29 +124,44 @@ describe("useHandedOffDraft", () => { }); describe("the dock’s hand-off control", () => { + /** + * The dock takes its words from the shared composer now (`BuddyDraftProvider`): its chips fill + * the box through the write-only half, and the composer inside reads the value. Standing the + * two contexts in for the provider is the only way to render it — and `draft` is the box's + * contents, which is what the hand-off is about. + */ + function withDraft(node: ReactNode, draft = "") { + return ( + + + {node} + + + ); + } + it("is absent on the page it would open", async () => { // Guarded in BuddyWidget rather than here; this documents the intent that a control // offering the page you are already reading is not offered at all. const { BuddyDock } = await import("../../../../src/features/buddy/components/BuddyDock"); render( - , + withDraft( + , + ), ); expect(screen.queryByLabelText("Open the full buddy page")).not.toBeInTheDocument(); @@ -153,24 +173,24 @@ describe("the dock’s hand-off control", () => { const user = userEvent.setup(); render( - , + withDraft( + , + "half a question", + ), ); await user.click(screen.getByLabelText("Open the full buddy page")); From 46a6a48a17e851cf7978ef2c29694feb9172ae96 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 10:03:28 +0200 Subject: [PATCH 02/10] perf(buddy): memoise the transcript, one memoised row per turn (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The split took the keystroke off the conversation's re-render path; this closes the half a streamed token walks. Every token lands in the same `messages` array the thread renders from, so the whole thread re-rendered per token — every reply re-parsed, every bubble's animation hooks re-evaluated — and with the thread memoised but its props unstable, every row would still have re-rendered in turn. * `BuddyThread` is memoised, and each turn is now its own memoised `BuddyThreadRow` — the shape `MessageRow` in the chat already uses. A token changes one message object, so one row re-renders and the others bail out on referential equality, Markdown and all. * `BuddyMarkdown` is memoised on `content`. A reply's text never changes once written, so a render that did not change it skips the parse entirely. (The row is the first line of defence; this is the second.) * The row contract is written where it will be read: the render callbacks must be stable functions and `lastMessageFooter` must come from one `useMemo`, because either rebuilt per render hands every row a new prop and undoes the memo. `renderQuestionAction`/`renderReplyAction` now say so, and both callers do it. * The footer's gate — "is this the latest reply, and has the buddy stopped writing" — moved up into the thread, so a row never has to know about its neighbours. `BuddyComposerIsolation` (new) is the guard that this stays true. It mounts `/buddy` with a 32-message transcript under the real provider and counts how many times a reply is parsed (the Markdown renderer is stubbed to count): typing must cost zero parses, and each streamed token exactly one — its own reply, and no other. It is the assertion the rest of the suite cannot make, because every other buddy test looks at a single surface and the lag lived in the wiring between them. --- .../buddy/components/BuddyMarkdown.tsx | 11 +- src/features/buddy/components/BuddyThread.tsx | 245 ++++++++++++------ .../buddy/BuddyComposerIsolation.test.tsx | 138 ++++++++++ 3 files changed, 311 insertions(+), 83 deletions(-) create mode 100644 tests/unit/features/buddy/BuddyComposerIsolation.test.tsx diff --git a/src/features/buddy/components/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index 7f567da8..ca7256c0 100644 --- a/src/features/buddy/components/BuddyMarkdown.tsx +++ b/src/features/buddy/components/BuddyMarkdown.tsx @@ -1,3 +1,4 @@ +import { memo } from "react"; import ReactMarkdown from "react-markdown"; import remarkGfm from "remark-gfm"; @@ -20,8 +21,14 @@ import remarkGfm from "remark-gfm"; * * `break-words` is safe to put on every `code`: inside a `pre` the whitespace is preserved and no * wrapping is allowed at all, so it applies to inline code only and code blocks still scroll. + * + * Memoised on `content`: parsing Markdown is the single most expensive thing a reply does, and a + * reply's text never changes once written — so a render that did not change the text (a keystroke + * that reaches a parent, a token that belongs to a different row) skips the whole parse. The + * memoised row above this is the first line of defence; this is the second, for the renders that + * legitimately re-render the message (a later token in the same reply re-parses only that reply). */ -export function BuddyMarkdown({ content }: { content: string }) { +function BuddyMarkdownImpl({ content }: { content: string }) { return (
); } + +export const BuddyMarkdown = memo(BuddyMarkdownImpl); diff --git a/src/features/buddy/components/BuddyThread.tsx b/src/features/buddy/components/BuddyThread.tsx index 580281de..9746e14f 100644 --- a/src/features/buddy/components/BuddyThread.tsx +++ b/src/features/buddy/components/BuddyThread.tsx @@ -1,4 +1,4 @@ -import { Fragment } from "react"; +import { Fragment, memo } from "react"; import type { ReactNode } from "react"; import { AlertCircle, RotateCcw } from "lucide-react"; import { Button } from "../../../components/ui/Button"; @@ -40,6 +40,9 @@ type BuddyThreadProps = { * verdict on the reply — but a hire does not flag an answer, they flag the question they * still need answered, and they may well want to send one they asked ten minutes ago. Under * every question, they can. Both surfaces pass it, so the corner window can escalate too. + * + * Must be referentially stable, like `renderReplyAction`: the rows below are memoised, and a + * fresh function per render of the caller would re-render every turn with it. */ renderQuestionAction?: (question: string) => ReactNode; /** @@ -48,6 +51,9 @@ type BuddyThreadProps = { * Where keeping something from the conversation belongs. The thread does not know what is worth * keeping or how — it hands over the text and lets the caller decide, which is what stops this * component from growing a dependency on the board. + * + * Must be referentially stable: a fresh function per render of the caller would hand every + * memoised row a new prop and re-parse every reply's markdown with it. */ renderReplyAction?: (reply: string, message: BuddyMessageView) => ReactNode; /** @@ -89,6 +95,133 @@ type BuddyThreadProps = { freshVisitShortcut?: string; }; +type BuddyThreadRowProps = { + message: BuddyMessageView; + /** Whether this row is the turn currently receiving tokens. */ + isStreaming: boolean; + showNames: boolean; + compact: boolean; + confirmAction: (messageId: string, action: ProposedAction) => void; + dismissAction: (messageId: string, actionId: string) => void; + renderQuestionAction?: (question: string) => ReactNode; + renderReplyAction?: (reply: string, message: BuddyMessageView) => ReactNode; + /** The greeting's suggested next step — present on the row it hangs under, nowhere else. */ + lastMessageFooter?: ReactNode; + onStartFreshVisit?: () => void; + freshVisitShortcut?: string; +}; + +/** + * One turn: the visit divider if it opens one, then the bubble. + * + * Extracted from the thread's map and memoised for the same reason `MessageRow` in the chat is: + * with the thread memoised, a keystroke never reaches it — and when a token arrives, only the row + * it belongs to re-renders, while every other row's props stay referentially equal and it bails + * out instead of re-running `ReactMarkdown` over its reply. In a fifty-message thread that is the + * difference between one markdown parse and fifty per keystroke (issue #236). + * + * Which makes referential stability a *contract* on the props: the render callbacks must come + * from `useCallback` in the caller, and `lastMessageFooter` from one `useMemo`. + * + * `lastMessageFooter` is resolved by the thread rather than here — "is this the last reply, and + * has the buddy stopped writing" is a question about the whole list, and answering it in the row + * would make each row depend on its neighbours. + */ +function BuddyThreadRowImpl({ + message, + isStreaming, + showNames, + compact, + confirmAction, + dismissAction, + renderQuestionAction, + renderReplyAction, + lastMessageFooter, + onStartFreshVisit, + freshVisitShortcut, +}: BuddyThreadRowProps) { + const isUser = message.role === "USER"; + const hasText = message.content.trim().length > 0; + const hasActions = (message.actions?.length ?? 0) > 0; + + // Until the first token (or an action proposal) arrives the streaming placeholder has + // nothing to show, and the typing bubble below already stands in for it — so skip it, + // otherwise an empty second bubble appears while the buddy is working. A turn that + // failed before writing a word is the exception: its reason *is* the message, and + // dropping it here is what made a failed reply look like no reply. + if (!isUser && !hasText && !hasActions && !message.error) return null; + + return ( + + {/* Everything above belongs to the last conversation; the buddy has just opened a + new one under it, grounded in what it remembers rather than in the text + above. Saying so is what stops the greeting reading as a non-sequitur + replying to a question from an hour ago. */} + {message.startsVisit && ( +
+
+ )} + + + {isUser && renderQuestionAction?.(message.content)} + {!isUser && hasText && renderReplyAction?.(message.content, message)} + {!isUser && hasActions && ( + + )} + {lastMessageFooter} + + } + > + {hasText ? ( + isUser ? ( + message.content + ) : ( + + ) + ) : undefined} + + + ); +} + +const BuddyThreadRow = memo(BuddyThreadRowImpl); + /** * The conversation itself: every message, in order, with whoever is talking beside it. * @@ -100,8 +233,13 @@ type BuddyThreadProps = { * because a flex item's default `min-width: auto` refuses to shrink below its content — without * it a wide code block widens the bubble, the column and the panel, and the per-block scrollers * never engage. + * + * Both this and every row in it are memoised: re-rendering a long thread is what used to make + * each keystroke re-run `ReactMarkdown` over every reply and every bubble's animation hooks + * (issue #236). A keystroke no longer reaches this component at all, and a token reaches one row + * — see `BuddyThreadRow` for the contract that keeps that true. */ -export function BuddyThread({ +function BuddyThreadImpl({ messages, isThinking, isStreaming = false, @@ -153,86 +291,27 @@ export function BuddyThread({
)} - {messages.map((message) => { - const isUser = message.role === "USER"; - const hasText = message.content.trim().length > 0; - const hasActions = (message.actions?.length ?? 0) > 0; - - // Until the first token (or an action proposal) arrives the streaming placeholder has - // nothing to show, and the typing bubble below already stands in for it — so skip it, - // otherwise an empty second bubble appears while the buddy is working. A turn that - // failed before writing a word is the exception: its reason *is* the message, and - // dropping it here is what made a failed reply look like no reply. - if (!isUser && !hasText && !hasActions && !message.error) return null; - - return ( - - {/* Everything above belongs to the last conversation; the buddy has just opened a - new one under it, grounded in what it remembers rather than in the text - above. Saying so is what stops the greeting reading as a non-sequitur - replying to a question from an hour ago. */} - {message.startsVisit && ( -
-
- )} - - - {isUser && renderQuestionAction?.(message.content)} - {!isUser && hasText && renderReplyAction?.(message.content, message)} - {!isUser && hasActions && ( - - )} - {!isThinking && message.id === lastAssistantId && lastMessageFooter} - - } - > - {hasText ? ( - isUser ? ( - message.content - ) : ( - - ) - ) : undefined} - - - ); - })} + {messages.map((message) => ( + + ))} {(isThinking || dinoGameActive) && ( ); } + +export const BuddyThread = memo(BuddyThreadImpl); diff --git a/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx new file mode 100644 index 00000000..b3485737 --- /dev/null +++ b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx @@ -0,0 +1,138 @@ +import { act, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { MemoryRouter } from "react-router-dom"; +import { BuddyPage } from "../../../../src/pages/BuddyPage"; +import { BuddyProvider } from "../../../../src/features/buddy/BuddyProvider"; +import { BuddyMarkdown } from "../../../../src/features/buddy/components/BuddyMarkdown"; +import type { BuddyMessage, BuddyStreamHandlers } from "../../../../src/features/buddy/types"; +import { getMessages, streamMessage } from "../../../../src/services/buddyService"; +import { BuddyTestProviders, createProjectValue } from "./buddyTestHarness"; + +/** + * Issue #236, pinned: a keystroke in the composer must cost one render of one box — never a + * re-parse of the conversation behind it. + * + * The regression this guards is not visible to any other test in the suite: every buddy test + * asserts what a *single* surface shows, and the lag lived in the wiring between them. Here the + * page is mounted with a long transcript under the real provider, and the number of times a reply + * is parsed is counted — zero for a keystroke, one per streamed token for the single reply + * receiving them. + */ +vi.mock("../../../../src/features/buddy/components/BuddyMarkdown", () => ({ + // Counted rather than rendered: how many *times* a reply is parsed is the subject here, and a + // stub that keeps the count answers it directly. What the renderer draws has its own tests. + BuddyMarkdown: vi.fn(({ content }: { content: string }) =>
{content}
), +})); + +vi.mock("../../../../src/services/buddyService", () => ({ + getMessages: vi.fn(), + // The visit has history, so nothing greets — see `ensureOpened`. + streamOpenBuddy: vi.fn(() => Promise.resolve()), + streamMessage: vi.fn(), + performAction: vi.fn(), + getSuggestions: vi.fn().mockResolvedValue([]), +})); + +/** Sixteen questions and sixteen replies — the "20–50 messages" case from the issue. */ +const LONG_CONVERSATION: BuddyMessage[] = Array.from({ length: 32 }, (_, index) => + index % 2 === 0 + ? { role: "USER", content: `Question ${index / 2}`, createdAt: "2026-08-03T00:00:00Z" } + : { + role: "ASSISTANT", + content: `Reply **${(index - 1) / 2}**\n\n- first\n- second`, + createdAt: "2026-08-03T00:00:00Z", + }, +); + +const markdown = vi.mocked(BuddyMarkdown); + +/** What the hook handed the streaming service, so a test can drive one token at a time. */ +let streamHandlers: BuddyStreamHandlers | null = null; +let finishStream: (() => void) | null = null; + +function renderPage() { + return render( + + + + + + + , + ); +} + +describe("the composer in a long conversation", () => { + beforeEach(() => { + markdown.mockClear(); + streamHandlers = null; + finishStream = null; + vi.mocked(getMessages).mockResolvedValue(LONG_CONVERSATION); + vi.mocked(streamMessage).mockImplementation((_content, handlers) => { + streamHandlers = handlers; + // Held open until the test says so, so a token can be driven deliberately. + return new Promise((resolve) => { + finishStream = resolve; + }); + }); + }); + + it("does not re-parse the conversation while the hire types", async () => { + const user = userEvent.setup(); + renderPage(); + + // The whole thread is on screen first: sixteen replies, each parsed exactly once. + await screen.findByText(/Reply \*\*15\*\*/); + expect(markdown.mock.calls.length).toBeGreaterThanOrEqual(16); + + markdown.mockClear(); + + await user.type(screen.getByLabelText("Message"), "why is this slow?"); + + // Every one of those replies stayed exactly as it was — the whole of #236 in one assertion. + expect(markdown).not.toHaveBeenCalled(); + }); + + it("sends what was typed, and re-parses only the reply that is streaming", async () => { + const user = userEvent.setup(); + renderPage(); + await screen.findByText(/Reply \*\*15\*\*/); + + const field = screen.getByLabelText("Message"); + await user.type(field, "what is left?"); + markdown.mockClear(); + await user.click(screen.getByRole("button", { name: "Send message" })); + + // The box gave up its words, and the turn is the hook's now. + expect(field).toHaveValue(""); + await waitFor(() => expect(vi.mocked(streamMessage)).toHaveBeenCalled()); + expect(vi.mocked(streamMessage).mock.calls[0][0]).toBe("what is left?"); + expect(streamHandlers).not.toBeNull(); + + // A send is one honest re-render of the conversation — a row joins it, and the fresh-visit + // control withdraws while the turn is in flight — so the count is reset *after* it. What + // follows is the real subject: each token belongs to one reply and must cost one parse. + markdown.mockClear(); + + act(() => { + streamHandlers?.onToken(" first"); + }); + expect(markdown).toHaveBeenCalledTimes(1); + + act(() => { + streamHandlers?.onToken(" second"); + }); + expect(markdown).toHaveBeenCalledTimes(2); + + act(() => { + streamHandlers?.onDone(); + finishStream?.(); + }); + + // The turn lands: what the tokens built is on screen, and no assertion above needed it to. + await screen.findByText(/first\s*second/); + }); +}); From 62bc4a8270d7f5d123774a9e3d25fc55f14a1667 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 11:24:17 +0200 Subject: [PATCH 03/10] fix(buddy): hold only the live turn's bot awake (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thread asked its rows "is this the newest message?" where it meant "is this the turn receiving tokens right now". The two only agree while a turn is running, so the newest reply was flagged as streaming for the rest of the conversation — and `BuddyMessage` hands that flag straight to `SleepyBot` as `canSleep={!isStreaming}`. A bot that may never sleep never does: the newest reply was the one bubble that stayed visibly working, forever. `Boolean(isStreaming && ...)` releases the flag the moment the turn ends. It is also a prop the memoised rows compare, so getting it exactly right is what keeps a finished turn from re-rendering the thread. The chat has drawn this line correctly from the start — `MessageRow` sets `canSleep={message.id !== streamingMessageId}` where `streamingMessageId` is null whenever nothing is streaming — and the buddy's row was forked from it without that null. This is the same line, in the buddy's shape. Two smaller things in the same corner, from the same reading of the file: - `lastAssistantId`'s note still described the escalation offer it used to carry. Escalation lives under the hire's own questions now; what the footer under a reply carries is the greeting's one suggested next step, and the note says so. - A reply that is only a proposal counts as a reply. The empty message the send loop appends is the one worth skipping, and skipping it should mean "nothing written yet, and nothing offered" — not "no prose, so look further back for something to hang the footer under". The new test renders the thread with the bubble stood in for and reads the flag off each row: nothing is flagged at rest, the last row while the turn runs, and nothing again once it lands. On the parent commit both assertions fail. --- src/features/buddy/components/BuddyThread.tsx | 26 ++++-- .../buddy/BuddyThreadStreaming.test.tsx | 86 +++++++++++++++++++ 2 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 tests/unit/features/buddy/BuddyThreadStreaming.test.tsx diff --git a/src/features/buddy/components/BuddyThread.tsx b/src/features/buddy/components/BuddyThread.tsx index 9746e14f..822f99cf 100644 --- a/src/features/buddy/components/BuddyThread.tsx +++ b/src/features/buddy/components/BuddyThread.tsx @@ -260,14 +260,26 @@ function BuddyThreadImpl({ freshVisitShortcut, }: BuddyThreadProps) { // The send loop appends an empty assistant message up front and streams into it, so the last - // one is the turn receiving tokens. + // one is the turn receiving tokens — while a turn is running at all. Being last is not on its + // own "live": holding the newest row awake forever is a bot that never sleeps, so the row is + // only flagged while tokens are actually arriving. The chat draws the same line in its + // `MessageRow` (`streamingMessageId` is null when idle) — see `SleepyBot`'s `canSleep`. const streamingId = messages[messages.length - 1]?.id; - // Which message the escalation offer hangs under: the buddy's most recent reply. Not every - // reply — an offer to give up repeated under all of them reads as the buddy expecting to fail. + // Which turn the footer hangs under: the buddy's most recent reply. Not every reply — the same + // suggestion repeated under all of them reads as the buddy repeating itself. (This started as + // the escalation offer, which now lives under the hire's own questions — see + // `renderQuestionAction` on the props above.) + // + // A reply carrying only a proposal counts as a reply: the *empty* message the send loop appends + // up front is the one to skip, and skipping it means "nothing written yet, and nothing offered". const lastAssistantId = [...messages] .reverse() - .find((message) => message.role === "ASSISTANT" && message.content.trim().length > 0)?.id; + .find( + (message) => + message.role === "ASSISTANT" && + (message.content.trim().length > 0 || (message.actions?.length ?? 0) > 0), + )?.id; return (
@@ -295,7 +307,11 @@ function BuddyThreadImpl({ ({ + // Stands in for the bubble so the flag it receives is readable from the DOM: the subject here + // is that flag, and the bot that reads it (`SleepyBot`'s `canSleep`) is three components down. + BuddyMessage: ({ + speaker, + children, + isStreaming, + }: { + speaker: string; + children?: ReactNode; + isStreaming?: boolean; + }) => ( +
+ {children} +
+ ), +})); + +const MESSAGES: BuddyMessageView[] = [ + { id: "u1", role: "USER", content: "what is left?", createdAt: "2026-08-03T00:00:00Z" }, + { id: "a1", role: "ASSISTANT", content: "Two things.", createdAt: "2026-08-03T00:00:01Z" }, + { id: "u2", role: "USER", content: "which two?", createdAt: "2026-08-03T00:00:02Z" }, + { + id: "a2", + role: "ASSISTANT", + content: "The packet and the review.", + createdAt: "2026-08-03T00:00:03Z", + }, +]; + +const BASE_PROPS = { + messages: MESSAGES, + isThinking: false, + activeTool: null, + confirmAction: vi.fn(), + dismissAction: vi.fn(), +}; + +/** One flag per row, in the order they are on screen. */ +function streamingFlags(): string[] { + return screen.getAllByTestId("buddy-row").map((row) => row.getAttribute("data-streaming") ?? "?"); +} + +describe("the thread's streaming flag", () => { + it("flags nothing while the conversation is at rest — the newest reply included", () => { + render(); + + // Being last is not being live. The newest reply's bot is as free to doze off as the first + // one's; anything else is a bubble that is visibly working forever. + expect(streamingFlags()).toEqual(["false", "false", "false", "false"]); + }); + + it("flags only the live turn, and only while it is live", () => { + const { rerender } = render(); + + rerender(); + + // The reply the tokens are landing in is the one held awake — and only that one. + expect(streamingFlags()).toEqual(["false", "false", "false", "true"]); + + rerender(); + + // And it is released the moment the turn ends: the bot settles back into the thread. + expect(streamingFlags()).toEqual(["false", "false", "false", "false"]); + }); +}); From 0ff67a2649c3c42c022e51372d0f4ab9ff2af1db Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 11:24:24 +0200 Subject: [PATCH 04/10] perf(buddy): keep the floating window out of the keystroke path (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page stopped re-rendering per keystroke in the first two commits. The widget had not: it read the draft through `useBuddy()`, and the widget is the one buddy surface mounted on every page — so a keystroke in the dock re-rendered the widget, and with it the dock, its header and every bubble in it. On `/buddy`, where the widget draws nothing, it still re-rendered on every character. Measured by the test added here, on the parent commit: eighteen keystrokes in the dock cost the floating surfaces 51 renders, and the same typing on `/buddy` cost 17. `useBuddy()` no longer reads the draft. It keeps the one thing it actually needed the setter for — the aiBuddyBus seeding effect ("Draft with AI"), which now takes the write-only half — and any surface that needs the value itself takes `useBuddyDraft()`. That was only possible because the hand-off stopped carrying the draft, which was the other half of the same leak. `goToPage` passed the words in history state and `useHandedOffDraft` applied them on arrival; but `BuddyDraftProvider` sits above the router, so the dock and the page have been reading one box since the first commit of this branch. The copy was a second mechanism for something the architecture already gave, and it was the reason `goToPage` depended on the draft. `useHandedOffDraft` is gone; the hand-off is `navigate("/buddy")`. On top of that, the dock and its header control are memoised. Every prop the dock takes is a plain value, a callback the widget holds in one identity, or a motion value — so a render of the widget that changes none of them (a resize while the dock is open, a drag across the screen) no longer walks the whole panel. `headerControl` is held in one identity for the same reason: built inline it would have been the one prop the memo always found changed. Tests: - The isolation file gains the floating half of the issue — a keystroke must cost zero renders of the widget/dock *and* zero markdown parses, with the render count proven wired before it is trusted, so a spy that never fires cannot pass vacuously. - `buddyDraftAcrossRoutes` (replacing the retired hook's tests) holds the replacement in place: the words cross because both surfaces read one box above the router, and clearing them on the page survives a back-and-forward that used to re-seed them out of the history entry. - The two tests that drive the session *through the composer* now use a test-only paired hook, `useBuddyWithDraft` — the pairing is what a test needs and no production surface does, so it lives in the harness. --- src/features/buddy/components/BuddyDock.tsx | 12 +- src/features/buddy/components/BuddyWidget.tsx | 42 +++- src/features/buddy/hooks/useBuddy.ts | 16 +- src/features/buddy/useHandedOffDraft.ts | 43 ---- src/pages/BuddyPage.tsx | 4 - .../buddy/BuddyComposerIsolation.test.tsx | 110 ++++++++-- .../buddy/buddyDraftAcrossRoutes.test.tsx | 205 ++++++++++++++++++ .../features/buddy/buddyEasterEggs.test.tsx | 5 +- .../unit/features/buddy/buddyTestHarness.tsx | 14 ++ tests/unit/features/buddy/useBuddy.test.tsx | 27 ++- .../features/buddy/useHandedOffDraft.test.tsx | 200 ----------------- 11 files changed, 379 insertions(+), 299 deletions(-) delete mode 100644 src/features/buddy/useHandedOffDraft.ts create mode 100644 tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx delete mode 100644 tests/unit/features/buddy/useHandedOffDraft.test.tsx diff --git a/src/features/buddy/components/BuddyDock.tsx b/src/features/buddy/components/BuddyDock.tsx index f4a6f0c1..23bb4f80 100644 --- a/src/features/buddy/components/BuddyDock.tsx +++ b/src/features/buddy/components/BuddyDock.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef } from "react"; +import { memo, useCallback, useEffect, useRef } from "react"; import type { ReactNode } from "react"; import { motion, useReducedMotion, type MotionValue } from "framer-motion"; import { Maximize2, MessageSquarePlus, Minus, X } from "lucide-react"; @@ -127,7 +127,7 @@ type BuddyDockProps = Pick< * the full viewport, and the caller changes the route as it lands. Without that the dock * vanished and a page appeared, and nobody could tell it was the same conversation. */ -export function BuddyDock({ +function BuddyDockImpl({ messages, isThinking, isStreaming, @@ -411,3 +411,11 @@ export function BuddyDock({ ); } + +/** + * Memoised, like `BuddyThread`: every prop on this panel is either a plain value, a callback the + * widget holds in one identity, or a motion value — so a render of the widget that changes none of + * them (a resize while the dock is open, a drag across the screen) no longer re-walks the dock's + * whole layout. `headerControl` is held in one identity at the call site for the same reason. + */ +export const BuddyDock = memo(BuddyDockImpl); diff --git a/src/features/buddy/components/BuddyWidget.tsx b/src/features/buddy/components/BuddyWidget.tsx index 0b6f2eb4..0d6fd6e9 100644 --- a/src/features/buddy/components/BuddyWidget.tsx +++ b/src/features/buddy/components/BuddyWidget.tsx @@ -66,7 +66,6 @@ export function BuddyWidget() { markGreetingPresented, isOpen, toggleOpen, - draft, confirmAction, dismissAction, suggestions, @@ -186,10 +185,18 @@ export function BuddyWidget() { useEffect(() => clearHandoffTimers, [clearHandoffTimers]); + /** + * Hands the conversation over to `/buddy`. + * + * Nothing has to ride along with it: the composer lives in `BuddyDraftProvider`, which sits above + * the router, so the page's box already holds the words this window holds — the hand-off is the + * same conversation on a wider surface, not a transfer. Carrying a copy through history state was + * a second mechanism for that, and it was what made this callback depend on the draft: every + * keystroke rebuilt `goToPage`, then `openFull`, then the dock. + */ const goToPage = useCallback(() => { - // The draft rides along in history state; `useHandedOffDraft` applies it once on the page. - void navigate(BUDDY_PAGE, { state: { draft } }); - }, [draft, navigate]); + void navigate(BUDDY_PAGE); + }, [navigate]); /** * The props the dock's memoised thread compares, each held in one identity. @@ -218,6 +225,22 @@ export function BuddyWidget() { const retryOpenAction = useCallback(() => void retryOpen(), [retryOpen]); const hideSuggestions = useCallback(() => setSuggestionsHidden(true), []); + /** + * The dock's header control, held in one identity for the same reason as the props above: the + * dock is memoised, and a switcher built inline here would be a fresh element on every render of + * the widget — a prop the memo would compare and always find changed. + */ + const headerControl = useMemo( + () => ( + void switchTeamProject(projectId)} + disabled={isTurnInFlight} + /> + ), + [teamProjectId, switchTeamProject, isTurnInFlight], + ); + /** * Grows the open dock into the page — one gesture instead of a cut. * @@ -341,14 +364,9 @@ export function BuddyWidget() { onHideSuggestions={hideSuggestions} // Hire conversation ↔ team conversations, in the header beside the title. The // switcher only *offers* the switch; the restore audit lives in the session - // (`useBuddyConversation` / `BuddyProvider`). - headerControl={ - void switchTeamProject(projectId)} - disabled={isTurnInFlight} - /> - } + // (`useBuddyConversation` / `BuddyProvider`). Memoised above, like the props around + // it: the dock is a memoised component now. + headerControl={headerControl} isExpanding={handoff !== "idle"} isRevealing={handoff === "revealing"} /> diff --git a/src/features/buddy/hooks/useBuddy.ts b/src/features/buddy/hooks/useBuddy.ts index 9cf0e8eb..69ea4082 100644 --- a/src/features/buddy/hooks/useBuddy.ts +++ b/src/features/buddy/hooks/useBuddy.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useState } from "react"; import { onOpenAiBuddy } from "../aiBuddyBus"; -import { useBuddyDraft } from "../buddyDraftContext"; +import { useBuddyDraftActions } from "../buddyDraftContext"; import { useBuddySession } from "../buddySessionContext"; import { useBuddySuggestions } from "./useBuddySuggestions"; @@ -16,13 +16,16 @@ import { useBuddySuggestions } from "./useBuddySuggestions"; * it already there — see the effect below for why that read must never be a bare open. Other * surfaces can open the dock and seed a draft via the aiBuddyBus (e.g. "Draft with AI" on the * human-buddy card). + * + * **The composer's half is deliberately not part of this hook** — no draft, no submit. This hook + * is what the widget calls, and the widget mounts on every page; reading the draft here put every + * keystroke back into the dock's render path (issue #236). Surfaces that need it take + * `useBuddyDraft()` (the value — read by the composer alone) or `useBuddyDraftActions()` (the + * write-only half, which is what the seeding effect below uses, and why it can stay). */ export function useBuddy() { const conversation = useBuddySession(); - // The composer's half lives in its own provider now — see `BuddyDraftProvider`. It is merged - // back in here because every surface that drives the dock wants "the buddy" as one thing, and - // the hand-off to `/buddy` has to read the words the hire was mid-way through typing. - const { draft, setDraft, handleSubmit } = useBuddyDraft(); + const { setDraft } = useBuddyDraftActions(); const { ensureOpened, teamProjectId } = conversation; const [isOpen, setIsOpen] = useState(false); @@ -73,9 +76,6 @@ export function useBuddy() { return { ...conversation, - draft, - setDraft, - handleSubmit, isOpen, toggleOpen, closeDock, diff --git a/src/features/buddy/useHandedOffDraft.ts b/src/features/buddy/useHandedOffDraft.ts deleted file mode 100644 index ca65c540..00000000 --- a/src/features/buddy/useHandedOffDraft.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { useEffect, useRef } from "react"; -import { useLocation, useNavigate } from "react-router-dom"; - -/** What the panel puts in history state when it hands a conversation over to `/buddy`. */ -export type BuddyHandoffState = { draft?: string }; - -/** - * Applies a draft handed over from the floating panel, exactly once. - * - * The panel and the page share one buddy *session* but not one composer, so the draft has to be - * carried across the navigation or it is silently thrown away. - * - * The history state is consumed and then removed (`replace`). Without that, going back and - * forward again — or a reload — re-seeds a draft the hire has since sent or deleted, overwriting - * whatever is in the box. A blank or absent draft does nothing at all. - * - * Call this from exactly one place per route. Two consumers on one page read the same - * payload, and the parent's effect runs after the child has already cleared it, so the second fires - * with a stale value and navigates again. - * - * @param setDraft The composer setter of whichever conversation is mounted. - */ -export function useHandedOffDraft(setDraft: (draft: string) => void): void { - const location = useLocation(); - const navigate = useNavigate(); - const handed = (location.state as BuddyHandoffState | null)?.draft; - - // Clearing the state is not enough on its own to make this fire once. Applying the draft - // re-renders the caller, and a caller whose `setDraft` is not referentially stable gives the - // effect a new dependency on that render — so it runs again *before* the navigation has taken - // the payload off the location, and applies the same draft twice. A test caught exactly that. - // The guard makes the hook independent of how the caller happens to define its setter. - const applied = useRef(null); - - useEffect(() => { - if (!handed?.trim() || applied.current === handed) return; - - applied.current = handed; - setDraft(handed); - // Replace rather than push: this is the same page, with the one-shot payload taken off it. - void navigate(location.pathname + location.search, { replace: true, state: null }); - }, [handed, setDraft, navigate, location.pathname, location.search]); -} diff --git a/src/pages/BuddyPage.tsx b/src/pages/BuddyPage.tsx index 31a2a2c8..8373e23f 100644 --- a/src/pages/BuddyPage.tsx +++ b/src/pages/BuddyPage.tsx @@ -17,7 +17,6 @@ import { useProjectContext } from "../features/projects/useProjectContext"; import { useBuddySuggestions } from "../features/buddy/hooks/useBuddySuggestions"; import { BuddyModeSwitcher } from "../features/buddy/components/BuddyModeSwitcher"; import { useGreetingReveal } from "../features/buddy/hooks/useGreetingReveal"; -import { useHandedOffDraft } from "../features/buddy/useHandedOffDraft"; import { announceBuddyPageReady } from "../features/buddy/aiBuddyBus"; import { NEW_CONVERSATION_CHORD, @@ -267,9 +266,6 @@ function BuddyMentorHome() { [isDesktop], ); - // Whatever they were typing in the dock when they asked for more room. - useHandedOffDraft(setDraft); - const hasUserMessage = messages.some((m) => m.role === "USER"); /** diff --git a/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx index b3485737..fc7eefb7 100644 --- a/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx +++ b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx @@ -1,10 +1,12 @@ import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ReactNode } from "react"; import { MemoryRouter } from "react-router-dom"; import { BuddyPage } from "../../../../src/pages/BuddyPage"; import { BuddyProvider } from "../../../../src/features/buddy/BuddyProvider"; import { BuddyMarkdown } from "../../../../src/features/buddy/components/BuddyMarkdown"; +import { BuddyWidget } from "../../../../src/features/buddy/components/BuddyWidget"; import type { BuddyMessage, BuddyStreamHandlers } from "../../../../src/features/buddy/types"; import { getMessages, streamMessage } from "../../../../src/services/buddyService"; import { BuddyTestProviders, createProjectValue } from "./buddyTestHarness"; @@ -34,6 +36,19 @@ vi.mock("../../../../src/services/buddyService", () => ({ getSuggestions: vi.fn().mockResolvedValue([]), })); +const { floatingRenders } = vi.hoisted(() => ({ floatingRenders: { count: 0 } })); + +vi.mock("../../../../src/features/buddy/buddyCorner", async (importOriginal) => ({ + ...(await importOriginal()), + // Counts the renders of everything that floats: the widget, the dock and the launcher are the + // three components that ask how big the window is. A keystroke must reach none of them, and + // nothing else in this file mounts them. + useViewportSize: () => { + floatingRenders.count += 1; + return { width: 1280, height: 800 }; + }, +})); + /** Sixteen questions and sixteen replies — the "20–50 messages" case from the issue. */ const LONG_CONVERSATION: BuddyMessage[] = Array.from({ length: 32 }, (_, index) => index % 2 === 0 @@ -51,6 +66,21 @@ const markdown = vi.mocked(BuddyMarkdown); let streamHandlers: BuddyStreamHandlers | null = null; let finishStream: (() => void) | null = null; +/** The long conversation is already there, and the next send is held open for tokens. */ +function stubLongConversation() { + markdown.mockClear(); + streamHandlers = null; + finishStream = null; + vi.mocked(getMessages).mockResolvedValue(LONG_CONVERSATION); + vi.mocked(streamMessage).mockImplementation((_content, handlers) => { + streamHandlers = handlers; + // Held open until the test says so, so a token can be driven deliberately. + return new Promise((resolve) => { + finishStream = resolve; + }); + }); +} + function renderPage() { return render( { - beforeEach(() => { - markdown.mockClear(); - streamHandlers = null; - finishStream = null; - vi.mocked(getMessages).mockResolvedValue(LONG_CONVERSATION); - vi.mocked(streamMessage).mockImplementation((_content, handlers) => { - streamHandlers = handlers; - // Held open until the test says so, so a token can be driven deliberately. - return new Promise((resolve) => { - finishStream = resolve; - }); - }); - }); + beforeEach(stubLongConversation); it("does not re-parse the conversation while the hire types", async () => { const user = userEvent.setup(); @@ -136,3 +154,69 @@ describe("the composer in a long conversation", () => { await screen.findByText(/first\s*second/); }); }); + +/** + * The other half of #236: the floating window. + * + * The issue is about the buddy, not about `/buddy` — and the widget is the surface that is always + * mounted, on every page and on `/buddy` too (where it renders nothing). It read the draft through + * `useBuddy()`, which put a keystroke in the dock — and a keystroke on the page — back into the + * widget's render path, taking the dock and every reply in it along for the ride. + */ +describe("the buddy's floating window", () => { + beforeEach(() => { + stubLongConversation(); + floatingRenders.count = 0; + }); + + function renderFloating(node: ReactNode, route = "/board") { + return render( + + + {node} + + , + ); + } + + it("does not re-render — or re-parse anything — for a keystroke in the dock", async () => { + const user = userEvent.setup(); + renderFloating(); + + // Open it and let the conversation arrive: this is the screen a hire types on. + await user.click(await screen.findByLabelText("Open buddy chat")); + const field = await screen.findByLabelText("Message"); + await waitFor(() => expect(vi.mocked(getMessages)).toHaveBeenCalled()); + + markdown.mockClear(); + const before = floatingRenders.count; + // The count is wired before it is trusted: a spy that never fires would pass vacuously. + expect(before).toBeGreaterThan(0); + + await user.type(field, "why is this slow?"); + + expect(markdown).not.toHaveBeenCalled(); + expect(floatingRenders.count).toBe(before); + }); + + it("does not re-render while the hire types on /buddy either", async () => { + const user = userEvent.setup(); + renderFloating( + <> + + + , + "/buddy", + ); + + await screen.findByText(/Reply \*\*15\*\*/); + const before = floatingRenders.count; + expect(before).toBeGreaterThan(0); + + await user.type(screen.getByLabelText("Message"), "why is this slow?"); + + expect(floatingRenders.count).toBe(before); + }); +}); diff --git a/tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx b/tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx new file mode 100644 index 00000000..7b7b0433 --- /dev/null +++ b/tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx @@ -0,0 +1,205 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi } from "vitest"; +import type { ReactNode } from "react"; +import { MemoryRouter, Route, Routes, useNavigate } from "react-router-dom"; +import { + BuddyDraftActionsContext, + BuddyDraftContext, + useBuddyDraft, +} from "../../../../src/features/buddy/buddyDraftContext"; +import { BuddyDraftProvider } from "../../../../src/features/buddy/BuddyDraftProvider"; +import { + BuddySessionContext, + type BuddySession, +} from "../../../../src/features/buddy/buddySessionContext"; + +/** + * The composer's words across navigation. + * + * They used to be carried in history state — the dock navigated to `/buddy` with the draft in + * `location.state`, and a hook on the page applied it on arrival. That was a second mechanism for + * something the shape of the app already gives: `BuddyDraftProvider` sits above the router, so the + * dock and the page read one box and the hand-off is the same conversation drawn wider. Keeping + * the copy was also what tied the widget's `goToPage` to the draft, and with it every keystroke to + * the dock (issue #236) — so these tests hold the replacement in place. + */ +describe("the composer's words across navigation", () => { + /** Stands in for the dock's composer and its "open full" control. */ + function Dock() { + const { draft, setDraft } = useBuddyDraft(); + const navigate = useNavigate(); + + return ( + <> + + + + ); + } + + /** The page's box, plus the two history moves the second test needs. */ + function Page() { + const { draft, setDraft } = useBuddyDraft(); + const navigate = useNavigate(); + + return ( + <> + {draft} + + + + ); + } + + function Board() { + const navigate = useNavigate(); + + return ( + <> + + + + ); + } + + /** + * The app's own shape: the provider above the router, so the one box outlives any single route. + * The session is stubbed to the two things the draft provider reads from it. + */ + function renderApp() { + const session = { + sendMessage: vi.fn().mockResolvedValue(undefined), + draftResetToken: 0, + } as unknown as BuddySession; + + return render( + + + + + } /> + } /> + + + + , + ); + } + + it("are the same words on the page as in the dock", async () => { + const user = userEvent.setup(); + renderApp(); + + await user.type(screen.getByLabelText("dock box"), "how do I run the migrations"); + await user.click(screen.getByRole("button", { name: "open full" })); + + expect(screen.getByTestId("page-box")).toHaveTextContent("how do I run the migrations"); + }); + + it("do not come back once cleared — nothing of the box rides in history", async () => { + const user = userEvent.setup(); + renderApp(); + + await user.type(screen.getByLabelText("dock box"), "half a question"); + await user.click(screen.getByRole("button", { name: "open full" })); + + await user.click(screen.getByRole("button", { name: "clear" })); + expect(screen.getByTestId("page-box")).toBeEmptyDOMElement(); + + // Back to the board, then forward again — the very history entry the dock navigated to. With + // a draft carried in that entry's state, arriving here re-seeded the box with words the hire + // had since deleted. + await user.click(screen.getByRole("button", { name: "back" })); + await user.click(screen.getByRole("button", { name: "forward" })); + + expect(screen.getByTestId("page-box")).toBeEmptyDOMElement(); + }); +}); + +describe("the dock’s hand-off control", () => { + /** + * The dock takes its words from the shared composer (`BuddyDraftProvider`): its chips fill the + * box through the write-only half, and the composer inside reads the value. Standing the two + * contexts in for the provider is the only way to render it — and `draft` is the box's + * contents, which is what the hand-off is about. + */ + function withDraft(node: ReactNode, draft = "") { + return ( + + + {node} + + + ); + } + + it("is absent on the page it would open", async () => { + // Guarded in BuddyWidget rather than here; this documents the intent that a control + // offering the page you are already reading is not offered at all. + const { BuddyDock } = await import("../../../../src/features/buddy/components/BuddyDock"); + + render( + withDraft( + , + ), + ); + + expect(screen.queryByLabelText("Open the full buddy page")).not.toBeInTheDocument(); + }); + + it("is offered when there is somewhere to go", async () => { + const { BuddyDock } = await import("../../../../src/features/buddy/components/BuddyDock"); + const onOpenFull = vi.fn(); + const user = userEvent.setup(); + + render( + withDraft( + , + "half a question", + ), + ); + + await user.click(screen.getByLabelText("Open the full buddy page")); + + expect(onOpenFull).toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/features/buddy/buddyEasterEggs.test.tsx b/tests/unit/features/buddy/buddyEasterEggs.test.tsx index 27f4fe2a..60905abb 100644 --- a/tests/unit/features/buddy/buddyEasterEggs.test.tsx +++ b/tests/unit/features/buddy/buddyEasterEggs.test.tsx @@ -1,7 +1,6 @@ import { renderHook, act, waitFor } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { useBuddy } from "../../../../src/features/buddy/hooks/useBuddy"; -import { BuddyProviderWithStubs } from "./buddyTestHarness"; +import { BuddyProviderWithStubs, useBuddyWithDraft } from "./buddyTestHarness"; import { http, HttpResponse } from "msw"; import { server } from "../../setup/vitest.setup"; @@ -35,7 +34,7 @@ describe("buddy easter eggs", () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); server.use(http.post("/api/v1/onboarding/me/buddy/open/stream", () => silentGreeting())); - const harness = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const harness = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); await waitFor(() => { expect(harness.result.current.messages).toHaveLength(0); }); diff --git a/tests/unit/features/buddy/buddyTestHarness.tsx b/tests/unit/features/buddy/buddyTestHarness.tsx index 8a11b827..ffaab424 100644 --- a/tests/unit/features/buddy/buddyTestHarness.tsx +++ b/tests/unit/features/buddy/buddyTestHarness.tsx @@ -12,6 +12,8 @@ import { type ProjectContextValue, } from "../../../../src/features/projects/ProjectContext"; import { BuddyProvider } from "../../../../src/features/buddy/BuddyProvider"; +import { useBuddyDraft } from "../../../../src/features/buddy/buddyDraftContext"; +import { useBuddy } from "../../../../src/features/buddy/hooks/useBuddy"; import type { UserProfile } from "../../../../src/services/types"; /** @@ -108,3 +110,15 @@ export function BuddyProviderWithStubs({ children }: { children: ReactNode }) { ); } + +/** + * The dock-driving hook plus the composer's half, as one object — for tests only. + * + * In the app the two are deliberately separate: `useBuddy()` is what the widget calls, and it + * must not read the draft, or every keystroke would re-render the dock (issue #236). A test that + * drives the session *through the composer* — "put these words in the box, then submit them" — + * needs both halves inside one render, and no production surface does, so the pair lives here. + */ +export function useBuddyWithDraft() { + return { ...useBuddy(), ...useBuddyDraft() }; +} diff --git a/tests/unit/features/buddy/useBuddy.test.tsx b/tests/unit/features/buddy/useBuddy.test.tsx index e3664c65..d86d3142 100644 --- a/tests/unit/features/buddy/useBuddy.test.tsx +++ b/tests/unit/features/buddy/useBuddy.test.tsx @@ -1,7 +1,6 @@ import { renderHook, act, waitFor } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { useBuddy } from "../../../../src/features/buddy/hooks/useBuddy"; -import { BuddyProviderWithStubs } from "./buddyTestHarness"; +import { BuddyProviderWithStubs, useBuddyWithDraft } from "./buddyTestHarness"; import { openAiBuddy } from "../../../../src/features/buddy/aiBuddyBus"; import { http, HttpResponse } from "msw"; import { server } from "../../setup/vitest.setup"; @@ -52,7 +51,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); expect(result.current.isOpen).toBe(false); await waitFor(() => { @@ -70,7 +69,7 @@ describe("useBuddy", () => { ), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -100,7 +99,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -156,7 +155,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.setDraft("what should I work on?"); @@ -179,7 +178,7 @@ describe("useBuddy", () => { it("opens a closed dock and seeds the composer", async () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); expect(result.current.isOpen).toBe(false); act(() => { @@ -201,7 +200,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { openAiBuddy({ draft: "> The migration runs on deploy.\n\n" }); @@ -222,7 +221,7 @@ describe("useBuddy", () => { it("keeps a closed dock's saved draft and puts the seed under it", async () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.setDraft("why does the deploy"); }); @@ -239,7 +238,7 @@ describe("useBuddy", () => { it("keeps an open dock's draft the same way", async () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); result.current.setDraft("why does the deploy"); @@ -257,7 +256,7 @@ describe("useBuddy", () => { it("does not stack the same seed twice", async () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { openAiBuddy({ draft: QUOTE }); @@ -274,7 +273,7 @@ describe("useBuddy", () => { it("toggles open state", () => { server.use(http.get("/api/v1/onboarding/me/buddy/messages", () => HttpResponse.json([]))); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -313,7 +312,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -394,7 +393,7 @@ describe("useBuddy", () => { }), ); - const hook = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const hook = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { hook.result.current.toggleOpen(); }); diff --git a/tests/unit/features/buddy/useHandedOffDraft.test.tsx b/tests/unit/features/buddy/useHandedOffDraft.test.tsx deleted file mode 100644 index 7b92f681..00000000 --- a/tests/unit/features/buddy/useHandedOffDraft.test.tsx +++ /dev/null @@ -1,200 +0,0 @@ -import { render, screen, waitFor } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { describe, it, expect, vi } from "vitest"; -import { useState } from "react"; -import type { ReactNode } from "react"; -import { MemoryRouter, Route, Routes, useNavigate } from "react-router-dom"; -import { useHandedOffDraft } from "../../../../src/features/buddy/useHandedOffDraft"; -import { - BuddyDraftActionsContext, - BuddyDraftContext, -} from "../../../../src/features/buddy/buddyDraftContext"; - -vi.mock("../../../../src/context/useAuth", () => ({ - useAuth: () => ({ - profile: { id: "u1", firstName: "Test", lastName: "User", profileIcon: null }, - }), -})); - -/** A page whose composer is seeded by whatever the panel handed over. */ -function Destination() { - const [draft, setDraft] = useState("already here"); - useHandedOffDraft(setDraft); - return {draft}; -} - -/** Stands in for the panel's "open full" control. */ -function Origin({ draft }: { draft?: string }) { - const navigate = useNavigate(); - return ( - - ); -} - -function renderHandoff(draft?: string) { - return render( - - - } /> - } /> - - , - ); -} - -describe("useHandedOffDraft", () => { - /** - * The panel and the page share a session but not a composer. A hand-off that dropped the draft - * would silently throw away what somebody was part-way through typing — worse than not - * offering the control. - */ - it("carries a part-typed question across to the page", async () => { - const user = userEvent.setup(); - renderHandoff("how do I run the migrations"); - - await user.click(screen.getByRole("button", { name: "open full" })); - - await waitFor(() => { - expect(screen.getByTestId("draft")).toHaveTextContent("how do I run the migrations"); - }); - }); - - /** - * Writing an empty string over whatever the page already had would be a regression of its own — - * there is simply nothing to carry. - */ - it("leaves the composer alone when nothing was typed", async () => { - const user = userEvent.setup(); - renderHandoff(" "); - - await user.click(screen.getByRole("button", { name: "open full" })); - - await waitFor(() => { - expect(screen.getByTestId("draft")).toHaveTextContent("already here"); - }); - }); - - it("does nothing when the page is reached without a hand-off", () => { - render( - - - } /> - - , - ); - - expect(screen.getByTestId("draft")).toHaveTextContent("already here"); - }); - - /** - * History state outlives the navigation. Without clearing it, a reload or a back-and-forward - * would re-seed a draft the hire has since sent or deleted, overwriting the box. - */ - it("consumes the draft so it cannot be applied twice", async () => { - const user = userEvent.setup(); - const seen: string[] = []; - - function Recording() { - const [draft, setDraft] = useState(""); - useHandedOffDraft((value) => { - seen.push(value); - setDraft(value); - }); - return {draft}; - } - - render( - - - } /> - } /> - - , - ); - - await user.click(screen.getByRole("button", { name: "open full" })); - await waitFor(() => { - expect(screen.getByTestId("draft")).toHaveTextContent("once"); - }); - - expect(seen).toEqual(["once"]); - }); -}); - -describe("the dock’s hand-off control", () => { - /** - * The dock takes its words from the shared composer now (`BuddyDraftProvider`): its chips fill - * the box through the write-only half, and the composer inside reads the value. Standing the - * two contexts in for the provider is the only way to render it — and `draft` is the box's - * contents, which is what the hand-off is about. - */ - function withDraft(node: ReactNode, draft = "") { - return ( - - - {node} - - - ); - } - - it("is absent on the page it would open", async () => { - // Guarded in BuddyWidget rather than here; this documents the intent that a control - // offering the page you are already reading is not offered at all. - const { BuddyDock } = await import("../../../../src/features/buddy/components/BuddyDock"); - - render( - withDraft( - , - ), - ); - - expect(screen.queryByLabelText("Open the full buddy page")).not.toBeInTheDocument(); - }); - - it("is offered when there is somewhere to go", async () => { - const { BuddyDock } = await import("../../../../src/features/buddy/components/BuddyDock"); - const onOpenFull = vi.fn(); - const user = userEvent.setup(); - - render( - withDraft( - , - "half a question", - ), - ); - - await user.click(screen.getByLabelText("Open the full buddy page")); - - expect(onOpenFull).toHaveBeenCalled(); - }); -}); From a5c17656e2a01b4d186bbd36f1bc4fedd7cd6e64 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 11:25:19 +0200 Subject: [PATCH 05/10] refactor(buddy): dismiss with the action itself, not a lookup by id `dismissAction` took a message id and an action id and searched the transcript for the action object, because only that object says whether the dismissal has a stored proposal to decline at the backend. That lookup is what forced the `messagesRef` next to it: reading `messages` state directly would have rebuilt the callback on every token, and every memoised row would then have re-rendered for a token that belongs to one of them. `confirmAction` has taken the action object all along, handed over by the card that drew it. `dismissAction` does now too: `BuddyActionProposals` passes what it rendered, the lookup goes away, and the ref and its effect go with it. The callback no longer depends on anything a token can change, and the question of whether a ref like that could be a render behind a click does not arise. The two assertions in the card's tests and the four calls in the team-mode tests follow the new shape. --- .../buddy/components/BuddyActionProposals.tsx | 6 ++-- .../buddy/components/BuddyConversation.tsx | 2 +- src/features/buddy/components/BuddyThread.tsx | 4 +-- .../buddy/hooks/useBuddyConversation.ts | 30 +++++++------------ .../buddy/BuddyActionProposals.test.tsx | 12 +++++--- .../features/buddy/useBuddyTeamMode.test.tsx | 8 ++--- 6 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/features/buddy/components/BuddyActionProposals.tsx b/src/features/buddy/components/BuddyActionProposals.tsx index 9048816b..82b761c5 100644 --- a/src/features/buddy/components/BuddyActionProposals.tsx +++ b/src/features/buddy/components/BuddyActionProposals.tsx @@ -13,7 +13,7 @@ type BuddyActionProposalsProps = { messageId: string; actions: ProposedAction[]; onConfirm: (messageId: string, action: ProposedAction) => void; - onDismiss: (messageId: string, actionId: string) => void; + onDismiss: (messageId: string, action: ProposedAction) => void; }; /** @@ -207,7 +207,7 @@ export function BuddyActionProposals({
; + +describe("the opener suggestion's place in the thread", () => { + it("hangs under the newest reply, not under an older one", () => { + render( + , + ); + + const footer = screen.getByRole("button", { name: "Start with the packet" }); + const newest = screen.getByText(REPLY); + + expect(newest.compareDocumentPosition(footer) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it("steps aside for a reply that brings its own next step", () => { + render( + , + ); + + // The reply's own offer is there… + expect(screen.getByRole("button", { name: /Start Task 0/ })).toBeInTheDocument(); + + // …and the opener's is not, anywhere: not doubled up under that reply, and not pushed back + // onto the greeting above it either. + expect(screen.queryByRole("button", { name: "Start with the packet" })).not.toBeInTheDocument(); + }); +}); From ff2e11618e02370fcfe08a7b4754ba54dd059e72 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 13:19:32 +0200 Subject: [PATCH 08/10] perf(buddy): memoise the conversation, and hold its element props in one identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thread, every row and the dock were memoised in the earlier passes; the conversation wrapping them was not. So a page re-render the conversation has no interest in — the rail opening, a toast landing, the visit divider moving — still walked the whole column, and the memo inside absorbed the cost instead of the work never starting. It is `memo` now, with the props list written down as the contract it actually is: plain values, motion values, callbacks the page holds in one identity, and elements it builds once. That last part needed a real fix rather than a comment: `aboveComposer` was a JSX block built inline in the page's render, which would have made it the one prop the memo always found changed — silently, and only in the profiler. It is a `useMemo` above the return now, and the chips' reasoning moved with it, to where the element is built instead of sitting in an attribute list. --- .../buddy/components/BuddyConversation.tsx | 16 ++++++- src/pages/BuddyPage.tsx | 43 ++++++++++++------- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/features/buddy/components/BuddyConversation.tsx b/src/features/buddy/components/BuddyConversation.tsx index 3f2f9054..b1c39fa5 100644 --- a/src/features/buddy/components/BuddyConversation.tsx +++ b/src/features/buddy/components/BuddyConversation.tsx @@ -1,4 +1,4 @@ -import { useCallback } from "react"; +import { memo, useCallback } from "react"; import type { ReactNode } from "react"; import type { BuddyMessageView, ProposedAction } from "../types"; import { BuddyComposer } from "./BuddyComposer"; @@ -83,7 +83,7 @@ type BuddyConversationProps = { * It scrolls down, never sideways — `overflow-x-hidden` plus the `min-w-0` chain running down * to `BuddyMarkdown`, where wide blocks get their own scrollers. */ -export function BuddyConversation({ +function BuddyConversationImpl({ messages, isThinking, activeTool, @@ -199,3 +199,15 @@ export function BuddyConversation({ ); } + +/** + * Memoised, and its props are the contract: everything in that list is a plain value, an element + * or callback the page holds in one identity (see the `useMemo`/`useCallback`s above its render), + * or a motion value. A fresh inline element added to it later — a `footer={`…`}` built in the + * page's render — is silently the one prop that always changed, and the memo stops paying. + * + * The thread *inside* this carries the per-message boundary; this one is about the page's own + * re-renders (the rail opening, a toast landing, a visit divider moving) not walking the whole + * conversation. + */ +export const BuddyConversation = memo(BuddyConversationImpl); diff --git a/src/pages/BuddyPage.tsx b/src/pages/BuddyPage.tsx index 8373e23f..08c48a62 100644 --- a/src/pages/BuddyPage.tsx +++ b/src/pages/BuddyPage.tsx @@ -336,6 +336,30 @@ function BuddyMentorHome() { [hasUserMessage, openerAction, greeting.isRevealing, sendMessage], ); + /** + * The suggestion row above the composer, held in one identity — the conversation below it is + * memoised now, and an element built inline in the render would be the one prop that always + * changed. + * + * The chips *fill* the composer instead of sending, which is why they sit on top of it. The + * hire presses send: the words stay theirs, and they can edit the question first — which is how + * somebody learns they are allowed to. The list is the backend's, built from the tools it + * actually mounts for this hire, so the chips and the mentor cannot disagree about whether this + * role has pull requests. Hire-only, matching the fetch gate: a team-mode conversation would + * otherwise show the heading over an empty list, since there is nothing team-scoped to load. + */ + const aboveComposer = useMemo( + () => + isHireMode && !hasUserMessage ? ( + + ) : undefined, + [isHireMode, hasUserMessage, suggestions, setDraft], + ); + // The keyboard half of the control in the visit divider. Gated the same way that control is: // a visit nobody has spoken in is already the fresh one, and re-opening it would only replay // the greeting — and, like the chat's, only while this is the half on screen, since the shell @@ -430,22 +454,9 @@ function BuddyMentorHome() { // the transcript is shorter than the viewport, which is exactly the first few turns this // control exists for. hasFloatingControl={(isHireMode && replies.hasAny && !rail.open) || hasUserMessage} - aboveComposer={ - // The chips *fill* the composer instead of sending, which is why they sit on top of - // it. The hire presses send: the words stay theirs, and they can edit the question - // first — which is how somebody learns they are allowed to. The list is the - // backend's, built from the tools it actually mounts for this hire, so the chips and - // the mentor cannot disagree about whether this role has pull requests. - // Hire-only, matching the fetch gate above: a team-mode conversation would otherwise - // show the heading over an empty list, since there is nothing team-scoped to load. - isHireMode && !hasUserMessage ? ( - - ) : undefined - } + // Built above, in one identity — the conversation is memoised, and the chips' own reasons + // are written where they are built. + aboveComposer={aboveComposer} focusComposerOnMount /> From 5655e86eac6f5954dd41bb8e96691bca61979d89 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 13:20:15 +0200 Subject: [PATCH 09/10] docs(buddy): point the render-time state pattern at one naming of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Adjust state during render" appears in eight files, each comment explaining it a little differently — enough that a reviewer read the two this PR touches as independently invented conventions rather than one pattern used twice. It is one pattern, with three rules, and they are now named once in `CODING_STANDARDS.md` § 3 (workspace root, deliberately unversioned): only the component's own state, only behind an inequality check, and never a ref — a ref write during render belongs in an effect, which `ChatProvider` also notes. The two comments here keep their own local why — what is being corrected, and what an effect would paint in the meantime — and point at the standard for the how. --- src/App.tsx | 1 + src/features/buddy/BuddyDraftProvider.tsx | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 03b2f11f..c66affee 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -55,6 +55,7 @@ function AppContent() { // happened to be out does not open with it already there, waiting for a mouse that never went // near it to leave. Reset during render rather than in an effect: it is a correction to state // that is already wrong for this render, not a synchronisation with anything outside React. + // (The pattern and its three rules are named once in `CODING_STANDARDS.md` § 3.) const [peekMode, setPeekMode] = useState(isFocused); if (peekMode !== isFocused) { setPeekMode(isFocused); diff --git a/src/features/buddy/BuddyDraftProvider.tsx b/src/features/buddy/BuddyDraftProvider.tsx index 9b638753..83615c0d 100644 --- a/src/features/buddy/BuddyDraftProvider.tsx +++ b/src/features/buddy/BuddyDraftProvider.tsx @@ -34,9 +34,10 @@ export function BuddyDraftProvider({ children }: { children: ReactNode }) { * * Told, not pulled: the session sits *above* this provider and cannot reach into its state, so * it bumps a token and the clearing happens on the way through. React's documented "adjust - * state when a prop changes" pattern — the same shape `BuddyPage` uses for its rail — rather - * than an effect, because an effect here would paint one frame of a draft belonging to a - * conversation that no longer exists, and cost a second render to fix it. + * state when a prop changes" pattern — the same shape `BuddyPage` uses for its rail, and named + * once with its rules in `CODING_STANDARDS.md` § 3 — rather than an effect, because an effect + * here would paint one frame of a draft belonging to a conversation that no longer exists, and + * cost a second render to fix it. */ const [seenResetToken, setSeenResetToken] = useState(draftResetToken); if (seenResetToken !== draftResetToken) { From 9cae8c6af404ebbbbb8c4f79c42e95250b24170c Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Mon, 28 Sep 2026 13:28:07 +0200 Subject: [PATCH 10/10] test(buddy): port the board-sync tests to the split draft API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #271 landed on `dev` while this branch was open, and the tests it added — the board synchronisation suite at the end of `useBuddy.test.tsx` — were written against `useBuddy()` still owning the draft: `result.current.setDraft(...)` and `handleSubmit(...)` taken off the same hook the always-mounted widget reads. That coupling is exactly what this branch removes, so the merge resolved the source files cleanly and left three render sites under the new blocks calling a hook that no longer carries the composer. CI runs the PR against the merge with `dev`, which is why the push run was green and the pull-request run was not. They go through the harness's `useBuddyWithDraft()` like every other sending test in the file — same providers (`Wrapper` is `QueryClientProvider` wrapping `BuddyProviderWithStubs`, draft provider included), same assertions, same turns. `tsc -b` and the type-aware eslint run recover, and the merged buddy + board suites are 63 files / 600 tests. Nothing else from #271 needed touching: the salvaged `useBuddyConversation` merged with the `dismissAction` change without a conflict, and `messagesRef` stays gone. --- tests/unit/features/buddy/useBuddy.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/features/buddy/useBuddy.test.tsx b/tests/unit/features/buddy/useBuddy.test.tsx index d019b181..8a200655 100644 --- a/tests/unit/features/buddy/useBuddy.test.tsx +++ b/tests/unit/features/buddy/useBuddy.test.tsx @@ -513,7 +513,7 @@ describe("useBuddy", () => { http.post("/api/v1/onboarding/me/buddy/actions", () => HttpResponse.json(outcome)), ); - const { result } = renderHook(() => useBuddy(), { wrapper: Wrapper }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: Wrapper }); act(() => { result.current.toggleOpen(); }); @@ -613,7 +613,7 @@ describe("useBuddy", () => { http.post("/api/v1/onboarding/me/buddy/messages", () => stream(answers[answered++])), ); - const { result } = renderHook(() => useBuddy(), { wrapper: Wrapper }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: Wrapper }); act(() => { result.current.toggleOpen(); }); @@ -623,7 +623,7 @@ describe("useBuddy", () => { } /** Sends one question and waits for its turn to end. */ - async function ask(result: { current: ReturnType }, text: string) { + async function ask(result: { current: ReturnType }, text: string) { act(() => { result.current.setDraft(text); });