fix(core): initialize title generation for default memory#1248
fix(core): initialize title generation for default memory#1248
Conversation
🦋 Changeset detectedLatest commit: 9ed9915 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughFixes conversation title generation when memory is provided globally to VoltAgent. Updates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/core/src/memory/manager/memory-manager.ts (2)
791-802: Minor:setMemory(memory)silently clears an existingtitleGenerator.When
memory instanceof MemoryandtitleGeneratoris omitted, Line 800 still writesundefinedover any previously configured generator. The single in-tree caller (Agent.__setDefaultMemory) always passes the generator so this is safe today, but it's a quiet footgun if another caller later wants to swap the backing store without touching title generation. Consider either (a) only updatingtitleGeneratorwhen explicitly provided, or (b) documenting the "pass memory ⇒ also pass generator" contract on the method.♻️ Option (a) — only overwrite when caller opts in
- setMemory(memory: Memory | false, titleGenerator?: ConversationTitleGenerator): void { + setMemory(memory: Memory | false, titleGenerator?: ConversationTitleGenerator): void { if (memory === false) { this.conversationMemory = undefined; this.titleGenerator = undefined; return; } if (memory instanceof Memory) { this.conversationMemory = memory; - this.titleGenerator = titleGenerator; + if (arguments.length >= 2) { + this.titleGenerator = titleGenerator; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/memory/manager/memory-manager.ts` around lines 791 - 802, The setMemory method currently overwrites this.titleGenerator with undefined when called without a generator; change it so that when memory instanceof Memory you always set this.conversationMemory = memory but only assign this.titleGenerator = titleGenerator if a non-undefined titleGenerator argument was passed (i.e., treat titleGenerator as optional). Update references in setMemory (and note callers like Agent.__setDefaultMemory remain compatible) so swapping the Memory store doesn't silently clear an existing ConversationTitleGenerator.
144-163: Optional: align race handling withensureConversationExists.In the duplicate/race branch of
ensureConversationExists(Lines 672–684) we callupdateConversation(conversationId, {})to refreshupdatedAt. The new race branch here just swallows and falls through tosaveMessageWithContext. That's fine in practice (the subsequent message save normally touches the conversation), but mirroring theupdateConversationcall would keep the two ensure-conversation paths behaviorally identical and avoid surprising divergence if a storage adapter ever decouples message writes from conversation timestamp updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/memory/manager/memory-manager.ts` around lines 144 - 163, The duplicate-race branch in createConversation currently swallows the error and falls through, diverging from ensureConversationExists which calls updateConversation(conversationId, {}) to refresh timestamps; modify the catch for createConversation in memory-manager.ts so that when isConversationAlreadyExistsError(createError) is true you call this.updateConversation(conversationId, {}) (or the existing updateConversation method) before continuing to saveMessageWithContext, keeping behavior consistent with ensureConversationExists and ensuring conversation.updatedAt is refreshed even on race.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/memory/manager/memory-manager.ts`:
- Around line 791-802: The setMemory method currently overwrites
this.titleGenerator with undefined when called without a generator; change it so
that when memory instanceof Memory you always set this.conversationMemory =
memory but only assign this.titleGenerator = titleGenerator if a non-undefined
titleGenerator argument was passed (i.e., treat titleGenerator as optional).
Update references in setMemory (and note callers like Agent.__setDefaultMemory
remain compatible) so swapping the Memory store doesn't silently clear an
existing ConversationTitleGenerator.
- Around line 144-163: The duplicate-race branch in createConversation currently
swallows the error and falls through, diverging from ensureConversationExists
which calls updateConversation(conversationId, {}) to refresh timestamps; modify
the catch for createConversation in memory-manager.ts so that when
isConversationAlreadyExistsError(createError) is true you call
this.updateConversation(conversationId, {}) (or the existing updateConversation
method) before continuing to saveMessageWithContext, keeping behavior consistent
with ensureConversationExists and ensuring conversation.updatedAt is refreshed
even on race.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1eb191f2-0c0d-44d6-8302-dcf3af93fa9c
📒 Files selected for processing (4)
.changeset/global-memory-title-generation.mdpackages/core/src/agent/agent.tspackages/core/src/memory/manager/memory-manager.tspackages/core/src/voltagent.spec.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
When
Memoryis passed globally toVoltAgent, preconstructed agents receive it later through__setDefaultMemory. That updated theMemoryManagerconversation memory, but did not initialize the conversation title generator, sogenerateTitlefell back to the default title.What is the new behavior?
__setDefaultMemorynow initializes the title generator from the default memory when assigning it to the agent memory manager. The memory manager also clears the generator when memory is disabled and handles concurrent conversation creation races while saving messages.fixes #1232
Notes for reviewers
Docs were not updated because this restores the documented
generateTitlebehavior for global memory.Validated with:
pnpm vitest run src/voltagent.spec.ts src/memory/manager/memory-manager.spec.ts --reporter=verbosepnpm biome check .changeset/global-memory-title-generation.md packages/core/src/agent/agent.ts packages/core/src/memory/manager/memory-manager.ts packages/core/src/voltagent.spec.tsexamples/basebefore and after the fix; after the fix the title resolves to the generated title instead of the fallback.Summary by cubic
Fixes conversation title generation when global
Memoryis provided toVoltAgent, so preconstructed agents now use generated titles instead of the fallback. Also hardens memory handling when disabled and during concurrent conversation creation. Fixes #1232.__setDefaultMemoryandMemoryManager.setMemory(set on assign, clear on disable).createConversationto avoid failures.Written for commit 9ed9915. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests