Repository navigation
feat(memory): add per-call memory control options (read/write) for ge… #906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1292,6 +1292,56 @@ ${memoryContext} | |
| Current user's request: ${currentInput}`; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether memory should be read (retrieved) for this call. | ||
| * Respects both the global memory SDK config and per-call overrides. | ||
| */ | ||
| private shouldReadMemory( | ||
| perCallMemory: { enabled?: boolean; read?: boolean } | undefined, | ||
| userId: unknown, | ||
| ): boolean { | ||
| if ( | ||
| !this.conversationMemoryConfig?.conversationMemory?.memory?.enabled || | ||
| !userId | ||
| ) { | ||
| return false; | ||
| } | ||
| if (perCallMemory?.enabled === false) { | ||
| return false; | ||
| } | ||
| if (perCallMemory?.read === false) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Determine whether memory should be written (stored) for this call. | ||
| * Respects both the global memory SDK config and per-call overrides. | ||
| */ | ||
| private shouldWriteMemory( | ||
| perCallMemory: { enabled?: boolean; write?: boolean } | undefined, | ||
| userId: unknown, | ||
| content: string | undefined | null, | ||
| ): boolean { | ||
| if ( | ||
| !this.conversationMemoryConfig?.conversationMemory?.memory?.enabled || | ||
| !userId | ||
| ) { | ||
| return false; | ||
| } | ||
| if (!content?.trim()) { | ||
| return false; | ||
| } | ||
| if (perCallMemory?.enabled === false) { | ||
| return false; | ||
| } | ||
| if (perCallMemory?.write === false) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Retrieve condensed memory for a user. | ||
| * Returns the input text enhanced with memory context, or unchanged if no memory. | ||
|
|
@@ -3546,9 +3596,12 @@ Current user's request: ${currentInput}`; | |
| ): void { | ||
| // Memory storage | ||
| if ( | ||
| this.conversationMemoryConfig?.conversationMemory?.memory?.enabled && | ||
| options.context?.userId && | ||
| generateResult.content?.trim() | ||
| this.shouldWriteMemory( | ||
| options.memory, | ||
| options.context?.userId, | ||
| generateResult.content, | ||
| ) && | ||
| options.context?.userId | ||
| ) { | ||
| this.storeMemoryInBackground( | ||
| originalPrompt ?? "", | ||
|
|
@@ -6110,7 +6163,7 @@ Current user's request: ${currentInput}`; | |
|
|
||
| // Memory retrieval | ||
| if ( | ||
| this.conversationMemoryConfig?.conversationMemory?.memory?.enabled && | ||
| this.shouldReadMemory(options.memory, options.context?.userId) && | ||
| options.context?.userId | ||
| ) { | ||
|
Comment on lines
6165
to
6168
|
||
| try { | ||
|
|
@@ -6575,9 +6628,11 @@ Current user's request: ${currentInput}`; | |
| } | ||
|
|
||
| if ( | ||
| this.conversationMemoryConfig?.conversationMemory?.memory?.enabled && | ||
| enhancedOptions.context?.userId && | ||
| accumulatedContent?.trim() | ||
| this.shouldWriteMemory( | ||
| enhancedOptions.memory, | ||
| enhancedOptions.context?.userId, | ||
| accumulatedContent, | ||
| ) | ||
| ) { | ||
| this.storeMemoryInBackground( | ||
| originalPrompt ?? "", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
memory.readis now part of the public per-call API, butgenerate()never performs condensed-memory retrieval (there’s no call path that usesshouldReadMemory()/retrieveMemory()for generate). As a result,GenerateOptions.memory.readhas no effect, and the docs/API contract are misleading. Consider adding the same pre-call retrieval step togenerate()(after auth/requestContext merging socontext.userIdis available) or remove/rename thereadoption for generate if it’s intentionally stream-only.