feat(ai): port shared chat-history persistence + UX fixes from openplc-web#747
Merged
Merged
Conversation
…c-web Mirrors the shared-layer changes from [openplc-web#385](Autonomy-Logic/openplc-web#385). Editor and web share `middleware/shared/ports/`, the AI slice, the shared workspace slice, the toast viewport, and the workspace screen layout — keeping these in sync prevents the two adapters from drifting on what the AI feature exposes to the platform. What's mirrored from web's frontend branch: - middleware/shared/ports/types.ts: - new `AIChatContentBlock` type (text / tool_use / tool_result) - `ChatMessage.content` widens to `string | AIChatContentBlock[]` - new optional `ChatMessage.conversationId` - frontend/store/slices/ai/types.ts + slice.ts: - `conversationId`, `conversations: ConversationSummary[]`, `isLoadingConversation` state - 7 new actions: setConversationId, setConversations, prependConversation, removeConversation, updateConversationTitle, replaceMessages, setLoadingConversation - `clearConversation` now also nulls `conversationId` - `updateMessageContent` widens to accept block-array content - frontend/store/__tests__/ai-slice.test.ts: 17 new tests covering all of the above. 68/68 passing. - frontend/store/slices/shared/slice.ts: `clearStatesOnCloseProject` now calls `aiActions.clearConversation()` so chat doesn't bleed across project switches. - frontend/store/slices/shared/types.ts: SharedRootState gains `AISlice &` so the typed `getState()` inside the shared slice can call `aiActions.*`. The web copy lacks this intersection but its tsc happens to skip the check; adding it everywhere is the correct shape since the AI slice is shared. (Web fix tracked as a follow-up.) - frontend/components/_features/[app]/toast/index.tsx: viewport moves to `top-4 left-1/2 -translate-x-1/2`. Slide-in animation consistently from-top. - frontend/screens/workspace-screen.tsx: chat panel ResizablePanel `defaultSize` 16% → 30%, range 16-25 → 20-50. What's NOT mirrored (web-adapter-specific): - middleware/adapters/web/services/ai/conversations/* (TanStack Query hooks bound to /ai/conversations) — the editor's AI integration is a future workstream; conversation persistence will land in the editor when its AI adapter is wired up. - middleware/adapters/web/components/ai-chat/* (chat panel, switcher, message bubble, agentic loop) — same reason. - router/pages/index-page.tsx (web routing). - The agentic-loop / panel persistence wiring fix (`lastLoadedConversationRef`) — only relevant once an AI chat panel exists in the editor. Validation: - pnpm run lint — 0 errors, 2 pre-existing warnings - pnpm run validate:arch — passes - pnpm exec tsc --noEmit — clean - jest src/frontend/store/__tests__/ai-slice.test.ts — 68/68 pass Tracking: epic DOPE-270 (shared layer changes mirrored). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughConversation tracking and message block support were added to the AI Redux slice (state, types, actions, tests). The shared root state now includes the AI slice and clears conversations on project close. Separately, UI tweaks adjust toast positioning/animation and widen the workspace chat panel sizing. ChangesConversation Management & State Integration
UI Positioning & Layout Tweaks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/frontend/screens/workspace-screen.tsx (1)
661-670:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
defaultSizeoverflow: all three panels sum to 114% when chat is open.Before this PR,
16 (explorer) + 68 (workspace) + 16 (chat) = 100%exactly. After the bump todefaultSize={30}the sum becomes 114%, soreact-resizable-panelswill silently normalize on mount. The chat panel will actually open at ~26% (≈ 30/114 × 100), not the intended 30%, and the workspace panel will be squeezed to ~60% instead of 68%.A secondary constraint conflict also exists:
chatPanel.maxSize (50) + workspacePanel.minSize (50) = 100%, which leaves exactly 0% for the explorer/source-control panel whenever the user drags chat to its maximum — collapsing the explorer entirely.🛠️ Proposed fix — keep the three-panel total at 100% when chat is open
Lower the workspace panel's
defaultSizeby the chat panel's new default contribution (30 − 16 = 14 pp), and tighten the chatmaxSizeto leave at least the explorer's minimum room:<ResizablePanel id='workspacePanel' order={2} - defaultSize={68} + defaultSize={54} minSize={50} className='flex h-full min-h-0 overflow-hidden' ><ResizablePanel id='chatPanel' order={3} defaultSize={30} minSize={20} - maxSize={50} + maxSize={40} className='relative flex h-full min-h-0 w-full' >Alternatively, make
workspacePanel.defaultSizereactive toisChatOpen:- defaultSize={68} + defaultSize={isChatOpen ? 54 : 68}
54 + 16 (explorer) + 30 (chat) = 100✓
workspacePanel.minSize(50) + chatPanel.maxSize(40) = 90→ leaves ≥ 10% for the explorer ✓🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/screens/workspace-screen.tsx` around lines 661 - 670, The three ResizablePanel defaults now sum >100% causing silent normalization; update the panel sizing so totals remain 100% and avoid zeroing the explorer: reduce the workspace panel's defaultSize from 68 to 54 (so 54 + 16 explorer + 30 chat = 100) and tighten chatPanel.maxSize from 50 to 40 to guarantee slack for the explorer (workspacePanel.minSize(50) + chatPanel.maxSize(40) = 90 leaves ≥10% for explorer); alternatively make workspacePanel.defaultSize compute based on isChatOpen (e.g., use 54 when isChatOpen true, otherwise keep previous default) and ensure IDs 'chatPanel', 'workspacePanel', and 'explorerPanel' are adjusted accordingly.src/frontend/store/slices/ai/slice.ts (1)
143-154:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
isLoadingConversationnot reset byclearConversation— stale loading indicator on project switch.If the user closes a project while a conversation is being fetched (
isLoadingConversation = true),clearConversationleaves that flag set. The next project would inherit a stale loading state in the chat panel until the flag is explicitly cleared by the next conversation operation.🛡️ Proposed fix
clearConversation: () => { setState( produce(({ ai }: AISlice) => { ai.messages = [] ai.error = null // Drop the active conversation pointer so the next /ai/chat call // is treated as a fresh conversation. The list itself stays — // the user can still see prior conversations and resume one. ai.conversationId = null + ai.isLoadingConversation = false }), ) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/store/slices/ai/slice.ts` around lines 143 - 154, clearConversation currently resets ai.messages, ai.error and ai.conversationId but leaves ai.isLoadingConversation untouched which can leave a stale loading state; update the clearConversation handler (the function named clearConversation that calls setState and produce on AISlice) to also set ai.isLoadingConversation = false so any in-flight or leftover loading flag is cleared when switching/closing projects.
🧹 Nitpick comments (2)
src/frontend/components/_features/[app]/toast/index.tsx (1)
20-20: 💤 Low valueStale inline comment.
// -> For fail toasts to be on topreferred to the previous bottom-right stacking intent. With the viewport now top-center and styling unrelated to fail-toast ordering on this element, the note is misleading mid-JSX. Consider removing it or rewording to describe the currentz-[100]rationale.✏️ Proposed cleanup
<PrimitiveToast.ToastViewport ref={ref} className={cn( 'absolute left-1/2 top-4 z-[100] flex h-fit max-h-36 w-[292px] -translate-x-1/2 px-4 xl:w-[420px]', className, )} - // -> For fail toasts to be on top {...props} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/components/_features/`[app]/toast/index.tsx at line 20, The inline comment "// -> For fail toasts to be on top" in the JSX of the Toast component is stale and misleading given the viewport is now top-center and ordering is unrelated; update the comment by either removing it or replacing it with a brief note explaining why the element has "z-[100]" (e.g., "ensure toasts render above other UI elements") so future readers understand the z-index intent; locate the comment in the Toast component JSX (index.tsx) and apply the change near the element that includes the z-[100] class.src/frontend/store/slices/ai/types.ts (1)
10-16: 💤 Low value
lastModelliteral union may need widening if additional models are introduced.
'haiku' | 'sonnet' | nullwill cause a TypeScript error if the backend starts returning a third model label (e.g.'opus'). Consider widening tostring | nullor documenting this as an intentional exhaustive union with a note to update alongside the backend.♻️ Proposed change (if widening is preferred)
export type ConversationSummary = { id: string title: string - lastModel: 'haiku' | 'sonnet' | null + /** Model label as normalized by the backend (e.g. 'haiku', 'sonnet'). */ + lastModel: string | null createdAt: string updatedAt: string }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/store/slices/ai/types.ts` around lines 10 - 16, The ConversationSummary type's lastModel field currently uses a narrow union ('haiku' | 'sonnet' | null) which will break if the backend returns new model labels; update the lastModel type in the ConversationSummary declaration to string | null (or another broader union) so it accepts any model label, and add a short comment on the ConversationSummary type to remind maintainers to tighten the type if the API becomes strictly enumerated in the future.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/frontend/screens/workspace-screen.tsx`:
- Around line 661-670: The three ResizablePanel defaults now sum >100% causing
silent normalization; update the panel sizing so totals remain 100% and avoid
zeroing the explorer: reduce the workspace panel's defaultSize from 68 to 54 (so
54 + 16 explorer + 30 chat = 100) and tighten chatPanel.maxSize from 50 to 40 to
guarantee slack for the explorer (workspacePanel.minSize(50) +
chatPanel.maxSize(40) = 90 leaves ≥10% for explorer); alternatively make
workspacePanel.defaultSize compute based on isChatOpen (e.g., use 54 when
isChatOpen true, otherwise keep previous default) and ensure IDs 'chatPanel',
'workspacePanel', and 'explorerPanel' are adjusted accordingly.
In `@src/frontend/store/slices/ai/slice.ts`:
- Around line 143-154: clearConversation currently resets ai.messages, ai.error
and ai.conversationId but leaves ai.isLoadingConversation untouched which can
leave a stale loading state; update the clearConversation handler (the function
named clearConversation that calls setState and produce on AISlice) to also set
ai.isLoadingConversation = false so any in-flight or leftover loading flag is
cleared when switching/closing projects.
---
Nitpick comments:
In `@src/frontend/components/_features/`[app]/toast/index.tsx:
- Line 20: The inline comment "// -> For fail toasts to be on top" in the JSX of
the Toast component is stale and misleading given the viewport is now top-center
and ordering is unrelated; update the comment by either removing it or replacing
it with a brief note explaining why the element has "z-[100]" (e.g., "ensure
toasts render above other UI elements") so future readers understand the z-index
intent; locate the comment in the Toast component JSX (index.tsx) and apply the
change near the element that includes the z-[100] class.
In `@src/frontend/store/slices/ai/types.ts`:
- Around line 10-16: The ConversationSummary type's lastModel field currently
uses a narrow union ('haiku' | 'sonnet' | null) which will break if the backend
returns new model labels; update the lastModel type in the ConversationSummary
declaration to string | null (or another broader union) so it accepts any model
label, and add a short comment on the ConversationSummary type to remind
maintainers to tighten the type if the API becomes strictly enumerated in the
future.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 18e215b4-abf2-43f0-a3c0-a2b33834aa12
📒 Files selected for processing (8)
src/frontend/components/_features/[app]/toast/index.tsxsrc/frontend/screens/workspace-screen.tsxsrc/frontend/store/__tests__/ai-slice.test.tssrc/frontend/store/slices/ai/slice.tssrc/frontend/store/slices/ai/types.tssrc/frontend/store/slices/shared/slice.tssrc/frontend/store/slices/shared/types.tssrc/middleware/shared/ports/types.ts
CI's `format / Format Check` step (`prettier --check src/`) flagged
`src/frontend/store/slices/ai/slice.ts` after the slice extensions
landed. The patch I applied from openplc-web kept that repo's
formatting verbatim — but the editor uses a stricter Prettier config
that re-collapses some of the multi-line `produce()` callbacks.
Ran `pnpm exec prettier --write` on the file. Only that one file
needed reformatting; the full `prettier --check 'src/**/*.{ts,tsx}'`
now reports "All matched files use Prettier code style!".
Other CI checks already pass on this PR (lint, build, arch, tsc).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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)
src/frontend/store/slices/ai/slice.ts (1)
143-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset
isLoadingConversationinclearConversationto avoid stale loading state
clearConversation(Line 143-152) clears messages/error/conversation pointer but leavesisLoadingConversationunchanged. If this action is used during project close/reset, the UI can remain stuck in a loading state.Proposed fix
clearConversation: () => { setState( produce(({ ai }: AISlice) => { ai.messages = [] ai.error = null + ai.isLoadingConversation = false // Drop the active conversation pointer so the next /ai/chat call // is treated as a fresh conversation. The list itself stays — // the user can still see prior conversations and resume one. ai.conversationId = null }), ) },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/store/slices/ai/slice.ts` around lines 143 - 152, The clearConversation reducer currently resets ai.messages, ai.error, and ai.conversationId but leaves ai.isLoadingConversation untouched, which can leave the UI stuck; update the clearConversation handler (the function named clearConversation inside the AISlice produce block used with setState/produce) to also set ai.isLoadingConversation = false so the loading flag is cleared when a conversation is reset. Ensure you modify the produce callback that mutates the ai object to include this assignment alongside the existing resets.
🧹 Nitpick comments (1)
src/frontend/store/slices/ai/slice.ts (1)
217-221: ⚡ Quick winAvoid storing caller-owned array references in Zustand state
setConversations(Line 217-221) andreplaceMessages(Line 254-259) assign incoming arrays directly. If callers mutate those arrays later, store state can change outsidesetState. Clone on write to keep state updates controlled.Proposed fix
setConversations: (conversations) => { setState( produce(({ ai }: AISlice) => { - ai.conversations = conversations + ai.conversations = [...conversations] }), ) }, @@ replaceMessages: (messages) => { setState( produce(({ ai }: AISlice) => { ai.messages = - messages.length > MAX_CONVERSATION_MESSAGES ? messages.slice(-MAX_CONVERSATION_MESSAGES) : messages + messages.length > MAX_CONVERSATION_MESSAGES + ? messages.slice(-MAX_CONVERSATION_MESSAGES) + : [...messages] ai.error = null }), ) },As per coding guidelines, "Never mutate state directly in Zustand slices; always use produce() from Immer for immutable updates".
Also applies to: 254-259
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/store/slices/ai/slice.ts` around lines 217 - 221, setConversations and replaceMessages are assigning caller-owned arrays directly into Zustand state; change these to clone the incoming arrays before assigning (e.g., create a shallow copy of conversations and of message arrays) inside the produce callback so the store never holds a reference to a mutable external array—update the assignments in setConversations and replaceMessages to use cloned copies (e.g., via slice/spread or a shallow map) when setting ai.conversations and the messages array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/frontend/store/slices/ai/slice.ts`:
- Around line 143-152: The clearConversation reducer currently resets
ai.messages, ai.error, and ai.conversationId but leaves ai.isLoadingConversation
untouched, which can leave the UI stuck; update the clearConversation handler
(the function named clearConversation inside the AISlice produce block used with
setState/produce) to also set ai.isLoadingConversation = false so the loading
flag is cleared when a conversation is reset. Ensure you modify the produce
callback that mutates the ai object to include this assignment alongside the
existing resets.
---
Nitpick comments:
In `@src/frontend/store/slices/ai/slice.ts`:
- Around line 217-221: setConversations and replaceMessages are assigning
caller-owned arrays directly into Zustand state; change these to clone the
incoming arrays before assigning (e.g., create a shallow copy of conversations
and of message arrays) inside the produce callback so the store never holds a
reference to a mutable external array—update the assignments in setConversations
and replaceMessages to use cloned copies (e.g., via slice/spread or a shallow
map) when setting ai.conversations and the messages array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2a1df740-de3b-4d45-b1d1-3437d0ca5b8f
📒 Files selected for processing (1)
src/frontend/store/slices/ai/slice.ts
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
1 participant
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.
Summary
Mirrors the shared-layer changes from openplc-web#385 so the editor and web don't drift on what the AI feature exposes through the shared port and store layers.
Web adapter–specific code is intentionally NOT included — the editor's AI integration is a future workstream. This PR only ports the bits both repos share.
What's mirrored
middleware/shared/ports/types.tsAIChatContentBlocktype;ChatMessage.contentwidens tostring | AIChatContentBlock[]; new optionalChatMessage.conversationIdfrontend/store/slices/ai/types.ts+slice.tsconversationId,conversations,isLoadingConversationstate; 7 new actions (setConversationId,setConversations,prependConversation,removeConversation,updateConversationTitle,replaceMessages,setLoadingConversation);clearConversationalso nullsconversationId;updateMessageContentaccepts block-array contentfrontend/store/__tests__/ai-slice.test.tsfrontend/store/slices/shared/slice.tsclearStatesOnCloseProjectnow callsaiActions.clearConversation()so chat doesn't bleed across project switchesfrontend/store/slices/shared/types.tsSharedRootStategainsAISlice &so the shared slice's typedgetState()can callaiActions.*frontend/components/_features/[app]/toast/index.tsxtop-4 left-1/2 -translate-x-1/2; slide-in consistently from-topfrontend/screens/workspace-screen.tsxResizablePaneldefaultSize16% → 30%, range 16-25 → 20-50What's NOT included (web-specific)
middleware/adapters/web/services/ai/conversations/*— TanStack Query hooks bound to/ai/conversationsmiddleware/adapters/web/components/ai-chat/*— chat panel, switcher, message bubble, agentic looprouter/pages/index-page.tsx— web routinglastLoadedConversationReffix — only matters once a chat panel exists in the editorNotable type fix
SharedRootStatewas missing theAISliceintersection in both repos. The editor's stricter tsc invocation surfaced it; web's happens to skip the check. AddingAISliceis the correct shape since the AI slice IS shared, so this PR fixes it here. A follow-up will mirror the same fix on web.Validation
pnpm run lint— 0 errors, 2 pre-existing warningspnpm run validate:arch— passespnpm exec tsc --noEmit— cleanjest src/frontend/store/__tests__/ai-slice.test.ts— 68/68 passTracking
Test plan
Saveaction)capabilities.hasAIAssistant)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI/UX Changes