Skip to content

feat(prompts): add facet guard and length limits to memory_merge_bundle#533

Open
lishixiang0705 wants to merge 1 commit intovolcengine:mainfrom
lishixiang0705:fix/memory-merge-facet-guard-and-length-limits
Open

feat(prompts): add facet guard and length limits to memory_merge_bundle#533
lishixiang0705 wants to merge 1 commit intovolcengine:mainfrom
lishixiang0705:fix/memory-merge-facet-guard-and-length-limits

Conversation

@lishixiang0705
Copy link
Copy Markdown

Problem

The current memory_merge_bundle.yaml template has three issues that degrade memory quality over time:

  1. No facet coherence check — Two memories sharing the same category (e.g. both preferences) are always merged, even when they describe completely unrelated topics (e.g. "Python code style" + "food preferences"). This produces bloated, semantically diluted memories.

  2. No length constraints — Merged content grows unbounded. In production, single memory items can exceed 1000+ characters, causing:

    • Embedding dilution (vector search scores cluster in a narrow band with ~0.02 spread)
    • High token cost when memories are injected into LLM context
    • Low retrieval precision
  3. Accumulate-only strategy — The template instructs to "keep non-conflicting details", which means memories only grow, never condense. Old, superseded facts persist alongside new ones.

Solution

Upgrade template to v2.0.0 with three changes:

1. Facet guard (decision: skip)

Before merging, the LLM must verify both memories describe the same specific facet/topic. If they cover different facets, output {"decision": "skip"} to keep them separate. Includes concrete examples for both cases.

2. Hard character limits

  • abstract: ≤ 80 characters
  • overview: ≤ 200 characters
  • content: ≤ 300 characters

When limits would be exceeded, aggressively summarize — drop older details first, preserve specific values (names, numbers, versions) over narrative.

3. Condensed snapshot strategy

Replace "keep non-conflicting details" with:

  • Conflicts → keep newer version only
  • Non-conflicts → condense to essential facts
  • Result should read as a clean, up-to-date snapshot — not a changelog

Breaking Changes

  • The decision field now accepts "skip" in addition to "merge". Callers that parse the merge output must handle this new value.
  • Merged content will be significantly shorter than before. Downstream systems that rely on verbose L2 content may need adjustment.

Testing

Tested in production with ~2800 vectors and ~76 leaf memory files. The facet guard correctly prevents cross-topic merges, and length limits keep individual memories within embedding-friendly bounds.

- Add 'skip' decision: reject merging memories with different facets
  even if they share the same category, preventing semantic dilution
- Add hard character limits: abstract ≤80, overview ≤200, content ≤300
- Change merge strategy from accumulate-all to condensed snapshot:
  conflicts resolved by keeping newer version only
- Bump template version from 1.0.0 to 2.0.0

Motivation: without facet checking, the merge prompt would combine
unrelated facts (e.g. 'Python code style' + 'food preferences') into
a single bloated memory just because both were categorized as
'preferences'. Without length limits, merged memories grew unbounded
(some exceeding 1000+ chars), causing embedding dilution and low
retrieval precision in downstream vector search.
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


lishixiang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

ZaynJarvis added a commit that referenced this pull request Apr 17, 2026
… fix

Bundles three in-flight contributor PRs (#533, #549, #951) with reviewer
feedback addressed, consolidated into a single set of focused edits.

memory_extraction.yaml (#549):
- Add length targets to the Three-Level Structure section: abstract
  ~50-80 chars, overview 3-5 bullets, content 2-4 sentences.
- Kept the concise guidance Zayn approved; dropped the BAD/GOOD content
  example blocks he flagged as redundant with the few-shot examples
  below, and kept all text in English per yangxinxin-7's language-mixing
  concern.

memory_merge_bundle.yaml (#533):
- Add facet coherence check: same category is not sufficient to merge;
  memories covering different facets (e.g. Python code style + food
  preference) must output {"decision": "skip"}.
- Add hard length limits: abstract ≤ 80, overview ≤ 200, content ≤ 300.
- Switch merge strategy from accumulate-all to condensed snapshot: on
  conflict keep newer value; do not retain superseded details.
- Bump template version 1.0.0 → 2.0.0.

memory_extractor.py (#549):
- Vectorize on `abstract or content` instead of `content`. Shorter text
  yields more discriminative embeddings and reduces score clustering.

semantic_processor.py + model_retry.py (#951):
- Fix memory semantic queue stall: _process_memory_directory() had two
  silent early-return paths (ls failure, write_file failure) that let
  on_dequeue() hit report_success() while the work actually failed —
  telemetry got marked_failed, but the queue's in_progress counter and
  processed count treated the message as done. Re-raise as RuntimeError
  so on_dequeue routes to report_error().
- Classify local filesystem errors (FileNotFoundError, PermissionError,
  IsADirectoryError, NotADirectoryError — including chained __cause__)
  as "permanent" in classify_api_error, so a bad path fails the queue
  entry instead of being re-enqueued forever.

Tests:
- tests/utils/test_circuit_breaker.py: cover the four filesystem error
  types and a chained FileNotFoundError.
- tests/storage/test_memory_semantic_stall.py: exercise on_dequeue
  through the real classifier — ls failure must hit on_error, empty dir
  must still hit on_success (no regression).
@ZaynJarvis
Copy link
Copy Markdown
Collaborator

Cherry-picked into #1522 for batch merge along with #549 and #951. The facet guard + length limits + condensed-snapshot strategy are all preserved; I tightened the prompt body a bit (39 changed lines vs. the original 64) but the three behavioural changes are identical.

Memory-quality effect is pending eval — I'll run retrieval precision / score-spread numbers on the bundled PR before merging. Thanks for the work @lishixiang0705.

@ZaynJarvis
Copy link
Copy Markdown
Collaborator

Following up on the earlier cherry-pick attempt — closed #1522 and re-did the cherry-pick properly so your original commit is preserved as-is via git cherry-pick (author attribution intact). Prompt-bundle changes (#533 + #549) are now in #1530 — memory effect eval pending before merge.

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants