Skip to content

fix #80582: Memory: skip markdown placeholder snippets during short-term promotion#92698

Closed
mushuiyu886 wants to merge 4 commits into
openclaw:mainfrom
mushuiyu886:feat/issue-80582
Closed

fix #80582: Memory: skip markdown placeholder snippets during short-term promotion#92698
mushuiyu886 wants to merge 4 commits into
openclaw:mainfrom
mushuiyu886:feat/issue-80582

Conversation

@mushuiyu886

@mushuiyu886 mushuiyu886 commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Problem: Short-term memory promotion could rank and promote markdown placeholder snippets from daily-note scaffolding.
  • Solution: Filter bare marker-only fragments and heading-plus-empty-marker markdown skeletons at every short-term promotion boundary before they can reach MEMORY.md.
  • What changed: Recording, recall-store normalization, ranking, direct candidate application, and rehydrated candidate application now share the same promotability check.
  • What did NOT change: This does not change public API, config, schema, provider routing, channel behavior, MEMORY.md format, or valid real-content promotion; external providers and channel semantics remain out of scope.
  • Why it matters / User impact: Empty markdown scaffolding should not become durable memory; filtering it keeps long-term memory focused on useful facts instead of polluting future context.
  • Fixes Memory: skip markdown placeholder snippets during short-term promotion #80582

Real behavior proof

  • Behavior or issue addressed: Short-term promotion could treat snippets such as -, *, >, fences, table separators, empty checkbox/bracket scaffolds such as [], - [ ], [x], or - [x], or ## Tagesnotizen\n-\n\n## Entscheidungen\n- as useful memory candidates and write empty placeholders into long-term MEMORY.md.
  • Real environment tested: Local OpenClaw worktree for extensions/memory-core, using the memory-core plugin-state test runtime and the real short-term promotion record/rank/apply functions against local memory files.
  • Exact steps or command run after this patch:
node scripts/test-projects.mjs extensions/memory-core/src/short-term-promotion.test.ts
node --import tsx --input-type=module <minimal memory promotion proof script>
pnpm exec oxfmt --check --threads=1 extensions/memory-core/src/short-term-promotion.ts extensions/memory-core/src/short-term-promotion.test.ts
git diff --check
  • Evidence after fix:
Test Files  1 passed (1)
Tests  82 passed (82)
[test] passed 1 Vitest shard in 46.47s

placeholder_candidates=0
valid_candidates=3
applied=3
has_valid_promotion=true
has_heading_promotion=true
has_checkbox_promotion=true
has_skeleton_promotion=false
has_empty_checkbox_promotion=false
has_bracket_promotion=false
has_checked_empty_promotion=false
minimal_memory_promotion_proof=passed

All matched files use the correct format.
git diff --check passed
  • Observed result after fix: Placeholder-only snippets are not recorded or ranked as promotion candidates. A real daily-note bullet, a meaningful heading-only snippet, and a checkbox snippet with real text all rank and promote successfully, and the generated MEMORY.md contains the valid memories while excluding markdown skeleton, empty checkbox, bracket-only, and checked-empty checkbox sections.
  • What was not tested: No live external provider, channel, Desktop, or remote memory service was tested. This patch is limited to the local memory-core short-term promotion path and its local MEMORY.md promotion behavior.

Review findings addressed

  • RF-001 Preserve meaningful heading snippets: Added a focused preserve case for ## API keys rotated; heading-only snippets with durable content now remain promotable and are proven to reach MEMORY.md.
  • RF-002 Reject marker-only markdown scaffolds: Expanded the shared placeholder-line classifier and tests to reject blockquote markers, fence markers, table separators, and repeated marker-only fragments without blocking real text.
  • RF-003 Reject empty checkbox/bracket placeholders: Added focused reject coverage for [ ], [], and - [ ], plus preserve coverage proving - [ ] rotate API keys and - [x] backups verified still promote.
  • RF-004 Reject checked empty checkbox placeholders: Extended the empty checkbox classifier and regression coverage to reject [x], [X], - [x], and - [X] while keeping checked checkbox items with real text promotable.

Regression Test Plan

  • Target test file: extensions/memory-core/src/short-term-promotion.test.ts
  • Scenario locked in: Markdown placeholder snippets are skipped when recorded/ranked, direct candidates that rehydrate to markdown skeletons are not applied to MEMORY.md, marker-only fragments and empty unchecked/checked checkbox or bracket scaffolds are rejected, and meaningful heading-only and checkbox-with-content snippets still promote.
  • Why this is the smallest reliable guardrail: The test exercises the memory-core promotion functions that own the regression without requiring external providers or unrelated channel setup.

