feat(chat): unify provider workflows and improve transcript performance - #1206
feat(chat): unify provider workflows and improve transcript performance#1206blackmammoth wants to merge 98 commits into
Conversation
Subscribe TaskMaster directly to websocket events so rapid broadcasts are handled individually without forcing every frame through shared React state.
Separate changing processing-session state from stable actions so consumers can subscribe without forcing the application shell to own the map.
Keep composer focus local, stabilize shell callbacks, memoize shell children, and let chat/sidebar consume session activity without routing updates through AppContent.
Moves every feature out of src/components into src/modules/<feature> and
dissolves the legacy top-level frontend directories:
- src/components/* -> src/modules/*, with the banned view/ layer removed and
subcomponents flattened to the module root or into named directories
(tabs/, modals/, markdown/, and the git-panel view groups)
- src/contexts, src/hooks, src/stores, src/lib, src/types, src/utils and
src/i18n are gone; their contents moved to the owning module or to
src/shared/{context,hooks,ui,types.ts,utils.ts,constants.ts,api.ts}
- i18n is now a feature module at src/modules/i18n (config, languages,
locales and LanguageSelector), exposing i18n and LanguageSelector
- src/shared/view/ui became src/shared/ui; llm-provider-logo became shared UI
- every application-source import uses the @/ alias; no interfaces remain
- 150 multi-file types now live in src/shared/types.ts, grouped per owner
- every feature module has an index.ts barrel and all cross-module imports
go through it
- the frontend no longer imports package.json: the version is injected by
Vite define and read via APP_VERSION in src/shared/constants.ts
Validation: oxlint 0 errors, tsc --noEmit clean, 57/57 client tests,
vite build succeeds.
…t module UI Second migration milestone, applying the skill's placement rules per module: - deletes every src/modules/*/types.ts and src/modules/*/constants.ts; each definition moved into its sole consumer, or (when two or more files use it) into src/shared/types.ts / src/shared/constants.ts - sidebar's utils/utils.ts is split into the descriptively named sidebarProjectFormatting.ts and sidebarStoredPreferences.ts; auth's and onboarding's module-level utils.ts are inlined into their sole consumers - prd-editor's two modals move into modals/ - every exported component now carries a comment naming its consumers - src/shared/ui exports at the declaration, drops exports with no consumers, and gains LLMProviderLogo; shared UI and context components are documented Validation: oxlint 0 errors, tsc --noEmit clean, 57/57 client tests, vite build succeeds.
Final migration milestone: - documents every definition in src/shared/types.ts, utils.ts and constants.ts, grouped with the required section separators - relocates shared types that ended up with a single user into that file (AuthUser, FileIconData, PreferenceToggleItem, the TaskMaster context types, GitStatusFileGroup) and un-exports the ones only shared/types.ts itself references - promotes FileOpenHandler/FileDiffInfo and AgentContextByProvider into src/shared/types.ts now that two files use each - prunes barrel exports that no consumer imports - main.tsx initialises i18n through the module barrel - clears the duplicate-import, import-order and unused-import warnings the migration introduced (lint warnings 172 vs 173 at baseline) Validation: npm run lint exit 0 with 0 errors, npm run typecheck clean, 57/57 client tests, npm run build:client succeeds.
…ver/ Burns down the meaningful part of the 172-warning backlog (now 122), keeping each change small and behaviour-preserving. Bugs - cursor-runtime.provider: `spawnCursor` awaited `resolveResumeModel` inside an async Promise executor, so a rejection was swallowed and the returned promise never settled. Both id/model lookups now run before the promise is created. - useChatSessionState: restore the `setIsLoadingMoreMessages` calls dropped in the session-store refactor (a4632dc). The state was never written, so ChatMessagesPane's "loading older messages" indicator was unreachable. - SidebarHeader: `LogoBlock` was declared inside the render, remounting the wordmark on every render. Hoisted to a module-level component. - CodeEditorSurface: `max-w-4xl max-w-none` conflicted; dropped the losing `max-w-none` so the rendered width is unchanged and the intent is explicit. - voice.service.test: `requestedOptions?.headers` fed a non-optional member access, which would throw instead of failing the assertion. Stale/unstable references - WebSocketContext: the connect effect referenced `connect` before its initialization and omitted it from the deps. Moved below the callback and added the dep (identical reconnect transitions); `connect` is now a named function expression so its reconnect timer does not capture itself mid-init. - useFileTreeOperations: same ordering issue for `handleDownload` and the two download helpers it dispatches to. - PaletteOpsContext: capture the registry object once per effect instead of reading `ref.current` from the cleanup. - useUiPreferences: `Math.random()` ran on every render to seed an instance id; replaced with `useId()`. - useSessionStore/useChatSessionState/AskUserQuestionPanel: stable empty-array constants so memoized values keep their identity. - FileTree: destructure `treeRef` out of the upload hook's result so its plain values are no longer treated as render-time ref reads. Derived state and dead code - useFileTreeSearch: derive `filteredFiles` with useMemo instead of mirroring it into state through an effect (one fewer render per keystroke, and the value no longer lags `files` by a render). - useFileTreeViewMode: read the persisted mode in the state initializer instead of overwriting the default from an effect. - useChatSessionState: drop the unused `isLoadingSessionRef`. - project-star.service: drop two redundant `Boolean()` casts. Left in place, deliberately: react/set-state-in-effect (74) and react/refs (16) are mostly localStorage/prop synchronization and latest-value ref mirrors whose fixes are structural, not local; react/only-export-components (24) is HMR-only and inherent to the context+hook and cva-variant file layout; the four preserve-manual-memoization findings are React Compiler bail-outs and the compiler is not enabled; the two static-components findings are false positives (an icon component picked from a lookup table); the remaining no-async-promise-executor executor already wraps its body in try/catch-reject, so removing it is a ~60-line reindent with no behavioural gain; and the one exhaustive-deps omission in the session-loading effect is intentional and now documented in place. Verified with npm run lint, typecheck, test (270 pass), test:client (57 pass) and build.
Expose ultracode for capable Claude models and remove the unused git-panel file selection controls.
P0: swap the client test runner from `tsx --test` to vitest so hooks and components with effects can be tested at all (jsdom + renderHook + RTL). Bug fixes: - Renaming the selected project left the workspace header and document title stale. `fetchProjects` wrote only `projects`, and `selectedProject` is a denormalized copy of one of its rows; the rename path never resynced it. - Opening Settings rewrote the four codeEditor* keys from a mount effect using a default of '14', silently changing the editor font size for anyone who had never set it. The keys and their defaults now come from one shared constant and are written only on an explicit edit. - The provider -> tool-settings key lookup was a nested ternary ending in Claude's key, so dropping a provider's arm would make it inherit Claude's skipPermissions. It is now an explicit per-provider map. Render fixes: - Streaming re-parsed the whole accumulated reply through remark/rehype/KaTeX and Prism every 100ms. The streaming row now splits at a block boundary so only the block still being written is re-parsed, and Markdown is memoized. - Syntax highlighting switched between two Prism theme objects, re-tokenizing every mounted code block on a theme toggle. The theme-dependent values are now CSS variables derived from those same objects, so a toggle is a style recalculation. - The sidebar search jump rendered the entire transcript and scanned the DOM, falling back to the nearest rendered row when the hit was off screen — a silent wrong answer. It now resolves the target index from the data and widens the window to exactly what that target needs. - Tool-group previews JSON.parsed tool inputs during render, on every render, because the memo could never hit. They are computed once while grouping. - Two no-op cast wrappers defeated memo(ChatMessagesPane) and memo(ChatInterface). - The editor-divider drag committed a width per mousemove and rebuilt a ResizeObserver per frame; it is now coalesced to one commit per frame and the duplicate width copy is derived instead of stored.
… jump Adversarial review of the previous commit found real regressions. Fixed: - StreamingMarkdown rendered its two halves in two separate `.prose` containers, so Tailwind Typography's first/last-child margin rules zeroed the gap at the seam and it popped back when the boundary moved. Both halves now render inside one container via a new MarkdownBody export. - The block-boundary search split link-reference and footnote definitions away from their usage (rendering literal `[x]` text), treated ``` and ~~~ as interchangeable so a nested fence escaped its code block, ignored `$$` display math, and missed tab-indented code. All four produced output that differed from the unsplit document mid-stream. - The search jump accepted the nearest rendered row on the first attempt, so a window that had not committed yet scrolled to an arbitrary message and flashed the highlight on it — the exact failure the rewrite was meant to remove. It now requires an exact match until the final attempt, where the nearest row is what maps a hit inside a collapsed tool group onto its group. The retry budget is back to ~3s, since widening the window can commit thousands of rows. - EditorSidebar's old class string was malformed and applied nothing, so the pane was shrinkable; emitting a real `flex-shrink-0` changed narrow-window layout. Reverted to the previous behaviour. - fetchProjects now drops superseded responses. Several triggers refresh concurrently and the selection is derived from the payload, so an out-of-order response could revert the workspace header and document title. Removed code with no reachable path: the empty-list guard in findSearchTargetIndex, the dark-side undefined guard in buildSyntaxTheme, the orphaned `uuid` field on the search-target state, and the `normalizeSearchSnippet` export that only its own test used. Tests: the streaming split is now verified by rendering both halves and the whole document through the real remark/rehype pipeline over every prefix of 16 fixtures — reverting any of the four boundary fixes fails it. The settings regression is now driven through useSettingsController rather than the storage helpers, and fails if the mount effect is reintroduced. Two tests that passed with or without the code they named were replaced. Added RTL cleanup, the search window arithmetic as a tested function, and a CI workflow — nothing ran the client suite before.
UI preferences were not a store: every call site held its own useReducer and they reconciled after the fact through a `ui-preferences:sync` CustomEvent tagged with a per-instance id. One toggle produced four localStorage writes and four DOM events, and useVoiceAvailable re-implemented the storage key, the JSON parse and the listener pair — once per assistant message row. There is now one UiPreferencesProvider in src/shared/context/, split into state and actions contexts so writers do not re-render on a toggle. The reducer and the legacy per-key migration move to src/shared/uiPreferences.ts, which is pure and directly testable. The same-tab CustomEvent is gone; the `storage` listener stays, because it is the only thing that carries a change to another tab. The dead setPreferences/resetPreferences/dispatch surface is not carried over. Separately, the sidebar subscribed to the whole session activity map while only ever reading `.keys()` and `.has()`. Every provider status frame rewrites an entry's statusText and allocates a new map, so the entire project and session tree re-rendered several times a second during a run. SessionProtectionProvider now also publishes a busy-id set whose identity is stable while membership is unchanged, and the sidebar consumes that. Also collapses the sidebar's two mutually-exclusive delete confirmations into one pendingDeletion union — they are portalled at the same z-index and could both be open — and drops the projectId/provider fields from the session payload, which nothing read. .oxlintrc.json gains src/shared/uiPreferences.ts in the frontend-shared-file element. New files under src/shared/*.ts match boundaries/include but no element pattern, so without this `boundaries/no-unknown` fails the lint outright.
…nnects - `api.getMentionableFiles` built a byte-identical URL to `api.getFiles`. Two names for one endpoint meant the chat @-mention list and the file tree looked like separate resources; the mention list now calls getFiles. - TaskMasterContext fetched the whole `/api/projects` list on boot — a second copy of the request the workspace already makes — and again on every `taskmaster-project-updated` frame, with its own reimplementation of the taskmaster merge. The only thing any consumer reads is `currentProject`, and the per-project endpoint already answers that, so refreshProjects now calls refreshCurrentProjectTaskMaster. The unused `projects` array goes with it, and `isLoading` — whose only setter lived in the removed code — with that. - `websocket_reconnected` was handled only by chat. The project list is maintained purely by incremental `session_upserted` deltas, so a session created or renamed while the socket was down never appeared until something else forced a refresh. useProjectsState now re-syncs on that event. - The two byte-identical `readJson` envelope helpers in the browser-use files are now one `readApiJson` in shared/api.ts. The other same-named helpers are deliberately different (bare casts whose callers inspect the payload, the git panel's abort-aware read) and are left alone rather than unified into an options soup that would change their behaviour.
Each removal was verified by locating every reference, not by grepping a name —
several symbols on a naive "exported but not imported" list (authenticatedFetch,
exportToMarkdown) are load-bearing intra-file.
Components: SettingsMainTabs, AgentListItem and VersionInfoSection each carried
a doc comment stating they have no consumer and each had exactly one reference —
their own export.
Chat state: `uploadingFiles` was only ever set to an empty Map, so the upload
progress overlay it fed in ComposerAttachment could never render; the state, the
prop chain and the unreachable branch are gone. Chat attachment uploads still
have no progress indicator — wiring one needs a shared authenticated-XHR path,
which is a larger change than this cleanup. `viewHiddenCount` was provably always
0 because its only non-zero setter, rewindMessages, was returned but never
destructured; it goes with clearMessages, and with the store's clearRealtime,
which had no other caller. The store also drops has/getSlot/appendRealtimeBatch/
setStatus from its public surface, and useChatSessionState drops isNearBottom
from its return object while keeping the function, which three internal callers
use for streaming auto-scroll.
shared/ui: AlertTitle, AlertDescription, CardDescription, CardAction,
CommandSeparator, ConfirmationAccepted and ConfirmationRejected appear in no JSX
anywhere and are not in the barrel, so no module could reach them. Removing
AlertDescription also made an alertVariants selector targeting its data-slot dead.
api.ts: taskmaster.{init,addTask,parsePRD,getTemplates,applyTemplate},
auth.logout and the fetch-based uploadFiles have no caller. The server routes are
untouched; these were unreachable client wrappers. GenerateTasksModal, the
natural consumer for parsePRD, makes no API call at all.
chat/tools: ToolRenderer imported its siblings back from the barrel that exports
it — a real ESM cycle. It now imports them directly, the two nested barrels are
gone, and the tools barrel is trimmed to the three symbols that cross the
directory boundary. A module has one barrel, its own index.ts.
shell/utils/auth.ts held no auth code; its only export duplicated
shared/utils.ts getSessionTitle. The two differ in null handling, so the guard
moved to the single call site rather than widening the shared helper.
…the MCP form `selected-provider` was read in six places by four different hand-rolled readers — only one of which validated the stored string — and written from three modules with no same-tab notification. The git panel's reader listens only for the cross-tab `storage` event, so after switching provider it returned the old value for the rest of the session. src/shared/selectedProvider.ts now owns the key, validates on read, and publishes a same-tab change event that reader subscribes to alongside `storage`. useMcpServers held the open form as isFormOpen + isGlobalFormOpen + editingServer: eight representable combinations, three of them legal, including both dialogs open at once. It is now one `serverForm` union. Both modals were also mounted permanently, each running a full useMcpServerForm while closed; they are now mounted only while open, which makes the `isOpen` prop and the `if (!isOpen) return null` guard dead — both removed, along with the reset effect's now-constant condition. Also drops eleven hook return values with no consumer (setPermissionMode, refreshServers, setFormData, setProviderAuthStatus, clearSaveStatus, endDrag, starCount, setDropTarget, validateFilename, projectSortOrder and the four useCodeEditorSettings setters), keeping the internal functions the hooks themselves call.
The sidebar held a rename as two id/draft pairs (editingProject/editingName and editingSession/editingSessionName). That made "a project and a session are both mid-rename" representable, and the raw draft was passed to every project row and every session row — so each keystroke changed props on the whole list, not just the row being edited. There is now one `activeRename` union. The list resolves it per row into an `isEditing` boolean and a draft that is a constant for every other row, so a keystroke changes props on exactly one row. With that in place SidebarProjectItem and SidebarSessionItem are memoized, and the two remaining inline arrows in projectListProps are hoisted into useCallback so the memo can actually bail — the sidebar re-renders roughly every 0.5-2s during a run from websocket session deltas.
api.ts is meant to be the endpoint map plus the request helpers the standard requires to live beside it. It also held the entire client-side JWT lifecycle — claim decoding, skew tolerance, refresh scheduling, storage, and the two events AuthContext and WebSocketContext listen to — none of which is an endpoint. Move that to src/shared/authToken.ts and repoint its five importers. Nothing is re-exported through api.ts: the auth surface is security-sensitive and worth naming at the import site. Fix the layer inversion at the top of api.ts while here. readVoiceConfig and voiceConfigHeaders are plain localStorage readers with no React in them, but they lived in hooks/useVoiceConfig, so api.ts — the bottom of the dependency graph — imported upwards from the hooks layer to attach request headers. They move to src/shared/voiceConfig.ts; the hook keeps only the hook. Both new files are registered in .oxlintrc.json's frontend-shared-file element. boundaries/no-unknown is an error, so a new src/shared/*.ts that is not listed fails the lint outright rather than warning. api.test.ts becomes authToken.test.ts and gains coverage for the parts that were previously untested: storage rejects a non-token without clobbering an existing session, an expired read clears the session and announces it exactly once, and the refresh delay lands at the halfway point and never goes negative. Each assertion was mutation-checked against the code it pins.
src/shared/ui is for components two or more feature modules use. PromptInput, Reasoning and Queue never had a second consumer: every one of their call sites is inside src/modules/chat. They are not primitives either — they are the chat composer form, the assistant-reasoning disclosure and the tool todo list, with chat vocabulary in their prop names. shared/ui/PromptInput.tsx -> modules/chat/composer/PromptInput.tsx shared/ui/Reasoning.tsx -> modules/chat/transcript/Reasoning.tsx shared/ui/Queue.tsx -> modules/chat/tools/Queue.tsx Collapsible, Card, Shimmer, Confirmation and Alert stay in shared/ui. The two-module rule is a criterion for admission, not for eviction, and those are genuine primitives with cross-module reach. Three things fall out of the move, all of them dead weight the shared barrel was keeping alive: - usePromptInput and useReasoning were exported but nothing outside their own file called them. They are module-private now, which also clears the two react(only-export-components) warnings those files carried. - PromptInputSubmit had a `status` prop that no call site ever passed, and fell back to `context?.status ?? 'ready'` when the context was missing. It is only ever rendered inside PromptInput, so the prop and the null-context branch were both unreachable — and that default would have rendered a send button for a composer that was actually streaming. It now reads the root's status directly. - The "use client" directives were copied in from ai-elements. Vite is not Next.js; these two files were the only ones in src/ carrying them. Consumer comments now name the component that actually renders each export rather than "the chat module", which stopped saying anything once the files moved into it. promptInputSubmit.test.tsx pins the status wiring: submit when ready, a stop control when streaming, and a hard failure outside PromptInput instead of the silent "ready" default. Each assertion was mutation-checked.
src/modules/chat had 26 component files in one directory with no signal about
what belonged to what. The import graph is in fact almost a tree, so the grouping
is read off it rather than invented:
composer/ everything ChatComposer renders — the menus, the attachment row,
the queued-draft card, the permission banner, the voice button,
the activity indicator, the token summary
transcript/ the message list and everything it renders — MessageComponent and
its controls, the markdown renderers, the tool group container,
the export menu, the load-all overlay, the empty state
modals/ CommandResultModal and the ModelLibraryPanel it opens
ChatInterface.tsx and index.ts stay at the root, which is now exactly the
module's entry component and its barrel.
Two placements differ from the obvious reading of the file names:
- PermissionRequestsBanner and ProviderSelectionEmptyState sound like top-level
chrome, but their only consumers are ChatComposer and ChatMessagesPane
respectively, so they sit with the component that renders them.
- ChatMessageImages is used by both MessageComponent and ComposerAttachment. It
renders a message's images and is named for that, so it lives in transcript/
and the composer imports across.
Only paths change. No file gains or loses an export, and no component's
behaviour is touched — the diff outside the renames is entirely import strings.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const texts: string[] = typeof content === 'string' | ||
| ? [content] | ||
| : Array.isArray(content) | ||
| ? content.filter((part: AnyRecord) => part?.type === 'text').map((part: AnyRecord) => String(part.text ?? '')) |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
|
|
||
| if (payload.type === 'reasoning') { | ||
| const summary = Array.isArray(payload.summary) | ||
| ? payload.summary.map((item: AnyRecord) => item?.text).filter(Boolean).join('\n') |
There was a problem hiding this comment.
React Doctor · react-doctor/js-flatmap-filter (warning)
This loops over your list twice because .map().filter(Boolean) makes two passes, so use .flatMap() to change & drop items in one pass
Fix → Use .flatMap(item => condition ? [value] : []) to change and drop items in one pass, instead of building a throwaway array in between
|
|
||
| if (payload.type === 'reasoning') { | ||
| const summaryText = Array.isArray(payload.summary) | ||
| ? payload.summary.map((item: AnyRecord) => item?.text).filter(Boolean).join('\n') |
There was a problem hiding this comment.
React Doctor · react-doctor/js-flatmap-filter (warning)
This loops over your list twice because .map().filter(Boolean) makes two passes, so use .flatMap() to change & drop items in one pass
Fix → Use .flatMap(item => condition ? [value] : []) to change and drop items in one pass, instead of building a throwaway array in between
| } | ||
|
|
||
| for (const configFile of OPENCODE_CONFIG_FILES) { | ||
| const config = await readOpenCodeJsonFile(path.join(configDir, configFile)); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for…of loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
| if (entries.size <= 1 || (totalBytes <= maxTotalFileBytes && entries.size <= maxEntries)) { | ||
| break; | ||
| } | ||
| totalBytes -= entries.get(key)!.size; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-non-null-assertion-on-maybe-undefined-result (warning)
.get(...) returns undefined when the key is absent, so asserting ! here crashes on the next access when the key misses; check for the key or handle the missing value.
Fix → Drop the ! on .find/.match/.get results and handle the miss (optional chaining, a guard, or a fallback). These built-ins return undefined/null when nothing matches, so the assertion just moves the crash one line later.
| // store, echoing each hydrate straight back. Comparing by content, not by | ||
| // reference, keeps the write one-directional: the reducer returns a fresh | ||
| // object for an incoming change, so an identity check would not catch it. | ||
| const lastPersistedRef = useRef<string>(JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-lazy-ref-init (warning)
useRef(stringify()) rebuilds this value on every render & throws it away.
Fix → Initialize the ref lazily so expensive values are not rebuilt and discarded on every render.
| @@ -71,6 +42,8 @@ export function useSessionProtection() { | |||
| const [processingSessions, setProcessingSessions] = useState<Map<string, SessionActivity>>( | |||
| new Map(), | |||
| ); | |||
| const processingSessionsRef = useRef<SessionActivityMap>(processingSessions); | |||
| processingSessionsRef.current = processingSessions; | |||
There was a problem hiding this comment.
React Doctor · react-doctor/no-ref-current-in-render (error)
This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Fix → Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
|
|
||
| const badgeVariants = cva( | ||
| export const badgeVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| // Keep visual variants centralized so all button usages stay consistent. | ||
| const buttonVariants = cva( | ||
| export const buttonVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| function writeMirror(): void { | ||
| try { | ||
| localStorage.setItem(MIRROR_STORAGE_KEY, JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/client-localstorage-no-version (warning)
localStorage.setItem("user-preferences", JSON.stringify(...)) has no version, so changing the data shape later crashes your users' saved sessions. Add one to the key (e.g. "user-preferences:v1").
Fix → Put a version in the storage key (e.g. "myKey:v1"). If you change the data shape later, old saved data can be ignored instead of crashing the app.
|
|
||
| return ( | ||
| <I18nextProvider i18n={i18n}> | ||
| <TranscriptRenderContext.Provider value={{ isExporting: true }}> |
There was a problem hiding this comment.
React Doctor · react-doctor/jsx-no-constructed-context-values (warning)
Every reader of this context redraws on each render because you build its value inline.
Fix → Wrap the context value in useMemo or move it outside the component so consumers do not redraw every render.
| } | ||
| searchScrollActiveRef.current = false; | ||
| setSearchTarget(null); |
There was a problem hiding this comment.
React Doctor · react-doctor/no-adjust-state-on-prop-change (warning)
This effect adjusts state after a prop changes, so users briefly see the stale value.
Fix → Remove the adjustment effect by deriving values during render, resetting the component with a key, or updating related state in the event that changes the prop. Avoid tracking the previous prop in more state, which preserves the duplication. See https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
| const phrase = normalizeSearchSnippet(target.snippet); | ||
| if (phrase.length >= MIN_SNIPPET_LENGTH) { | ||
| const matchIndex = messages.findIndex((message) => | ||
| getSearchableText(message).includes(phrase), |
There was a problem hiding this comment.
React Doctor · react-doctor/js-set-map-lookups (warning)
This scales poorly because array.includes() inside a loop scans the whole list every time. Use a Set for constant-time lookups.
Fix → Use a Set or Map when you check for the same items over and over. Array.includes/find scans the whole list each time
| // Keep search results visible by opening every matching ancestor directory once per query update. | ||
| expandDirectories(collectExpandedDirectoryPaths(filtered)); | ||
| }, [files, searchQuery, expandDirectories]); | ||
| expandDirectories(collectExpandedDirectoryPaths(filteredFiles)); |
There was a problem hiding this comment.
React Doctor · react-doctor/no-pass-data-to-parent (warning)
Handing data back to a parent from a useEffect costs your users an extra render.
Fix → Fetch the data in the parent and pass it down as a prop (or return it from the hook), instead of handing it back up through a prop callback in a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#passing-data-to-the-parent
| // Keep search results visible by opening every matching ancestor directory once per query update. | ||
| expandDirectories(collectExpandedDirectoryPaths(filtered)); | ||
| }, [files, searchQuery, expandDirectories]); | ||
| expandDirectories(collectExpandedDirectoryPaths(filteredFiles)); |
There was a problem hiding this comment.
React Doctor · react-doctor/no-pass-live-state-to-parent (warning)
Pushing state up to a parent from a useEffect costs your users an extra render.
Fix → Move the state up to the parent (or return it from the hook), instead of handing it back up through a prop callback in a useEffect. See https://react.dev/learn/you-might-not-need-an-effect#notifying-parent-components-about-state-changes
|
|
||
| function writeMirror(): void { | ||
| try { | ||
| localStorage.setItem(MIRROR_STORAGE_KEY, JSON.stringify(Object.fromEntries(drafts))); |
There was a problem hiding this comment.
React Doctor · react-doctor/client-localstorage-no-version (warning)
localStorage.setItem("chat-drafts", JSON.stringify(...)) has no version, so changing the data shape later crashes your users' saved sessions. Add one to the key (e.g. "chat-drafts:v1").
Fix → Put a version in the storage key (e.g. "myKey:v1"). If you change the data shape later, old saved data can be ignored instead of crashing the app.
| // store, echoing each hydrate straight back. Comparing by content, not by | ||
| // reference, keeps the write one-directional: the reducer returns a fresh | ||
| // object for an incoming change, so an identity check would not catch it. | ||
| const lastPersistedRef = useRef<string>(JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-lazy-ref-init (warning)
useRef(stringify()) rebuilds this value on every render & throws it away.
Fix → Initialize the ref lazily so expensive values are not rebuilt and discarded on every render.
|
|
||
| const badgeVariants = cva( | ||
| export const badgeVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| // Keep visual variants centralized so all button usages stay consistent. | ||
| const buttonVariants = cva( | ||
| export const buttonVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| function writeMirror(): void { | ||
| try { | ||
| localStorage.setItem(MIRROR_STORAGE_KEY, JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/client-localstorage-no-version (warning)
localStorage.setItem("user-preferences", JSON.stringify(...)) has no version, so changing the data shape later crashes your users' saved sessions. Add one to the key (e.g. "user-preferences:v1").
Fix → Put a version in the storage key (e.g. "myKey:v1"). If you change the data shape later, old saved data can be ignored instead of crashing the app.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
7b078bd to
d3cb06a
Compare
| if (entries.size <= 1 || (totalBytes <= maxTotalFileBytes && entries.size <= maxEntries)) { | ||
| break; | ||
| } | ||
| totalBytes -= entries.get(key)!.size; |
There was a problem hiding this comment.
React Doctor · react-doctor/no-non-null-assertion-on-maybe-undefined-result (warning)
.get(...) returns undefined when the key is absent, so asserting ! here crashes on the next access when the key misses; check for the key or handle the missing value.
Fix → Drop the ! on .find/.match/.get results and handle the miss (optional chaining, a guard, or a fallback). These built-ins return undefined/null when nothing matches, so the assertion just moves the crash one line later.
| // Sequentially: a session can only have one run at a time, and two due | ||
| // messages for the same session must not race each other into it. | ||
| for (const row of due) { | ||
| await sendClaimedMessage(row, runtime); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for…of loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
| @@ -90,7 +92,7 @@ export default function LoginForm() { | |||
| <button | |||
There was a problem hiding this comment.
React Doctor · react-doctor/no-transition-all (warning)
Your users see janky animation because transition-all animates every property that changes, including expensive layout ones and instant ones like focus rings. Name the properties: transition-colors, transition-opacity, or transition-transform.
Fix → List the specific properties: transition: "opacity 200ms, transform 200ms". In Tailwind, use transition-colors, transition-opacity, or transition-transform
| @@ -136,7 +138,7 @@ export default function SetupForm() { | |||
| <button | |||
There was a problem hiding this comment.
React Doctor · react-doctor/no-transition-all (warning)
Your users see janky animation because transition-all animates every property that changes, including expensive layout ones and instant ones like focus rings. Name the properties: transition-colors, transition-opacity, or transition-transform.
Fix → List the specific properties: transition: "opacity 200ms, transform 200ms". In Tailwind, use transition-colors, transition-opacity, or transition-transform
| const response = await authenticatedFetch(`/api/browser-use/sessions/${selectedSession.id}/stop`, { method: 'POST' }); | ||
| await readJson(response); | ||
| const response = await api.browserUse.stopSession(selectedSession.id); | ||
| await readApiJson(response); | ||
| }); | ||
|
|
||
| const deleteSession = () => runAction(async () => { |
There was a problem hiding this comment.
React Doctor · react-doctor/no-impure-state-updater (error)
This state updater performs the nested state update "setIsFullscreen()". React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.
Fix → Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.
|
|
||
| function writeMirror(): void { | ||
| try { | ||
| localStorage.setItem(MIRROR_STORAGE_KEY, JSON.stringify(Object.fromEntries(drafts))); |
There was a problem hiding this comment.
React Doctor · react-doctor/client-localstorage-no-version (warning)
localStorage.setItem("chat-drafts", JSON.stringify(...)) has no version, so changing the data shape later crashes your users' saved sessions. Add one to the key (e.g. "chat-drafts:v1").
Fix → Put a version in the storage key (e.g. "myKey:v1"). If you change the data shape later, old saved data can be ignored instead of crashing the app.
| // store, echoing each hydrate straight back. Comparing by content, not by | ||
| // reference, keeps the write one-directional: the reducer returns a fresh | ||
| // object for an incoming change, so an identity check would not catch it. | ||
| const lastPersistedRef = useRef<string>(JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/rerender-lazy-ref-init (warning)
useRef(stringify()) rebuilds this value on every render & throws it away.
Fix → Initialize the ref lazily so expensive values are not rebuilt and discarded on every render.
|
|
||
| const badgeVariants = cva( | ||
| export const badgeVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| // Keep visual variants centralized so all button usages stay consistent. | ||
| const buttonVariants = cva( | ||
| export const buttonVariants = cva( |
There was a problem hiding this comment.
React Doctor · react-doctor/only-export-components (warning)
This file exports non-components, so Fast Refresh can't safely preserve component state.
Fix → Move non-component exports out of component files so Fast Refresh can preserve component state instead of full-reloading.
|
|
||
| function writeMirror(): void { | ||
| try { | ||
| localStorage.setItem(MIRROR_STORAGE_KEY, JSON.stringify(preferences)); |
There was a problem hiding this comment.
React Doctor · react-doctor/client-localstorage-no-version (warning)
localStorage.setItem("user-preferences", JSON.stringify(...)) has no version, so changing the data shape later crashes your users' saved sessions. Add one to the key (e.g. "user-preferences:v1").
Fix → Put a version in the storage key (e.g. "myKey:v1"). If you change the data shape later, old saved data can be ignored instead of crashing the app.
Drops the CI workflow, the react-doctor agent skill, and the doctor script/devDependency.
The composer's token counter is driven by token_budget status events
emitted per SDK stream message. Two message classes corrupted it during
generation:
- System task_progress/task_notification events carry a top-level usage
shaped {total_tokens, tool_uses, duration_ms}; reading Anthropic keys
off it produced an all-zero budget that flashed "0" until the next
assistant message restored the real number.
- Subagent messages (parent_tool_use_id set) carry the subagent's own,
smaller context usage, making the counter drop and bounce back.
extractTokenBudget now only reads assistant/result messages from the
main thread. The client additionally ignores token_budget events not
stamped with the viewed session, so a second running session can no
longer overwrite the visible counter.
Codex reports API failures twice: the stream emits an error item or turn.failed, then the SDK throws "Codex Exec exited with code N: <stderr>" when the process dies. The catch block sent that raw stderr dump as a second error message, so the rendered error card was followed by unrelated CLI log lines. Skip the thrown wrapper when the stream already surfaced the failure; startup failures still report as before.
This reverts commit 50cdb74.
This reverts commit 5ab3171.
The Agent SDK emits a subagent's own prompt as a user message carrying parent_tool_use_id. The runtime forwarded it like any other message, so it rendered as a top-level user bubble right below the Agent tool card that already shows that prompt — and it vanished on reload, because Claude keeps that turn in the subagent's sidechain rather than the session transcript. Skip those echoes in the stream loop. Subagent tool traffic and the real session prompt are unaffected.
…ith hard reload Give the environment enough time to actually restart before reloading, and bypass the cache on reload so stale assets aren't served post-update.
The Claude CLI only offers bypass-permissions in its shift+tab mode cycle when launched with --dangerously-skip-permissions, so shell-tab sessions could never enable it. Add a Bypass toggle to the shell header (Claude provider only, seeded from the chat composer's skip-permissions setting) that relaunches the CLI with the flag, for both fresh and resumed sessions.
…indows The Agent SDK launches pathToClaudeCodeExecutable with a raw child_process.spawn, which never consults PATH or PATHEXT. The resolver's fallback was the bare string `claude`, so whenever resolution failed the SDK reported "Claude Code native binary not found at claude" on a machine where the CLI is installed and on PATH. That fallback is reachable whenever the only Claude Code visible on PATH is an older npm install shipping cli.js instead of bin/claude.exe. Return undefined instead and leave the option unset, so the SDK falls back to the native binary it ships; an explicitly configured CLAUDE_CLI_PATH is still honoured verbatim, and the auth probe keeps the bare command since cross-spawn resolves shims. Claude-Session: https://claude.ai/code/session_01MeP3mSuvuPQ1wxHC3r1rnz
The composer's counter shows context-window occupancy, but the runtime also fed it the turn-ending `result` message. `result.usage` is the turn's bill — every API request it made, summed — so a multi-request turn reported several times the context the conversation actually holds, and the next assistant message dropped it back. A turn that spawns a subagent absorbs that agent's requests into the same sum, which is why the bouncing only showed up once subagents were running. Assistant messages are now the only source for the counter; the cumulative reading survives as `extractCumulativeTokenBudget`, used only when a run produced no per-assistant usage at all. The transcript-derived writer of the same counter had two matching defects: `<synthetic>` rows (interrupts, API errors, "No response requested.") carry an all-zero usage block rather than none, so one landing last zeroed the counter on the next history refresh; and sidechain rows reported a subagent's context. Both are now skipped.
…live A running subagent's rows stream in stamped with the spawning Task's tool id (parentToolUseId), but the client projection ignored the stamp and rendered them as the session's own top-level tool calls. They only moved inside the subagent panel after a refresh, when the server ships the sidecar-indexed timeline as subagentTools on the Task row. normalizedToChatMessages now folds parented rows into that row's SubagentActivity timeline as they arrive — tool calls with results attached, assistant prose and thinking, the echoed task prompt excluded to match the server's reader. The projection cache tracks the newest folded row per container so the panel keeps growing, and when a mid-run history refresh attaches a partial server timeline the longer of the two wins.
…fresh Answering a permission prompt resolves it over the inbound socket only (chat.permission-response -> resolveToolApproval), while the original permission_request frame sits in the run's replay buffer. A mid-run page refresh subscribes from lastSeq 0 and replays the whole buffer, so the request re-arrived with nothing to retract it and the already-answered prompt resurrected — and a second tab watching the same run kept it forever, since only cancellations ever emitted an outbound frame. The Claude runtime now emits permission_resolved on the run stream when a client answers, so it lands in the replay buffer and on every attached socket. The client removes the prompt's requestId on that frame, keeps it out of the message store like the other permission kinds, and skips the unread-attention mark for resolutions in background sessions.
Two concurrently authored variations document the same six subsystems — websocket transport, conversation handoff, the realtime stream, scrolling, lazy loading, and tool views. They are kept side by side so the better one can be chosen or the two merged; the top-level README compares them and records which set was fact-checked against source. The single-file realtime map and the virtualized-lists assessment they supersede are removed. Both sets already describe the permission_resolved frame from the previous commit, with the shifted line citations corrected.
The variation-2 set was not fact-checked against the source, so the side-by-side comparison ends here: variation-1 — whose claims were re-verified in an adversarial pass — is promoted to docs/architecture/ as the single chat-runtime documentation, and the comparison README is replaced by the set's own index. Internal cross-links are relative and survive the move unchanged.
6325322 to
7f09933
Compare
2 participants
Closes #1214
Closes #1209
Closes #1208
Closes #1203
Closes #1201
Closes #1186
Closes #1182
Closes #1100
Closes #1010
Closes #922
Closes #813
Closes #554
Closes #409
Summary
This PR brings Claude and Codex conversations onto a shared chat model, expands conversation workflows across both providers, and substantially improves performance for large transcripts and project lists.
What changed
Unified provider experience
Conversation workflows
Persistence and synchronization
auth.db.Performance
Reliability fixes
Database changes
Adds migrations and repositories for:
Existing sessions and preferences are preserved through the migration path.
Testing
Added focused unit and integration coverage for:
Large real-world transcripts and live provider flows were also used to validate the main performance and conversation workflows.