Skip to content

fix(intelligence): soft mark forgotten memories - #984

Merged
lightzt99 merged 1 commit into
oceanbase:mainfrom
wayyoungboy:fix/intelligence-forget-soft-mark
Jun 8, 2026
Merged

fix(intelligence): soft mark forgotten memories#984
lightzt99 merged 1 commit into
oceanbase:mainfrom
wayyoungboy:fix/intelligence-forget-soft-mark

Conversation

@wayyoungboy

Copy link
Copy Markdown
Member

Summary

This PR completes the intelligent-forgetting soft-mark behavior introduced for Memory.get() in #282.

When EbbinghausIntelligencePlugin returns delete_flag=True, memories should be marked as forgotten instead of being physically removed from storage. The sync Memory.get() path already does this, but Memory.search(), AsyncMemory.get(), and AsyncMemory.search() still called delete_memory / delete_memory_async.

Changes

  • Add a forget-marker payload helper for sync and async memory modules.
  • Update Memory.search() to call update_memory() with should_forget and marked_for_forgetting_at instead of deleting.
  • Update AsyncMemory.search() to call update_memory_async() instead of deleting.
  • Update AsyncMemory.get() to soft mark and return the current result, matching Memory.get() behavior.
  • Add regression tests covering sync search, async search, and async get soft-mark behavior.

Test plan

  • pytest tests/unit/test_intelligence_forget_soft_mark.py -v
  • pytest tests/unit/intelligence -v
  • pytest tests/unit/test_memory.py -v
  • git diff --check

@lightzt99

Copy link
Copy Markdown
Collaborator

@wayyoungboy The four-path alignment in this PR is correct. While reviewing, we traced the full lifecycle of the forget marker and found a broader architectural gap worth discussing.

What this PR fixes

Before #282, all four intelligence-plugin paths hard-deleted memories. #282 fixed only Memory.get(). This PR fixes the remaining three — Memory.search(), AsyncMemory.get(), and AsyncMemory.search() — so all four paths now consistently soft-mark rather than physically delete.

The write/read gap

When a memory is soft-marked, two fields are written to storage:

should_forget = True
marked_for_forgetting_at = <timestamp>

However, a full-text search of the codebase shows these fields are written but never read by any component:

Component Reads should_forget? Effect
Storage search/list/get Forgotten memories returned as normal
API response transform Fields dropped before reaching consumers
Plugin on_get/on_search Recalculates should_forget() from scratch each time
GC / cleanup job Does not exist

The same gap exists for review_schedule, which process_memory_metadata() generates on every memory access but get_review_schedule() has zero external callers.

The Ebbinghaus lifecycle: four signals, two consumers

Signal              Written by              Consumed by
─────────────────────────────────────────────────────────
review_schedule     process_memory_metadata()   ❌ none
should_forget       on_get / on_search          ❌ none
should_promote      process_memory_metadata()   ✅ IntelligentMemoryManager
should_archive      process_memory_metadata()   ✅ IntelligentMemoryManager

The promotion/archival signals have a consumer (optimize_memories() manages in-memory tiers). The forget and review signals produce data that no component acts on.

Suggested next steps

The soft-mark fix in this PR is a necessary first step, but the forget mechanism becomes meaningful only when a reader exists. Two natural follow-ups:

  1. Filter layer: storage queries should exclude should_forget=True memories by default, with an opt-in flag (include_forgotten=True) for audit/recovery scenarios.
  2. GC job: periodically hard-delete memories where marked_for_forgetting_at exceeds a configurable retention window (e.g., 30 days), giving a two-stage safety net: soft-mark → grace period → hard delete.

These are outside the scope of this PR but worth tracking as follow-up issues.

@wayyoungboy

Copy link
Copy Markdown
Member Author

Review update: no blocking issues found in this PR.

I agree with the new comment that should_forget / marked_for_forgetting_at still need a reader path to become fully effective, such as default filtering plus an opt-in include_forgotten mode, and eventually a retention/GC job. That is a real follow-up gap, but I would not block this PR on it: this change fixes the narrower inconsistency where Memory.search(), AsyncMemory.get(), and AsyncMemory.search() still hard-deleted while Memory.get() had already moved to soft marking.

Local verification:

PYTHONPATH=src pytest tests/unit/test_intelligence_forget_soft_mark.py -q
# 3 passed

git diff --check
# passed

@lightzt99

Copy link
Copy Markdown
Collaborator

Thanks @wayyoungboy for the thorough analysis. Agreed — this PR closes the consistency gap, and the filter/GC follow-up is tracked separately. Merging.

@lightzt99
lightzt99 merged commit 85a46c4 into oceanbase:main Jun 8, 2026
17 checks passed
ShiXing321 pushed a commit to ShiXing321/powermem that referenced this pull request Jul 15, 2026
Co-authored-by: LINxiansheng <LINxiansheng@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants