fix(editor): resolve UX inconsistencies in the AI chat interface - #14850
Merged
darkskygit merged 13 commits intoMay 6, 2026
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds AI chat tabbed multi-session UI with per-workspace persistence, drag-and-drop attachments (50MB limit) backed by new attachment utilities, registers ChangesAI Chat — Tabs, Drag & Drop, Attachments
Tooltip theming
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatInput
participant AttachmentUtils
participant ImageHandler
participant ChipHandler
participant Toast
User->>ChatInput: Drag files over input
activate ChatInput
ChatInput->>ChatInput: Show drag-over overlay
deactivate ChatInput
User->>ChatInput: Drop files
activate ChatInput
ChatInput->>AttachmentUtils: addFilesToChat(files, handlers)
activate AttachmentUtils
AttachmentUtils->>AttachmentUtils: Split images / non-images
par Image Processing
AttachmentUtils->>ImageHandler: addImages(imageFiles)
activate ImageHandler
ImageHandler-->>AttachmentUtils: images handled
deactivate ImageHandler
and Non-Image Processing
AttachmentUtils->>AttachmentUtils: Validate sizes
alt size < 50MB
AttachmentUtils->>ChipHandler: addChip({file, state:'processing'})
activate ChipHandler
ChipHandler-->>AttachmentUtils: chip added
deactivate ChipHandler
else size >= 50MB
AttachmentUtils->>Toast: show oversize error
end
end
deactivate AttachmentUtils
ChatInput->>ChatInput: Emit analytics (addEmbeddingDoc control='dragDrop')
deactivate ChatInput
sequenceDiagram
participant User
participant ChatUI
participant TabComponent
participant SessionManager
participant LocalStorage
rect rgba(100, 150, 200, 0.5)
ChatUI->>LocalStorage: Read stored session IDs
LocalStorage-->>ChatUI: sessionIds[]
ChatUI->>SessionManager: Fetch session fragments
SessionManager-->>ChatUI: Sessions loaded
ChatUI->>TabComponent: Render tabs
end
rect rgba(150, 100, 200, 0.5)
User->>TabComponent: Click tab / Close tab
TabComponent->>SessionManager: onSelectTab / onCloseTab
SessionManager->>ChatUI: Update active session / openTabs
ChatUI->>LocalStorage: Persist openTabs
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
blocksuite/affine/components/src/tooltip/tooltip.ts (1)
27-44:⚠️ Potential issue | 🟡 MinorUse the
cssVarV2helper function like other tooltip implementations in the codebase.The v2 tokens
--affine-v2-tooltips-backgroundand--affine-v2-tooltips-foregroundexist and are properly registered. However, this file uses raw CSS variable strings while other tooltip components (packages/frontend/component/src/ui/tooltip/styles.css.tsandblocksuite/affine/blocks/table/src/add-button-css.ts) use thecssVarV2helper function. Replace the raw strings withcssVarV2('tooltips/background')andcssVarV2('tooltips/foreground')for consistency and maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blocksuite/affine/components/src/tooltip/tooltip.ts` around lines 27 - 44, Replace raw CSS variable strings for tooltip colors with the cssVarV2 helper for consistency: update occurrences of 'var(--affine-v2-tooltips-background, var(--affine-tooltip))' and 'var(--affine-v2-tooltips-foreground, var(--affine-white))' in tooltip.ts to use cssVarV2('tooltips/background') and cssVarV2('tooltips/foreground') respectively, and set TOOLTIP_ARROW_COLOR to the cssVarV2('tooltips/background') value so the file matches other tooltip implementations (look for the styled template and the TOOLTIP_ARROW_COLOR constant to locate the replacements).packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx (1)
278-289:⚠️ Potential issue | 🟠 MajorRemove deleted sessions from
openTabs.Deleting a chat history entry does not remove the corresponding tab, so the tab strip can keep showing a server-deleted session and persist that stale ID.
Suggested fix
cleanupSession: async sessionToDelete => { await AIProvider.histories?.cleanup( sessionToDelete.workspaceId, sessionToDelete.docId || undefined, [sessionToDelete.sessionId] ); + setOpenTabs(prev => + prev.filter(tab => tab.sessionId !== sessionToDelete.sessionId) + ); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx` around lines 278 - 289, The delete handler currently cleans server histories but doesn't remove the corresponding tab from the local openTabs state, leaving a stale tab id; update the logic so that when a session is deleted (in cleanupSession and/or onActiveSessionDeleted) you also remove that sessionId from the openTabs array and update the active session if needed: locate cleanupSession, isActiveSession, onActiveSessionDeleted, openTabs, session and newSession and after calling AIProvider.histories.cleanup remove any openTabs entries matching sessionToDelete.sessionId (and if the deleted session was active, call newSession or set a fallback session) so the tab strip no longer shows server-deleted sessions.
🧹 Nitpick comments (8)
packages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsx (1)
25-26: Use a theme token for the filled sidebar segment.Line 25 hard-codes
#1E96EB, so the open-state indicator will not follow custom themes or future accent-color changes. Prefer the project’s semantic primary/accent CSS token with a fallback.♻️ Proposed direction
- fill="#1E96EB" + fill="var(--affine-primary-color, `#1E96EB`)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsx` around lines 25 - 26, Replace the hard-coded fill="#1E96EB" on the SVG path (the element with d="M15.25 6h3.25a.5.5 0 0 1 .5.5v11a.5.5 0 0 1-.5.5h-3.25z") with the project’s semantic accent/primary CSS token and a hex fallback (e.g., use a CSS var like var(--accent-primary, `#1E96EB`) or the established token name in our theme) so the open-state indicator follows custom themes and accent-color changes.blocksuite/affine/components/src/tooltip/tooltip.ts (1)
43-71: Optional: arrow won't live-update on theme change.
TOOLTIP_ARROW_COLORis embedded via template-string interpolation into theborderColorshorthand that is then applied throughstyleMapas an inline style. Since it's a literalvar(...)string, it will still re-resolve when CSS variables change, so this is fine functionally — just noting that the tooltip body uses a stylesheet rule (which is the idiomatic path) while the arrow uses inline style. Consider moving the triangle colors into the stylesheet (e.g., per-placement arrow classes) for consistency and to avoid theunsafeCSS-style string concat pattern. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@blocksuite/affine/components/src/tooltip/tooltip.ts` around lines 43 - 71, The tooltip arrow currently inlines borderColor via the TOOLTIP_ARROW_COLOR template string inside triangleMap (symbols: TOOLTIP_ARROW_COLOR, TRIANGLE_HEIGHT, triangleMap) which mixes inline styles with stylesheet usage and can prevent consistent theme updates; instead move the arrow triangle CSS into the component stylesheet by creating per-placement classes (e.g., .tooltip-arrow--top / --right / --bottom / --left) that use the same CSS variable (var(--affine-v2-tooltips-background, var(--affine-tooltip))) for border-color, then update the render logic in tooltip.ts to assign the appropriate placement class to the arrow element rather than setting borderColor via styleMap from triangleMap and remove the template-string concatenation with TOOLTIP_ARROW_COLOR.packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts (1)
498-531: Nit:combine(dropTargetCleanup)is a no‑op wrapper here.
combineis designed to merge multiple cleanup functions. With a single source it just adds an indirection. You can either assign the cleanup directly, or plan to drop additional sources (monitor, external drop, etc.) through the same helper.♻️ Proposed simplification
- const dropTargetCleanup = dropTargetForElements({ + this._internalDropCleanup = dropTargetForElements({ element: el, ... }); - this._internalDropCleanup = combine(dropTargetCleanup);And drop the
combineimport if nothing else uses it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts` around lines 498 - 531, In _setupInternalDropTarget(), don't wrap the single cleanup function with combine; assign dropTargetCleanup directly to this._internalDropCleanup and remove the unused combine import (or keep combine only if you plan to merge additional cleanup sources later); update references to dropTargetCleanup and ensure combine is deleted from imports when no longer used.packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts (1)
5-5: Minor:MAX_ATTACHMENT_SIZEisn't exported, and the user‑facing size is duplicated as a magic string.The PR summary / barrel
index.tsimplyMAX_ATTACHMENT_SIZEis part of the public API ofai-chat-chips, but it's declared withoutexportsoexport * from './attachment-utils'only re‑exports the type and function. Also, the"less than 50MB"literal will drift if the constant ever changes.♻️ Proposed tweak
-const MAX_ATTACHMENT_SIZE = 50 * 1024 * 1024; +export const MAX_ATTACHMENT_SIZE = 50 * 1024 * 1024; +const MAX_ATTACHMENT_SIZE_MB = MAX_ATTACHMENT_SIZE / (1024 * 1024); @@ - if (file.size > MAX_ATTACHMENT_SIZE) { - toast(`${file.name} is too large, please upload a file less than 50MB`); - return; - } + if (file.size > MAX_ATTACHMENT_SIZE) { + toast( + `${file.name} is too large, please upload a file less than ${MAX_ATTACHMENT_SIZE_MB}MB` + ); + return; + }Also applies to: 33-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts` at line 5, The constant MAX_ATTACHMENT_SIZE is declared but not exported and the user-facing message uses a duplicated "less than 50MB" literal that will drift if the constant changes; export MAX_ATTACHMENT_SIZE from attachment-utils (add export to the declaration) and update any user-facing string generation (the code that currently emits the "less than 50MB" text around lines 33-34) to compute the human-readable size from MAX_ATTACHMENT_SIZE (e.g., derive MB from bytes and format it) so the public API matches the barrel export and the displayed size always reflects the constant.packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts (2)
24-24: Nit: redundant nullish fallback.
String.prototype.splitalways returns an array with at least one element, soraw.split('\n')[0] ?? rawis equivalent toraw.split('\n')[0].Proposed change
- const firstLine = raw.split('\n')[0] ?? raw; + const firstLine = raw.split('\n', 1)[0]; return truncate(firstLine);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts` at line 24, The expression assigning firstLine in ai-chat-tabs.ts uses a redundant nullish fallback; replace the line that sets the constant firstLine (currently using raw.split('\n')[0] ?? raw) with a simpler raw.split('\n')[0] so the code relies on split's guaranteed single-element array and removes the unnecessary "?? raw" fallback.
192-207: Consider guarding the initialscrollIntoViewcall.On first mount
updated()fires for every@propertychange, soscrollIntoView({ behavior: 'smooth' })runs even when the tab is already fully visible. With many tabs hydrated at once this can trigger a noticeable smooth-scroll animation on initial load and (becausescrollIntoViewwalks up scroll ancestors) may momentarily scroll the surrounding page beforeinline: 'nearest'short-circuits. Gating by "only when the active tab is actually out of view" (getBoundingClientRectvs. its scroll parent) avoids the jitter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts` around lines 192 - 207, The current updated() handler always calls scrollIntoView for the element found by renderRoot.querySelector(`[data-session-id="${this.activeSessionId}"]`), causing unwanted smooth-scroll on initial mount; change updated() in the ai-chat-tabs component to first determine whether that activeTab is actually outside its scrollable container (compute activeTab.getBoundingClientRect() and compare to the scroll parent’s client bounding rect — locate the scroll parent by walking up via closest/scrollable overflow or fall back to document/viewport) and only call activeTab.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'nearest' }) if it is out of view. Ensure you reference activeSessionId and renderRoot as in the diff and avoid calling scrollIntoView when the element is already fully visible.packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx (2)
502-518: Minor: tab strip effect has no teardown.If this component ever unmounts without the whole page tearing down (e.g. hot reload, route change within the same workspace), the
AIChatTabselement is left attached to a detached container and theonSelectTab/onCloseTabclosures keep pointing at stalecloseTab/onOpenSessionfrom the unmounted render. Matches the existingchatToolpattern, so not a new regression — flagging for future cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 502 - 518, The effect that creates and attaches the AIChatTabs element leaves it mounted and its callbacks referencing stale closures when the component unmounts; update the useEffect that uses chatTabsContainerRef/AIChatTabs to return a cleanup function that removes the tabs element from chatTabsContainerRef (or calls a destructor if AIChatTabs exposes one), nulls out tabs.onSelectTab and tabs.onCloseTab, and calls setChatTabs(null) so handlers don't retain stale closeTab/onOpenSession references; ensure you reference the same AIChatTabs instance created in the effect and clear it on cleanup.
423-468: Hydration race: an active session created before hydration resolves will be pushed ahead of restored tabs.The "sync openTabs with active session" effect (lines 471–485) runs immediately on first render as soon as
currentSessionis set (e.g. viarestore pinned session), appending that session toopenTabs. The hydration promise fromgetSessionresolves later and then appends any previously-persisted IDs after it, so the restored tabs end up to the right of the session created this page load — opposite of the order the user left them in.Consider prepending hydrated entries (or seeding
openTabssynchronously from the stored IDs before resolving sessions) so the restored tab order is preserved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 423 - 468, The hydration effect for open tabs (the useEffect that reads `ai-chat-open-tabs:${workspaceId}` and calls `client.getSession`) can race with the other effect that syncs `currentSession` into `openTabs`, causing a newly-created session to appear before restored tabs; fix by ensuring hydrated entries are prepended or by seeding `openTabs` synchronously from the stored IDs before awaiting `client.getSession`: when building `hydrated` results, insert them before existing tabs (use `setOpenTabs(prev => { const seen = new Set(prev.map(t=>t.sessionId)); const merged = [...hydrated.filter(e=>!seen.has(e.sessionId)), ...prev]; return merged; })`) or initially map stored IDs to placeholder entries (or sessionId-only objects) and set that as initial `openTabs` so the later-resolved sessions replace placeholders in-place rather than appending.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts`:
- Around line 513-531: The onDrop handler is hardcoding
track.$.chatPanel.chatPanelInput.addEmbeddingDoc which ignores independentMode
and misattributes standalone /chat events; update the tracking call in onDrop
(and the native file-drop handler around the other block) to branch on
independentMode like add-popover.ts's _track does and route to
track.$.intelligence when independentMode is true, passing the same payload
(control: 'dragDrop' / method: 'doc' or 'file') and preserving existing
arguments; also add the required import type { EventArgs } from '@affine/track'
if it's not present.
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts`:
- Around line 18-26: In deriveTabTitle, remove the redundant null-coalescing
fallback by replacing the expression that sets firstLine (currently
`raw.split('\n')[0] ?? raw`) with a direct extraction (`raw.split('\n')[0]`)
since raw is already validated non-empty; update the code that returns
truncate(firstLine) accordingly so the function uses the simplified firstLine
extraction while preserving DEFAULT_TAB_TITLE and truncate usage.
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 488-499: The effect currently exits early when openTabs is empty,
leaving stale IDs in localStorage; modify the useEffect that watches [openTabs,
workspaceId] so it always writes to localStorage (compute storageKey from
workspaceId and call localStorage.setItem) even when openTabs.length === 0 —
persist an empty array (JSON.stringify([])) to clear previous IDs; keep the
try/catch around localStorage.setItem and still guard only on missing
workspaceId, not on empty openTabs, referencing the existing useEffect,
storageKey, openTabs, workspaceId, and localStorage.setItem symbols.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 373-450: The effect that reads/writes
`ai-chat-open-tabs:${workspaceId}` currently doesn't fully scope to the active
workspace and avoids writing empty arrays, causing stale tabs to leak between
workspaces and preventing clearing localStorage; update the three useEffect
blocks (the initializer that reads localStorage, the one that adds/updates a
session via `setOpenTabs`, and the persister that writes localStorage) to always
use the current `doc.workspace.id` storageKey, ensure the initializer
filters/loads only IDs for that workspace, and change the persister to write an
empty array when `openTabs` is empty (i.e., always call localStorage.setItem
with JSON.stringify(openTabs.map(...)) rather than skipping on
`!openTabs.length`) so stale entries are removed when switching workspaces.
---
Outside diff comments:
In `@blocksuite/affine/components/src/tooltip/tooltip.ts`:
- Around line 27-44: Replace raw CSS variable strings for tooltip colors with
the cssVarV2 helper for consistency: update occurrences of
'var(--affine-v2-tooltips-background, var(--affine-tooltip))' and
'var(--affine-v2-tooltips-foreground, var(--affine-white))' in tooltip.ts to use
cssVarV2('tooltips/background') and cssVarV2('tooltips/foreground')
respectively, and set TOOLTIP_ARROW_COLOR to the cssVarV2('tooltips/background')
value so the file matches other tooltip implementations (look for the styled
template and the TOOLTIP_ARROW_COLOR constant to locate the replacements).
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 278-289: The delete handler currently cleans server histories but
doesn't remove the corresponding tab from the local openTabs state, leaving a
stale tab id; update the logic so that when a session is deleted (in
cleanupSession and/or onActiveSessionDeleted) you also remove that sessionId
from the openTabs array and update the active session if needed: locate
cleanupSession, isActiveSession, onActiveSessionDeleted, openTabs, session and
newSession and after calling AIProvider.histories.cleanup remove any openTabs
entries matching sessionToDelete.sessionId (and if the deleted session was
active, call newSession or set a fallback session) so the tab strip no longer
shows server-deleted sessions.
---
Nitpick comments:
In `@blocksuite/affine/components/src/tooltip/tooltip.ts`:
- Around line 43-71: The tooltip arrow currently inlines borderColor via the
TOOLTIP_ARROW_COLOR template string inside triangleMap (symbols:
TOOLTIP_ARROW_COLOR, TRIANGLE_HEIGHT, triangleMap) which mixes inline styles
with stylesheet usage and can prevent consistent theme updates; instead move the
arrow triangle CSS into the component stylesheet by creating per-placement
classes (e.g., .tooltip-arrow--top / --right / --bottom / --left) that use the
same CSS variable (var(--affine-v2-tooltips-background, var(--affine-tooltip)))
for border-color, then update the render logic in tooltip.ts to assign the
appropriate placement class to the arrow element rather than setting borderColor
via styleMap from triangleMap and remove the template-string concatenation with
TOOLTIP_ARROW_COLOR.
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts`:
- Line 5: The constant MAX_ATTACHMENT_SIZE is declared but not exported and the
user-facing message uses a duplicated "less than 50MB" literal that will drift
if the constant changes; export MAX_ATTACHMENT_SIZE from attachment-utils (add
export to the declaration) and update any user-facing string generation (the
code that currently emits the "less than 50MB" text around lines 33-34) to
compute the human-readable size from MAX_ATTACHMENT_SIZE (e.g., derive MB from
bytes and format it) so the public API matches the barrel export and the
displayed size always reflects the constant.
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts`:
- Around line 498-531: In _setupInternalDropTarget(), don't wrap the single
cleanup function with combine; assign dropTargetCleanup directly to
this._internalDropCleanup and remove the unused combine import (or keep combine
only if you plan to merge additional cleanup sources later); update references
to dropTargetCleanup and ensure combine is deleted from imports when no longer
used.
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts`:
- Line 24: The expression assigning firstLine in ai-chat-tabs.ts uses a
redundant nullish fallback; replace the line that sets the constant firstLine
(currently using raw.split('\n')[0] ?? raw) with a simpler raw.split('\n')[0] so
the code relies on split's guaranteed single-element array and removes the
unnecessary "?? raw" fallback.
- Around line 192-207: The current updated() handler always calls scrollIntoView
for the element found by
renderRoot.querySelector(`[data-session-id="${this.activeSessionId}"]`), causing
unwanted smooth-scroll on initial mount; change updated() in the ai-chat-tabs
component to first determine whether that activeTab is actually outside its
scrollable container (compute activeTab.getBoundingClientRect() and compare to
the scroll parent’s client bounding rect — locate the scroll parent by walking
up via closest/scrollable overflow or fall back to document/viewport) and only
call activeTab.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline:
'nearest' }) if it is out of view. Ensure you reference activeSessionId and
renderRoot as in the diff and avoid calling scrollIntoView when the element is
already fully visible.
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 502-518: The effect that creates and attaches the AIChatTabs
element leaves it mounted and its callbacks referencing stale closures when the
component unmounts; update the useEffect that uses
chatTabsContainerRef/AIChatTabs to return a cleanup function that removes the
tabs element from chatTabsContainerRef (or calls a destructor if AIChatTabs
exposes one), nulls out tabs.onSelectTab and tabs.onCloseTab, and calls
setChatTabs(null) so handlers don't retain stale closeTab/onOpenSession
references; ensure you reference the same AIChatTabs instance created in the
effect and clear it on cleanup.
- Around line 423-468: The hydration effect for open tabs (the useEffect that
reads `ai-chat-open-tabs:${workspaceId}` and calls `client.getSession`) can race
with the other effect that syncs `currentSession` into `openTabs`, causing a
newly-created session to appear before restored tabs; fix by ensuring hydrated
entries are prepended or by seeding `openTabs` synchronously from the stored IDs
before awaiting `client.getSession`: when building `hydrated` results, insert
them before existing tabs (use `setOpenTabs(prev => { const seen = new
Set(prev.map(t=>t.sessionId)); const merged =
[...hydrated.filter(e=>!seen.has(e.sessionId)), ...prev]; return merged; })`) or
initially map stored IDs to placeholder entries (or sessionId-only objects) and
set that as initial `openTabs` so the later-resolved sessions replace
placeholders in-place rather than appending.
In
`@packages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsx`:
- Around line 25-26: Replace the hard-coded fill="#1E96EB" on the SVG path (the
element with d="M15.25 6h3.25a.5.5 0 0 1 .5.5v11a.5.5 0 0 1-.5.5h-3.25z") with
the project’s semantic accent/primary CSS token and a hex fallback (e.g., use a
CSS var like var(--accent-primary, `#1E96EB`) or the established token name in our
theme) so the open-state indicator follows custom themes and accent-color
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96e81198-f942-4ed4-b721-c36defd33a5e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
blocksuite/affine/components/src/tooltip/tooltip.tspackages/frontend/core/package.jsonpackages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/add-popover.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/index.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-toolbar.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/index.tspackages/frontend/core/src/blocksuite/ai/effects/app.tspackages/frontend/core/src/blocksuite/ai/effects/registry.tspackages/frontend/core/src/desktop/pages/workspace/chat/index.css.tspackages/frontend/core/src/desktop/pages/workspace/chat/index.tsxpackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.css.tspackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsxpackages/frontend/core/src/modules/workbench/view/route-container.tsxpackages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsxpackages/frontend/track/src/events.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx (1)
439-460:⚠️ Potential issue | 🟠 MajorFilter tabs by workspace instead of resetting after persistence.
setOpenTabs([])runs after the persistence effect, so on a workspace switch staleopenTabscan still be written under the new workspace key and briefly rendered viatabs.sessions = openTabs. Prefer derivingworkspaceOpenTabsand using it for both persistence andAIChatTabs.sessions. This is the same workspace-scoping concern previously raised, but the current reset still happens too late in the effect order.Also applies to: 648-663
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx` around lines 439 - 460, The persistence effect is writing stale openTabs to the new workspace key because setOpenTabs([]) runs later; instead derive a workspace-scoped array (e.g. workspaceOpenTabs = openTabs.filter(t => t.workspaceId === doc?.workspace.id || t.sessionId in persistedSessionsFor(doc?.workspace.id))) and use workspaceOpenTabs for both localStorage persistence (storageKey) and for passing to AIChatTabs.sessions, and remove the separate "reset tabs" useEffect that calls setOpenTabs([]) on workspaceId change so state is never cleared after persistence; update references to openTabs -> workspaceOpenTabs in the effect and the AIChatTabs props.
🧹 Nitpick comments (1)
packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts (1)
23-35: Avoid unbounded parallel attachment processing.
Promise.allstarts every non-imageaddChipworkflow at once. For large multi-file drops, this can spike client/network work; process sequentially or add a small concurrency cap.♻️ Sequential processing option
const others = files.filter(file => !file.type.startsWith('image/')); - await Promise.all( - others.map(async file => { - if (file.size > MAX_ATTACHMENT_SIZE) { - toast(`${file.name} is too large, please upload a file less than 50MB`); - return; - } - await addChip({ - file, - state: 'processing', - }); - }) - ); + for (const file of others) { + if (file.size > MAX_ATTACHMENT_SIZE) { + toast(`${file.name} is too large, please upload a file less than 50MB`); + continue; + } + await addChip({ + file, + state: 'processing', + }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts` around lines 23 - 35, The current logic uses Promise.all over others.map which starts all non-image addChip workflows concurrently; replace this with a sequential or bounded-concurrency approach to avoid spiking client/network load. Iterate over the array produced by files.filter(...) (or use a small worker pool) and for each file run the existing size check against MAX_ATTACHMENT_SIZE, call toast when too large, and then await addChip({ file, state: 'processing' }) before proceeding to the next (or limit simultaneous awaits to N if you prefer a concurrency cap). Keep the existing addChip, MAX_ATTACHMENT_SIZE, and toast usages and ensure errors from addChip are handled per-item so one failure doesn't stop processing the rest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts`:
- Around line 489-495: firstUpdated currently calls _setupInternalDropTarget
only once, but disconnectedCallback clears _internalDropCleanup causing
reattached elements to lose the internal doc drop target; move the
_setupInternalDropTarget call into connectedCallback guarded so it runs on each
attach without duplicating (check for existing _internalDropCleanup or a boolean
flag before calling) and keep the cleanup in disconnectedCallback (clear
_internalDropCleanup and call it there as already implemented) so reconnected
instances re-register the internal drop target; update connectedCallback,
firstUpdated, and disconnectedCallback accordingly to reference those
methods/fields.
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 423-500: The openTabs array isn't scoped to a workspace when
workspaceId changes, causing stale session IDs to be read/written to the new
storage key; update the hydration and persistence useEffect logic so openTabs is
filtered or reset by workspaceId: in the hydration effect (the useEffect that
reads `ai-chat-open-tabs:${workspaceId}` and calls setOpenTabs) only merge
entries whose entry.workspaceId === workspaceId (or reset setOpenTabs([]) before
starting hydration), and in the persistence effect (the useEffect that writes
localStorage for `ai-chat-open-tabs:${workspaceId}`) write only
openTabs.filter(tab => tab.workspaceId === workspaceId) (or skip writing until
the workspace-scoped tabs are set); also ensure AIChatTabs receives the
workspace-scoped tabs (pass a workspaceOpenTabs variable to AIChatTabs.sessions)
so rendering is scoped per workspace.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 373-421: The effect that hydrates persisted chat tabs currently
exits early when AIProvider.session is not ready but only lists doc in its
dependency array, so hydration never re-runs when the session later becomes
available; update the useEffect to also depend on the session readiness (e.g.,
include AIProvider.session or AIProvider.slots.sessionReady) and ensure the same
guard logic (if (!doc || !sessionService) return) remains so the promise-based
hydration in sessionService.getSession (used in the Promise.all map) runs once
the provider is ready and calls setOpenTabs to merge hydrated entries.
---
Duplicate comments:
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 439-460: The persistence effect is writing stale openTabs to the
new workspace key because setOpenTabs([]) runs later; instead derive a
workspace-scoped array (e.g. workspaceOpenTabs = openTabs.filter(t =>
t.workspaceId === doc?.workspace.id || t.sessionId in
persistedSessionsFor(doc?.workspace.id))) and use workspaceOpenTabs for both
localStorage persistence (storageKey) and for passing to AIChatTabs.sessions,
and remove the separate "reset tabs" useEffect that calls setOpenTabs([]) on
workspaceId change so state is never cleared after persistence; update
references to openTabs -> workspaceOpenTabs in the effect and the AIChatTabs
props.
---
Nitpick comments:
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.ts`:
- Around line 23-35: The current logic uses Promise.all over others.map which
starts all non-image addChip workflows concurrently; replace this with a
sequential or bounded-concurrency approach to avoid spiking client/network load.
Iterate over the array produced by files.filter(...) (or use a small worker
pool) and for each file run the existing size check against MAX_ATTACHMENT_SIZE,
call toast when too large, and then await addChip({ file, state: 'processing' })
before proceeding to the next (or limit simultaneous awaits to N if you prefer a
concurrency cap). Keep the existing addChip, MAX_ATTACHMENT_SIZE, and toast
usages and ensure errors from addChip are handled per-item so one failure
doesn't stop processing the rest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4810d50c-5016-442d-a66a-d6c6f19b2001
📒 Files selected for processing (6)
packages/frontend/core/src/blocksuite/ai/components/ai-chat-chips/attachment-utils.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.tspackages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.tspackages/frontend/core/src/desktop/pages/workspace/chat/index.tsxpackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsxpackages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/frontend/core/src/modules/workbench/view/sidebar/sidebar-header.tsx
- packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts (1)
518-544:combine()with a single cleanup is redundant.
combineis designed to merge multiple cleanup fns fromdropTargetForElements/monitorForElements/etc.; wrapping a single cleanup adds no value. Either storedropTargetCleanupdirectly, or actually combine it with something (e.g. amonitorForElementsregistration) if that's the eventual intent.♻️ Proposed simplification
- const dropTargetCleanup = dropTargetForElements({ + this._internalDropCleanup = dropTargetForElements({ element: el, canDrop: ({ source }) => { ... }, ... }); - this._internalDropCleanup = combine(dropTargetCleanup);Then drop the
combineimport if no longer used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts` around lines 518 - 544, The code wraps the single cleanup returned from dropTargetForElements in combine unnecessarily; instead assign dropTargetCleanup directly to this._internalDropCleanup (i.e., this._internalDropCleanup = dropTargetCleanup) and remove the now-unused combine import, unless you actually intend to merge multiple disposables (in which case add the other cleanup like monitorForElements to combine). Ensure you update references to combine and verify _internalDropCleanup still receives the cleanup function from dropTargetCleanup.packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx (1)
218-233: Same stale-closure concern as the detail-page panel'scloseTab.
openTabsin the dep array means each closed-tab invocation captures the array at render time; burst-closing tabs can cause the later handler to overwrite the earliersetOpenTabswith a filter computed from pre-update state. Use the functional updater (setOpenTabs(prev => ...)) and derive the fallback inside the updater so successive calls compose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 218 - 233, The closeTab callback uses the stale openTabs array from its closure which can cause lost updates when multiple tabs are closed quickly; change it to call setOpenTabs with a functional updater (setOpenTabs(prev => ...)) and compute the next tabs and chosen fallback inside that updater, then after updating decide whether to call onOpenSession(fallback.sessionId) or createFreshSession(), ensuring you reference the same symbols (closeTab, setOpenTabs, openTabs logic, currentSession?.sessionId, onOpenSession, createFreshSession) so successive calls compose correctly without relying on the captured openTabs value.packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx (1)
294-309:closeTabreads from a staleopenTabsclosure across rapid successive calls.
closeTabis memoized againstopenTabs, so each invocation sees whateveropenTabswas at the time React last rendered the callback. If the user rapidly closes two tabs in the same tick (e.g., keyboard shortcut held, or middle-click spam), the second call can observe the pre-first-close array, computenextfrom stale state, and overwrite the in-flight update — effectively resurrecting the first-closed tab.Prefer the functional updater form so successive calls compose correctly:
const closeTab = useCallback( (sessionId: string) => { - const idx = openTabs.findIndex(tab => tab.sessionId === sessionId); - if (idx === -1) return; - const next = openTabs.filter(tab => tab.sessionId !== sessionId); - setOpenTabs(next); - if (session?.sessionId !== sessionId) return; - const fallback = next[idx] ?? next[idx - 1] ?? next[0]; + let fallback: CopilotChatHistoryFragment | undefined; + let wasPresent = false; + setOpenTabs(prev => { + const idx = prev.findIndex(tab => tab.sessionId === sessionId); + if (idx === -1) return prev; + wasPresent = true; + const next = prev.filter(tab => tab.sessionId !== sessionId); + fallback = next[idx] ?? next[idx - 1]; + return next; + }); + if (!wasPresent || session?.sessionId !== sessionId) return; if (fallback) { openSession(fallback.sessionId).catch(console.error); } else { newSession().catch(console.error); } }, - [newSession, openSession, openTabs, session?.sessionId] + [newSession, openSession, session?.sessionId] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx` around lines 294 - 309, closeTab currently reads openTabs from a stale closure; change it to compute and apply state updates using the functional updater on setOpenTabs so rapid successive closes compose correctly: inside closeTab use setOpenTabs(prev => { const idx = prev.findIndex(t => t.sessionId === sessionId); if (idx === -1) return prev; const next = prev.filter(t => t.sessionId !== sessionId); /* store fallback candidate(s) from next and prev positions to return after updater */ return next; }); after the updater resolve the same fallback logic using the previous/next arrays you constructed (or return the chosen fallback sessionId from within the updater via a local variable captured by the outer scope) and then call openSession(fallbackId) or newSession(), and update the hook deps to remove direct openTabs dependency (keep newSession, openSession, session?.sessionId, setOpenTabs) so closeTab no longer relies on a possibly stale openTabs closure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts`:
- Around line 566-568: The overlay text "Drop to attach" is hardcoded in the
ai-chat-input render template; replace it with the project's translation helper
(call it from the component, e.g. this.i18n.translate('chat.drop_to_attach') or
this.t('chat.drop_to_attach')) where the template currently checks
this.isDragOver and renders the div with class "chat-panel-input-drop-overlay",
and add the corresponding translation key ("chat.drop_to_attach" or similar) to
the i18n resource files so the label localizes consistently with the rest of the
chat UI.
- Around line 717-742: The drag overlay can stick because _dragEnterCounter and
isDragOver are only updated in
_handleDragEnter/_handleDragLeave/_handleDragOver; add a safety reset that
clears _dragEnterCounter = 0 and sets isDragOver = false when the drag session
ends (e.g., on window 'dragend', window 'drop', and window 'blur' events) and
wire those listeners up when the component mounts and remove them when it
unmounts (or in the same lifecycle hooks that attach existing element
listeners); ensure the reset handler only runs if _dragHasFiles is true for the
original event or always clears regardless, and reference the existing symbols
(_dragHasFiles, _handleDragEnter, _handleDragLeave, _handleDragOver,
_dragEnterCounter, isDragOver) so the new listeners call the same state-reset
logic and clean up properly.
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 423-505: The persistence race deletes stored tab IDs because the
persist effect runs before async hydration completes; add a boolean state
tabsHydrated (useState(false)), set it true at the end of the hydration
useEffect (the one that reads localStorage and calls client.getSession),
including the early-return path when rawIds is empty, and set it false in the
workspace reset effect before calling setOpenTabs([]). In the persist useEffect
(the one that writes/removes ai-chat-open-tabs:${workspaceId}), short-circuit
and do nothing unless tabsHydrated is true so localStorage is only updated after
hydration has finished.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 389-476: The persist effect is wiping localStorage before async
hydration completes because persistence runs on mount and workspace changes
before the hydration Promise finishes; add a hydration-complete guard (e.g., a
boolean state like isHydrated) used by the hydration effect (the Promise.all
that calls sessionService.getSession and setOpenTabs) to set true when done and
reset to false on workspaceId change, then early-return from the persist effect
(the effect that reads/writes localStorage using storageKey and openTabs) unless
isHydrated is true; also ensure the workspace reset effect (which calls
setOpenTabs([])) also clears isHydrated so persistence is gated during switches.
---
Nitpick comments:
In
`@packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts`:
- Around line 518-544: The code wraps the single cleanup returned from
dropTargetForElements in combine unnecessarily; instead assign dropTargetCleanup
directly to this._internalDropCleanup (i.e., this._internalDropCleanup =
dropTargetCleanup) and remove the now-unused combine import, unless you actually
intend to merge multiple disposables (in which case add the other cleanup like
monitorForElements to combine). Ensure you update references to combine and
verify _internalDropCleanup still receives the cleanup function from
dropTargetCleanup.
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 218-233: The closeTab callback uses the stale openTabs array from
its closure which can cause lost updates when multiple tabs are closed quickly;
change it to call setOpenTabs with a functional updater (setOpenTabs(prev =>
...)) and compute the next tabs and chosen fallback inside that updater, then
after updating decide whether to call onOpenSession(fallback.sessionId) or
createFreshSession(), ensuring you reference the same symbols (closeTab,
setOpenTabs, openTabs logic, currentSession?.sessionId, onOpenSession,
createFreshSession) so successive calls compose correctly without relying on the
captured openTabs value.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 294-309: closeTab currently reads openTabs from a stale closure;
change it to compute and apply state updates using the functional updater on
setOpenTabs so rapid successive closes compose correctly: inside closeTab use
setOpenTabs(prev => { const idx = prev.findIndex(t => t.sessionId ===
sessionId); if (idx === -1) return prev; const next = prev.filter(t =>
t.sessionId !== sessionId); /* store fallback candidate(s) from next and prev
positions to return after updater */ return next; }); after the updater resolve
the same fallback logic using the previous/next arrays you constructed (or
return the chosen fallback sessionId from within the updater via a local
variable captured by the outer scope) and then call openSession(fallbackId) or
newSession(), and update the hook deps to remove direct openTabs dependency
(keep newSession, openSession, session?.sessionId, setOpenTabs) so closeTab no
longer relies on a possibly stale openTabs closure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 341b6771-98d6-42ed-acf9-94490993a9a0
📒 Files selected for processing (3)
packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.tspackages/frontend/core/src/desktop/pages/workspace/chat/index.tsxpackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx (1)
194-201:⚠️ Potential issue | 🟠 MajorGuard missing sessions before updating
currentSession.
client.getSessioncan returnundefinedwhen a persisted tab points to a deleted/missing session, but Line 200 stores that value directly. Prune the stale tab and avoid puttingcurrentSessioninto anundefinedstate.Suggested fix
try { const session = await client.getSession(workspaceId, sessionId); + if (!session) { + setOpenTabs(prev => prev.filter(tab => tab.sessionId !== sessionId)); + return; + } setCurrentSession(session); reMountChatContent(); chatTool?.closeHistoryMenu();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 194 - 201, onOpenSession currently assigns the result of client.getSession(workspaceId, sessionId) directly to setCurrentSession, but client.getSession can return undefined for deleted/stale sessions; update onOpenSession to check the returned session before calling setCurrentSession and reMountChatContent: if session is undefined, remove/prune the stale tab (e.g., via your existing tab removal logic) and do not call setCurrentSession or reMountChatContent; otherwise proceed to setCurrentSession(session) and reMountChatContent as before; ensure isOpeningSession is still cleared in finally and guard currentSession check remains using currentSession?.sessionId.
♻️ Duplicate comments (2)
packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx (1)
390-485:⚠️ Potential issue | 🟠 MajorMake tab hydration workspace-scoped instead of a boolean.
Line 410 can set
tabsHydratedtotruewhen no saved tabs exist, but the reset effect at Lines 482-484 runs later on the same initial mount and sets it back tofalse. That leaves persistence permanently gated, so newly created tabs in a fresh workspace may never be saved. The same boolean also still allows one render where the previous workspace’s hydrated state is used for the new workspace.Track the hydrated workspace id, and reset before hydration/persistence for actual workspace changes.
Suggested direction
- const [tabsHydrated, setTabsHydrated] = useState(false); + const [hydratedTabsWorkspaceId, setHydratedTabsWorkspaceId] = useState< + string | null + >(null); + const previousTabsWorkspaceIdRef = useRef<string | undefined>(undefined); + + const workspaceId = doc?.workspace.id; + + useEffect(() => { + if (previousTabsWorkspaceIdRef.current === undefined) { + previousTabsWorkspaceIdRef.current = workspaceId; + return; + } + if (previousTabsWorkspaceIdRef.current === workspaceId) return; + + previousTabsWorkspaceIdRef.current = workspaceId; + setOpenTabs([]); + setHydratedTabsWorkspaceId(null); + }, [workspaceId]); useEffect(() => { const sessionService = AIProvider.session; - if (!doc || !sessionService) return; - const workspaceId = doc.workspace.id; + if (!workspaceId || !sessionService) return; const storageKey = `ai-chat-open-tabs:${workspaceId}`; @@ if (!rawIds.length) { - setTabsHydrated(true); + setHydratedTabsWorkspaceId(workspaceId); return; } @@ - setTabsHydrated(true); + setHydratedTabsWorkspaceId(workspaceId); }) .catch(error => { console.error(error); - if (!cancelled) setTabsHydrated(true); + if (!cancelled) setHydratedTabsWorkspaceId(workspaceId); }); @@ - }, [doc, sessionServiceReady]); + }, [sessionServiceReady, workspaceId]); @@ useEffect(() => { - if (!doc || !tabsHydrated) return; - const storageKey = `ai-chat-open-tabs:${doc.workspace.id}`; + if (!workspaceId || hydratedTabsWorkspaceId !== workspaceId) return; + const storageKey = `ai-chat-open-tabs:${workspaceId}`; @@ - }, [doc, openTabs, tabsHydrated]); - - // Reset tabs on workspace change so tabs don't leak across workspaces. - const workspaceId = doc?.workspace.id; - useEffect(() => { - setOpenTabs([]); - setTabsHydrated(false); - }, [workspaceId]); + }, [hydratedTabsWorkspaceId, openTabs, workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx` around lines 390 - 485, The boolean tabsHydrated must become workspace-scoped: replace tabsHydrated/setTabsHydrated with hydratedWorkspaceId/setHydratedWorkspaceId (string|null) and treat the workspace as hydrated when hydratedWorkspaceId === current workspace id; in the hydration effect (uses AIProvider.session, sessionServiceReady, rawIds, Promise.all and setOpenTabs) setHydratedWorkspaceId(workspaceId) when hydration completes (including the early-return case when no saved tabs) instead of setTabsHydrated(true); in the persistence effect (uses doc, openTabs) only write/remove localStorage when hydratedWorkspaceId === doc.workspace.id; and in the workspace-reset effect (uses workspaceId) clear openTabs and setHydratedWorkspaceId(null) so hydration/persistence cannot leak between workspaces. Ensure all references to tabsHydrated are updated to the new hydratedWorkspaceId checks.packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx (1)
424-514:⚠️ Potential issue | 🟠 MajorMake tab persistence wait for the current workspace’s hydration.
This has the same ordering problem as the sidebar panel: when localStorage has no saved IDs, Lines 442-444 queue
tabsHydrated(true), then Lines 511-513 queuetabsHydrated(false)on the same initial mount. Persistence can stay disabled forever for fresh workspaces. On workspace switches, the old workspace’stabsHydrated === truecan also be observed before reset runs.Use a hydrated workspace id instead of a boolean, and reset it only for actual workspace changes before hydration/persistence run.
Suggested direction
- const [tabsHydrated, setTabsHydrated] = useState(false); + const [hydratedTabsWorkspaceId, setHydratedTabsWorkspaceId] = useState< + string | null + >(null); + const previousTabsWorkspaceIdRef = useRef<string | undefined>(workspaceId); @@ + // Reset tabs on workspace change so tabs don't leak across workspaces. + useEffect(() => { + if (previousTabsWorkspaceIdRef.current === workspaceId) return; + previousTabsWorkspaceIdRef.current = workspaceId; + setOpenTabs([]); + setHydratedTabsWorkspaceId(null); + }, [workspaceId]); + useEffect(() => { if (!workspaceId) return; @@ if (!rawIds.length) { - setTabsHydrated(true); + setHydratedTabsWorkspaceId(workspaceId); return; } @@ - setTabsHydrated(true); + setHydratedTabsWorkspaceId(workspaceId); }) .catch(error => { console.error(error); - if (!cancelled) setTabsHydrated(true); + if (!cancelled) setHydratedTabsWorkspaceId(workspaceId); }); @@ useEffect(() => { - if (!workspaceId || !tabsHydrated) return; + if (!workspaceId || hydratedTabsWorkspaceId !== workspaceId) return; const storageKey = `ai-chat-open-tabs:${workspaceId}`; @@ - }, [openTabs, workspaceId, tabsHydrated]); - - // Reset tabs on workspace change so tabs don't leak across workspaces. - useEffect(() => { - setOpenTabs([]); - setTabsHydrated(false); - }, [workspaceId]); + }, [hydratedTabsWorkspaceId, openTabs, workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 424 - 514, Replace the boolean tabsHydrated with a hydratedWorkspaceId string|null and use that to gate persistence: in the storage-hydration effect that reads localStorage (the useEffect starting with "if (!workspaceId) return;"), call setHydratedWorkspaceId(workspaceId) when hydration completes (instead of setTabsHydrated(true)) and only treat entries as hydrated when hydratedWorkspaceId === workspaceId; in the persistence effect that writes localStorage (the useEffect watching openTabs, workspaceId, tabsHydrated), change the guard to require hydratedWorkspaceId === workspaceId before persisting; and in the reset effect (the useEffect that currently calls setOpenTabs([]); setTabsHydrated(false); on workspaceId change) set hydratedWorkspaceId to null when workspaceId actually changes so hydration and persistence are scoped to the correct workspace. Ensure all references to tabsHydrated/setTabsHydrated are replaced with hydratedWorkspaceId/setHydratedWorkspaceId and comparisons use equality to workspaceId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 194-201: onOpenSession currently assigns the result of
client.getSession(workspaceId, sessionId) directly to setCurrentSession, but
client.getSession can return undefined for deleted/stale sessions; update
onOpenSession to check the returned session before calling setCurrentSession and
reMountChatContent: if session is undefined, remove/prune the stale tab (e.g.,
via your existing tab removal logic) and do not call setCurrentSession or
reMountChatContent; otherwise proceed to setCurrentSession(session) and
reMountChatContent as before; ensure isOpeningSession is still cleared in
finally and guard currentSession check remains using currentSession?.sessionId.
---
Duplicate comments:
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 424-514: Replace the boolean tabsHydrated with a
hydratedWorkspaceId string|null and use that to gate persistence: in the
storage-hydration effect that reads localStorage (the useEffect starting with
"if (!workspaceId) return;"), call setHydratedWorkspaceId(workspaceId) when
hydration completes (instead of setTabsHydrated(true)) and only treat entries as
hydrated when hydratedWorkspaceId === workspaceId; in the persistence effect
that writes localStorage (the useEffect watching openTabs, workspaceId,
tabsHydrated), change the guard to require hydratedWorkspaceId === workspaceId
before persisting; and in the reset effect (the useEffect that currently calls
setOpenTabs([]); setTabsHydrated(false); on workspaceId change) set
hydratedWorkspaceId to null when workspaceId actually changes so hydration and
persistence are scoped to the correct workspace. Ensure all references to
tabsHydrated/setTabsHydrated are replaced with
hydratedWorkspaceId/setHydratedWorkspaceId and comparisons use equality to
workspaceId.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 390-485: The boolean tabsHydrated must become workspace-scoped:
replace tabsHydrated/setTabsHydrated with
hydratedWorkspaceId/setHydratedWorkspaceId (string|null) and treat the workspace
as hydrated when hydratedWorkspaceId === current workspace id; in the hydration
effect (uses AIProvider.session, sessionServiceReady, rawIds, Promise.all and
setOpenTabs) setHydratedWorkspaceId(workspaceId) when hydration completes
(including the early-return case when no saved tabs) instead of
setTabsHydrated(true); in the persistence effect (uses doc, openTabs) only
write/remove localStorage when hydratedWorkspaceId === doc.workspace.id; and in
the workspace-reset effect (uses workspaceId) clear openTabs and
setHydratedWorkspaceId(null) so hydration/persistence cannot leak between
workspaces. Ensure all references to tabsHydrated are updated to the new
hydratedWorkspaceId checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0cc70af9-8749-41a2-8160-49555f185767
📒 Files selected for processing (3)
packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.tspackages/frontend/core/src/desktop/pages/workspace/chat/index.tsxpackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/frontend/core/src/blocksuite/ai/components/ai-chat-input/ai-chat-input.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx (1)
398-453: Hydration re-fires on every doc change within the same workspace.The effect's dependency array is
[doc, sessionServiceReady], but the actual hydration key isdoc.workspace.id. When the user navigates between docs inside the same workspace,docchanges → this effect re-runs →Promise.all(rawIds.map(id => sessionService.getSession(...)))re-fetches every persisted tab from the server on every navigation. With a handful of open tabs and frequent navigation, this is a noticeable extra network cost and no state is actually being changed (the merge intoopenTabsdedups bysessionId).Narrowing the dependency to the workspace id avoids the redundant work while still re-hydrating correctly across workspaces and when the session service becomes ready:
♻️ Suggested refactor
- useEffect(() => { - const sessionService = AIProvider.session; - if (!doc || !sessionService) return; - const workspaceId = doc.workspace.id; + const hydrateWorkspaceId = doc?.workspace.id; + useEffect(() => { + const sessionService = AIProvider.session; + if (!hydrateWorkspaceId || !sessionService) return; + const workspaceId = hydrateWorkspaceId; const storageKey = `ai-chat-open-tabs:${workspaceId}`; // ... - }, [doc, sessionServiceReady]); + }, [hydrateWorkspaceId, sessionServiceReady]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx` around lines 398 - 453, The effect currently depends on `doc`, causing hydration to re-run on any doc change even within the same workspace; extract the workspace id (e.g. `const workspaceId = doc?.workspace.id ?? null`) and change the useEffect to depend on `workspaceId` and `sessionServiceReady` instead of `doc`, using `if (!workspaceId || !AIProvider.session) return;` at the top; keep the rest of the logic (`storageKey`, `rawIds`, `sessionService.getSession`, `setOpenTabs`, `setHydratedWorkspaceId`, and the cancel flag) unchanged so hydration only runs when the workspace id or service readiness actually change.packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx (1)
227-242: Minor: prefer functionalsetOpenTabsincloseTab.
closeTabreadsopenTabsfrom the closure (triggering a new callback identity on every state change) and computesnextimperatively. This works because the tabs effect at 524-540 re-bindsonCloseTabeach render, but a functional update would be more robust and dropopenTabsfrom the dependency list:♻️ Suggested refactor
const closeTab = useCallback( (sessionId: string) => { - const idx = openTabs.findIndex(tab => tab.sessionId === sessionId); - if (idx === -1) return; - const next = openTabs.filter(tab => tab.sessionId !== sessionId); - setOpenTabs(next); - if (currentSession?.sessionId !== sessionId) return; - const fallback = next[idx] ?? next[idx - 1] ?? next[0]; + let fallback: NonNullable<CopilotSession> | undefined; + let shouldSwitch = false; + setOpenTabs(prev => { + const idx = prev.findIndex(tab => tab.sessionId === sessionId); + if (idx === -1) return prev; + const next = prev.filter(tab => tab.sessionId !== sessionId); + shouldSwitch = currentSession?.sessionId === sessionId; + fallback = next[idx] ?? next[idx - 1] ?? next[0]; + return next; + }); + if (!shouldSwitch) return; if (fallback) { onOpenSession(fallback.sessionId).catch(console.error); } else { createFreshSession().catch(console.error); } }, - [createFreshSession, currentSession?.sessionId, onOpenSession, openTabs] + [createFreshSession, currentSession?.sessionId, onOpenSession] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx` around lines 227 - 242, closeTab reads openTabs from the closure which forces it to change identity on every tabs update; convert it to use the functional state updater so it computes next from the previous state inside setOpenTabs(prev => { ... }) and returns the new array, then after updating determine if the closed session was the current one and pick fallback from the computed next (use prev to compute idx and fallback) to call onOpenSession(fallback.sessionId) or createFreshSession(); reference functions/vars: closeTab, setOpenTabs, openTabs (replace with prev), currentSession, onOpenSession, createFreshSession and remove openTabs from the dependency array of useCallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/frontend/core/src/desktop/pages/workspace/chat/index.tsx`:
- Around line 227-242: closeTab reads openTabs from the closure which forces it
to change identity on every tabs update; convert it to use the functional state
updater so it computes next from the previous state inside setOpenTabs(prev => {
... }) and returns the new array, then after updating determine if the closed
session was the current one and pick fallback from the computed next (use prev
to compute idx and fallback) to call onOpenSession(fallback.sessionId) or
createFreshSession(); reference functions/vars: closeTab, setOpenTabs, openTabs
(replace with prev), currentSession, onOpenSession, createFreshSession and
remove openTabs from the dependency array of useCallback.
In
`@packages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx`:
- Around line 398-453: The effect currently depends on `doc`, causing hydration
to re-run on any doc change even within the same workspace; extract the
workspace id (e.g. `const workspaceId = doc?.workspace.id ?? null`) and change
the useEffect to depend on `workspaceId` and `sessionServiceReady` instead of
`doc`, using `if (!workspaceId || !AIProvider.session) return;` at the top; keep
the rest of the logic (`storageKey`, `rawIds`, `sessionService.getSession`,
`setOpenTabs`, `setHydratedWorkspaceId`, and the cancel flag) unchanged so
hydration only runs when the workspace id or service readiness actually change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8321c579-ebe2-446d-a6b1-f5e65f110ee3
📒 Files selected for processing (2)
packages/frontend/core/src/desktop/pages/workspace/chat/index.tsxpackages/frontend/core/src/desktop/pages/workspace/detail-page/tabs/chat.tsx
Contributor
Author
|
@darkskygit Kindly review this pr whenever you get time. |
Member
|
Hello, an unpushed refactor of AI modules involves changes to the same files of your pr, which is likely to conflict with your branch. |
Contributor
Author
Thanks for the heads-up, @darkskygit - that's totally fine. I'll keep this branch as-is and rebase the pr once the AI modules refactor lands. |
darkskygit
force-pushed
the
canary
branch
5 times, most recently
from
April 29, 2026 11:31
3403237 to
78a9942
Compare
Contributor
Author
|
@darkskygit any updates?? |
Member
refactor merged, you can rebase now |
Addresses issue toeverything#14189 (point 3): - Tooltip primitive switched from the non-theme-aware --affine-tooltip / --affine-white CSS vars to --affine-v2-tooltips-background / --affine-v2-tooltips-foreground, with the old vars kept as fallbacks. Tooltips now auto-invert in dark mode across the entire app, not just the chat panel. - Chat-history dropdown icon changed from the ambiguous ArrowDownSmallIcon chevron to HistoryIcon, which is semantically clear. Added data-testid for e2e coverage.
Addresses issue toeverything#14189 (point 2): - Native drag-and-drop on the chat input accepts OS files (images go into the image preview grid, other files become attachment chips) with the same 50 MB cap as the "+" picker. - Internal AFFiNE document drags from the nav panel land as doc chips, handled via pragmatic-drag-and-drop's dropTargetForElements so the existing DnD payload plumbing is reused. - The chat input shows a "Drop to attach" overlay during drag, reusing the focused-border token for visual consistency. - The image/file routing logic from add-popover was extracted into a shared addFilesToChat helper so the file picker and drop handler stay in lockstep. - Analytics: extended the addEmbeddingDoc.control union with 'dragDrop' so drop-originated attachments are distinguishable from button-initiated ones. - Declared @atlaskit/pragmatic-drag-and-drop as a direct dependency of @affine/core (previously transitive via @affine/component).
Addresses issue toeverything#14189 (point 1): Replaces the previous "+" creates a fresh chat and replaces the current one behavior with a browser/IDE-style tab strip above the chat content. Both the sidebar chat panel and the standalone /chat route are wired identically. - New Lit component ai-chat-tabs renders one tab per open session with a close "x", active-tab highlight, horizontal scroll when tabs overflow (with a wheel-to-horizontal converter so mouse wheel and trackpad vertical swipes scroll the strip), and auto-scroll-into-view when the active tab changes (so a newly created tab slides into view). - Tab title is derived from the session's stored title, falling back to the first user message, falling back to "New chat". - Open tabs are persisted per-workspace in localStorage (ai-chat-open-tabs:{workspaceId}) and rehydrated on mount via the existing AIProvider.session / CopilotClient.getSession path. - Close-tab removes the tab from the strip but leaves the session on the server (consistent with the existing session-history dropdown), and switches the active tab to an adjacent one; if no tabs remain, starts a fresh session. - "+" in the toolbar still calls the same newSession / createFreshSession handlers, but the previous chat now remains as a tab instead of being torn down. - The tab strip shares the header row with the existing toolbar icons (pin, history, +), with a border-bottom spanning the whole row for visual grouping. - Registers the ai-chat-tabs custom element via the existing AI effects registry so both React-rooted and BlockSuite-rooted trees pick it up.
…oggle Minor UX polish on the right-sidebar toggle, surfaced while testing the AI chat panel work for toeverything#14189 but applying workbench-wide: - Adds tooltips ("Open sidebar" on the route-container button that appears when the sidebar is hidden, "Close sidebar" on the sidebar-header button that appears when the sidebar is expanded). Both previously had no hover affordance. - Introduces a small RightSidebarOpenIcon variant used by the close button so the "sidebar is open" state is visually distinct from the "sidebar is closed" state — same base outline as RightSidebarIcon, right panel filled in the AFFiNE accent color.
- ai-chat-input: route drag-drop analytics via a new _trackDragDrop helper that branches on independentMode, so the standalone /chat page logs events under track.$.intelligence like the sibling add-popover._track does. Previously hardcoded to track.$.chatPanel.
- ai-chat-tabs: simplify deriveTabTitle's first-line extraction (raw.split('\n')[0] ?? raw was unreachable since raw is guaranteed non-empty at that point).
- chat/index.tsx (standalone) and chat.tsx (sidebar): mirror empty-tab state to localStorage instead of leaving stale IDs behind when the user closes all tabs and the fresh-session fallback errors.
- chat.tsx (sidebar): reset openTabs to [] when the active workspace changes, so tabs don't leak across workspaces if the component is reused.
- Strip descriptive inline comments that restate what well-named code already conveys.
- ai-chat-input: move internal doc drop-target registration from firstUpdated to connectedCallback (awaiting updateComplete, with an idempotency guard). firstUpdated runs once per element lifetime, so disconnect-then-reconnect was leaving the pragmatic drop target unattached while the native file drop kept working. - chat/index.tsx (standalone): add a workspace-change reset effect matching the sidebar variant, so openTabs can't leak into another workspace before hydration fires under the new storage key. - chat.tsx (sidebar): re-run the tab hydration effect when AIProvider.session transitions to ready. The sibling initPanel effect already uses AIProvider.slots.sessionReady; without the same trigger for hydration, persisted tabs could silently fail to load on the delayed-session startup path.
- chat.tsx (sidebar) and chat/index.tsx (standalone): gate the persist effect on a new tabsHydrated state flag. Without the gate, the persist effect runs once on mount with openTabs === [] before the async hydration resolves, calling localStorage.removeItem and wiping the stored IDs. If the user closed the tab during the hydration window, their tabs would not come back next mount. The flag is set true at every hydration exit path (including the empty-rawIds early-return and the error branch), and reset false when the workspace changes so the next workspace's hydration gets a fresh chance. - ai-chat-input: add window-level dragleave/drop/dragend listeners to reset the "Drop to attach" overlay if the drag session ends without dragleave or drop firing on the input (Esc-cancel, release outside the window, drop elsewhere). Previously the overlay could remain visible until the user dragged over the input again.
- Both chat variants: replace the tabsHydrated boolean with hydratedWorkspaceId (string | null). The persist effect now writes only when hydratedWorkspaceId === workspaceId, so hydration / persistence cannot leak across workspaces even in race windows where an in-flight hydration resolves after the workspace has already switched. Removes the need to manually unset the flag in the reset effect - workspace change alone invalidates the equality check. - Both chat variants: when getSession returns null/undefined (session was deleted elsewhere or is no longer accessible), drop the stale tab from openTabs instead of setting currentSession to null and remounting an empty chat. This was a pre-existing latent bug that becomes user-visible with multi-tab persistence, since stale tab IDs can now outlive the sessions they point to.
- closeTab (both chat variants): compute the new openTabs array inside a functional setState updater (setOpenTabs(prev => ...)) and capture the fallback via an outer let. Drops openTabs from the useCallback dependency array so closeTab keeps a stable identity across tab updates, removing unnecessary re-runs of the effect that wires tabs.onCloseTab on the Lit element. - chat.tsx (sidebar): hoist workspaceId (derived from doc?.workspace.id) to a single const and swap the hydration and persistence effects to depend on workspaceId instead of doc. The effects only re-run when the workspace actually changes, not on every in-workspace doc navigation - avoiding N redundant getSession calls per doc switch.
Anexus5919
force-pushed
the
fix/ai-chat-panel-ux-14189
branch
from
May 3, 2026 18:29
cfa1bf6 to
76c5316
Compare
Contributor
Author
@darkskygit rebased onto the latest canary in 76c5316. All 9 commits replayed cleanly with no conflicts. yarn install + yarn lint:ox both clean. Ready for re-review whenever you have time. Thanks! |
darkskygit
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## canary #14850 +/- ##
==========================================
+ Coverage 55.40% 56.55% +1.14%
==========================================
Files 3006 3144 +138
Lines 168964 171058 +2094
Branches 24933 25189 +256
==========================================
+ Hits 93621 96748 +3127
+ Misses 72277 71154 -1123
- Partials 3066 3156 +90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Re-runs `yarn affine init && yarn affine gql build && yarn affine i18n build && yarn affine server genconfig` to regenerate the selfhost config schema after the upstream copilot refactor introduced new fields. Resolves the "Check Git Status" CI step.
Contributor
Author
|
@darkskygit pushed 73f47c9 to fix Check Git Status (regenerated Two observations on the previous CI failures:
This PR doesn't touch Thanks! |
Contributor
Author
|
@darkskygit
|
Member
|
use you can refer to |
Contributor
Author
|
@darkskygit you were right - the four-effect setup had a real ordering race (the reset effect could clobber Refactored in a5f8536:
Both the standalone |
darkskygit
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #14189.
Fixes the three UX issues reported in the original bug report, plus one small
adjacent polish on the right-sidebar toggle that was requested during review.
Each concern in the issue is addressed end-to-end, with the same treatment
applied to both places the AI chat panel lives: the sidebar chat panel
(right panel on a doc page) and the standalone
/chatpage.1.
+button → persistent multi-session tabs (issue point 1)Before: clicking
+calledcreateFreshSession()(standalone) ornewSession()(sidebar), both of which tore down the current chat contentand replaced it in place. There was no way to keep two chats open at once.
After: a browser/IDE-style tab strip lives above the chat content. Each
open session gets its own tab with a close
×; the active tab ishighlighted;
+now adds a tab rather than replacing the chat.Details
ai-chat-tabs(packages/frontend/core/src/blocksuite/ai/components/ai-chat-toolbar/ai-chat-tabs.ts).session.title→ first user message →"New chat".wheelhandler that convertsmouse wheel / trackpad vertical swipe into horizontal scroll (native
horizontal trackpad swipes also work natively via
overflow-x: auto).scrollIntoView({ inline: 'nearest' })on active tab change, so anewly created or newly selected tab slides into view instead of staying
hidden behind the toolbar.
×removes the tab from the strip but leaves the session on theserver (matches the existing Chat history dropdown semantics — the
session is still reachable there). Closing the active tab switches to an
adjacent one; closing the last tab starts a fresh session.
localStorageunder
ai-chat-open-tabs:{workspaceId}. On mount, the React pages hydratethose IDs via
AIProvider.session.getSession/CopilotClient.getSession— no new backend or schema work.(chat.tsx (sidebar) and
chat/index.tsx (standalone))
— hydrate → sync active session into tabs → persist.
(pin / history /
+), separated byflex: 1+min-width: 0so thetabs scroll cleanly up to the toolbar boundary.
ShadowlessElementbase class injects its static CSS globally, and the:hostselector does not match in a React-rooted DOM — the component usestag-selector CSS (
ai-chat-tabs { display: flex; … }) instead.2. Drag-and-drop attachments (issue point 2)
Before: the chat input accepted no DnD. Attaching anything required the
+→ file-picker flow.After: the chat input accepts OS files via native HTML5 DnD and AFFiNE
documents via the repo's existing pragmatic-drag-and-drop infrastructure.
Details
dragenter/over/leave/drop) onai-chat-input.ts
accept OS files: images go into the image preview grid, other files become
attachment chips, with the same 50 MB per-file cap as the
+picker.handled via
dropTargetForElementsfrom@atlaskit/pragmatic-drag-and-drop(same library the rest of the appalready uses for internal DnD).
focused-border token (
--affine-v2-layer-insideBorder-primaryBorder) forvisual consistency with the focused state.
add-popover.tswas factored into a shared helperattachment-utils.ts
(
addFilesToChat), so the+picker and the drop handler stay inlockstep.
addEmbeddingDoc.controlunion inevents.ts with
'dragDrop'sodrag-originated attachments are distinguishable from button-initiated
ones in telemetry.
@atlaskit/pragmatic-drag-and-dropis promoted from a transitivedependency (via
@affine/component) to a direct dependency of@affine/coreandyarn.lockis refreshed accordingly.3. Chat-history tooltip + icon (issue point 3)
Before: hovering the chat-history button showed a tooltip whose
background did not invert for dark theme (
--affine-tooltipis nottheme-aware), and the icon was
ArrowDownSmallIcon— a chevron that doesnot convey "history."
After: the tooltip primitive itself is theme-aware (every tooltip in
the app benefits, not just the chat one), and the icon is the
semantically-clear
HistoryIcon.Details
var(--affine-v2-tooltips-background, var(--affine-tooltip))andvar(--affine-v2-tooltips-foreground, var(--affine-white)). The V2tokens auto-invert with theme; the old vars remain as fallbacks so
components that override via the existing
tooltipStyleescape hatchcontinue to work.
ArrowDownSmallIcon→HistoryIcon; addeddata-testid="ai-panel-chat-history"for future e2e coverage.4. Right-sidebar toggle: tooltips + open-state icon (adjacent polish)
Not part of the original issue, but surfaced while testing the tab strip —
neither of the two right-sidebar toggle buttons had hover affordance, and
both used the same icon regardless of the sidebar's state.
tooltip="Open sidebar"on the route-container button shown whenthe sidebar is hidden.
tooltip="Close sidebar"on the sidebar-header button shown whenthe sidebar is expanded.
RightSidebarOpenIconvariant: same outline as
RightSidebarIcon, but with the right panelfilled in the AFFiNE accent color to convey the open state. Icon shape
change is self-contained — no new icon asset added to
@blocksuite/icons.Commits
2adc0c7— fix(ai-chat): theme-aware tooltip + semantic chat-history icon (2 files)bf26974— feat(ai-chat): drag-and-drop file and doc attachments in chat input (7 files)fca29c8— feat(ai-chat): persistent multi-session tab strip (8 files)7d5dffe— feat(workbench): tooltips and open-state icon for the right-sidebar toggle (2 files)Kept ordered smallest → largest blast radius so the history is easy to bisect.
Test plan
Verified locally against a fresh server stack (postgres / redis / mailpit via
compose, migrations run) signed in as
dev@affine.pro, in both/chatandthe sidebar chat on a doc page, in light and dark themes:
+picker).+picker, paste-image — all unchanged.+adds tab; click tab switches chat;×removes tab and switches to adjacent; close last tab → new fresh tab spawns.+click.yarn lint:oxand lint-staged both clean on every commit.Not verified locally (no local model key configured): the assistant actually
streams a response. Drop/chip flow is independent of that path.
Out of scope / follow-ups
reuse existing reducer / state paths. Happy to add tests if reviewers
prefer.
@affine/nativeis not required for the web dev stack; I only built@affine/server-native. Irrelevant to the PR diff.Summary by CodeRabbit
New Features
UI/UX
Behavior
Analytics