Skip to content

fix(memory): preserve full consolidation input - #5379

Open
dajiaohuang wants to merge 1 commit into
HKUDS:mainfrom
dajiaohuang:fix/5377-preserve-consolidation-input
Open

fix(memory): preserve full consolidation input#5379
dajiaohuang wants to merge 1 commit into
HKUDS:mainfrom
dajiaohuang:fix/5377-preserve-consolidation-input

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Summary

  • replace lossy consolidation truncation with lossless bounded chunks
  • defer history writes until every chunk succeeds, preserving raw fallback behavior on failure
  • cover token-triggered consolidation, idle compaction, oversized inputs, and Unicode chunk boundaries

Testing

  • uv run --no-sync ruff check nanobot/agent/memory.py nanobot/utils/helpers.py tests/agent/test_consolidator.py tests/utils/test_helpers.py
  • uv run --no-sync pytest tests/utils/test_helpers.py tests/agent/test_consolidator.py tests/agent/test_auto_compact.py tests/agent/test_memory_store.py tests/agent/test_dream.py -q (210 passed)
  • uv run --no-sync pytest -q (6190 passed, 51 skipped; 5 environment-specific failures reproduced on unmodified origin/main)

Fixes #5377

@Solaris-star Solaris-star left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked head fd50eb5c40fb179c765d9337cc80b75586c30315 in a clean checkout.

  • uv run --no-sync ruff check nanobot/agent/memory.py nanobot/utils/helpers.py tests/agent/test_consolidator.py tests/utils/test_helpers.py — passed
  • uv run --no-sync pytest tests/utils/test_helpers.py tests/agent/test_consolidator.py tests/agent/test_auto_compact.py tests/agent/test_memory_store.py tests/agent/test_dream.py -q210 passed
  • uv run --no-sync basedpyright nanobot/agent/memory.py nanobot/utils/helpers.py — 0 errors

The joined chunks are lossless, the cursor advances only after the complete archive attempt, and the delayed history writes avoid leaving a partial summary before the raw fallback. I did not find a blocking correctness issue in the reviewed files.

One operational trade-off worth keeping visible: very large histories now make serial provider calls and return a concatenated _last_summary, so latency/cost and injected summary size grow with the number of chunks. If that is intentional, the current tests document the behavior well; if not, a later pass could aggregate/cap the returned summary separately from the per-chunk history entries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix priority: p2 Normal backlog: minor bug, enhancement, docs, cleanup, edge case, or unvalidated proposal. test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: consolidation truncates archive input but advances past the full message batch

3 participants