Merge risk

  • Risk labels considered: None.
  • Risk explanation: Compatibility/default behavior is preserved for real-content memory promotions. The diff only narrows short-term memory promotion eligibility for empty markdown placeholders; it does not change public API, config, schema, provider routing, channel behavior, MEMORY.md format, or migration behavior. Related PR scan/issue initialization did not identify an open linked PR that already covers Memory: skip markdown placeholder snippets during short-term promotion #80582; changing external providers, channels, or long-term memory schema is out of scope.
  • Why acceptable: The change is focused to the memory-core promotion path, keeps real-content bullets and meaningful headings promotable, and is covered by targeted regression tests plus a local end-to-end promotion proof.

Root Cause

  • Root cause: The root cause is a missing validation invariant in the short-term promotion source path: only empty or dreaming-contaminated snippets were rejected, while markdown scaffolding from daily notes could normalize into non-empty placeholder strings. Because those placeholder strings passed state recording, ranking, rehydration, and application, they could be persisted as durable memory.
  • Why this is root-cause fix: This fixes the source invariant because the shared promotability validation now runs before snippets enter the recall state and again before rehydrated candidates are applied. The change blocks the invalid placeholder state at the promotion owner instead of masking the symptom with downstream MEMORY.md cleanup after pollution has already occurred.
  • Fix classification: Root cause fix.
  • Maintainer-ready confidence: High.
  • Patch quality notes: Production code and regression tests are paired; scope is limited to extensions/memory-core short-term promotion.
  • Architecture / source-of-truth check: short-term-promotion.ts owns the source-of-truth path for record, rank, rehydrate, and apply decisions before long-term memory is written, so the filter is placed before downstream MEMORY.md persistence.

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 13, 2026
@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 13, 2026, 11:18 AM ET / 15:18 UTC.

Summary
The PR adds a shared memory-core short-term promotion promotability predicate and regression tests to filter markdown placeholder snippets during store normalization, recording, ranking, direct apply, and rehydrated apply.

PR surface: Source +46, Tests +262. Total +308 across 2 files.

Reproducibility: yes. Current main is source-reproducible because non-empty markdown placeholders pass the existing empty/dreaming-contamination filters into recording, ranking, and apply; the linked issue also includes a sanitized workspace cleanup signal.

Review metrics: 1 noteworthy metric.

  • Promotion eligibility gates: 1 persisted load gate, 5 runtime gates changed. These gates decide which short-term memory state survives normalization, recording, ranking, direct apply, and rehydration before reaching MEMORY.md.

Stored data model
Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/short-term-promotion.test.ts, unknown-data-model-change: extensions/memory-core/src/short-term-promotion.ts, vector/embedding metadata: extensions/memory-core/src/short-term-promotion.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR intentionally drops placeholder-only persisted short-term recall entries during normalization, ranking, direct apply, and rehydrated apply; maintainers should be comfortable with that session-state pruning before merge.

Maintainer options:

  1. Merge with memory-pruning awareness (recommended)
    Accept that placeholder-only short-term recall entries are dropped at load, rank, and apply time, then merge once normal required checks stay green.
  2. Pause for broader memory semantics
    If maintainers want an explicit policy for pruning existing recall-store entries, pause until that session-memory semantics decision is made.

Next step before merge

  • No ClawSweeper repair lane is needed; the latest head addresses the prior predicate hole and leaves only normal maintainer merge review.

Security
Cleared: The diff changes memory-core filtering logic and colocated tests only, with no dependency, workflow, package metadata, secret-handling, or code-execution surface changes.

Review details

Best possible solution:

Land the shared promotability guard once maintainers accept the intentional pruning of placeholder-only short-term recall entries and required checks remain green.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main is source-reproducible because non-empty markdown placeholders pass the existing empty/dreaming-contamination filters into recording, ranking, and apply; the linked issue also includes a sanitized workspace cleanup signal.

Is this the best way to solve the issue?

Yes. Centralizing the guard in short-term-promotion.ts at each entry and reuse boundary is the best owner-boundary shape; downstream MEMORY.md cleanup alone would leave polluted recall state behind.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 45056a463ab2.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a local memory-core promotion path showing placeholders skipped, valid snippets promoted, and MEMORY.md excluding the tested skeletons.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a local memory-core promotion path showing placeholders skipped, valid snippets promoted, and MEMORY.md excluding the tested skeletons.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a focused memory-quality bug fix with limited blast radius, but it affects durable MEMORY.md promotion behavior.
  • merge-risk: 🚨 session-state: The PR changes which short-term recall entries survive normalization, ranking, and apply before becoming durable memory.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a local memory-core promotion path showing placeholders skipped, valid snippets promoted, and MEMORY.md excluding the tested skeletons.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a local memory-core promotion path showing placeholders skipped, valid snippets promoted, and MEMORY.md excluding the tested skeletons.
Evidence reviewed

PR surface:

Source +46, Tests +262. Total +308 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 53 7 +46
Tests 1 262 0 +262
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 315 7 +308

