Skip to content

fix(recall): keep stable empty recall wrapper#329

Open
Arreboi06 wants to merge 1 commit into
TencentCloud:mainfrom
Arreboi06:feat/optimize-prompt-cache-hit-rate
Open

fix(recall): keep stable empty recall wrapper#329
Arreboi06 wants to merge 1 commit into
TencentCloud:mainfrom
Arreboi06:feat/optimize-prompt-cache-hit-rate

Conversation

@Arreboi06

@Arreboi06 Arreboi06 commented Jun 30, 2026

Copy link
Copy Markdown

Summary

Re-scoped per reviewer triage: #375 is now the canonical scoped runtime mitigation for #120, and #195 already owns Gateway/Hermes context splitting.

This PR is no longer trying to be the #120 baseline fix. It is a small follow-up candidate for the one behavior not covered by that baseline: keeping the existing <relevant-memories> dynamic recall wrapper stable when an L1 search runs but returns zero hits.

Review map

The diff is intentionally small. The quickest review path is:

What changed

  • Keep a <relevant-memories>...</relevant-memories> block when recall search is attempted and returns no L1 memories.
  • Use a short empty-state placeholder inside that existing wrapper.
  • Preserve existing behavior when real L1 memories are recalled.
  • Preserve existing behavior when recall search is skipped, such as empty user text.
  • Add focused tests around the zero-hit, hit, and skipped-search cases.

What this deliberately does not change

Why this is a follow-up to #375

#375 addresses the canonical OpenClaw runtime mitigation for #120: dynamic recall placement and history cleanup. This PR only adds the stable-wrapper detail so prefix-matching cache providers do not see the recall prompt layout alternate between "has wrapper" and "no wrapper" on L1 hit vs zero-hit turns.

The implementation stays inside performAutoRecall and distinguishes:

  • search attempted + zero hits -> stable empty wrapper
  • search attempted + hits -> existing real recall wrapper
  • search skipped -> no injected wrapper

Verification

  • TDD red check: npm test -- src/core/hooks/__tests__/stable-wrapper.test.ts failed before implementation because prependContext was undefined for zero-hit recall.
  • Focused test: npm test -- src/core/hooks/__tests__/stable-wrapper.test.ts -> 3 tests passed.
  • Full test suite: npm test -> 5 test files passed, 70 tests passed.
  • Build: npm run build -> plugin build and script TypeScript builds passed.
  • Whitespace: git diff --check origin/main...HEAD -> passed.

Build note: tsdown prints a Node.js v22.17.0 deprecation warning, but the build exits successfully.

@Yuntong8888

Copy link
Copy Markdown
Collaborator

Hi @Arreboi06 👋,
Thank you for submitting this PR and participating in Tencent Rhino-bird Open-source Training Program!
We have successfully received your submission. The program is currently in full swing, and we will complete the Code Review for you as soon as possible. Please keep an eye on the status notifications for this PR so you can follow up promptly once the review feedback is provided.
Thanks again for your contribution and open-source spirit! 🚀

@Arreboi06 Arreboi06 force-pushed the feat/optimize-prompt-cache-hit-rate branch from c77cf18 to 21de932 Compare July 8, 2026 18:39
@Arreboi06 Arreboi06 changed the title Feat/optimize prompt cache hit rate(#120) fix(recall): stabilize prompt cache context Jul 8, 2026
@Arreboi06

Arreboi06 commented Jul 8, 2026

Copy link
Copy Markdown
Author

Implementation note for reviewers

This update keeps PR #329 focused on issue #120 and rewrites the branch into a reviewable fix: 1 commit, scoped only to prompt-cache stability and recall-history cleanup.

Why this addresses #120

The regression described in #120 is caused by dynamic recall context changing the prompt shape across turns, which hurts prefix-matching prompt caches used by OpenAI-compatible providers.

This PR fixes the TencentDB-Agent-Memory side of that problem by introducing a stable cache contract:

  • Keeps a stable <relevant-memories>...</relevant-memories> wrapper even when no L1 memory is recalled.
  • Separates stable context from dynamic context by default:
    • stable persona / scene navigation / memory tool guide -> appendSystemContext
    • dynamic L1 recall snippets -> prependContext
  • Keeps backward compatibility for existing Gateway clients through context.
  • Adds cache-aware Gateway fields for newer clients:
    • append_system_context
    • prepend_context
    • stable_wrapper_used
  • Adds independent escape hatches:
    • recall.cacheOptimization.stableWrapper
    • recall.cacheOptimization.splitSystemContext
  • Extracts <relevant-memories> cleanup into a tested utility, so the stable wrapper improves prompt shape without being persisted into conversation history.

Why this is intentionally scoped

The original PR branch had unrelated adapter SDK changes mixed into the diff. This branch now excludes that unrelated work and only contains the issue #120 cache optimization. That should make review and merge much easier.

This PR provides the correct structured output for cache-aware placement. If OpenClaw wants to place stable additions before a host-side CACHE_BOUNDARY, it can now consume the separated fields without having to parse mixed recall text.

Verification

Verified locally:

  • npm test
    • 6 test files passed
    • 77 tests passed
  • npm run build
    • plugin build passed
    • script TypeScript builds passed
  • git diff --check
    • no whitespace errors
  • openclaw.plugin.json
    • JSON parse passed

The new behavior tests cover:

  • default cache optimization config
  • disabling stableWrapper
  • disabling splitSystemContext
  • stable no-memory wrapper behavior
  • dynamic L1 memories staying in prependContext
  • stable persona / tools guide staying in appendSystemContext
  • single-channel fallback when split mode is disabled
  • cleanup of injected recall wrappers before persistence

This keeps the implementation small while directly targeting the high-impact cache-hit regression reported in #120.

@Arreboi06 Arreboi06 force-pushed the feat/optimize-prompt-cache-hit-rate branch from 21de932 to 28792cb Compare July 8, 2026 19:37
@YOMXXX

YOMXXX commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Triage note: #375 is now the canonical scoped runtime mitigation for #120.

This PR overlaps on cache-friendly recall context and history cleanup, but introduces additional recall.cacheOptimization knobs plus Gateway split fields. #375 is the smaller verified baseline for #120; Gateway context splitting is already handled separately by #195 for Hermes.

Recommendation: re-scope as a follow-up only if stable wrapper behavior is still needed after #375. Avoid bundling Gateway response contract changes into the #120 OpenClaw runtime mitigation.

@Arreboi06 Arreboi06 force-pushed the feat/optimize-prompt-cache-hit-rate branch from 28792cb to 92d962d Compare July 9, 2026 15:35
@Arreboi06 Arreboi06 changed the title fix(recall): stabilize prompt cache context fix(recall): keep stable empty recall wrapper Jul 9, 2026
@Arreboi06

Arreboi06 commented Jul 9, 2026

Copy link
Copy Markdown
Author

Re-scoped this PR according to the triage note.

I removed the broader #120 baseline scope from #329:

The branch now contains only a small follow-up candidate: performAutoRecall keeps the existing <relevant-memories> wrapper stable when an L1 search is attempted but returns zero hits. Real recall hits are unchanged, and skipped recall still injects nothing.

This is intended to complement #375 rather than compete with it. #375 remains the canonical scoped runtime mitigation for #120; #195 remains the Gateway/Hermes split-context work.

Verification after re-scope:

  • npm test -- src/core/hooks/__tests__/stable-wrapper.test.ts: 3 tests passed
image
  • npm test: 5 test files passed, 70 tests passed
image
  • npm run build: passed
image
  • git diff --check origin/main...HEAD: passed
image

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.

3 participants