fix(shared): mirror pre-deploy fixes from openplc-web - #780
Conversation
Mirror of openplc-web#412 — keeps src/frontend/** and the shared
globals.css byte-identical between the two IDEs so the Shared Surface
Sync CI check stays green.
Bundled (per file):
- src/frontend/components/_molecules/menu-bar/menus/help.tsx
→ Community Support points at edge.autonomylogic.com/forum.
- src/frontend/store/slices/ai/{types,slice,index}.ts +
src/frontend/store/slices/index.ts
→ Drop the silent 50-message store cap so long conversations don't
lose their oldest turns. addMessage / replaceMessages keep every
message; backend's 500/conv cap remains the only ceiling.
- src/frontend/store/__tests__/ai-slice.test.ts
→ Flip the two cap-related tests to assert no truncation.
- src/backend/shared/styles/globals.css
→ Add `@keyframes spin` so Tailwind's `animate-spin` utility actually
spins; the project-wide tailwind config overrode `theme.keyframes`
instead of extending it and silently killed the default.
Mirror of: Autonomy-Logic/openplc-web#412
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughPR removes message truncation logic from the AI Redux slice to allow unlimited message retention, adds a CSS fallback for the spinner animation, and updates the help menu forum link. Supporting type definitions and re-exports are adjusted accordingly. ChangesCore Updates
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)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/frontend/store/slices/ai/slice.ts`:
- Around line 288-293: replaceMessages currently assigns the caller-owned
messages array by reference (ai.messages = messages), which can cause external
mutations to affect store state; inside the replaceMessages reducer (the
setState/produce callback for AISlice) replace that assignment with a cloned
array (e.g., a shallow copy like [...messages] or messages.slice()) so the store
owns its copy of messages and ai.error is still cleared.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b39ceaa1-602c-43e5-be33-b837cf4b031e
📒 Files selected for processing (7)
src/backend/shared/styles/globals.csssrc/frontend/components/_molecules/menu-bar/menus/help.tsxsrc/frontend/store/__tests__/ai-slice.test.tssrc/frontend/store/slices/ai/index.tssrc/frontend/store/slices/ai/slice.tssrc/frontend/store/slices/ai/types.tssrc/frontend/store/slices/index.ts
💤 Files with no reviewable changes (3)
- src/frontend/store/slices/ai/index.ts
- src/frontend/store/slices/index.ts
- src/frontend/store/slices/ai/types.ts
| replaceMessages: (messages) => { | ||
| setState( | ||
| produce(({ ai }: AISlice) => { | ||
| ai.messages = | ||
| messages.length > MAX_CONVERSATION_MESSAGES ? messages.slice(-MAX_CONVERSATION_MESSAGES) : messages | ||
| ai.messages = messages | ||
| ai.error = null | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect replaceMessages call sites with context
rg -n --type=ts --type=tsx -C4 '\breplaceMessages\s*\(' src
# Inspect nearby in-place array mutations that may affect message arrays
rg -n --type=ts --type=tsx -C2 '\.(push|pop|shift|unshift|splice|sort|reverse)\(' src/frontend | rg -n 'message|messages' -iRepository: Autonomy-Logic/openplc-editor
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact implementation around the reviewed lines
echo "==== src/frontend/store/slices/ai/slice.ts (around replaceMessages) ===="
nl -ba src/frontend/store/slices/ai/slice.ts | sed -n '250,340p'
# Find all replaceMessages call sites (ts/tsx only)
echo "==== replaceMessages call sites ===="
rg -n --glob '*.ts' --glob '*.tsx' -C4 '\breplaceMessages\s*\(' src/frontend
# Heuristic: find in-place mutations of things named message/messages nearby in frontend
echo "==== Heuristic: message/messages in-place mutations in src/frontend ===="
rg -n --glob '*.ts' --glob '*.tsx' -C2 '\.(push|pop|shift|unshift|splice|sort|reverse)\(' src/frontend \
| rg -n 'message|messages' -iRepository: Autonomy-Logic/openplc-editor
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== src/frontend/store/slices/ai/slice.ts (around replaceMessages) ===="
cat -n src/frontend/store/slices/ai/slice.ts | sed -n '250,340p'
echo "==== replaceMessages call sites ===="
rg -n --glob '*.ts' --glob '*.tsx' -C4 '\breplaceMessages\s*\(' src/frontend
echo "==== Heuristic: message/messages in-place mutations in src/frontend ===="
rg -n --glob '*.ts' --glob '*.tsx' -C2 '\.(push|pop|shift|unshift|splice|sort|reverse)\(' src/frontend \
| rg -n 'message|messages' -iRepository: Autonomy-Logic/openplc-editor
Length of output: 5433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== replaceMessages usages in whole src (ts/tsx/js/jsx) ===="
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' -C2 '\breplaceMessages\b' src
echo "==== aiActions.replaceMessages call sites in whole src ===="
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' -C4 'aiActions\.replaceMessages\s*\(' src
echo "==== Heuristic: look for caller mutating a 'messages' variable that is passed to replaceMessages ===="
# After a replaceMessages call, see if the file later mutates arrays named messages/messages*
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' -C6 'replaceMessages\s*\(' src \
| rg -n 'push|pop|splice|sort|reverse|unshift|shift' -i
echo "==== Direct search for messages variable mutations in files that mention replaceMessages ===="
files=$(rg -l --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' '\breplaceMessages\b' src || true)
if [ -n "$files" ]; then
echo "$files" | head -n 50
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' -C2 '\.(push|pop|shift|unshift|splice|sort|reverse)\(' $files | rg -n 'message|messages' -i
else
echo "No files found containing replaceMessages"
fiRepository: Autonomy-Logic/openplc-editor
Length of output: 5942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== ChatMessage type (ai slice types.ts) ===="
rg -n --glob '*.ts' 'export type ChatMessage|export interface ChatMessage|type ChatMessage|interface ChatMessage' src/frontend/store/slices/ai/types.ts
echo "---- context ----"
sed -n '1,260p' src/frontend/store/slices/ai/types.ts
echo "==== ai/messages mutation patterns in slice.ts (around line ~120-200) ===="
sed -n '100,220p' src/frontend/store/slices/ai/slice.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== types.ts size ===="
wc -l src/frontend/store/slices/ai/types.ts
echo "==== Search for ChatMessage in types.ts ===="
rg -n 'ChatMessage' src/frontend/store/slices/ai/types.ts
echo "==== types.ts (relevant sections) ===="
cat -n src/frontend/store/slices/ai/types.ts | sed -n '1,260p'
echo "==== slice.ts size ===="
wc -l src/frontend/store/slices/ai/slice.ts
echo "==== slice.ts around messages mutation ===="
cat -n src/frontend/store/slices/ai/slice.ts | sed -n '110,210p'Repository: Autonomy-Logic/openplc-editor
Length of output: 13469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== replaceMessages occurrences in entire repo ===="
rg -n --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' '\breplaceMessages\b' .
echo "==== external direct mutations of ai.messages (set/assign) ===="
rg -n --glob '*.ts' --glob '*.tsx' '\bai\.messages\s*=' src/frontend
echo "==== external in-place array mutations on ai.messages (heuristic) ===="
rg -n --glob '*.ts' --glob '*.tsx' '\bai\.messages\.(push|pop|splice|sort|reverse|shift|unshift)\(' src/frontend
echo "==== mutations of message arrays named messages/message outside slice.ts (heuristic) ===="
rg -n --glob '*.ts' --glob '*.tsx' '\.(push|pop|splice|sort|reverse|shift|unshift)\(' src/frontend \
| rg -n 'message|messages' -i
echo "==== inspect any bracket access 'replaceMessages' usages (heuristic) ===="
rg -n --glob '*.ts' --glob '*.tsx' '\[['"'"]replaceMessages['"'"]\]' src/frontendRepository: Autonomy-Logic/openplc-editor
Length of output: 2031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== replaceMessages( patterns in src/frontend ===="
rg -n --glob '*.ts' --glob '*.tsx' '\breplaceMessages\s*\(' src/frontend
echo "==== .replaceMessages( patterns in src/frontend ===="
rg -n --glob '*.ts' --glob '*.tsx' '\.replaceMessages\s*\(' src/frontend
echo "==== destructured replaceMessages usage patterns ===="
rg -n --glob '*.ts' --glob '*.tsx' '(replaceMessages\s*=\s*|{[^}]*replaceMessages[^}]*})' src/frontendRepository: Autonomy-Logic/openplc-editor
Length of output: 1054
Avoid storing caller-owned messages array by reference in replaceMessages
src/frontend/store/slices/ai/slice.ts sets ai.messages = messages inside Immer, which aliases the caller’s array; if the caller mutates that array later, ai.messages can drift outside the store update flow. Only unit tests currently call this action, but cloning on write keeps store state ownership intact.
Proposed fix
replaceMessages: (messages) => {
setState(
produce(({ ai }: AISlice) => {
- ai.messages = messages
+ ai.messages = [...messages]
ai.error = null
}),
)
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| replaceMessages: (messages) => { | |
| setState( | |
| produce(({ ai }: AISlice) => { | |
| ai.messages = | |
| messages.length > MAX_CONVERSATION_MESSAGES ? messages.slice(-MAX_CONVERSATION_MESSAGES) : messages | |
| ai.messages = messages | |
| ai.error = null | |
| }), | |
| replaceMessages: (messages) => { | |
| setState( | |
| produce(({ ai }: AISlice) => { | |
| ai.messages = [...messages] | |
| ai.error = 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 288 - 293,
replaceMessages currently assigns the caller-owned messages array by reference
(ai.messages = messages), which can cause external mutations to affect store
state; inside the replaceMessages reducer (the setState/produce callback for
AISlice) replace that assignment with a cloned array (e.g., a shallow copy like
[...messages] or messages.slice()) so the store owns its copy of messages and
ai.error is still cleared.
|
Note on the failing This check fails on pre-existing drift, not on anything this PR introduced. The 7 files this PR touches ( Verified by running
The drift is between The web-side equivalent of this check (on PR #412) passed, probably because its checkout fell back to editor's Reconciling the 130 drifted files is a separate cleanup task; this PR is intentionally scoped to the pre-deploy fixes only. Suggest merging on the 7 byte-identical files being correct and tracking the broader sync separately. 🤖 Generated with Claude Code |
1 participant
Summary
Mirror of openplc-web#412 — keeps
src/frontend/**and the sharedsrc/backend/shared/styles/globals.cssbyte-identical between the two IDEs so the Shared Surface Sync CI check stays green.This PR contains only the shared-code changes. The chat UX work, PWA fix, and middleware-adapter changes from the web PR are web-only and not included here.
Files (byte-identical to the web PR)
src/frontend/components/_molecules/menu-bar/menus/help.tsx— Community Support link points atedge.autonomylogic.com/forum.src/frontend/store/slices/ai/types.ts+slice.ts+index.ts+src/frontend/store/slices/index.ts— drop the silent 50-message store cap (MAX_CONVERSATION_MESSAGES).addMessage/replaceMessageskeep every message; the backend's 500/conv cap stays the only ceiling.src/frontend/store/__tests__/ai-slice.test.ts— flip the two cap-related tests to assert no truncation.src/backend/shared/styles/globals.css— add@keyframes spinso Tailwind'sanimate-spinutility actually spins (the project's tailwind config overrodetheme.keyframesinstead of extending it, silently killing the default keyframe and breaking everyanimate-spinuse across the app).Mirror of
https://github.com/Autonomy-Logic/openplc-web/pull/412
Test plan
npm run lint/npm run testpass locally in editoranimate-spinnow actually spins🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
Updates