Repository navigation
Python: fix(core): stop losing multiline consolidated memories on reload - #9240
Yufeng He (he-yufeng) wants to merge 3 commits into
Conversation
The write path normalizes every memory through _normalize_memory_text before it reaches a record, but _consolidate_topic hands the LLM's memories strings to MemoryTopicRecord raw. A consolidated memory with a line break is written by to_markdown as a bullet whose continuation lands on a non-bullet line, and from_markdown only reads "- " bullets, so a fresh store silently returns just the first line while the raw file still shows the rest. MemoryTopicRecord now normalizes each memory at construction, keeping the existing drop-empty tolerance, so single-line memories are a record invariant and the markdown round-trip is stable no matter which path supplied the text. Verified with the full agent-framework-core suite: 8536 tests, 0 failures; ruff and pyright clean on the changed files. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
🟢 Approval recommended
The focused normalization fix addresses the reported data loss and is covered at both record and consolidation levels.
0 open findings
What changed in this PR
Normalizes memory text at record construction to prevent multiline consolidated memories from being truncated after Markdown reload.
Changes:
- Enforces single-line, deduplicated memory records.
- Adds unit and consolidation round-trip coverage for LF and CRLF content.
| File | Description |
|---|---|
python/packages/core/agent_framework/_harness/_memory.py |
Normalizes memories during record initialization. |
python/packages/core/tests/core/test_harness_memory.py |
Tests normalization and fresh-store reload behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Yufeng He (@he-yufeng) Thanks for the contribution. The current PR template's required AI Assistance field is unanswered. Please update the PR body to state whether material AI assistance was used and, if so, identify the assisted areas. |
| # Collapse line breaks up front: the markdown format stores one bullet per line, | ||
| # and the consolidation path hands LLM-supplied strings in raw, so a memory kept | ||
| # multiline here would write fine but lose its continuation on the next read. | ||
| self.memories = _dedupe_strings([_normalize_memory_text(memory) for memory in memories if memory.strip()]) |
There was a problem hiding this comment.
Yufeng He (@he-yufeng) This only normalizes memories as they are newly constructed. Existing pre-fix files with a continuation line (for example, - Deploy staging. followed by Then deploy production.) still lose the continuation because from_markdown ignores non-bullet lines before MemoryTopicRecord normalization runs. Please make from_markdown fold non-empty continuation lines into the preceding memory and add a regression loading raw legacy Markdown from disk.
There was a problem hiding this comment.
Done in 51fe190. from_markdown now folds a non-empty bare line in the memories section back into the bullet it continues, so a pre-normalization file like - Deploy staging. followed by Then deploy production. loads as one memory instead of dropping the tail before MemoryTopicRecord normalization ever runs. Heading-looking lines still cannot be swallowed into a memory. Added test_memory_topic_record_folds_legacy_continuation_lines (raw legacy markdown in, folded memory out, plus a write-read round-trip); it fails against the previous parser and passes now. Full test_harness_memory.py green (36 passed), ruff check and format clean.
The constructor normalization only covers memories constructed after the fix; a pre-normalization file still loses its continuation on load because the memories section ignores bare lines. Fold a non-empty bare line back into the bullet it continues and cover the legacy shape, round-trip included.
GitHub Actions
Active
GitHub Actions
Active
|
Eduard van Valkenburg (@eavanvalkenburg) done, the body now uses the current template and the AI Assistance field is filled in (assisted areas: implementation, tests, PR description). |
GitHub Actions
Active
Motivation & Context
consolidate_memoriesaccepts a valid JSON response whosememoriesarray contains a string with an ordinary line break and reports success, but a freshMemoryFileStorereads back only the first line:to_markdownwrites the break verbatim as a non-bullet line in the Memories section, andfrom_markdownonly reads-bullets, so the continuation silently disappears on reload while the raw file still shows it. The write path does not hit this because_merge_memorynormalizes every memory through_normalize_memory_textbefore it reaches the record.Description & Review Guide
MemoryTopicRecord.__init__now runs each memory through_normalize_memory_text, the same normalizer the write path uses, while keeping the existing drop-empty tolerance. Single-line memories become a record invariant, so the markdown round-trip is stable no matter which path supplied the text.write_memory: line breaks fold into spaces instead of truncating on the next read. The on-disk format is unchanged, and existing single-line records are byte-identical after a rewrite._consolidate_topic, so every current and future intake path inherits the invariant.Related Issue
Fixes #9228
AI Assistance
AI assistance details: implementation, tests, and this PR description. The regression tests listed in the checks were actually run locally and pass.
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.