Fix restored history, vectorization, swipe navigation, and Gallery agents - #4847
Conversation
|
Warning Review limit reached
Next review available in: 6 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request adds Gallery image-agent selection and fixes mobile swipe detection, Memory Recall re-vectorization concurrency, Lorebook vectorization persistence, and sharded chat-history recovery. It adds regression tests, localization strings, scripts, and changelog entries. ChangesChat interactions
Vectorization workflows
Sharded history recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatGalleryDrawer
participant ChatGallery
participant ChatRoleplaySurface
User->>ChatGalleryDrawer: Open Gallery
ChatGalleryDrawer->>ChatGallery: Pass eligible image agents
User->>ChatGallery: Select an image agent
ChatGallery->>ChatRoleplaySurface: Trigger selected agent
ChatRoleplaySurface->>ChatRoleplaySurface: Force image generation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@packages/client/src/components/chat/ChatGallery.tsx`:
- Line 119: Add a ref for the illustrate-agent menu and a document-level
pointerdown handler tied to illustrateMenuOpen that closes the menu when the
event target is outside that menu. Update the menu rendering around the
illustrateMenuOpen state and its handlers, following the existing outside-click
pattern used by floating menus such as those in GameSurface.tsx, while
preserving clicks inside the menu.
In `@packages/client/src/components/chat/ChatGalleryDrawer.tsx`:
- Line 86: Update useCapabilityAgentRegistry to accept an enabled parameter like
useAgentConfigs, use that parameter to disable fetching when false, and pass
open from ChatGalleryDrawer so the registry request is gated consistently with
useInstalledCapabilityPackages and useAgentConfigs.
In `@packages/client/src/components/lorebooks/LorebookEditor.tsx`:
- Line 2878: Update the save/vectorization flow around handleSaveLorebook and
onBeforeVectorize so form mutations cannot occur while the pre-vectorization
save is awaited. Either disable relevant editor inputs for that await or capture
a form revision before onBeforeVectorize and abort vectorization when the
revision changes, ensuring stale settings are not persisted or processed.
In `@packages/server/src/db/file-backed-store.ts`:
- Around line 1245-1251: Validate expectedRowCount before the recovery condition
in the surrounding recovery logic: only allow recovery when the manifest value
for tables.messages is a positive safe integer, rejecting strings, fractions,
NaN, and other malformed values. Preserve the existing zero-shard and
absent-monolith checks, and add a regression covering string or fractional
tables.messages values to ensure stale .pre-shard data is not restored.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55b2a449-2195-4a83-a301-3dd8ace4b31d
📒 Files selected for processing (18)
CHANGELOG.mde2e/core-flows.e2e.tspackage.jsonpackages/client/src/components/chat/ChatArea.tsxpackages/client/src/components/chat/ChatCommonOverlays.tsxpackages/client/src/components/chat/ChatGallery.tsxpackages/client/src/components/chat/ChatGalleryDrawer.tsxpackages/client/src/components/chat/ChatRoleplaySurface.tsxpackages/client/src/components/chat/ChatSettingsDrawer.tsxpackages/client/src/components/chat/ConversationView.tsxpackages/client/src/components/game/GameSurface.tsxpackages/client/src/components/lorebooks/LorebookEditor.tsxpackages/client/src/localization/locales/en.jsonpackages/server/src/db/file-backed-store.tspackages/server/src/routes/lorebooks.routes.tspackages/server/src/services/memory-recall.tsscripts/regressions/memory-recall-revectorize.regression.tsscripts/regressions/message-sharding.regression.ts
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/client/src/components/lorebooks/LorebookEditor.tsx (1)
1496-1502: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not close the editor when
handleSaveLorebookreturnsfalse.At Lines 1855-1858, the save-and-close handler ignores this result. If the request fails or a newer edit occurs during the request,
handleSaveLorebookreturnsfalse, but the handler still closes the editor and discards the unsaved form state.Close the editor only after a successful save. Keep the warning visible when the save fails or becomes stale.
Proposed fix
onClick={async () => { - await handleSaveLorebook(); + const saved = await handleSaveLorebook(); + if (!saved) return; setShowUnsavedWarning(false); closeDetail(); }}🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx` around lines 1496 - 1502, Update the save-and-close handler around handleSaveLorebook so it closes the editor only when that method returns true; preserve the current form and warning state when saving fails or the result is stale.
🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx`:
- Around line 1496-1502: Update the save-and-close handler around
handleSaveLorebook so it closes the editor only when that method returns true;
preserve the current form and warning state when saving fails or the result is
stale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6694c09b-2bd8-44c3-8602-97531ac1a282
📒 Files selected for processing (7)
e2e/core-flows.e2e.tspackages/client/src/components/chat/ChatGallery.tsxpackages/client/src/components/chat/ChatGalleryDrawer.tsxpackages/client/src/components/lorebooks/LorebookEditor.tsxpackages/client/src/hooks/use-capability-packages.tspackages/server/src/db/file-backed-store.tsscripts/regressions/message-sharding.regression.ts
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)
packages/client/src/components/lorebooks/LorebookEditor.tsx (2)
2885-2885: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConfirm Re-vectorize All before saving edits.
onBeforeVectorize()runs before the Re-vectorize All confirmation. If the user cancels the confirmation, the unsaved Lorebook changes have already been persisted. Move the pre-vectorization save after the confirmation block.🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx` at line 2885, Reorder the save flow around the Lorebook editor’s `onBeforeVectorize()` call so the Re-vectorize All confirmation completes before any pre-vectorization save occurs. Ensure cancelling that confirmation exits without persisting unsaved Lorebook changes, while preserving the existing save behavior after confirmation.
1496-1502: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent discard and close from racing with the in-flight save.
Line 1496 checks the form revision only after
updateLorebook.mutateAsynccompletes. It does not cancel the request. While Line 1856 awaits, the discard action and the back button remain active. If the user discards or closes the editor, the pending request can still persist the old form snapshot.Disable close and discard actions while
saving, and serialize or cancel overlapping save requests.Proposed guard
const handleClose = useCallback(() => { + if (saving) return; if (lorebookDirty) { setShowUnsavedWarning(true); } else { closeDetail(); } -}, [lorebookDirty, closeDetail]); +}, [lorebookDirty, saving, closeDetail]); <button onClick={() => { setShowUnsavedWarning(false); setLorebookDirty(false); closeDetail(); }} + disabled={saving} >Also applies to: 1856-1857
🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx` around lines 1496 - 1502, Update the save flow around updateLorebook.mutateAsync and the close/discard handlers near the save call to prevent races: disable back, close, and discard actions whenever saving is true, and serialize or cancel overlapping save requests so an in-flight save cannot persist a stale form snapshot after the editor is closed or discarded. Preserve the existing formRevision check and success/error behavior.
🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx`:
- Line 2885: Reorder the save flow around the Lorebook editor’s
`onBeforeVectorize()` call so the Re-vectorize All confirmation completes before
any pre-vectorization save occurs. Ensure cancelling that confirmation exits
without persisting unsaved Lorebook changes, while preserving the existing save
behavior after confirmation.
- Around line 1496-1502: Update the save flow around updateLorebook.mutateAsync
and the close/discard handlers near the save call to prevent races: disable
back, close, and discard actions whenever saving is true, and serialize or
cancel overlapping save requests so an in-flight save cannot persist a stale
form snapshot after the editor is closed or discarded. Preserve the existing
formRevision check and success/error behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f827a12a-2c17-46fe-a53b-1795460e8ee6
📒 Files selected for processing (2)
e2e/core-flows.e2e.tspackages/client/src/components/lorebooks/LorebookEditor.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/core-flows.e2e.ts (1)
12545-12553: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that cancellation does not start vectorization.
The test checks only
saveRequestCount. An implementation can call/vectorizewhen the dialog opens, and this test still passes. RecordvectorizeRequestCountbefore the click and assert that it is unchanged after cancellation.Proposed test assertion
const saveCountBeforeCancel = saveRequestCount; + const vectorizeCountBeforeCancel = vectorizeRequestCount; await revectorizeButton.click(); const revectorizeDialog = page.getByRole("dialog").filter({ hasText: "Re-vectorize All Entries" }); await expect(revectorizeDialog).toBeVisible(); await revectorizeDialog.getByRole("button", { name: "Cancel", exact: true }).click(); expect(saveRequestCount).toBe(saveCountBeforeCancel); + expect(vectorizeRequestCount).toBe(vectorizeCountBeforeCancel);🤖 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 `@e2e/core-flows.e2e.ts` around lines 12545 - 12553, Update the cancellation test around the lorebook dialog flow to capture the current vectorizeRequestCount before clicking cancel, then assert it remains unchanged afterward. Keep the existing saveRequestCount assertion and use the existing vectorizeRequestCount tracking symbol.
🤖 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 `@packages/client/src/components/lorebooks/LorebookEditor.tsx`:
- Around line 2909-2911: Update handleVectorize to use an in-flight guard set
before showConfirmDialog, preventing concurrent vectorization confirmation flows
from starting. Keep the vectorization request blocked while the shared
onBeforeVectorize/handleSaveLorebook save is pending, and clear the guard in a
finally block so later attempts can proceed.
---
Outside diff comments:
In `@e2e/core-flows.e2e.ts`:
- Around line 12545-12553: Update the cancellation test around the lorebook
dialog flow to capture the current vectorizeRequestCount before clicking cancel,
then assert it remains unchanged afterward. Keep the existing saveRequestCount
assertion and use the existing vectorizeRequestCount tracking symbol.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a2e511a3-ddae-4433-9391-7eb2af0b766a
📒 Files selected for processing (2)
e2e/core-flows.e2e.tspackages/client/src/components/lorebooks/LorebookEditor.tsx
|
@coderabbitai review |
|
|
@coderabbitai review |
|
1 participant
Why
Five assigned reports expose separate but user-visible dead ends: Conversation swipe navigation watches the wrong scroll surface; re-vectorization can race background memory work; Lorebook vectorization can execute before pending settings are saved and then report a misleading zero result; a restored profile can retain its pre-shard message backup while exposing no history; and custom image agents cannot be launched from the Gallery.
Scope
Changes
Validation
Risk notes
The restored-history repair only recovers a preserved pre-shard backup when the current manifest expects rows and no shard data exists; it never merges ambiguous partial histories. Memory work is serialized per chat and releases its queue entry after completion. Gallery preserves its existing one-click behavior whenever no eligible custom image agent is active.
Summary by CodeRabbit