fix(memory-core): preserve keyword-only hybrid results#92524
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 21, 2026, 2:25 AM ET / 06:25 UTC. Summary PR surface: Source +24, Tests +144. Total +168 across 2 files. Reproducibility: yes. from source and supplied proof: the manager path can produce a keyword-only weighted result below Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Rebase onto current Do we have a high-confidence way to reproduce the issue? Yes from source and supplied proof: the manager path can produce a keyword-only weighted result below Is this the best way to solve the issue? Unclear. The manager-side boundary is a plausible narrow fix and avoids inferring overlap from line ranges, but the best permanent rule must be compared with the active merge-layer approach before merge. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f1205f5f0bcd. Label changesLabel justifications:
Evidence reviewedPR surface: Source +24, Tests +144. Total +168 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (3 earlier review cycles) |
83e2b8c to
ece616c
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
This pull request has been automatically marked as stale due to inactivity. |
…textScore when keyword/vector chunk IDs do
0f5dd47 to
c461225
Compare
Summary
Fixes hybrid memory search so keyword-only FTS hits survive the post-merge minScore boundary even when another vector result already passes the strict threshold.
Fixes #92337.
Real Behavior Proof
/Users/stellarfish/openclaw-92337-boundary, running the memory-core manager throughnode_modules/.bin/tsx --conditions=developmentwith a real sqlite memory index, trigram FTS enabled, and a local mock embedding provider registered through OpenClaw's provider registry.memory/vector.mdandmemory/keyword.md, configuredstore.fts.tokenizer: "trigram", synced throughgetMemorySearchManager(...).sync(), then ranmanager.search("飞书", { maxResults: 10, minScore: 0.35 }).{ "query": "飞书", "fts": { "enabled": true, "available": true }, "resultCount": 2, "results": [ { "path": "memory/vector.md", "score": 0.7, "vectorScore": 1, "textScore": 0, "snippet": "# Vector\nvector-anchor semantic entry\n" }, { "path": "memory/keyword.md", "score": 0.3, "vectorScore": 0, "textScore": 1, "snippet": "# Keyword\n飞书插件配置 已完成\n" } ] }vectorScore: 0,textScore: 1, and weighted score0.3, even thoughminScoreis0.35.Changes
mergeHybridResultsexact-id semantics and leave the shared FTS token normalizer untouched.minScore, including overlapping vector+keyword hits.mergeHybridResults, including MMR/temporal ordering.Validation
Latest main refresh at
c461225:node scripts/run-vitest.mjs extensions/memory-core/src/memory/hybrid.test.ts— 15 passedoxfmtandoxlint— passedgit diff --check upstream/main...HEAD— passed@openclaw/fs-safe/advancedexportsopenclaw/ci-gateRisks
maxResults, so keyword-only hits can be trimmed if higher-ranked selected results fill the requested limit.