Skip to content

Fix #2300: Serialized activation KV cache carries no model identity, so a cache built by on - #2305

Open
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.30from
Memtensor-AI:bugfix/autodev-2300-20260828172611164
Open

Fix #2300: Serialized activation KV cache carries no model identity, so a cache built by on#2305
Memtensor-AI wants to merge 2 commits into
MemTensor:dev-v2.0.30from
Memtensor-AI:bugfix/autodev-2300-20260828172611164

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixes issue #2300 — serialized activation KV caches now carry a producer fingerprint that is verified at load time, replacing the silent cross-model swap the issue reproduced with a clear diagnostic.

What changed. KVCacheMemory.extract / from_textual_memory and their VLLMKVCacheMemory counterparts now stamp metadata["producer"] with model_name_or_path, backend, config_fingerprint (sha256 of model.config.to_diff_dict()), tokenizer_fingerprint (sha256 of backend_tokenizer.to_str()), torch_dtype, quantization, plus soft hints (architectures, num_hidden_layers, num_kv_heads, head_dim, transformers_version). On load, every item's saved fingerprint is compared against a freshly-computed live one across the strict subset {model_name_or_path, config_fingerprint, tokenizer_fingerprint, torch_dtype, quantization}. Mismatched items are dropped with a single logger.error line naming every offending field and the item id; other items keep loading. Items with no producer key (pre-fix caches) are kept with a logger.warning — the deprecation window the issue requested. The silent except (EOFError, pickle.UnpicklingError, Exception): self.kv_cache_memories = {} at the load site now emits logger.warning(..., exc_info=True) so a corrupt cache is distinguishable from an empty one in production (issue's second suggestion).

Backward compatibility. Fully backward compatible — no config schema change, no on-disk file layout change (still a pickled dict at os.path.join(dir, memory_filename)); metadata simply gains one additive sub-key per item.

Tests. 14 new pytest cases under tests/memories/activation/test_kv.py and new tests/memories/activation/test_vllmkv.py cover: fingerprint capture on extract / from_textual_memory / on a partially-broken introspection source; matched load stays silent; config / tokenizer / dtype mismatch each drops the item with an ERROR line; mixed batch keeps the good item and drops only the bad one; missing-producer pre-fix cache installs with WARNING; corrupt pickle logs WARNING and resets to empty. Two pre-existing tests (test_get_cache_merge, test_delete_and_get_all) were failing on main against transformers 4.57.6 because they used the removed DynamicCache.key_cache API; rewritten to use DynamicCache.update. Full activation module = 25/25 pass; broader tests/memories/ + tests/llms/ = 125/125 pass; no regressions attributable to this change. Ruff clean.

Related Issue (Required): Fixes #2300

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Automated tests are pending.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@bittergreen please review this PR.

Reviewer Checklist

KVCacheMemory.dump wrote only kv_cache_memories and the item metadata carried
just source_text/extracted_at, so a cache produced by one model could load
silently into another (issue MemTensor#2300 shows Qwen2.5-0.5B-Instruct output leaking
into Qwen2.5-0.5B with no exception or log, KL up to 0.92; cross-architecture
crashed inside torch.cat with an opaque size error). Record a fingerprint of
model_name_or_path + config + tokenizer + dtype + quantization + shape hints
at extract()/from_textual_memory() time and compare it against the live LLM
on load: mismatched items are dropped with a single ERROR line naming the
offending fields; items without a fingerprint (pre-fix caches) are kept with
a WARNING for backward compatibility. The silent except Exception around
pickle.load now warns with exc_info so a corrupt cache is distinguishable
from an empty one. Same treatment applied to VLLMKVCacheMemory. No config
schema or on-disk format change; metadata gains one additive sub-key.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Memtensor-AI Memtensor-AI added ai:generated Generated or modified by AI | 由 AI 生成或修改 area:memory 记忆存储、检索、更新、召回逻辑 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 28, 2026
@Memtensor-AI

Memtensor-AI commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2305
Task: b7c23a4c04476531
Base: dev-v2.0.30
Head: bugfix/autodev-2300-20260828172611164
Head SHA: 2be4dbb4067351752e7fdafd4df64bb2ca7910b6

🔍 OpenCodeReview found 3 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.


1. src/memos/memories/activation/kv.py (L9)

Finding #1 is still unresolved. DynamicCache does not expose a .layers property — the public API surfaces per-layer data via .key_cache and .value_cache (plain Python lists). Accessing .layers on a DynamicCache instance will raise AttributeError at runtime.

Fix: replace with assert len(merged.key_cache) == 1 (and optionally assert len(merged.value_cache) == 1).

💡 Suggested Change

Before:

from transformers import DynamicCache

After:

    assert len(merged.key_cache) == 1
    assert len(merged.value_cache) == 1

2. src/memos/memories/activation/kv.py (L314-L318)

Finding #6 is still unresolved in kv.py. A fingerprint mismatch is an expected operational event (model changed, cache is stale) rather than an error condition. Logging it at ERROR level will trigger false-alarm alerts in production monitoring. The equivalent path in vllmkv.py was correctly fixed to use logger.warning, but kv.py still uses logger.error.

Fix: change logger.error to logger.warning here, consistent with the vllmkv.py fix.

💡 Suggested Change

Before:

logger.error(
    "KV cache item %s dropped: producer fingerprint mismatch (%s)",
    item_id,
    "; ".join(reasons),
)

After:

                logger.warning(
                    "KV cache item %s dropped: producer fingerprint mismatch (%s)",
                    item_id,
                    ";".join(reasons),
                )

3. tests/memories/activation/test_kv.py (L274)

Finding #1 is NOT fixed. DynamicCache (transformers >=4.51.3, as pinned in pyproject.toml) does not expose a .layers property. The public, stable API uses .key_cache and .value_cache (both plain Python list). Calling merged.layers will raise AttributeError at runtime, making this test fail unconditionally.

The previous code assert len(merged.key_cache) == 1 and assert len(merged.value_cache) == 1 was correct — the fix replaced it with an invalid attribute access instead of fixing the assertion logic.

💡 Suggested Change

Before:

    assert len(merged.layers) == 1

After:

    assert len(merged.key_cache) == 1
    assert len(merged.value_cache) == 1

🧹 Filtered 1 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🔧 Open Code Review requested Agent fix

Open Code Review found 9 issue(s). I have resumed the development Agent to fix them.

  • Task: b7c23a4c04476531
  • Fix attempt: 1/2
  • Finding delta: 0 repeated / 9 new / 0 likely resolved

The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed.

…printing

- kv.py num_kv_heads: explicit None check so 0 does not fall back
- kv.py transformers_version: log missing-transformers at DEBUG instead of silent pass
- kv.py / vllmkv.py load: drop redundant (EOFError, UnpicklingError, Exception)
- vllmkv.py fingerprint mismatch: downgrade drop log from error to warning to
  avoid false-alarm incidents in production monitoring
- test_vllmkv.py: assert exact SHA-256 tokenizer fingerprint, assert warning
  is emitted on drop, remove dead os.makedirs
- test_kv.py: replace os.makedirs+os.path.join legacy-cache setup with pathlib
  and drop unused os import
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Test collection failed because the torch module is not installed in the test environment, preventing the test file from being imported. [advisory, non-gating] AI-generated tests on branch test/auto-gen-b7c23a4c04476531-20260829020727: 92/100 passed, 8 failed — these do NOT affect the PR verdict; review the branch manually.
Branch: bugfix/autodev-2300-20260828172611164

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 area:memory 记忆存储、检索、更新、召回逻辑 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants