From eeb2d23b8653fdfdd3c8196cf82bdae714742937 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:15:08 -0700 Subject: [PATCH] fix(memory): accept new_text as an alias for content (#86642) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit memory replace/remove target an entry by old_text but supply the replacement via content — an asymmetric pairing. Callers naturally reach for new_text to mirror old_text (it's exactly the patch tool's old_string/new_string shape), which left content empty and errored 'content is required'. The failure also rendered tersely, making the cause easy to miss and costing a retry. Accept new_text as an alias for content on both shapes: - single-op: coalesce content = content or new_text in memory_tool() + a new_text param wired through the registry handler. - batch ops: content = op.content or op.new_text (and in the approval-gate preview builder). - schema: document the alias on content and add a new_text property to the single-op params and batch item props so strict validators accept it. content wins if both are set. Tests: new_text alias on single add/replace, batch add/replace, and content-wins-over-new_text. 43/43 across memory tool + schema suites. Co-authored-by: Teknium --- tests/tools/test_memory_tool.py | 41 +++++++++++++++++++++++++++++++++ tools/memory_tool.py | 29 +++++++++++++++++++---- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_memory_tool.py b/tests/tools/test_memory_tool.py index d3fd4d1a0e..0582703abb 100644 --- a/tests/tools/test_memory_tool.py +++ b/tests/tools/test_memory_tool.py @@ -306,6 +306,30 @@ class TestMemoryToolDispatcher: assert "content is required" in result["error"] assert "current_entries" not in result + def test_new_text_alias_for_content_on_replace(self, store): + # A caller mirroring old_text with new_text (the patch tool's shape) + # must succeed instead of erroring 'content is required'. + store.add("memory", "fact A") + result = json.loads( + memory_tool(action="replace", old_text="fact A", new_text="fact A refined", store=store) + ) + assert result["success"] is True + assert "fact A refined" in store.memory_entries + assert "fact A" not in [e for e in store.memory_entries if e == "fact A"] + + def test_new_text_alias_for_content_on_add(self, store): + result = json.loads(memory_tool(action="add", new_text="added via new_text", store=store)) + assert result["success"] is True + assert "added via new_text" in store.memory_entries + + def test_content_wins_when_both_content_and_new_text_set(self, store): + result = json.loads( + memory_tool(action="add", content="the real one", new_text="ignored", store=store) + ) + assert result["success"] is True + assert "the real one" in store.memory_entries + assert "ignored" not in store.memory_entries + class TestMemoryBatch: """The 'operations' batch shape: atomic, all-or-nothing, final-budget.""" @@ -330,6 +354,23 @@ class TestMemoryBatch: assert "usage" in result + def test_batch_new_text_alias_for_content(self, store): + # new_text works inside batch ops too (both add and replace). + store.add("memory", "old entry") + result = json.loads(memory_tool( + target="memory", + operations=[ + {"action": "replace", "old_text": "old entry", "new_text": "updated entry"}, + {"action": "add", "new_text": "batched via new_text"}, + ], + store=store, + )) + assert result["success"] is True + assert "updated entry" in store.memory_entries + assert "batched via new_text" in store.memory_entries + assert "old entry" not in store.memory_entries + + def test_batch_duplicate_add_is_noop_not_failure(self, store): store.add("memory", "already here") result = json.loads(memory_tool( diff --git a/tools/memory_tool.py b/tools/memory_tool.py index d77aa9e285..44effd02c2 100644 --- a/tools/memory_tool.py +++ b/tools/memory_tool.py @@ -599,7 +599,7 @@ class MemoryStore: for i, op in enumerate(operations): op = op or {} act = op.get("action") - content = (op.get("content") or "").strip() + content = (op.get("content") or op.get("new_text") or "").strip() old_text = (op.get("old_text") or "").strip() pos = f"Operation {i + 1} ({act or 'unknown'})" @@ -991,12 +991,13 @@ def _apply_batch_write_gate(target: str, operations: List[Dict[str, Any]]) -> Op for op in operations: op = op or {} act = op.get("action", "?") + _op_content = op.get("content") or op.get("new_text") or "" if act == "remove": detail_lines.append(f"- remove: {op.get('old_text', '')}") elif act == "replace": - detail_lines.append(f"- replace: {op.get('old_text', '')} -> {op.get('content', '')}") + detail_lines.append(f"- replace: {op.get('old_text', '')} -> {_op_content}") else: - detail_lines.append(f"- {act}: {op.get('content', '')}") + detail_lines.append(f"- {act}: {_op_content}") detail = "\n".join(detail_lines) decision = wa.evaluate_gate(wa.MEMORY, inline_summary=summary, inline_detail=detail) @@ -1057,6 +1058,7 @@ def memory_tool( target: str = "memory", content: str = None, old_text: str = None, + new_text: str = None, operations: Optional[List[Dict[str, Any]]] = None, store: Optional[MemoryStore] = None, ) -> str: @@ -1068,11 +1070,22 @@ def memory_tool( - Batch: operations=[{action, content?, old_text?}, ...] applied atomically against the final char budget in ONE call. + ``new_text`` is accepted as an alias for ``content`` on both shapes. The + replace/remove ops target by ``old_text`` and supply the replacement via + ``content``; callers naturally reach for ``new_text`` to mirror + ``old_text`` (it's the patch tool's ``old_string``/``new_string`` shape), + which silently left ``content`` empty and errored. Coalescing here removes + that trap. + Returns JSON string with results. """ if store is None: return tool_error("Memory is not available. It may be disabled in config or this environment.", success=False) + # Accept new_text as an alias for content (single-op path). See docstring. + if content is None and new_text is not None: + content = new_text + # Some strict providers fill optional schema fields with JSON null rather # than omitting them. Treat ``target: null`` as omitted so memory writes # still use the documented default store instead of failing validation. @@ -1196,12 +1209,16 @@ MEMORY_SCHEMA = { }, "content": { "type": "string", - "description": "The entry content. Required for 'add' and 'replace' (single-op shape)." + "description": "The entry content. Required for 'add' and 'replace' (single-op shape). Alias: 'new_text' is also accepted (mirrors old_text)." }, "old_text": { "type": "string", "description": "REQUIRED for 'replace' and 'remove' (single-op shape): a short unique substring identifying the existing entry to modify. Omit only for 'add'." }, + "new_text": { + "type": "string", + "description": "Alias for 'content' (single-op shape). Provided so the replace/remove old_text/new_text pairing works; if both are set, 'content' wins." + }, "operations": { "type": "array", "description": ( @@ -1213,7 +1230,8 @@ MEMORY_SCHEMA = { "type": "object", "properties": { "action": {"type": "string", "enum": ["add", "replace", "remove"]}, - "content": {"type": "string", "description": "Entry content for add/replace."}, + "content": {"type": "string", "description": "Entry content for add/replace. Alias: 'new_text'."}, + "new_text": {"type": "string", "description": "Alias for 'content' in a batch op."}, "old_text": {"type": "string", "description": "Substring identifying the entry for replace/remove."}, }, "required": ["action"], @@ -1237,6 +1255,7 @@ registry.register( target=args.get("target", "memory"), content=args.get("content"), old_text=args.get("old_text"), + new_text=args.get("new_text"), operations=args.get("operations"), store=kw.get("store")), check_fn=check_memory_requirements,