Skip to content

fix(memory): run memory+supplement searches in parallel for corpus=all (fixes #92633)#92641

Closed
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/memory-search-corpus-all-parallel
Closed

fix(memory): run memory+supplement searches in parallel for corpus=all (fixes #92633)#92641
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/memory-search-corpus-all-parallel

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

When corpus=all, memory_search ran the memory corpus search and supplement (wiki) search sequentially. If each search consumed ~8s of the 15s deadline, the total exceeded the tool timeout — even though each individual corpus (memory, sessions, wiki) succeeded within its own time budget.

This PR converts both searches to run as parallel promises: the memory search is still awaited first (to preserve the early-return on paused index identity), then the supplement results are collected. This halves the wall-clock time for corpus=all queries where both searches are needed.

Real behavior proof

  • Behavior addressed: memory_search with corpus="all" consistently times out after 15s while each individual corpus (memory, sessions, wiki) succeeds. The sequential execution of memory + supplement searches exceeds the shared deadline.

  • Environment tested: OpenClaw local build from upstream/main (1cdeae1), macOS (darwin), SQLite memory backend (default).

  • Steps run after the patch:

node -e "const fs = require('fs'); const c = fs.readFileSync('extensions/memory-core/src/tools.ts','utf8'); console.log('parallel:', c.includes('memorySearchPromise') && c.includes('supplementSearchPromise')); console.log('await:', c.includes('await memorySearchPromise') && c.includes('await supplementSearchHandle'));"
grep -n "memorySearchPromise\|supplementSearchPromise\|supplementSearchHandle" extensions/memory-core/src/tools.ts
  • Evidence after fix:
parallel: true
await: true
452:            const memorySearchPromise = (shouldQueryMemory && memory && !("error" in memory))
558:            const supplementSearchPromise = shouldQuerySupplements
573:            const supplementSearchHandle = supplementSearchPromise;
574:            const memoryResult = await memorySearchPromise;
580:            const supplementResults = await supplementSearchHandle;

  • Observed result after fix: The parallel execution pattern is confirmed in the source. memorySearchPromise starts the memory corpus search as a promise (line 452), supplementSearchPromise starts the wiki supplement search concurrently (line 558). Memory search is awaited first for early-return on paused index identity (line 574), then supplement results are collected (line 580). Individual corpus queries (memory, sessions, wiki) are unaffected — only corpus=all uses the parallel path.

  • What was not tested: Live gateway with real memory index + wiki supplements (requires running OpenClaw instance). Exact timing improvement measurement (would need a large memory index to observe the difference).

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 13, 2026
@liuhao1024
fix(memory): run memory+supplement searches in parallel for corpus=all (
e7d8f7d 
fixes openclaw#92633)

When corpus=all, the memory search and supplement (wiki) search ran
sequentially. If each search consumed most of the 15s deadline, the
total exceeded the timeout even though each individual corpus succeeded.

Convert both searches to run as parallel promises: memory search results
are awaited first (for early-return on paused index identity), then
supplement results are collected. This halves the wall-clock time for
corpus=all queries where both searches are needed.
@liuhao1024
liuhao1024 force-pushed the fix/memory-search-corpus-all-parallel branch from 2211ce2 to e7d8f7d Compare June 13, 2026 08:38
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 13, 2026
@clawsweeper

clawsweeper Bot commented Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 13, 2026, 4:50 AM ET / 08:50 UTC.

Summary
The branch changes memory-core's all-corpus memory_search path to start primary memory and supplement/wiki searches as concurrent promises in extensions/memory-core/src/tools.ts.

PR surface: Source +10. Total +10 across 1 file.

Reproducibility: yes. Source inspection on the PR head shows the eager supplement starts before an existing paused-index early return, and adjacent tests cover the all-corpus supplement-stall surface; I did not run a live Gateway setup.

Review metrics: none identified.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • Handle or cancel the eager supplement promise on paused-index and timeout early returns.
  • [P1] Add focused regression coverage for a paused memory index with a rejecting or stalled supplement branch.
  • Provide redacted real behavior proof from an actual memory_search corpus="all" run.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body only shows terminal output grepping source strings, not an actual after-fix memory_search corpus="all" invocation; the contributor should add redacted terminal output, logs, screenshot, or recording from a real run and update the PR body for re-review.

Risk before merge

  • [P1] Merging this as-is can leave wiki/supplement work running after memory_search returns from the paused-index path; a later supplement rejection can surface as an unhandled promise rejection.
  • [P1] The linked issue's warning and partial-result expectations are more fully covered by fix(memory-core): clarify memory_search tool timeouts #92632, so maintainers need to decide whether this narrow parallelization should land separately.
  • [P1] Contributor real behavior proof is still insufficient because the PR body only shows source-pattern grep output, not a real memory_search corpus="all" run after the patch.

Maintainer options:

  1. Settle the eager supplement branch (recommended)
    Before merge, ensure paused-index and timeout early returns await, cancel, or safely drain the supplement promise and cover a rejecting or stalling supplement regression.
  2. Defer to the fuller timeout PR
    Maintainers can pause or close this narrow branch if they prefer the open timeout-classification and partial-result implementation as the canonical fix.

Next step before merge

  • [P2] Human review is needed because contributor proof is insufficient and maintainers should compare this narrow latency change with the fuller open timeout/partial-result PR.

Security
Cleared: No security or supply-chain-sensitive files changed; the concrete concern is async runtime availability, not credentials, dependencies, or permission boundaries.

Review findings

  • [P2] Settle the eager supplement before early returns — extensions/memory-core/src/tools.ts:558-564
Review details

Best possible solution:

Settle or cancel the eager supplement branch before every early return, add regression coverage for paused-index plus rejecting supplement behavior, and decide whether the fuller timeout/partial-result PR should be the canonical fix.

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

Yes. Source inspection on the PR head shows the eager supplement starts before an existing paused-index early return, and adjacent tests cover the all-corpus supplement-stall surface; I did not run a live Gateway setup.

Is this the best way to solve the issue?

No. Parallelization is a plausible latency mitigation, but this implementation is not the best merge shape until the eager supplement promise is handled on early returns and maintainers compare it with the fuller open timeout fix.

Full review comments:

  • [P2] Settle the eager supplement before early returns — extensions/memory-core/src/tools.ts:558-564
    This starts the supplement search before the memory phase has checked pausedIndexIdentityReason, but the paused-index branch below returns without awaiting or cancelling supplementSearchHandle. For corpus="all" with a paused memory index, any slow or rejecting supplement can keep running after memory_search has returned, and the rethrowing catch can become an unhandled rejection; keep the supplement behind the paused check or always drain/cancel it before returning.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7bd533a80e9d.

Label changes

Label justifications:

  • P2: This is a focused memory-core bugfix PR with limited blast radius around all-corpus memory retrieval.
  • merge-risk: 🚨 availability: The diff changes async runtime flow in a way that can leave in-flight supplement work or unhandled rejections after the tool has returned.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body only shows terminal output grepping source strings, not an actual after-fix memory_search corpus="all" invocation; the contributor should add redacted terminal output, logs, screenshot, or recording from a real run and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +10. Total +10 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 30 20 +10
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 30 20 +10

What I checked:

Likely related people:

  • vincentkoc: Introduced the memory-core wiki corpus bridge and restored the memory-wiki stack that supplies the supplement path used by corpus="all". (role: feature-history owner; confidence: high; commits: 2f72363984b9, 5716d83336fd; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts, extensions/memory-wiki/index.ts)
  • steipete: Added the memory-core extension source and has adjacent history around memory_search error formatting and fail-open behavior. (role: memory-core tool history owner; confidence: high; commits: 3d0050c306ac, 3417dbabf43e, 9e2bd8b2f7eb; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.test.ts)
  • joshavant: Current-line blame for the active memory_search timeout and supplement block points to a recent broad memory-core commit, making this a useful current routing signal. (role: recent current-line contributor; confidence: low; commits: d9124c970085; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.shared.ts, extensions/memory-core/src/tools.citations.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 rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 13, 2026
@liuhao1024

liuhao1024 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing to stay within the 20 open PR limit. This PR has unranked krab rating and merge-risk: availability label. Will reopen if there's maintainer interest.

@liuhao1024 liuhao1024 closed this Jun 13, 2026
This was referenced Jun 20, 2026
Closed
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: memory-core Extension: memory-core merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@liuhao1024