diff --git a/.oxlintrc.json b/.oxlintrc.json index c08fe72..522155d 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -24,7 +24,7 @@ "basalt/raw-size-literal": "error", "basalt/card-inset": "error", "basalt/chart-in-raw-surface": "error", - "basalt/raw-scroll-container": "warn", + "basalt/raw-scroll-container": "error", "basalt/visx-boundary": "error", "basalt/visx-tooltip": "error", "basalt/token-layer-boundary": "error", diff --git a/apps/playground/src/demo/AgentAnchorToEndDemoPage.tsx b/apps/playground/src/demo/AgentAnchorToEndDemoPage.tsx new file mode 100644 index 0000000..a78ddcb --- /dev/null +++ b/apps/playground/src/demo/AgentAnchorToEndDemoPage.tsx @@ -0,0 +1,271 @@ +/** + * AgentAnchorToEndDemoPage — basalt-ui 1.13.0 playground gate demo: the ONE combination no other + * demo in this repo puts together — a virtualized `ThreadTranscript` with a genuinely STREAMING + * turn appending to its tail, long enough that the tail sits well below the fold. + * + * `anchorTo: 'end'` + `followOnAppend` (`thread-message.tsx`'s `VirtualizedRowsInner`) exist + * precisely to keep a virtualized transcript pinned to the bottom while new content streams in, and + * until this page nothing exercised both halves at once: `AgentTranscriptVirtualizeDemoPage` is 500 + * STATIC messages (nothing ever streams into it), `AgentInlineFeedVirtualizedRowDemoPage` is a + * virtualized row that never receives `liveParts`. Neither can show whether the pinned-to-bottom + * behavior actually holds up against a live append. + * + * The scroll threshold that governs this (`scrollEndThreshold`, hardcoded to 64px inside + * `thread-message.tsx` — NOT a public prop; `VirtualizeOptions` exposes `overscan`/`estimateSize`/ + * `initialScroll`, not the threshold itself) is not asserted here, only made LEGIBLE. This page reads + * the real DOM the same way a consumer's own instrumentation would have to — there is no exposed + * ref/prop onto the internal virtualizer's scroll node — via a capture-phase `scroll` listener on a + * wrapper div whose only child is `ThreadTranscript`'s own root (the virtualized branch renders + * exactly one scrollable element as its root, both for the real virtualizer and its `Suspense` + * fallback, so `wrapper.firstElementChild` is unambiguous across that swap). The distance readout + * below the transcript updates live from whichever scroll actually happened, whether the user drove + * it or the virtualizer's own auto-follow did. + * + * 1.13.0: `VirtualizeOptions.initialScroll` now defaults to `'end'` — this page doesn't override it, + * so it now opens ALREADY pinned to the seeded tail (badge reads "pinned", distance ~0px) instead of + * mounting at message #0, 8,913px away. That flips which half of the demo is interesting: scrolling + * DOWN to the tail is no longer the setup step, scrolling UP away from it is. + * + * Drive it: scroll up first (the badge flips to "held", the distance count climbs and stops moving — + * you're now off the tail). Click "Start streaming turn": the stream writes new content below the + * fold and your held position does NOT move, proving `followOnAppend` really did stop re-anchoring + * once you left the threshold, not just once you left the bottom of a static list. Scroll back down + * past ~64px from the bottom and following resumes on its own — badge flips to "pinned", the distance + * count tracks each new chunk back down near zero as it streams in. + * + * Note (expected, not a bug): the distance readout is driven entirely by real `scroll` events — see + * the effect below — so while you're held away from the bottom it FREEZES rather than climbing as + * the stream keeps appending content further below the fold; nothing you can see moved, so nothing + * fires a `scroll` event to react to. It catches up the instant you scroll again. This is the + * tradeoff of reading the real DOM the way a consumer's own instrumentation would have to, and it is + * deliberately NOT "fixed" with a timer or a live resize watch — that would defeat the point of + * observing exactly what a real `scroll` listener sees, no more. + */ +import { Badge, Box, Button, Group, Paper, Stack, Text, Title } from '@mantine/core' +import { EmptyState } from 'basalt-ui' +import { createThreadsStore, heuristicOutcome, useAgentThreadRuns } from 'basalt-ui/agent' +import type { AgentPart, AgentThread, AgentTransport } from 'basalt-ui/agent' +import { ThreadTranscript } from 'basalt-ui/agent-chat' +import { useCallback, useEffect, useRef, useState } from 'react' +import { buildLongThread } from './agent-long-thread' +import { IconReset, IconSparkle } from './icons' + +// ── Scripted, slow, word-by-word stream — long enough to scroll around mid-flight ────────────── +// Same "no signal handling" idiom as AgentStopMidStreamDemoPage's script: this demo never stops the +// turn early, so there is nothing for an abort to interrupt. + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +const ANCHOR_STEP_DELAY = 220 + +const ANCHOR_DEMO_ANSWER = + 'Walking through the anchor-to-end mechanics end to end: the virtualizer measures the scroll ' + + 'container on every append, and when the pane was already within scrollEndThreshold of the bottom ' + + 'before that append, it re-anchors to the new bottom on the very same frame. Scroll away from the ' + + 'bottom and that re-anchor stops firing entirely — your position holds even while dozens more rows ' + + 'keep arriving underneath it. Scroll back down past the threshold and following resumes ' + + 'immediately, with no extra click required on your part. This is exactly the rhythm a real chat ' + + 'client needs: stay pinned while someone is reading the latest turn as it writes itself, but never ' + + 'yank the viewport out from under someone who scrolled up to re-read an earlier message while the ' + + 'answer keeps streaming in below.' +const ANCHOR_DEMO_WORDS = ANCHOR_DEMO_ANSWER.split(' ') + +async function* anchorDemoScript(): AsyncGenerator { + const id = crypto.randomUUID() + for (const [index, word] of ANCHOR_DEMO_WORDS.entries()) { + await sleep(ANCHOR_STEP_DELAY) + yield { id, type: 'text', text: index === 0 ? word : ` ${word}` } + } +} + +const anchorDemoTransport: AgentTransport = { + stream: () => anchorDemoScript(), +} + +// ── Store + seed ───────────────────────────────────────────────────────────────── + +const useAnchorThreads = createThreadsStore({ key: 'playground-agent-anchor-to-end', version: 1 }) + +const SEED_MESSAGE_COUNT = 60 +const TRANSCRIPT_HEIGHT = 420 +// Mirrors thread-message.tsx's internal DEFAULT_VIRTUALIZE_SCROLL_END_THRESHOLD — not importable +// (not part of VirtualizeOptions, so not part of the public surface at all); reproduced here purely +// as a legend for this page's own readout, not asserted against anything. +const SCROLL_END_THRESHOLD = 64 + +// ── Page ────────────────────────────────────────────────────────────────────── + +export function AgentAnchorToEndDemoPage() { + const store = useAnchorThreads() + const { runs, start } = useAgentThreadRuns({ + transport: anchorDemoTransport, + store, + resolveOutcome: heuristicOutcome, + }) + + // Seeds ONE already-settled, deliberately long thread straight into the store — same + // no-start()-in-a-mount-effect discipline as AgentThreadFeedInlineDemoPage's fix 2 (see its own + // doc): buildLongThread's messages are synchronous, so nothing here ever registers a controller + // for a StrictMode double-invoke to abort. + const seededRef = useRef(false) + useEffect(() => { + if (seededRef.current || store.threads.length > 0) return + seededRef.current = true + const id = store.create() + const messages = buildLongThread(SEED_MESSAGE_COUNT) + for (const message of messages) store.appendMessage(id, message) + store.setStatus(id, 'done') + const firstMessage = messages[0] + const lastMessage = messages[messages.length - 1] + const snapshot: AgentThread = { + id, + messages, + outcome: null, + status: 'done', + read: true, + createdAt: firstMessage?.createdAt ?? Date.now(), + updatedAt: lastMessage?.createdAt ?? Date.now(), + } + store.setOutcome(id, heuristicOutcome(snapshot)) + }, [store]) + + const thread = store.threads[0] + const run = thread !== undefined ? runs.get(thread.id) : undefined + const streaming = run !== undefined + + const wrapperRef = useRef(null) + const [distanceFromBottom, setDistanceFromBottom] = useState(null) + + // Capture-phase, not bubble: `scroll` events don't bubble in most browsers, but capture always + // sees them regardless of which descendant actually scrolled. Filtering to `wrapper.firstElementChild` + // (re-read fresh on every event, never cached) is what keeps this from ever matching a nested + // scroller instead — a fenced code block in one of the seeded messages sets its own overflow, and + // that element is a grandchild, never the wrapper's direct child. + useEffect(() => { + const wrapper = wrapperRef.current + if (wrapper === null) return + const measure = (target: HTMLElement) => { + const distance = target.scrollHeight - target.scrollTop - target.clientHeight + setDistanceFromBottom(Math.max(0, Math.round(distance))) + } + const handleScroll = (event: Event) => { + const target = event.target + if (!(target instanceof HTMLElement) || target !== wrapper.firstElementChild) return + measure(target) + } + wrapper.addEventListener('scroll', handleScroll, { capture: true, passive: true }) + + // The one-shot rAF this replaced measured too early: `ThreadTranscript`'s virtualized branch + // sits behind a lazy `import('@tanstack/react-virtual')` (thread-message.tsx), so the very first + // commit is the `Suspense` fallback — an EMPTY scroll node whose scrollHeight equals its own + // clientHeight. A single rAF after mount still often landed inside that fallback window, reading + // `scrollHeight - scrollTop - clientHeight` as exactly 0 no matter how far the real seeded + // content sits below the fold — precisely the false "pinned" reading this page exists to avoid + // showing a human. Instead of guessing a frame count, a `MutationObserver` on the wrapper catches + // the EXACT moment the real virtualizer replaces the fallback — its sizer element (the + // `height: getTotalSize()` box) appearing as the scroll node's one child — and takes ONE read + // right then, same "one extra initial read" intent as the rAF it replaces, just correctly timed. + // Deliberately NOT kept alive past that: the readout goes back to being purely `scroll`-event- + // driven afterward (see the module doc's "Note" on this) — a stream that grows the pane's total + // height while the user is scrolled away from the bottom does NOT move this number until the next + // real scroll. That is a real, expected limitation of reading the DOM the way a consumer's own + // instrumentation would have to, not something to paper over with a live ResizeObserver here. + const observer = new MutationObserver(() => { + const scrollNode = wrapper.firstElementChild + const sizer = scrollNode?.firstElementChild + if (!(scrollNode instanceof HTMLElement) || !(sizer instanceof HTMLElement)) return + measure(scrollNode) + observer.disconnect() + }) + observer.observe(wrapper, { childList: true, subtree: true }) + // Covers the case where `@tanstack/react-virtual`'s lazy import already resolved during an + // earlier mount THIS session (`LazyVirtualizedRows` caches its resolution permanently for the + // module's lifetime) — the sizer can already be present synchronously, with no mutation left to + // observe. + const scrollNode = wrapper.firstElementChild + const sizer = scrollNode?.firstElementChild + if (scrollNode instanceof HTMLElement && sizer instanceof HTMLElement) { + measure(scrollNode) + observer.disconnect() + } + + return () => { + wrapper.removeEventListener('scroll', handleScroll, { capture: true }) + observer.disconnect() + } + }, [thread]) + + const handleStart = useCallback(() => { + if (thread === undefined) return + start(thread.id, 'Give me a long, detailed answer so I can scroll around while it streams in.') + }, [thread, start]) + + const handleReset = useCallback(() => { + store.clear() + seededRef.current = false + setDistanceFromBottom(null) + }, [store]) + + const pinned = distanceFromBottom !== null && distanceFromBottom <= SCROLL_END_THRESHOLD + + return ( + +
+ Agent chat — anchor to end while streaming (virtualized) + + {SEED_MESSAGE_COUNT} settled messages, virtualized at {TRANSCRIPT_HEIGHT}px — opens + already pinned to the seeded tail (that's the new default: a virtualized transcript + mounts scrolled to the newest message). Scroll up first to hold your position, then click + "Start streaming turn" and watch the badge stay flipped to held while new content lands + below the fold. Scroll back down past the threshold and following resumes on its own. + +
+ + + + + + {pinned ? 'pinned — following new content' : 'not pinned — position held'} + + + {distanceFromBottom === null ? 'not yet measured' : `${distanceFromBottom}px from bottom`} + + + + + + {thread === undefined ? ( + + } + title="Seeding…" + description="Building the seeded transcript." + variant="section" + /> + + ) : ( + + )} + + + + ) +} diff --git a/apps/playground/src/demo/AgentInlineFeedVirtualizedRowDemoPage.tsx b/apps/playground/src/demo/AgentInlineFeedVirtualizedRowDemoPage.tsx new file mode 100644 index 0000000..cdd7dca --- /dev/null +++ b/apps/playground/src/demo/AgentInlineFeedVirtualizedRowDemoPage.tsx @@ -0,0 +1,100 @@ +/** + * AgentInlineFeedVirtualizedRowDemoPage — the case convergence flagged and no unit test can reach: + * a virtualized `ThreadTranscript` nested inside an inline `ThreadFeedRow`. + * + * `ThreadFeedRow` hides its body with `display: none` while collapsed but keeps it MOUNTED (see its + * module doc — that's the whole point of the lazy-mount-then-kept-mounted invariant). A virtualized + * transcript's scroll element measures 0 height while `display: none`, and whether it recovers on + * re-expand depends on `ResizeObserver` firing on the display toggle — real in a browser, entirely + * absent from happy-dom (the unit harness has no layout engine at all). This page is the only place + * that question can actually be answered: expand the row (first real mount, at full height — + * confirm it scrolls smoothly), collapse it, then re-expand it, and look at whether the transcript + * comes back scrollable and correctly measured, or stuck looking collapsed/zero-height until the + * window resizes. + */ +import { Paper, Stack, Text, Title } from '@mantine/core' +import type { AgentThread } from 'basalt-ui/agent' +import { ThreadFeedRow } from 'basalt-ui/agent-chat' +import type { ComposerSubmit } from 'basalt-ui/agent-chat' +import { useCallback, useMemo, useState } from 'react' +import { buildLongThread } from './agent-long-thread' + +const ROW_MESSAGE_COUNT = 200 +const TRANSCRIPT_HEIGHT = 420 +const SEND_LOG_MAX = 5 + +export function AgentInlineFeedVirtualizedRowDemoPage() { + const seedMessages = useMemo(() => buildLongThread(ROW_MESSAGE_COUNT), []) + const [thread, setThread] = useState(() => ({ + id: 'virtualized-row-demo', + messages: seedMessages, + outcome: { + title: 'Release retro thread', + summary: `${ROW_MESSAGE_COUNT} messages, windowed`, + status: 'done', + }, + status: 'done', + read: true, + createdAt: seedMessages[0]?.createdAt ?? Date.now(), + updatedAt: seedMessages[seedMessages.length - 1]?.createdAt ?? Date.now(), + })) + const [expanded, setExpanded] = useState(false) + const [sendLog, setSendLog] = useState([]) + + const handleSend = useCallback((payload: ComposerSubmit) => { + const message = { + id: crypto.randomUUID(), + role: 'user' as const, + parts: [{ id: crypto.randomUUID(), type: 'text' as const, text: payload.text }], + createdAt: Date.now(), + } + setThread((prev) => ({ ...prev, messages: [...prev.messages, message], updatedAt: Date.now() })) + setSendLog((prev) => [`sent: "${payload.text}"`, ...prev].slice(0, SEND_LOG_MAX)) + }, []) + + return ( + +
+ Virtualized transcript inside a collapsed row + + Expand the row below (first mount — the virtualizer sets up at full height, scrolled to + the newest message by default; confirm it scrolls smoothly through {ROW_MESSAGE_COUNT}{' '} + messages). Collapse it, then re-expand it a few times: does the transcript come back + scrollable and correctly measured at the SAME position you left it, or does it look + stuck/zero-height until you resize the window, or reset back to the newest message? + happy-dom has no layout engine, so this has never been observed anywhere but here. + +
+ + + setExpanded((current) => !current)} + onSend={handleSend} + virtualize + height={TRANSCRIPT_HEIGHT} + /> + + + + + Sent + + {sendLog.length === 0 ? ( + + Nothing sent yet. + + ) : ( + + {sendLog.map((line, index) => ( + + {line} + + ))} + + )} + + + ) +} diff --git a/apps/playground/src/demo/AgentThreadFeedInlineDemoPage.tsx b/apps/playground/src/demo/AgentThreadFeedInlineDemoPage.tsx new file mode 100644 index 0000000..cff5aa6 --- /dev/null +++ b/apps/playground/src/demo/AgentThreadFeedInlineDemoPage.tsx @@ -0,0 +1,357 @@ +/** + * AgentThreadFeedInlineDemoPage — basalt-ui 1.13.0 playground gate demos 1 + 4: the inline-expanding + * Slack row (`ThreadFeedRow`, driven through `ThreadFeed`'s `renderRow` escape hatch) next to the + * unchanged inbox row (`ThreadOutcomeCard`, `ThreadFeed`'s default `'outcome'` variant) — same + * `AgentThread[]`, rendered two ways side by side, so the two shapes are visibly different + * components, not one component in two skins. + * + * DISCOVERED WHILE BUILDING THIS DEMO: `ThreadFeed`'s built-in `variant="inline"` row does NOT wire + * `liveParts`/`liveStatus`/`onStop` (see `ThreadFeedProps.renderRow`'s own doc — it names this gap + * explicitly). There is no way to watch a thread stream live through the built-in row alone. This + * page therefore drives `ThreadFeedRow` directly via `renderRow`, the documented escape hatch for + * exactly that gap — `variant="inline"` on its own would show only the FINISHED transcript. + * + * The left (inline) panel also makes `ThreadFeedRow`'s LOAD-BEARING mount invariant (see its module + * doc: mount lazily on first expand, then never unmount, hide via `display: none` only) visible + * rather than trusted. Each row's transcript carries a `reasoning`-renderer hijack — real AgentPart + * key, not a foreign one — that mounts a `MountProbe` alongside the row's real "Thinking" UI. A + * SEPARATE counter tracks how many times the per-thread transport's `stream()` was actually invoked. + * Expand a row, note both counts, collapse it, re-expand it several times: neither counter may move + * — a genuinely new turn (sending another message) is the only thing allowed to move either one. + * If a future Mantine bump ever flips `ThreadFeedRow`'s collapse mechanics onto `` (see its + * module doc), this is where that regression shows up first: both counters would climb on every + * re-expand instead of staying flat. + * + * Caveat: in dev (this playground runs ``), the FIRST expand of a row can read "mounted + * 2×" instead of "1×" — React's intentional double-invoke of a fresh effect on first mount, not a + * bug in `ThreadFeedRow`. What matters is that the count stops moving after that first expand. + * + * TWO FIXES FROM THE 1.13.0 GATE, both demo-side only (see each site's own comment): + * + * 1. `onStop` is now wired to the left panel's `ThreadFeedRow`s. It was never forwarded before — + * `ThreadFeedRow` only shows its composer's Stop action when `onStop` is DEFINED (it has no way + * to infer "a run exists" from `liveStatus` alone), so a live row's textarea disabled with no + * visible way to cancel it. + * + * 2. The three seed threads are no longer started via `start()` inside a mount effect. Under + * ``, that effect's cleanup — a SEPARATE effect inside `useAgentThreadRuns` itself, + * which aborts every in-flight controller so a fiber whose effects re-run without unmounting + * doesn't leak stream subscriptions — fires between this page's two mount passes and aborts all + * three freshly-started runs before either the reload-reconciler or the abort path can settle + * `runs` back to empty. `stop()` can't help here either: it is gated on `controllersRef` still + * holding the thread's controller, and that map was already cleared by the same cleanup. The + * result was three threads permanently reading "streaming" with an unremovable phantom entry — + * a REAL framework defect, SINCE FIXED IN 1.13.0, but NOT where this note originally pointed: + * the fix is in `use-agent-thread-runs.ts`'s unmount-cleanup effect, which now tears down the + * `runs` entries for exactly the threadIds it aborts. `consumeAndFinalize`'s + * `if (controller.signal.aborted) return` guard was deliberately left alone (it carries a + * comment saying so): from inside that loop, a `controllersRef` mismatch cannot be told apart + * from "a newer resumed run already owns this key", so a teardown there would clobber a live + * successor. Do not "finish the job" at that guard. + * This page no longer trips it either way: seeding now writes three ALREADY-SETTLED threads into + * the store (`store.appendMessage` + `store.setStatus('done')` + `store.setOutcome`, no + * `start()`, no controller, nothing for that cleanup to abort), so a fresh load lands directly on + * three expandable, non-streaming threads. `start()` is still exercised for real the first time a + * composer sends a message — that call happens from a click, long after both StrictMode mount + * passes have settled, so it never meets the race above. + */ +import { Badge, Box, Button, Group, Paper, SimpleGrid, Stack, Text, Title } from '@mantine/core' +import { createThreadsStore, heuristicOutcome, useAgentThreadRuns } from 'basalt-ui/agent' +import type { + AgentPart, + AgentThread, + AgentTransport, + ChatMessage, + ForeignPart, + PartRenderer, + PartRenderers, + ReasoningPart, +} from 'basalt-ui/agent' +import { ThreadFeed, ThreadFeedRow, threadPartRenderers } from 'basalt-ui/agent-chat' +import type { ComposerSubmit } from 'basalt-ui/agent-chat' +import type { JSX } from 'react' +import { useCallback, useEffect, useMemo, useReducer, useRef, useState } from 'react' +import { AGENT_SCENARIOS, scenarioTransport } from './agent-scenarios' +import { IconReset } from './icons' + +// ── Seed scenarios (skip 'error' — this demo is about mount/stream counting, not failure) ──────── + +const SEED_SCENARIOS = AGENT_SCENARIOS.filter((scenario) => scenario.value !== 'error') + +let scenarioCursor = 0 +function nextSeedScenario() { + const scenario = SEED_SCENARIOS[scenarioCursor % SEED_SCENARIOS.length] + scenarioCursor += 1 + if (scenario === undefined) throw new Error('SEED_SCENARIOS is empty') + return scenario +} + +const SEED_PROMPTS = [ + 'How should I structure the shell/router seam?', + 'Compare the trade-offs of virtualizing this transcript.', + 'What changed in the last release?', +] + +// ── Per-thread counting transport — the "stream started" half of the invariant ─────────────────── + +/** Resolved once per thread id and cached by `useAgentThreadRuns` (see its own doc) — `stream()` + * itself is called once per turn on that thread, which is exactly what `onStreamStart` counts. */ +function makeCountingTransport( + onStreamStart: (threadId: string) => void, +): (threadId: string) => AgentTransport { + return (threadId) => ({ + stream(input, signal) { + onStreamStart(threadId) + return scenarioTransport(nextSeedScenario(), 'normal').stream(input, signal) + }, + }) +} + +// ── Per-thread counters — plain refs + a manual re-render, not React state ──────────────────────── +// (state would need a fresh Map identity per bump; a ref + forced re-render is the cheaper, simpler +// escape hatch for a value this page only ever reads back for display, never diffs against.) + +function usePerThreadCounter(): readonly [Map, (id: string) => void] { + const counts = useRef>(new Map()) + const [, bump] = useReducer((n: number) => n + 1, 0) + const increment = useCallback((id: string) => { + counts.current.set(id, (counts.current.get(id) ?? 0) + 1) + bump() + }, []) + return [counts.current, increment] as const +} + +// ── The mount probe — proves ThreadFeedRow's lazy-mount-then-kept-mounted guarantee from OUTSIDE +// the package, by riding the SAME `renderers` open-registry seam a consumer would use for a real +// foreign part. ───────────────────────────────────────────────────────────────────────────────── + +const ReasoningView = threadPartRenderers.reasoning! + +function MountProbe({ + threadId, + onMount, +}: { + threadId: string + onMount: (id: string) => void +}): null { + // Empty-ish deps (threadId/onMount are both stable for a given row) — fires once per genuine + // mount of THIS component instance, and never again while it stays mounted, regardless of how + // many times the row around it re-renders (streaming deltas, collapse/expand toggles, ...). + useEffect(() => { + onMount(threadId) + }, [threadId, onMount]) + return null +} + +function ProbedReasoning({ + threadId, + onMount, + part, + settled, +}: { + threadId: string + onMount: (id: string) => void + part: ReasoningPart + settled: boolean +}): JSX.Element { + return ( + <> + + + + ) +} + +/** Hijacks the real `reasoning` AgentPart key via `ThreadFeedRow`'s open `renderers` registry — + * chosen over registering a genuinely foreign part type because it needs no `BasaltRegister` + * augmentation and keeps every store/outcome-resolver call in this file on the default `AgentPart` + * generic. Re-uses the exported `threadPartRenderers.reasoning` so the row's real "Thinking" + * disclosure still renders unchanged — this is additive instrumentation, not a replacement UI. */ +function makeReasoningProbe( + threadId: string, + onMount: (id: string) => void, +): PartRenderer { + return (ctx) => ( + + ) +} + +// ── Store ────────────────────────────────────────────────────────────────────────────────────── + +const useThreads = createThreadsStore({ key: 'playground-thread-feed-inline', version: 2 }) + +const PANEL_HEIGHT = 520 + +export function AgentThreadFeedInlineDemoPage() { + const store = useThreads() + const [mountCounts, bumpMount] = usePerThreadCounter() + const [streamCounts, bumpStream] = usePerThreadCounter() + const transport = useMemo(() => makeCountingTransport(bumpStream), [bumpStream]) + const { runs, start, stop } = useAgentThreadRuns({ + transport, + store, + resolveOutcome: heuristicOutcome, + }) + + // `renderRow` bypasses `ThreadFeed`'s own collapsedId tracking entirely (see its doc) — this + // panel owns expand/collapse itself, one row open at a time, mirroring the outcome panel's + // single-selection model. + const [expandedId, setExpandedId] = useState(null) + const [outcomeSelectedId, setOutcomeSelectedId] = useState(null) + + // Seeds three ALREADY-SETTLED threads straight into the store — deliberately NOT via start() + // (see this module's doc, fix 2, for the StrictMode abort race that produced when it was). Each + // scenario's `.parts(prompt)` is a synchronous, fully-formed AgentPart[] (no transport, no + // controller, nothing an unmount-cleanup effect could ever abort), so both StrictMode mount + // passes see the same three finished threads and neither leaves anything registered to tear down. + const seededRef = useRef(false) + useEffect(() => { + if (seededRef.current || store.threads.length > 0) return + seededRef.current = true + for (const prompt of SEED_PROMPTS) { + const scenario = nextSeedScenario() + const id = store.create() + const createdAt = Date.now() + const userMessage: ChatMessage = { + id: crypto.randomUUID(), + role: 'user', + parts: [{ id: crypto.randomUUID(), type: 'text', text: prompt }], + createdAt, + } + const assistantMessage: ChatMessage = { + id: crypto.randomUUID(), + role: 'assistant', + parts: scenario.parts(prompt), + createdAt: createdAt + 1, + finish: 'complete', + } + store.appendMessage(id, userMessage) + store.appendMessage(id, assistantMessage) + store.setStatus(id, 'done') + store.setOutcome( + id, + heuristicOutcome({ + id, + messages: [userMessage, assistantMessage], + outcome: null, + status: 'done', + read: false, + createdAt, + updatedAt: createdAt + 1, + }), + ) + } + }, [store]) + + const handleSend = useCallback( + (thread: AgentThread, payload: ComposerSubmit) => { + start(thread.id, payload.text) + }, + [start], + ) + + const handleReset = useCallback(() => { + store.clear() + seededRef.current = false + }, [store]) + + const inlineRow = useCallback( + (thread: AgentThread) => { + const expanded = thread.id === expandedId + const run = runs.get(thread.id) + return ( + + + + {`mounted ${mountCounts.get(thread.id) ?? 0}×`} + + + {`stream started ${streamCounts.get(thread.id) ?? 0}×`} + + + setExpandedId((current) => (current === id ? null : id))} + {...(run !== undefined + ? { liveParts: run.parts, liveStatus: 'streaming' as const } + : {})} + // `PartRenderers` is augmented PROGRAM-WIDE by ./agent-part-registry.type-guard.ts + // ('data-toolProgress' | 'data-chart') — module augmentation is global for this whole + // tsconfig, so the augmented type no longer accepts an arbitrary extra key by direct + // assignment (excess-property-checked, unlike `definePartRenderers`'s own const-generic + // call site — see that fixture's Fixture 3 for the asymmetry). This demo intentionally + // hijacks the REAL 'reasoning' key, which was never meant to be constrained by that + // augmentation at all, so the cast below is a scoped, deliberate escape — not a case the + // augmented type was ever supposed to validate. + renderers={ + { reasoning: makeReasoningProbe(thread.id, bumpMount) } as unknown as PartRenderers + } + onSend={(payload) => handleSend(thread, payload)} + onStop={() => stop(thread.id)} + /> + + ) + }, + [expandedId, runs, mountCounts, streamCounts, bumpMount, handleSend, stop], + ) + + return ( + +
+ Thread feed — inline row vs outcome row + + The same three threads, rendered two ways: the Slack-shaped inline row on the left (expand + in place, transcript + composer), the unchanged inbox row on the right. Both load already + settled — expand a row on the left, watch its two counters, then collapse and re-expand it + a few times — both must stay flat. Sending a new message (the composer at the bottom of an + expanded row) is the only thing allowed to move either counter, and now shows a Stop + button while it streams. + +
+ + + + + variant="inline" (renderRow → ThreadFeedRow) + + + + + + + + variant="outcome" (ThreadOutcomeCard, unchanged) + + + + + + + + + + + + ) +} diff --git a/apps/playground/src/demo/AgentTranscriptVirtualizeDemoPage.tsx b/apps/playground/src/demo/AgentTranscriptVirtualizeDemoPage.tsx new file mode 100644 index 0000000..5c56868 --- /dev/null +++ b/apps/playground/src/demo/AgentTranscriptVirtualizeDemoPage.tsx @@ -0,0 +1,177 @@ +/** + * AgentTranscriptVirtualizeDemoPage — basalt-ui 1.13.0 playground gate demos 2 + 3: a 500-message + * `ThreadTranscript` with `virtualize` on, plus copy / regenerate / relative timestamps / author + * grouping on the same thread. + * + * The 500 messages (`./agent-long-thread`) are deliberately NOT uniform height: one-line acks, + * bulleted medium replies, and long fenced-code deep-dives sit side by side, and some consecutive + * same-author runs land inside the 5-minute grouping window (chrome collapses) while others land + * just outside it (chrome stays) — a uniform thread would hide both a `measureElement` regression + * and a grouping-boundary regression. + * + * happy-dom (the unit test harness) has no layout engine, so `virtualize`'s actual scroll/measure + * behavior has never been observed anywhere but here — this page is where a human confirms the + * windowed pane scrolls smoothly. The "Virtualize" switch toggles the SAME 500 messages through + * the unwindowed path so the cost of turning it off is felt directly, not just asserted. + * + * The `virtualize: true` + omitted `height` tsc error this gate also asks for is proven as a + * committed fixture instead of re-created live here — see + * `./agent-transcript-virtualize.type-guard.ts` beside this file. + * + * 1.13.0 GATE ADDITION: `overscan`/`estimateSize`/`initialScroll` — the fields of the consumer- + * supplied `VirtualizeOptions` object form (`virtualize: { overscan, estimateSize, initialScroll }` + * instead of the bare `virtualize: true` used everywhere else in this repo) — had never been + * exercised from outside the package. The controls below feed all three straight through: push + * `estimateSize` far from the true row height and scroll to feel the rows visibly jump as they + * measure in, drop `overscan` to 0 and scroll fast to see blank frames at the windowing edge, or + * flip `initialScroll` to see the pane mount at the newest message (`'end'`, the shipped default) + * versus the oldest (`'start'`) on the SAME 500-message thread. + */ +import { + Badge, + Group, + List, + NumberInput, + Paper, + SegmentedControl, + Stack, + Switch, + Text, + Title, +} from '@mantine/core' +import { ThreadTranscript } from 'basalt-ui/agent-chat' +import type { VirtualizeProps } from 'basalt-ui/agent-chat' +import { VX } from 'basalt-ui/tokens' +import { useMemo, useState } from 'react' +import { buildLongThread } from './agent-long-thread' + +const MESSAGE_COUNT = 500 +const TRANSCRIPT_HEIGHT = 560 +const REGENERATE_LOG_MAX = 5 +// Mirrors thread-message.tsx's own internal defaults (DEFAULT_VIRTUALIZE_OVERSCAN / +// DEFAULT_VIRTUALIZE_ESTIMATE_SIZE) purely as this page's starting point — neither is exported, so +// these are just reasonable initial values for the two inputs below, not a reference to the source. +// KEEP THESE IN STEP with those constants: this page ALWAYS passes the object form, so a starting +// value that disagrees with the package default means the shipped default is the one thing this +// page can never show. (1.13.0 raised estimateSize 96 → 160 for exactly the jump-on-first-descent +// reason the copy below describes; 96 was left behind here and is restored to parity.) +const DEFAULT_OVERSCAN = 6 +const DEFAULT_ESTIMATE_SIZE = 160 +// Mirrors the shipped `VirtualizeOptions.initialScroll` default — see the note above, same reasoning. +const DEFAULT_INITIAL_SCROLL = 'end' + +export function AgentTranscriptVirtualizeDemoPage() { + const messages = useMemo(() => buildLongThread(MESSAGE_COUNT), []) + const [virtualizeOn, setVirtualizeOn] = useState(true) + const [overscan, setOverscan] = useState(DEFAULT_OVERSCAN) + const [estimateSize, setEstimateSize] = useState(DEFAULT_ESTIMATE_SIZE) + const [initialScroll, setInitialScroll] = useState<'end' | 'start'>(DEFAULT_INITIAL_SCROLL) + const [regenerateLog, setRegenerateLog] = useState([]) + + const virtualizeProps: VirtualizeProps = virtualizeOn + ? { virtualize: { overscan, estimateSize, initialScroll }, height: TRANSCRIPT_HEIGHT } + : {} + + return ( + +
+ Transcript virtualization — {MESSAGE_COUNT} messages + + Scroll the pane below to confirm it stays smooth with virtualization on. Toggle it off to + feel the cost of rendering all {MESSAGE_COUNT} rows unwindowed on the same thread. Hover + any message for its relative timestamp, a copy action, and — on the last assistant message + only — Regenerate. Scroll through a few turns to see consecutive same-author messages: + some collapse their role label and chrome (grouped, inside the 5-minute window), some + don't (same role, but the gap crossed the window). overscan,{' '} + estimateSize, and initialScroll below are the consumer-supplied{' '} + VirtualizeOptions — push them off their defaults and scroll to feel the + difference. Switching initialScroll remounts the pane below so you can see it + land at the newest message ('end', the shipped default) versus the + oldest ('start'). + +
+ + + setVirtualizeOn(event.currentTarget.checked)} + /> + setOverscan(typeof value === 'number' ? value : DEFAULT_OVERSCAN)} + min={0} + max={50} + disabled={!virtualizeOn} + w={110} + /> + + setEstimateSize(typeof value === 'number' ? value : DEFAULT_ESTIMATE_SIZE) + } + min={16} + max={800} + disabled={!virtualizeOn} + w={130} + /> + setInitialScroll(value === 'start' ? 'start' : 'end')} + disabled={!virtualizeOn} + data={[ + { label: 'initialScroll: end', value: 'end' }, + { label: 'initialScroll: start', value: 'start' }, + ]} + /> + + {`${messages.length} messages`} + + + + + + setRegenerateLog((prev) => + [`Regenerate requested for ${messageId}`, ...prev].slice(0, REGENERATE_LOG_MAX), + ), + }} + {...virtualizeProps} + /> + + + + + Regenerate log + + {regenerateLog.length === 0 ? ( + + Hover the LAST assistant message — already in view when initialScroll is{' '} + 'end', otherwise scroll down — and click Regenerate. + + ) : ( + + {regenerateLog.map((line, index) => ( + + + {line} + + + ))} + + )} + + + ) +} diff --git a/apps/playground/src/demo/agent-long-thread.ts b/apps/playground/src/demo/agent-long-thread.ts new file mode 100644 index 0000000..faaf813 --- /dev/null +++ b/apps/playground/src/demo/agent-long-thread.ts @@ -0,0 +1,159 @@ +/** + * agent-long-thread — a deterministic, genuinely variable-height 500-message thread backing the + * virtualization gate demo (`AgentTranscriptVirtualizeDemoPage`). + * + * Three things this generator is deliberately shaped to exercise, all at once: + * - VARIABLE row heights (one-line acks, bulleted medium replies, long fenced-code deep-dives) — + * uniform rows would hide a `measureElement` regression in the virtualizer. + * - Consecutive same-author runs both INSIDE the 5-minute grouping window (role chrome collapses) + * and just OUTSIDE it (role chrome stays), so the Slack-rhythm boundary is actually exercised, + * not just the common alternating-speaker case. + * - Old-to-new timestamps spread across real hours, so `formatRelativeTime` reads naturally + * ("3 hours ago", not a wall of "just now"). + */ +import type { ChatMessage } from 'basalt-ui/agent' + +const SHORT_REPLIES = [ + 'Got it — will do.', + 'That looks right to me.', + 'Confirmed, no issues.', + 'No blockers on my end.', + 'Makes sense, thanks.', +] as const + +function mediumReply(n: number): string { + return ( + `Here's a quick rundown for item #${n}:\n\n` + + `- Checked the current implementation\n` + + `- Confirmed the edge case around empty input\n` + + `- No regressions in the existing test suite\n\n` + + `Let me know if you want a deeper look at any of these.` + ) +} + +function longReply(n: number): string { + return ( + `## Deep dive — item #${n}\n\n` + + `This one took a bit longer to trace through. The root cause sits in how the accumulator ` + + `merges partial updates: when two deltas arrive in the same tick, the second overwrites the ` + + `first instead of appending.\n\n` + + '```ts\n' + + `function merge(parts: Part[], next: Part): Part[] {\n` + + ` const index = parts.findIndex((p) => p.id === next.id)\n` + + ` if (index === -1) return [...parts, next]\n` + + ` const copy = [...parts]\n` + + ` copy[index] = next // should append text, not replace it\n` + + ` return copy\n` + + `}\n` + + '```\n\n' + + `A few follow-ups worth tracking separately:\n\n` + + `1. Add a regression test for the same-tick double-delta case.\n` + + `2. Audit the other three call sites that share this helper.\n` + + `3. Document the invariant so the next person doesn't reintroduce it.\n\n` + + `None of this blocks the current release — flagging it for the next pass.` + ) +} + +const SHORT_PROMPT_TEMPLATES = [ + 'Ship it?', + 'Any concerns?', + 'Status on #%d?', + 'Still good?', +] as const + +function pickPrompt(turn: number): string { + // Non-null: `turn % SHORT_PROMPT_TEMPLATES.length` is always in bounds for this fixed literal array. + const template = SHORT_PROMPT_TEMPLATES[turn % SHORT_PROMPT_TEMPLATES.length]! + return template.includes('%d') ? template.replace('%d', String(turn)) : template +} + +function pickReply(turn: number): string { + const mod = turn % 3 + // Non-null: same in-bounds-by-construction reasoning as pickPrompt above. + if (mod === 0) return SHORT_REPLIES[turn % SHORT_REPLIES.length]! + if (mod === 1) return mediumReply(turn) + return longReply(turn) +} + +function pickFollowUp(turn: number): string { + return SHORT_REPLIES[turn % SHORT_REPLIES.length]! +} + +function textPart(text: string): { id: string; type: 'text'; text: string } { + return { id: crypto.randomUUID(), type: 'text', text } +} + +function makeMessage(role: ChatMessage['role'], createdAt: number, text: string): ChatMessage { + return { + id: crypto.randomUUID(), + role, + parts: [textPart(text)], + createdAt, + ...(role === 'assistant' ? { finish: 'complete' as const } : {}), + } +} + +const GROUPED_GAP_MS = 90_000 // 1.5 min — inside the 5-min grouping window +const UNGROUPED_GAP_MS = 6 * 60_000 // 6 min — just outside it +const REPLY_GAP_MS = 2 * 60_000 +const TURN_GAP_MS = 22 * 60_000 + +/** + * Builds `count` messages (oldest first) — a long-running thread that started roughly + * `count * ~26min` in the past and ends "just now"-ish, cycling through six turn shapes so both + * grouped and ungrouped consecutive-same-author runs occur, alongside short/medium/long replies. + */ +export function buildLongThread(count: number): ChatMessage[] { + const messages: ChatMessage[] = [] + // This starting point is only a rough headroom guess — it does not need to land the newest + // message near "now" on its own, because every timestamp is shifted by one offset after + // generation (below) so the newest message lands at/just before `Date.now()` regardless of how + // the cycle mix actually plays out. + let cursor = Date.now() - count * TURN_GAP_MS + let turn = 0 + + while (messages.length < count) { + const cycle = turn % 6 + + messages.push(makeMessage('user', cursor, pickPrompt(turn))) + cursor += GROUPED_GAP_MS + + // Grouped user follow-up: same role, well inside the 5-minute window. + if (cycle === 1 && messages.length < count) { + messages.push(makeMessage('user', cursor, `Actually, one more thing — ${pickPrompt(turn)}`)) + cursor += GROUPED_GAP_MS + } + + cursor += REPLY_GAP_MS + if (messages.length < count) { + messages.push(makeMessage('assistant', cursor, pickReply(turn))) + } + + if (cycle === 3 && messages.length < count) { + // Grouped assistant follow-up: same role, inside the window. + cursor += GROUPED_GAP_MS + messages.push(makeMessage('assistant', cursor, `One more note: ${pickFollowUp(turn)}`)) + } else if (cycle === 4 && messages.length < count) { + // Same role, but OUTSIDE the window — proves the boundary, not just the common case. + cursor += UNGROUPED_GAP_MS + messages.push( + makeMessage('assistant', cursor, `Coming back after a pause — ${pickFollowUp(turn)}`), + ) + } + + cursor += TURN_GAP_MS + turn += 1 + } + + const sliced = messages.slice(0, count) + + // Shift every timestamp by one offset so the newest message lands at/just before `Date.now()` — + // a post-generation correction rather than a retuned constant, so it stays correct even if the + // cycle mix above changes later (unlike the previous headroom guess, which assumed a worst case + // that didn't hold: cycle 0 alone emits 2 messages per turn, not 1, so the demo's newest message + // was landing days in the past instead of "just now"). + const newest = sliced.at(-1) + if (newest === undefined) return sliced + const shift = Date.now() - newest.createdAt + return sliced.map((message) => ({ ...message, createdAt: message.createdAt + shift })) +} diff --git a/apps/playground/src/demo/agent-transcript-virtualize.type-guard.ts b/apps/playground/src/demo/agent-transcript-virtualize.type-guard.ts new file mode 100644 index 0000000..4c71c32 --- /dev/null +++ b/apps/playground/src/demo/agent-transcript-virtualize.type-guard.ts @@ -0,0 +1,68 @@ +// PROVES (consumer-facing, not just package-internal): ThreadTranscriptProps and ThreadFeedRowProps +// both inherit VirtualizeProps' virtualize-implies-height guard through the PUBLIC +// `basalt-ui/agent-chat` subpath a real app imports — reproduces the "virtualize: true with no +// height" tsc error the release notes ask for, from the public prop types rather than re-deriving +// the internal `VirtualizeProps` union already proven at its definition by +// packages/basalt-ui/src/agent-chat/virtualize.type-guard.test.ts. +import type { AgentThread } from 'basalt-ui/agent' +import type { ThreadFeedRowProps, ThreadTranscriptProps } from 'basalt-ui/agent-chat' + +function acceptTranscript(props: ThreadTranscriptProps): ThreadTranscriptProps { + return props +} + +function acceptRow(props: ThreadFeedRowProps): ThreadFeedRowProps { + return props +} + +const messages: ThreadTranscriptProps['messages'] = [] + +const thread: AgentThread = { + id: 'fixture-thread', + messages: [], + outcome: null, + status: 'pending', + read: false, + createdAt: 0, + updatedAt: 0, +} + +function noopToggle(_id: string): void { + // fixture-only no-op +} + +function noopSend(): void { + // fixture-only no-op +} + +// ── Valid combinations — must type-check with no error ──────────────────────── + +acceptTranscript({ messages }) +acceptTranscript({ messages, virtualize: true, height: 400 }) +acceptRow({ thread, expanded: false, onToggle: noopToggle, onSend: noopSend }) +acceptRow({ + thread, + expanded: false, + onToggle: noopToggle, + onSend: noopSend, + virtualize: true, + height: 300, +}) + +// ── Invalid combinations — each MUST be a tsc error ─────────────────────────── + +// @ts-expect-error `height` is required when `virtualize: true` (ThreadTranscriptProps) +acceptTranscript({ messages, virtualize: true }) + +// @ts-expect-error `height` is forbidden when `virtualize` is omitted/false (ThreadTranscriptProps) +acceptTranscript({ messages, height: 400 }) + +// @ts-expect-error `height` is required when `virtualize: true` (ThreadFeedRowProps) +acceptRow({ thread, expanded: false, onToggle: noopToggle, onSend: noopSend, virtualize: true }) + +// @ts-expect-error `height` is forbidden when `virtualize` is omitted/false (ThreadFeedRowProps) +acceptRow({ thread, expanded: false, onToggle: noopToggle, onSend: noopSend, height: 400 }) + +// PROVES: the virtualize/height union guard (packages/basalt-ui/src/agent-chat/virtualize.ts) holds +// on the actual public prop types a consumer imports (ThreadTranscriptProps, ThreadFeedRowProps), +// not only on the internal VirtualizeProps type it's built from. diff --git a/apps/playground/src/demo/nav-model.tsx b/apps/playground/src/demo/nav-model.tsx index 602acb1..5281275 100644 --- a/apps/playground/src/demo/nav-model.tsx +++ b/apps/playground/src/demo/nav-model.tsx @@ -249,6 +249,34 @@ export const NAV_MODEL: SidebarSection[] = [ icon: , href: '/threads-adapter', }, + { + key: 'agent-thread-feed-inline', + label: 'Agent thread feed (inline)', + mobile: true, + icon: , + href: '/agent-thread-feed-inline', + }, + { + key: 'agent-transcript-virtualize', + label: 'Agent transcript virtualize', + mobile: true, + icon: , + href: '/agent-transcript-virtualize', + }, + { + key: 'agent-inline-feed-virtualized', + label: 'Agent inline feed (virtualized row)', + mobile: true, + icon: , + href: '/agent-inline-feed-virtualized', + }, + { + key: 'agent-anchor-to-end', + label: 'Agent anchor to end (streaming)', + mobile: true, + icon: , + href: '/agent-anchor-to-end', + }, ], }, { diff --git a/apps/playground/src/routes/agent-anchor-to-end.tsx b/apps/playground/src/routes/agent-anchor-to-end.tsx new file mode 100644 index 0000000..9e37a3c --- /dev/null +++ b/apps/playground/src/routes/agent-anchor-to-end.tsx @@ -0,0 +1,7 @@ +import { createFileRoute } from '@tanstack/react-router' +import { AgentAnchorToEndDemoPage } from '../demo/AgentAnchorToEndDemoPage' + +export const Route = createFileRoute('/agent-anchor-to-end')({ + staticData: { title: 'Agent anchor to end' }, + component: AgentAnchorToEndDemoPage, +}) diff --git a/apps/playground/src/routes/agent-inline-feed-virtualized.tsx b/apps/playground/src/routes/agent-inline-feed-virtualized.tsx new file mode 100644 index 0000000..3a7b289 --- /dev/null +++ b/apps/playground/src/routes/agent-inline-feed-virtualized.tsx @@ -0,0 +1,7 @@ +import { createFileRoute } from '@tanstack/react-router' +import { AgentInlineFeedVirtualizedRowDemoPage } from '../demo/AgentInlineFeedVirtualizedRowDemoPage' + +export const Route = createFileRoute('/agent-inline-feed-virtualized')({ + staticData: { title: 'Agent inline feed (virtualized row)' }, + component: AgentInlineFeedVirtualizedRowDemoPage, +}) diff --git a/apps/playground/src/routes/agent-thread-feed-inline.tsx b/apps/playground/src/routes/agent-thread-feed-inline.tsx new file mode 100644 index 0000000..6e62b2d --- /dev/null +++ b/apps/playground/src/routes/agent-thread-feed-inline.tsx @@ -0,0 +1,7 @@ +import { createFileRoute } from '@tanstack/react-router' +import { AgentThreadFeedInlineDemoPage } from '../demo/AgentThreadFeedInlineDemoPage' + +export const Route = createFileRoute('/agent-thread-feed-inline')({ + staticData: { title: 'Agent thread feed (inline)' }, + component: AgentThreadFeedInlineDemoPage, +}) diff --git a/apps/playground/src/routes/agent-transcript-virtualize.tsx b/apps/playground/src/routes/agent-transcript-virtualize.tsx new file mode 100644 index 0000000..a98dcf7 --- /dev/null +++ b/apps/playground/src/routes/agent-transcript-virtualize.tsx @@ -0,0 +1,7 @@ +import { createFileRoute } from '@tanstack/react-router' +import { AgentTranscriptVirtualizeDemoPage } from '../demo/AgentTranscriptVirtualizeDemoPage' + +export const Route = createFileRoute('/agent-transcript-virtualize')({ + staticData: { title: 'Agent transcript virtualize' }, + component: AgentTranscriptVirtualizeDemoPage, +}) diff --git a/packages/basalt-ui/AGENTS.md b/packages/basalt-ui/AGENTS.md index c5a8ddb..1b880b6 100644 --- a/packages/basalt-ui/AGENTS.md +++ b/packages/basalt-ui/AGENTS.md @@ -6,27 +6,27 @@ Toolchain: Bun runtime, oxlint + oxfmt, conventional commits (empty scope), `mas ## Subpath ownership -| Subpath | Layer | Purpose | -| --------------------------- | --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `basalt-ui` | mantine-coupled | BasaltProvider, createBasaltTheme, BasaltShell + sidebar/mobile-nav/breadcrumbs, NavCountBadge, ThemeToggle, ThreadWorkspace + thread-chat components, dashboard composites (DeltaBadge, StatCard with threshold tone, EmptyState, SettingsSection/SettingsRow/DangerZone) | -| `basalt-ui/charts` | headless | visx chart primitives, sparklines, hooks, and token re-exports (Mantine-free) | -| `basalt-ui/tokens` | headless | VX token refs, buildPaletteCss, defineSeries, seriesTokens, groupTokens, alpha (Mantine-free) | -| `basalt-ui/theme-lab` | mantine-coupled | ThemeLabControls, applyOverrides, COLOR_GROUPS — a low-level inspector for the non-derived structural tokens; identity/color tuning lives in DeriveControls | -| `basalt-ui/vite` | mantine-coupled | basaltViteConfig(opts) — Vite preset for basalt-ui consumer apps; basaltAppPlugin(opts) — PWA head, manifest, and icon metadata derived from the token palette | -| `basalt-ui/guard` | headless | checkSource, GUARD_RULES, Finding types — the headless theme-guard core | -| `basalt-ui/query` | headless | createBasaltQueryClient, transport-agnostic unwrap, lazy BasaltQueryDevtools | -| `basalt-ui/router-tanstack` | headless | TanStack Router bridge: useBasaltNav (active route) + useRouterBreadcrumbs + createSearchParamStore (single-select URL-state store) + createMultiSearchParamStore (multi-select URL-state store) | -| `basalt-ui/forms` | mantine-coupled | Mantine form adapter: useBasaltForm, field, FormErrorSummary, useFormDraft (Standard Schema) | -| `basalt-ui/notifications` | mantine-coupled | Mantine notifications: notify helpers, typed registry, persisted history, NotificationBell, NotificationCenter | -| `basalt-ui/commands` | mantine-coupled | typed command bus + overlay controller, toSpotlightActions, ShortcutsHelp, BasaltOverlays | -| `basalt-ui/data` | mantine-coupled | Convenience barrel pulling both TanStack Table + Virtual peer groups: BasaltDataTable, BasaltVirtualList (Mantine-rendered) — prefer ./data/table or ./data/virtual for per-feature opt-in | -| `basalt-ui/data/table` | mantine-coupled | BasaltDataTable: a sortable data table over TanStack Table, rendered with Mantine (Mantine-rendered) | -| `basalt-ui/data/virtual` | mantine-coupled | BasaltVirtualList: a windowed virtual list over TanStack Virtual, rendered with Mantine (Mantine-rendered) | -| `basalt-ui/agent` | headless | Headless streaming-chat layer: useAgentStream, aiSdkTransport (recommended default) + edenTransport, isResumable/ResumableAgentTransport (stream-resumption seam), PartList, coalesceParts, the ForeignPart/definePartRenderers/narrowAgentPart open part-registry seam, plus the multi-thread createThreadsStore + useAgentThreadRuns + outcome-resolver seam (Mantine-free) | -| `basalt-ui/agent-chat` | mantine-coupled | Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed, ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx. | -| `basalt-ui/content` | mantine-coupled | Prose (article/chat typography), CodeBlock (shiki, optional peer), Callout, TableOfContents, ReadingProgress, Markdown (react-markdown + remark-gfm, optional peers; `streaming` is a rendering mode ONLY — `contentTrust` is the independent security input, and any surface rendering agent/model output must pin `contentTrust="untrusted"`, the sole input to the image-origin allowlist; a `fenceRenderers` registry — settledOnly/FenceRenderer/FenceRenderers/FenceRenderContext; `sanitizeSchema`, an additions-only SanitizeSchemaExtension merged over BASALT_SANITIZE_SCHEMA via mergeSanitizeSchema; the remend streaming-repair pass is now a lazy optional peer), MermaidDiagram (beautiful-mermaid, optional peer), mdxComponents/createMdxComponents, ArticleLayout (docs-page frame), ArticleCard/ArticleGrid (overview cards), Article model (sortArticles/filterArticles/formatArticleDate), ArticleFilterBar (category/tags filter UI), toArticleActions (Spotlight projector, @mantine/spotlight type-only), GuideLink/GuideDrawer (contextual-help drawer) — the content/prose surface | -| `basalt-ui/state` | headless | createPersistedState (versioned localStorage) + useOnlineStatus — Mantine-free state primitives | -| `basalt-ui/connectivity` | mantine-coupled | ConnectivityProvider (aggregates browser online/offline, React Query onlineManager, SSE, and health-check pings into one status), useConnectivity, and ConnectivityIndicator — auto-mounted by BasaltProvider | +| Subpath | Layer | Purpose | +| --------------------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `basalt-ui` | mantine-coupled | BasaltProvider, createBasaltTheme, BasaltShell + sidebar/mobile-nav/breadcrumbs, NavCountBadge, ThemeToggle, ThreadWorkspace + thread-chat components, dashboard composites (DeltaBadge, StatCard with threshold tone, EmptyState, SettingsSection/SettingsRow/DangerZone) | +| `basalt-ui/charts` | headless | visx chart primitives, sparklines, hooks, and token re-exports (Mantine-free) | +| `basalt-ui/tokens` | headless | VX token refs, buildPaletteCss, defineSeries, seriesTokens, groupTokens, alpha (Mantine-free) | +| `basalt-ui/theme-lab` | mantine-coupled | ThemeLabControls, applyOverrides, COLOR_GROUPS — a low-level inspector for the non-derived structural tokens; identity/color tuning lives in DeriveControls | +| `basalt-ui/vite` | mantine-coupled | basaltViteConfig(opts) — Vite preset for basalt-ui consumer apps; basaltAppPlugin(opts) — PWA head, manifest, and icon metadata derived from the token palette | +| `basalt-ui/guard` | headless | checkSource, GUARD_RULES, Finding types — the headless theme-guard core | +| `basalt-ui/query` | headless | createBasaltQueryClient, transport-agnostic unwrap, lazy BasaltQueryDevtools | +| `basalt-ui/router-tanstack` | headless | TanStack Router bridge: useBasaltNav (active route) + useRouterBreadcrumbs + createSearchParamStore (single-select URL-state store) + createMultiSearchParamStore (multi-select URL-state store) | +| `basalt-ui/forms` | mantine-coupled | Mantine form adapter: useBasaltForm, field, FormErrorSummary, useFormDraft (Standard Schema) | +| `basalt-ui/notifications` | mantine-coupled | Mantine notifications: notify helpers, typed registry, persisted history, NotificationBell, NotificationCenter | +| `basalt-ui/commands` | mantine-coupled | typed command bus + overlay controller, toSpotlightActions, ShortcutsHelp, BasaltOverlays | +| `basalt-ui/data` | mantine-coupled | Convenience barrel pulling both TanStack Table + Virtual peer groups: BasaltDataTable, BasaltVirtualList (Mantine-rendered) — prefer ./data/table or ./data/virtual for per-feature opt-in | +| `basalt-ui/data/table` | mantine-coupled | BasaltDataTable: a sortable data table over TanStack Table, rendered with Mantine (Mantine-rendered) | +| `basalt-ui/data/virtual` | mantine-coupled | BasaltVirtualList: a windowed virtual list over TanStack Virtual, rendered with Mantine (Mantine-rendered) | +| `basalt-ui/agent` | headless | Headless streaming-chat layer: useAgentStream, aiSdkTransport (recommended default) + edenTransport, isResumable/ResumableAgentTransport (stream-resumption seam), PartList, coalesceParts, the ForeignPart/definePartRenderers/narrowAgentPart open part-registry seam, plus the multi-thread createThreadsStore + useAgentThreadRuns + outcome-resolver seam (Mantine-free) | +| `basalt-ui/agent-chat` | mantine-coupled | Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed (variant/renderRow), ThreadFeedRow (inline-expanding Slack row, lazily mounted + kept mounted), ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props, per-message MessageAffordances, groupConsecutive, and an optional virtualize/height windowing mode whose VirtualizeOptions carry overscan/estimateSize/initialScroll — a virtualized transcript opens scrolled to the newest message unless initialScroll is "start"), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx, and @tanstack/react-virtual the same way through the lazy import() behind ThreadTranscript virtualize (absent peer degrades to an unwindowed, height-bound pane). | +| `basalt-ui/content` | mantine-coupled | Prose (article/chat typography), CodeBlock (shiki, optional peer), Callout, TableOfContents, ReadingProgress, Markdown (react-markdown + remark-gfm, optional peers; `streaming` is a rendering mode ONLY — `contentTrust` is the independent security input, and any surface rendering agent/model output must pin `contentTrust="untrusted"`, the sole input to the image-origin allowlist; a `fenceRenderers` registry — settledOnly/FenceRenderer/FenceRenderers/FenceRenderContext; `sanitizeSchema`, an additions-only SanitizeSchemaExtension merged over BASALT_SANITIZE_SCHEMA via mergeSanitizeSchema; the remend streaming-repair pass is now a lazy optional peer), MermaidDiagram (beautiful-mermaid, optional peer), mdxComponents/createMdxComponents, ArticleLayout (docs-page frame), ArticleCard/ArticleGrid (overview cards), Article model (sortArticles/filterArticles/formatArticleDate), ArticleFilterBar (category/tags filter UI), toArticleActions (Spotlight projector, @mantine/spotlight type-only), GuideLink/GuideDrawer (contextual-help drawer) — the content/prose surface | +| `basalt-ui/state` | headless | createPersistedState (versioned localStorage) + useOnlineStatus — Mantine-free state primitives | +| `basalt-ui/connectivity` | mantine-coupled | ConnectivityProvider (aggregates browser online/offline, React Query onlineManager, SSE, and health-check pings into one status), useConnectivity, and ConnectivityIndicator — auto-mounted by BasaltProvider | ## Hard rules diff --git a/packages/basalt-ui/README.md b/packages/basalt-ui/README.md index 3897a9d..a43b44b 100644 --- a/packages/basalt-ui/README.md +++ b/packages/basalt-ui/README.md @@ -219,31 +219,31 @@ deleted rather than silently surviving into the next real skew. ## Subpath exports -| Subpath | Mantine? | Purpose | -| ------------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `.` | coupled | `BasaltProvider`, `createBasaltTheme` / `baseTheme` / `cssVariablesResolver`, `BasaltShell` + sidebar / mobile-nav / breadcrumbs / page-header, `NavCountBadge`, `SidebarAccount` + the provider-agnostic account contract (`BasaltAccountProps`/`State`/`Actions`), `ThreadWorkspace` + thread-chat components, shell types — the root entry still re-exports the thread-chat components; `./agent-chat` takes the same components standalone, without `BasaltProvider`, the shell, the dashboard composites, or `./connectivity` | -| `./charts` | **free** | visx chart primitives, sparklines, hooks, and token re-exports | -| `./tokens` | **free** | `VX` token refs, `buildPaletteCss`, `defineSeries`, `seriesTokens`, `groupTokens`, `alpha`, `ColorPair` / `SeriesMap` types | -| `./theme-lab` | coupled | `ThemeLabControls`, `applyOverrides`, `loadOverrides`, `COLOR_GROUPS` for live theme inspection | -| `./vite` | — | `basaltViteConfig(opts)` — Vite preset for basalt-ui consumer apps; `basaltAppPlugin(opts)` — PWA head, manifest, and icon metadata derived from the token palette | -| `./guard` | **free** | `checkSource`, `GUARD_RULES`, `Finding` types — the headless theme-guard core | -| `./query` | **free** | `createBasaltQueryClient`, transport-agnostic `unwrap`, lazy `BasaltQueryDevtools` | -| `./router-tanstack` | **free** | TanStack Router bridge: `useBasaltNav` (active route) + `useRouterBreadcrumbs` | -| `./forms` | coupled | Mantine form adapter: `useBasaltForm`, `field`, `FormErrorSummary`, `useFormDraft` (Standard Schema) | -| `./notifications` | coupled | Mantine notifications: `notify` helpers, typed registry, persisted history, `NotificationBell` | -| `./commands` | coupled | Typed command bus + overlay controller, `toSpotlightActions`, `ShortcutsHelp`, `BasaltOverlays` | -| `./data` | coupled | Convenience barrel pulling both peer groups: `BasaltDataTable`, `BasaltVirtualList` (Mantine-rendered) — prefer `./data/table` / `./data/virtual` for per-feature opt-in | -| `./data/table` | coupled | `BasaltDataTable` — sortable data table over TanStack Table, rendered with Mantine | -| `./data/virtual` | coupled | `BasaltVirtualList` — windowed virtual list over TanStack Virtual, rendered with Mantine | -| `./agent` | **free** | Headless streaming layer (`useAgentStream`, `PartList`) + multi-thread `createThreadsStore` / `useAgentThreadRuns` / outcome seam | -| `./agent-chat` | coupled | Mantine-styled thread-chat components over `./agent`: `ThreadWorkspace`, `ThreadFeed`, `ThreadOutcomeCard`, `ThreadDetailPanel`, `Composer`, `ThreadTranscript`, `threadPartRenderers` — **requires `motion`**, not merely an optional peer; see [Requirements](#requirements) | -| `./state` | **free** | `createPersistedState` (versioned localStorage) + `useOnlineStatus` — Mantine-free state primitives | -| `./connectivity` | coupled | `ConnectivityProvider` (aggregates browser/React-Query/SSE/health-check status), `useConnectivity`, `ConnectivityIndicator` — auto-mounted by `BasaltProvider` | -| `./content` | coupled | `Prose`, `CodeBlock`, `Callout`, `TableOfContents`, `ReadingProgress`, `Markdown`, `MermaidDiagram`, `ArticleLayout`, `ArticleCard` / `ArticleGrid`, `GuideLink` / `GuideDrawer`, `mdxComponents` | -| `./styles.css` | — | `@layer basalt` base styles, iOS input safety net, font stack | -| `./tokens.css` | **free** | Prebuilt `--vx-*` stylesheet — the default `buildPaletteCss()` output as a plain file, for a consumer with no bundler, React or Mantine; `basalt-ui tokens:css` re-emits it with a custom scheme selector | -| `./configs/*` | — | Raw toolchain presets — oxlint, oxfmt, tsconfig (base/react-app/node), lefthook | -| `./llms.txt` | — | Machine-readable surface map — one entry per published subpath with import specifier, description, layer, and optional peers | +| Subpath | Mantine? | Purpose | +| ------------------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `.` | coupled | `BasaltProvider`, `createBasaltTheme` / `baseTheme` / `cssVariablesResolver`, `BasaltShell` + sidebar / mobile-nav / breadcrumbs / page-header, `NavCountBadge`, `SidebarAccount` + the provider-agnostic account contract (`BasaltAccountProps`/`State`/`Actions`), `ThreadWorkspace` + thread-chat components, shell types — the root entry still re-exports the thread-chat components; `./agent-chat` takes the same components standalone, without `BasaltProvider`, the shell, the dashboard composites, or `./connectivity` | +| `./charts` | **free** | visx chart primitives, sparklines, hooks, and token re-exports | +| `./tokens` | **free** | `VX` token refs, `buildPaletteCss`, `defineSeries`, `seriesTokens`, `groupTokens`, `alpha`, `ColorPair` / `SeriesMap` types | +| `./theme-lab` | coupled | `ThemeLabControls`, `applyOverrides`, `loadOverrides`, `COLOR_GROUPS` for live theme inspection | +| `./vite` | — | `basaltViteConfig(opts)` — Vite preset for basalt-ui consumer apps; `basaltAppPlugin(opts)` — PWA head, manifest, and icon metadata derived from the token palette | +| `./guard` | **free** | `checkSource`, `GUARD_RULES`, `Finding` types — the headless theme-guard core | +| `./query` | **free** | `createBasaltQueryClient`, transport-agnostic `unwrap`, lazy `BasaltQueryDevtools` | +| `./router-tanstack` | **free** | TanStack Router bridge: `useBasaltNav` (active route) + `useRouterBreadcrumbs` | +| `./forms` | coupled | Mantine form adapter: `useBasaltForm`, `field`, `FormErrorSummary`, `useFormDraft` (Standard Schema) | +| `./notifications` | coupled | Mantine notifications: `notify` helpers, typed registry, persisted history, `NotificationBell` | +| `./commands` | coupled | Typed command bus + overlay controller, `toSpotlightActions`, `ShortcutsHelp`, `BasaltOverlays` | +| `./data` | coupled | Convenience barrel pulling both peer groups: `BasaltDataTable`, `BasaltVirtualList` (Mantine-rendered) — prefer `./data/table` / `./data/virtual` for per-feature opt-in | +| `./data/table` | coupled | `BasaltDataTable` — sortable data table over TanStack Table, rendered with Mantine | +| `./data/virtual` | coupled | `BasaltVirtualList` — windowed virtual list over TanStack Virtual, rendered with Mantine | +| `./agent` | **free** | Headless streaming layer (`useAgentStream`, `PartList`) + multi-thread `createThreadsStore` / `useAgentThreadRuns` / outcome seam | +| `./agent-chat` | coupled | Mantine-styled thread-chat components over `./agent`: `ThreadWorkspace`, `ThreadFeed` (`variant`/`renderRow`), `ThreadFeedRow` (inline-expanding Slack row, lazily mounted + kept mounted), `ThreadOutcomeCard`, `ThreadDetailPanel`, `Composer`, `ThreadTranscript` (`groupConsecutive`/`affordances`/`virtualize` — windowed transcripts open at the newest message unless `initialScroll: 'start'`; `virtualize` is enabled by the optional `@tanstack/react-virtual` peer, absent it degrades to an unwindowed, height-bound pane), `threadPartRenderers` — **requires `motion`**, not merely an optional peer; see [Requirements](#requirements) | +| `./state` | **free** | `createPersistedState` (versioned localStorage) + `useOnlineStatus` — Mantine-free state primitives | +| `./connectivity` | coupled | `ConnectivityProvider` (aggregates browser/React-Query/SSE/health-check status), `useConnectivity`, `ConnectivityIndicator` — auto-mounted by `BasaltProvider` | +| `./content` | coupled | `Prose`, `CodeBlock`, `Callout`, `TableOfContents`, `ReadingProgress`, `Markdown`, `MermaidDiagram`, `ArticleLayout`, `ArticleCard` / `ArticleGrid`, `GuideLink` / `GuideDrawer`, `mdxComponents` | +| `./styles.css` | — | `@layer basalt` base styles, iOS input safety net, font stack | +| `./tokens.css` | **free** | Prebuilt `--vx-*` stylesheet — the default `buildPaletteCss()` output as a plain file, for a consumer with no bundler, React or Mantine; `basalt-ui tokens:css` re-emits it with a custom scheme selector | +| `./configs/*` | — | Raw toolchain presets — oxlint, oxfmt, tsconfig (base/react-app/node), lefthook | +| `./llms.txt` | — | Machine-readable surface map — one entry per published subpath with import specifier, description, layer, and optional peers | Named exports only — no default exports. @@ -609,22 +609,25 @@ Wire the drift gate to catch doctrine falling behind after a basalt-ui upgrade: ## Requirements -| Peer | Version | Notes | -| ----------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `react` / `react-dom` | `^19` | required | -| `@mantine/core` | `^9.3` | required | -| `@mantine/hooks` | `^9.3` | required | -| `@tanstack/react-query` | — | required for the root `.` entry — `BasaltProvider` hard-requires it at build time; NOT required by `./agent-chat`, which doesn't touch it | -| `motion` | `12.42.0` | required for both the root `.` entry and `./agent-chat` — both export `ThreadFeed`/`ThreadDetailPanel` (`agent-chat/thread-feed.tsx` / `thread-detail-panel.tsx`), which import `motion/react` eagerly; the root's `ThemeToggle` also uses it | +| Peer | Version | Notes | +| ------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `react` / `react-dom` | `^19` | required | +| `@mantine/core` | `^9.3` | required | +| `@mantine/hooks` | `^9.3` | required | +| `@tanstack/react-query` | — | required for the root `.` entry — `BasaltProvider` hard-requires it at build time; NOT required by `./agent-chat`, which doesn't touch it | +| `motion` | `12.42.0` | required for both the root `.` entry and `./agent-chat` — both export `ThreadFeed`/`ThreadDetailPanel` (`agent-chat/thread-feed.tsx` / `thread-detail-panel.tsx`), which import `motion/react` eagerly; the root's `ThemeToggle` also uses it | +| `@tanstack/react-virtual` | `>=3.13.26 <4` | optional for `./agent-chat` — enables `ThreadTranscript`'s `virtualize` windowing mode via a lazy `import()`; absent it degrades to an unwindowed, height-bound pane | `remend` was required through 1.10.x — `content/markdown.tsx` imported it at the top of the module, so both the root `.` entry and `./agent-chat`'s `ThreadWorkspace`/`ThreadTranscript` hard-required it transitively. It is now a genuinely optional, lazy peer (dynamic `import()`); see [`./content`](#content--prose--markdown) below. -`./agent-chat`'s `optionalPeers` in `llms.txt`/`surfaces.ts` lists `remend` and `motion` (npm has no -per-subpath optionality in `peerDependenciesMeta`) — only `motion` needs to be treated as required -when installing just this subpath; `remend` is accurately optional there too now. +`./agent-chat`'s `optionalPeers` in `llms.txt`/`surfaces.ts` lists `remend`, `motion`, and +`@tanstack/react-virtual` (npm has no per-subpath optionality in `peerDependenciesMeta`) — only +`motion` needs to be treated as required when installing just this subpath; `remend` and +`@tanstack/react-virtual` are accurately optional there too, both reached only through a lazy +`import()` (the latter behind `ThreadTranscript`'s `virtualize` option). Optional peer batteries and their packages are listed per battery above. diff --git a/packages/basalt-ui/agent/rules/basalt-agent.md b/packages/basalt-ui/agent/rules/basalt-agent.md index 53b6a07..96902eb 100644 --- a/packages/basalt-ui/agent/rules/basalt-agent.md +++ b/packages/basalt-ui/agent/rules/basalt-agent.md @@ -257,6 +257,20 @@ composer) on the right, collapsing to a single pane below 768px. The lower-level `threadPartRenderers`) are exported too for bespoke layouts. Motion (feed insert, panel slide) runs on the shared `MOTION_*` tokens and honours `useReducedMotion`. +`ThreadFeed` also takes a `variant` (`'outcome'`, the default above, or `'inline'` for +`ThreadFeedRow`, the Slack-style row that expands in place instead of opening a separate detail +panel — lazily mounted on first expand, then kept mounted and hidden via CSS on every collapse +after that) and a `renderRow` override that takes priority over `variant` entirely, for full control +over live-run wiring (`onSend`/`onStop`/`liveParts`/`liveStatus`) the built-in `'inline'` row +doesn't expose on its own; wire a real `onSend` on `ThreadFeed` itself to make the built-in row's +composer usable, or omit it and the row's composer renders disabled rather than a live control that +silently discards input. `ThreadTranscript` also takes `groupConsecutive` (suppresses role +label/chrome on same-speaker runs), a per-message `affordances` contract (timestamp/copy/regenerate/ +custom actions), and an optional `virtualize`/`height` windowing mode for very long threads. A +windowed transcript owns its own scroll node (never nest it in `BasaltStickToBottom`) and scrolls +itself to the newest message once on mount — `virtualize={{ initialScroll: 'start' }}` opts out, and +is what a consumer restoring its own saved scroll position wants so the two don't fight. + **Boundary:** the headless layer (`createThreadsStore`, `useAgentThreadRuns`, the outcome types) stays Mantine-free in `./agent`; the components are Mantine-coupled and ship from `./agent-chat` (also re-exported from the root entry). Never add `@mantine/*` under `src/agent/**` — it is diff --git a/packages/basalt-ui/configs/oxlint-plugin.js b/packages/basalt-ui/configs/oxlint-plugin.js index 66ae088..ecc695a 100644 --- a/packages/basalt-ui/configs/oxlint-plugin.js +++ b/packages/basalt-ui/configs/oxlint-plugin.js @@ -372,9 +372,13 @@ const OVERFLOW_KEYS = new Set(['overflow', 'overflowY']) const SCROLLING_VALUES = new Set(['auto', 'scroll']) /** - * Reports a `style` object property that turns a node into its own scroll container. Warning-level - * by design: whether a raw scroll box is wrong depends on who owns the scroll node, which no AST - * check can see — so this steers rather than blocks, and `theme-allow` opts out. + * Reports a `style` object property that turns a node into its own scroll container. Whether a raw + * scroll box is wrong depends on who owns the scroll node, which no AST check can see — so the + * `theme-allow` comment is a first-class part of the rule, not an escape valve for exceptional + * cases: a component that legitimately owns its scroll node (`BasaltStickToBottom`, + * `BasaltVirtualList`, `ThreadTranscript`'s virtualized pane) declares that ownership with the + * comment and moves on. Severity went `off` → `warn` (1.12.0) → `error` (1.13.0) once every live + * site in the repo carried that declaration; the opt-out mechanism is identical at either level. */ const rawScrollContainer = { meta: { diff --git a/packages/basalt-ui/configs/oxlint-plugin.test.ts b/packages/basalt-ui/configs/oxlint-plugin.test.ts index 12b3d9b..dc9817c 100644 --- a/packages/basalt-ui/configs/oxlint-plugin.test.ts +++ b/packages/basalt-ui/configs/oxlint-plugin.test.ts @@ -417,21 +417,21 @@ describe('basalt/ai-sdk-major', () => { }) }) -// ── raw-scroll-container — promotion (off → warn shipped) ──────────────────── +// ── raw-scroll-container — promotion (off → warn → error shipped) ─────────── describe('basalt/raw-scroll-container promotion', () => { - it('ships "warn" (not "off") in the consumer preset', () => { + it('ships "error" (not "warn"/"off") in the consumer preset', () => { const shipped = JSON.parse( readFileSync(resolve(import.meta.dirname, 'oxlint.json'), 'utf8'), ) as { rules: Record } - expect(shipped.rules['basalt/raw-scroll-container']).toBe('warn') + expect(shipped.rules['basalt/raw-scroll-container']).toBe('error') }) - it('stays "warn" repo-local', () => { + it('stays at the shipped level ("error") repo-local', () => { const repoLocal = JSON.parse( readFileSync(resolve(import.meta.dirname, '..', '..', '..', '.oxlintrc.json'), 'utf8'), ) as { rules: Record } - expect(repoLocal.rules['basalt/raw-scroll-container']).toBe('warn') + expect(repoLocal.rules['basalt/raw-scroll-container']).toBe('error') }) it('still flags a raw overflow:auto style property at the shipped level', () => { @@ -447,3 +447,32 @@ describe('basalt/raw-scroll-container promotion', () => { expect(rules).toContain('raw-scroll-container') }) }) + +// ── agent rules — promotion (warn → error shipped) ────────────────────────── +// The three agent-chat rules were promoted warn → error in the SHIPPED preset alongside +// raw-scroll-container above, but had no equivalent lock — `scripts/gen-oxlint.ts` only regenerates +// the `overrides` array; this top-level `rules` block is hand-maintained and +// `tests/oxlint-preset-sync.test.ts` only checks `overrides` against `projectBanList('shipped')`, so +// a later accidental revert of one of these three back to "warn" would pass the suite silently. + +describe('basalt agent rules promotion', () => { + it.each(['basalt/agent-resume-guard', 'basalt/agent-no-raw-usechat', 'basalt/ai-sdk-major'])( + '%s ships "error" (not "warn"/"off") in the consumer preset', + (rule) => { + const shipped = JSON.parse( + readFileSync(resolve(import.meta.dirname, 'oxlint.json'), 'utf8'), + ) as { rules: Record } + expect(shipped.rules[rule]).toBe('error') + }, + ) + + it.each(['basalt/agent-resume-guard', 'basalt/agent-no-raw-usechat', 'basalt/ai-sdk-major'])( + '%s stays at the shipped level ("error") repo-local', + (rule) => { + const repoLocal = JSON.parse( + readFileSync(resolve(import.meta.dirname, '..', '..', '..', '.oxlintrc.json'), 'utf8'), + ) as { rules: Record } + expect(repoLocal.rules[rule]).toBe('error') + }, + ) +}) diff --git a/packages/basalt-ui/configs/oxlint.json b/packages/basalt-ui/configs/oxlint.json index 1a19b43..6bceeab 100644 --- a/packages/basalt-ui/configs/oxlint.json +++ b/packages/basalt-ui/configs/oxlint.json @@ -20,12 +20,12 @@ "basalt/raw-size-literal": "warn", "basalt/card-inset": "error", "basalt/chart-in-raw-surface": "error", - "basalt/raw-scroll-container": "warn", + "basalt/raw-scroll-container": "error", "basalt/visx-boundary": "error", "basalt/visx-tooltip": "error", - "basalt/agent-resume-guard": "warn", - "basalt/agent-no-raw-usechat": "warn", - "basalt/ai-sdk-major": "warn" + "basalt/agent-resume-guard": "error", + "basalt/agent-no-raw-usechat": "error", + "basalt/ai-sdk-major": "error" }, "overrides": [ { diff --git a/packages/basalt-ui/llms.txt b/packages/basalt-ui/llms.txt index 4b35aec..ed62a08 100644 --- a/packages/basalt-ui/llms.txt +++ b/packages/basalt-ui/llms.txt @@ -96,7 +96,7 @@ Import: import { ... } from 'basalt-ui/data' Description: Convenience barrel pulling both TanStack Table + Virtual peer groups: BasaltDataTable, BasaltVirtualList (Mantine-rendered) — prefer ./data/table or ./data/virtual for per-feature opt-in Layer: mantine-coupled Rule: basalt-data -OptionalPeers: @tanstack/react-table@>=8 <9, @tanstack/react-virtual@>=3 <4 +OptionalPeers: @tanstack/react-table@>=8 <9, @tanstack/react-virtual@>=3.13.26 <4 ## basalt-ui/data/table Import: import { ... } from 'basalt-ui/data/table' @@ -110,7 +110,7 @@ Import: import { ... } from 'basalt-ui/data/virtual' Description: BasaltVirtualList: a windowed virtual list over TanStack Virtual, rendered with Mantine (Mantine-rendered) Layer: mantine-coupled Rule: basalt-data -OptionalPeers: @tanstack/react-virtual@>=3 <4 +OptionalPeers: @tanstack/react-virtual@>=3.13.26 <4 ## basalt-ui/agent Import: import { ... } from 'basalt-ui/agent' @@ -121,10 +121,10 @@ OptionalPeers: ai@^7.0.15, use-stick-to-bottom@^1.1.6 ## basalt-ui/agent-chat Import: import { ... } from 'basalt-ui/agent-chat' -Description: Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed, ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx. +Description: Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed (variant/renderRow), ThreadFeedRow (inline-expanding Slack row, lazily mounted + kept mounted), ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props, per-message MessageAffordances, groupConsecutive, and an optional virtualize/height windowing mode whose VirtualizeOptions carry overscan/estimateSize/initialScroll — a virtualized transcript opens scrolled to the newest message unless initialScroll is "start"), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx, and @tanstack/react-virtual the same way through the lazy import() behind ThreadTranscript virtualize (absent peer degrades to an unwindowed, height-bound pane). Layer: mantine-coupled Rule: basalt-agent -OptionalPeers: ai@^7.0.15, motion@12.42.0, remend@1.3.0, use-stick-to-bottom@^1.1.6, react-markdown@^10.1.0, remark-gfm@^4.0.1, shiki@^4.3.0, @shikijs/langs@^4.3.1, @shikijs/themes@^4.3.1, beautiful-mermaid@^1.1.0 +OptionalPeers: ai@^7.0.15, motion@12.42.0, remend@1.3.0, use-stick-to-bottom@^1.1.6, react-markdown@^10.1.0, remark-gfm@^4.0.1, shiki@^4.3.0, @shikijs/langs@^4.3.1, @shikijs/themes@^4.3.1, beautiful-mermaid@^1.1.0, @tanstack/react-virtual@>=3.13.26 <4 ## basalt-ui/content Import: import { ... } from 'basalt-ui/content' diff --git a/packages/basalt-ui/package.json b/packages/basalt-ui/package.json index ef04b9c..5002c42 100644 --- a/packages/basalt-ui/package.json +++ b/packages/basalt-ui/package.json @@ -190,7 +190,7 @@ "@tanstack/react-query-devtools": "^5.101.0", "@tanstack/react-router": "^1.170.0", "@tanstack/react-table": ">=8 <9", - "@tanstack/react-virtual": ">=3 <4", + "@tanstack/react-virtual": ">=3.13.26 <4", "@visx/axis": "4.0.0", "@visx/curve": "4.0.0", "@visx/event": "4.0.0", diff --git a/packages/basalt-ui/scripts/export-surface.json b/packages/basalt-ui/scripts/export-surface.json index 97e69dc..1a6693d 100644 --- a/packages/basalt-ui/scripts/export-surface.json +++ b/packages/basalt-ui/scripts/export-surface.json @@ -27,6 +27,7 @@ "ThemeToggle", "ThreadDetailPanel", "ThreadFeed", + "ThreadFeedRow", "ThreadOutcomeCard", "ThreadTranscript", "ThreadWorkspace", @@ -272,6 +273,7 @@ "Composer", "ThreadDetailPanel", "ThreadFeed", + "ThreadFeedRow", "ThreadOutcomeCard", "ThreadTranscript", "ThreadWorkspace", diff --git a/packages/basalt-ui/scripts/pack-test.sh b/packages/basalt-ui/scripts/pack-test.sh index ce3cc70..53d0aac 100755 --- a/packages/basalt-ui/scripts/pack-test.sh +++ b/packages/basalt-ui/scripts/pack-test.sh @@ -75,7 +75,7 @@ bun add "$ABS_TGZ" \ @mantine/form @mantine/notifications @mantine/spotlight @mantine/modals \ "@tanstack/react-query@^5.101.0" "@tanstack/react-query-devtools@^5.101.0" \ "@tanstack/react-router@^1.170.0" \ - "@tanstack/react-table@>=8" "@tanstack/react-virtual@>=3" \ + "@tanstack/react-table@>=8 <9" "@tanstack/react-virtual@>=3.13.26 <4" \ "react-markdown@^10.1.0" "remark-gfm@^4.0.1" \ "use-stick-to-bottom@^1.1.6" \ vite \ diff --git a/packages/basalt-ui/src/agent-chat/index.ts b/packages/basalt-ui/src/agent-chat/index.ts index 8d8a86a..9de4a19 100644 --- a/packages/basalt-ui/src/agent-chat/index.ts +++ b/packages/basalt-ui/src/agent-chat/index.ts @@ -19,6 +19,18 @@ export type { ThreadWorkspaceProps } from './thread-workspace' export { ThreadFeed } from './thread-feed' export type { ThreadFeedProps } from './thread-feed' +// ── ThreadFeedRow ───────────────────────────────────────────────────────────── +export { ThreadFeedRow } from './thread-feed-row' +export type { ThreadFeedRowProps } from './thread-feed-row' + +// ── Shared transcript/row contracts ─────────────────────────────────────────── +// Type-only. Both are named in the PUBLIC props of components exported above +// (`ThreadTranscriptProps.affordances`, `ThreadFeedRowProps`'s virtualize union), so a consumer +// that wants to hold one in a typed variable needs to be able to name it. No runtime export here — +// the resolution/defaults live inside the components. +export type { MessageAffordances } from './message-affordances' +export type { VirtualizeOptions, VirtualizeProps } from './virtualize' + // ── ThreadOutcomeCard ───────────────────────────────────────────────────────── export { ThreadOutcomeCard } from './thread-outcome-card' export type { ThreadOutcomeCardProps } from './thread-outcome-card' diff --git a/packages/basalt-ui/src/agent-chat/message-affordances.ts b/packages/basalt-ui/src/agent-chat/message-affordances.ts new file mode 100644 index 0000000..d6712c6 --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/message-affordances.ts @@ -0,0 +1,61 @@ +/** + * MessageAffordances — the per-message hover-row/action contract shared by `ThreadTranscript` and + * a virtualized `ThreadFeedRow` (both render `MessageBlock`, so the affordance surface lives here + * rather than being redeclared on each consumer). + * + * Type-only module — the rendering (the actual hover row, copy button, regenerate button) lives in + * `thread-message.tsx`. `MessageAffordances` itself IS public (re-exported, type-only, from the + * agent-chat barrel and the root): it is named in `ThreadTranscriptProps.affordances` and + * `ThreadFeedRowProps.affordances`, so a consumer has to be able to name it. `DEFAULT_AFFORDANCES` + * stays internal — resolving unset fields is the components' job, not the consumer's. + */ +import type { ReactNode } from 'react' +import type { ChatMessage, TranscriptPart } from '../agent' + +export type MessageAffordances = { + /** + * How the per-message timestamp renders. + * @default 'relative' + */ + readonly timestamp?: 'relative' | 'absolute' | 'none' + /** + * Whether a copy action is offered on the hover row. Copies the message's COALESCED text (the + * same merged/de-duplicated text `coalesceParts` produces for display), not the raw `parts` + * array — a message can accumulate multiple adjacent/by-id text parts while streaming, and + * copying the raw parts would either duplicate text or copy fragments the user never saw + * assembled. + * @default true + */ + readonly copy?: boolean + /** + * Regenerate action, shown on the LAST ASSISTANT message only (never on earlier turns, never on + * user messages). Receives that message's `id`. `useAgentThreadRuns`'s `retry` is + * THREAD-keyed (`retry: (threadId: string) => void`), replaying the last user input for the + * whole thread — the consumer bridges this messageId-keyed callback to that thread-keyed retry; + * the framework only ever hands back the messageId, it does not know about threads. + */ + readonly onRegenerate?: (messageId: string) => void + /** + * Extra actions appended to the hover row, after the built-in copy/regenerate actions. + * + * This is a RENDER prop, not an event handler — it is called during render, once per message, + * and its return value is rendered. That means it is treated differently from `onRegenerate`: + * `onRegenerate` is identity-stabilized internally (a fresh inline literal every render costs + * nothing), while `actions` is compared by REFERENCE, because its reference is the only signal + * that its output may have changed. A fresh inline `actions={() => …}` literal therefore + * re-renders every message block on every render of the transcript. That is correct — an action + * whose output depends on consumer state (a pin/star toggle) must be allowed to update — but on + * a long thread it is worth avoiding: wrap `actions` in the consumer's own `useCallback`, keyed + * on whatever state it actually reads. + */ + readonly actions?: (ctx: { readonly message: ChatMessage }) => ReactNode +} + +/** + * Resolved defaults for the optional {@link MessageAffordances} fields — one place, so + * `ThreadTranscript` and a virtualized `ThreadFeedRow` cannot drift on what "unset" means. + */ +export const DEFAULT_AFFORDANCES: Required> = { + timestamp: 'relative', + copy: true, +} diff --git a/packages/basalt-ui/src/agent-chat/relative-time.test.ts b/packages/basalt-ui/src/agent-chat/relative-time.test.ts new file mode 100644 index 0000000..f2d640f --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/relative-time.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, test } from 'bun:test' +import { formatRelativeTime } from './relative-time' + +// Unit boundaries (ms) mirrored from relative-time.ts — kept in sync deliberately rather than +// importing the private table, so a drift between the two shows up as a test failure. +const YEAR_MS = 31_536_000_000 +const MONTH_MS = 2_628_000_000 +const WEEK_MS = 604_800_000 +const DAY_MS = 86_400_000 +const HOUR_MS = 3_600_000 +const MINUTE_MS = 60_000 + +describe('formatRelativeTime', () => { + test('sub-minute past → "just now"', () => { + expect(formatRelativeTime(Date.now() - 1)).toBe('just now') + expect(formatRelativeTime(Date.now() - 30_000)).toBe('just now') + expect(formatRelativeTime(Date.now() - (MINUTE_MS - 1_000))).toBe('just now') + }) + + test('sub-minute future → "just now" (the threshold is symmetric on |diff|)', () => { + expect(formatRelativeTime(Date.now() + 1)).toBe('just now') + expect(formatRelativeTime(Date.now() + (MINUTE_MS - 1))).toBe('just now') + }) + + test('minute boundary', () => { + expect(formatRelativeTime(Date.now() - MINUTE_MS)).toBe('1 minute ago') + }) + + test('hour boundary', () => { + expect(formatRelativeTime(Date.now() - HOUR_MS)).toBe('1 hour ago') + }) + + test('day boundary', () => { + expect(formatRelativeTime(Date.now() - DAY_MS)).toBe('yesterday') + }) + + test('week boundary', () => { + expect(formatRelativeTime(Date.now() - WEEK_MS)).toBe('last week') + }) + + test('month boundary', () => { + expect(formatRelativeTime(Date.now() - MONTH_MS)).toBe('last month') + }) + + test('year boundary', () => { + expect(formatRelativeTime(Date.now() - YEAR_MS)).toBe('last year') + }) + + test('future timestamp: the function handles negative diffMs (diff = timestamp - Date.now())', () => { + // diffMs is positive here (timestamp is ahead of "now"), so Math.round(diffMs / unit.ms) is + // positive too, and Intl.RelativeTimeFormat's 'auto' numeric mode renders the "in X" / "next + // X" / "tomorrow" forms rather than the "X ago" / "last X" forms used for the past. + expect(formatRelativeTime(Date.now() + HOUR_MS)).toBe('in 1 hour') + expect(formatRelativeTime(Date.now() + DAY_MS)).toBe('tomorrow') + expect(formatRelativeTime(Date.now() + WEEK_MS)).toBe('next week') + }) + + test('non-finite/wrong-type inputs degrade to empty string instead of throwing', () => { + // Intl.RelativeTimeFormat.format throws a RangeError on any of these. The pre-fix code called + // it unguarded, so each of the following would have thrown out of formatRelativeTime — except + // `null`, which coerces to 0 via `timestamp - Date.now()` and produced a nonsense-but-non- + // throwing "X years ago" string. The guard now normalizes all six to the same '' result. + expect(formatRelativeTime(NaN)).toBe('') + expect(formatRelativeTime(Infinity)).toBe('') + expect(formatRelativeTime(-Infinity)).toBe('') + // @ts-expect-error — runtime type violation (undefined at a `number`-typed call site), the exact + // shape a `ThreadsStoreAdapter` bug or corrupted localStorage JSON would produce. + expect(formatRelativeTime(undefined)).toBe('') + // @ts-expect-error — runtime type violation (ISO string), the single most likely real-world + // adapter mistake (a server returning `created_at` as a string). + expect(formatRelativeTime('2024-01-01T00:00:00Z')).toBe('') + // @ts-expect-error — runtime type violation (null); previously coerced to 0 rather than + // throwing, now normalized to the same '' as every other non-finite input. + expect(formatRelativeTime(null)).toBe('') + }) + + test('finite-but-absurd timestamps still format instead of throwing', () => { + // Number.isFinite is true for both, so these fall through the new guard unchanged — the guard + // must not over-trigger on merely-extreme (but valid) input. + expect(() => formatRelativeTime(8_640_000_000_000_000)).not.toThrow() // year 275760, JS Date max + expect(() => formatRelativeTime(-8_640_000_000_000_000)).not.toThrow() // symmetric min + expect(() => formatRelativeTime(-1_000_000_000_000)).not.toThrow() // pre-1970 epoch + }) + + test('"at(-1)!" fallback path — documented as effectively unreachable', () => { + // RELATIVE_TIME_UNITS.find(...) ?? RELATIVE_TIME_UNITS.at(-1)! only falls through to the + // fallback when .find returns undefined. The array's last (smallest) entry is 'minute' at + // 60_000ms, exactly the threshold the "just now" early-return already guards below — so for + // every absMs that reaches the .find call (absMs >= 60_000), 'minute' itself always matches + // first-or-later, and the ?? branch is defensive dead code under the current threshold table. + // This test exercises the boundary that the fallback would produce if it were ever reachable + // (it currently is not, verified by construction): the same 'minute' unit as the last table row. + expect(formatRelativeTime(Date.now() - MINUTE_MS)).toBe('1 minute ago') + }) +}) diff --git a/packages/basalt-ui/src/agent-chat/relative-time.ts b/packages/basalt-ui/src/agent-chat/relative-time.ts new file mode 100644 index 0000000..b239818 --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/relative-time.ts @@ -0,0 +1,55 @@ +/** + * formatRelativeTime — dependency-free relative-time formatting (no date-fns). + * + * Shared between `ThreadOutcomeCard` (inbox row timestamps) and `ThreadTranscript` (per-message + * timestamps). 'en'-hardcoded for now — see the module-level note below for the deferred locale + * seam. + * + * Internal to `agent-chat/` — not part of the public barrel. + */ + +const RELATIVE_TIME_FORMAT = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }) + +const RELATIVE_TIME_UNITS: readonly { + readonly unit: Intl.RelativeTimeFormatUnit + readonly ms: number +}[] = [ + { unit: 'year', ms: 31_536_000_000 }, + { unit: 'month', ms: 2_628_000_000 }, + { unit: 'week', ms: 604_800_000 }, + { unit: 'day', ms: 86_400_000 }, + { unit: 'hour', ms: 3_600_000 }, + { unit: 'minute', ms: 60_000 }, +] + +/** + * Formats an epoch-ms timestamp as a short relative string ("3 hours ago", "just now"). + * + * A non-finite `timestamp` — `NaN`, `±Infinity`, or a non-number value that slipped past the + * `ChatMessage.createdAt: number` type at runtime (an ISO string or `undefined`/`null` from a + * hand-rolled `ThreadsStoreAdapter`, or from JSON-deserialized `localStorage` state) renders as an + * empty string rather than reaching `Intl.RelativeTimeFormat.format`, which throws a `RangeError` + * on any non-finite input. This is render-path code — one bad `createdAt` on one message must not + * blank the whole transcript (the standing rule `spliceText`/`coalesceParts` already hold) — so it + * degrades instead of throwing, and warns in dev, matching `mergePart`'s clamp-and-warn precedent + * (`clampOffset` in `../agent/merge.ts`). The warning is NOT deduplicated, also matching that + * precedent — but note the call sites differ: `clampOffset` runs once per wire event, whereas this + * runs during render, once per message, so a single bad `createdAt` in a long transcript warns + * once per message per render. That is loud on purpose (a `createdAt` that isn't a number is an + * adapter bug worth fixing, not worth muting), and it costs nothing in production. + */ +export function formatRelativeTime(timestamp: number): string { + if (!Number.isFinite(timestamp)) { + if (process.env['NODE_ENV'] !== 'production') { + console.warn( + `[basalt] formatRelativeTime: non-finite timestamp ${String(timestamp)} — rendering empty string`, + ) + } + return '' + } + const diffMs = timestamp - Date.now() + const absMs = Math.abs(diffMs) + if (absMs < 60_000) return 'just now' + const unit = RELATIVE_TIME_UNITS.find(({ ms }) => absMs >= ms) ?? RELATIVE_TIME_UNITS.at(-1)! + return RELATIVE_TIME_FORMAT.format(Math.round(diffMs / unit.ms), unit.unit) +} diff --git a/packages/basalt-ui/src/agent-chat/thread-feed-row.test.tsx b/packages/basalt-ui/src/agent-chat/thread-feed-row.test.tsx new file mode 100644 index 0000000..edede77 --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/thread-feed-row.test.tsx @@ -0,0 +1,510 @@ +/** + * ThreadFeedRow — the lazy-mount / keep-mounted invariant this component exists to guarantee + * (AGENT-CHAT-SPEC.md §12): never render the transcript before the first expand, never unmount it + * after. The `effectFireCount` probe is what makes the "no effect re-fire" half of that provable — + * a re-mount would bump it, a CSS-only hide/show never does. + */ +import { MantineProvider } from '@mantine/core' +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { useEffect } from 'react' +import { definePartRenderers } from '../agent' +import type { + AgentThread, + ChatMessage, + ForeignPart, + PartRenderer, + PartRenderers, + TranscriptPart, +} from '../agent' +import { ThreadFeedRow } from './thread-feed-row' + +afterEach(cleanup) + +function buildThread(parts: TranscriptPart[]): AgentThread { + return { + id: 'thread-1', + messages: [{ id: 'm1', role: 'assistant', parts, createdAt: 0 }], + outcome: { title: 'A resolved thread', summary: 'Summary text', status: 'done' }, + status: 'done', + read: true, + createdAt: 0, + updatedAt: 0, + } +} + +let effectFireCount = 0 + +/** Mounted once per real mount of the subtree it lives in — proves whether a hide/show cycle + * re-mounted it (increments again) or merely hid it with CSS (stays put). */ +function EffectProbe(): null { + useEffect(() => { + effectFireCount += 1 + }, []) + return null +} + +const probeRenderers: PartRenderers = definePartRenderers({ + probe: () => , +}) + +/** Renders whatever `part.type` reached it — used to prove `fallbackRenderer` crossed the row's + * seam into `ThreadTranscript`, since a dropped prop falls back to the built-in dev-only chip + * instead (a different testid, not this one). */ +const rowFallbackRenderer: PartRenderer = ({ part }) => ( + {part.type} +) + +function renderRow(expanded: boolean) { + const thread = buildThread([{ id: 'p1', type: 'probe' }]) + return render( + + {}} + renderers={probeRenderers} + onSend={() => {}} + /> + , + ) +} + +describe('ThreadFeedRow — lazy mount, kept mounted', () => { + test('collapsed initially: the transcript body is not in the DOM', () => { + renderRow(false) + + expect(screen.queryByTestId('thread-feed-row-body')).toBeNull() + }) + + test('expanding mounts the transcript body', () => { + const { rerender } = render( + + {}} + renderers={probeRenderers} + onSend={() => {}} + /> + , + ) + expect(screen.queryByTestId('thread-feed-row-body')).toBeNull() + + rerender( + + {}} + renderers={probeRenderers} + onSend={() => {}} + /> + , + ) + + expect(screen.getByTestId('thread-feed-row-body')).toBeDefined() + }) + + test('collapsing again keeps the subtree mounted, hidden via CSS only', () => { + const thread = buildThread([{ id: 'p1', type: 'probe' }]) + const { rerender } = render( + + {}} + renderers={probeRenderers} + onSend={() => {}} + /> + , + ) + expect(screen.getByTestId('thread-feed-row-body')).toBeDefined() + + rerender( + + {}} + renderers={probeRenderers} + onSend={() => {}} + /> + , + ) + + // Still present — a `queryByTestId`-returns-null assertion here would be the "unmounted" + // behavior this test exists to rule out. + const body = screen.getByTestId('thread-feed-row-body') + expect(body).toBeDefined() + expect(body.style.display).toBe('none') + }) + + test('re-expanding after a collapse does NOT re-fire the subtree effect', () => { + effectFireCount = 0 + const thread = buildThread([{ id: 'p1', type: 'probe' }]) + const props = { + thread, + onToggle: () => {}, + renderers: probeRenderers, + onSend: () => {}, + } as const + + const { rerender } = render( + + + , + ) + expect(effectFireCount).toBe(1) + + rerender( + + + , + ) + rerender( + + + , + ) + + // A re-mount (the defect this invariant guards against) would bump this to 2. + expect(effectFireCount).toBe(1) + }) + + test('clicking the header calls onToggle with the thread id', () => { + // A plain `let` narrows to `null` at the assertion below (TS can't prove the callback ran) — + // a mutable holder object sidesteps that, since object property writes aren't narrowed the + // same way across the closure boundary. + const toggled: { id: string | null } = { id: null } + const thread: AgentThread = { + ...buildThread([{ id: 'p1', type: 'text', text: 'hi' }]), + outcome: null, + } + render( + + { + toggled.id = id + }} + onSend={() => {}} + /> + , + ) + + screen.getByText('Untitled thread') + screen.getByRole('button').click() + + expect(toggled.id).toBe('thread-1') + }) +}) + +// ── The composer half of `liveStatus` ──────────────────────────────────────────────────────── +// `onStop` was documented ("shown as the composer's Stop action while liveStatus === 'streaming'") +// but never forwarded together with the `streaming` flag `Composer` actually gates on +// (`composer.tsx`'s `showStop`/`inputDisabled`) — so Stop never appeared and a second turn could +// still be typed into a live thread. These assert the composer itself reflects `liveStatus`. + +describe('ThreadFeedRow — composer reflects liveStatus', () => { + test('liveStatus="streaming" with onStop shows Stop instead of Send, and calls onStop', () => { + let stops = 0 + render( + + {}} + liveStatus="streaming" + onStop={() => { + stops += 1 + }} + onSend={() => {}} + /> + , + ) + + expect(screen.queryByLabelText('Send message')).toBeNull() + fireEvent.click(screen.getByLabelText('Stop generating')) + expect(stops).toBe(1) + }) + + test('liveStatus="streaming" disables the composer textarea', () => { + render( + + {}} + liveStatus="streaming" + onStop={() => {}} + onSend={() => {}} + /> + , + ) + + expect((screen.getByRole('textbox') as HTMLTextAreaElement).disabled).toBe(true) + }) + + test('liveStatus="done" (or unset) shows Send, not Stop, and leaves the textarea enabled', () => { + render( + + {}} + liveStatus="done" + onStop={() => {}} + onSend={() => {}} + /> + , + ) + + expect(screen.getByLabelText('Send message')).toBeDefined() + expect(screen.queryByLabelText('Stop generating')).toBeNull() + expect((screen.getByRole('textbox') as HTMLTextAreaElement).disabled).toBe(false) + }) +}) + +// ── The row → transcript seam ──────────────────────────────────────────────────────────────── +// `ThreadFeedRow` and `ThreadTranscript` were built in parallel: the row declared the shared +// contract (`affordances`, and later `groupConsecutive`/`virtualize`) while the transcript grew the +// props that consume it. Each half was correct alone and the pair still compiled with the row +// silently DROPPING all three on the floor — a defect no type or single-component test can see. +// These assert the forwarding itself: every one of them goes red if a prop stops being passed +// through, which is exactly the failure that shipped. + +function threadWith(messages: ChatMessage[]): AgentThread { + return { ...buildThread([]), messages } +} + +const T0 = 1_000_000_000 + +function textMessage( + id: string, + role: ChatMessage['role'], + createdAt: number, +): ChatMessage { + return { id, role, parts: [{ id: `${id}-p1`, type: 'text', text: `body ${id}` }], createdAt } +} + +describe('ThreadFeedRow forwards the transcript contract it declares', () => { + test('affordances reach the row’s messages — a custom action renders inside the body', () => { + render( + + {}} + affordances={{ actions: () => extra }} + onSend={() => {}} + /> + , + ) + + // Dropping the prop leaves the DEFAULT affordance row (timestamp + copy) rendering perfectly — + // which is why the assertion is on the CONSUMER-supplied action, the one thing that cannot + // appear unless `affordances` actually crossed the seam. + expect(screen.getByTestId('row-custom-action')).toBeDefined() + }) + + test('affordances’ onRegenerate reaches the last assistant message', () => { + const regenerated: { id: string | null } = { id: null } + render( + + {}} + affordances={{ + onRegenerate: (id) => { + regenerated.id = id + }, + }} + onSend={() => {}} + /> + , + ) + + screen.getByText('Regenerate').click() + + expect(regenerated.id).toBe('fm2') + }) + + test('liveParts + liveStatus reach the transcript as an in-flight live message', () => { + render( + + {}} + liveParts={[{ id: 'live-p1', type: 'text', text: 'partial reply' }]} + liveStatus="streaming" + onSend={() => {}} + /> + , + ) + + // `ThreadTranscript` synthesizes a `__live__` message from `liveParts` only when it actually + // receives both props — dropping either leaves this testid absent. + expect(screen.getByTestId('agent-message-__live__')).toBeDefined() + expect(screen.getByText('partial reply')).toBeDefined() + }) + + test('renderers reaches the transcript — a registered part type renders the consumer renderer', () => { + const rowRenderers: PartRenderers = definePartRenderers({ + 'row-custom': () => custom, + }) + + render( + + {}} + renderers={rowRenderers} + onSend={() => {}} + /> + , + ) + + expect(screen.getByTestId('row-custom-part')).toBeDefined() + }) + + test('fallbackRenderer reaches the transcript for an unregistered foreign part', () => { + render( + + {}} + fallbackRenderer={rowFallbackRenderer} + onSend={() => {}} + /> + , + ) + + expect(screen.getByTestId('row-fallback-part').textContent).toBe('unregistered-foreign-type') + }) + + test('composerProps reaches the row’s Composer', () => { + render( + + {}} + onSend={() => {}} + composerProps={{ placeholder: 'row-composer-placeholder' }} + /> + , + ) + + expect(screen.getByPlaceholderText('row-composer-placeholder')).toBeDefined() + }) + + test('groupConsecutive={false} reaches the transcript', () => { + const messages = [ + textMessage('fm1', 'assistant', T0), + textMessage('fm2', 'assistant', T0 + 60_000), + ] + + render( + + {}} + onSend={() => {}} + /> + , + ) + // Default (`true`, forwarded as `undefined` and defaulted inside the transcript): the second + // same-role message inside the 5-minute window groups. + expect(screen.getByTestId('agent-message-fm2').getAttribute('data-grouped')).toBe('true') + + // A fresh mount, not a `rerender` — the first tree is torn down here, so the second render + // starts from a clean DOM and `getByTestId` can't match the stale row. + cleanup() + render( + + {}} + groupConsecutive={false} + onSend={() => {}} + /> + , + ) + + expect(screen.getByTestId('agent-message-fm2').getAttribute('data-grouped')).toBe('false') + }) +}) + +describe('ThreadFeedRow forwards virtualize/height to the transcript', () => { + let originalOffsetHeight: PropertyDescriptor | undefined + + beforeEach(() => { + originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight') + // Same reason as thread-message.test.tsx's virtualization block: happy-dom has no layout + // engine, so an unpatched offsetHeight of 0 makes TanStack Virtual compute an empty viewport. + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, value: 300 }) + }) + + afterEach(() => { + if (originalOffsetHeight !== undefined) { + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', originalOffsetHeight) + } else { + delete (HTMLElement.prototype as { offsetHeight?: number }).offsetHeight + } + }) + + test('an expanded row with virtualize windows its thread instead of rendering every message', async () => { + const messages = Array.from({ length: 120 }, (_, i) => + // 10 minutes apart — never groups, keeping this orthogonal to the grouping seam above. + textMessage(`vfm${i}`, i % 2 === 0 ? 'user' : 'assistant', T0 + i * 10 * 60_000), + ) + + const { container } = render( + + {}} + virtualize + height={300} + onSend={() => {}} + /> + , + ) + + // Deliberately asserts WINDOWING, not which end is anchored. `waitFor` also rides out the + // lazy `@tanstack/react-virtual` import: until it resolves, the Suspense fallback renders all + // 120 rows unwindowed — the same count a row that DROPPED `virtualize` would render forever, + // which is what makes the timeout a real failure rather than a slow pass. + await waitFor(() => { + const rendered = container.querySelectorAll('[data-testid^="agent-message-"]').length + expect(rendered).toBeGreaterThan(0) + expect(rendered).toBeLessThan(120) + }) + }) +}) diff --git a/packages/basalt-ui/src/agent-chat/thread-feed-row.tsx b/packages/basalt-ui/src/agent-chat/thread-feed-row.tsx new file mode 100644 index 0000000..fdf77cf --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/thread-feed-row.tsx @@ -0,0 +1,274 @@ +/** + * ThreadFeedRow — the inline-expanding "Slack shape" projection of one `AgentThread`: a header row + * that expands IN PLACE to reveal the thread's live transcript and a composer, instead of opening a + * separate detail pane (compare `ThreadOutcomeCard` + `ThreadDetailPanel`, the inbox variant). + * `ThreadOutcomeCard` is untouched by this — it stays inbox-shaped by design and deliberately never + * renders live text (its module doc, and its `isPreviewing` branch, which shows a skeleton rather + * than the in-flight turn); this is a second, sibling component, not a mode on the first. + * + * LOAD-BEARING INVARIANT — read before touching the expand/collapse mechanics: once a row has been + * expanded for the first time, its transcript+composer subtree mounts LAZILY (nothing renders before + * that first expand) and then STAYS MOUNTED for the rest of the row's lifetime. Collapsing hides it + * with CSS only (`display: none`); it never unmounts. + * + * Do NOT delegate this to Mantine `Collapse`. On the installed `@mantine/core@9.3.0` a bare + * `` happens to do the right thing (children stay mounted, hidden via + * `display: none` from `useCollapse`) — but Mantine master has already flipped the defaults to + * `keepMounted: true` + `keepMountedMode: 'activity'`, which wraps children in React 19 ``. + * `` DESTROYS the subtree's effects and RE-CREATES them on show, so the day + * this repo bumps Mantine, a bare `Collapse` would silently start re-firing every effect in the + * transcript on every re-open — for a streaming transport's subscription/resume effect, a literal + * duplicate stream replay. Own the show/hide here, in plain CSS, so no upstream default change can + * flip the semantics out from under this component. There are two separate guarantees and both are + * required: never render before the first expand (lazy), and never unmount after (kept). + * + * @example + * import { ThreadFeedRow } from 'basalt-ui' + * + * setOpenId((current) => (current === id ? null : id))} + * onSend={(payload) => send(thread.id, payload)} + * /> + */ +import { Box, Group, Stack, Text, UnstyledButton } from '@mantine/core' +import type { JSX } from 'react' +import { useState } from 'react' +import type { + AgentThread, + ForeignPart, + PartRenderer, + PartRenderers, + StreamStatus, + TranscriptPart, +} from '../agent' +import { VX } from '../tokens' +import type { ComposerProps, ComposerSubmit } from './composer' +import { Composer } from './composer' +import type { MessageAffordances } from './message-affordances' +import { formatRelativeTime } from './relative-time' +import { ThreadTranscript } from './thread-message' +import { resolveVirtualize } from './virtualize' +import type { VirtualizeProps } from './virtualize' + +/** A minimal, dependency-free chevron — rotates 90deg when expanded (same inline-svg idiom as + * `composer.tsx`'s `SendGlyph`/`StopGlyph`: no icon-library dependency for a one-off glyph). */ +function ChevronGlyph({ expanded }: { expanded: boolean }): JSX.Element { + return ( + + + + ) +} + +type ThreadFeedRowBase = { + /** + * The thread this row projects. Typed against `AgentThread` — not the plain, + * `AgentPart`-defaulted `AgentThread` `ThreadFeed` uses, and not a type parameter. A consumer + * with a registered `ForeignPart` union passes its own threads straight through with no generic + * argument and no cast, because `TranscriptPart` is the widest part type and arrays are + * structurally covariant: `AgentPart[]` and `ConsumerPart[]` both widen into `TranscriptPart[]`. + * Widening the row is what lets both `ThreadFeed` (non-generic, `AgentThread`) and a + * part-registry consumer feed the same component. + */ + readonly thread: AgentThread + /** Whether this row's transcript+composer body is currently visible. */ + readonly expanded: boolean + /** Called with the thread's id when the header is clicked/activated. */ + readonly onToggle: (id: string) => void + /** The live (in-flight) assistant turn's parts, when a run is streaming for this thread. */ + readonly liveParts?: readonly TranscriptPart[] + /** The live run's status — drives the in-progress indicator on the live block. */ + readonly liveStatus?: StreamStatus + /** Consumer renderers keyed by part.type, forwarded to `ThreadTranscript`. */ + readonly renderers?: PartRenderers + /** Forwarded to `ThreadTranscript` — see its own prop doc. */ + readonly fallbackRenderer?: PartRenderer + /** + * Per-message hover-row contract (timestamp/copy/regenerate/actions), forwarded verbatim to the + * row's `ThreadTranscript`. Shared with it via `message-affordances.ts` so the two cannot drift + * on what an unset field means. + */ + readonly affordances?: MessageAffordances + /** Forwarded to `ThreadTranscript` — suppresses role label + chrome on same-speaker runs. + * @default true */ + readonly groupConsecutive?: boolean + /** Called with the composer's submit payload. */ + readonly onSend: (payload: ComposerSubmit) => void + /** Shown as the composer's Stop action while `liveStatus === 'streaming'`. */ + readonly onStop?: () => void + /** Forwarded to the row's `Composer`, minus the three props this component always wires itself + * (`onSubmit`, `onStop`, and `streaming` — the last derived from `liveStatus` so a consumer can't + * pass one that disagrees with the row's own live status). */ + readonly composerProps?: Omit +} + +/** + * `virtualize`/`height` are forwarded straight to the row's `ThreadTranscript`, carrying the same + * union guard: an inline row holding a very long thread can window it, and doing so REQUIRES a + * `height` (the transcript then owns a fixed-height scroll node inside the expanded body, instead + * of the body growing to the thread's full length). Omit both and the row's transcript is + * content-sized, as before. + */ +export type ThreadFeedRowProps = ThreadFeedRowBase & VirtualizeProps + +/** The row header's title: the resolved outcome title, else a plain placeholder — this row never + * falls back to scanning the first user message the way `ThreadOutcomeCard`'s `promptOf` does, + * since an inline row's own expanded transcript already shows that prompt a scroll away. */ +function rowTitle(thread: AgentThread): string { + return thread.outcome?.title ?? 'Untitled thread' +} + +/** + * One inline-expanding thread row: a header (title + relative timestamp) that toggles a lazily + * mounted, kept-mounted transcript + composer body. See the module doc for the mount lifecycle + * invariant this component exists to guarantee. + * + * @example + * + */ +export function ThreadFeedRow(props: ThreadFeedRowProps): JSX.Element { + const { + thread, + expanded, + onToggle, + liveParts, + liveStatus, + renderers, + fallbackRenderer, + affordances, + groupConsecutive, + onSend, + onStop, + composerProps, + } = props + + // Resolved through the shared narrowing point rather than by destructuring `virtualize`/`height` + // apart — see `resolveVirtualize`'s doc for why the two must be read together. + const virtualized = resolveVirtualize(props) + const virtualizeProps: VirtualizeProps = + virtualized === null ? {} : { virtualize: virtualized.options, height: virtualized.height } + + // Lazy-mount + keep-mounted: flips true on the row's first expand and never resets. Everything + // below this line renders — once — the first time `expanded` becomes true, and stays in the tree + // (hidden via `display` only) for every collapse/expand after that. See the module doc. + // + // Set during render, not a `useEffect` — React's documented "adjusting state as props change" + // pattern (bails out via the `!hasOpened` guard, so this runs at most once per mount). A + // `useEffect` would flip `hasOpened` only AFTER the first expand's render had already committed + // with the body absent, so the header's first expand painted one tick before the body appeared. + // Setting it here makes React restart this render with `hasOpened` already true, so the header + // and the newly-lazy-mounted body commit in the same paint. + const [hasOpened, setHasOpened] = useState(expanded) + if (expanded && !hasOpened) { + setHasOpened(true) + } + + const summary = thread.outcome?.summary ?? '' + + // The composer half of `liveStatus`: `Composer` only shows its Stop action and gates the + // textarea when it is TOLD a run is streaming (`composer.tsx`'s `showStop`/`inputDisabled`) — it + // has no way to infer that from `onStop` alone. Forwarding `onStop` without this was the defect: + // Stop never rendered and a second turn could still be typed into a live thread. + const streaming = liveStatus === 'streaming' + + return ( + + onToggle(thread.id)} + w="100%" + aria-expanded={expanded} + style={{ + display: 'block', + padding: 'var(--mantine-spacing-xs) var(--mantine-spacing-sm)', + }} + > + + + + {rowTitle(thread)} + + {summary.length > 0 && ( + + {summary} + + )} + + + {formatRelativeTime(thread.updatedAt)} + + + + + {hasOpened && ( + + + + + + + )} + + ) +} diff --git a/packages/basalt-ui/src/agent-chat/thread-feed.test.tsx b/packages/basalt-ui/src/agent-chat/thread-feed.test.tsx new file mode 100644 index 0000000..f9deabc --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/thread-feed.test.tsx @@ -0,0 +1,241 @@ +/** + * ThreadFeed — `variant`/`renderRow` (AGENT-CHAT-SPEC.md §12, additive to the existing + * `ThreadOutcomeCard` inbox behaviour). Covers both the reduced-motion and the animated + * (`AnimatePresence`) render branches, since `variant`/`renderRow` have to work identically in + * both — see `thread-feed.tsx`'s module doc for why that split exists in the first place. + */ +import { MantineProvider } from '@mantine/core' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, test } from 'bun:test' +import type { ReactElement } from 'react' +import { useState } from 'react' +import type { AgentThread } from '../agent' +import type { ComposerSubmit } from './composer' +import { ThreadFeed } from './thread-feed' + +afterEach(cleanup) + +function buildThreads(): AgentThread[] { + return [ + { + id: 't1', + messages: [], + outcome: { title: 'First thread', summary: 'Summary one', status: 'done' }, + status: 'done', + read: true, + createdAt: 0, + updatedAt: 0, + }, + ] +} + +/** Temporarily forces `useReducedMotion()`'s result for the duration of `fn` — `@mantine/hooks`' + * `useMediaQuery` reads `window.matchMedia(query).matches` inside its mount effect, which RTL's + * `render` flushes synchronously (wrapped in `act`) before returning. See `use-media-query.mjs`. */ +function withReducedMotion(matches: boolean, fn: () => T): T { + const original = window.matchMedia + window.matchMedia = (query: string): MediaQueryList => + ({ + matches, + media: query, + onchange: null, + addListener: () => {}, + removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, + }) as MediaQueryList + try { + return fn() + } finally { + window.matchMedia = original + } +} + +function renderFeed(ui: ReactElement) { + return render({ui}) +} + +describe.each([ + ['animated', false], + ['reduced motion', true], +])('ThreadFeed row selection (%s)', (_label, reducedMotion) => { + test('variant "outcome" (default) renders ThreadOutcomeCard unchanged', () => { + withReducedMotion(reducedMotion, () => { + renderFeed( {}} />) + }) + + expect(screen.getByText('First thread')).toBeDefined() + expect(screen.getByText('Summary one')).toBeDefined() + }) + + test('variant "inline" renders ThreadFeedRow instead of ThreadOutcomeCard', () => { + withReducedMotion(reducedMotion, () => { + renderFeed( + {}} + variant="inline" + />, + ) + }) + + // ThreadFeedRow renders the same title text, but as a toggle button (role=button) rather than + // ThreadOutcomeCard's selectable row — the observable difference between the two variants. + expect(screen.getByText('First thread')).toBeDefined() + expect(screen.getByRole('button').getAttribute('aria-expanded')).toBe('false') + }) + + test('renderRow overrides BOTH variants', () => { + withReducedMotion(reducedMotion, () => { + renderFeed( + {}} + renderRow={(thread) =>
{thread.id}
} + />, + ) + }) + + expect(screen.getByTestId('custom-t1')).toBeDefined() + // The default outcome-card content must NOT have rendered instead. + expect(screen.queryByText('First thread')).toBeNull() + }) + + test('renderRow overrides the "inline" variant too', () => { + withReducedMotion(reducedMotion, () => { + renderFeed( + {}} + variant="inline" + renderRow={(thread) =>
{thread.id}
} + />, + ) + }) + + expect(screen.getByTestId('custom-t1')).toBeDefined() + expect(screen.queryByRole('button')).toBeNull() + }) +}) + +// ── The inline row's own collapse tracking ─────────────────────────────────────────────────── +// Wiring `onToggle={onSelect}` directly (the previous shape) meant a plain, natural +// `onSelect={setActiveId}` consumer could never collapse a row: re-selecting the same id twice is +// a no-op state update, so `aria-expanded` got stuck at `true` forever. These use exactly that +// natural, non-toggling setter — the trap case — to prove the fix without leaning on a consumer +// that already special-cases re-selection. + +/** A minimal natural consumer: a plain `useState` setter passed straight through as `onSelect`, + * the exact shape the previous wiring broke. */ +function NaiveSingleSelectFeed({ threads }: { threads: AgentThread[] }): ReactElement { + const [activeId, setActiveId] = useState(null) + return ( + + ) +} + +describe('ThreadFeed inline variant — collapse decoupled from onSelect', () => { + test('clicking an unselected row expands it and reports the selection', () => { + render( + + + , + ) + + const header = screen.getByRole('button') + expect(header.getAttribute('aria-expanded')).toBe('false') + + fireEvent.click(header) + + expect(header.getAttribute('aria-expanded')).toBe('true') + }) + + test('clicking the already-selected row collapses it — the defect this fix targets', () => { + render( + + + , + ) + + const header = screen.getByRole('button') + fireEvent.click(header) + expect(header.getAttribute('aria-expanded')).toBe('true') + + // The second click, on the SAME row, with a naive `setActiveId` that turns this into a no-op + // state update — this is exactly the click the previous `onToggle={onSelect}` wiring couldn't + // collapse from. + fireEvent.click(header) + + expect(header.getAttribute('aria-expanded')).toBe('false') + }) + + test('re-clicking after a collapse re-expands the same row', () => { + render( + + + , + ) + + const header = screen.getByRole('button') + fireEvent.click(header) // expand + fireEvent.click(header) // collapse + fireEvent.click(header) // expand again + + expect(header.getAttribute('aria-expanded')).toBe('true') + }) +}) + +// ── The built-in inline row's composer: real send channel vs. visibly inert ───────────────── +// `variant='inline'` used to wire the row's composer to a hardcoded no-op — a live, enabled +// textarea + Send button that silently discarded whatever was typed into it. These assert both +// halves of the fix: `onSend` makes it actually work, and omitting `onSend` renders the composer +// disabled instead of a live dead end. + +describe('ThreadFeed inline variant — composer send channel', () => { + test('onSend is called with the thread and the composer payload on submit', () => { + const sent: { thread: AgentThread | null; payload: ComposerSubmit | null } = { + thread: null, + payload: null, + } + const threads = buildThreads() + + render( + + {}} + variant="inline" + onSend={(thread, payload) => { + sent.thread = thread + sent.payload = payload + }} + /> + , + ) + + const textarea = screen.getByRole('textbox') as HTMLTextAreaElement + expect(textarea.disabled).toBe(false) + + fireEvent.change(textarea, { target: { value: 'hello there' } }) + fireEvent.click(screen.getByLabelText('Send message')) + + expect(sent.thread?.id).toBe('t1') + expect(sent.payload?.text).toBe('hello there') + }) + + test('without onSend, the composer renders disabled rather than a live no-op', () => { + render( + + {}} variant="inline" /> + , + ) + + const textarea = screen.getByRole('textbox') as HTMLTextAreaElement + expect(textarea.disabled).toBe(true) + }) +}) diff --git a/packages/basalt-ui/src/agent-chat/thread-feed.tsx b/packages/basalt-ui/src/agent-chat/thread-feed.tsx index df1ec32..518ba6a 100644 --- a/packages/basalt-ui/src/agent-chat/thread-feed.tsx +++ b/packages/basalt-ui/src/agent-chat/thread-feed.tsx @@ -1,53 +1,158 @@ /** - * ThreadFeed — a scrollable, animated list of `ThreadOutcomeCard` rows for a multi-thread inbox. + * ThreadFeed — a scrollable, animated list of thread rows for a multi-thread inbox. * * Wraps the list in `AnimatePresence` (`popLayout`) so removing/reordering threads reflows the * remaining rows smoothly; each row is a `motion.div` keyed by `thread.id` (never index — index * keys would corrupt the layout animation identity when threads are prepended/removed). Branches * on `useReducedMotion` for a plain, unanimated `Stack`. * + * Row content is picked by `variant` (default `'outcome'`, today's `ThreadOutcomeCard` inbox + * behaviour, unchanged) or `'inline'` (`ThreadFeedRow`, the Slack shape) — or by `renderRow`, which + * takes priority over `variant` entirely when supplied. The built-in `'inline'` row sends through + * `onSend` when supplied; omit it and the row's composer renders visibly disabled rather than + * silently discarding input — see `onSend`'s own doc. + * * @example * import { ThreadFeed } from 'basalt-ui' * * */ -import { ScrollArea, Stack } from '@mantine/core' +import { Box, ScrollArea, Stack } from '@mantine/core' import { useReducedMotion } from '@mantine/hooks' import { AnimatePresence, motion } from 'motion/react' -import type { JSX } from 'react' +import type { JSX, ReactNode } from 'react' +import { useEffect, useState } from 'react' import type { AgentThread } from '../agent' import { MOTION_SPRING } from '../motion' +import type { ComposerSubmit } from './composer' +import { ThreadFeedRow } from './thread-feed-row' import { ThreadOutcomeCard } from './thread-outcome-card' +/** A submit handler for the built-in `'inline'` row when the consumer supplies no `onSend` — paired + * with `composerProps={{ disabled: true }}` so it is unreachable, not silently reachable. See + * `onSend`'s doc for why an inert composer beats a live one that eats input. */ +function noopSend(): void {} + export type ThreadFeedProps = { readonly threads: AgentThread[] /** The currently open thread id, or null when none is selected. */ readonly activeId: string | null - /** Called with a thread's id when its row is selected. */ + /** Called with a thread's id when its row is selected. Never called to report a collapse — see + * `variant`'s doc for how the built-in `'inline'` row's expand/collapse is decoupled from this. */ readonly onSelect: (id: string) => void + /** + * Row shape: `'outcome'` (default) renders today's `ThreadOutcomeCard` inbox row, unchanged. + * `'inline'` renders `ThreadFeedRow`, the Slack-style inline-expanding row, wired to this feed's + * own `activeId` for which row is open (so at most one row is open at a time, mirroring the + * inbox's single-selection model) — with its OWN collapse tracking layered on top, not a raw + * pass-through of `onSelect` as `onToggle`. `onSelect` is documented as a selection event, and + * collapsing a row that remains the active thread isn't one; wiring `onToggle={onSelect}` + * directly (the previous shape) meant a plain `onSelect={setActiveId}` consumer could never + * collapse a row, since re-selecting the same id is a no-op state update. Collapsing here is + * local, UI-only state, reset whenever `activeId` changes to point at a DIFFERENT thread. Since + * this reset is a `useEffect` keyed on `activeId`, re-driving it to the SAME id from outside + * this component is a no-op React dep comparison, so it does not clear the override. + */ + readonly variant?: 'outcome' | 'inline' + /** + * Wires the built-in `'inline'` row's composer to a real send channel — called with the thread + * and the composer's submit payload. Omit it and the row's composer renders disabled (visibly + * inert) instead of a live control that silently discards whatever is typed into it; a consumer + * that needs the composer live must supply this (or `renderRow`, for full control over the row). + */ + readonly onSend?: (thread: AgentThread, payload: ComposerSubmit) => void + /** Overrides row rendering entirely, for BOTH variants — when supplied, this is called for every + * thread instead of either built-in row. Gives full control over live-run wiring + * (`onStop`/`liveParts`/`liveStatus`) the built-in `'inline'` row doesn't expose; see `variant` + * and `onSend`. */ + readonly renderRow?: (thread: AgentThread) => ReactNode +} + +function defaultRow( + thread: AgentThread, + variant: 'outcome' | 'inline', + activeId: string | null, + collapsedId: string | null, + onToggle: (id: string) => void, + onSelect: (id: string) => void, + onSend: ((thread: AgentThread, payload: ComposerSubmit) => void) | undefined, +): ReactNode { + if (variant === 'inline') { + const expanded = thread.id === activeId && thread.id !== collapsedId + return ( + onSend(thread, payload) : noopSend} + {...(onSend === undefined ? { composerProps: { disabled: true } } : {})} + /> + ) + } + return ( + onSelect(thread.id)} + /> + ) } /** - * A scrollable feed of thread rows, animated on add/remove/reorder. Each row renders via - * `ThreadOutcomeCard`; selection highlighting is derived from `activeId`. + * A scrollable feed of thread rows, animated on add/remove/reorder. Row content comes from + * `renderRow` when supplied, else from the built-in `variant` row (`ThreadOutcomeCard` or + * `ThreadFeedRow`) — see `ThreadFeedProps` for the full contract. * * @example * select(id)} /> */ -export function ThreadFeed({ threads, activeId, onSelect }: ThreadFeedProps): JSX.Element { +export function ThreadFeed({ + threads, + activeId, + onSelect, + variant = 'outcome', + onSend, + renderRow, +}: ThreadFeedProps): JSX.Element { const reduceMotion = useReducedMotion() + // The inline row's own manual-collapse override — see `ThreadFeedProps.variant`'s doc. Only ever + // meaningful for the id it names; a stale value left over for a since-deselected thread is inert + // (the per-row `expanded` check below always requires `thread.id === activeId` too). + const [collapsedId, setCollapsedId] = useState(null) + // Any externally-driven move of `activeId` to a DIFFERENT thread clears a stale override. React + // bails on a same-value dep, so re-driving `activeId` to the id it already holds (e.g. the SAME + // thread re-selected after being manually collapsed) does NOT re-run this effect. A collapse + // triggered by this component's own header click does NOT touch `activeId` at all (see + // `handleInlineToggle`), so it does not re-trigger this effect either. + useEffect(() => { + setCollapsedId(null) + }, [activeId]) + + function handleInlineToggle(id: string): void { + const isOpen = id === activeId && id !== collapsedId + if (isOpen) { + // Collapsing the already-selected row is a visual-only action. `onSelect` is documented as a + // SELECTION event; a row that remains the active thread while visually collapsed hasn't had + // its selection change, so this does not call it. + setCollapsedId(id) + return + } + setCollapsedId(null) + onSelect(id) + } + + const rowFor = (thread: AgentThread): ReactNode => + renderRow !== undefined + ? renderRow(thread) + : defaultRow(thread, variant, activeId, collapsedId, handleInlineToggle, onSelect, onSend) + if (reduceMotion) { return ( {threads.map((thread) => ( - onSelect(thread.id)} - /> + {rowFor(thread)} ))} @@ -67,11 +172,7 @@ export function ThreadFeed({ threads, activeId, onSelect }: ThreadFeedProps): JS exit={{ opacity: 0, scale: 0.97 }} transition={MOTION_SPRING} > - onSelect(thread.id)} - /> + {rowFor(thread)} ))} diff --git a/packages/basalt-ui/src/agent-chat/thread-message.test.tsx b/packages/basalt-ui/src/agent-chat/thread-message.test.tsx index 894a90d..30b7b33 100644 --- a/packages/basalt-ui/src/agent-chat/thread-message.test.tsx +++ b/packages/basalt-ui/src/agent-chat/thread-message.test.tsx @@ -7,11 +7,26 @@ * boundary in a controlled render-count harness. */ import { MantineProvider } from '@mantine/core' -import { cleanup, render, screen } from '@testing-library/react' -import { afterEach, describe, expect, test } from 'bun:test' +import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from 'bun:test' import { definePartRenderers } from '../agent' import type { ChatMessage, TranscriptPart } from '../agent' -import { messageBlockRenderCounter, ThreadTranscript } from './thread-message' +import { + applyInitialScroll, + MAX_INITIAL_SCROLL_ATTEMPTS, + messageBlockRenderCounter, + nonVirtualizedRowsFallback, + resolveGuardedMeasurement, + resolveInitialScrollAction, + ThreadTranscript, +} from './thread-message' +import type { InitialScrollState } from './thread-message' + +// `nonVirtualizedRowsFallback` is a plain function component but its export name (this module's +// test-only-escape-hatch convention, matching `messageBlockRenderCounter`) is lowercase-first, so +// JSX would treat `` as a native DOM tag rather than a component +// reference. Alias it to a capitalized local binding for JSX use below. +const VirtualizeFallback = nonVirtualizedRowsFallback afterEach(cleanup) @@ -40,6 +55,22 @@ function buildMessages(count: number): ChatMessage[] { }) } +/** Stubs `navigator.clipboard.writeText`, returning the array of copied strings. happy-dom does + * not implement the Clipboard API at all. */ +function stubClipboard(): string[] { + const calls: string[] = [] + Object.defineProperty(window.navigator, 'clipboard', { + configurable: true, + value: { + writeText: (text: string) => { + calls.push(text) + return Promise.resolve() + }, + }, + }) + return calls +} + describe('resolution order', () => { test('a registered consumer renderer wins over the built-in union', () => { const renderers = definePartRenderers({ @@ -348,4 +379,907 @@ describe('MessageBlock memoization (AGENT-CHAT-SPEC.md §9)', () => { expect(messageBlockRenderCounter.count).toBe(1) }) + + test('flipping `groupConsecutive` re-renders only the messages whose `grouped` value actually flips', () => { + const t0 = 1_000_000_000 + const messages: ChatMessage[] = [ + // No predecessor — `grouped` is false either way, this block must NOT re-render. + { id: 'g0', role: 'user', parts: [{ id: 'g0-p1', type: 'text', text: 'q' }], createdAt: t0 }, + // Different-role predecessor — `grouped` is false either way, must NOT re-render. + { + id: 'g1', + role: 'assistant', + parts: [{ id: 'g1-p1', type: 'text', text: 'a' }], + createdAt: t0 + 1_000, + }, + // Same-role, 1 minute after g1 — `grouped` flips true -> false. The ONLY block that must + // re-render. + { + id: 'g2', + role: 'assistant', + parts: [{ id: 'g2-p1', type: 'text', text: 'b' }], + createdAt: t0 + 60_000, + }, + ] + + const { rerender } = render( + + + , + ) + messageBlockRenderCounter.count = 0 + + // Same `messages` array + every message object reference unchanged — only the boolean prop + // flips. + rerender( + + + , + ) + + expect(messageBlockRenderCounter.count).toBe(1) + }) + + test('a consumer passing a fresh `affordances` object literal each render does not force a re-render', () => { + const messages = buildMessages(10) + const { rerender } = render( + + + , + ) + messageBlockRenderCounter.count = 0 + + // A BRAND NEW object literal, same field values, same `messages` reference — nothing has + // actually changed from any consumer's point of view. + rerender( + + + , + ) + + expect(messageBlockRenderCounter.count).toBe(0) + }) +}) + +function singleMessage(createdAt: number): ChatMessage[] { + return [ + { + id: 'aff1', + role: 'assistant', + parts: [{ id: 'aff1-p1', type: 'text', text: 'hello there' }], + createdAt, + }, + ] +} + +/** Stands in for a consumer's own `useCallback`-wrapped `actions` — see the memo-bail-out test. */ +const STABLE_ACTIONS = (): null => null + +describe('per-message affordances (AGENT-CHAT-SPEC.md §11)', () => { + test('timestamp: relative (the default) renders a relative label', () => { + render( + + + , + ) + + expect(within(screen.getByTestId('message-affordances-aff1')).getByText(/ago/)).toBeDefined() + }) + + test('timestamp: absolute renders a locale date/time string, not a relative one', () => { + const createdAt = new Date('2024-01-01T12:00:00Z').getTime() + render( + + + , + ) + + const row = within(screen.getByTestId('message-affordances-aff1')) + expect(row.getByText(new Date(createdAt).toLocaleString())).toBeDefined() + expect(row.queryByText(/ago/)).toBeNull() + }) + + test('timestamp: none renders no timestamp text at all', () => { + render( + + + , + ) + + expect(within(screen.getByTestId('message-affordances-aff1')).queryByText(/ago/)).toBeNull() + }) + + test('copy copies the message COALESCED text, not a naive join of the raw parts', async () => { + const calls = stubClipboard() + const messages: ChatMessage[] = [ + { + id: 'aff2', + role: 'assistant', + parts: [ + { id: 'p1', type: 'text', text: 'Hello' }, + // A source part splits the run — a non-text part sitting between two text parts. Naive + // `parts.map(p => p.text).join('')` would blow up on `.text` not existing here; the + // coalesced-segment walk skips it and joins the two text runs with a blank line. + { id: 'p2', type: 'source', url: 'https://example.com' }, + { id: 'p3', type: 'text', text: 'World' }, + ], + createdAt: Date.now(), + }, + ] + + render( + + + , + ) + + fireEvent.click(screen.getByLabelText('Copy message')) + + await waitFor(() => expect(calls).toEqual(['Hello\n\nWorld'])) + }) + + test('regenerate appears on the LAST assistant message only, and calls back with its id', () => { + const calls: string[] = [] + const now = Date.now() + const messages: ChatMessage[] = [ + { id: 'u1', role: 'user', parts: [{ id: 'u1-p1', type: 'text', text: 'q' }], createdAt: now }, + { + id: 'a1', + role: 'assistant', + parts: [{ id: 'a1-p1', type: 'text', text: 'first reply' }], + createdAt: now + 1_000, + }, + { + id: 'u2', + role: 'user', + parts: [{ id: 'u2-p1', type: 'text', text: 'q2' }], + createdAt: now + 2_000, + }, + { + id: 'a2', + role: 'assistant', + parts: [{ id: 'a2-p1', type: 'text', text: 'second reply' }], + createdAt: now + 3_000, + }, + ] + + render( + + calls.push(messageId) }} + /> + , + ) + + expect(screen.getAllByText('Regenerate')).toHaveLength(1) + const lastRow = within(screen.getByTestId('message-affordances-a2')) + fireEvent.click(lastRow.getByText('Regenerate')) + expect(calls).toEqual(['a2']) + }) + + test('custom actions render alongside the built-in affordances, on every message', () => { + render( + + , + }} + /> + , + ) + + expect(screen.getByText('pin-aff1')).toBeDefined() + }) + + test('the live/streaming message never renders an affordance row', () => { + render( + + + , + ) + + expect(screen.queryByTestId('message-affordances-__live__')).toBeNull() + expect(screen.queryByLabelText('Copy message')).toBeNull() + }) + + test('renders no affordance row at all when every affordance resolves to off', () => { + render( + + + , + ) + + // No onRegenerate (so no regenerate control, even though this IS the last assistant message) + // and no custom `actions` either — nothing for the row to show, so `MessageAffordanceRow` + // renders `null` rather than an empty strip. + expect(screen.queryByTestId('message-affordances-aff1')).toBeNull() + }) + + test('a fresh onRegenerate literal each render does not force a re-render either', () => { + const messages = buildMessages(10) + const { rerender } = render( + + id }} /> + , + ) + messageBlockRenderCounter.count = 0 + + // A brand-new handler literal every render (the realistic consumer shape this finding names) — + // `useStableCallback` must absorb this so `resolvedAffordances`'s reference stays put. Note + // this covers `onRegenerate` ONLY: `actions` is a render prop and is deliberately compared by + // reference (see the two tests below, and `MessageAffordances.actions`'s own doc). + rerender( + + id }} /> + , + ) + + expect(messageBlockRenderCounter.count).toBe(0) + }) + + test('a consumer `actions` render prop reflects the consumer state it closes over', () => { + // The counterweight to the test above, and the reason `actions` is exempt from + // `useStableCallback`: `actions` is invoked DURING render to produce nodes, so identity- + // stabilizing it would freeze `resolvedAffordances` → `settledRows` → the very JSX element + // objects React reconciles, and the consumer's control would render its first-render output + // for the transcript's whole lifetime. This is the shape any real pin/star/bookmark action + // takes. + const messages = singleMessage(Date.now()) + const { rerender } = render( + + pinned: no }} + /> + , + ) + + expect(screen.getByTestId('pin').textContent).toBe('pinned: no') + + rerender( + + pinned: yes }} + /> + , + ) + + expect(screen.getByTestId('pin').textContent).toBe('pinned: yes') + }) + + test('a REFERENCE-STABLE `actions` still gets the memo bail-out', () => { + // The documented opt-out: a consumer that wraps `actions` in its own `useCallback` (modelled + // here by a module-scope constant) keeps the whole `affordances`-by-reference bail-out, so + // exempting `actions` costs nothing for a consumer that cares about it. + const messages = buildMessages(10) + const { rerender } = render( + + + , + ) + messageBlockRenderCounter.count = 0 + + rerender( + + + , + ) + + expect(messageBlockRenderCounter.count).toBe(0) + }) +}) + +describe('affordance row visibility — hover OR focus-within (a11y)', () => { + test('a descendant holding focus reveals the row, not just mouse hover', () => { + render( + + + , + ) + + const row = screen.getByTestId('message-affordances-aff1') + expect(row.style.opacity).toBe('0') + + // The row stays mounted regardless of visibility (see `MessageAffordanceRowProps.visible`'s + // doc), so its Copy control is focusable even before hover/focus reveals it. `focusIn` (not + // `focus`, which does not bubble) is what `useFocusWithin`'s underlying `addEventListener` + // actually listens for — matching how a real Tab keypress reaches focus in the browser. + fireEvent.focusIn(screen.getByLabelText('Copy message')) + + expect(row.style.opacity).toBe('1') + + fireEvent.focusOut(screen.getByLabelText('Copy message')) + + expect(row.style.opacity).toBe('0') + }) +}) + +describe('groupConsecutive — the Slack rhythm (AGENT-CHAT-SPEC.md §11, default true)', () => { + const T0 = 1_000_000_000 + + function pair( + deltaMs: number, + roleA: ChatMessage['role'] = 'assistant', + roleB: ChatMessage['role'] = 'assistant', + ): ChatMessage[] { + return [ + { + id: 'gp1', + role: roleA, + parts: [{ id: 'gp1-p1', type: 'text', text: 'first' }], + createdAt: T0, + }, + { + id: 'gp2', + role: roleB, + parts: [{ id: 'gp2-p1', type: 'text', text: 'second' }], + createdAt: T0 + deltaMs, + }, + ] + } + + test('same role within 5 minutes groups — the second message drops its role label and chrome', () => { + render( + + + , + ) + + expect(screen.getAllByText('Assistant')).toHaveLength(1) + expect(screen.getByTestId('agent-message-gp2').getAttribute('data-grouped')).toBe('true') + }) + + test('same role at exactly the 5-minute boundary still groups', () => { + render( + + + , + ) + + expect(screen.getAllByText('Assistant')).toHaveLength(1) + }) + + test('same role at 5 minutes + 1ms does NOT group', () => { + render( + + + , + ) + + expect(screen.getAllByText('Assistant')).toHaveLength(2) + expect(screen.getByTestId('agent-message-gp2').getAttribute('data-grouped')).toBe('false') + }) + + test('different roles never group, regardless of how close in time', () => { + render( + + + , + ) + + expect(screen.getByText('You')).toBeDefined() + expect(screen.getByText('Assistant')).toBeDefined() + }) + + test('the first message never groups — there is no predecessor', () => { + render( + + + , + ) + + expect(screen.getByTestId('agent-message-gp1').getAttribute('data-grouped')).toBe('false') + }) + + test('groupConsecutive={false} disables grouping entirely', () => { + render( + + + , + ) + + expect(screen.getAllByText('Assistant')).toHaveLength(2) + }) +}) + +/** A minimal fake matching `MeasurementCacheHost`'s structural shape — no ResizeObserver, no + * layout engine, no `@tanstack/react-virtual` import at all needed to prove the guard branch. */ +function fakeMeasurementHost({ + cached, + estimate = 999, +}: { readonly cached?: number; readonly estimate?: number } = {}) { + const itemSizeCache = new Map() + if (cached !== undefined) itemSizeCache.set('row-key', cached) + return { + indexFromElement: () => 0, + itemSizeCache, + options: { + getItemKey: () => 'row-key', + estimateSize: () => estimate, + }, + } +} + +describe('resolveGuardedMeasurement — a 0 ResizeObserver reading is never a real row measurement', () => { + const element = document.createElement('div') + + test('a genuine (non-zero) measurement passes through unchanged', () => { + const host = fakeMeasurementHost({ cached: 40 }) + expect(resolveGuardedMeasurement(host, element, 145)).toBe(145) + }) + + test('a 0 reading with a cached size falls back to the LAST-KNOWN size, not 0 — this is the exact mechanism that stops a collapsed virtualized row from poisoning itemSizeCache', () => { + const host = fakeMeasurementHost({ cached: 220 }) + expect(resolveGuardedMeasurement(host, element, 0)).toBe(220) + }) + + test('a 0 reading with no prior measurement (first mount) falls back to estimateSize, not 0', () => { + const host = fakeMeasurementHost({ estimate: 160 }) + expect(resolveGuardedMeasurement(host, element, 0)).toBe(160) + }) +}) + +describe('resolveInitialScrollAction — the pure decision behind "a virtualized transcript opens at the newest message"', () => { + test('"start" resolves to skip-done regardless of row count or container height', () => { + expect(resolveInitialScrollAction('start', 50, 300)).toBe('skip-done') + expect(resolveInitialScrollAction('start', 0, 0)).toBe('skip-done') + }) + + test('an empty transcript resolves to skip-done even when targeting "end" — nothing to scroll to', () => { + expect(resolveInitialScrollAction('end', 0, 300)).toBe('skip-done') + }) + + test('a non-positive container height resolves to skip-retry, NOT skip-done — a hidden ancestor must not permanently strand the transcript at the top', () => { + expect(resolveInitialScrollAction('end', 50, 0)).toBe('skip-retry') + expect(resolveInitialScrollAction('end', 50, -1)).toBe('skip-retry') + }) + + test('a genuinely measured, non-empty, "end"-targeted mount resolves to scroll', () => { + expect(resolveInitialScrollAction('end', 50, 300)).toBe('scroll') + }) +}) + +describe('applyInitialScroll — the ACROSS-CALLS guard that makes the jump fire once, ever', () => { + // A fake `scrollToEnd` is the boundary here rather than a real DOM `Element.scrollTo` spy on + // purpose: a real `Virtualizer`'s own `followOnAppend` ALSO calls `scrollTo` on a later append + // (verified against the installed virtual-core 3.17.1 — `setOptions`'s edge-key-change branch), + // making that call indistinguishable from this effect's own at the DOM boundary. `applyInitialScroll` + // is the exact function `VirtualizedRowsInner`'s effect calls, so this proves the real guard, not + // a hand-mirrored copy of it. `isAtEnd` is likewise a fake rather than a real DOM `scrollTop` read + // — this describe block proves the STATE MACHINE (fire → verify → settle-or-retry, bounded), not + // the real browser race the fix exists for, which happy-dom cannot observe (no layout engine — + // see the `initialScroll` describe block below for what IS provable against the real component). + + test('fires once, stays unsettled until a later call confirms it via isAtEnd, then never fires again', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + const isAtEnd = mock(() => true) + + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + // Not yet permanent: firing is not landing. Verification happens on the NEXT call. + expect(state.hasApplied).toBe(false) + + // Simulates the effect re-running on every later streamed append (rows.length keeps growing) + // — the exact scenario that must NOT re-trigger the jump once it has settled. + applyInitialScroll(state, 'end', 61, () => 300, scrollToEnd, isAtEnd) + expect(state.hasApplied).toBe(true) + + applyInitialScroll(state, 'end', 62, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + }) + + test('re-fires when isAtEnd reports the jump was reverted before settling — the clobber-recovery path', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + let atEnd = false + const isAtEnd = () => atEnd + + // First attempt: fires, unconfirmed. + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + + // Next commit: the DOM reports NOT at the end (something else clobbered `scrollTop` in the + // meantime, e.g. virtual-core's own `_willUpdate` anchor branch) — re-fires rather than + // accepting the reverted position as final. + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(2) + expect(state.hasApplied).toBe(false) + + // The DOM now holds: settles on the next confirming call. + atEnd = true + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(2) + expect(state.hasApplied).toBe(true) + + // A further call must not fire again. + applyInitialScroll(state, 'end', 61, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(2) + }) + + test('gives up after MAX_INITIAL_SCROLL_ATTEMPTS rather than retrying forever against a DOM that never confirms', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + // never confirms — the pathological case this bound exists for + const isAtEnd = mock(() => false) + + for (let i = 0; i < 20; i += 1) { + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + } + + expect(state.hasApplied).toBe(true) + expect(scrollToEnd).toHaveBeenCalledTimes(MAX_INITIAL_SCROLL_ATTEMPTS) + + // Once given up, further calls are true no-ops. + applyInitialScroll(state, 'end', 61, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(MAX_INITIAL_SCROLL_ATTEMPTS) + }) + + test('"start" marks done WITHOUT ever calling scrollToEnd — mutation-proof for the skip-done branch not silently becoming skip-retry', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + const isAtEnd = mock(() => false) + + applyInitialScroll(state, 'start', 60, () => 300, scrollToEnd, isAtEnd) + + expect(scrollToEnd).not.toHaveBeenCalled() + expect(isAtEnd).not.toHaveBeenCalled() + expect(state.hasApplied).toBe(true) + }) + + test('an empty transcript marks done WITHOUT ever calling scrollToEnd', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + const isAtEnd = mock(() => false) + + applyInitialScroll(state, 'end', 0, () => 300, scrollToEnd, isAtEnd) + + expect(scrollToEnd).not.toHaveBeenCalled() + expect(isAtEnd).not.toHaveBeenCalled() + expect(state.hasApplied).toBe(true) + }) + + test('a hidden (0px) container retries on a later call instead of being permanently stranded, then settles once confirmed', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + const isAtEnd = mock(() => true) + + // First attempt: mounted behind a `display: none` ancestor — measures 0. + applyInitialScroll(state, 'end', 60, () => 0, scrollToEnd, isAtEnd) + expect(scrollToEnd).not.toHaveBeenCalled() + expect(state.hasApplied).toBe(false) + + // Second attempt — the row was re-expanded, so the SAME row count now measures. Deliberately + // NOT a changed count: the commit that un-hides a `ThreadFeedRow` body changes nothing the + // effect could key a dependency array on, which is why that effect has none. Fires, unconfirmed. + applyInitialScroll(state, 'end', 60, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + expect(state.hasApplied).toBe(false) + + // Third attempt (next commit): confirmed settled. + applyInitialScroll(state, 'end', 61, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + expect(state.hasApplied).toBe(true) + + // A fourth attempt must not fire again. + applyInitialScroll(state, 'end', 62, () => 300, scrollToEnd, isAtEnd) + expect(scrollToEnd).toHaveBeenCalledTimes(1) + }) + + test('the container-height read happens at most once per mount — verification never touches it, only isAtEnd', () => { + const state: InitialScrollState = { hasApplied: false, attempts: 0 } + const scrollToEnd = mock(() => {}) + const getContainerHeight = mock(() => 300) + const isAtEnd = mock(() => true) + + applyInitialScroll(state, 'end', 60, getContainerHeight, scrollToEnd, isAtEnd) + expect(getContainerHeight).toHaveBeenCalledTimes(1) + expect(isAtEnd).not.toHaveBeenCalled() + + // Verification call — reads `isAtEnd`, not `getContainerHeight` (no forced `offsetHeight` + // layout once the row count / container height have already done their one job). + applyInitialScroll(state, 'end', 61, getContainerHeight, scrollToEnd, isAtEnd) + expect(getContainerHeight).toHaveBeenCalledTimes(1) + expect(isAtEnd).toHaveBeenCalledTimes(1) + expect(state.hasApplied).toBe(true) + + // Every later commit (one per streamed chunk on a live thread) re-runs the dependency-free + // effect — a settled transcript must touch neither thunk. + applyInitialScroll(state, 'end', 62, getContainerHeight, scrollToEnd, isAtEnd) + expect(getContainerHeight).toHaveBeenCalledTimes(1) + expect(isAtEnd).toHaveBeenCalledTimes(1) + }) +}) + +function manyMessages(count: number): ChatMessage[] { + const t0 = 1_000_000_000 + return Array.from({ length: count }, (_, i) => ({ + id: `vm${i}`, + role: i % 2 === 0 ? 'user' : 'assistant', + // 10 minutes apart — never groups, keeping this describe block orthogonal to grouping. + parts: [{ id: `vm${i}-p1`, type: 'text', text: `message ${i}` }], + createdAt: t0 + i * 10 * 60_000, + })) +} + +// `virtual-core` itself unconditionally calls `scrollTo({ top: 0, behavior: undefined })` the +// moment ANY virtualizer first attaches its scroll element (`_willUpdate`'s +// `this.scrollElement !== scrollElement` branch, verified against the installed 3.17.1 source) — +// that call exists with or without this feature and is not evidence of anything. The initial-scroll +// effect's own call is `scrollToEnd({ behavior: 'auto' })`, so an explicit `behavior: 'auto'` +// argument is what's genuinely diagnostic — and, on the very FIRST mount (before any append has +// ever run `setOptions` again), it can only have come from that effect: `followOnAppend`'s own +// `behavior: 'auto'` re-scroll is gated on `prevOptions !== undefined`, i.e. it never fires on +// construction. (A later append's `followOnAppend` call is indistinguishable from the effect's own +// at this DOM boundary — the `applyInitialScroll` describe block above proves the once-ever +// guarantee with a fake `scrollToEnd` instead, precisely because of that ambiguity.) +// Typed structurally rather than as `ReturnType`: `spyOn` is generic over the object +// and key being spied on, so referencing its return type with the type arguments unresolved leaves +// `mock.calls` uninferrable and every callback parameter an implicit `any` (a tsc error under this +// package's strict config — `bun test` alone never sees it). +function autoBehaviorCalls(spy: { + readonly mock: { readonly calls: readonly unknown[][] } +}): number { + return spy.mock.calls.filter( + (call) => (call[0] as { behavior?: string } | undefined)?.behavior === 'auto', + ).length +} + +describe('virtualization (AGENT-CHAT-SPEC.md §9)', () => { + let originalOffsetHeight: PropertyDescriptor | undefined + + beforeEach(() => { + originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'offsetHeight') + // happy-dom has no layout engine — every element's offsetHeight is always 0, which makes + // TanStack Virtual compute a zero-height viewport and render nothing at all. A fixed value + // gives it a real, deterministic non-degenerate viewport to compute a visible window against. + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, value: 300 }) + }) + + afterEach(() => { + if (originalOffsetHeight !== undefined) { + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', originalOffsetHeight) + } else { + delete (HTMLElement.prototype as { offsetHeight?: number }).offsetHeight + } + }) + + test('virtualize renders a windowed subset of a large thread, once the lazy import resolves', async () => { + const messages = manyMessages(200) + const { container } = render( + + + , + ) + + // The Suspense fallback (`VirtualizeSuspenseFallback`) is an EMPTY placeholder with no row + // content at all, so waiting for ANY rendered row can only resolve once the lazy + // `@tanstack/react-virtual` import has settled and the real virtualizer has mounted — unlike + // the assertion this replaces (`await screen.findByText('message 199')`), which resolved from + // the OLD full-row-tree fallback at SYNCHRONOUS tick 0, before any `await`, regardless of + // whether the import ever resolved or the virtualizer ever ran. + // + // Which row lands first is deliberately NOT asserted: happy-dom has no layout/scroll engine + // (every element's `scrollHeight`/`offsetHeight` beyond the stubbed constant above is 0), so + // `anchorTo: 'end'` cannot be proven in this harness — observed locally, the virtualizer mounts + // starting at index 0, not the tail, here. That behaviour remains unverified until the browser + // gate; this test only proves the async mount + windowing, not the anchor direction. + await waitFor(() => { + const renderedCount = container.querySelectorAll('[data-testid^="agent-message-"]').length + expect(renderedCount).toBeGreaterThan(0) + expect(renderedCount).toBeLessThan(200) + }) + }) + + test('virtualize keeps windowing with an active live/streaming turn appended', async () => { + const messages = manyMessages(80) + const { container } = render( + + + , + ) + + await waitFor(() => { + expect(container.querySelectorAll('[data-testid^="agent-message-"]').length).toBeGreaterThan( + 0, + ) + }) + + // `anchorTo: 'end'` cannot be relied on in happy-dom (no layout/scroll engine — see the + // previous test's comment), so the live block does not necessarily paint on its own. This + // proves the settled/live SPLIT (the `settledRows`/`liveRow`/`rows` memos from finding #2) still + // hands the virtualizer one COMBINED row set that genuinely includes the live block, by + // manually scrolling the pane to its reported end and confirming the live message is what + // renders there — not that a live message got dropped when it was pulled out of the settled + // memo. + const scrollElement = container.querySelector('div[style*="overflow: auto"]') + if (scrollElement === null) + throw new Error('expected the virtualizer scroll container to exist') + ;(scrollElement as HTMLElement).scrollTop = Number.MAX_SAFE_INTEGER + fireEvent.scroll(scrollElement) + + await waitFor(() => { + expect(screen.queryByText('streaming reply')).not.toBeNull() + }) + + // 80 settled + 1 live = 81 total; still windowed even scrolled to the tail. + const renderedCount = container.querySelectorAll('[data-testid^="agent-message-"]').length + expect(renderedCount).toBeGreaterThan(0) + expect(renderedCount).toBeLessThan(81) + }) + + test('the non-virtual path is unchanged — every message renders, unwindowed', () => { + const messages = manyMessages(12) + const { container } = render( + + + , + ) + + expect(container.querySelectorAll('[data-testid^="agent-message-"]').length).toBe(12) + }) + + test('the optional-peer-absent degrade target renders every row unwindowed, and never throws', () => { + const rows = Array.from({ length: 30 }, (_, i) => ({ + key: `fallback-row-${i}`, + node:
{`row ${i}`}
, + })) + + let container: HTMLElement | undefined + expect(() => { + ;({ container } = render( + + + , + )) + }).not.toThrow() + + expect(container).toBeDefined() + expect(container?.querySelectorAll('[data-testid^="fallback-row-"]').length).toBe(30) + expect(screen.getByTestId('fallback-row-0')).toBeDefined() + expect(screen.getByTestId('fallback-row-29')).toBeDefined() + }) + + // happy-dom has no scroll engine, so scroll POSITION can't be asserted (see the module-level + // comment on the first test above). What CAN be asserted, genuinely, is the SCROLL INTENT: a + // spy on `Element.prototype.scrollTo` — the exact DOM entry point `virtual-core`'s `elementScroll` + // calls at the bottom of `scrollToEnd()` (verified against the installed 3.17.1 source; happy-dom + // implements `scrollTo` as a real, synchronous `scrollTop` write for the `'auto'`/default + // behavior this effect uses, so the spy observes a genuine call, not a no-op the DOM silently + // swallows). This is the boundary the brief asks for: a fake/spy at the edge, not a pretended + // observation of scroll position. + describe('the default open-at-the-newest-message jump (VirtualizeOptions.initialScroll)', () => { + test('fires once the lazy virtualizer mounts', async () => { + const scrollToSpy = spyOn(Element.prototype, 'scrollTo') + const messages = manyMessages(60) + const { container } = render( + + + , + ) + + await waitFor(() => { + expect( + container.querySelectorAll('[data-testid^="agent-message-"]').length, + ).toBeGreaterThan(0) + }) + expect(autoBehaviorCalls(scrollToSpy)).toBeGreaterThan(0) + + scrollToSpy.mockRestore() + }) + + test('a virtualizer that mounted behind a hidden ancestor lands the jump on the commit that makes it measurable', async () => { + // The reachable version of `virtualize.ts`'s third+fourth composition rules meeting: the + // lazy `import('@tanstack/react-virtual')` settles ASYNCHRONOUSLY, so a `ThreadFeedRow` + // collapsed during that window mounts `VirtualizedRowsInner` behind `display: none` — every + // measurement reads 0, and the commit that later un-hides the body changes neither the row + // count nor the virtualizer identity. A dependency-keyed effect never re-runs for that + // commit, which stranded the transcript at message #0 for the rest of the row's life. + // happy-dom has no per-element layout to hide, so the describe block's 300px prototype stub + // is overridden to 0 to reach the same state, then restored to stand in for the re-expand. + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, value: 0 }) + const scrollToSpy = spyOn(Element.prototype, 'scrollTo') + const messages = manyMessages(60) + // A FRESH element on every render pass: React bails out of re-rendering a subtree whose + // element object is reference-identical to the previous one, which would skip the very + // commit this test is about. + const tree = () => ( + + + + ) + const { container, rerender } = render(tree()) + + // A 0px viewport windows to no rows at all, so wait on the real virtualizer's sizer instead + // (same signal the empty-transcript test below uses). + await waitFor(() => { + expect(container.querySelector('div[style*="overflow: auto"] > div')).not.toBeNull() + }) + expect(autoBehaviorCalls(scrollToSpy)).toBe(0) + + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + value: 300, + }) + rerender(tree()) + + expect(autoBehaviorCalls(scrollToSpy)).toBeGreaterThan(0) + + scrollToSpy.mockRestore() + }) + + test('initialScroll: "start" suppresses the jump entirely', async () => { + const scrollToSpy = spyOn(Element.prototype, 'scrollTo') + const messages = manyMessages(200) + const { container } = render( + + + , + ) + + await waitFor(() => { + expect( + container.querySelectorAll('[data-testid^="agent-message-"]').length, + ).toBeGreaterThan(0) + }) + expect(autoBehaviorCalls(scrollToSpy)).toBe(0) + + scrollToSpy.mockRestore() + }) + + test('an empty transcript never scrolls, and never throws', async () => { + const scrollToSpy = spyOn(Element.prototype, 'scrollTo') + + let container: HTMLElement | undefined + expect(() => { + ;({ container } = render( + + + , + )) + }).not.toThrow() + + // No message rows ever exist to wait on, so instead wait for the REAL virtualizer's nested + // sizer `Box` (present only once `LazyVirtualizedRows` — not `VirtualizeSuspenseFallback`, + // which renders a single childless pane — has mounted). + await waitFor(() => { + expect(container?.querySelector('div[style*="overflow: auto"] > div')).not.toBeNull() + }) + expect(autoBehaviorCalls(scrollToSpy)).toBe(0) + + scrollToSpy.mockRestore() + }) + }) }) diff --git a/packages/basalt-ui/src/agent-chat/thread-message.tsx b/packages/basalt-ui/src/agent-chat/thread-message.tsx index 81be880..e77ab14 100644 --- a/packages/basalt-ui/src/agent-chat/thread-message.tsx +++ b/packages/basalt-ui/src/agent-chat/thread-message.tsx @@ -29,8 +29,8 @@ import { Text, UnstyledButton, } from '@mantine/core' -import { useDisclosure } from '@mantine/hooks' -import { Fragment, memo, useMemo } from 'react' +import { useDisclosure, useFocusWithin, useHover, useMergedRef } from '@mantine/hooks' +import { Fragment, lazy, memo, Suspense, useLayoutEffect, useMemo, useRef } from 'react' import type { JSX } from 'react' import type { AgentPart, @@ -49,8 +49,14 @@ import type { } from '../agent' import { coalesceParts, narrowAgentPart, PartList } from '../agent' import { Markdown } from '../content/markdown' +import { CopyAction } from '../content/copy-action' import { VX } from '../tokens' +import { DEFAULT_AFFORDANCES } from './message-affordances' +import type { MessageAffordances } from './message-affordances' +import { formatRelativeTime } from './relative-time' import { ToolChip } from './tool-chip' +import { resolveVirtualize } from './virtualize' +import type { VirtualizeOptions, VirtualizeProps } from './virtualize' /** The mono, uppercase, letter-spaced micro-label idiom (docs/DESIGN-SPEC.md §3) — shared by the * transcript's role labels and the reasoning/tool-call headers below. */ @@ -266,6 +272,99 @@ function resolveSegments( return segments } +/** Concatenates every resolved `text` part across a message's segments, in order, joined by a + * blank line between non-adjacent runs (adjacent text parts are already merged by `coalesceParts` + * inside `resolveSegments`). This is what the per-message copy affordance copies — the message's + * COALESCED text, not its raw `parts`, so a message that streamed in several by-id text chunks + * copies one clean string rather than the fragments the user never saw assembled. Foreign + * (consumer-registered) segments carry no `AgentPart`-typed text and are skipped. */ +function extractCoalescedText(segments: readonly ResolvedSegment[]): string { + const chunks: string[] = [] + for (const segment of segments) { + if (segment.kind !== 'agent') continue + for (const part of segment.parts) { + if (part.type === 'text') chunks.push(part.text) + } + } + return chunks.join('\n\n') +} + +// ── MessageAffordanceRow — the per-message hover row (AGENT-CHAT-SPEC.md §11) ───────────────── + +const GROUP_WINDOW_MS = 5 * 60_000 + +/** True when `message` continues the SAME speaker's immediately-preceding turn within the Slack + * grouping window — the predecessor is absent (first message), a different role, or more than + * `GROUP_WINDOW_MS` apart all resolve to false. */ +function isConsecutiveWithPredecessor( + message: ChatMessage, + predecessor: ChatMessage | undefined, +): boolean { + if (predecessor === undefined) return false + if (predecessor.role !== message.role) return false + return message.createdAt - predecessor.createdAt <= GROUP_WINDOW_MS +} + +type MessageAffordanceRowProps = { + readonly message: ChatMessage + readonly coalescedText: string + readonly affordances: MessageAffordances + readonly isLastAssistant: boolean + /** Driven by the parent block's hover state OR its focus-within state — a visual reveal-on-hover + * strip that must ALSO reveal for a sighted keyboard-only user tabbing a control into view (hover + * alone left a focused Copy/Regenerate control invisible, with no focus ring visible either). The + * row stays mounted (and clickable/focusable) at all times rather than being removed from the + * DOM — that part of "keyboard/AT users never lose the controls a mouse user gets for free" held + * even before this fix for screen-reader users (opacity keeps elements in the a11y tree); visible + * focus for SIGHTED keyboard users is the half that did not, until this `focused` term was added. */ + readonly visible: boolean +} + +/** The per-message hover row: relative/absolute/none timestamp, a copy-coalesced-text action, a + * regenerate action (last assistant message only), and any consumer `actions`. Renders `null` when + * every affordance resolves to off (nothing to show, no empty strip taking up rhythm). */ +function MessageAffordanceRow({ + message, + coalescedText, + affordances, + isLastAssistant, + visible, +}: MessageAffordanceRowProps): JSX.Element | null { + const timestampMode = affordances.timestamp ?? DEFAULT_AFFORDANCES.timestamp + const showTimestamp = timestampMode !== 'none' + const showCopy = affordances.copy ?? DEFAULT_AFFORDANCES.copy + const showRegenerate = isLastAssistant && affordances.onRegenerate !== undefined + const customActions = affordances.actions?.({ message }) + + if (!showTimestamp && !showCopy && !showRegenerate && customActions === undefined) return null + + const timestampLabel = + timestampMode === 'absolute' + ? new Date(message.createdAt).toLocaleString() + : formatRelativeTime(message.createdAt) + + return ( + + {showTimestamp && ( + + {timestampLabel} + + )} + {showCopy && } + {showRegenerate && ( + affordances.onRegenerate?.(message.id)}> + Regenerate + + )} + {customActions} + + ) +} + // ── MessageBlock (memoized) ──────────────────────────────────────────────────── const ROLE_LABEL: Record = { @@ -294,6 +393,20 @@ type MessageBlockProps = { readonly streaming?: boolean readonly renderers: PartRenderers readonly fallbackRenderer: PartRenderer + /** Suppresses the role label and surface chrome — this message continues the same speaker's + * immediately-preceding turn within the grouping window (the Slack rhythm, AGENT-CHAT-SPEC.md + * §_). Computed by `ThreadTranscript` (a property of the SEQUENCE, not of one message in + * isolation) and passed down. @default false */ + readonly grouped?: boolean + /** Per-message hover-row affordances. `undefined` renders NO affordance row at all — this is how + * `ThreadTranscript` opts the live/streaming message out (an epoch-0 `createdAt` and a + * half-streamed coalesced text would otherwise render a nonsensical "56 years ago" timestamp and + * a copy button that copies a fragment). Every settled message gets a resolved (defaults-merged) + * object instead. */ + readonly affordances?: MessageAffordances + /** True only for the single most-recent SETTLED assistant message — the sole message + * `affordances.onRegenerate` renders a control for. @default false */ + readonly isLastAssistant?: boolean } /** @@ -309,37 +422,71 @@ function MessageBlockImpl({ streaming = false, renderers, fallbackRenderer, + grouped = false, + affordances, + isLastAssistant = false, }: MessageBlockProps): JSX.Element { messageBlockRenderCounter.count += 1 const settled = !streaming + const { hovered, ref: hoverRef } = useHover() + // `focused` covers the sighted-keyboard-user gap hover alone leaves open (see + // `MessageAffordanceRowProps.visible`'s doc) — merged onto the SAME node as `hoverRef` via + // `useMergedRef` since both hooks need their own callback ref on this one element. + const { focused, ref: focusWithinRef } = useFocusWithin() + const ref = useMergedRef(hoverRef, focusWithinRef) const segments = useMemo( () => resolveSegments(message.parts, renderers, fallbackRenderer), [message.parts, renderers, fallbackRenderer], ) const finishIndicator = message.finish === undefined ? undefined : FINISH_INDICATOR[message.finish] + // Guarded on `affordances`: the live/streaming message is pushed into `rows` WITHOUT this prop + // (see `MessageBlockProps.affordances`'s doc), so `MessageAffordanceRow` never renders for it — + // computing `extractCoalescedText` for it anyway would be pure waste, and `segments` for a live + // message grows every streamed chunk, so unconditionally recomputing it scaled total work across + // one streamed reply with the SQUARE of the response length. + const coalescedText = useMemo( + () => (affordances === undefined ? '' : extractCoalescedText(segments)), + [affordances, segments], + ) + + // Grouped continuations drop the role label — but a streaming loader or a finish badge is + // per-message state (not sequence rhythm) and must still surface even mid-group, so the header + // row itself only disappears when there is truly nothing left for it to show. + const showHeader = !grouped || streaming || finishIndicator !== undefined // Assistant/user surfaces are differentiated by subtle vs panel tokens, radius 7 - // (VX.radiusCard, docs/DESIGN-SPEC.md §5) — the user's own turn sits a shade quieter than the reply. + // (VX.radiusCard, docs/DESIGN-SPEC.md §5) — the user's own turn sits a shade quieter than the + // reply. A grouped continuation drops this chrome entirely (transparent, no shadow, no radius) + // — that absence, alongside the suppressed role label, IS the Slack rhythm. return ( - - {ROLE_LABEL[message.role]} - {streaming && } - {finishIndicator !== undefined && ( - - {finishIndicator.label} - - )} - + {showHeader && ( + + {!grouped && {ROLE_LABEL[message.role]}} + {streaming && } + {finishIndicator !== undefined && ( + + {finishIndicator.label} + + )} + + )} {segments.map((segment) => segment.kind === 'agent' ? ( ), )} + {affordances !== undefined && ( + + )} ) @@ -369,18 +525,431 @@ function MessageBlockImpl({ * re-render must not force through every settled message's `MessageBlock`. `renderers`/ * `fallbackRenderer` are included too: both are memoized once at the `ThreadTranscript` level (see * below), so in practice they're stable across renders, but a genuine change to either (a consumer - * swapping its renderer map) must still invalidate the memo. */ + * swapping its renderer map) must still invalidate the memo. `grouped` and `isLastAssistant` are + * cheap booleans recomputed every `ThreadTranscript` render — including them keeps a message from + * rendering yesterday's grouping/regenerate-eligibility when a NEIGHBOUR changes, at negligible + * comparator cost. `affordances` is a single object `ThreadTranscript` re-resolves only when its + * OWN fields change (see `resolvedAffordances` below) — comparing it by reference here is what lets + * a consumer's fresh-object-literal `affordances` prop NOT force every block to re-render. */ function areMessageBlockPropsEqual(prev: MessageBlockProps, next: MessageBlockProps): boolean { return ( prev.message === next.message && prev.streaming === next.streaming && prev.renderers === next.renderers && - prev.fallbackRenderer === next.fallbackRenderer + prev.fallbackRenderer === next.fallbackRenderer && + prev.grouped === next.grouped && + prev.affordances === next.affordances && + prev.isLastAssistant === next.isLastAssistant ) } const MessageBlock = memo(MessageBlockImpl, areMessageBlockPropsEqual) +// ── Virtualization (AGENT-CHAT-SPEC.md §9) ───────────────────────────────────── +// +// `@tanstack/react-virtual` is an OPTIONAL peer (see `./virtualize`'s module doc). `agent-chat` is +// ONE package.json export subpath — unlike `basalt-ui/data/virtual`, which gets to statically +// import the peer because it lives behind its OWN subpath (a consumer who never imports it never +// resolves the peer either) — so a static top-level import here would make every `ThreadTranscript` +// consumer require the peer, even ones that never pass `virtualize`. Lazy + dynamic `import()` + +// `.catch()` degrade, mirroring `../agent/stick-to-bottom.tsx`'s established pattern exactly, keeps +// the non-virtualized path working with the package absent. + +/** Default extra rows rendered beyond the viewport, each side. Higher than + * `BasaltVirtualList`'s generic-list default (5) — a streaming transcript's scroll bursts benefit + * from more pre-rendered neighbours above/below the fold. */ +const DEFAULT_VIRTUALIZE_OVERSCAN = 6 + +/** Default estimated row height in px, used before a row has been measured. Chat rows run taller + * than a generic list row (role header + multi-line prose), hence higher than + * `BasaltVirtualList`'s default of 40. Measured mean row height on the shipped 500-message demo + * thread is ~145px (total size climbed 49,486 → 72,448px over one descent, ~934px/screenful) — an + * estimate below the mean shrinks the scrollbar thumb continuously for the whole first scroll + * down. TanStack's own guidance for dynamically measured lists is to estimate toward the LARGER + * end: overestimating settles the thumb downward once real measurements land, instead of + * shrinking it. 160 rounds up from the measured mean with headroom for the estimate to still be + * defensible on a shorter/plainer demo thread. */ +const DEFAULT_VIRTUALIZE_ESTIMATE_SIZE = 160 + +/** Default `scrollEndThreshold` — how close (px) to the end still counts as "at the end" for + * `followOnAppend`/`anchorTo: 'end'` to keep tracking rather than freezing the scroll position. */ +const DEFAULT_VIRTUALIZE_SCROLL_END_THRESHOLD = 64 + +/** Default `VirtualizeOptions.initialScroll` — see `./virtualize`'s fourth composition rule for + * why a chat transcript opens at the newest message rather than the oldest. */ +const DEFAULT_VIRTUALIZE_INITIAL_SCROLL: NonNullable = 'end' + +/** Caps `applyInitialScroll`'s clobber-recovery retries (see `./virtualize`'s fourth composition + * rule for the full mechanism this guards). A small fixed budget, not a time/frame-based deadline: + * every observed case of virtual-core's `_willUpdate` clobbering this effect's jump resolves within + * one extra commit, so this exists purely as a backstop against a pathological DOM that never + * reports settled — not as the expected path. Once exhausted, the effect gives up and marks the + * jump permanently done wherever the last attempt landed, rather than fighting the DOM (or a user + * who has since scrolled away) forever. Exported (like `messageBlockRenderCounter`) purely so its + * test can assert the exact bound without duplicating the number; not part of the public surface. */ +export const MAX_INITIAL_SCROLL_ATTEMPTS = 5 + +type TranscriptRow = { + readonly key: string + readonly node: JSX.Element +} + +type VirtualizedTranscriptProps = { + readonly rows: readonly TranscriptRow[] + readonly height: number | string + readonly overscan: number + readonly estimateSize: number + readonly initialScroll: NonNullable +} + +/** + * The degrade target: every row rendered unwindowed inside a fixed-height scroll container. + * This is what a `virtualize: true` transcript falls back to when `@tanstack/react-virtual` fails + * to resolve (peer absent) — same visual shape (a scrollable `height`-bound pane), just without + * windowing. + * + * NOT used as the `Suspense` fallback (see `VirtualizeSuspenseFallback` below) — mounting every + * real `MessageBlock` subtree for the one tick the lazy import takes to settle would make that + * settle a genuine unmount/remount of every rendered row, re-firing each row's effects once and + * risking hitting a message mid-stream during the resolve window. + */ +function NonVirtualizedRows({ rows, height }: VirtualizedTranscriptProps): JSX.Element { + return ( + // theme-allow — degrade target owns its own scroll node, same as the real virtualizer below. + + + {rows.map((row) => ( + {row.node} + ))} + + + ) +} + +/** Test-only escape hatch (mirrors `messageBlockRenderCounter` above) — proves the peer-absent + * degrade target itself renders every row without windowing and without throwing, without having + * to fight `React.lazy`'s permanent once-per-module-instance resolution cache to simulate the + * peer's absence at the `import()` layer (see thread-message.test.tsx's virtualization describe + * block for why that approach was rejected). Not exported from `agent-chat/index.ts`. */ +export const nonVirtualizedRowsFallback = NonVirtualizedRows + +/** + * Minimal structural slice of `Virtualizer` (virtual-core 3.17.1) that + * {@link resolveGuardedMeasurement} needs — `indexFromElement`/`itemSizeCache`/`options` are all + * public instance members, but typing against the real (generic, peer-only) `Virtualizer` class + * would force this test-only-exported helper to import the peer's runtime types at module scope, + * the exact static-import-forces-the-peer problem the surrounding `lazy(() => import(...))` exists + * to avoid. A structural type sidesteps that without loosening to `any`. Generic over the item + * element type (rather than fixed at `Element`) so it stays exactly assignable FROM a real + * `Virtualizer` at the call site below — `indexFromElement` is + * declared with arrow-property (not method) syntax upstream, so it is checked contravariantly, and + * a fixed `Element` parameter there does not structurally match a real instance's narrower + * `TItemElement` parameter. + */ +type MeasurementCacheHost = { + readonly indexFromElement: (node: TItemElement) => number + readonly itemSizeCache: Map + readonly options: { + readonly getItemKey: (index: number) => unknown + readonly estimateSize: (index: number) => number + } +} + +/** + * Resolves the size a `measureElement` implementation should report, given what the underlying + * (default or custom) measurement already produced: that value unchanged when it is a genuine + * measurement, or the row's own last-known size when it is `0` — never a real reading for a mounted + * row, so a `0` only ever means the ResizeObserver measured through a `display: none` (or otherwise + * unlaid-out) ancestor, e.g. a virtualized transcript sitting inside a collapsed `ThreadFeedRow`. + * Falling through to it unguarded is what corrupted `itemSizeCache`/`measurementsCache`: virtual- + * core's default `resizeItem` writes whatever `measureElement` returns with no floor, so every + * mounted row remeasures at 0px the instant an ancestor hides, `getTotalSize()` collapses within + * the same tick, and the collapse is never undone — a later real remeasurement only OVERWRITES + * entries the ResizeObserver happens to re-fire for, it does not restore ones that silently kept + * their poisoned 0 (see `virtualize.ts`'s composition-rule doc for the full mechanism and the + * measured numbers). + * + * Takes the already-computed `size` plus the {@link MeasurementCacheHost} slice, rather than + * wrapping the `measureElement` function itself, so this stays a plain, non-generic-over-a- + * generic-function call at its one call site below (`instance` there is `Virtualizer` for THAT call's concrete type arguments, not the peer's own generic + * `measureElement` export — wrapping the latter directly hits TS's higher-rank inference limits on + * a doubly-generic HOF). That shape is also what makes this unit-testable with a fake host object + * and no ResizeObserver/layout engine at all (happy-dom has neither). Not exported from + * `agent-chat/index.ts`: a test-only escape hatch, matching + * `messageBlockRenderCounter`/`nonVirtualizedRowsFallback` above. + */ +export function resolveGuardedMeasurement( + host: MeasurementCacheHost, + element: TItemElement, + size: number, +): number { + if (size > 0) return size + // Same key derivation `resizeItem` itself uses (index → getItemKey), read BEFORE `resizeItem` + // writes the new (bad) size — `itemSizeCache` still holds the last-good measurement here. + const index = host.indexFromElement(element) + const key = host.options.getItemKey(index) + return host.itemSizeCache.get(key) ?? host.options.estimateSize(index) +} + +/** + * Decides what `VirtualizedRowsInner`'s one-shot initial-scroll effect should do this render, + * given the inputs the effect closes over. Pure and peer/DOM-free — pulled out of the lazy-loaded + * closure for the same reason `resolveGuardedMeasurement` is: it stays unit-testable without a + * real `Virtualizer` instance or `@tanstack/react-virtual` itself. + * + * `'start'` and an empty transcript resolve to `'skip-done'` — there is nothing to scroll to, and + * DONE is permanent (see `virtualize.ts`'s fourth composition rule: the real scroll must fire at + * most once, ever, so a later append never yanks a user who has scrolled up). A non-positive + * `containerHeight` — the transcript's own scroll node measuring `0`, e.g. mounted behind a + * `display: none` ancestor per `virtualize.ts`'s third composition rule — resolves to + * `'skip-retry'`: NOT done, so the caller's effect (which re-runs after EVERY commit until the jump + * lands — see {@link applyInitialScroll}) gets another chance the next time this subtree renders, + * instead of permanently stranding the transcript at the top the moment it becomes visible. Only a + * genuinely measured, non-empty, `'end'`-targeted mount resolves to `'scroll'`. + */ +export function resolveInitialScrollAction( + initialScroll: NonNullable, + rowCount: number, + containerHeight: number, +): 'scroll' | 'skip-done' | 'skip-retry' { + if (initialScroll === 'start') return 'skip-done' + if (rowCount === 0) return 'skip-done' + if (containerHeight <= 0) return 'skip-retry' + return 'scroll' +} + +/** The mutable half of the one-shot initial-scroll contract. `hasApplied` is whether it has + * PERMANENTLY finished — either there was nothing to do (`'start'`/empty transcript), or + * `scrollToEnd` fired and was subsequently CONFIRMED (via a real DOM read — see + * {@link applyInitialScroll}'s doc for why virtual-core's own tracked offset can't be trusted for + * this) to have survived whatever else committed since. `attempts` counts how many times + * `scrollToEnd` has been (re-)fired this mount; 0 means never attempted, and also gates whether a + * call re-derives {@link resolveInitialScrollAction} at all. One instance lives for the lifetime of + * a `VirtualizedRowsInner` mount, held in a `useRef` so it survives re-renders without itself + * triggering one. */ +export type InitialScrollState = { hasApplied: boolean; attempts: number } + +/** + * The stateful half of "fires once, on first mount, and never again" (`virtualize.ts`'s fourth + * composition rule): wraps {@link resolveInitialScrollAction}'s per-call decision with the + * ACROSS-CALLS memory that decision alone can't carry. Exported (like `resolveGuardedMeasurement`) + * so `VirtualizedRowsInner`'s effect below calls this SAME function rather than a hand-mirrored + * copy for tests — a fake `scrollToEnd` is the only DOM/peer-free way to prove the guard, since a + * real `Virtualizer`'s own `followOnAppend` also calls the real DOM `scrollTo` on a later append, + * making that specific call indistinguishable from this one at the DOM boundary (see this + * function's test suite for the measurement that ruled that boundary out). + * + * Firing `scrollToEnd()` once is NOT the same as landing there: virtual-core 3.17.1's own + * `_willUpdate` (verified against the installed dist) writes to the SAME scroll container from + * `this.scrollOffset`, which updates only from the ASYNCHRONOUS native `scroll` event — never from + * the synchronous call that requested a scroll. A commit that runs `_willUpdate` before this + * effect gets a turn (element-attach) or that re-derives an anchor while `scrollOffset` still holds + * the pre-jump value writes that STALE offset straight back to `scrollTop`, undoing the jump within + * about one extra commit of it landing (see `./virtualize`'s fourth composition rule for the full + * trace this was diagnosed from). So `resolveInitialScrollAction`'s `'scroll'` outcome does not, by + * itself, set `hasApplied`: `isAtEnd` — a real `scrollTop`/`scrollHeight`/`clientHeight` read, the + * one piece of ground truth that is NEVER stale, unlike virtual-core's own tracked offset — is + * consulted on the NEXT call (i.e. the next commit, exactly the cadence this dependency-free effect + * already runs on) to confirm the jump survived. If it didn't, `scrollToEnd` fires again; if it did, + * `hasApplied` is set and the effect is inert for the rest of the mount. `MAX_INITIAL_SCROLL_ATTEMPTS` + * bounds this so a DOM that pathologically never reports settled — or a user who scrolls away mid- + * measurement — can never be fought forever; once exhausted, `hasApplied` is set regardless, wherever + * the last attempt left the container. + * + * `state.hasApplied` is the ONLY thing that can permanently stop this from calling `scrollToEnd` + * again: `'start'` and an empty transcript set it immediately (nothing to verify), and `'skip-retry'` + * deliberately leaves it `false` so a transcript that mounted behind a hidden ancestor gets another + * attempt on the caller's next commit, rather than being silently abandoned the moment it becomes + * visible. That retry — and the clobber-recovery above — is why the caller's effect carries NO + * dependency array: the commit that matters (a collapsed `ThreadFeedRow` flipping its body back to + * `display: block`, or virtual-core's own follow-up commit after this effect's own jump) changes + * neither the row count nor the virtualizer identity, so a dependency-keyed effect would never + * re-run for it. + * + * `getContainerHeight` is a THUNK, not a number, for the same reason: reading `offsetHeight` forces + * a synchronous layout, and it is read at most ONCE per mount — only on the very first call + * (`attempts === 0`) — never again while verifying or retrying, so a landed transcript still pays + * nothing for the effect it keeps re-running (a streaming transcript commits on every chunk). + * `isAtEnd` is cheaper (a plain property read, no forced reflow beyond what the browser already + * tracks) and is exactly what the verification/retry loop above consults instead. + */ +export function applyInitialScroll( + state: InitialScrollState, + initialScroll: NonNullable, + rowCount: number, + getContainerHeight: () => number, + scrollToEnd: () => void, + isAtEnd: () => boolean, +): void { + if (state.hasApplied) return + + // Already fired at least once this mount — verify it survived rather than re-deriving + // `resolveInitialScrollAction` (row count / container height are irrelevant to "did the DOM + // keep the scroll we already decided on"). + if (state.attempts > 0) { + if (isAtEnd() || state.attempts >= MAX_INITIAL_SCROLL_ATTEMPTS) { + state.hasApplied = true + return + } + scrollToEnd() + state.attempts += 1 + return + } + + const action = resolveInitialScrollAction(initialScroll, rowCount, getContainerHeight()) + if (action === 'skip-retry') return + if (action === 'skip-done') { + state.hasApplied = true + return + } + scrollToEnd() + state.attempts += 1 +} + +/** Reads the transcript's own scroll container to answer "is it currently at (or within + * `threshold`px of) the end" — the ground-truth check {@link applyInitialScroll} verifies its jump + * against. Deliberately NOT `virtualizer.isAtEnd()`/`getScrollOffset()`: both are backed by + * `this.scrollOffset`, the exact value that lags a real scroll until the next native `scroll` + * event fires — using it here would make the verification vulnerable to the identical staleness + * that causes the clobber in the first place. `scrollTop`/`scrollHeight`/`clientHeight` are plain + * DOM properties with no such lag: `Element.scrollTo` with `'auto'` (this module's own behavior) + * updates `scrollTop` synchronously, so a read immediately after — or, as here, on the next + * commit — reflects reality. `null` (container not yet attached) reads as "not at end" rather than + * throwing, matching every other guard in this file's tolerance for a not-yet-mounted ref. */ +function isContainerAtEnd(container: HTMLDivElement | null, threshold: number): boolean { + if (container === null) return false + const distanceFromEnd = container.scrollHeight - container.clientHeight - container.scrollTop + return distanceFromEnd <= threshold +} + +/** Lazy-loaded real virtualizer. Resolves once, permanently, for the lifetime of this module + * instance — identical caching behaviour to `../agent/stick-to-bottom.tsx`'s `LazyStickToBottom`. */ +const LazyVirtualizedRows = lazy(() => + import('@tanstack/react-virtual') + .then(({ useVirtualizer, measureElement: defaultMeasureElement }) => { + function VirtualizedRowsInner({ + rows, + height, + overscan, + estimateSize, + initialScroll, + }: VirtualizedTranscriptProps): JSX.Element { + const parentRef = useRef(null) + const initialScrollStateRef = useRef({ + hasApplied: false, + attempts: 0, + }) + + // Resolved research facts (virtual-core 3.17.1, carried by react-virtual 3.14.3): + // `getItemKey` returning `message.id` (NOT the default index) so prepending/streaming + // doesn't scramble anchoring; `anchorTo: 'end'` + `followOnAppend` for chat-scroll rhythm; + // `useFlushSync: false` per the same React-19 opt-out `BasaltVirtualList` documents. + const virtualizer = useVirtualizer({ + count: rows.length, + getScrollElement: () => parentRef.current, + estimateSize: () => estimateSize, + overscan, + getItemKey: (index) => rows[index]?.key ?? index, + anchorTo: 'end', + followOnAppend: true, + scrollEndThreshold: DEFAULT_VIRTUALIZE_SCROLL_END_THRESHOLD, + useFlushSync: false, + // Guards against the hidden-ancestor cache-poisoning mechanism + // `resolveGuardedMeasurement`'s doc and `virtualize.ts`'s composition rules describe — + // NOT `enabled: false`, which virtual-core 3.17.1 wires to WIPE + // `itemSizeCache`/`measurementsCache` entirely (`getMeasurements`'s `!enabled` branch) + // rather than pause measurement, so toggling it off/on around a collapse would be + // strictly worse than the bug it was meant to fix. + measureElement: (element, entry, instance) => + resolveGuardedMeasurement( + instance, + element, + defaultMeasureElement(element, entry, instance), + ), + }) + + // The one-shot "open at the newest message" jump (`virtualize.ts`'s fourth composition + // rule) — see `applyInitialScroll`'s doc for the guard mechanics, including why this effect + // deliberately carries NO dependency array. Short version: this subtree can mount while + // INVISIBLE (the lazy `import()` above settles asynchronously, so a `ThreadFeedRow` + // collapsed during that window mounts it behind `display: none`), and the commit that makes + // it visible again changes no dependency a keyed array could watch. That SAME dependency- + // free cadence is also what lets this effect recover from virtual-core's own `_willUpdate` + // clobbering the jump on the commit right after it lands (see `applyInitialScroll`'s doc) — + // `state.hasApplied` only becomes permanent once `isContainerAtEnd` confirms it on a later + // call, not the instant `scrollToEnd` is invoked. `parentRef.current.offsetHeight` (not + // `getBoundingClientRect`) matches what virtual-core's own `getRect` reads for the scroll + // container, so a hidden ancestor is treated as unmeasured in exactly the cases virtual-core + // itself would also treat that way; it is passed as a thunk (like the `isContainerAtEnd` + // read below) so nothing is forced once the jump has landed and been confirmed. + useLayoutEffect(() => { + applyInitialScroll( + initialScrollStateRef.current, + initialScroll, + rows.length, + () => parentRef.current?.offsetHeight ?? 0, + () => virtualizer.scrollToEnd({ behavior: 'auto' }), + () => isContainerAtEnd(parentRef.current, DEFAULT_VIRTUALIZE_SCROLL_END_THRESHOLD), + ) + }) + + return ( + // theme-allow — TanStack Virtual measures/scrolls this element (never nest in BasaltStickToBottom, see ./virtualize). + + + {virtualizer.getVirtualItems().map((virtualItem) => { + const row = rows[virtualItem.index] + if (row === undefined) return null + return ( + + {row.node} + + ) + })} + + + ) + } + return { default: VirtualizedRowsInner } + }) + .catch(() => ({ default: NonVirtualizedRows })), +) + +/** + * Lightweight `Suspense` fallback for `VirtualizedTranscript` — shown only for the one tick the + * dynamic `@tanstack/react-virtual` import takes to resolve. Deliberately NOT `NonVirtualizedRows`: + * see that component's doc for why mounting every real row here would be a genuine (if one-time) + * unmount/remount of the whole transcript. An empty, correctly-sized scroll pane avoids a layout + * flash without paying that cost — it settles state before any row has meaningfully mounted. + */ +function VirtualizeSuspenseFallback({ height }: { readonly height: number | string }): JSX.Element { + // theme-allow — placeholder owns its own scroll node, matching the real virtualizer's shape. + return +} + +function VirtualizedTranscript(props: VirtualizedTranscriptProps): JSX.Element { + return ( + }> + + + ) +} + // ── ThreadTranscript ────────────────────────────────────────────────────────── const EMPTY_RENDERERS: PartRenderers = {} @@ -397,9 +966,58 @@ type ThreadTranscriptBase = { /** Called for a part whose type is neither an AgentPart variant nor a registered key. Defaults * to a visible `UnknownPartChip` outside production, `null` in production — never throws. */ readonly fallbackRenderer?: PartRenderer + /** Per-message hover-row affordances (timestamp/copy/regenerate/custom actions). Unset fields + * fall back to `DEFAULT_AFFORDANCES`. Never shown on the live/streaming message — see + * `MessageBlockProps.affordances`. */ + readonly affordances?: MessageAffordances + /** Suppresses the role label and surface chrome on a message whose predecessor shares its role + * and landed within 5 minutes — the Slack rhythm. + * @default true */ + readonly groupConsecutive?: boolean } -export type ThreadTranscriptProps = ThreadTranscriptBase +export type ThreadTranscriptProps = ThreadTranscriptBase & VirtualizeProps + +/** + * Wraps a possibly-fresh-every-render EVENT HANDLER in a wrapper function whose IDENTITY never + * changes across renders (while it is defined), always calling through to the LATEST version via a + * ref — the same "mirror the latest value every render" idiom `use-agent-thread-runs.ts` uses for + * `storeRef`/`transportRef`, applied to a callback instead of a value. + * + * This is what lets `resolvedAffordances` below key its memo on the handler's PRESENCE (defined vs + * not) rather than on ITS reference — a consumer passing a fresh inline `onRegenerate={(id) => …}` + * literal every render (the common case; see `MessageAffordances`'s own doc) no longer defeats + * `resolvedAffordances`'s memo, and transitively `MessageBlock`'s `affordances`-by-reference + * bail-out. Only a genuine defined-to-undefined (or back) transition changes the returned wrapper's + * presence. + * + * ONLY EVER APPLY THIS TO AN EVENT HANDLER — never to a render prop. The trick works precisely + * because the wrapper is invoked AFTER render (from a click), so freezing its identity costs + * nothing: React re-reads the ref at call time. A render prop (`MessageAffordances.actions`, which + * `MessageAffordanceRow` invokes DURING render to produce nodes) is the opposite case: its identity + * is the only signal React has that its OUTPUT might differ, so freezing it makes every memo above + * it bail out and the consumer's actions render permanently stale — a `pin`/`star` control wired to + * consumer state would never update again for the transcript's whole lifetime. `actions` is + * therefore deliberately keyed by reference in `resolvedAffordances`; a consumer that wants the + * memo win there wraps its own `actions` in `useCallback`, which is a choice only the consumer can + * make because only it knows what the closure reads. + */ +function useStableCallback( + callback: ((...args: Args) => R) | undefined, +): ((...args: Args) => R) | undefined { + const callbackRef = useRef(callback) + callbackRef.current = callback + // Deliberately empty deps: this is what keeps `stable`'s identity constant across renders. It + // reads `callbackRef.current` at CALL time (not at creation time), so it always invokes whatever + // the consumer most recently passed — never a closure captured at first mount. + const stable = useMemo<(...args: Args) => R>( + () => + (...args: Args) => + callbackRef.current?.(...args) as R, + [], + ) + return callback === undefined ? undefined : stable +} /** * Renders a thread's settled messages, each role-labelled ("You" / "Assistant"). Each part @@ -407,18 +1025,35 @@ export type ThreadTranscriptProps = ThreadTranscriptBase * the built-in six, then `fallbackRenderer`. When `liveParts` is non-empty, an extra in-progress * assistant block is appended at the tail. * + * Set `virtualize` (with a required `height`) to window a long thread over + * `@tanstack/react-virtual` (an optional peer) instead of rendering every message. The virtualized + * transcript owns its own fixed-height scroll container and must NOT be nested inside + * `BasaltStickToBottom` — see `./virtualize`'s module doc. With the peer absent, `virtualize` still + * renders (unwindowed, inside the same fixed-height pane) rather than throwing. A virtualized + * transcript scrolls itself to the newest message once, on mount (`initialScroll: 'start'` opts + * out) — see `./virtualize`'s fourth composition rule. + * * @example * import { ThreadTranscript } from 'basalt-ui' * + * + * @example + * // Windowed, for a long thread: + * */ -export function ThreadTranscript({ - messages, - liveParts, - liveStatus, - renderers: renderersProp, - fallbackRenderer = DEFAULT_FALLBACK_RENDERER, -}: ThreadTranscriptProps): JSX.Element { - // Mirrors PartList's own `useMemo` (part-list.tsx:237-240) — avoids rebuilding the renderer +export function ThreadTranscript(props: ThreadTranscriptProps): JSX.Element { + const { + messages, + liveParts, + liveStatus, + renderers: renderersProp, + fallbackRenderer = DEFAULT_FALLBACK_RENDERER, + affordances: affordancesProp, + groupConsecutive = true, + } = props + const virtualized = resolveVirtualize(props) + + // Mirrors PartList's own `useMemo` (part-list.tsx:290) — avoids rebuilding the renderer // lookup on every streaming re-render. const renderers = useMemo(() => renderersProp ?? EMPTY_RENDERERS, [renderersProp]) @@ -427,17 +1062,85 @@ export function ThreadTranscript({ return { id: '__live__', role: 'assistant', parts: liveParts as TranscriptPart[], createdAt: 0 } }, [liveParts]) - return ( - - {messages.map((message) => ( - - ))} - {liveMessage !== null && ( + const lastAssistantMessageId = useMemo(() => { + for (let i = messages.length - 1; i >= 0; i -= 1) { + const candidate = messages[i] + if (candidate?.role === 'assistant') return candidate.id + } + return null + }, [messages]) + + // `onRegenerate` is an EVENT HANDLER, so it goes through `useStableCallback` before this memo + // sees it — without that, a consumer's fresh inline `{ onRegenerate: (id) => … }` literal (the + // common case) changes every render, so keying the deps array on the raw field defeated this memo + // for exactly that case: `resolvedAffordances` (and transitively every `MessageBlock`'s + // `affordances` reference-equality bail-out) recomputed every render regardless of the + // timestamp/copy fields actually being stable. Keying on the STABLE wrapper's presence fixes + // that; only a genuine defined-to-undefined transition on the underlying handler invalidates. + // + // `actions` is deliberately NOT given the same treatment: it is a RENDER prop (invoked during + // render to produce nodes), so its reference is the only signal that its OUTPUT may have changed. + // Stabilizing it froze every consumer action at its first-render output for the transcript's + // whole lifetime. It is keyed by reference here, which means a fresh inline `actions` literal + // legitimately re-renders the blocks — the correct trade, and one a consumer can opt out of with + // its own `useCallback`. See `useStableCallback`'s doc. + const stableOnRegenerate = useStableCallback(affordancesProp?.onRegenerate) + const actions = affordancesProp?.actions + + const resolvedAffordances = useMemo( + () => ({ + timestamp: affordancesProp?.timestamp ?? DEFAULT_AFFORDANCES.timestamp, + copy: affordancesProp?.copy ?? DEFAULT_AFFORDANCES.copy, + // `exactOptionalPropertyTypes` forbids assigning a possibly-`undefined` value to an optional + // property directly — spread each field in only when it is actually present, rather than + // widening `MessageAffordances` itself to accept an explicit `undefined`. + ...(stableOnRegenerate !== undefined && { onRegenerate: stableOnRegenerate }), + ...(actions !== undefined && { actions }), + }), + [affordancesProp?.timestamp, affordancesProp?.copy, stableOnRegenerate, actions], + ) + + // Settled rows are memoized SEPARATELY from the live block: `liveMessage`/`liveStatus` change on + // every streamed chunk (a fresh `liveParts` array produces a fresh `liveMessage` object each + // time), so keeping them in the SAME memo as the settled rows recomputed every settled message's + // JSX descriptor on every chunk — O(n) allocation per token on a long thread, and it handed + // `useVirtualizer` a new `rows`/`count`/`getItemKey` identity every chunk too. Appending the live + // row OUTSIDE this memo means a streamed chunk only ever re-allocates the one live row plus a + // cheap wrapping array — the settled `MessageBlock` elements are the SAME object references + // React saw last render, so it bails out of that whole subtree without even reaching each + // `MessageBlock`'s memo comparator. + const settledRows = useMemo(() => { + const out: TranscriptRow[] = [] + messages.forEach((message, index) => { + out.push({ + key: message.id, + node: ( + + ), + }) + }) + return out + }, [ + messages, + renderers, + fallbackRenderer, + groupConsecutive, + lastAssistantMessageId, + resolvedAffordances, + ]) + + const liveRow = useMemo(() => { + if (liveMessage === null) return null + return { + key: liveMessage.id, + node: (
- )} + ), + } + }, [liveMessage, liveStatus, renderers, fallbackRenderer]) + + const rows = useMemo( + () => (liveRow === null ? settledRows : [...settledRows, liveRow]), + [settledRows, liveRow], + ) + + if (virtualized !== null) { + return ( + + ) + } + + return ( + + {rows.map((row) => ( + {row.node} + ))} ) } diff --git a/packages/basalt-ui/src/agent-chat/thread-outcome-card.tsx b/packages/basalt-ui/src/agent-chat/thread-outcome-card.tsx index 20644b7..553e26a 100644 --- a/packages/basalt-ui/src/agent-chat/thread-outcome-card.tsx +++ b/packages/basalt-ui/src/agent-chat/thread-outcome-card.tsx @@ -23,6 +23,7 @@ import { useHover } from '@mantine/hooks' import type { JSX } from 'react' import type { AgentThread, ThreadStatus } from '../agent' import { alpha, VX } from '../tokens' +import { formatRelativeTime } from './relative-time' // ── Status badge — shown ONLY for states that need a glance (attention/error). ──── // done/pending/streaming stay badge-free: a settled feed shouldn't be a wall of green chips — @@ -35,31 +36,6 @@ const STATUS_BADGE: Partial< error: { label: 'Failed', statusToken: VX.status.bad }, } -// ── Dependency-free relative-time helper (no date-fns) ──────────────────────── - -const RELATIVE_TIME_FORMAT = new Intl.RelativeTimeFormat('en', { numeric: 'auto' }) - -const RELATIVE_TIME_UNITS: readonly { - readonly unit: Intl.RelativeTimeFormatUnit - readonly ms: number -}[] = [ - { unit: 'year', ms: 31_536_000_000 }, - { unit: 'month', ms: 2_628_000_000 }, - { unit: 'week', ms: 604_800_000 }, - { unit: 'day', ms: 86_400_000 }, - { unit: 'hour', ms: 3_600_000 }, - { unit: 'minute', ms: 60_000 }, -] - -/** Formats an epoch-ms timestamp as a short relative string ("3 hours ago", "just now"). */ -function formatRelativeTime(timestamp: number): string { - const diffMs = timestamp - Date.now() - const absMs = Math.abs(diffMs) - if (absMs < 60_000) return 'just now' - const unit = RELATIVE_TIME_UNITS.find(({ ms }) => absMs >= ms) ?? RELATIVE_TIME_UNITS.at(-1)! - return RELATIVE_TIME_FORMAT.format(Math.round(diffMs / unit.ms), unit.unit) -} - // ── Row bodies ───────────────────────────────────────────────────────────────── /** Preview skeleton shown while a thread has no resolved outcome yet (never raw prompt/live text). */ diff --git a/packages/basalt-ui/src/agent-chat/thread-workspace.test.tsx b/packages/basalt-ui/src/agent-chat/thread-workspace.test.tsx new file mode 100644 index 0000000..3d2b34f --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/thread-workspace.test.tsx @@ -0,0 +1,169 @@ +/** + * ThreadWorkspace — hydration gating (D1). + * + * A store built with `createAdapterThreadsStore` starts `!hydrated` until its first `listThreads` + * succeeds. Before this fix, `ThreadWorkspace` read only `threads.length` to decide between the + * feed and the empty state, so a server-backed store with real threads (a database, a real API) + * would render "no threads yet" for the whole initial round trip, then swap to the populated feed + * once it landed — a defect invisible in the playground's demo only because that demo's in-memory + * adapter seeds an empty Map, so there is nothing to flash to. + * + * These tests drive `ThreadWorkspace` directly against a hand-built `ThreadsStore` (not a real + * `createAdapterThreadsStore` instance) so each case can assert one exact `{ hydrated, threads }` + * combination without racing a real async adapter. + */ +import { MantineProvider } from '@mantine/core' +import { cleanup, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, test } from 'bun:test' +import type { ReactElement } from 'react' +import type { AgentThread, AgentTransport, OutcomeResolver, ThreadsStore } from '../agent' +import { createThreadsStore } from '../agent' +import { ThreadWorkspace } from './thread-workspace' + +afterEach(cleanup) + +// `useMediaQuery` reads `window.matchMedia` inside a mount effect RTL flushes synchronously — +// force the wide-viewport branch so every case renders the feed pane (same idiom as +// thread-feed.test.tsx's `withReducedMotion`). +function withWideViewport(fn: () => T): T { + const original = window.matchMedia + window.matchMedia = (query: string): MediaQueryList => + ({ + matches: false, + media: query, + onchange: null, + addListener: () => {}, + removeListener: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, + }) as MediaQueryList + try { + return fn() + } finally { + window.matchMedia = original + } +} + +function noop(): void {} + +/** A transport that never resolves — no case here calls `start()`, so `stream` is never invoked. */ +const stubTransport: AgentTransport = { + stream: (): AsyncGenerator => { + throw new Error('stubTransport.stream must not be called by these tests') + }, +} + +const stubResolveOutcome: OutcomeResolver = async (thread) => ({ + title: thread.id, + summary: '', + status: 'done', +}) + +function buildThread(id: string): AgentThread { + return { + id, + messages: [], + outcome: { title: id, summary: '', status: 'done' }, + status: 'done', + read: true, + createdAt: 0, + updatedAt: 0, + } +} + +/** A minimal fixed `ThreadsStore` — every mutator is a no-op since no case here calls one. */ +function makeStore(overrides: Partial): ThreadsStore { + return { + threads: [], + activeId: null, + hydrated: true, + error: undefined, + select: noop, + create: () => 'unused', + appendMessage: noop, + setOutcome: noop, + setStatus: noop, + setResumeToken: noop, + markRead: noop, + remove: noop, + clear: noop, + ...overrides, + } +} + +function renderWorkspace(store: ThreadsStore): ReactElement { + return ( + + store} + transport={stubTransport} + resolveOutcome={stubResolveOutcome} + /> + + ) +} + +describe('ThreadWorkspace hydration gating', () => { + test('hydrating + empty: renders the hydrating hold, never the empty-state copy', () => { + withWideViewport(() => { + render(renderWorkspace(makeStore({ hydrated: false, threads: [] }))) + }) + + expect(screen.getByTestId('thread-workspace-hydrating')).toBeDefined() + expect(screen.queryByText('No threads yet')).toBeNull() + }) + + test('hydrating + already-populated: renders the feed content, not the hydrating hold', () => { + withWideViewport(() => { + render(renderWorkspace(makeStore({ hydrated: false, threads: [buildThread('cached-1')] }))) + }) + + expect(screen.getByText('cached-1')).toBeDefined() + expect(screen.queryByTestId('thread-workspace-hydrating')).toBeNull() + }) + + test('failed load: renders the error state, never the hydrating hold', () => { + withWideViewport(() => { + render( + renderWorkspace( + makeStore({ hydrated: false, error: new Error('listThreads failed'), threads: [] }), + ), + ) + }) + + expect(screen.getByTestId('thread-workspace-error')).toBeDefined() + expect(screen.queryByTestId('thread-workspace-hydrating')).toBeNull() + }) + + test('hydrated + empty: renders the empty state (no threads really is the truth)', () => { + withWideViewport(() => { + render(renderWorkspace(makeStore({ hydrated: true, threads: [] }))) + }) + + expect(screen.getByText('No threads yet')).toBeDefined() + expect(screen.queryByTestId('thread-workspace-hydrating')).toBeNull() + }) + + test('real synchronous createThreadsStore: never shows the hydrating hold, even on first render', () => { + // The real localStorage-backed store (hydrated pinned `true` — see thread.test.ts), not the + // hand-built stub above, so this proves the gate reads a genuine ThreadsStore correctly and + // does not regress the synchronous path with a permanent skeleton. + const useThreads = createThreadsStore({ key: 'thread-workspace-sync-empty', version: 1 }) + + withWideViewport(() => { + render( + + + , + ) + }) + + expect(screen.getByText('No threads yet')).toBeDefined() + expect(screen.queryByTestId('thread-workspace-hydrating')).toBeNull() + }) +}) diff --git a/packages/basalt-ui/src/agent-chat/thread-workspace.tsx b/packages/basalt-ui/src/agent-chat/thread-workspace.tsx index ec44f89..a4fc9ed 100644 --- a/packages/basalt-ui/src/agent-chat/thread-workspace.tsx +++ b/packages/basalt-ui/src/agent-chat/thread-workspace.tsx @@ -32,11 +32,12 @@ * ) * } */ -import { Box, Divider, Flex, Stack, Text } from '@mantine/core' +import { Box, Divider, Flex, Skeleton, Stack, Text } from '@mantine/core' import { useMediaQuery } from '@mantine/hooks' import type { JSX, ReactNode } from 'react' import type { AgentPart, AgentTransport, OutcomeResolver, ThreadsStore } from '../agent' import { useAgentThreadRuns } from '../agent' +import { VX } from '../tokens' import { Composer } from './composer' import { ThreadDetailPanel } from './thread-detail-panel' import { ThreadFeed } from './thread-feed' @@ -56,11 +57,23 @@ export type ThreadWorkspaceProps = { readonly resolveOutcome: OutcomeResolver /** Placeholder for the new-thread composer pinned under the feed. */ readonly newThreadPlaceholder?: string - /** Rendered in place of the feed while there are no threads yet. */ + /** + * Rendered in place of the feed once the store is hydrated and there really are no threads. + * Shown ONLY when `store.hydrated` is true — while an async store's initial load is still in + * flight, `ThreadWorkspace` holds a neutral loading state instead (see `FeedHydratingState`), + * so a server-backed workspace never asserts "no threads yet" before it actually knows that. + */ readonly emptyState?: ReactNode + /** + * Rendered in place of the feed when the store's initial load has failed — `!store.hydrated && + * store.error !== undefined` (see `ThreadsStore.hydrated`'s doc for the discriminant). Shown + * instead of `FeedHydratingState`, so a rejected `listThreads` surfaces a legible failure rather + * than an endless skeleton. + */ + readonly errorState?: ReactNode } -/** Default hint shown in the feed pane before any thread exists (overridable via `emptyState`). */ +/** Default hint shown in the feed pane once hydrated with no threads (overridable via `emptyState`). */ function FeedEmptyState(): JSX.Element { return ( @@ -74,6 +87,50 @@ function FeedEmptyState(): JSX.Element { ) } +/** + * Neutral hold shown in the feed pane while an async store's initial `listThreads` is still in + * flight, has no threads to show yet, AND has not failed (`!store.hydrated && threads.length === + * 0 && store.error === undefined`). Never shown for the synchronous `createThreadsStore`, whose + * `hydrated` is always `true` — see `ThreadsStore.hydrated`'s doc. A store that already holds + * cached threads while revalidating (`!hydrated` with a non-empty `threads`) skips this entirely + * and renders the feed normally. A failed load renders `FeedErrorState` instead of this. + */ +function FeedHydratingState(): JSX.Element { + return ( + + + + + + ) +} + +/** + * Default failure hold shown in the feed pane when an async store's initial `listThreads` has + * rejected (`!store.hydrated && store.error !== undefined`), overridable via `errorState`. Never + * shown for the synchronous `createThreadsStore`, whose `error` is always `undefined` — see + * `ThreadsStore.error`'s doc. + */ +function FeedErrorState(): JSX.Element { + return ( + + + Couldn't load threads + + + Something went wrong loading your threads. Try refreshing the page. + + + ) +} + /** * A two-pane master-detail thread workspace: a scrollable feed of threads with an anchored * new-thread composer, and a detail panel for the open thread's live transcript. Collapses to a @@ -88,6 +145,7 @@ export function ThreadWorkspace({ resolveOutcome, newThreadPlaceholder, emptyState, + errorState, }: ThreadWorkspaceProps): JSX.Element { const store = useThreads() const runs = useAgentThreadRuns({ transport, store, resolveOutcome }) @@ -130,8 +188,14 @@ export function ThreadWorkspace({ const feed = ( - {store.threads.length === 0 ? ( - (emptyState ?? ) + {threads.length === 0 ? ( + store.hydrated ? ( + (emptyState ?? ) + ) : store.error !== undefined ? ( + (errorState ?? ) + ) : ( + + ) ) : ( )} diff --git a/packages/basalt-ui/src/agent-chat/virtualize.ts b/packages/basalt-ui/src/agent-chat/virtualize.ts new file mode 100644 index 0000000..c6e0de1 --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/virtualize.ts @@ -0,0 +1,132 @@ +/** + * Virtualization contract shared by the transcript/feed components that can opt into + * `@tanstack/react-virtual` for long lists. + * + * The `VirtualizeProps` union IS the guard: a virtualizer needs a measured scroll container, so + * turning `virtualize` on REQUIRES a `height` and turning it off (or omitting it) FORBIDS one. See + * `virtualize.type-guard.test.ts` beside this file for a compile-time proof that the union actually + * rejects the invalid combinations. + * + * The one piece of runtime here is {@link resolveVirtualize} — the single narrowing point every + * component that accepts `VirtualizeProps` goes through, so no call site has to re-derive (or + * assert its way past) the union's `virtualize`-implies-`height` link. + * + * The other half of the contract, not encoded in the types (it's a runtime/composition rule, not + * a shape): a virtualized transcript OWNS ITS OWN SCROLL NODE (the element `getScrollElement` + * measures) and must NOT be nested inside `BasaltStickToBottom` — unlike the non-virtual path, + * which `ThreadDetailPanel` nests inside `BasaltStickToBottom` today (its transcript body). + * Stacking a virtualizer's own scroll container inside `BasaltStickToBottom`'s would give the + * transcript two competing owners of "which element scrolls" and break both the virtualizer's + * `scrollToEnd` and stick-to-bottom's anchor tracking. + * + * A third composition rule, same category (runtime, not a shape): a virtualized transcript + * tolerates being hidden via `display: none` on an ANCESTOR (e.g. sitting inside a collapsible + * `ThreadFeedRow`) — collapsing does not corrupt its measurement cache, and re-expanding restores + * the same scroll position (same top-visible row) it had before the collapse. This is NOT free of + * virtual-core: hiding an ancestor makes every mounted row's ResizeObserver box report `0` (a + * `display: none` ancestor un-lays-out its descendants), and virtual-core 3.17.1's default + * `measureElement`/`resizeItem` write that `0` into `itemSizeCache` with no floor — unguarded, a + * few collapse/expand cycles permanently shrink `getTotalSize()` and silently scroll the visible + * window to a DIFFERENT part of the transcript (`scrollTop` stays put; the content under it moves). + * `thread-message.tsx` closes this with a `measureElement` override (`resolveGuardedMeasurement`) + * that treats a `0` reading as unreliable and keeps the last-known size instead of committing it — + * see that function's doc for the full mechanism, including why `useVirtualizer`'s `enabled` option + * is NOT the fix (it wipes the measurement cache outright, worse than the bug). A consumer combining a + * virtualized transcript with its own hide-via-CSS container gets this for free; a consumer who + * unmounts/remounts the transcript instead of hiding it is unaffected either way (a fresh + * `useVirtualizer` call has no stale cache to poison). + * + * A fourth composition rule: a virtualized transcript SCROLLS ITSELF on mount. `initialScroll` + * defaults to `'end'` — `anchorTo: 'end'` and `followOnAppend` (both wired in `thread-message.tsx`) + * only keep an ALREADY-at-the-end transcript pinned as content changes; neither performs the initial + * jump, so left alone a freshly mounted virtualized transcript renders at message #0, not the + * newest. The fix fires exactly once, on first mount, via `scrollToEnd()` — never again, so a user + * who has scrolled up is never yanked back down by a later append (that is precisely the failure + * `followOnAppend` exists to avoid, and re-introducing it here would be worse than the defect this + * closes). A consumer wiring its OWN scroll-restoration (e.g. `initialOffset` / + * `initialMeasurementsCache`, restoring a prior session's position) wants `initialScroll: 'start'` — + * otherwise this default jump fires first and the restoration has to fight it. `initialScroll` is + * ignored (no scroll, no throw) for an empty transcript, and is deferred rather than treated as done + * if the transcript's own scroll container measures 0px on mount (e.g. mounted behind a hidden + * ancestor) — it then lands on the first commit where the container is genuinely measurable, which + * is what makes "opens at the newest message" hold for a transcript whose lazy virtualizer settled + * while its `ThreadFeedRow` was collapsed. See `thread-message.tsx`'s `applyInitialScroll` for the + * guard, and `VirtualizedRowsInner` for why its effect is deliberately dependency-free. + * + * Firing `scrollToEnd()` once is not sufficient on its own to land there, because virtual-core + * 3.17.1's own `_willUpdate` writes to the SAME scroll container on the very next commit it runs + * on, from state that lags behind the DOM: `_scrollToOffset` only ever records the write's + * intent (`_intendedScrollOffset`) — `this.scrollOffset` itself updates ASYNCHRONOUSLY, from the + * native `scroll` event, not from the call that requested it. So a commit that runs `_willUpdate` + * before this effect had a chance to fire (element-attach) or that re-derives an anchor from + * `getScrollOffset()` while `this.scrollOffset` is still the pre-jump value (the anchor branch, + * gated on `pendingScrollAnchor`) writes that stale offset — 0, on first mount — straight back to + * `scrollTop`, clobbering this effect's jump within roughly one extra commit of it landing. + * `applyInitialScroll` survives this the same way the hidden-ancestor case above already + * (accidentally) does: by not treating one successful call as permanent. It fires, then — on a + * LATER call, i.e. a later commit, exactly the cadence this dependency-free effect already runs on + * — reads the container's REAL `scrollTop`/`scrollHeight`/`clientHeight` (never virtual-core's own + * `scrollOffset`, which is exactly the stale value that caused the clobber) to check whether the + * jump survived; if it didn't, it re-fires, bounded by `MAX_INITIAL_SCROLL_ATTEMPTS` so a DOM that + * pathologically never reports settled can never be fought forever. `resolveInitialScrollAction` + * itself is untouched by any of this — it is still the one-shot "should I even attempt this" + * decision; `applyInitialScroll` layers the across-commits verification on top. + * + * `initialOffset` (`number | (() => number)`, `virtual-core`'s own construction-time seed for + * `scrollOffset`) was investigated as a way to avoid the clobber altogether and rejected: it takes + * a concrete pixel number, not an `'end'` sentinel, and the only number available at the point + * it would need to resolve — before any row has ever been measured — is an ESTIMATE + * (`estimateSize` × count), not the real total. It would relocate the transcript's initial paint + * from message #0 to an approximately-right position, but the moment real measurements replace + * those estimates the total size (and therefore the true end offset) changes, which still needs a + * corrective `scrollToEnd()` once rows have actually measured — the exact write this whole + * mechanism exists to protect. It also can't reference the `virtualizer` instance it would be + * computing an offset for without a separate ref indirection, since it is itself one of the options + * passed to construct that instance. Not a fix for this defect, just a smaller wrong first paint. + */ + +/** Tuning knobs passed through to `@tanstack/react-virtual`'s `useVirtualizer`. */ +export type VirtualizeOptions = { + /** Extra rows rendered beyond the visible viewport, each side. Library default: 1. */ + readonly overscan?: number + /** Estimated row height in px, used before a row has been measured. */ + readonly estimateSize?: number + /** Where the transcript sits on first mount. Default 'end' — a chat transcript opens at the + newest message. 'start' opens at the oldest. See this module's fourth composition rule + above for the mechanism and why a consumer restoring its own scroll position wants 'start'. */ + readonly initialScroll?: 'end' | 'start' +} + +/** + * `virtualize: false` (or omitted) → no `height` prop (content-sized, non-scrolling-owner layout). + * `virtualize: true | VirtualizeOptions` → `height` is REQUIRED — the virtualizer measures a fixed + * scroll container, so there is no valid virtualized layout without one. + */ +export type VirtualizeProps = + | { readonly virtualize?: false; readonly height?: never } + | { readonly virtualize: true | VirtualizeOptions; readonly height: number | string } + +/** The enabled branch of {@link VirtualizeProps}, with `height` no longer optional. */ +export type ResolvedVirtualize = { + readonly options: VirtualizeOptions + readonly height: number | string +} + +/** + * Narrows a `VirtualizeProps` pair once, centrally: `null` when virtualization is off, otherwise + * the options object (normalizing the bare `virtualize: true` shorthand to `{}`) alongside the + * `height` the union guarantees is present. Every component accepting `VirtualizeProps` resolves + * through here rather than destructuring the two fields apart — destructuring severs the link TS + * needs, which is what forces a non-null assertion on `height` at each call site. + */ +export function resolveVirtualize(props: VirtualizeProps): ResolvedVirtualize | null { + const { virtualize } = props + if (virtualize === undefined || virtualize === false) return null + // The union already REQUIRES `height` on this branch and rejects it on the other — that guarantee + // is enforced where it matters, at every call site (see virtualize.type-guard.test.ts). What TS + // won't do is propagate the narrowing FROM `virtualize` TO `height`: `virtualize`'s enabled branch + // is `true | VirtualizeOptions`, and the object constituent disqualifies it as a discriminant + // property. So the pairing is re-stated once, here, instead of a `height!` at each consumer. + const height = props.height as number | string + return { options: virtualize === true ? {} : virtualize, height } +} diff --git a/packages/basalt-ui/src/agent-chat/virtualize.type-guard.test.ts b/packages/basalt-ui/src/agent-chat/virtualize.type-guard.test.ts new file mode 100644 index 0000000..31fcf4b --- /dev/null +++ b/packages/basalt-ui/src/agent-chat/virtualize.type-guard.test.ts @@ -0,0 +1,49 @@ +/** + * Compile-time proof that `VirtualizeProps` actually rejects the invalid `virtualize`/`height` + * combinations (not just documentation). + * + * This mirrors the `apps/playground/src/*.type-guard.ts` convention (one `@ts-expect-error` per bad + * line, proven by CI's `tsc --noEmit`) but lives HERE, beside the type, rather than in the + * playground app. `VirtualizeProps` IS public now (re-exported type-only from the agent-chat barrel + * and the root), so a playground fixture would also work — this stays put because a source-relative + * import proves the union at its definition, before the build, and needs no rebuild to stay honest. + * A co-located `.type-guard.ts` file under `src/` would get picked up by tsup's + * `src/**\/*.{ts,tsx}` build glob and ship an unused fixture in the published package; naming it + * `*.test.ts` excludes it from that glob (tsup.config.ts excludes `src/**\/*.test.{ts,tsx}`) while + * this package's own `tsc --noEmit` (and `bun test`, which type-checks test files per the repo + * convention) still verifies it. + */ +import { describe, expect, test } from 'bun:test' +import type { VirtualizeProps } from './virtualize' + +function accept(props: VirtualizeProps): VirtualizeProps { + return props +} + +// ── Valid combinations — must type-check with no error ──────────────────────── + +accept({}) +accept({ virtualize: false }) +accept({ virtualize: true, height: 400 }) +accept({ virtualize: true, height: '100%' }) +accept({ virtualize: { overscan: 3 }, height: 400 }) +accept({ virtualize: { estimateSize: 72 }, height: '50vh' }) + +// ── Invalid combinations — each MUST be a tsc error, one directive per bad line ─ + +// @ts-expect-error `height` is forbidden when `virtualize` is false +accept({ virtualize: false, height: 400 }) +// @ts-expect-error `height` is required when `virtualize: true` +accept({ virtualize: true }) +// @ts-expect-error `height` is required when `virtualize` is a VirtualizeOptions object +accept({ virtualize: { overscan: 2 } }) +// @ts-expect-error `height` is forbidden when `virtualize` is omitted (defaults to the `false` branch) +accept({ height: 400 }) + +describe('VirtualizeProps (type-guard)', () => { + test('is a compile-time-only fixture — see the @ts-expect-error directives above', () => { + // No runtime behavior to assert: the proof is that this file type-checks at all. A single + // trivial assertion keeps this a normal bun:test file rather than an empty-suite oddity. + expect(true).toBe(true) + }) +}) diff --git a/packages/basalt-ui/src/agent/adapter.test.ts b/packages/basalt-ui/src/agent/adapter.test.ts index a90bf9f..f163bbb 100644 --- a/packages/basalt-ui/src/agent/adapter.test.ts +++ b/packages/basalt-ui/src/agent/adapter.test.ts @@ -134,6 +134,21 @@ describe('threadsStoreAdapterContract', () => { expect(new Set(cases.map((c) => c.name)).size).toBe(cases.length) }) + test("the contract suite itself does not depend on crypto.randomUUID — it still runs to completion via the getRandomValues fallback (mintThreadId, since every id it mints here is a THREAD id, never appendMessage's idempotency key)", async () => { + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: { getRandomValues: originalCrypto.getRandomValues.bind(originalCrypto) }, + configurable: true, + }) + try { + for (const conformanceCase of threadsStoreAdapterContract(() => createMemoryAdapter())) { + await conformanceCase.run() + } + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) + test('FAILS an adapter whose appendMessage is not idempotent on message.id', async () => { const cases = threadsStoreAdapterContract(() => { const memory = createMemoryAdapter() @@ -417,6 +432,228 @@ describe('createAdapterThreadsStore', () => { expect(result.current.threads[0]?.read).toBe(true) }) + // ── rolled-back create() and its dependent writes ─────────────────────────── + + test('a rolled-back create surfaces its own error, not a cascade from the writes queued behind it', async () => { + const calls: string[] = [] + const memory = createMemoryAdapter() + const useThreads = createAdapterThreadsStore({ + ...memory, + createThread: async () => { + calls.push('createThread') + throw new Error('create-boom') + }, + // Mirrors the real finding: a real backend REJECTS a write against a row that was never + // created, it does not silently no-op the way the permissive reference `memory` adapter + // does — so these reproduce the exact "unknown thread" cascade from the console trace. + markRead: async (id) => { + calls.push('markRead') + if ((await memory.loadThread(id)) === null) + throw new Error(`markRead: unknown thread ${id}`) + await memory.markRead(id) + }, + appendMessage: async (i) => { + calls.push('appendMessage') + if ((await memory.loadThread(i.threadId)) === null) { + throw new Error(`appendMessage: unknown thread ${i.threadId}`) + } + await memory.appendMessage(i) + }, + setStatus: async (i) => { + calls.push('setStatus') + if ((await memory.loadThread(i.threadId)) === null) { + throw new Error(`setStatus: unknown thread ${i.threadId}`) + } + await memory.setStatus(i) + }, + }) + + const { result } = renderHook(() => useThreads()) + await waitFor(() => { + expect(result.current.hydrated).toBe(true) + }) + + // Exactly the observed sequence: create() -> markRead() -> appendMessage() -> setStatus() in + // one synchronous block, every dependent write chained behind the same rejected createThread. + let id = '' + act(() => { + id = result.current.create() + result.current.markRead(id) + result.current.appendMessage(id, makeMessage('m-first')) + result.current.setStatus(id, 'streaming') + }) + + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error) + }) + await settle() + + // The surfaced error names the create, not one of the three cascade rejections that would + // otherwise overwrite it in issue order. + expect((result.current.error as Error).message).toBe('create-boom') + // The dependent writes never reached the adapter at all — dropped, not run-and-failed. + expect(calls).toEqual(['createThread']) + // Rollback semantics are untouched: the thread is gone, both locally and server-side. + expect(result.current.threads).toHaveLength(0) + expect(await memory.loadThread(id)).toBeNull() + }) + + test('a rolled-back create does not stall or spoil the write queue for a different thread', async () => { + const memory = createMemoryAdapter() + const calls: string[] = [] + let createCalls = 0 + const useThreads = createAdapterThreadsStore({ + ...memory, + createThread: async (i) => { + createCalls += 1 + if (createCalls === 1) { + calls.push('createThread:doomed') + throw new Error('create-boom') + } + calls.push('createThread:healthy') + await memory.createThread(i) + }, + markRead: async (id) => { + calls.push(`markRead:${id}`) + await memory.markRead(id) + }, + }) + + const { result } = renderHook(() => useThreads()) + await waitFor(() => { + expect(result.current.hydrated).toBe(true) + }) + + let doomed = '' + let healthy = '' + act(() => { + doomed = result.current.create() + result.current.markRead(doomed) + healthy = result.current.create() + result.current.markRead(healthy) + }) + + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error) + }) + await waitFor(async () => { + expect(await memory.loadThread(healthy)).not.toBeNull() + }) + await settle() + + expect((result.current.error as Error).message).toBe('create-boom') + // doomed's markRead was dropped; healthy's own write for a DIFFERENT thread ran normally. + expect(calls).toEqual(['createThread:doomed', 'createThread:healthy', `markRead:${healthy}`]) + expect((await memory.loadThread(healthy))?.read).toBe(true) + expect(await memory.loadThread(doomed)).toBeNull() + }) + + test('a genuine failure that arrives after a rolled-back create still surfaces', async () => { + const memory = createMemoryAdapter() + let createCalls = 0 + const useThreads = createAdapterThreadsStore({ + ...memory, + createThread: async (i) => { + createCalls += 1 + if (createCalls === 1) throw new Error('create-boom') + await memory.createThread(i) + }, + markRead: async () => { + // A genuine, unrelated failure against a HEALTHY thread — not a cascade of the rollback. + throw new Error('markRead-boom') + }, + }) + + const { result } = renderHook(() => useThreads()) + await waitFor(() => { + expect(result.current.hydrated).toBe(true) + }) + + act(() => { + result.current.create() + }) + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error) + }) + expect((result.current.error as Error).message).toBe('create-boom') + + let healthy = '' + act(() => { + healthy = result.current.create() + }) + await waitFor(async () => { + expect(await memory.loadThread(healthy)).not.toBeNull() + }) + + act(() => { + result.current.markRead(healthy) + }) + + await waitFor(() => { + expect((result.current.error as Error).message).toBe('markRead-boom') + }) + }) + + test('a rolled-back create keeps dropping its thread’s writes after the queue drains', async () => { + // The run's COMPLETION path (useAgentThreadRuns: appendMessage(assistant) -> setOutcome -> + // setStatus -> setResumeToken) lands when the stream ends, long after the create's own + // rollback has drained the per-thread chain. Those writes target the same rolled-back row, so + // they are the same cascade as the synchronous send-path trio — just later. If the failed-id + // record is cleared when the chain drains, the fix above covers the first ~200ms and then the + // cascade returns at stream end, which is precisely when the user is looking at the thread. + const calls: string[] = [] + const memory = createMemoryAdapter() + const useThreads = createAdapterThreadsStore({ + ...memory, + createThread: async () => { + calls.push('createThread') + throw new Error('create-boom') + }, + appendMessage: async (i) => { + calls.push('appendMessage') + if ((await memory.loadThread(i.threadId)) === null) { + throw new Error(`appendMessage: unknown thread ${i.threadId}`) + } + await memory.appendMessage(i) + }, + setStatus: async (i) => { + calls.push('setStatus') + if ((await memory.loadThread(i.threadId)) === null) { + throw new Error(`setStatus: unknown thread ${i.threadId}`) + } + await memory.setStatus(i) + }, + }) + + const { result } = renderHook(() => useThreads()) + await waitFor(() => { + expect(result.current.hydrated).toBe(true) + }) + + let id = '' + act(() => { + id = result.current.create() + }) + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error) + }) + // Drain fully: the create's chain entry is gone by the time the completion writes arrive. + await settle() + await settle() + expect((result.current.error as Error).message).toBe('create-boom') + + // The completion path fires now, against the row that was never materialized. + act(() => { + result.current.appendMessage(id, makeMessage('m-assistant')) + result.current.setStatus(id, 'done') + }) + await settle() + await settle() + + expect(calls).toEqual(['createThread']) + expect((result.current.error as Error).message).toBe('create-boom') + }) + test('setResumeToken(undefined) clears the key through the adapter', async () => { const memory = createMemoryAdapter() const useThreads = createAdapterThreadsStore(memory) diff --git a/packages/basalt-ui/src/agent/adapter.ts b/packages/basalt-ui/src/agent/adapter.ts index 93d44e5..c2426c1 100644 --- a/packages/basalt-ui/src/agent/adapter.ts +++ b/packages/basalt-ui/src/agent/adapter.ts @@ -168,7 +168,11 @@ type ThreadPatch = { * - **Roll back on rejection.** A rejected write's patch is discarded, the rejection is surfaced * on `error`, and the store re-lists to converge on whatever the server really did. Patches are * tracked individually, so a failing write rolls back only itself and not whatever else was in - * flight beside it. + * flight beside it. A rejected `create()` additionally drops every later single-thread write for + * that id (`markRead`/`appendMessage`/`setStatus`/… — both the ones chained directly behind it and + * the run's completion writes at stream end) instead of letting each fail independently against a + * row that no longer exists — see `failedCreateIds`. Without that, the create's own error is + * overwritten by a run of "unknown thread" rejections that name nothing a caller can act on. * - **`hydrated` means "a `listThreads` has succeeded"**, not "a load has been attempted". A * store that is `!hydrated` with a non-undefined `error` failed to load; check both. * - **`error` latches the most recent failure until something disproves it.** A successful @@ -412,6 +416,31 @@ export function createAdapterThreadsStore( */ const chains = new Map>() + /** + * Thread ids whose `createThread` has rejected and been rolled back. The send path issues + * `create()` → `markRead()` → `appendMessage()` → `setStatus()` in one synchronous block, all + * chained behind the same per-thread queue above — so when `createThread` rejects, the three + * dependent writes queued right behind it are guaranteed to reach the adapter next, each against + * a row that was never materialized. Left alone, each of those independently rejects with its own + * "unknown thread" error and overwrites the one that actually explains what happened (the + * create's), so the user ends up staring at the last cascade error instead of the root cause. + * + * Recording the id here lets `mutate` recognize a single-thread write against a rolled-back + * create and drop it silently instead of running a doomed commit — see `mutate`'s `isCreate` + * guard. + * + * Entries are NEVER removed, deliberately. The cascade is not confined to the synchronous block: + * `useAgentThreadRuns`' COMPLETION path (`appendMessage(assistant)` → `setOutcome` → `setStatus` + * → `setResumeToken`) fires when the stream ends, long after this thread's write chain has + * drained — same rolled-back row, same "unknown thread" rejections, and landing at exactly the + * moment the user is watching the thread. Clearing the entry when the chain drains would fix the + * first few hundred milliseconds and hand the cascade straight back at stream end. Retaining it + * is safe and bounded: `create()` always mints its own id (`mintThreadId`, never caller-supplied), + * so an id in here can never be re-created and the set only grows by one per FAILED create in a + * session. + */ + const failedCreateIds = new Set() + /** `task` must never reject, so a failed write cannot poison the chain behind it. */ function enqueue(threadIds: readonly string[], task: () => Promise): void { const ids = [...new Set(threadIds)] @@ -421,6 +450,7 @@ export function createAdapterThreadsStore( for (const id of ids) chains.set(id, run) void run.finally(() => { // Only the tail clears its own entry, so a later write chained behind it is not orphaned. + // `failedCreateIds` is deliberately NOT cleared here — see its doc. for (const id of ids) if (chains.get(id) === run) chains.delete(id) }) } @@ -438,6 +468,12 @@ export function createAdapterThreadsStore( readonly apply: ThreadPatch['apply'] readonly applyActive?: (activeId: string | null) => string | null readonly commit: () => Promise + /** + * True only for `create()`'s own write. On rejection, marks its thread id as failed so the + * dependent writes chained behind it in the same synchronous send-path block recognize the row + * was rolled back — see `failedCreateIds`. + */ + readonly isCreate?: boolean }): void { const patch: ThreadPatch = { apply: memoizeApply(i.apply), @@ -448,9 +484,30 @@ export function createAdapterThreadsStore( const tokenAtIssue = errorToken recompute() enqueue(i.threadIds, async (): Promise => { + // A single-thread write against a create that already rolled back cannot succeed — the row + // was never materialized, and running it only manufactures a second "unknown thread" error + // that would bury the create's own. This covers both the writes chained directly behind the + // create and the run's later completion writes (see `failedCreateIds`). Scoped to + // single-thread writes deliberately: + // `clear()`'s multi-id commit already has its own per-id fan-out (`Promise.allSettled`) and + // `removeThread` is contractually a no-op on an unknown id anyway, so folding this check into + // a multi-id write would risk dropping legitimate deletes for OTHER, healthy threads in the + // same batch just because one id in it happened to be orphaned. + const [soleThreadId] = i.threadIds + if ( + !i.isCreate && + i.threadIds.length === 1 && + soleThreadId !== undefined && + failedCreateIds.has(soleThreadId) + ) { + drop(patch) + recompute() + return + } try { await i.commit() } catch (cause) { + if (i.isCreate) for (const threadId of i.threadIds) failedCreateIds.add(threadId) // Roll back: discard THIS patch only, leaving any other in-flight patch applied. drop(patch) error = cause @@ -540,6 +597,7 @@ export function createAdapterThreadsStore( mutate({ threadIds: [id], apply: (threads) => (threads.some((t) => t.id === id) ? [...threads] : [thread, ...threads]), + isCreate: true, commit: () => adapter.createThread({ id, @@ -738,7 +796,7 @@ export function threadsStoreAdapterContract( return [ define('createThread then listThreads returns the thread with empty defaults', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) const threads = await a.listThreads() const found = threads.filter((t) => t.id === id) @@ -754,19 +812,19 @@ export function threadsStoreAdapterContract( }), define('createThread persists meta', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id, meta: { source: 'contract' } }) const thread = await loadOrThrow(a, id) assert(thread.meta?.['source'] === 'contract', 'meta did not round-trip') }), define('loadThread returns null for an unknown id', async (a) => { - const thread = await a.loadThread(crypto.randomUUID()) + const thread = await a.loadThread(mintThreadId()) assert(thread === null, 'loadThread must return null (not throw, not undefined) when absent') }), define('appendMessage is idempotent on message.id', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) const msg = message('contract-msg-1') await a.appendMessage({ threadId: id, message: msg }) @@ -780,7 +838,7 @@ export function threadsStoreAdapterContract( }), define('appendMessage preserves order for distinct ids', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.appendMessage({ threadId: id, message: message('contract-msg-1') }) await a.appendMessage({ threadId: id, message: message('contract-msg-2') }) @@ -792,7 +850,7 @@ export function threadsStoreAdapterContract( }), define('setStatus round-trips', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.setStatus({ threadId: id, status: 'streaming' }) assert((await loadOrThrow(a, id)).status === 'streaming', "status did not become 'streaming'") @@ -801,7 +859,7 @@ export function threadsStoreAdapterContract( }), define('setOutcome round-trips', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) const outcome: AgentOutcome = { title: 'Title', summary: 'Summary', status: 'done' } await a.setOutcome({ threadId: id, outcome }) @@ -812,7 +870,7 @@ export function threadsStoreAdapterContract( }), define('setResumeToken round-trips and clears with undefined', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.setResumeToken({ threadId: id, token: 'tok-1' }) assert((await loadOrThrow(a, id)).resumeToken === 'tok-1', 'resumeToken did not round-trip') @@ -824,7 +882,7 @@ export function threadsStoreAdapterContract( }), define('markRead marks the thread read', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) assert((await loadOrThrow(a, id)).read === false, 'a new thread must start unread') await a.markRead(id) @@ -835,7 +893,7 @@ export function threadsStoreAdapterContract( // The store keeps a write's optimistic patch applied until a listThreads STARTED after that // write resolved lands in the confirmed base. An eventually-consistent list (a stale read // replica, a cached response) breaks that and flashes the pre-write state back into the UI. - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.appendMessage({ threadId: id, message: message('contract-msg-raw') }) await a.setStatus({ threadId: id, status: 'streaming' }) @@ -856,7 +914,7 @@ export function threadsStoreAdapterContract( async (a) => { // The store serializes writes per thread id, so this exact sequence — the send path's // create-then-first-message — is what a real adapter receives. Each write must find the row. - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.markRead(id) await a.appendMessage({ threadId: id, message: message('contract-msg-dep') }) @@ -872,7 +930,7 @@ export function threadsStoreAdapterContract( ), define('removeThread removes the thread', async (a) => { - const id = crypto.randomUUID() + const id = mintThreadId() await a.createThread({ id }) await a.removeThread(id) assert(await a.loadThread(id).then((t) => t === null), 'loadThread must be null after remove') @@ -881,7 +939,7 @@ export function threadsStoreAdapterContract( }), define('removeThread of an unknown id does not throw', async (a) => { - await a.removeThread(crypto.randomUUID()) + await a.removeThread(mintThreadId()) }), ] } diff --git a/packages/basalt-ui/src/agent/ai-sdk-transport.test.ts b/packages/basalt-ui/src/agent/ai-sdk-transport.test.ts index ca9124a..f14f21d 100644 --- a/packages/basalt-ui/src/agent/ai-sdk-transport.test.ts +++ b/packages/basalt-ui/src/agent/ai-sdk-transport.test.ts @@ -592,3 +592,67 @@ describe('aiSdkTransport — deterministic ids and replay idempotency', () => { ) }) }) + +describe('aiSdkTransport — id minting under degraded crypto', () => { + test('the fixed chatId (mintThreadId, low collision cost) mints fine with no usable crypto at all — construction never throws', () => { + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + expect(() => + aiSdkTransport({ + api: '/api/chat', + fetch: mockFetch(() => { + throw new Error('must not be called') + }), + }), + ).not.toThrow() + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) + + test('the outbound user message id (mintMessageId, an idempotency key on the AI SDK backend) works via getRandomValues when randomUUID is unavailable', async () => { + const chunks: UIMessageChunk[] = [ + { type: 'text-start', id: 't1' }, + { type: 'text-end', id: 't1' }, + ] + const transport = aiSdkTransport({ + api: '/api/chat', + fetch: mockFetch(async () => scriptedResponse(chunks)), + }) + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: { getRandomValues: originalCrypto.getRandomValues.bind(originalCrypto) }, + configurable: true, + }) + try { + const parts = await collect(transport.stream('hi')) + expect(parts[0]?.type).toBe('start') + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) + + test('the outbound user message id THROWS on a host with no usable crypto at all, rather than silently colliding', async () => { + const transport = aiSdkTransport({ + api: '/api/chat', + fetch: mockFetch(() => { + throw new Error('must not be called — must throw before any network call') + }), + }) + const gen = transport.stream('hi') + + // The synthesized StartPart is yielded first, before mintMessageId is ever reached. + const start = await gen.next() + expect(start.value).toMatchObject({ type: 'start' }) + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + await expect(gen.next()).rejects.toThrow(/idempotency key/) + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) +}) diff --git a/packages/basalt-ui/src/agent/ai-sdk-transport.ts b/packages/basalt-ui/src/agent/ai-sdk-transport.ts index cd5ce42..f510aef 100644 --- a/packages/basalt-ui/src/agent/ai-sdk-transport.ts +++ b/packages/basalt-ui/src/agent/ai-sdk-transport.ts @@ -48,6 +48,7 @@ * }) */ import { assertNever } from '../register' +import { mintMessageId, mintThreadId } from './id' import { TERMINAL_TOOL_STATES } from './parts' import type { AgentPart, AgentPartDraft } from './parts' import type { ResumableAgentTransport } from './transport' @@ -401,7 +402,10 @@ export function aiSdkTransport( const { httpTransport, readUIMessageStream: readStream } = await resolveAiSdk() const userMessage: UIMessage = { - id: crypto.randomUUID(), + // This id is what AI SDK's own backend persistence/dedup keys turns hang off — the same + // idempotency-key cost as basalt's own ChatMessage.id (see mintMessageId's doc), so + // mintMessageId, not mintThreadId, is the right helper here. + id: mintMessageId(), role: 'user', parts: [{ type: 'text', text: input }], } @@ -425,7 +429,9 @@ export function aiSdkTransport( } } - const fixedChatId = crypto.randomUUID() + // Client-side conversation namespace, not an idempotency key — a collision here would merge two + // conversations' index-addressed part ids, the same low collision cost `mintThreadId` covers. + const fixedChatId = mintThreadId() return { ...makeTransport(fixedChatId), forThread: (chatId: string) => makeTransport(chatId), diff --git a/packages/basalt-ui/src/agent/id.test.ts b/packages/basalt-ui/src/agent/id.test.ts index 4d27b28..5dbd94d 100644 --- a/packages/basalt-ui/src/agent/id.test.ts +++ b/packages/basalt-ui/src/agent/id.test.ts @@ -1,9 +1,15 @@ /** * withPartIds — stamps `${runId}#${n}` onto any draft part arriving without an id; idempotent for * drafts that already have one (untouched, and doesn't advance the counter). + * + * mintThreadId/mintMessageId — the two id-minting helpers behind every unguarded + * `crypto.randomUUID()` call site this file's siblings used to have. Both share rungs 1 + * (`crypto.randomUUID`) and 2 (`crypto.getRandomValues`, hand-assembled into a UUIDv4); they + * deliberately diverge on rung 3 (no crypto at all) — `mintThreadId` degrades, `mintMessageId` + * throws. See each function's own doc for why. */ import { describe, expect, test } from 'bun:test' -import { withPartIds } from './id' +import { mintMessageId, mintThreadId, withPartIds } from './id' import type { AgentPartDraft } from './parts' async function* gen(values: T[]): AsyncGenerator { @@ -16,6 +22,33 @@ async function collect(source: AsyncGenerator): Promise { return out } +const UUID_V4_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +/** Rung 2: randomUUID absent, getRandomValues present (a real, non-secure-context host). */ +function withRung2Crypto(run: () => T): T { + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: { getRandomValues: originalCrypto.getRandomValues.bind(originalCrypto) }, + configurable: true, + }) + try { + return run() + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } +} + +/** Rung 3: no usable crypto at all. */ +function withRung3Crypto(run: () => T): T { + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + return run() + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } +} + describe('withPartIds', () => { test('stamps a sequential id on every draft missing one', async () => { // Annotated: all three literals uniformly omit `id`, so without this TS infers an `id`-less @@ -73,3 +106,64 @@ describe('withPartIds', () => { ]) }) }) + +describe('mintThreadId', () => { + test('rung 1 (crypto.randomUUID): returns a valid UUIDv4 string', () => { + const id = mintThreadId() + expect(id).toMatch(UUID_V4_RE) + }) + + test('rung 1: many mints are distinct', () => { + const ids = Array.from({ length: 1000 }, () => mintThreadId()) + expect(new Set(ids).size).toBe(1000) + }) + + test('rung 2 (getRandomValues only): assembles a valid UUIDv4 — version/variant bits set', () => { + const id = withRung2Crypto(() => mintThreadId()) + expect(id).toMatch(UUID_V4_RE) + }) + + test('rung 2: many mints are distinct', () => { + const ids = withRung2Crypto(() => Array.from({ length: 1000 }, () => mintThreadId())) + expect(new Set(ids).size).toBe(1000) + }) + + test('rung 3 (no usable crypto at all): does NOT throw, still returns distinct non-empty strings', () => { + const ids = withRung3Crypto(() => Array.from({ length: 50 }, () => mintThreadId())) + for (const id of ids) { + expect(typeof id).toBe('string') + expect(id.length).toBeGreaterThan(0) + } + // Weak (Date.now() + Math.random()), not cryptographically random — but still distinct across + // this many mints in one tick, since Math.random() varies even when Date.now() doesn't. + expect(new Set(ids).size).toBe(50) + }) +}) + +describe('mintMessageId', () => { + test('rung 1 (crypto.randomUUID): returns a valid UUIDv4 string', () => { + const id = mintMessageId() + expect(id).toMatch(UUID_V4_RE) + }) + + test('rung 1: many mints are distinct', () => { + const ids = Array.from({ length: 1000 }, () => mintMessageId()) + expect(new Set(ids).size).toBe(1000) + }) + + test('rung 2 (getRandomValues only): assembles a valid UUIDv4 — version/variant bits set', () => { + const id = withRung2Crypto(() => mintMessageId()) + expect(id).toMatch(UUID_V4_RE) + }) + + test('rung 2: many mints are distinct', () => { + const ids = withRung2Crypto(() => Array.from({ length: 1000 }, () => mintMessageId())) + expect(new Set(ids).size).toBe(1000) + }) + + test('rung 3 (no usable crypto at all): THROWS rather than minting a non-random id', () => { + // Diverges from mintThreadId's rung 3 on purpose — appendMessage's idempotency key must never + // silently degrade to a non-collision-resistant id. See mintMessageId's own doc. + expect(() => withRung3Crypto(() => mintMessageId())).toThrow(/idempotency key/) + }) +}) diff --git a/packages/basalt-ui/src/agent/id.ts b/packages/basalt-ui/src/agent/id.ts index 3335642..064da67 100644 --- a/packages/basalt-ui/src/agent/id.ts +++ b/packages/basalt-ui/src/agent/id.ts @@ -1,51 +1,121 @@ /** - * Id-minting helpers shared by the thread stores: `mintThreadId` (a client-side thread id, used by - * both `./thread`'s `createThreadsStore` and `./adapter`'s `createAdapterThreadsStore`) and - * `withPartIds` (stamps sequence ids onto streamed parts). Related in theme — both mint an id a - * store needs — unrelated in mechanism; grouped here because they're the two id-minting leaves the - * two thread stores both sit above. + * Id-minting helpers shared across the agent layer: `mintThreadId` (a low-stakes client-side id — + * threads, run/chat namespaces — used by `./thread`'s `createThreadsStore`, `./adapter`'s + * `createAdapterThreadsStore`/`threadsStoreAdapterContract`, `./use-agent-stream`'s per-send run + * id, and `aiSdkTransport`'s fixed chat id), `mintMessageId` (the higher-stakes id `appendMessage` + * treats as its only idempotency key), and `withPartIds` (stamps sequence ids onto streamed + * parts). Grouped here because they're the id-minting leaves every store/hook/transport in this + * layer sits above — not because they share a mechanism (`mintMessageId` deliberately diverges + * from `mintThreadId` on rung 3; see its own doc). */ /** - * Mints a client-side thread id. `crypto.randomUUID()` exists only in SECURE CONTEXTS (HTTPS or - * localhost) — a consumer served over plain HTTP on a LAN hostname (a homelab dashboard on a bare - * IP, a staging box) would otherwise have `create()`, about the most basic user action a thread - * store has, throw a plain TypeError. A store used from a browser must not throw on a user gesture - * — the same principle already applied twice this release (`spliceText` clamps rather than throws, - * `coalesceParts` degrades rather than throws). + * Hand-assembles a UUIDv4 string from 16 random bytes — the shared rung-2 mechanism both + * `mintThreadId` and `mintMessageId` use once `crypto.randomUUID` is unavailable but + * `crypto.getRandomValues` still is. The version/variant bits are set per RFC 4122, since this + * rung hands back raw bytes, not a formatted UUID. + */ +function assembleUuidV4(bytes: Uint8Array): string { + const hex = Array.from({ length: 16 }, (_, i) => { + const raw = bytes.at(i) ?? 0 + // Version 4 (bits 12-15 of time_hi_and_version) and variant 10 (bits 6-7 of + // clock_seq_hi_and_reserved) — the only structure a UUIDv4 promises over raw random bytes. + const byte = i === 6 ? (raw & 0x0f) | 0x40 : i === 8 ? (raw & 0x3f) | 0x80 : raw + return byte.toString(16).padStart(2, '0') + }).join('') + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` +} + +/** + * Mints a client-side id for a LOW-COLLISION-COST use: a thread id, a per-send run-id namespace, a + * fixed chat id — anything minted client-side that is NOT the idempotency key some write depends + * on. `crypto.randomUUID()` exists only in SECURE CONTEXTS (HTTPS or localhost) — a consumer served + * over plain HTTP on a LAN hostname (a homelab dashboard on a bare IP, a staging box) would + * otherwise have `create()`, about the most basic user action a thread store has, throw a plain + * TypeError. A store used from a browser must not throw on a user gesture — the same principle + * already applied twice this release (`spliceText` clamps rather than throws, `coalesceParts` + * degrades rather than throws). * * Fallback chain, each rung reached only when the one above it is unavailable: * 1. `crypto.randomUUID()` — cryptographically random, RFC 4122 UUID. The normal path. * 2. `crypto.getRandomValues()` — still cryptographically random; hand-assembled into a UUIDv4 - * (the version/variant bits are set per RFC 4122, since this rung hands back raw bytes, not a - * formatted UUID). + * via `assembleUuidV4`. * 3. No usable `crypto` at all (missing entirely, or missing both methods above — an old WebView, * an SSR shim). NOT cryptographically random and NOT collision-resistant in general. This rung - * exists solely so `create()` never throws on such a host. It is acceptable ONLY because this - * id is minted a handful of times per client session and is not the idempotency key any write - * here depends on — `ThreadsStoreAdapter.appendMessage`'s idempotency key is the MESSAGE id, - * not this thread id, so a collision on this rung costs a locally merged/duplicated thread, not - * silent data loss on the append path. Do not reuse this rung for anything with a higher - * collision cost than that. + * exists solely so a caller never throws on such a host. It is acceptable ONLY because an id + * minted here is minted a handful of times per client session and is not the idempotency key + * any write depends on — `ThreadsStoreAdapter.appendMessage`'s idempotency key is the MESSAGE + * id (see `mintMessageId`), not this one, so a collision on this rung costs a locally + * merged/duplicated thread (or two runs sharing a part-id namespace), not silent data loss on + * the append path. Do not reuse this rung for anything with a higher collision cost than that + * — see `mintMessageId` for the id that needs one. */ export function mintThreadId(): string { if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { return crypto.randomUUID() } if (typeof crypto !== 'undefined' && typeof crypto.getRandomValues === 'function') { - const bytes = crypto.getRandomValues(new Uint8Array(16)) - const hex = Array.from({ length: 16 }, (_, i) => { - const raw = bytes.at(i) ?? 0 - // Version 4 (bits 12-15 of time_hi_and_version) and variant 10 (bits 6-7 of - // clock_seq_hi_and_reserved) — the only structure a UUIDv4 promises over raw random bytes. - const byte = i === 6 ? (raw & 0x0f) | 0x40 : i === 8 ? (raw & 0x3f) | 0x80 : raw - return byte.toString(16).padStart(2, '0') - }).join('') - return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` + return assembleUuidV4(crypto.getRandomValues(new Uint8Array(16))) } return `id-${Date.now().toString(36)}-${Math.random().toString(36).slice(2)}` } +/** + * Mints a message id — the value `ThreadsStoreAdapter.appendMessage` treats as its ONLY + * idempotency key (see that method's contract doc in `./adapter`). Shares `mintThreadId`'s rungs + * 1-2 (`crypto.randomUUID()`, then `crypto.getRandomValues()` assembled into a UUIDv4) — both are + * cryptographically random and collision-resistant, and rung 2 is the fix that actually matters in + * practice (a plain-HTTP/LAN host still has `getRandomValues`; only `randomUUID` is secure-context- + * gated). + * + * Rung 3 deliberately DIVERGES from `mintThreadId`: it THROWS instead of degrading to a + * non-random fallback. A thread id's rung-3 collision costs a locally merged/duplicated thread — + * annoying, recoverable, visible. A message id's collision cost is different IN KIND, not degree: + * `appendMessage` is idempotent on this id, so two messages that collide are not two rows, they + * are ONE — the second write silently no-ops and its content is gone, with nothing downstream able + * to tell. Degrading here would trade a loud, debuggable failure for silent, permanent data loss. + * + * This deliberately diverges from this layer's other standing rule — render-path code degrades, + * never throws (`spliceText` clamps, `coalesceParts` degrades, a consumer fence renderer must not + * take the message down) — because minting a message id is NOT render-path code. It runs once, in + * the write path that constructs a message before handing it to a store (`start()`, + * `consumeAndFinalize`'s finalize step, `finalizeStop`, `aiSdkTransport`'s per-turn user message), + * never inside the per-chunk accumulation/render loop those functions guard. A host with no usable + * `crypto` at all (missing both `randomUUID` and `getRandomValues`) is vanishingly rare — no + * runtime with a DOM/fetch surface worth targeting ships without a Crypto object — and an uncaught + * throw here is a loud, debuggable failure rather than a message silently vanishing with no trace, + * but WHERE it surfaces differs by call site (audited, not assumed — this paragraph previously + * claimed a single shared catch handled all of them, which was false): + * - `consumeAndFinalize`'s own assistantMessage mint runs inside that function's try/catch, so a + * throw there is correctly turned into `onFailureStatus` ('error' from `start()`, 'interrupted' + * from the mount-time resume) — a visibly stuck-then-settled run, as originally claimed. + * - `useAgentThreadRuns.start()`'s userMessage mint runs BEFORE any state is touched + * (`appendMessage`/`setStatus`/the controller registration all come after it), so a throw there + * is a true no-op that propagates synchronously out of `start()` itself — not a stuck run at + * all, since nothing ever started. See `UseAgentThreadRunsReturn.start`'s `@throws` doc. + * - `finalizeStop`'s stoppedMessage mint is wrapped in its own try/catch precisely so a throw + * here cannot wedge the thread at 'streaming' forever (stop() has already torn down this run's + * bookkeeping by the time finalizeStop runs, so a second stop() is always a no-op) — see + * `finalizeStop`'s doc for the guarded behavior and the resulting 'error' status. + * A stuck spinner is debuggable; silently discarded chat history is not — that half of the + * original claim stands regardless of which of the three paths above catches it. + * + * @throws {Error} when neither `crypto.randomUUID` nor `crypto.getRandomValues` is available. + */ +export function mintMessageId(): string { + if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { + return crypto.randomUUID() + } + if (typeof crypto !== 'undefined' && typeof crypto.getRandomValues === 'function') { + return assembleUuidV4(crypto.getRandomValues(new Uint8Array(16))) + } + throw new Error( + 'mintMessageId: no usable crypto (both randomUUID and getRandomValues are unavailable) — ' + + 'refusing to mint a non-random message id, since ThreadsStoreAdapter.appendMessage treats ' + + 'this id as its only idempotency key and a collision would silently drop a message.', + ) +} + /** * withPartIds — stamps a stable id onto every draft part arriving without one. * diff --git a/packages/basalt-ui/src/agent/thread.ts b/packages/basalt-ui/src/agent/thread.ts index e7e5bcf..1775f42 100644 --- a/packages/basalt-ui/src/agent/thread.ts +++ b/packages/basalt-ui/src/agent/thread.ts @@ -145,6 +145,10 @@ export type ThreadsStore = { * store built with `createAdapterThreadsStore` (./adapter), where it stays `false` until the * first `listThreads` SUCCEEDS. Pair it with `error` — `!hydrated && error !== undefined` is a * failed load, `!hydrated && error === undefined` is still loading. + * + * Gating an empty state on this is the CALLER's job when hand-assembling `ThreadFeed` / + * `ThreadDetailPanel`; the shipped `ThreadWorkspace` already does it internally, so an + * async store never flashes "no threads yet" through it before it actually knows. */ readonly hydrated: boolean /** diff --git a/packages/basalt-ui/src/agent/use-agent-stream.test.tsx b/packages/basalt-ui/src/agent/use-agent-stream.test.tsx index 9e85590..e25ed28 100644 --- a/packages/basalt-ui/src/agent/use-agent-stream.test.tsx +++ b/packages/basalt-ui/src/agent/use-agent-stream.test.tsx @@ -262,4 +262,26 @@ describe('useAgentStream', () => { // Only the SECOND (surviving) call's part made it into final state — the first is discarded. expect(capturedParts).toEqual([{ id: 'call-2', type: 'text', text: 'call-2' }]) }) + + test('send() still streams without throwing on a host with no usable crypto at all — its runId is a low-collision-cost mint (mintThreadId), never the message-id one', async () => { + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + const transport: AgentTransport = { + async *stream() { + yield { type: 'text', text: 'a' } as AgentPartDraft as AgentPart + }, + } + const { result } = renderHook(() => useAgentStream({ transport })) + + await act(async () => { + await result.current.send('hi') + }) + + expect(result.current.status).toBe('done') + expect(result.current.parts).toHaveLength(1) + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) }) diff --git a/packages/basalt-ui/src/agent/use-agent-stream.ts b/packages/basalt-ui/src/agent/use-agent-stream.ts index ede3168..7e883b0 100644 --- a/packages/basalt-ui/src/agent/use-agent-stream.ts +++ b/packages/basalt-ui/src/agent/use-agent-stream.ts @@ -27,7 +27,7 @@ * } */ import { useCallback, useEffect, useRef, useState } from 'react' -import { withPartIds } from './id' +import { mintThreadId, withPartIds } from './id' import { isStartPart } from './parts' import { mergePart } from './merge' import type { PartLike } from './merge' @@ -107,8 +107,11 @@ export function useAgentStream part.type)).toEqual(['text', 'tool', 'text']) }) }) + +describe('useAgentThreadRuns — message-id minting under degraded crypto', () => { + test('start() mints distinct message ids via crypto.getRandomValues when randomUUID is unavailable', async () => { + const store = createTestThreadsStore() + const threadA = store.create() + const threadB = store.create() + + const transport: AgentTransport = { + async *stream(input) { + yield { id: 'p1', type: 'text', text: `done:${input}` } + }, + } + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: { getRandomValues: originalCrypto.getRandomValues.bind(originalCrypto) }, + configurable: true, + }) + try { + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + act(() => { + result.current.start(threadA, 'input-a') + result.current.start(threadB, 'input-b') + }) + + await waitFor(() => { + expect(store.threads.find((t) => t.id === threadA)?.status).toBe('done') + expect(store.threads.find((t) => t.id === threadB)?.status).toBe('done') + }) + + const allMessageIds = store.threads.flatMap((t) => t.messages.map((m) => m.id)) + expect(allMessageIds.length).toBeGreaterThan(0) + expect(new Set(allMessageIds).size).toBe(allMessageIds.length) + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + }) + + test('start() THROWS on a host with no usable crypto at all, rather than silently minting a colliding message id', () => { + const store = createTestThreadsStore() + const threadId = store.create() + + const transport: AgentTransport = { + async *stream() { + yield { id: 'p1', type: 'text', text: 'unreachable' } + }, + } + + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + expect(() => { + act(() => { + result.current.start(threadId, 'hi') + }) + }).toThrow(/idempotency key/) + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + + // The deliberate divergence documented on mintMessageId: throwing means the write never + // happened at all (no half-appended message, no orphaned run entry) rather than silently + // dropping content behind a colliding id. + expect(store.threads.find((t) => t.id === threadId)?.messages).toHaveLength(0) + expect(result.current.runs.has(threadId)).toBe(false) + }) +}) + +// A ThreadsStore double that delegates everything to a real test store EXCEPT appendMessage, +// which throws for messages matching `shouldThrow` — lets a test simulate consumer-adapter code +// (appendMessage) throwing without touching the shared createTestThreadsStore factory used by +// every other test in this file. +function wrapStoreWithThrowingAppend( + store: ThreadsStore, + shouldThrow: (message: AgentThread['messages'][number]) => boolean, +): ThreadsStore { + return { + get threads() { + return store.threads + }, + get activeId() { + return store.activeId + }, + select: store.select, + create: store.create, + appendMessage(id, message) { + if (shouldThrow(message)) { + throw new Error('consumer appendMessage boom') + } + store.appendMessage(id, message) + }, + setOutcome: store.setOutcome, + setStatus: store.setStatus, + setResumeToken: store.setResumeToken, + markRead: store.markRead, + remove: store.remove, + clear: store.clear, + hydrated: store.hydrated, + error: store.error, + } +} + +/** As above, but for the settle half of `finalizeStop` — a consumer store whose `setStatus` throws + * (a remote-backed adapter rejecting a thread removed mid-stream is the realistic shape). Narrowed + * by `shouldThrow` because the hook ALSO calls `setStatus` from its mount-time orphan sweep, which + * is a different call site with a different (non-wedge) failure mode. */ +function wrapStoreWithThrowingSetStatus( + store: ThreadsStore, + shouldThrow: (status: AgentThread['status']) => boolean, +): ThreadsStore { + return { + get threads() { + return store.threads + }, + get activeId() { + return store.activeId + }, + select: store.select, + create: store.create, + appendMessage: store.appendMessage, + setOutcome: store.setOutcome, + setStatus(id, status) { + if (shouldThrow(status)) throw new Error('consumer setStatus boom') + store.setStatus(id, status) + }, + setResumeToken: store.setResumeToken, + markRead: store.markRead, + remove: store.remove, + clear: store.clear, + hydrated: store.hydrated, + error: store.error, + } +} + +describe('useAgentThreadRuns — finalizeStop must reach a terminal state no matter what', () => { + test('stop() on a host with no usable crypto still reaches a terminal status and clears the run entry (was: wedged at "streaming" forever)', async () => { + const store = createTestThreadsStore() + const threadId = store.create() + + const gate = deferred() + const transport: AgentTransport = { + async *stream() { + yield { id: 'p1', type: 'text', text: 'partial' } + await gate.promise + yield { id: 'p2', type: 'text', text: 'never arrives' } + }, + } + + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + act(() => { + result.current.start(threadId, 'hi') + }) + + await waitFor(() => { + expect(result.current.runs.get(threadId)?.parts).toHaveLength(1) + }) + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { value: undefined, configurable: true }) + try { + act(() => { + result.current.stop(threadId) + }) + + const thread = store.threads.find((t) => t.id === threadId) + // Before the fix: mintMessageId's throw escaped finalizeStop entirely, so setStatus/setRuns + // never ran and this stayed 'streaming' forever. + expect(thread?.status).toBe('error') + // The stopped message never minted an id, so appendMessage was never even attempted — + // only the original user message is present. + expect(thread?.messages).toHaveLength(1) + expect(result.current.runs.has(threadId)).toBe(false) + + // A second stop() must not be the user's only recourse — and it isn't: controllersRef + // already had this threadId deleted before finalizeStop ever ran, so this is an ordinary + // no-op, not a retry of the failed append. + act(() => { + result.current.stop(threadId) + }) + expect(store.threads.find((t) => t.id === threadId)?.status).toBe('error') + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + + gate.resolve(undefined) + }) + + test("stop() reaches a terminal status and clears the run entry even when the consumer's appendMessage throws (was: wedged at 'streaming' forever)", async () => { + const baseStore = createTestThreadsStore() + const threadId = baseStore.create() + const store = wrapStoreWithThrowingAppend( + baseStore, + (message) => message.role === 'assistant' && message.finish === 'stopped', + ) + + const gate = deferred() + const transport: AgentTransport = { + async *stream() { + yield { id: 'p1', type: 'text', text: 'partial' } + await gate.promise + yield { id: 'p2', type: 'text', text: 'never arrives' } + }, + } + + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + act(() => { + result.current.start(threadId, 'hi') + }) + + await waitFor(() => { + expect(result.current.runs.get(threadId)?.parts).toHaveLength(1) + }) + + act(() => { + result.current.stop(threadId) + }) + + const thread = store.threads.find((t) => t.id === threadId) + expect(thread?.status).toBe('error') + // Only the user message — the consumer's appendMessage rejected the stopped message. + expect(thread?.messages).toHaveLength(1) + expect(result.current.runs.has(threadId)).toBe(false) + + // A second stop() remains a true no-op — not the user's only recourse. + act(() => { + result.current.stop(threadId) + }) + expect(store.threads.find((t) => t.id === threadId)?.status).toBe('error') + + gate.resolve(undefined) + }) + + test("stop() clears the hook's own run entry even when the consumer's setStatus throws (the second half of the same wedge)", async () => { + const baseStore = createTestThreadsStore() + const threadId = baseStore.create() + const store = wrapStoreWithThrowingSetStatus(baseStore, (status) => status === 'done') + + const gate = deferred() + const transport: AgentTransport = { + async *stream() { + yield { id: 'p1', type: 'text', text: 'partial' } + await gate.promise + yield { id: 'p2', type: 'text', text: 'never arrives' } + }, + } + + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + act(() => { + result.current.start(threadId, 'hi') + }) + + await waitFor(() => { + expect(result.current.runs.get(threadId)?.parts).toHaveLength(1) + }) + + act(() => { + result.current.stop(threadId) + }) + + // The store's own status is whatever the throwing adapter left it as — unrecoverable from + // here. What IS this hook's to guarantee is that `runs` no longer reports an in-flight turn: + // that entry is the hook's own state and nothing later can ever clear it (stop() already + // deleted the thread from controllersRef), so a throw above the teardown stranded it forever. + expect(result.current.runs.has(threadId)).toBe(false) + // The partial content still landed — the append runs before the settle. + expect(store.threads.find((t) => t.id === threadId)?.messages).toHaveLength(2) + + act(() => { + result.current.stop(threadId) + }) + expect(result.current.runs.has(threadId)).toBe(false) + + gate.resolve(undefined) + }) + + test('stop() still succeeds via crypto.getRandomValues when randomUUID is unavailable (rung 2 — the rung that matters in practice)', async () => { + const store = createTestThreadsStore() + const threadId = store.create() + + const gate = deferred() + const transport: AgentTransport = { + async *stream() { + yield { id: 'p1', type: 'text', text: 'partial' } + await gate.promise + yield { id: 'p2', type: 'text', text: 'never arrives' } + }, + } + + const originalCrypto = globalThis.crypto + Object.defineProperty(globalThis, 'crypto', { + value: { getRandomValues: originalCrypto.getRandomValues.bind(originalCrypto) }, + configurable: true, + }) + try { + const { result } = renderHook(() => useAgentThreadRuns({ transport, store, resolveOutcome })) + + act(() => { + result.current.start(threadId, 'hi') + }) + + await waitFor(() => { + expect(result.current.runs.get(threadId)?.parts).toHaveLength(1) + }) + + act(() => { + result.current.stop(threadId) + }) + + const thread = store.threads.find((t) => t.id === threadId) + expect(thread?.status).toBe('done') + expect(thread?.messages).toHaveLength(2) + expect(thread?.messages[1]?.finish).toBe('stopped') + expect(result.current.runs.has(threadId)).toBe(false) + } finally { + Object.defineProperty(globalThis, 'crypto', { value: originalCrypto, configurable: true }) + } + + gate.resolve(undefined) + }) +}) diff --git a/packages/basalt-ui/src/agent/use-agent-thread-runs.ts b/packages/basalt-ui/src/agent/use-agent-thread-runs.ts index ceacae9..8289863 100644 --- a/packages/basalt-ui/src/agent/use-agent-thread-runs.ts +++ b/packages/basalt-ui/src/agent/use-agent-thread-runs.ts @@ -40,7 +40,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import type { Dispatch, MutableRefObject, SetStateAction } from 'react' import type { ChatMessage } from './history' -import { withPartIds } from './id' +import { mintMessageId, mintThreadId, withPartIds } from './id' import { mergePart } from './merge' import type { PartLike } from './merge' import type { AgentOutcome, OutcomeResolver } from './outcome' @@ -103,18 +103,39 @@ export type UseAgentThreadRunsArgs = { export type UseAgentThreadRunsReturn = { /** Live stream state per thread id, for threads with a run in progress or just completed. */ readonly runs: ReadonlyMap> - /** Start a new turn on `threadId`. No-op if that thread already has a stream in flight. */ + /** + * Start a new turn on `threadId`. No-op if that thread already has a stream in flight. + * + * @throws {Error} synchronously, before any state is touched, on a host with no usable + * `crypto` at all (see `mintMessageId` in `./id.ts` — the id it mints here is the new user + * ChatMessage's idempotency key, so it degrades to a throw rather than a silent collision). + * This throw is a true no-op: it happens before `appendMessage`/`setStatus`/the run's + * controller are registered, so nothing is left half-started — the thread is exactly as it was + * before the call. A caller invoking `start` from an event handler on a target that must + * tolerate this (very old WebViews, non-DOM SSR shims) should wrap the call in its own + * try/catch; this rung is vanishingly rare in practice (see `mintMessageId`'s own doc). + */ readonly start: (threadId: string, input: string) => void /** * Replay the last user input sent on `threadId` (same code path as `start`). No-op if * `threadId` has never had a turn started, or already has one in flight. + * + * @throws {Error} same condition and same true-no-op guarantee as `start` — see its doc. */ readonly retry: (threadId: string) => void /** - * Abort the in-flight stream for `threadId` (no-op if idle). Preserves whatever content had - * already arrived: if the run had accumulated any parts, they're persisted as an assistant + * Abort the in-flight stream for `threadId` (true no-op only if BOTH `controllersRef` and + * `runs` have nothing for it — i.e. the thread genuinely isn't live). Preserves whatever content + * had already arrived: if the run had accumulated any parts, they're persisted as an assistant * ChatMessage with `finish: 'stopped'` (unless that message already landed — see finalizeStop), * then distilled into an outcome and the thread is settled to 'done'. + * + * Defense-in-depth: if `runs` reports `threadId` as `'streaming'` but no controller is + * registered for it (a phantom entry — not reachable via any path in this hook today, since the + * unmount-cleanup effect and `finalizeStop` both tear down `controllersRef` and `runs` together, + * but guarded against here so nothing in this file can EVER wedge a thread the UI shows as live + * with no way to reach a terminal state), `stop()` still settles it instead of silently + * no-opping forever. */ readonly stop: (threadId: string) => void /** Abort every in-flight stream across all threads, settling each the same way stop() would. */ @@ -123,9 +144,14 @@ export type UseAgentThreadRunsReturn = { // ── defaults ────────────────────────────────────────────────────────────────── -/** Default toUserParts: wraps raw input in a single text part. */ +/** + * Default toUserParts: wraps raw input in a single text part. This part's id is a display/merge + * identity WITHIN a message's own `parts` array — it is never the value `appendMessage` idempotes + * on (that's the enclosing `ChatMessage.id`, minted separately by the caller) — so `mintThreadId` + * is the right low-collision-cost helper here, not `mintMessageId`. + */ function defaultToUserParts(input: string): AgentPart[] { - return [{ id: crypto.randomUUID(), type: 'text', text: input }] + return [{ id: mintThreadId(), type: 'text', text: input }] } /** @@ -199,6 +225,18 @@ async function consumeAndFinalize(args: { for await (const part of generator) { // Guard: a newer call superseded this stream's controller for this thread, or it // was aborted — stop updating state. + // + // Deliberately NOT a `runs` teardown site (considered and rejected): `controllersRef` + // mismatch here is ambiguous between "this run was aborted and nothing replaced it" and "a + // newer run (a mount-reconcile resume, keyed by the SAME threadId) has already taken over + // and is live in `runs` right now". Only the first case should ever clear `runs[threadId]`, + // and only the abort's OWN originator can tell the two apart without a race — the unmount- + // cleanup effect (see its doc) does that teardown synchronously, in the same tick as the + // abort, before any newer run could exist to be confused with. If this guard also deleted + // `runs[threadId]`, a superseding resume's fresh entry would be clobbered out from under it + // by its OWN predecessor's late-settling loop iteration — reintroducing a wedge instead of + // fixing one. `stop()`'s own abort path has the same property (finalizeStop's teardown runs + // synchronously before `stop()` returns, never racing a subsequent call). if (controllersRef.current.get(threadId) !== controller) return if (controller.signal.aborted) return if (isStartPart(part)) { @@ -221,7 +259,9 @@ async function consumeAndFinalize(args: { } const assistantMessage: ChatMessage = { - id: crypto.randomUUID(), + // appendMessage's idempotency key — mintMessageId, never mintThreadId (see that helper's + // doc): a collided id here would silently drop this message, not merge a duplicate thread. + id: mintMessageId(), role: 'assistant', parts, createdAt: Date.now(), @@ -285,11 +325,37 @@ async function consumeAndFinalize(args: { * invoking this. `alreadyAppended` is precomputed by the caller (stop()) from `appendedRef` — see * that ref's doc for why this is an explicit marker rather than an inferred comparison. * - * Ordering: the append, forcing status to 'done', clearing the resume token, and tearing down the - * run entry ALL happen synchronously (this function's body runs synchronously up to its one - * `await`) — status is never derived from the resolved outcome, so it doesn't need to wait on one, - * matching useAgentStream's stop(), which settles 'done' immediately rather than blocking on async - * work. Only `setOutcome` — which genuinely needs the resolved value — happens after the await. + * Ordering / the append-failure guard: the append is attempted FIRST (so a successful stop still + * reads as 'done' with its partial content intact — this is the common case and the one that + * matters most), but it is wrapped in its own try/catch that CANNOT prevent the terminal + * transition below it. This is deliberate, not incidental: by the time this function runs, `stop()` + * has already deleted `threadId` from `controllersRef` and `appendedRef` (see `stop()`'s own doc), + * so a second `stop()` call is unconditionally a no-op regardless of what happens here — if the + * append (or `mintMessageId`, which now also throws on rung 3 — see `./id.ts`) throws and nothing + * downstream ran, the thread would be wedged at 'streaming' FOREVER with no way for the user to + * clear it. That is exactly the standing invariant this layer forbids (see this hook's module + * doc / the render-path degrade rule), so it must not depend on the append succeeding. The + * alternative ordering — set status/teardown first, append after — was considered and rejected: + * it would mark the thread 'done' before the content the user is about to lose is even attempted, + * which is a strictly worse failure mode for the (overwhelmingly common) success path than this + * try/catch is for the (vanishingly rare) failure path. + * + * On an append failure, status becomes 'error' instead of 'done' — mirroring + * `consumeAndFinalize`'s own catch path (`onFailureStatus`), which is the existing precedent for + * "the persist step failed, tell the user" in this file — and `resolveOutcome` is skipped + * entirely, again matching that catch path: there is no complete/consistent snapshot worth + * distilling into an outcome when the turn's own final message never made it into the store. + * + * `setStatus`/`setResumeToken` are guarded for the SAME reason, and this is not belt-and-braces: + * they are consumer code exactly as `appendMessage` is (a `createThreadsStore` adapter over a + * remote backend, a store that rejects a threadId removed mid-stream), and guarding only the + * append leaves the identical wedge one line further down. The single thing that must ALWAYS run + * is the `setRuns` teardown — that entry is the HOOK'S OWN state, it is what `runs.get(threadId)` + * reports to the UI as "a turn is in flight", and by this point `stop()` has already deleted the + * thread from `controllersRef`, so no later `stop()` can ever reach here again to clean it up. A + * throw above it therefore strands a phantom run for the lifetime of the hook. Each store call + * gets its OWN try so a failing `setStatus` cannot also skip clearing the resume token — a + * surviving token is separately harmful (it is what a later resume replays from). */ async function finalizeStop(args: { threadId: string @@ -301,19 +367,39 @@ async function finalizeStop(args: { }): Promise { const { threadId, parts, alreadyAppended, storeRef, resolveOutcome, setRuns } = args + let appendFailed = false if (!alreadyAppended && parts.length > 0) { - const stoppedMessage: ChatMessage = { - id: crypto.randomUUID(), - role: 'assistant', - parts, - createdAt: Date.now(), - finish: 'stopped', + try { + const stoppedMessage: ChatMessage = { + // Same idempotency-key reasoning as consumeAndFinalize's assistantMessage — mintMessageId. + // This can throw on rung 3 (no usable crypto — see ./id.ts) just like appendMessage + // (consumer code) can throw for its own reasons; both are caught below so neither can + // wedge the thread — see this function's doc. + id: mintMessageId(), + role: 'assistant', + parts, + createdAt: Date.now(), + finish: 'stopped', + } + storeRef.current.appendMessage(threadId, stoppedMessage) + } catch { + appendFailed = true } - storeRef.current.appendMessage(threadId, stoppedMessage) } - storeRef.current.setStatus(threadId, 'done') - storeRef.current.setResumeToken(threadId, undefined) + let settleFailed = appendFailed + try { + storeRef.current.setStatus(threadId, appendFailed ? 'error' : 'done') + } catch { + settleFailed = true + } + try { + storeRef.current.setResumeToken(threadId, undefined) + } catch { + settleFailed = true + } + + // Unconditional — the one step that cannot be allowed to be skipped. See this function's doc. setRuns((prev) => { if (!prev.has(threadId)) return prev const next = new Map(prev) @@ -321,6 +407,10 @@ async function finalizeStop(args: { return next }) + // No outcome to resolve for a turn whose own terminal message never landed, or whose settle + // never took — see this function's doc on why this mirrors consumeAndFinalize's catch path. + if (settleFailed) return + const snapshot = storeRef.current.threads.find((thread) => thread.id === threadId) if (snapshot === undefined) return const outcome: AgentOutcome = await resolveOutcome(snapshot) @@ -419,11 +509,40 @@ export function useAgentThreadRuns({ // skipping the orphan-resume path and wedging the thread in 'streaming' forever. Reachable // whenever this fiber's effects re-run without the fiber itself unmounting (React 19 StrictMode // double-invoke; `` hide/show) — see this hook's `@example`-adjacent doc / the F3 note. + // + // Also tears down the matching `runs` entries — the F3 fix above stopped this from wedging + // `controllersRef`/the *persisted* thread status, but left `runs` (this hook's own transient + // state) untouched, and on a re-run-without-unmount the component (and its `runs` state) survive + // this cleanup. Without this, the entry the aborted controller was updating stays in `runs` + // reporting `status: 'streaming'` forever: the mount-reconcile effect below DOES correctly settle + // the persisted thread (to a resumed run, or to 'interrupted' if not resumable), but a consumer + // deriving "is this thread live" from `runs.has(id)` — the whole reason `runs` exists — keeps + // seeing a phantom in-flight turn no controller is driving anymore, and `stop()` on it was a + // permanent no-op (nothing left in `controllersRef` to abort) — see `stop()`'s own doc. Scoped to + // exactly the threadIds this pass is aborting (not a blind `runs.clear()`): the mount-reconcile + // effect below runs AFTER this cleanup completes, in the same synchronous commit (both are + // `[]`-dep effects on this same fiber, so StrictMode/`` destroy-then-recreate both + // together, in declaration order) — a resumable thread's reconcile pass registers a FRESH + // controller and a fresh `runs` entry there, and this teardown must never clobber that. It + // can't: by the time this cleanup runs, nothing has created a NEW entry for these threadIds yet + // (that only happens in the reconcile effect, still to come), so scoping to the ids this pass is + // itself aborting is exact, not merely defensive. useEffect( () => () => { + const abortedThreadIds = Array.from(controllersRef.current.keys()) controllersRef.current.forEach((controller) => controller.abort()) controllersRef.current.clear() appendedRef.current.clear() + if (abortedThreadIds.length === 0) return + setRuns((prev) => { + let next: Map> | undefined + for (const threadId of abortedThreadIds) { + if (!prev.has(threadId)) continue + next ??= new Map(prev) + next.delete(threadId) + } + return next ?? prev + }) }, [], ) @@ -500,7 +619,8 @@ export function useAgentThreadRuns({ lastInputRef.current.set(threadId, input) const userMessage: ChatMessage = { - id: crypto.randomUUID(), + // Same idempotency-key reasoning as consumeAndFinalize's assistantMessage — mintMessageId. + id: mintMessageId(), role: 'user', parts: toUserPartsRef.current(input), createdAt: Date.now(), @@ -557,7 +677,23 @@ export function useAgentThreadRuns({ // runsRef still holding the PREVIOUS render's (empty) map. Gating on runsRef here would read // `undefined`, wrongly treat this as a no-op, leave the controller registered and never // abort it — stranding the run in 'streaming' (see the F4 regression test). - if (controller === undefined) return + if (controller === undefined) { + // No controller — but if `runs` still reports this thread as live, that's a phantom entry + // (see this hook's `stop()` JSDoc): nothing is driving it, and with no controller to key + // off of, this is the only remaining path that can ever settle it. Not reachable via any + // path in THIS file today (the unmount-cleanup effect and finalizeStop both tear down + // `controllersRef` and `runs` in the same synchronous step — see their docs), but a true + // no-op here would mean "the UI shows this thread streaming forever, and Stop can never + // clear it" the instant that invariant is ever broken by a future change. Settle it via the + // same finalizeStop() the normal path uses, so there is still exactly one teardown routine + // for "a run is ending" regardless of how it got here. + if (!runsRef.current.has(threadId)) return + const parts = runsRef.current.get(threadId)?.parts ?? [] + const alreadyAppended = appendedRef.current.has(threadId) + appendedRef.current.delete(threadId) + void finalizeStop({ threadId, parts, alreadyAppended, storeRef, resolveOutcome, setRuns }) + return + } controllersRef.current.delete(threadId) // Read the accumulated parts straight out of the live run entry (runsRef mirrors `runs` as of diff --git a/packages/basalt-ui/src/agent/use-agent-thread-runs.wedge.test.tsx b/packages/basalt-ui/src/agent/use-agent-thread-runs.wedge.test.tsx index 5591d2f..cecc9c9 100644 --- a/packages/basalt-ui/src/agent/use-agent-thread-runs.wedge.test.tsx +++ b/packages/basalt-ui/src/agent/use-agent-thread-runs.wedge.test.tsx @@ -48,6 +48,17 @@ * that the thread SETTLES (leaves `'streaming'`) rather than wedging — the resume() call count is * asserted too, but as an observed constant with the doubling explained, not as evidence of a * single attempt. + * + * (d) is a SEPARATE, later-release defect discovered in the same family: the F3 fix above made the + * PERSISTED thread settle correctly (to a resume, or to 'interrupted'), but left the hook's OWN + * `runs` state untouched by the unmount-cleanup effect — so on a fiber that survives (StrictMode + * double-invoke, `` hide/show), `runs` kept reporting the aborted run as `'streaming'` + * forever, even once the persisted thread had already settled. Unlike (b)/(c) (which use a + * RESUMABLE transport, so a fresh `runs` entry gets created by the surviving resume), (d) uses a + * NON-resumable transport — the case where nothing ever replaces the phantom entry, and `stop()` + * on it was a permanent no-op (see this hook's `stop()` JSDoc). Verified manually: reverting the + * `runs` teardown in the unmount-cleanup effect makes (d) fail — `runs.has(threadId)` stays `true` + * after the thread settles to `'interrupted'` — while (a)/(b)/(c) are unaffected by that revert. */ import { describe, expect, test } from 'bun:test' import type { JSX } from 'react' @@ -304,4 +315,73 @@ describe('useAgentThreadRuns — F3 mount-reconcile / unmount-cleanup wedge', () }) expect(resumeCalls).toBe(2) }) + + test('(d) BLOCKING regression: a non-resumable run aborted by an hide/show cycle leaves no phantom `runs` entry, and stop() still reaches a terminal state', async () => { + const store = createTestThreadsStore() + + // No `resume` at all — unlike (c), this transport cannot be resumed. Hangs until aborted; + // never resolves on its own (matching the real playground repro this test pins). + const transport: AgentTransport = { + // oxlint-disable-next-line require-yield -- hangs until aborted; never has anything to yield + async *stream(_input, signal) { + await new Promise((resolve) => { + signal?.addEventListener('abort', () => resolve(undefined)) + }) + }, + } + + let externalSetMode: ((mode: 'visible' | 'hidden') => void) | undefined + let hookResult: ReturnType> | undefined + + function Probe(): null { + hookResult = useAgentThreadRuns({ transport, store, resolveOutcome }) + return null + } + + function Harness(): JSX.Element { + const [mode, setMode] = useState<'visible' | 'hidden'>('visible') + externalSetMode = setMode + return ( + + + + ) + } + + await act(async () => { + render() + }) + + const threadId = store.create() + act(() => { + hookResult?.start(threadId, 'hello') + }) + expect(hookResult?.runs.get(threadId)?.status).toBe('streaming') + + // Hide then show — the cleanup-without-unmount path. The fiber (and its refs/state) survive; + // only its effects are destroyed and recreated, exactly as React 19.2's does in + // PRODUCTION (see the file doc above) or as StrictMode does on every dev mount. + await act(async () => { + externalSetMode?.('hidden') + }) + await act(async () => { + externalSetMode?.('visible') + }) + + await waitFor(() => { + expect(store.threads.find((t) => t.id === threadId)?.status).toBe('interrupted') + }) + + // The BLOCKING defect this test pins: `runs` must not still report this thread as 'streaming' + // once the persisted thread has settled to 'interrupted' — that phantom entry is what made + // stop() a permanent no-op. + expect(hookResult?.runs.has(threadId)).toBe(false) + + // stop() on the now-settled thread reaches (stays at) a terminal state — not a wedge. + act(() => { + hookResult?.stop(threadId) + }) + expect(store.threads.find((t) => t.id === threadId)?.status).toBe('interrupted') + expect(hookResult?.runs.has(threadId)).toBe(false) + }) }) diff --git a/packages/basalt-ui/src/data/index.ts b/packages/basalt-ui/src/data/index.ts index df77985..1af6f49 100644 --- a/packages/basalt-ui/src/data/index.ts +++ b/packages/basalt-ui/src/data/index.ts @@ -7,7 +7,11 @@ * * Optional peers: * - @tanstack/react-table >=8 <9 (BasaltDataTable) - * - @tanstack/react-virtual >=3 <4 (BasaltVirtualList) + * - @tanstack/react-virtual >=3.13.26 <4 (BasaltVirtualList) + * + * BasaltVirtualList itself works on any 3.x; the declared floor is set by ./agent-chat's + * ThreadTranscript virtualize mode (it calls scrollToEnd/anchorTo, added in virtual-core 3.16.0, + * first pinned by react-virtual 3.13.26). npm has one peer range per package, so do not lower it. * * Install with: * bun add @tanstack/react-table @tanstack/react-virtual diff --git a/packages/basalt-ui/src/data/virtual-list.tsx b/packages/basalt-ui/src/data/virtual-list.tsx index a3de0c9..d12dc2e 100644 --- a/packages/basalt-ui/src/data/virtual-list.tsx +++ b/packages/basalt-ui/src/data/virtual-list.tsx @@ -1,7 +1,8 @@ /** * ./data — BasaltVirtualList: a windowed virtual list over @tanstack/react-virtual, * rendered with a Mantine Box scroll container. - * Optional peer: @tanstack/react-virtual >=3 <4. + * Optional peer: @tanstack/react-virtual >=3.13.26 <4. This list works on any 3.x — the floor is + * set by ./agent-chat's virtualize mode (see ./data/index.ts), so do not lower it. * * @example * import { BasaltVirtualList } from 'basalt-ui/data' diff --git a/packages/basalt-ui/src/data/virtual.ts b/packages/basalt-ui/src/data/virtual.ts index 5b28f36..883213d 100644 --- a/packages/basalt-ui/src/data/virtual.ts +++ b/packages/basalt-ui/src/data/virtual.ts @@ -2,7 +2,8 @@ * ./data/virtual — BasaltVirtualList: a windowed virtual list over @tanstack/react-virtual, * rendered with a Mantine Box scroll container. * - * Optional peer: @tanstack/react-virtual >=3 <4. + * Optional peer: @tanstack/react-virtual >=3.13.26 <4. This list works on any 3.x — the floor is + * set by ./agent-chat's virtualize mode (see ./data/index.ts), so do not lower it. * * Use this fine subpath instead of the `./data` barrel when your app only needs the virtual list * — it does NOT value-import @tanstack/react-table, so the data-table peer is never required. diff --git a/packages/basalt-ui/src/index.ts b/packages/basalt-ui/src/index.ts index ca5dc72..c797d14 100644 --- a/packages/basalt-ui/src/index.ts +++ b/packages/basalt-ui/src/index.ts @@ -113,6 +113,7 @@ export type { GuardKind } from './guard/types' export { ThreadWorkspace, ThreadFeed, + ThreadFeedRow, ThreadOutcomeCard, ThreadDetailPanel, Composer, @@ -123,6 +124,7 @@ export { export type { ThreadWorkspaceProps, ThreadFeedProps, + ThreadFeedRowProps, ThreadOutcomeCardProps, ThreadDetailPanelProps, ComposerProps, @@ -130,6 +132,9 @@ export type { ComposerAttachment, ComposerHandle, ThreadTranscriptProps, + MessageAffordances, + VirtualizeOptions, + VirtualizeProps, ToolChipProps, } from './agent-chat' export { diff --git a/packages/basalt-ui/src/surfaces.ts b/packages/basalt-ui/src/surfaces.ts index 2e5f381..a5b4300 100644 --- a/packages/basalt-ui/src/surfaces.ts +++ b/packages/basalt-ui/src/surfaces.ts @@ -385,7 +385,7 @@ export const SURFACES = { skill: ['basalt-app'], guardKinds: [], description: - 'Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed, ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx.', + 'Mantine-styled thread-chat components over basalt-ui/agent: ThreadWorkspace, ThreadFeed (variant/renderRow), ThreadFeedRow (inline-expanding Slack row, lazily mounted + kept mounted), ThreadOutcomeCard, ThreadDetailPanel, Composer, ThreadTranscript (open part-renderer registry via its renderers/fallbackRenderer props, per-message MessageAffordances, groupConsecutive, and an optional virtualize/height windowing mode whose VirtualizeOptions carry overscan/estimateSize/initialScroll — a virtualized transcript opens scrolled to the newest message unless initialScroll is "start"), threadPartRenderers, ToolChip (Mantine-coupled). motion is required, not optional — ThreadFeed/ThreadDetailPanel import motion/react eagerly, so this subpath fails to resolve without it installed even though peerDependenciesMeta marks it optional (npm has no per-subpath optionality). remend is genuinely optional here — ThreadTranscript reaches it only through the lazy dynamic import() inside content/markdown.tsx, and @tanstack/react-virtual the same way through the lazy import() behind ThreadTranscript virtualize (absent peer degrades to an unwindowed, height-bound pane).', optionalPeers: [ 'ai', 'motion', @@ -397,6 +397,7 @@ export const SURFACES = { '@shikijs/langs', '@shikijs/themes', 'beautiful-mermaid', + '@tanstack/react-virtual', ], forbiddenImports: [], }, diff --git a/packages/basalt-ui/src/theme/shadow-surfaces.test.ts b/packages/basalt-ui/src/theme/shadow-surfaces.test.ts index 4fb09a1..3e41707 100644 --- a/packages/basalt-ui/src/theme/shadow-surfaces.test.ts +++ b/packages/basalt-ui/src/theme/shadow-surfaces.test.ts @@ -386,6 +386,15 @@ export const SHADOW_SURFACES: readonly ShadowSurfaceEntry[] = [ site: 'default', roundedBy: 'borderRadius: VX.radiusCard (co-declared in the same inline style object).', }, + { + file: 'agent-chat/thread-feed-row.tsx', + site: 'default', + roundedBy: + 'borderRadius: VX.radiusCard (co-declared in the same inline style object as the shadow). ' + + 'This box also carries overflow: hidden, which is what lets the header button and the ' + + 'expanded body square off against the row’s rounded corners; an element’s own overflow ' + + 'never clips its own ring, so the shadow is unaffected.', + }, // ── agent ──────────────────────────────────────────────────────────────────────────────────── { file: 'agent/stick-to-bottom.tsx',