fix(memory): migrate legacy memories_captured v2 scopes - #1668
fix(memory): migrate legacy memories_captured v2 scopes#1668sentry[bot] wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e26e755. Configure here.
| .object({ | ||
| ...capturedMemoryFields, | ||
| scope: z.enum(MEMORY_SCOPES), | ||
| scope: z.preprocess((val) => currentScope(val as any), z.enum(MEMORY_SCOPES)), |
There was a problem hiding this comment.
Missing compatibility removal TODO
Low Severity
The new z.preprocess path tolerates legacy personal/conversation scopes at parse time, but there is no TODO(vX.Y.Z) naming that legacy state. Repo comment policy requires a dated removal marker on transitional compatibility branches.
Reviewed by Cursor Bugbot for commit e26e755. Configure here.
![sentry-junior[bot]](https://avatars.githubusercontent.com/in/2960336?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good to me. Coercing legacy personal/conversation scopes on the v2 memories_captured schema is the right read-path fix for residual pre-migration rows, and currentScope already matches the rename. I updated the events unit test that still expected the old Zod throw so it locks in the coercion instead.
Update the events regression test to assert coerce-on-read instead of expecting a Zod throw, and drop the preprocess any cast.
There was a problem hiding this comment.
Looks good to me. Coercing legacy personal/conversation on the v2 memories_captured parse path is the right fix for residual pre-migration rows, and the events test now locks both directions of the rename. Follow-up on the branch drops the preprocess any cast while keeping currentScope typed for render.
Co-Authored-By: David Cramer <david@sentry.io>
There was a problem hiding this comment.
Looks good to me. Coercing legacy personal/conversation on the v2 memories_captured parse path is the right residual-row fix alongside the #1667 SQL rewrite, and the events test now locks both directions of the mapping plus render metadata.
|
id prefer we somehow migrate these at the schema level, or the serializer for v1 does a coerce or something? it'd be a lot easier to just have versioned schemas that coerce to a shared final schema |
1 participant
Conversation history can still contain
memories_capturedv2 rows with the old scope labelspersonal/conversation.After the scope rename, the v2 schema only accepts
private/public, so reading those rows throws aZodErrorand breaks conversation loads (JUNIOR-8Q).#1667 already rewrites stored v2 rows in SQL. This PR adds coerce-on-read in
capturedMemorySchemaso leftover or unmigrated rows still parse, using the existingcurrentScopemapping:personal→privateconversation→publicRegression coverage now asserts successful parse + render instead of expecting the Zod throw.
Requested by David Cramer.
--
View Junior Session [Sentry]