Repository navigation
Python: drop the memory placeholder only when it is the sole entry - #9726
Manohar Paturi (ManoharPaturi) wants to merge 2 commits into
Conversation
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
Manohar Paturi (@ManoharPaturi) One material data-loss case remains in python/packages/core/agent_framework/_harness/_memory.py:524-525. The new condition still collapses a legitimate memories=["none yet"] record to an empty list, so the real singleton and the placeholder remain indistinguishable. Please encode genuine placeholder-valued memories unambiguously on write, decode them on read while preserving existing empty-placeholder behavior, and add a singleton round-trip regression.
A genuine memory whose exact text equals the empty-list placeholder used to be dropped on read, so memories=[none yet] collapsed to an empty list. The write side now marks a genuine placeholder-valued memory with a distinguishing suffix, and the reader decodes the mark before applying the sole-placeholder drop, so a marked singleton round trips exactly while a bare placeholder written for an empty list still reads back empty and unmarked files keep their previous meaning. Tests cover the singleton round trip, the written marker form, and legacy unmarked behavior. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
7ab1db8 to
0179ccb
Compare
|
Fixed in 0179ccb, verified on all four shapes before pushing. The write side now marks a genuine placeholder-valued memory with a distinguishing suffix, and the reader decodes that mark before applying the sole-placeholder drop, so the marked singleton round trips exactly while a bare placeholder written for an empty list still reads back empty and unmarked files keep their previous meaning. Concretely: memories=[none yet] persists as a marked line and reads back as [none yet]; the empty list still reads back empty; a mixed [none yet, real fact] keeps both; and a legacy unmarked file whose sole entry is the bare placeholder still reads back empty. The singleton test fails on the previous head and passes now, the memory suite passes at 38 tests, and ruff check and format are clean. |
|
Eduard van Valkenburg (@eavanvalkenburg) the PR body now carries the required Contribution Checklist with the non-breaking item confirmed, matching the template. Everything from your review is addressed in the latest push and the thread replies have the verification details. |
|
Manohar Paturi (@ManoharPaturi) The singleton A record with Please escape literal marker-shaped content as well as the genuine |
Marker-shaped memories gain one more marker on write and lose exactly one on read, and files only decode markers when their header records them, so legacy literal marker text keeps its exact bytes. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
|
Fixed in 4d0416d, with all three shapes reproduced on the previous head first. The encoding now terminates instead of applying once. A memory shaped like the placeholder, or like the placeholder already carrying markers, gains one more Distinguishing newly encoded values from legacy literal content needed provenance at the file level, so files that carry any marker now record an The regressions cover the literal value, both values together, and a file written by base, and all three fail on the previous head. The memory suite passes at 41 tests and ruff check and format are clean. |
|
Manohar Paturi (@ManoharPaturi), thanks for taking the time to investigate and implement this. We want to review and agree on #9725 before accepting an implementation PR, rather than evaluating the issue and implementation together. I’m closing this PR for now. Please wait for maintainer direction on the issue before opening a new implementation PR. |
Motivation & Context
to_markdown()writes- none yetas the placeholder for an empty memory list, andfrom_markdown()filtered every bullet equal to that text so the placeholder would not become a memory. The filter could not tell the placeholder from a real memory whose text happened to be exactlynone yet, so that memory was silently dropped on every read. Reproduced on main: a topic written with["none yet", "real fact"]reads back as["real fact"], and the loss repeats on every reparsing pass.Description & Review Guide
Related Issue
Fixes #9725
AI Assistance
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.