diff --git a/apps/console/CLAUDE.md b/apps/console/CLAUDE.md index 5125eaab..b4391a64 100644 --- a/apps/console/CLAUDE.md +++ b/apps/console/CLAUDE.md @@ -42,16 +42,21 @@ via their pre-hooks. - **Entry** — `src/main.tsx` mounts ``. The persisted theme is applied *before* first paint to avoid a light flash. - **State** — `src/store.tsx` holds all app state and actions (`route`, - `resolveConflict`, `send`, view/selection setters) in **three** contexts, split + `resolveConflict`, `send`, view/selection setters) in **four** contexts, split by how often each changes: `data` (engine answers + every action, and every - action has a stable identity), `nav` (view and selection), `input` (the search - box and the chat composer — changes per keystroke). `useStoreData()` / - `useStoreNav()` / `useStoreInput()` are the narrow hooks; `useStore()` merges - all three and re-renders on any of them — it has no production callers left, - only test mocks, and new code should not add one. Callbacks read the freshest - values through refs so they don't re-subscribe. State is in-memory only — - reloads reset it. See the subscribe-narrowly gotcha below before adding a - consumer. + action has a stable identity), `nav` (view and selection), `input` (the toolbar + search box — changes per keystroke), `chat` (the Ask composer, its transcript + and its busy flag — changes per keystroke). `useStoreData()` / `useStoreNav()` + / `useStoreInput()` / `useStoreChat()` are the narrow hooks; `useStore()` + merges all four and re-renders on any of them — it has no production callers + left, only test mocks, and new code should not add one. The two typing + surfaces are deliberately separate contexts: `query` is read by the Header + that owns the field and by all five searchable views, the composer only by + `ChatPanel`, and while they shared one context a question typed into the Ask + slide-over repainted the view under it per character. Callbacks read the + freshest values through refs so they don't re-subscribe. State is in-memory + only — reloads reset it. See the subscribe-narrowly gotcha below before + adding a consumer. - **Views** — `src/views/` (Canvas, Overview, Sources, Triage, Conflicts, Concepts, Files). `App.tsx` is the shell: topbar + subbar + routed view, plus the Triage S/R/D keyboard handler. The canvas view stays full-height inside @@ -101,7 +106,10 @@ via their pre-hooks. triage/activity fixtures. Live errors are typed (`LiveDataError`) and rendered honestly — never a silent fallback to demo. - **Chat** — `src/components/ChatPanel.tsx` + `store.send()` call - `window.claude.complete` when present and fall back to canned answers. + `window.claude.complete` when present and fall back to canned answers. The + panel is the only component that calls `useStoreChat()` (the other caller is + `useStore()` itself), and should stay that way: the hook re-renders its caller + for every character typed into the composer. Key files: `src/store.tsx` (state), `src/theme.ts` (`css()` + tokens), `src/styles.css` (shell/theme variables), `src/views/Canvas.tsx` (pan/zoom layout), @@ -129,7 +137,33 @@ Key files: `src/store.tsx` (state), `src/theme.ts` (`css()` + tokens), `SEARCHABLE_VIEWS` (`shell-navigation.ts`) must be in the case table in `render-hygiene.test.tsx`, which types a query that matches nothing and asserts the list actually empties. That suite deliberately holds both halves: - the sidebar must NOT repaint on a keystroke, and the active view MUST. + the sidebar must NOT repaint on a search keystroke, and the active view MUST. + It holds the same pair for the composer, table-driven over the same + `SEARCHABLE_VIEWS` gate, with the Ask panel open over each view: the view must + NOT repaint, and the composer must still hold what was typed. +- **Count renders inside the memo boundary, not at a leaf.** Views render no + icons, so the chat cases needed a probe of their own, and the first one — + counting `LayerChip`, which `Concepts` renders per row — was blind by + construction: one `memo` between the view and the chip (match-highlighting, + virtualization) zeroes the probe, and zero is what the test asserts. `counted()` + in `render-hygiene.test.tsx` instead re-exports the view module with a counter + wrapping the view's own inner function (unwrapping `React.memo`), so the view's + hooks become the counter's hooks and every context it subscribes to is + observed. A wrapper AROUND the view counts the parent, which a context-driven + re-render never touches — the same reason `React.Profiler` reports zero here. +- **A `data` value that changes identity per provider render defeats the whole + split, and demo-mode tests cannot see it.** `App` subscribes to `data` and + owns every memoized child, so `data` changing identity on every provider + render *is* a whole-tree repaint per keystroke. That shipped: `activity` was + `mode === 'demo' ? demoActivity : []`, and the inline `[]` — live mode only — + gave `data` a new identity every render. Both context splits measured zero in + `render-hygiene.test.tsx` and bought nothing in the Mac app, because + `createDataSource()` with no query string picks demo, the one mode that took + the stable branch. Hence `NO_ACTIVITY` in `store.tsx`, and + `render-hygiene.live.test.tsx`, which mounts the store with a source that + reports `mode: 'live'` while still answering from the demo bundle and pins the + same properties there. Anything added to the `data` memo's dependency list + must be stable across a render in **both** modes. - **Prefer `C.*` / `css()` over raw styles** so both themes and the reduced-motion / focus-visible rules keep working. - **`css()` is a simple `;`/`:` splitter** — no nested rules, no `url(...)` with diff --git a/apps/console/src/App.tsx b/apps/console/src/App.tsx index 1e3e4c23..83cdf799 100644 --- a/apps/console/src/App.tsx +++ b/apps/console/src/App.tsx @@ -67,9 +67,11 @@ function ErrorState({ kind, message, reload }: { kind: LiveErrorKind; message: s } export function App() { - // Deliberately three narrow subscriptions rather than `useStore()`: the shell + // Deliberately two narrow subscriptions rather than `useStore()`: the shell // owns every memoized child below, so an App render is a whole-tree render. - // Typing in the toolbar search must not cause one. + // Typing — in the toolbar search or the chat composer — must not cause one, + // which is also why `data` must not change identity per provider render (see + // NO_ACTIVITY in store.tsx). const { setView, openChat, closeChat, route, loading, load, error, reload, retryNow, mode, sources, loadErrors, openFilesScope } = useStoreData() const { view, chatOpen } = useStoreNav() // Undefined = not yet decided by the auto-trigger effect below; true/false diff --git a/apps/console/src/components/BackgroundActivity.test.tsx b/apps/console/src/components/BackgroundActivity.test.tsx index df66da17..bdace9d6 100644 --- a/apps/console/src/components/BackgroundActivity.test.tsx +++ b/apps/console/src/components/BackgroundActivity.test.tsx @@ -11,10 +11,13 @@ import { LiveDataError } from '../api' import type { BackgroundTask, Store } from '../store' const mocks = vi.hoisted(() => ({ store: { current: null as unknown as Store } })) -// The store is three contexts now; this component reads the data half. +// The store is four contexts now; this component reads the data half. All four +// hooks are declared even though this component reads one, because the factory +// REPLACES the module: a hook left out is a `not a function` crash the day some +// child reaches for it, and tsc cannot see into an untyped factory. vi.mock('../store', () => { const store = () => mocks.store.current - return { useStore: store, useStoreData: store, useStoreNav: store, useStoreInput: store } + return { useStore: store, useStoreData: store, useStoreNav: store, useStoreInput: store, useStoreChat: store } }) let container: HTMLDivElement diff --git a/apps/console/src/components/ChatPanel.tsx b/apps/console/src/components/ChatPanel.tsx index 58c369bb..1eb5f9b2 100644 --- a/apps/console/src/components/ChatPanel.tsx +++ b/apps/console/src/components/ChatPanel.tsx @@ -1,13 +1,13 @@ import { memo, useEffect, useRef, useState } from 'react' import { C, css, lc, MONO } from '../theme' -import { useStoreData, useStoreInput } from '../store' +import { useStoreChat, useStoreData } from '../store' const SUGGESTIONS = ['What database do we use?', 'How do we handle on-call?'] const MCP_CONFIG_CMD = 'claude mcp add contextcake -- node mcp-server.mjs --manifest layers.json' function ChatPanelInner({ keyboardSuspended = false, onConnectAgent, onClose }: { keyboardSuspended?: boolean; onConnectAgent?: () => void; onClose: () => void }) { const { setChatInput, send } = useStoreData() - const { chatMessages, chatBusy, chatInput } = useStoreInput() + const { chatMessages, chatBusy, chatInput } = useStoreChat() const scrollRef = useRef(null) const inputRef = useRef(null) const panelRef = useRef(null) diff --git a/apps/console/src/components/SetupWizard.test.tsx b/apps/console/src/components/SetupWizard.test.tsx index a3c86c0d..51427b8c 100644 --- a/apps/console/src/components/SetupWizard.test.tsx +++ b/apps/console/src/components/SetupWizard.test.tsx @@ -12,7 +12,7 @@ vi.mock('../api', async (importOriginal) => ({ })) vi.mock('../store', () => { const store = () => ({ reload: mocks.reload }) - return { useStore: store, useStoreData: store, useStoreNav: store, useStoreInput: store } + return { useStore: store, useStoreData: store, useStoreNav: store, useStoreInput: store, useStoreChat: store } }) let container: HTMLDivElement diff --git a/apps/console/src/render-hygiene.live.test.tsx b/apps/console/src/render-hygiene.live.test.tsx new file mode 100644 index 00000000..7e2763d6 --- /dev/null +++ b/apps/console/src/render-hygiene.live.test.tsx @@ -0,0 +1,188 @@ +// @vitest-environment jsdom +// The render-budget suite next door mounts the store in demo mode, because that +// is what `createDataSource()` picks with no query string. Demo mode is also the +// only mode in which the provider's `activity` is a stable module constant — in +// live mode it was a fresh `[]` on every provider render, which changed the +// identity of the whole `data` context every time ANY provider state moved. +// +// That defeated the context split outright: `App` subscribes to `data` and owns +// every memoized child, so a keystroke repainted the tree in exactly the mode +// the Mac app ships in, while the suite next door measured zero and passed. +// +// So this file pins the same properties with the store in live mode. It is a +// separate file because `vi.mock` is per-file and the suite next door must stay +// on the demo path. +// +// ONE INVARIANT HOLDS THESE TESTS UP: every window between capturing a baseline +// count and asserting on it must stay synchronous. Live mode really does arm +// the poll loop (one 5s timer per test, cleared on unmount), and a poll that +// landed mid-window would legitimately move the numbers — `setLastRefreshAt` +// feeds `load`, and `load` is a dependency of the `data` memo. No poll can +// interleave with straight-line code, which is why the counts are exact today. +// Add an `await` between a baseline and its assertion and this becomes a +// five-second timing flake that CI will find long before you do. +import { act, type ComponentType } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { App } from './App' +import { Header } from './components/Header' +import { ChatPanel } from './components/ChatPanel' +import { StoreProvider, useStoreData } from './store' +import { ThemeModeProvider } from './theme-mode' +import { Concepts } from './views/Concepts' + +type AnyComponent = (props: Record) => unknown + +const renders: Record = {} + +// A live-mode source that still answers from the demo bundle. The mode flag is +// the whole variable under test: the payloads are identical either way, and +// swapping in a real LiveSource would only measure jsdom's missing network. +// +// The Proxy returns unbound methods, which then run with `this` set to the +// proxy. That resolves only because DemoSource's fields are TypeScript +// `private` — a compile-time marker over an ordinary property, so `this.bundle` +// re-enters the trap and finds the target's value. Convert one of them to a +// real `#private` field and every call through here throws, surfacing as the +// store's generic `refreshError` rather than as an obvious test failure. Bind +// to the target here if that day comes. +vi.mock('./api', async () => { + const actual = await vi.importActual('./api') + return { + ...actual, + createDataSource: () => new Proxy(actual.createDataSource('demo'), { + get: (target, key) => (key === 'mode' ? 'live' : Reflect.get(target, key, target)), + }), + } +}) + +/** Render counter inside the memo boundary — see the note in render-hygiene.test.tsx. */ +async function counted(name: string, actual: Record) { + const { memo } = await import('react') + const exported = actual[name] + const wasMemo = typeof exported !== 'function' + const Inner = (wasMemo ? (exported as { type: unknown }).type : exported) as AnyComponent + const Counted = (props: Record) => { + renders[name] = (renders[name] ?? 0) + 1 + return Inner(props) + } + return { ...actual, [name]: wasMemo ? memo(Counted as unknown as ComponentType) : Counted } +} + +vi.mock('./views/Concepts', async () => counted('Concepts', await vi.importActual('./views/Concepts'))) +vi.mock('./App', async () => counted('App', await vi.importActual('./App'))) + +let container: HTMLDivElement +let root: Root + +/** + * Stands in for `App`: subscribes to `data` and nothing else. App owns every + * memoized child in the shell, so one render of this is one render of the tree + * — and unlike App it drags in no routing, no wizard and no IPC. + */ +function DataConsumerProbe() { + useStoreData() + renders.probe = (renders.probe ?? 0) + 1 + return null +} + +function typeInto(field: HTMLInputElement | HTMLTextAreaElement, text: string) { + const proto = field instanceof HTMLTextAreaElement + ? window.HTMLTextAreaElement.prototype + : window.HTMLInputElement.prototype + const setter = Object.getOwnPropertyDescriptor(proto, 'value')?.set + act(() => { + setter?.call(field, text) + field.dispatchEvent(new Event('input', { bubbles: true })) + }) +} + +beforeEach(() => { + ;(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + for (const key of Object.keys(renders)) delete renders[key] + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.unstubAllGlobals() + window.history.replaceState(null, '', '/') + document.documentElement.removeAttribute('data-theme') +}) + +describe('the render budget holds in live mode too', () => { + it('keeps a keystroke off the data context, in either typing surface', async () => { + window.location.hash = '#/concepts' + await act(async () => root.render( + + +
{}} onAsk={() => {}} /> + + {}} /> + , + )) + await act(async () => { await Promise.resolve() }) + await act(async () => { await Promise.resolve() }) + + const composer = container.querySelector('.cc-ask-panel textarea') + expect(composer, 'the chat panel rendered no composer').toBeTruthy() + const search = container.querySelector('input[data-context-search]') + expect(search, 'the toolbar rendered no search field').toBeTruthy() + const conceptId = container.querySelector('.cc-navigator-detail > div > button.cc-h-bd-strong code')?.textContent + expect(conceptId, 'the view rendered no concept rows — live mode served nothing').toBeTruthy() + + const viewBefore = renders.Concepts ?? 0 + const probeBefore = renders.probe ?? 0 + expect(viewBefore, 'the view never rendered — the probe is counting nothing').toBeGreaterThan(0) + expect(probeBefore, 'the data probe never rendered').toBeGreaterThan(0) + + // A question typed over the top of a view touches neither the view nor the + // shell that hosts it. + for (const text of ['w', 'wh', 'wha', 'what']) typeInto(composer!, text) + expect(composer!.value).toBe('what') + expect((renders.Concepts ?? 0) - viewBefore).toBe(0) + expect((renders.probe ?? 0) - probeBefore).toBe(0) + + // And a search keystroke reaches the view it filters — without repainting + // the shell, which is the property the original context split bought. + const viewBeforeSearch = renders.Concepts ?? 0 + const probeBeforeSearch = renders.probe ?? 0 + typeInto(search!, conceptId!) + expect(renders.Concepts ?? 0).toBeGreaterThan(viewBeforeSearch) + expect((renders.probe ?? 0) - probeBeforeSearch).toBe(0) + }) + + /** + * The probe above stands in for `App`'s data subscription, which is what this + * bug traveled through — but it is a stand-in, and it cannot see a NEW + * subscription added to the real App. Adding `useStoreChat()` there would + * repaint the shell's own inline JSX on every character and leave the suite + * green, because every child it renders is memoized and would bail. + * + * So this mounts the real thing and opens the panel the way a user does. + */ + it('does not re-render the shell itself while a question is typed into it', async () => { + window.history.replaceState(null, '', '/#/concepts') + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => window.setTimeout(() => cb(0), 0)) + await act(async () => root.render( + , + )) + await act(async () => { await Promise.resolve() }) + await act(async () => { await Promise.resolve() }) + + const ask = container.querySelector('.cc-toolbar-ask') + expect(ask, 'the toolbar rendered no Ask button').toBeTruthy() + await act(async () => { ask!.click() }) + const composer = container.querySelector('.cc-ask-panel textarea') + expect(composer, 'clicking Ask opened no composer').toBeTruthy() + + const before = renders.App ?? 0 + expect(before, 'the shell never rendered — the probe is counting nothing').toBeGreaterThan(0) + for (const text of ['w', 'wh', 'wha', 'what']) typeInto(composer!, text) + expect(composer!.value).toBe('what') + expect((renders.App ?? 0) - before).toBe(0) + }) +}) diff --git a/apps/console/src/render-hygiene.test.tsx b/apps/console/src/render-hygiene.test.tsx index fbfd656b..8e6567fb 100644 --- a/apps/console/src/render-hygiene.test.tsx +++ b/apps/console/src/render-hygiene.test.tsx @@ -14,6 +14,7 @@ import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { Sidebar } from './components/Sidebar' import { Header } from './components/Header' +import { ChatPanel } from './components/ChatPanel' import { StoreProvider } from './store' import { SEARCHABLE_VIEWS, type ViewId } from './shell-navigation' import { Concepts } from './views/Concepts' @@ -38,6 +39,42 @@ vi.mock('./components/icons', async () => { return counted }) +/** + * A view module, re-exported with a render counter at the top of the view's own + * render. + * + * The counter has to sit INSIDE the memo boundary. A wrapper component counts + * the parent, and a context-driven re-render never touches the parent — the + * same reason `React.Profiler` reports zero here. Calling the view's inner + * function inline makes its hooks this component's hooks, so every context the + * view subscribes to re-renders the counter. + * + * Counting a leaf the view happens to render does NOT have that property, and + * that was this suite's first attempt: `LayerChip`, which Concepts renders per + * row. One `memo` between the view and the chip — ordinary match-highlighting + * or list virtualization, on a repo that talks about 3,000-concept vaults — + * zeroes the probe, and zero is what these tests assert. The paired canary + * doesn't save it either, because a memo key that tracks the query satisfies + * the canary without a context ever reaching the view. + */ +async function counted(name: string, actual: Record) { + const { memo } = await import('react') + const exported = actual[name] + const wasMemo = typeof exported !== 'function' + const Inner = (wasMemo ? (exported as { type: unknown }).type : exported) as AnyComponent + const Counted = (props: Record) => { + renders[name] = (renders[name] ?? 0) + 1 + return Inner(props) + } + return { ...actual, [name]: wasMemo ? memo(Counted as unknown as ComponentType) : Counted } +} + +vi.mock('./views/Triage', async () => counted('Triage', await vi.importActual('./views/Triage'))) +vi.mock('./views/Concepts', async () => counted('Concepts', await vi.importActual('./views/Concepts'))) +vi.mock('./views/Conflicts', async () => counted('Conflicts', await vi.importActual('./views/Conflicts'))) +vi.mock('./views/Sources', async () => counted('Sources', await vi.importActual('./views/Sources'))) +vi.mock('./views/Files', async () => counted('Files', await vi.importActual('./views/Files'))) + let container: HTMLDivElement let root: Root @@ -48,11 +85,14 @@ function pointer(kind: string, clientX: number): PointerEvent { return event } -function typeInto(input: HTMLInputElement, text: string) { - const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')?.set +function typeInto(field: HTMLInputElement | HTMLTextAreaElement, text: string) { + const proto = field instanceof HTMLTextAreaElement + ? window.HTMLTextAreaElement.prototype + : window.HTMLInputElement.prototype + const setter = Object.getOwnPropertyDescriptor(proto, 'value')?.set act(() => { - setter?.call(input, text) - input.dispatchEvent(new Event('input', { bubbles: true })) + setter?.call(field, text) + field.dispatchEvent(new Event('input', { bubbles: true })) }) } @@ -152,22 +192,26 @@ describe('render hygiene', () => { */ const NO_MATCH = 'zzzzznomatchzzzzz' -/** `rows` names the list this view filters — the thing a query has to shrink. */ -const SEARCH_CASES: { view: ViewId; Component: ComponentType; rows: string }[] = [ +/** + * `rows` names the list this view filters — the thing a query has to shrink. + * `probe` is the key `counted()` counts this view's own renders under. + */ +const SEARCH_CASES: { view: ViewId; Component: ComponentType; rows: string; probe: string }[] = [ // Signal cards; the decision panel beside them uses h2. - { view: 'triage', Component: Triage, rows: 'h3' }, - { view: 'concepts', Component: Concepts, rows: '.cc-navigator-detail > div > button.cc-h-bd-strong' }, - { view: 'conflicts', Component: Conflicts, rows: '.cc-conflict-list > button[role="option"]' }, - { view: 'sources', Component: Sources, rows: 'button[role="option"]' }, - { view: 'files', Component: Files, rows: '[role="treeitem"]' }, + { view: 'triage', Component: Triage, rows: 'h3', probe: 'Triage' }, + { view: 'concepts', Component: Concepts, rows: '.cc-navigator-detail > div > button.cc-h-bd-strong', probe: 'Concepts' }, + { view: 'conflicts', Component: Conflicts, rows: '.cc-conflict-list > button[role="option"]', probe: 'Conflicts' }, + { view: 'sources', Component: Sources, rows: 'button[role="option"]', probe: 'Sources' }, + { view: 'files', Component: Files, rows: '[role="treeitem"]', probe: 'Files' }, ] -async function mountView(view: ViewId, Component: ComponentType) { +async function mountView(view: ViewId, Component: ComponentType, { chat = false } = {}) { window.location.hash = `#/${view}` await act(async () => root.render(
{}} onAsk={() => {}} /> + {chat && {}} />} , )) // The demo bundle resolves through a promise chain; let it land. @@ -199,3 +243,49 @@ describe('a search keystroke reaches the view', () => { }) } }) + +/** + * The chat composer is the second thing a user types into, and the one the + * split above did not cover: it lives in a slide-over rendered OVER the active + * view, and the view has nothing to say about it. While `chatInput` shared a + * context with `query`, every character of a question repainted whichever view + * happened to be underneath. + * + * Both halves are asserted for the same reason the search suite pairs its own: + * "the view did not re-render" is trivially satisfiable by a composer that + * stopped updating, and "the composer updated" says nothing about the tree + * beneath it. + */ +describe('a chat keystroke stays inside the chat', () => { + // Same table, same completeness gate as the search half: every view the shell + // offers a search box for is also a view the Ask panel can open over. + it('covers every view the panel can open over', () => { + expect(new Set(SEARCH_CASES.map((entry) => entry.view))).toEqual(SEARCHABLE_VIEWS) + }) + + for (const { view, Component, probe } of SEARCH_CASES) { + it(`leaves ${view} alone under the panel, while a search keystroke still reaches it`, async () => { + await mountView(view, Component, { chat: true }) + const composer = container.querySelector('.cc-ask-panel textarea') + expect(composer, 'the chat panel rendered no composer').toBeTruthy() + const search = container.querySelector('input[data-context-search]') + expect(search, `${view} is searchable but the toolbar rendered no search field`).toBeTruthy() + + const before = renders[probe] ?? 0 + expect(before, `${view} never rendered — the probe is counting nothing`).toBeGreaterThan(0) + for (const text of ['w', 'wh', 'wha', 'what']) typeInto(composer!, text) + + // The composer is live: four characters, and it holds all four. Without + // this half, a composer that stopped updating would satisfy the next line. + expect(composer!.value).toBe('what') + // And the view underneath sat every one of them out. + expect((renders[probe] ?? 0) - before).toBe(0) + + // The other half — the same view still repaints for the box that IS its + // own, so "did not re-render" can't be answered by a view that never does. + const beforeSearch = renders[probe] ?? 0 + typeInto(search!, NO_MATCH) + expect(renders[probe] ?? 0).toBeGreaterThan(beforeSearch) + }) + } +}) diff --git a/apps/console/src/store.tsx b/apps/console/src/store.tsx index 0b7edb18..1174e5a6 100644 --- a/apps/console/src/store.tsx +++ b/apps/console/src/store.tsx @@ -47,6 +47,9 @@ export interface ChatMessage { canned?: boolean } +/** Live mode has no activity feed. Shared so its identity never moves. */ +const NO_ACTIVITY: Activity[] = [] + const initialMessages: ChatMessage[] = [ { role: 'assistant', intro: true, text: "Ask me anything about your team's knowledge. I read the resolved cascade — Company, Team, and your Personal layer — and tell you which layer each answer comes from." }, ] @@ -178,7 +181,7 @@ function asLiveDataError(e: unknown): LiveDataError { } /** - * The store is three contexts, not one, and the split is by how often each + * The store is four contexts, not one, and the split is by how often each * changes rather than by subject. * * A single context value memoized over ~25 dependencies meant one keystroke in @@ -188,13 +191,23 @@ function asLiveDataError(e: unknown): LiveDataError { * * data — engine answers and every action. Changes when the cascade changes. * nav — where the user is. Changes on navigation and selection. - * input — the search box and the chat composer. Changes per keystroke. + * input — the toolbar search box. Changes per keystroke. + * chat — the Ask composer and its transcript. Changes per keystroke. + * + * The two typing surfaces are separate contexts because they have disjoint + * audiences: `query` is read by the Header that owns the field and by all five + * searchable views, the composer only by the Ask panel. Sharing one context + * meant a question typed into a panel floating OVER a view repainted the view + * for every character of it. + * + * None of this survives a `data` value that changes identity per provider + * render — see NO_ACTIVITY above, which is what that mistake looks like. * * Actions all live in `data` and are all stable identities, so a memoized child * that takes one as a prop keeps its memo. * - * `useStore()` still hands back all three merged, for consumers that genuinely - * read across them; it re-renders on any of the three, which is the cost of + * `useStore()` still hands back all four merged, for consumers that genuinely + * read across them; it re-renders on any of the four, which is the cost of * that convenience. Prefer the narrow hooks in anything on a hot path. */ export interface StoreData { @@ -277,19 +290,32 @@ export interface StoreNav { chatOpen: boolean } -/** The two things a user types into. Changes per keystroke — subscribe narrowly. */ +/** + * The toolbar search box. Changes per keystroke — subscribe narrowly, and only + * where a query actually filters something on screen. + */ export interface StoreInput { query: string +} + +/** + * The Ask panel: what is being typed, what has been said, and whether an answer + * is in flight. Read by the panel and nothing else — which is the whole point + * of it being its own context, since the panel renders over a view that must + * not repaint while a question is being typed into it. + */ +export interface StoreChat { chatBusy: boolean chatInput: string chatMessages: ChatMessage[] } -export type Store = StoreData & StoreNav & StoreInput +export type Store = StoreData & StoreNav & StoreInput & StoreChat const StoreDataContext = createContext(null) const StoreNavContext = createContext(null) const StoreInputContext = createContext(null) +const StoreChatContext = createContext(null) export function StoreProvider({ children }: { children: ReactNode }) { const source = useMemo(() => createDataSource(), []) @@ -321,7 +347,13 @@ export function StoreProvider({ children }: { children: ReactNode }) { // Triage signals and the activity feed have no resolver equivalent — demo-only // fixtures (D6: live-mode triage is read-only, and there is no signal API). const [signals, setSignals] = useState(mode === 'demo' ? initialSignals : []) - const activity = mode === 'demo' ? demoActivity : [] + // NO_ACTIVITY, not a fresh `[]`: this is a dependency of the `data` memo, so + // an inline literal gave `data` a new identity on every provider render — and + // in live mode, which is the only mode that took that branch, that defeated + // the entire context split. `App` subscribes to `data` and owns every + // memoized child, so a keystroke anywhere repainted the whole tree in the + // mode the Mac app ships in, while the demo-mode render tests measured zero. + const activity = mode === 'demo' ? demoActivity : NO_ACTIVITY const initial = useMemo(initialRoute, []) const [view, setViewState] = useState(initial.view) @@ -963,15 +995,19 @@ export function StoreProvider({ children }: { children: ReactNode }) { [view, triageTab, selSignal, selConflict, selConcept, filesScope, filesPath, chatOpen], ) - const input = useMemo( - () => ({ query, chatBusy, chatInput, chatMessages }), - [query, chatBusy, chatInput, chatMessages], + const input = useMemo(() => ({ query }), [query]) + + const chat = useMemo( + () => ({ chatBusy, chatInput, chatMessages }), + [chatBusy, chatInput, chatMessages], ) return ( - {children} + + {children} + ) @@ -992,19 +1028,30 @@ export function useStoreNav(): StoreNav { return required(useContext(StoreNavContext), 'useStoreNav') } -/** Search box and chat composer. Re-renders per keystroke — subscribe last. */ +/** The toolbar search box. Re-renders per keystroke — subscribe last. */ export function useStoreInput(): StoreInput { return required(useContext(StoreInputContext), 'useStoreInput') } /** - * All three at once. Convenient, and correspondingly expensive: a consumer of - * this re-renders on every keystroke whether or not it reads `query`. Reach for - * the narrow hooks in anything that renders more than a few nodes. + * The Ask panel's own state. Re-renders per keystroke in the composer, so this + * belongs to the panel — a view that reaches for it signs itself up to repaint + * while someone types a question over the top of it. + */ +export function useStoreChat(): StoreChat { + return required(useContext(StoreChatContext), 'useStoreChat') +} + +/** + * All four at once. Convenient, and correspondingly expensive: a consumer of + * this re-renders on every keystroke in either typing surface, whether or not + * it reads either one. Reach for the narrow hooks in anything that renders more + * than a few nodes. */ export function useStore(): Store { const data = useStoreData() const nav = useStoreNav() const input = useStoreInput() - return useMemo(() => ({ ...data, ...nav, ...input }), [data, nav, input]) + const chat = useStoreChat() + return useMemo(() => ({ ...data, ...nav, ...input, ...chat }), [data, nav, input, chat]) } diff --git a/apps/console/src/views/Conflicts.test.tsx b/apps/console/src/views/Conflicts.test.tsx index dd5cbc21..2c49cdff 100644 --- a/apps/console/src/views/Conflicts.test.tsx +++ b/apps/console/src/views/Conflicts.test.tsx @@ -10,7 +10,7 @@ const mocks = vi.hoisted(() => ({ useStore: vi.fn(), })) -vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore })) +vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore, useStoreChat: mocks.useStore })) let container: HTMLDivElement let root: Root diff --git a/apps/console/src/views/Files.test.tsx b/apps/console/src/views/Files.test.tsx index 5dabf360..f73f38d0 100644 --- a/apps/console/src/views/Files.test.tsx +++ b/apps/console/src/views/Files.test.tsx @@ -57,11 +57,13 @@ vi.mock('../store', async () => { return { filesScope, filesPath } } const useStoreInput = () => ({ query: mocks.store.query }) + const useStoreChat = () => ({ chatBusy: false, chatInput: '', chatMessages: [] }) return { useStoreData, useStoreNav, useStoreInput, - useStore: () => ({ ...useStoreData(), ...useStoreNav(), ...useStoreInput() }), + useStoreChat, + useStore: () => ({ ...useStoreData(), ...useStoreNav(), ...useStoreInput(), ...useStoreChat() }), } }) diff --git a/apps/console/src/views/Overview.test.tsx b/apps/console/src/views/Overview.test.tsx index 7b9e063c..0f127e97 100644 --- a/apps/console/src/views/Overview.test.tsx +++ b/apps/console/src/views/Overview.test.tsx @@ -5,7 +5,7 @@ import { afterEach, beforeEach, expect, it, vi } from 'vitest' import { Overview } from './Overview' const mocks = vi.hoisted(() => ({ useStore: vi.fn(), setView: vi.fn() })) -vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore })) +vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore, useStoreChat: mocks.useStore })) let container: HTMLDivElement let root: Root diff --git a/apps/console/src/views/Sources.test.tsx b/apps/console/src/views/Sources.test.tsx index b8c4bae3..2da38c8f 100644 --- a/apps/console/src/views/Sources.test.tsx +++ b/apps/console/src/views/Sources.test.tsx @@ -11,7 +11,7 @@ import type { Source } from '../data' const mocks = vi.hoisted(() => ({ apiFetch: vi.fn(), useStore: vi.fn(), reload: vi.fn(), openFilesScope: vi.fn() })) vi.mock('../api', () => ({ apiFetch: mocks.apiFetch })) -vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore })) +vi.mock('../store', () => ({ useStore: mocks.useStore, useStoreData: mocks.useStore, useStoreNav: mocks.useStore, useStoreInput: mocks.useStore, useStoreChat: mocks.useStore })) let container: HTMLDivElement let root: Root