Repository navigation
Python: fix(core): keep distinct non-ASCII memory topics in separate files - #9227
Yufeng He (he-yufeng) wants to merge 3 commits into
Conversation
…files _slugify_topic strips everything outside [a-z0-9], so two unrelated topics can collapse onto one filename: 旅行计划 and 饮食偏好 both fall through to the memory-topic fallback, café and cafè both become caf. The second write then reads back as the first topic with merged memories, and deleting one removes the shared file under both names. Keep the readable slug byte-identical when the derivation is lossless (slug == normalized.lower(), so existing safe ASCII stems are untouched) and suffix a short sha256 digest of the normalized topic otherwise, the same collision-resistance standard _storage_key_segment already applies to owner and source segments in this file. get_topic and delete_topic fall back to the pre-digest path when the new file is absent, and write_topic absorbs a legacy-named file into the new name, so stores written by older versions stay readable and migrate on first rewrite. Fixes microsoft#9205
LI (ktz03)
left a comment
There was a problem hiding this comment.
Tested 61ed9bc18f6941b6e5ad90cfc851c12acac73ec4 through the public memory tools and MemoryFileStore with real temporary Markdown files. Fresh CJK/accented pairs are separated correctly, and safe ASCII stems retain their filenames. Same-topic rewrites also read and migrate files generated by the previous implementation.
There is still a blocking identity check missing from the legacy fallback and cleanup paths. A legacy filename is only a possible match; it can belong to a different topic:
store.write_topic(session, record("café", "accent fact"), source_id="memory")
store.write_topic(session, record("caf", "ASCII fact"), source_id="memory")
store.delete_topic(session, source_id="memory", topic="café")
# Both files are removed; the independent ASCII topic "caf" is now missing.Reversing the two writes removes caf.md during write_topic("café") itself. The public write_memory tool also still merges caf followed by café into one topic because get_topic("café") accepts caf.md without checking its stored title. The same issue appears during upgrade: a file generated by the previous implementation for café, followed by a write to distinct cafè, absorbs the new fact into café and leaves cafè unreadable.
Please verify that a fallback file belongs to the requested logical topic before reading, migrating, or deleting it, and add both write-order/delete controls alongside the valid same-topic migration tests. This should preserve explicit lookup by slug while preventing the fallback from treating an unrelated topic as a legacy alias.
Validation scope: Windows/Python 3.14.3, 16 public compatibility scenarios, including four upgrade cases using files generated in separate processes by the unchanged implementation. Cached dependencies; no full core suite, remote model, or other-platform validation.
AI assistance: AI-assisted review and reproduction.
The pre-digest fallback path treats any legacy-named file as an alias for the requested topic. The slug strips accents and non-ASCII text, so a same-stem file can belong to a different topic: after writing both "café" and "caf", deleting "café" removed "caf.md" too, writing "café" absorbed "caf.md", and a pre-digest file for "café" answered lookups for "cafè". A legacy-named file now counts as a fallback alias only when the topic recorded inside the file matches the requested topic; get_topic, delete_topic, and the write_topic absorb all gate on it. Same-topic migration of stores written by older versions is unchanged. Verified with the full agent-framework-core suite: 6728 tests, 0 failures; ruff and pyright clean on the changed files. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
|
Thanks for the careful review, and for running the real-file compatibility matrix. Your three scenarios all reproduced as genuine data loss, and the fix is in 864a5b5. The gap was that the legacy fallback treated any legacy-named file as an alias for the requested topic. The slug derivation is lossy by design (accents and non-ASCII fold away), so a same-stem file can belong to a different topic. The fix gates every legacy-path use on identity: a legacy-named file is a fallback alias only when the topic recorded in its
Your repros are pinned as regression tests: Verification: full |
LI (ktz03)
left a comment
There was a problem hiding this comment.
The legacy identity checks in 864a5b5cc75c54aaa7afd0fe2f00915c6e780cd8 address the earlier read/write/delete scenarios. Both write orders now preserve caf separately from café, and the valid same-topic migration controls still work.
There is another upgrade path that fails before a rewrite can migrate the file. list_topics() parses an existing caf.md headed # café and returns the new slug caf-850f7dc4. MemoryContextProvider.before_run() then selects the topic and calls get_topic(topic=entry.slug). That lookup searches for caf-850f7dc4.md, while the only persisted file is still caf.md; the legacy candidate cannot be derived from the already-digested slug. The result is FileNotFoundError, so a relevant prompt aborts during memory preparation.
Minimal reproduction using the actual previous layout and file contents:
import asyncio
from pathlib import Path
from tempfile import TemporaryDirectory
from agent_framework import (
AgentSession, MemoryContextProvider, MemoryFileStore, Message, SessionContext,
)
async def main():
with TemporaryDirectory() as root:
legacy = Path(root) / "memory/alice/memory/topics/caf.md"
legacy.parent.mkdir(parents=True)
legacy.write_text(
"# café\n\nUpdated: 2026-10-09T02:00:00Z\nSessions: -\n\n"
"## Summary\nlegacy fact\n\n## Memories\n- legacy fact\n",
encoding="utf-8",
)
session = AgentSession(session_id="one")
session.state["owner"] = "alice"
store = MemoryFileStore(root, owner_state_key="owner")
assert store.get_topic(session, source_id="memory", topic="café").memories == ["legacy fact"]
provider = MemoryContextProvider(store=store, recent_turns=0, max_extractions=0)
context = SessionContext(
session_id=session.session_id,
input_messages=[Message(role="user", contents=["legacy fact"])],
)
await provider.before_run(agent=None, session=session, context=context, state={})
asyncio.run(main())
# FileNotFoundError: No memory topic named 'caf-850f7dc4' was found for this owner.Please cover the complete existing-file → index → selected-topic injection path before a write, keeping the legacy heading identity checks. The index pointers should also resolve to persisted records before migration. Broadly trusting a legacy filename again would reintroduce the issue fixed in the previous revision.
I generated the legacy files with the unchanged implementation in separate processes. café, 旅行计划, and Travel Plans all pass the same automatic injection on the baseline and fail on this head; project-a remains a normal control. Explicit same-topic rewrites migrate the three files and make injection work again. The earlier 16 compatibility scenarios were also repeated successfully. Scope: Windows/Python 3.14.3, public provider/store/tool calls and real temporary files with cached dependencies; no full core suite, external model/service, or other OS validation.
AI assistance: AI-assisted review and reproduction.
list_topics parses a pre-digest file such as caf.md headed "# café" and returns the current digested slug, but the file on disk still uses the legacy name. before_run then calls get_topic(entry.slug) for the selected entry, where neither the digested path nor a legacy candidate derivable from that slug exists, so memory preparation dies with FileNotFoundError. Consolidation silently skips the topic the same way. A record whose file still sits at the pre-digest name for its topic is now keyed by the on-disk stem, so MEMORY.md pointers and get_topic(entry.slug) resolve to the persisted file before a rewrite migrates it. The heading identity check on the fallback paths is unchanged: a same-stem file owned by a different topic still does not alias. Verified with the full agent-framework-core suite: 8483 tests, 0 failures; ruff and pyright clean on the changed files. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
|
Both upgrade paths are covered in 1ba70db, thanks for the precise repro. The break was in The fix re-keys such records by the stem that actually exists on disk, only when the stem is exactly the pre-digest derivation of the parsed topic. Your repro is pinned as |
Motivation & Context
MemoryFileStorederives a topic's filename with_slugify_topic, which strips every character outside[a-z0-9]. Two unrelated topics can therefore collapse onto one file:旅行计划and饮食偏好both fall through to thememory-topicfallback,caféandcafèboth becomecaf. The second write reads back as the first topic with merged memories, anddelete_memory_topicon either name removes the shared file. Silent data loss for any non-ASCII deployment, reproduced against current main in #9205.Description & Review Guide
_slugify_topicnow returns its existing output unchanged when the derivation is lossless (slug == normalized.lower()), so every safe ASCII stem keeps the exact filename older versions wrote. When the projection discarded information (non-ASCII, accents, case, punctuation), it appends an 8-char sha256 digest of the normalized topic:café->caf-850f7dc4,旅行计划->memory-topic-f9d6fe13. This is the same collision-resistance standard_storage_key_segmentalready applies to owner and source segments in this file.get_topic/delete_topicfall back to the pre-digest path when the new file is absent, andwrite_topicabsorbs a legacy-named file into the new name so a migrated topic never lists twice.MemoryFileStore(_legacy_topic_path,_topic_read_candidates) and on whether keeping the readable prefix plus digest is the right trade against a full_storage_key_segmentswitch (that helper would base32-encode any non-safe topic, losing the readable stems this store exposes in tool output and index pointer lines).Related Issue
Fixes #9205
AI Assistance
AI assistance details: implementation and tests were drafted with AI assistance; I verified the reproduction, the fix, and the full core test suite locally.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.