diff --git a/src/App.tsx b/src/App.tsx index 03b2f11f2..c66affee8 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 new file mode 100644 index 000000000..83615c0d2 --- /dev/null +++ b/src/features/buddy/BuddyDraftProvider.tsx @@ -0,0 +1,94 @@ +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, 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) { + 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 f5edb9d7d..4f1a4bdfb 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 000000000..f519e14af --- /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/BuddyActionProposals.tsx b/src/features/buddy/components/BuddyActionProposals.tsx index 9048816b2..82b761c53 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({
); } + +/** + * 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/BuddyMarkdown.tsx b/src/features/buddy/components/BuddyMarkdown.tsx index 7f567da83..ca7256c01 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 580281dec..fdc93e36e 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"; @@ -22,13 +22,11 @@ type BuddyThreadProps = { /** Confirms a buddy-proposed action (the only path that mutates). */ confirmAction: (messageId: string, action: ProposedAction) => void; /** Declines a proposed action; nothing changes. */ - dismissAction: (messageId: string, actionId: string) => void; + dismissAction: (messageId: string, action: ProposedAction) => void; /** Names above the bubbles — on for the page, off in the dock. */ showNames?: boolean; /** The dock's narrow layout — see `BuddyMessage`'s `compact`. */ compact?: boolean; - /** Rendered above the first message: what came back from the hire's PM. */ - before?: ReactNode; /** * Rendered under the buddy's most recent reply — the greeting's suggested next step. */ @@ -40,6 +38,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 +49,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 +93,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, action: ProposedAction) => 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 +231,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, @@ -110,7 +246,6 @@ export function BuddyThread({ dismissAction, showNames = false, compact = false, - before, lastMessageFooter, renderQuestionAction, renderReplyAction, @@ -122,19 +257,29 @@ export function BuddyThread({ 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 (
- {before} - {/* Above the thread rather than in it: what failed is the whole conversation, so there is nothing below for it to belong to -- and on a first visit there is nothing below at all. `alert`, because it arrives without the hire doing anything. */} @@ -153,86 +298,38 @@ 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/src/features/buddy/components/BuddyWidget.tsx b/src/features/buddy/components/BuddyWidget.tsx index bd794db83..0d6fd6e9f 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"; @@ -66,9 +66,6 @@ export function BuddyWidget() { markGreetingPresented, isOpen, toggleOpen, - draft, - setDraft, - handleSubmit, confirmAction, dismissAction, suggestions, @@ -188,10 +185,61 @@ 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. + * + * 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), []); + + /** + * 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. @@ -294,25 +342,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,21 +357,16 @@ 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`). - 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 76d450058..69ea40827 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 { useBuddyDraftActions } from "../buddyDraftContext"; import { useBuddySession } from "../buddySessionContext"; import { useBuddySuggestions } from "./useBuddySuggestions"; @@ -15,10 +16,17 @@ 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(); - const { ensureOpened, setDraft, teamProjectId } = conversation; + const { setDraft } = useBuddyDraftActions(); + const { ensureOpened, teamProjectId } = conversation; const [isOpen, setIsOpen] = useState(false); diff --git a/src/features/buddy/hooks/useBuddyConversation.ts b/src/features/buddy/hooks/useBuddyConversation.ts index 2b9c8953a..7e92b5fab 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, @@ -176,7 +174,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 @@ -522,7 +528,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(); @@ -841,19 +849,23 @@ export function useBuddyConversation( /** * Declines a proposed action — nothing changes; the conversation simply continues. * + * The action itself arrives from the card that drew it, the way `confirmAction`'s does. It used + * to arrive as an id and be looked up in the transcript, which is what forced a + * `messagesRef` to keep this callback's identity stable — the callback now depends on nothing + * that a token can change, and the lookup (and its staleness question) is gone with it. + * * A stored proposal is declined *at the backend* rather than only on screen, because it may * also be sitting in another tab waiting to be confirmed: dismissal closes that door too, and * only the backend can. A hire offer was never stored, so there is nothing to tell the * backend — declining is purely local, as it has always been. */ const dismissAction = useCallback( - (messageId: string, actionId: string) => { - const action = messages - .find((m) => m.id === messageId) - ?.actions?.find((a) => a.id === actionId); + (messageId: string, action: ProposedAction) => { + const actionId = action.id; - // Unknown action: nothing to decline at the backend, but still worth putting away here. - if (!action || !("proposalId" in action)) { + // A hire offer, or one that arrived without its details: nothing to decline at the backend, + // but still worth putting away here. + if (!("proposalId" in action)) { patchAction(messageId, actionId, { status: "dismissed" }); return; } @@ -900,7 +912,7 @@ export function useBuddyConversation( } })(); }, - [beginDecision, endDecision, messages, patchAction], + [beginDecision, endDecision, patchAction], ); /** @@ -933,7 +945,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); @@ -994,35 +1007,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, @@ -1043,10 +1027,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/features/buddy/useHandedOffDraft.ts b/src/features/buddy/useHandedOffDraft.ts deleted file mode 100644 index ca65c5404..000000000 --- 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 62ffb11f5..08c48a623 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,11 +12,11 @@ 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"; import { useGreetingReveal } from "../features/buddy/hooks/useGreetingReveal"; -import { useHandedOffDraft } from "../features/buddy/useHandedOffDraft"; import { announceBuddyPageReady } from "../features/buddy/aiBuddyBus"; import { NEW_CONVERSATION_CHORD, @@ -168,10 +168,7 @@ function BuddyMentorHome() { isOpening, activeTool, openerAction, - draft, - setDraft, sendMessage, - handleSubmit, confirmAction, dismissAction, openError, @@ -189,6 +186,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]); @@ -263,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"); /** @@ -302,6 +302,64 @@ 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 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 @@ -374,38 +432,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} @@ -416,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 /> diff --git a/tests/unit/features/buddy/BuddyActionProposals.test.tsx b/tests/unit/features/buddy/BuddyActionProposals.test.tsx index fe66cdb05..19fa17ec8 100644 --- a/tests/unit/features/buddy/BuddyActionProposals.test.tsx +++ b/tests/unit/features/buddy/BuddyActionProposals.test.tsx @@ -53,7 +53,7 @@ describe("BuddyActionProposals", () => { await userEvent.click(screen.getByRole("button", { name: /Not now/ })); - expect(onDismiss).toHaveBeenCalledWith("m1", "a1"); + expect(onDismiss).toHaveBeenCalledWith("m1", expect.objectContaining({ id: "a1" })); expect(onConfirm).not.toHaveBeenCalled(); }); @@ -489,7 +489,7 @@ describe("BuddyActionProposals", () => { ); // Declining is still available: a proposal you cannot verify still needs an answer. await userEvent.click(screen.getByRole("button", { name: /Not now/ })); - expect(onDismiss).toHaveBeenCalledWith("m1", "s1"); + expect(onDismiss).toHaveBeenCalledWith("m1", expect.objectContaining({ id: "s1" })); expect(onConfirm).not.toHaveBeenCalled(); }); @@ -571,8 +571,12 @@ describe("BuddyActionProposals", () => { await userEvent.click(screen.getByRole("button", { name: /Not now/ })); - expect(onDismiss).toHaveBeenCalledWith("m1", "s1"); - expect(onDismiss.mock.calls[0][1]).not.toBe("prop-9"); + // The card hands over the whole action, as it does for a confirm — nothing here names the + // stored proposal; the session reads `proposalId` off the object and declines it there. + expect(onDismiss).toHaveBeenCalledWith( + "m1", + expect.objectContaining({ proposalId: "prop-9" }), + ); }); it("hides the buttons once the outcome is known, whatever it was", () => { diff --git a/tests/unit/features/buddy/BuddyComposerCaret.test.tsx b/tests/unit/features/buddy/BuddyComposerCaret.test.tsx index d475846e0..a0ea7695d 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/BuddyComposerIsolation.test.tsx b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx new file mode 100644 index 000000000..fc7eefb7e --- /dev/null +++ b/tests/unit/features/buddy/BuddyComposerIsolation.test.tsx @@ -0,0 +1,222 @@ +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"; + +/** + * 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([]), +})); + +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 + ? { 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; + +/** 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( + + + + + + + , + ); +} + +describe("the composer in a long conversation", () => { + beforeEach(stubLongConversation); + + 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/); + }); +}); + +/** + * 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/BuddyConversation.test.tsx b/tests/unit/features/buddy/BuddyConversation.test.tsx index dae0d0b81..1a98be3ff 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 6f60a11cb..63d10ddd9 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/BuddyThreadFooter.test.tsx b/tests/unit/features/buddy/BuddyThreadFooter.test.tsx new file mode 100644 index 000000000..c8cde3e76 --- /dev/null +++ b/tests/unit/features/buddy/BuddyThreadFooter.test.tsx @@ -0,0 +1,96 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { ReactNode } from "react"; +import { BuddyThread } from "../../../../src/features/buddy/components/BuddyThread"; +import type { BuddyMessageView } from "../../../../src/features/buddy/types"; + +/** + * Where the greeting's suggested next step lands, and where it steps aside. + * + * `lastMessageFooter` is the opener action's button — shown only while nobody has spoken — so it + * belongs under the newest reply rather than an older one. Two rules decide where it goes, and + * both are easy to lose in a refactor of the row selection: it follows the *newest* reply even + * when that reply has no prose (a proposal-only turn is still a turn), and it yields to a reply + * that brings a next step of its own, because two offers under one bubble read as two competing + * ones. + * + * The overlap the second rule guards cannot happen today — `streamOpenBuddy` has no + * `action_proposal` case and never writes `actions` onto the greeting — which is exactly why it + * is worth a test rather than only a comment: the day a greeting can carry a proposal, this file + * fails instead of the page quietly showing both. + * + * The bubble is stood in for (its avatar wants an auth provider, and its drawing has its own + * tests); what this file reads is the order of what lands inside it. + */ +vi.mock("../../../../src/features/buddy/components/BuddyMessage", () => ({ + BuddyMessage: ({ children, footer }: { children?: ReactNode; footer?: ReactNode }) => ( +
+ {children} + {footer} +
+ ), +})); + +const OPENING = "Welcome. Let's get you set up."; +const REPLY = "Step one is the packet."; + +function message(overrides: Partial & { id: string }): BuddyMessageView { + return { + role: "ASSISTANT", + content: "", + createdAt: "2026-08-03T00:00:00Z", + ...overrides, + }; +} + +const BASE_PROPS = { + isThinking: false, + isStreaming: false, + activeTool: null, + confirmAction: vi.fn(), + dismissAction: vi.fn(), +}; + +const SUGGESTED_STEP = ; + +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(); + }); +}); diff --git a/tests/unit/features/buddy/BuddyThreadStreaming.test.tsx b/tests/unit/features/buddy/BuddyThreadStreaming.test.tsx new file mode 100644 index 000000000..738c2904b --- /dev/null +++ b/tests/unit/features/buddy/BuddyThreadStreaming.test.tsx @@ -0,0 +1,86 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { ReactNode } from "react"; +import { BuddyThread } from "../../../../src/features/buddy/components/BuddyThread"; +import type { BuddyMessageView } from "../../../../src/features/buddy/types"; + +/** + * Who counts as "receiving tokens right now" — the flag the thread hands down to its rows. + * + * It drives the bot beside a reply: `BuddyMessage` passes it on as `SleepyBot`'s + * `canSleep={!isStreaming}`, and a bot held awake for its whole life never does the thing it is + * named for. The thread used to answer "is this the newest message?" rather than "is this the live + * turn?" — the same answer only while a turn is actually running — so the newest reply's bot never + * slept. The chat draws the line correctly in its `MessageRow` (`streamingMessageId` is null when + * idle); this is the buddy's version of it. + */ +vi.mock("../../../../src/features/buddy/components/BuddyMessage", () => ({ + // 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"]); + }); +}); diff --git a/tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx b/tests/unit/features/buddy/buddyDraftAcrossRoutes.test.tsx new file mode 100644 index 000000000..7b7b0433c --- /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 27f4fe2a0..60905abb7 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 8a11b8273..ffaab424e 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 55c482abb..8a2006552 100644 --- a/tests/unit/features/buddy/useBuddy.test.tsx +++ b/tests/unit/features/buddy/useBuddy.test.tsx @@ -2,8 +2,7 @@ import { renderHook, act, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { ReactNode } from "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 { delay, http, HttpResponse } from "msw"; import { server } from "../../setup/vitest.setup"; @@ -55,7 +54,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); expect(result.current.isOpen).toBe(false); await waitFor(() => { @@ -73,7 +72,7 @@ describe("useBuddy", () => { ), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -103,7 +102,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -159,7 +158,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.setDraft("what should I work on?"); @@ -182,7 +181,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(() => { @@ -204,7 +203,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" }); @@ -225,7 +224,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"); }); @@ -242,7 +241,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"); @@ -260,7 +259,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 }); @@ -277,7 +276,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(); @@ -316,7 +315,7 @@ describe("useBuddy", () => { }), ); - const { result } = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const { result } = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { result.current.toggleOpen(); @@ -397,7 +396,7 @@ describe("useBuddy", () => { }), ); - const hook = renderHook(() => useBuddy(), { wrapper: BuddyProviderWithStubs }); + const hook = renderHook(() => useBuddyWithDraft(), { wrapper: BuddyProviderWithStubs }); act(() => { hook.result.current.toggleOpen(); }); @@ -514,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(); }); @@ -614,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(); }); @@ -624,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); }); diff --git a/tests/unit/features/buddy/useBuddyTeamMode.test.tsx b/tests/unit/features/buddy/useBuddyTeamMode.test.tsx index 36b43c967..2f571f7f7 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); @@ -481,7 +475,7 @@ describe("useBuddyConversation — team mode", () => { const { result, message, action } = await openTeamWithProposal(); act(() => { - result.current.dismissAction(message.id, action.id); + result.current.dismissAction(message.id, action); }); // "Dismissed — nothing changed" would be a lie over a change that already happened; the @@ -509,7 +503,7 @@ describe("useBuddyConversation — team mode", () => { const { result, message, action } = await openTeamWithProposal(); act(() => { - result.current.dismissAction(message.id, action.id); + result.current.dismissAction(message.id, action); }); await waitFor(() => { expect(result.current.messages.find((m) => m.id === message.id)?.actions?.[0].status).toBe( @@ -519,7 +513,7 @@ describe("useBuddyConversation — team mode", () => { failDismiss = false; act(() => { - result.current.dismissAction(message.id, action.id); + result.current.dismissAction(message.id, action); }); await waitFor(() => { expect(result.current.messages.find((m) => m.id === message.id)?.actions?.[0].status).toBe( @@ -548,7 +542,7 @@ describe("useBuddyConversation — team mode", () => { // exactly the race a disable-on-render alone cannot catch. act(() => { result.current.confirmAction(message.id, action); - result.current.dismissAction(message.id, action.id); + result.current.dismissAction(message.id, action); }); await waitFor(() => { diff --git a/tests/unit/features/buddy/useHandedOffDraft.test.tsx b/tests/unit/features/buddy/useHandedOffDraft.test.tsx deleted file mode 100644 index 59926d63b..000000000 --- a/tests/unit/features/buddy/useHandedOffDraft.test.tsx +++ /dev/null @@ -1,180 +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 { MemoryRouter, Route, Routes, useNavigate } from "react-router-dom"; -import { useHandedOffDraft } from "../../../../src/features/buddy/useHandedOffDraft"; - -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", () => { - 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( - , - ); - - 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( - , - ); - - await user.click(screen.getByLabelText("Open the full buddy page")); - - expect(onOpenFull).toHaveBeenCalled(); - }); -});