Skip to content

fix: skip qmd zero-hit sync retry in memory_search#90117

Closed
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:fix/issue-90023-qmd-zero-hit
Closed

fix: skip qmd zero-hit sync retry in memory_search#90117
arkyu2077 wants to merge 1 commit into
openclaw:mainfrom
arkyu2077:fix/issue-90023-qmd-zero-hit

Conversation

@arkyu2077

@arkyu2077 arkyu2077 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • skip the forced sync-and-retry path when memory.backend = "qmd" returns zero hits
  • keep the existing retry behavior for other backends
  • add a regression test covering qmd zero-hit searches

Testing

  • pnpm vitest run extensions/memory-core/src/tools.test.ts

Closes #90023

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 8:53 AM ET / 12:53 UTC.

Summary
The branch adds a QMD backend guard around the memory_search zero-hit sync retry and a regression test asserting QMD zero-hit searches do not retry.

PR surface: Source +4, Tests +24. Total +28 across 2 files.

Reproducibility: yes. at source level: current main still synchronously forces manager.sync({ reason: "search", force: true }) after zero-hit searches, and the canonical issue includes live QMD reports for that path. I did not run an independent live QMD timing repro in this read-only review.

Review metrics: 1 noteworthy metric.

  • QMD zero-hit fallback: 1 backend-specific retry condition changed. QMD no longer uses the generic forced sync/retry after empty results, so maintainers should review the latency versus freshness tradeoff before merge.

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

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90023
Summary: This PR is a candidate fix for the canonical QMD zero-hit memory_search forced-sync stall; related PRs overlap, but none is merged or currently a clean superseding close target.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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:

  • [P1] Add redacted real QMD proof showing zero-hit memory_search returns promptly without a forced QMD update.
  • Refresh the branch against current main while preserving the QMD-only guard and regression coverage.
  • [P1] Coordinate with the proof-supplied sibling so maintainers land one canonical fix path.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists only a Vitest command and no after-fix real QMD CLI, gateway, log, live-output, or recording evidence for this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The contributor has not supplied after-fix real QMD output for this branch; the PR body lists only a Vitest command.
  • [P1] GitHub reports the branch is conflicting, so it needs refresh before it can be a landing candidate.
  • [P2] Skipping QMD's zero-hit forced sync changes the existing stale-index freshness fallback: QMD users can receive prompt empty results instead of immediate search-time repair.
  • [P1] A proof-sufficient sibling PR targets the same guard but is also conflicting, so maintainers need to pick one canonical fix path.

Maintainer options:

  1. Refresh and prove the narrow guard (recommended)
    Resolve the conflict and require redacted QMD CLI, gateway, log, or live output showing zero-hit memory_search returns promptly without forcing a QMD update.
  2. Accept prompt empty QMD results
    Maintainers can intentionally accept the latency-over-freshness tradeoff where QMD zero-hit searches return empty instead of forcing immediate repair.
  3. Choose a canonical sibling
    Pause this branch if maintainers prefer the proof-supplied sibling or a broader bounded-sync approach as the single landing path.

Next step before merge

  • [P1] Human follow-up is needed because the branch is conflicting, contributor proof is missing, and maintainers should choose the canonical fix path among overlapping PRs.

Security
Cleared: No concrete security or supply-chain concern found; the diff only changes memory-core runtime branching and a colocated regression test.

Review details

Best possible solution:

Land one refreshed canonical memory-core fix that keeps QMD update work out of the interactive zero-hit memory_search path while preserving non-QMD retry behavior or moving QMD repair to a bounded/background refresh.

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

Yes at source level: current main still synchronously forces manager.sync({ reason: "search", force: true }) after zero-hit searches, and the canonical issue includes live QMD reports for that path. I did not run an independent live QMD timing repro in this read-only review.

Is this the best way to solve the issue?

Yes as a narrow mitigation, but it is not merge-ready yet. If maintainers require immediate QMD freshness repair on zero hits, a bounded or background refresh path is safer than an unconditional synchronous update.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a focused memory-core bug fix for a real QMD memory_search stall with limited blast radius.
  • merge-risk: 🚨 compatibility: Skipping QMD's zero-hit forced sync changes fallback behavior for users whose QMD index is stale during search.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists only a Vitest command and no after-fix real QMD CLI, gateway, log, live-output, or recording evidence for this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +4, Tests +24. Total +28 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 5 1 +4
Tests 1 24 0 +24
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 29 1 +28

What I checked:

Likely related people:

  • TurboTheTurtle: Recent merged work kept memory_search in transient QMD mode and touched both the tool and QMD manager surfaces. (role: recent memory_search and QMD lifecycle contributor; confidence: high; commits: ddacb7ba39d4, 6aff1e8f9ea4, 9408380ae729; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • dreamhunter2333: Authored the recent memory_search deadline/AbortSignal work around the same tool timeout path. (role: recent memory_search timeout contributor; confidence: medium; commits: 8d72cb9401e5; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/tools.test.ts)
  • vincentkoc: Current shallow blame carries the zero-hit block through a recent snapshot, and recent QMD watcher/probe work is adjacent to this search/sync path. (role: recent QMD area contributor; confidence: medium; commits: 35d7cb0bffe7, dbcae5b78d9f, 6aff1e8f9ea4; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts)
  • osolmaz: Recent history touched memory index identity and QMD query/rerank configuration near stale-index and zero-hit behavior. (role: adjacent memory index and QMD config contributor; confidence: medium; commits: a4b4fed41287, 0dbf17471b41, b7a5bcba7886; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts, packages/memory-host-sdk/src/host/backend-config.ts)
  • steipete: Recent commits adjusted memory recall stall handling and QMD timeout/config defaults near this behavior. (role: recent memory-core hardening contributor; confidence: medium; commits: 9e2bd8b2f7eb, 1ac037d94894, c36ba9ea7a2d; files: extensions/memory-core/src/tools.ts, extensions/memory-core/src/memory/qmd-manager.ts, packages/memory-host-sdk/src/host/backend-config.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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 4, 2026
This was referenced Jun 19, 2026
Closed
Merged
Open
Closed
Closed
Closed
This was referenced Jul 1, 2026
Closed
Closed
@vincentkoc vincentkoc closed this Jul 1, 2026
@vincentkoc

vincentkoc commented Jul 1, 2026

Copy link
Copy Markdown
Member

Closed as superseded by #90030, merged in 90e31be.

The landed fix keeps the zero-hit retry for one-shot QMD CLI searches, where it bootstraps a newly configured index, and skips only the synchronous retry for the long-lived QMD manager path that caused interactive stalls. This PR removes the one-shot bootstrap retry too, so the narrower landed behavior is the canonical fix.

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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

QMD memory_search zero-hit queries trigger synchronous force sync and stall interactive turns

2 participants

@arkyu2077 @vincentkoc