From 3aa94f8bfd3a37688cadfa2f573fc333aa0891a1 Mon Sep 17 00:00:00 2001 From: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:02:04 +0530 Subject: [PATCH] Python: give each file memory its own description sidecar The sidecar name stripped the file extension, so notes.md and notes shared one sidecar and one file's description overwrote or deleted the other's. The suffix now appends to the full name, sidecars written under the previous name are still read through a fallback, and writes and deletes retire both spellings. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com> --- .../agent_framework/_harness/_file_memory.py | 28 +++++++- .../tests/core/test_harness_file_memory.py | 71 ++++++++++++++++--- 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_memory.py b/python/packages/core/agent_framework/_harness/_file_memory.py index b64a112376b..fc8dbfef189 100644 --- a/python/packages/core/agent_framework/_harness/_file_memory.py +++ b/python/packages/core/agent_framework/_harness/_file_memory.py @@ -95,9 +95,16 @@ def _description_file_name(file_name: str) -> str: """Return the companion description file name for ``file_name``. - The suffix replaces the original extension when present (so ``notes.md`` - becomes ``notes_description.md``); otherwise it is appended. + The suffix is appended to the full name. Stripping the extension first let + ``notes.md`` and ``notes`` share one sidecar, so one file's description + overwrote or deleted the other's; appending keeps the mapping injective + while extension-less names keep their existing sidecar name. """ + return f"{file_name}{_DESCRIPTION_SUFFIX}" + + +def _legacy_description_file_name(file_name: str) -> str: + """Return the sidecar name used before the suffix was made injective.""" dot_index = file_name.rfind(".") if dot_index > 0: return f"{file_name[:dot_index]}{_DESCRIPTION_SUFFIX}" @@ -323,6 +330,13 @@ def _resolve_working_folder(self, context: SessionContext) -> str: ) return _storage_key_segment(raw_scope, encoded_prefix=_ENCODED_SCOPE_PREFIX) + async def _read_description(self, working_folder: str, file_name: str) -> str | None: + """Read a file's description sidecar, including ones written under the legacy name.""" + description = await self.store.read(_combine_paths(working_folder, _description_file_name(file_name))) + if description is not None: + return description + return await self.store.read(_combine_paths(working_folder, _legacy_description_file_name(file_name))) + async def _rebuild_index(self, working_folder: str) -> None: """Rebuild the ``memories.md`` index for ``working_folder``. @@ -335,7 +349,7 @@ async def _rebuild_index(self, working_folder: str) -> None: lines = ["# Memory Index", ""] for file_name in sorted_files[:_MAX_INDEX_ENTRIES]: - description = await self.store.read(_combine_paths(working_folder, _description_file_name(file_name))) + description = await self._read_description(working_folder, file_name) if description and description.strip(): lines.append(f"- **{file_name}**: {description.strip()}") else: @@ -378,6 +392,7 @@ async def file_memory_write(file_name: str, content: str, description: str | Non path = _combine_paths(working_folder, normalized) desc_path = _combine_paths(working_folder, _description_file_name(normalized)) + legacy_desc_path = _combine_paths(working_folder, _legacy_description_file_name(normalized)) # Every mutating tool locks the whole working folder, not just the file, because each # write and delete rebuilds the folder's memories.md index. The lock is shared by all # providers on the store, including file-access tools editing a file in this folder, so @@ -387,8 +402,12 @@ async def file_memory_write(file_name: str, content: str, description: str | Non await self.store.write(path, content) if description and description.strip(): await self.store.write(desc_path, description) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) else: await self.store.delete(desc_path) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) await self._rebuild_index(working_folder) except ValueError as exc: return f"Could not write file '{file_name}': {exc}" @@ -427,10 +446,13 @@ async def file_memory_delete(file_name: str) -> str: path = _combine_paths(working_folder, normalized) desc_path = _combine_paths(working_folder, _description_file_name(normalized)) + legacy_desc_path = _combine_paths(working_folder, _legacy_description_file_name(normalized)) async with _store_write_lock(self.store, working_folder): try: deleted = await self.store.delete(path) await self.store.delete(desc_path) + if legacy_desc_path != desc_path: + await self.store.delete(legacy_desc_path) await self._rebuild_index(working_folder) except ValueError as exc: return f"Could not delete file '{file_name}': {exc}" diff --git a/python/packages/core/tests/core/test_harness_file_memory.py b/python/packages/core/tests/core/test_harness_file_memory.py index 5d8f7394be2..32570d82500 100644 --- a/python/packages/core/tests/core/test_harness_file_memory.py +++ b/python/packages/core/tests/core/test_harness_file_memory.py @@ -55,13 +55,23 @@ async def _prepare( return context, tools -def test_description_file_name_replaces_extension() -> None: - """The description sidecar replaces a known extension and appends otherwise.""" - assert _description_file_name("notes.md") == "notes_description.md" - assert _description_file_name("data.json") == "data_description.md" +def test_description_file_name_is_injective_across_extensions() -> None: + """The description sidecar appends to the full name so extensions cannot collide.""" + assert _description_file_name("notes.md") == "notes.md_description.md" + assert _description_file_name("data.json") == "data.json_description.md" assert _description_file_name("noext") == "noext_description.md" - # Leading-dot files have no stem, so the suffix is appended. + # Leading-dot files keep the suffix appended as well. assert _description_file_name(".hidden") == ".hidden_description.md" + # names with and without an extension no longer share one sidecar. + assert _description_file_name("notes.md") != _description_file_name("notes") + + +def test_legacy_description_file_name_still_resolves_for_old_stores() -> None: + """Sidecars written before the injective naming stay readable for extension files.""" + from agent_framework._harness._file_memory import _legacy_description_file_name # pyright: ignore[reportPrivateUsage] + + assert _legacy_description_file_name("notes.md") == "notes_description.md" + assert _legacy_description_file_name("noext") == "noext_description.md" def test_is_internal_file_detects_sidecars_and_index() -> None: @@ -152,7 +162,7 @@ async def test_description_sidecar_is_written_and_listed() -> None: ) assert "with description" in _text(result) - sidecar = await store.read(_combine_paths("user-1", "arch_description.md")) + sidecar = await store.read(_combine_paths("user-1", "arch.md_description.md")) assert sidecar == "system architecture" listed = json.loads(_text(await list_files.invoke())) @@ -160,7 +170,7 @@ async def test_description_sidecar_is_written_and_listed() -> None: # Re-saving without a description removes the sidecar. await save.invoke(arguments={"file_name": "arch.md", "content": "big content"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) is None + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) is None listed_again = json.loads(_text(await list_files.invoke())) assert listed_again == [{"name": "arch.md", "type": "file", "description": None}] @@ -172,10 +182,53 @@ async def test_delete_removes_sidecar() -> None: _, tools = await _prepare(provider) await tools["file_memory_write"].invoke(arguments={"file_name": "arch.md", "content": "x", "description": "desc"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) == "desc" + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) == "desc" await tools["file_memory_delete"].invoke(arguments={"file_name": "arch.md"}) - assert await store.read(_combine_paths("user-1", "arch_description.md")) is None + assert await store.read(_combine_paths("user-1", "arch.md_description.md")) is None + + +async def test_description_sidecars_stay_separate_for_names_differing_by_extension() -> None: + """``notes.md`` and ``notes`` own distinct description sidecars.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + save = tools["file_memory_write"] + list_files = tools["file_memory_ls"] + + await save.invoke( + arguments={"file_name": "notes.md", "content": "architecture notes", "description": "architecture"} + ) + await save.invoke(arguments={"file_name": "notes", "content": "scratch", "description": "scratch"}) + + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "architecture" + assert await store.read(_combine_paths("user-1", "notes_description.md")) == "scratch" + + # Rewriting the extension-less file without a description must not touch the other's sidecar. + await save.invoke(arguments={"file_name": "notes", "content": "more scratch"}) + assert await store.read(_combine_paths("user-1", "notes.md_description.md")) == "architecture" + + listed = json.loads(_text(await list_files.invoke())) + descriptions = {entry["name"]: entry["description"] for entry in listed} + assert descriptions == {"notes.md": "architecture", "notes": None} + + +async def test_legacy_description_sidecar_still_feeds_the_index() -> None: + """Descriptions written under the old sidecar name keep appearing in the index.""" + store = InMemoryAgentFileStore() + provider = FileMemoryProvider(store=store, scope="user-1") + _, tools = await _prepare(provider) + + await store.write(_combine_paths("user-1", "legacy.md"), "old") + await store.write(_combine_paths("user-1", "legacy_description.md"), "old description") + await tools["file_memory_write"].invoke( + arguments={"file_name": "fresh.txt", "content": "new", "description": "new desc"} + ) + + index = await store.read(_combine_paths("user-1", _MEMORY_INDEX_FILE_NAME)) + assert index is not None + assert "old description" in index + assert "new desc" in index async def test_index_is_rebuilt_and_injected_on_next_run() -> None: