fix(memory): preserve semantically similar memories (#2303)

## Description

Prevent memory_save from automatically deleting semantically similar but
distinct memories. The previous fire-and-forget deduplication path
deleted existing memories at cosine similarity scores of 0.92 or higher
after the save had already returned success. Similarity remains
available as a consolidation hint, while supersession now requires an
explicit memory_update or memory_delete operation.

  ## Type of Change

  - [x] Bug fix (non-breaking change that fixes an issue)
  - [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
  - [ ] Documentation update
  - [ ] Performance improvement
  - [ ] Code refactoring (no functional changes)

  ## Changes Made

  - Removed the automatic background deletion scheduled by memory_save.
- Removed the automatic-dedup threshold and background coroutine that
were no longer needed.
  - Preserved the existing similarity search and consolidation hint.
  - Kept explicit memory_update and memory_delete behavior unchanged.
- Added a regression test proving that distinct memories survive even at
0.99 simulated similarity.
  - Added an Unreleased changelog entry.

  ## Testing

  - [x] Unit tests pass (pytest)
  - [x] Linting passes (ruff check .)
  - [x] Type checking passes (mypy headroom)
  - [x] New tests added for new functionality
  - [x] Manual testing performed

  ### Test Output

$ uv run --extra dev --frozen pytest
tests/test_memory_handler_native_ops.py
  33 passed

  $ uv run --extra dev --frozen ruff check .
  All checks passed!

$ uv run --extra dev --frozen ruff format --check
headroom/proxy/memory_handler.py tests/test_memory_handler_native_ops.py
  2 files already formatted

  $ uv run --extra dev --frozen mypy headroom --ignore-missing-imports
  Success: no issues found in 504 source files

  $ uv run --extra dev --frozen pytest
  9361 passed, 565 skipped, 4 failed

The four full-suite failures are unrelated to this diff: the Anthropic
compaction test passed in isolation; the Codex recovery test exceeded
the macOS AF_UNIX path limit; the dashboard test expects text absent
from the existing implementation; and the content-router test expects a
  fallback absent from the existing strategy chain.

The repository-wide format check also flags pre-existing formatting in
the untouched headroom/proxy/handlers/anthropic.py.

  ## Real Behavior Proof

- Environment: macOS on Apple Silicon, CPython 3.12.13, real
LocalBackend, temporary SQLite database, and the local
sentence-transformers
    embedding backend; no external provider or model API.

- Exact command / steps: Ran uv run --extra dev --frozen python with a
temporary database, saved User's primary backend framework at work is
FastAPI., queried its similarity to User's primary backend framework at
home is FastAPI., saved the second fact through
    MemoryHandler._execute_save, and listed the user's memories.

- Observed result: The real embedding similarity was 0.9387, above the
former 0.92 deletion threshold. The second save returned saved,
included the consolidation hint, retained the original memory, and left
both distinct facts in the database (memory_count: 2).

- Not tested: Live OpenAI or Anthropic provider calls, a deployed proxy
or MCP client session, and Qdrant or Neo4j memory backends. These
paths share the handler policy changed here; backend-specific explicit
update and delete behavior is unchanged.

  ## Review Readiness

  - [x] I have performed a self-review
  - [x] This PR is ready for human review

  ## Checklist

  - [x] My code follows the project's style guidelines
  - [x] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have made corresponding changes to the documentation
  - [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
  - [ ] New and existing unit tests pass locally with my changes
  - [x] I have updated the CHANGELOG.md if applicable

  ## Additional Notes

The documentation and code-comment checklist items are not applicable
because this change removes unsafe behavior without introducing a new
public interface or complex implementation. The full-suite checkbox
remains unchecked because four unrelated tests failed locally, as
  documented above.
This commit is contained in:
Gautam Sharma 2026-07-17 00:02:38 +05:30 committed by GitHub
parent 26b43f64d6
commit 5279c33b19
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 36 additions and 81 deletions

View file

@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **install:** `headroom install apply --env KEY=VALUE` (repeatable) passes environment variables into supervised runners (macOS launchd, Linux systemd/cron, Windows services/tasks). These runners previously started with a bare environment and did not inherit the interactive shell's exports — e.g. a custom `HEADROOM_WORKSPACE_DIR` never reached the supervised process, so `headroom install agent run` looked for its manifest in the wrong location and failed outright even though `install apply` itself succeeded. `--env` values are merged into `DeploymentManifest.base_env` last, so they can override auto-derived defaults, and are threaded into the generated `run-headroom.sh`/`ensure-headroom.sh` (and Windows equivalents) as `export`/`$env:` lines before the `exec`.
### Fixed
- **memory:** preserve semantically similar memories after `memory_save`. Cosine similarity now produces a consolidation hint only; it no longer schedules a background deletion, because related memories can describe distinct facts. Supersession remains available through the explicit `memory_update` path with a caller-supplied memory ID.
- **mcp:** reap orphaned `headroom mcp serve` processes when the launching client dies. An MCP stdio server relies on stdin EOF to shut down, but an abrupt client `SIGKILL` leaves the SDK's blocking stdin-reader thread wedged, so `server.run()` never returns; the process is reparented to init/launchd (`ppid == 1`) and lingers, pinning one Python interpreter + tree-sitter grammars per dead session (observed 3+ simultaneously). `run_stdio()` now runs a parent-death watchdog alongside `server.run()` that fires when the captured parent pid changes and `os._exit(0)`s from inside the stdio context manager, bypassing the same wedged teardown ([#2185](https://github.com/headroomlabs-ai/headroom/issues/2185), [#1761](https://github.com/headroomlabs-ai/headroom/issues/1761)).
- **cache/prefix-freeze:** resolve `PrefixCacheTracker`s per conversation lineage within a session id, so concurrent conversations sharing a fallback id no longer thrash one tracker's frozen-prefix state ([#2085](https://github.com/headroomlabs-ai/headroom/issues/2085)). Without an `x-headroom-session-id` header the fallback id hashes `model + system prompt` — identical across a Claude Code session and every one of its parallel subagents (and any set of sessions reusing one system prompt). On the shared tracker their interleaved histories cross-contaminate the freeze state: the forwarded prefix is byte-unstable on nearly every turn and the provider prompt cache is re-written instead of read — reported as ~4.4x cache-creation inflation and a 2.53x net cost increase under Claude Code. `SessionTrackerStore.resolve_tracker` now reuses the tracker whose previous request messages are a prefix of the incoming history (client histories are append-only, so a conversation's next request always extends its previous one), starts a fresh lineage when the history diverges or was rewritten (client-side compaction — that provider cache line is gone anyway), and caps lineages per session id (`PrefixFreezeConfig.max_lineages_per_session`, default 32; over-cap conversations share one overflow tracker instead of evicting established lineages, so a fan-out storm past the cap degrades only its own tail — and `0` disables lineage splitting). Matching compares the original client bytes under the same canonical cross-turn equivalence as the cache-stable delta path (`_canonicalize_for_prefix_compare`), so a moved cache breakpoint, string<->block content sugar, or per-turn transport annotations do not read as a rewrite. Separately, the fallback id now hashes only the LEADING run of `role:"system"` messages: agentic clients interleave `<system-reminder>` turns into the history as actual system-role messages (hook output, skills lists, truncation notices), and hashing those rotated the session id mid-conversation — orphaning the prefix tracker and every other session-sticky subsystem (beta headers, CCR/memory registries, the compression cache) each time a reminder landed. Both handler paths now derive the session id and the lineage from the same original client bytes, so a turn-dependent hook rewrite cannot rotate one without the other. The session id itself never changes: session-sticky state keyed on it (beta-header stickiness, CCR and memory-tool registries, the compression cache) is untouched, and a single-conversation session keeps its exact previous behavior (the first lineage lives under the bare id).
- **proxy/bedrock:** wire `PrefixCacheTracker` updates into both Bedrock backend paths (`handle_anthropic_messages`'s non-streaming branch in `anthropic.py`, and `_stream_response_bedrock` in `streaming.py`). `update_from_response()` was previously only called from the direct-Anthropic-API branch; both Bedrock branches returned before ever reaching it, so the tracker's state stayed permanently empty for the life of a session on any `--backend bedrock` deployment: `extract_cache_stable_delta()` always saw no previous turn, and `--mode cache` fell back to full unmodified passthrough on every turn instead of freezing the already-cached prefix and compressing only the new suffix.

View file

@ -177,8 +177,7 @@ class MemoryHandler:
- Native tool: Anthropic's memory_20250818 built-in tool (experimental)
"""
# Cosine similarity thresholds for dedup
DEDUP_AUTO_THRESHOLD = 0.92 # Auto-supersede (same fact, different wording)
# Cosine similarity threshold for dedup hints
DEDUP_HINT_THRESHOLD = 0.75 # Suggest merge to LLM (related, possibly duplicate)
def __init__(self, config: MemoryConfig, agent_type: str = "unknown") -> None:
@ -1197,7 +1196,7 @@ your responses, not to drive new actions."""
provider: str = "anthropic",
request_context: RequestContext | None = None,
) -> str:
"""Execute memory_save tool with provenance, dedup hints, and async background dedup."""
"""Execute memory_save tool with provenance and dedup hints."""
content = input_data.get("content", "")
if not content:
return json.dumps({"status": "error", "error": "content is required"})
@ -1239,7 +1238,8 @@ your responses, not to drive new actions."""
metadata=provenance_metadata,
)
# Search for similar existing memories (for hints + async dedup)
# Search for similar existing memories so the caller can decide whether
# to merge them through the explicit memory_update path.
similar_memories = []
try:
results = await backend.search_memories(
@ -1276,12 +1276,6 @@ your responses, not to drive new actions."""
f"or ignore if these are distinct facts."
)
# Async background dedup: auto-supersede obvious duplicates
if similar_memories:
asyncio.create_task(
self._background_dedup(memory.id, similar_memories, effective_user_id, backend)
)
logger.info(
"event=memory_save user=%s scope=%s agent=%s provider=%s similar=%d",
effective_user_id,
@ -1293,51 +1287,6 @@ your responses, not to drive new actions."""
return json.dumps(result)
async def _background_dedup(
self,
new_memory_id: str,
similar_results: list[Any],
user_id: str,
backend: Any | None = None,
) -> None:
"""Auto-supersede obvious duplicates in background (fire-and-forget).
If an existing memory has >0.92 cosine similarity to the new one,
mark the older one as superseded. This runs asynchronously and
never blocks the tool response.
``backend`` defaults to the legacy ``self._backend`` so existing
non-routed callers keep working; routed callers pass the same
per-project backend they wrote to so dedup never crosses
workspaces.
"""
target = backend if backend is not None else self._backend
if target is None:
return
try:
for result in similar_results:
if result.score < self.DEDUP_AUTO_THRESHOLD:
continue
if result.memory.id == new_memory_id:
continue
old = result.memory
# Skip if already superseded
if old.metadata.get("superseded_by"):
continue
# Mark old memory as superseded by deleting it
# (update_memory creates a new version — for dedup we just remove the duplicate)
if hasattr(target, "delete_memory"):
await target.delete_memory(old.id)
logger.info(
f"Memory dedup: removed '{old.content[:50]}' "
f"(superseded by {new_memory_id}, {result.score:.2f} cosine, "
f"agent={old.metadata.get('source_agent', '?')})"
)
except Exception as e:
logger.warning(f"Memory background dedup failed: {e}")
async def _execute_search(
self,
input_data: dict[str, Any],

View file

@ -704,9 +704,7 @@ async def test_warmup_embedder_and_close(handler: MemoryHandler) -> None:
@pytest.mark.asyncio
async def test_execute_memory_tool_save_and_background_dedup(
handler: MemoryHandler, monkeypatch: pytest.MonkeyPatch
) -> None:
async def test_execute_memory_tool_save_returns_dedup_hint(handler: MemoryHandler) -> None:
backend = FakeBackend()
handler._backend = backend
@ -718,14 +716,6 @@ async def test_execute_memory_tool_save_and_background_dedup(
"error": "content is required",
}
created_tasks: list[object] = []
def fake_create_task(coro): # noqa: ANN001
created_tasks.append(coro)
coro.close()
return SimpleNamespace()
monkeypatch.setattr("headroom.proxy.memory_handler.asyncio.create_task", fake_create_task)
backend.search_results = [
make_result(
"other",
@ -757,7 +747,7 @@ async def test_execute_memory_tool_save_and_background_dedup(
assert "Similar memory exists" in saved["note"]
assert "saved by claude" in saved["note"]
assert backend.saved[-1]["metadata"]["source_provider"] == "openai"
assert len(created_tasks) == 1
assert backend.deleted == []
backend.raise_on = "save"
errored = json.loads(await handler._execute_memory_tool("memory_save", {"content": "x"}, "u1"))
@ -765,9 +755,7 @@ async def test_execute_memory_tool_save_and_background_dedup(
@pytest.mark.asyncio
async def test_execute_save_handles_search_failure_and_background_dedup_filters(
handler: MemoryHandler,
) -> None:
async def test_execute_save_handles_search_failure(handler: MemoryHandler) -> None:
backend = FakeBackend()
handler._backend = backend
@ -775,18 +763,35 @@ async def test_execute_save_handles_search_failure_and_background_dedup_filters(
saved = json.loads(await handler._execute_save({"content": "Useful fact"}, "u1"))
assert saved == {"status": "saved", "memory_id": "mem-1", "content": "Useful fact"}
backend.raise_on = None
similar = [
make_result("mem-1", "same", score=0.99),
make_result("old-1", "duplicate", score=0.95, metadata={}),
make_result("old-2", "already handled", score=0.99, metadata={"superseded_by": "new"}),
make_result("old-3", "too low", score=0.5, metadata={}),
]
await handler._background_dedup("mem-1", similar, "u1")
assert backend.deleted == ["old-1"]
backend.raise_on = "delete"
await handler._background_dedup("mem-1", [make_result("old-4", "duplicate", score=0.95)], "u1")
@pytest.mark.asyncio
async def test_execute_save_preserves_high_similarity_distinct_memory(
handler: MemoryHandler,
) -> None:
backend = FakeBackend()
handler._backend = backend
backend.search_results = [
make_result(
"existing-memory",
"User uses Python at work",
score=0.99,
metadata={"source_agent": "claude"},
)
]
saved = json.loads(
await handler._execute_save(
{"content": "User prefers Python for side projects"},
"u1",
)
)
# Give any accidentally scheduled background work a chance to run.
await asyncio.sleep(0)
assert saved["status"] == "saved"
assert "Similar memory exists" in saved["note"]
assert "ignore if these are distinct facts" in saved["note"]
assert backend.deleted == []
def test_inject_tools_extract_query_and_has_tool_calls(