refactor(memory): Refactor memories to use ID-based persistence and new memories UI#8294
Merged
Subash-Mohan merged 16 commits intomainfrom Feb 11, 2026
Merged
refactor(memory): Refactor memories to use ID-based persistence and new memories UI#8294Subash-Mohan merged 16 commits intomainfrom
Subash-Mohan merged 16 commits intomainfrom
Conversation
…dal handlingfeat: refactor Memories component to use modal context and improve modal handling
… disable resizing
…r personalization logic
…r in MemoriesModal
…light functionality in MemoriesModal
…ogic in MemoriesModal
Contributor
Greptile OverviewGreptile SummaryThis PR refactors user memories from a delete-and-recreate flow to an ID-based upsert model, updating the backend personalization schemas/APIs and the frontend types accordingly. On the web side, the inline settings memories editor is replaced with a new The main correctness issue found is in the new modal list rendering: the mapped fragments are missing keys, which will produce React warnings and can lead to incorrect UI/state association when adding/removing memories. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
autonumber
participant UI as SettingsPage/Memories UI
participant Modal as MemoriesModal
participant Hook as useMemoryManager
participant Pers as useUserPersonalization
participant API as manage/users API
participant DB as update_user_personalization (DB)
UI->>Modal: open (View/Add or skeleton LineItem)
UI->>Modal: pass memories + onSaveMemories
Modal->>Hook: init localMemories from props
Modal->>Hook: handleAddMemory()
Hook-->>Modal: returns new local id
Modal->>Modal: setFocusMemoryId(newId)
Modal->>Hook: handleUpdateMemory(index, value)
Modal->>Hook: handleBlurMemory(index)
Hook->>Pers: onSaveMemories(newMemories)
Pers->>API: PATCH /manage/user/personalization (memories: [{id, content}])
API->>DB: update_user_personalization(user_id, memories)
DB->>DB: delete missing ids
DB->>DB: update matching ids
DB->>DB: insert items with id=None
DB-->>API: commit
API-->>Pers: success
Pers-->>Hook: success boolean
Hook-->>Modal: notify popup
Modal->>Hook: handleRemoveMemory(index)
Hook->>Pers: onSaveMemories(without removed)
Pers->>API: PATCH personalization
API->>DB: upsert/delete
DB-->>API: commit
API-->>Pers: success
Pers-->>Hook: success boolean
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+203
to
+207
| {filteredMemories.map(({ memory, originalIndex }) => ( | ||
| <> | ||
| <MemoryItem | ||
| key={memory.id} | ||
| memory={memory} |
Contributor
There was a problem hiding this comment.
Missing keys on fragments
filteredMemories.map(...) returns a fragment (<>...</>) but the key is on MemoryItem instead of the fragment. React will warn and can mis-associate state when inserting/removing memories. Put the key on the fragment (or wrap in a keyed element) so the list item identity is stable.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 18 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="web/src/app/css/line-item.css">
<violation number="1" location="web/src/app/css/line-item.css:123">
P2: Add a `[data-transient="true"]` rule so programmatic hover matches the hover background for the skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.) [FEEDBACK_USED]</violation>
<violation number="2" location="web/src/app/css/line-item.css:131">
P2: Add a `[data-transient="true"]` rule so programmatic hover matches the hover background for the emphasized skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.) [FEEDBACK_USED]</violation>
</file>
<file name="web/src/components/settings/Memories.tsx">
<violation number="1" location="web/src/components/settings/Memories.tsx:28">
P2: Clear targetMemoryId when opening the modal without a specific memory target; otherwise the modal can keep highlighting the previously selected memory when users click “View/Add” or the empty-state action.</violation>
</file>
<file name="web/src/hooks/useMemoryManager.ts">
<violation number="1" location="web/src/hooks/useMemoryManager.ts:32">
P2: Memories with `id: null` are treated as existing (`isNew: false`), so saves will send the negative placeholder ID instead of null. Mark null IDs as new so persistence keeps `id: null`.</violation>
</file>
<file name="web/src/refresh-components/modals/MemoriesModal.tsx">
<violation number="1" location="web/src/refresh-components/modals/MemoriesModal.tsx:204">
P3: Add a key to the fragment returned by the map; the key on MemoryItem doesn’t cover the fragment wrapper, so React will warn and may mis-handle list reconciliation when the Separator is present.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| .line-item-button-skeleton-emphasized { | ||
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-02; |
Contributor
There was a problem hiding this comment.
P2: Add a [data-transient="true"] rule so programmatic hover matches the hover background for the emphasized skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/src/app/css/line-item.css, line 131:
<comment>Add a `[data-transient="true"]` rule so programmatic hover matches the hover background for the emphasized skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.) </comment>
<file context>
@@ -118,6 +118,32 @@
+}
+
+.line-item-button-skeleton-emphasized {
+ @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-02;
+
+ [data-keyboard-nav="true"] &:hover {
</file context>
Suggested change
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-02; | |
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-02; | |
| &[data-transient="true"] { | |
| @apply bg-background-tint-02; | |
| } |
|
|
||
| /* Skeleton Variant - dashed border placeholder style */ | ||
| .line-item-button-skeleton { | ||
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-01; |
Contributor
There was a problem hiding this comment.
P2: Add a [data-transient="true"] rule so programmatic hover matches the hover background for the skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/src/app/css/line-item.css, line 123:
<comment>Add a `[data-transient="true"]` rule so programmatic hover matches the hover background for the skeleton variant.
(Based on your team's feedback about treating [data-transient="true"] as hover.) </comment>
<file context>
@@ -118,6 +118,32 @@
+/* Skeleton Variant - dashed border placeholder style */
+.line-item-button-skeleton {
+ @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-01;
+
+ [data-keyboard-nav="true"] &:hover {
</file context>
Suggested change
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-01; | |
| @apply bg-transparent border border-dashed border-border-01 hover:bg-background-tint-01; | |
| &[data-transient="true"] { | |
| @apply bg-background-tint-01; | |
| } |
…vent duplicate saves
…open and updating new memory detection logic
raunakab
approved these changes
Feb 11, 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.
Description
Memoriescomponent that displays memories asFileTilepreviews and opens a fullMemoriesModalfor viewing, searching, adding, and editinguseMemoryManagerhookFileTile,ButtonTile,SvgTextLinesicon, andskeletonvariant forLineItemsmallvariant toIconButtonandresizableprop toInputTextAreaScreen.Recording.2026-02-10.at.5.47.17.PM.mov
Screen.Recording.2026-02-10.at.6.02.04.PM.mov
How Has This Been Tested?
Tested by adding, updating and deleting memories.
Additional Options
Summary by cubic
Refactored memory persistence to ID-based upsert and introduced a new Memories UI with a modal, improving auto-save behavior and preventing duplicate saves. Enforces a 10-memory limit with server-side validation and UI guards; orders by newest and resets focus/highlight on open for a smoother add/edit/search experience.
New Features
Migration
Written for commit fe948c9. Summary will update on new commits.