Skip to content

Python: drop the memory placeholder only when it is the sole entry - #9726

Closed
Manohar Paturi (ManoharPaturi) wants to merge 2 commits into
microsoft:mainfrom
ManoharPaturi:fix/memory-none-yet-placeholder
Closed

Manohar Paturi (ManoharPaturi) wants to merge 2 commits into
microsoft:mainfrom
ManoharPaturi:fix/memory-none-yet-placeholder

Conversation

@ManoharPaturi

@ManoharPaturi Manohar Paturi (ManoharPaturi) commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

to_markdown() writes - none yet as the placeholder for an empty memory list, and from_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 exactly none 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

  • What are the major changes? Parsing now keeps every bullet, and the placeholder is removed once after the loop, only when it is the sole entry.
  • What is the impact of these changes? That sole-entry shape is the only one the write side produces for an empty memory list, so empty lists still round trip to empty. A genuine memory equal to the placeholder text survives.
  • What do you want reviewers to focus on? The two round trip tests: the mixed list keeps both memories (fails on main), and the empty list still reads back empty.

Related Issue

Fixes #9725

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

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.

@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

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.

@eavanvalkenburg

Copy link
Copy Markdown
Member

Manohar Paturi (@ManoharPaturi) The singleton none yet regression is fixed, but the new marker introduces a separate data-loss case in python/packages/core/agent_framework/_harness/_memory.py:543-545.

A record with memories=["none yet <!-- genuine -->"] round-trips through to_markdown() / from_markdown() as ["none yet"], losing the literal marker. Both strings together serialize identically and collapse into one memory on reload. The new reader also changes a legacy file containing the literal marker, which the base reader preserves.

Please escape literal marker-shaped content as well as the genuine none yet value, and distinguish newly encoded values from legacy literal content. Add regressions for the literal value, both values together, and a base-written file.

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>
@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

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 <!-- genuine --> on write and loses exactly one on read. So the literal none yet <!-- genuine --> persists with a doubled marker and comes back byte for byte, and a genuine none yet stored next to the literal round trips as two distinct memories instead of collapsing onto one.

Distinguishing newly encoded values from legacy literal content needed provenance at the file level, so files that carry any marker now record an Encoding: genuine memory markers v1 line in the header, before the content sections. Markers decode only when that line is present, and a file written by base holding the literal marker text reads back verbatim, exactly as the base reader leaves it. The line is matched by exact equality in the header zone only, so the same text inside a summary or inside a memory stays content.

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.

@eavanvalkenburg

Copy link
Copy Markdown
Member

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.

This branch was previously deployed

1 inactive deployment
github-app-auth — 4d0416da Deployed Oct 29, 2026 by ManoharPaturi via add_label #26492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: a memory whose text equals the none yet placeholder is silently dropped on read

2 participants