Repository navigation
fix(memory)!: wire contextTokenBudget and autoExtractEntities, remove autoRecordMessages and consolidationEnabled (closes #1579) - #1827
Merged
Conversation
…autoRecordMessages and consolidationEnabled (closes #1579) MemoryManagerConfig had four options no manager read. - contextTokenBudget is now the budget getRelevantContext uses when a call passes no maxTokens, through MemoryManager.contextTokenBudget (2000 by default; an explicit maxTokens still wins). - autoExtractEntities makes recordMessage and recordConversation run extractEntities over each user and assistant message, best-effort. - autoRecordMessages and consolidationEnabled are removed with the 5-parameter overloads: a manager cannot record messages it is not given, and automatic consolidation has no trigger. The pendingUntilFixed contextTokenBudget test in MemoryGuideSpec is promoted; the guide, README, CHANGELOG and migration guide match. Signed-off-by: kannupriyakalra <kannupriyakalra@gmail.com>
…nfig-1579 Signed-off-by: kannupriyakalra <kannupriyakalra@gmail.com> # Conflicts: # CHANGELOG.md
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Signed-off-by: kannupriyakalra <kannupriyakalra@gmail.com>
Signed-off-by: kannupriyakalra <kannupriyakalra@gmail.com>
Signed-off-by: kannupriyakalra <kannupriyakalra@gmail.com>
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
1 participant
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.
Summary
Closes #1579
MemoryManagerConfighad six options, and onlydefaultImportance(plusconsolidationConfig) was read by a manager. This PR decides each of the four dead options: two are wired and tested, two are removed with a migration note. Every option left onMemoryManagerConfignow changes whatSimpleMemoryManager/LLMMemoryManagerdo.Decisions, per option
contextTokenBudgetgetRelevantContextalready takes amaxTokens; letting a manager carry its budget means code that only has aMemoryManager(an agent loop, a prompt builder) gets the operator's budget without threading an int through. The trait'smaxTokensdefault is nowcontextTokenBudget, a new concrete member onMemoryManager(defaultMemoryManager.DefaultContextTokenBudget= 2000, so existing third-party implementations compile and behave as before); the shared managers override it from their config. An explicitmaxTokensstill wins. Default stays 2000, so nobody's output changes unless they set it.autoExtractEntitiesrecordMessage(and sorecordConversation) callsextractEntities(content, Some(conversationId))on each user and assistant message after recording it. System prompts and tool output are not analysed (instructions and raw JSON are not facts about the world, and tool output can be large). Extraction is best-effort: the message is already recorded, so a failed LLM call is logged as a warning and the manager holding the message is returned, rather than reporting a failure for a record that succeeded (and, for SQLite, is already persisted). It stays opt-in (false); onLLMMemoryManagerit costs one LLM call per message, onSimpleMemoryManagerit does nothing because that manager'sextractEntitiesextracts nothing (documented).autoRecordMessagesrecordMessage/recordConversation; there is no message stream for it to record automatically, andllm4s-memorydoes not depend onllm4s-agent(nor should it). Its default wastrue, which told readers recording was automatic when it never was.consolidationEnabledrecord*; none of that exists, and designing it is a feature of its own.consolidateMemoriesalready runs when called and readsconsolidationConfig, which stays.llm4s-memoryis Beta and has no MiMa baseline, so removal now is cheaper than freezing two no-op fields into 0.5.0. With the two fields gone, the "binary-compatible 5-parameter" constructor /apply/copyoverloads (which took them) are removed as well.What changed
MemoryManager.scala:MemoryManagerConfig(autoExtractEntities, defaultImportance, contextTokenBudget, consolidationConfig); Scaladoc states what each option does;MemoryManager.contextTokenBudget+MemoryManager.DefaultContextTokenBudget;getRelevantContext(query, maxTokens = contextTokenBudget, ...);ConsolidationConfigScaladoc no longer refers toconsolidationEnabled;fullFeatured=autoExtractEntities = true.BaseMemoryManagerOps.scala:contextTokenBudgetfrom config; best-effort extraction after recording user/assistant messages.MemoryManagerConfigBehaviourSpec;MemoryManagerConfigBinaryCompatSpecreplaced byMemoryManagerConfigSpec(value semantics of the new shape); thependingUntilFixedtest inMemoryGuideSpecis promoted to a normal test (with a positive control), plus a test for the new guide snippet.docs/guide/agents/memory.md(options table, recording / consolidation / extraction / budget paragraphs),modules/memory/README.md,docs/migrations/0-4-1-to-0-5-0.md, CHANGELOG (Added + Removed with migration).Acceptance criteria → tests
contextTokenBudgetchanges behaviourMemoryManagerConfigBehaviourSpec: "contextTokenBudget should bound getRelevantContext when the call passes no maxTokens", "let the memory in when the configured budget has room for it", "default to 2000 tokens", "give way to an explicit maxTokens, in either direction", "be kept by the manager returned from a record call", "be honoured by LLMMemoryManager"; "MemoryManager.contextTokenBudget should default to 2000 for an implementation that does not override it"autoExtractEntitieschanges behaviourMemoryManagerConfigBehaviourSpec: "extract entities from a recorded user message when true", "make no LLM call and store no entity when false", "analyse user and assistant messages of a recorded conversation, but not system or tool messages", "keep the recorded message when extraction fails", "have no effect on SimpleMemoryManager"autoRecordMessages,consolidationEnabledremoved with a CHANGELOG migration note### Removedentry + migration guide;MemoryManagerConfigSpecpins the remaining options (defaults,apply,copy,equals/hashCodeover every option,toString)pendingUntilFixedtest from #1577 promoted or deletedMemoryGuideSpec"keep the context within contextTokenBudget when the call passes no maxTokens" (now a normal test)MemoryGuideSpec"build the configuration of the Configuration Options block"; guide/README text updatedEvidence the tests fail without the fix
With the wiring disabled in
BaseMemoryManagerOps(extraction guarded byfalse,contextTokenBudgetreturning the constant 2000) and the tests unchanged:(On
mainitself the new tests do not compile: there is noMemoryManager.contextTokenBudget, and the promoted guide test waspendingUntilFixedbecause it failed.)Checks run locally
memory/test— 1645 passed, 0 failedmemoryPostgres/Test/compile,samples/Test/compile,samples/testOnly *ComparisonGuideSpec *UpgradeGuideSpec— passmemory/scalafmtCheckAll,scalafmtSbtCheck,memory/scalafixAll --check,coveragePolicyCheck,memory/doc— passcoverage memory/test memory/coverageReport— 91.38% statements (floor 80)scripts/check-doc-links.sh,check-doc-nav-order.sh,check-doc-support.sh,check-doc-versions.sh,check-docs-site-links.sh— passBehaviour change / migration
MemoryManagerConfig.autoRecordMessages,consolidationEnabled, and the 5-parameter constructor/apply/copyoverloads. Drop the arguments; record turns withrecordMessage/recordConversationand callconsolidateMemoriesyourself. Positional calls shift to(autoExtractEntities, defaultImportance, contextTokenBudget, consolidationConfig).getRelevantContextwithoutmaxTokensnow uses the configuredcontextTokenBudget(default unchanged at 2000).autoExtractEntities = true(andMemoryManagerConfig.fullFeatured) now makesLLMMemoryManagercall the LLM once per recorded user/assistant message.