Repository navigation
feat(config): default to no short-term or semantic memory, and infer the backend - #1580
Merged
Merged
Conversation
…the backend A deployment that runs the server on its own, with a hand-written config and no chart, gets whatever the code defaults are. Those defaults assumed a full installation: short-term memory on, semantic memory on, and long-term memory on Neo4j. A lab running MemMachine for retrieval alone had to turn two things off and switch a third before it matched what it wanted. Short-term memory is now off unless asked for. Two changes were needed, not one: the field default, and the merge, which read True whenever a short_term_memory section existed at all - so the field default never applied to any real config. Configuring the block and enabling it are now separate statements. Semantic memory is off by default. Note this only decides the fully-configured case: _auto_disable_when_incomplete already forced it off whenever llm_model, embedding_model or the storage fields were missing, so the old default of True only took effect for a deployment that had wired all of them up. The long-term backend is now inferred from which fields a config fills in rather than defaulting. The two backends share no field names, so naming one is an unambiguous statement: a config pointing at a `vector_store` cannot mean the declarative backend, which has no such field. The discriminator itself is untouched, deliberately. Flipping `None -> declarative` would repoint every pre-discriminator config at Qdrant, against data sitting in Neo4j - and those configs name a `vector_graph_store`, so inference resolves them exactly as before. test_old_param_data_without_backend_ loads_as_declarative still passes unchanged. Defaulting to event when nothing is named would not work either: such a config has no store ids, so the event backend cannot start. Two shipped configs relied on the old defaults and now state what they want, so that this change does not quietly alter them: locomo_config.yaml ran with short-term memory on, and deployments/helm configures both memories and would otherwise have shipped an llm_model and message_capacity that were never used. Verified: 1964 tests pass. Seven new tests cover the changed behaviour, and each was checked against the old code to confirm it fails there - which caught one that did not. Asserting the semantic default on a bare instance passes whatever the default is, because the auto-disable fires first; the test now supplies every required field so that nothing but the default decides. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Nr9kacmpFVTTfkZRw6esxP
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 9, 2026
…tup (speedkick) `semantic_memory.enabled: false` only skipped the semantic service in start(). Every request path that could name the semantic type still resolved the semantic stack lazily and used it: add_episodes, query_search and list_search branched on target_memories alone, and the SemanticManager getters built storage, config store, embedder and language model on first use without reading the flag (running Alembic migrations on the request path for pgvector). The flag changed who paid to bring the stack up, not whether it came up. With a complete config the request succeeded and did real work; with the incomplete config that MemMachine#1580 now produces by default, get_semantic_storage raised from inside the request. Deletion had the same gap with a twist. memmachine.py defined _cleanup_semantic_history twice; both arrived in MemMachine#1151. The later definition, which requests the semantic service unconditionally, shadowed the earlier one that caught ResourceNotReadyError, so with semantic memory disabled every project deletion raised in the worker, dropped the job, and left the session in `delete` status for good (MemMachine#1575). delete_episodes requested the service unconditionally too. Three changes: - ResourceManagerImpl.get_semantic_manager raises ResourceNotReadyError("Semantic memory is disabled.") when the flag is off, so nothing semantic is constructed; get_semantic_service and get_semantic_session_manager go through it. The flag is read only on the server side: MemMachine routes requests, and the composition root refuses to build a component the deployment does not have. SemanticResourceManager and everything below it never see it. The semantic-only MemMachine operations (features, set types, ...) now fail with that message instead of building a stack the operator turned off. - MemMachine gains enabled_memory_types, read from the two configuration flags, and add_episodes, query_search and list_search filter target_memories through it. A request naming a disabled type gets None for that field, exactly as when it did not name the type. Skipping rather than rejecting is forced by the client: memmachine_client sends both types on every add and search, so rejecting would fail every client call against the default configuration. Episodic gets the same treatment, so a semantic-only deployment no longer stores the episode and then raises "Episodic memory is disabled" on the same request. The MCP tools keep passing ALL_MEMORY_TYPES, which now means every type the server has. - The shadowed _cleanup_semantic_history is removed; the surviving one is called only when semantic memory is enabled, matching the guard _delete_queued_session already applied to the semantic delete itself, and delete_episodes resolves the semantic service under the same guard. The integration conftest built a complete SemanticMemoryConf without saying enabled=True and relied on the lazy build; it now says it. The batched deletion test flips semantic memory on so its per-batch cleanup assertion keeps meaning something. Fixes MemMachine#1575 Fixes MemMachine#1600 Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_018nSC7JKXByD1WADJukvupg
edwinyyyu
added a commit
to edwinyyyu/MemMachine
that referenced
this pull request
Sep 9, 2026
…tup (speedkick) `semantic_memory.enabled: false` only skipped the semantic service in start(). Every request path that could name the semantic type still resolved the semantic stack lazily and used it: add_episodes, query_search and list_search branched on target_memories alone, and the SemanticManager getters built storage, config store, embedder and language model on first use without reading the flag (running Alembic migrations on the request path for pgvector). The flag changed who paid to bring the stack up, not whether it came up. With a complete config the request succeeded and did real work; with the incomplete config that MemMachine#1580 now produces by default, get_semantic_storage raised from inside the request. Deletion had the same gap with a twist. memmachine.py defined _cleanup_semantic_history twice; both arrived in MemMachine#1151. The later definition, which requests the semantic service unconditionally, shadowed the earlier one that caught ResourceNotReadyError, so with semantic memory disabled every project deletion raised in the worker, dropped the job, and left the session in `delete` status for good (MemMachine#1575). delete_episodes requested the service unconditionally too. Three changes: - ResourceManagerImpl.get_semantic_manager raises ResourceNotReadyError("Semantic memory is disabled.") when the flag is off, so nothing semantic is constructed; get_semantic_service and get_semantic_session_manager go through it. The flag is read only on the server side: MemMachine routes requests, and the composition root refuses to build a component the deployment does not have. SemanticResourceManager and everything below it never see it. The semantic-only MemMachine operations (features, set types, ...) now fail with that message instead of building a stack the operator turned off. - MemMachine gains enabled_memory_types, read from the two configuration flags, and add_episodes, query_search and list_search filter target_memories through it. A request naming a disabled type gets None for that field, exactly as when it did not name the type. Skipping rather than rejecting is forced by the client: memmachine_client sends both types on every add and search, so rejecting would fail every client call against the default configuration. Episodic gets the same treatment, so a semantic-only deployment no longer stores the episode and then raises "Episodic memory is disabled" on the same request. The MCP tools keep passing ALL_MEMORY_TYPES, which now means every type the server has. - The shadowed _cleanup_semantic_history is removed; the surviving one is called only when semantic memory is enabled, matching the guard _delete_queued_session already applied to the semantic delete itself, and delete_episodes resolves the semantic service under the same guard. - Two more mixed paths reached the semantic stack unconditionally and would have started failing on a disabled deployment once the composition root refused: the MCP delete tool's _delete_memories called delete_features even with no semantic uids named, and delete_all asked for the semantic manager before touching the episode store. The first now calls delete_features only when semantic uids were named; the second touches the semantic stores only when semantic memory is enabled. The integration conftest built a complete SemanticMemoryConf without saying enabled=True and relied on the lazy build; it now says it. The batched deletion test flips semantic memory on so its per-batch cleanup assertion keeps meaning something. Fixes MemMachine#1575 Fixes MemMachine#1600 Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_018nSC7JKXByD1WADJukvupg
9 tasks done
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
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.
A deployment that runs the server on its own — hand-written config, no chart,
no Kubernetes — gets whatever the code defaults are. Those defaults assumed a
full installation: short-term memory on, semantic memory on, long-term memory on
Neo4j. A lab wanting MemMachine for retrieval alone had to turn two things off
and switch a third before it matched what it wanted.
Short-term memory: off unless asked for
Two changes were needed, not one. The field default was
True, but the mergealso read
Truewhenever ashort_term_memorysection existed at all:so the field default never applied to any real config. Configuring the block and
enabling it are now separate statements.
Semantic memory: off by default
With a caveat worth stating:
_auto_disable_when_incompletealready forced itoff whenever
llm_model,embedding_modelor the storage fields were missing.The old default of
Truetherefore only took effect for a deployment that hadwired all of them up — which is also why the first version of the test for this
was vacuous (see Verification).
Long-term backend: inferred, not defaulted
The two backends share no field names, so naming one is an unambiguous
statement of intent — a config pointing at a
vector_storecannot mean thedeclarative backend, which has no such field.
vector_store/segment_storevector_graph_storebackend:set explicitlyWhat this deliberately does not do
The discriminator is untouched. Flipping
None -> declarativewould repointevery pre-discriminator config at Qdrant, against data sitting in Neo4j. Those
configs name a
vector_graph_store, so inference resolves them exactly asbefore, and
test_old_param_data_without_backend_loads_as_declarativepassesunchanged.
Defaulting to event when nothing is named would not work either — such a
config has no store ids, so the event backend cannot start. Defaulting there
just moves the failure.
Configs that relied on the old defaults
Both now state what they want, so this change does not quietly alter them:
evaluation/episodic_memory/locomo_config.yamlran with short-term memory ondeployments/helmconfigures both memories, and would otherwise have shippedan
llm_modelandmessage_capacitythat were never usedVerification
1964 tests pass, ruff and format clean.
Seven new tests cover the changed behaviour, and each was run against the old
code to confirm it fails there — which caught one that did not. Asserting the
semantic default on a bare
SemanticMemoryConfpasses whatever the default is,because the auto-disable fires first. That test now supplies every required
field so that nothing but the default decides the answer.
Who this affects
Anyone whose config omits these settings — the standalone case this is for.
Deployments using the platform chart are unaffected: it writes
backend: event,short_term_memory_enabledandsemantic_memory.enabledexplicitly, and anexplicit setting always beats a default.
This is a behaviour change on upgrade for standalone users who relied on the
old defaults, which is the main thing to weigh. Worth noting for semantic memory
in particular: turning it back on later hands the whole accumulated backlog to
process_set_idsin one call.