What I checked:

  • Root policy read: Read the full root AGENTS.md and applied the PR review depth, plugin-boundary, session-state, and proof guidance. (AGENTS.md:1, 45056a463ab2)
  • Scoped extension policy read: Read the scoped extensions guide; this change stays inside the bundled memory-core plugin boundary and does not expand SDK surface. (extensions/AGENTS.md:1, 45056a463ab2)
  • Current issue context: The linked issue reports placeholder-only snippets and heading-plus-empty-bullet skeletons reaching MEMORY.md, with a sanitized field cleanup signal of 89 placeholder promotion entries removed.
  • Current main recording behavior: Current main only rejects empty or dreaming-contaminated snippets during recall recording, so non-empty markdown placeholders can enter the short-term recall store. (extensions/memory-core/src/short-term-promotion.ts:1396, 45056a463ab2)
  • Current main ranking/apply behavior: Current main ranking and apply paths filter dreaming contamination but not markdown placeholders, so stored non-empty placeholders can still be ranked, rehydrated, and written. (extensions/memory-core/src/short-term-promotion.ts:1769, 45056a463ab2)
  • PR predicate and gates: PR head adds isPromotableShortTermSnippet, rejects checked empty boxes, and wires the shared predicate into store normalization, recall recording, grounded recording, ranking, direct apply, and rehydrated apply. (extensions/memory-core/src/short-term-promotion.ts:353, 0ac1c19199b1)

Likely related people:

  • vignesh07: Authored the merged weighted short-term promotion flow that introduced the ranking/apply surface this PR modifies. (role: feature introducer; confidence: high; commits: 4c1022c73b39; files: extensions/memory-core/src/short-term-promotion.ts, extensions/memory-core/src/short-term-promotion.test.ts)
  • mbelinky: Authored merged REM preview/replay and grounded backfill changes that expanded the same short-term promotion and grounded candidate paths. (role: recent area contributor; confidence: high; commits: 79348f73c8b6, e8209e4cf9b8; files: extensions/memory-core/src/short-term-promotion.ts, extensions/memory-core/src/short-term-promotion.test.ts, extensions/memory-core/src/dreaming-phases.ts)
  • gumadeiras: Authored the merged dreaming self-ingestion guard, the closest existing contamination-filtering precedent in this promotion pipeline. (role: adjacent filtering contributor; confidence: medium; commits: 0c4e0d703023; files: extensions/memory-core/src/short-term-promotion.ts, extensions/memory-core/src/short-term-promotion.test.ts)
  • Peter Steinberger: Authored nearby memory-core promotion alignment and short-term record-helper refactors in the same module history. (role: recent area contributor; confidence: medium; commits: f7670bde7efa, 61fbc9ad2e5d; files: extensions/memory-core/src/short-term-promotion.ts, extensions/memory-core/src/short-term-promotion.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 13, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: M and removed size: S proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 13, 2026
@mushuiyu886

mushuiyu886 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the two P2 findings in the latest head:

  • preserves meaningful heading snippets such as ## API keys rotated and proves they still promote
  • rejects marker-only placeholders including blockquotes, fences, table separators, and repeated markers

@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 13, 2026
@mushuiyu886

mushuiyu886 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the checked-empty checkbox finding in the latest head:

  • rejects [x], [X], - [x], and - [X] as placeholder-only snippets
  • keeps checked checkbox items with real text, such as - [x] backups verified, promotable
  • refreshed local memory-core test/proof evidence in the PR body

@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 13, 2026
@mushuiyu886 mushuiyu886 mentioned this pull request Jun 13, 2026
Merged
@vincentkoc vincentkoc self-assigned this Jun 13, 2026
@vincentkoc

vincentkoc commented Jun 14, 2026

Copy link
Copy Markdown
Member

maintainer deep review confirmed placeholder-only short-term recall/promotion is a real bug, but this PR is not safe to land.

After several canonical repair attempts and 130 focused passing tests, final autoreview still found the core heuristic is both over- and under-broad: meaningful headings are dropped when QMD includes adjacent formatting; standard empty ordered lists/tagged fences still pass; legacy flattened checkbox skeletons survive; and recall audit events report skipped hits as recorded.

This is no longer a narrow regex fix. The canonical repair needs a typed/source-shape-aware normalization step that extracts meaningful content before storage/promotion and records accepted/skipped audit results truthfully, rather than continuing to grow ambiguous Markdown heuristics.

I am closing this PR rather than landing silent memory loss or incomplete filtering. The linked issue should remain open for that bounded representation-level fix.

Focused proof passed (130 tests and git diff --check), but final autoreview correctly remains blocking.

@vincentkoc vincentkoc closed this Jun 14, 2026
@github-actions github-actions Bot mentioned this pull request Jun 14, 2026
Open
This was referenced Jun 18, 2026
Closed
Closed
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

extensions: memory-core Extension: memory-core merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory: skip markdown placeholder snippets during short-term promotion

2 participants

@mushuiyu886 @vincentkoc