fix(memory): preserve FTS5 BM25 relevance in bm25RankToScore (#5767) by lsdcc01 · Pull Request #33757 · openclaw/openclaw

lsdcc01 · 2026-03-04T01:57:22Z

Summary

Problem: FTS5 bm25() returns negative values (more negative = more relevant), but bm25RankToScore clamped all negative values to 0 via Math.max(0, rank), resulting in identical normalized scores (1.0) for all keyword matches.
Why it matters: Hybrid ranking effectively treated keyword search as a boolean signal, discarding BM25 relevance information.
What changed: For negative ranks, use relevance = -rank and score = relevance/(1+relevance); for non-negative ranks, keep existing 1/(1+rank) behavior.
What did NOT change: Non-negative rank handling preserved.

Change Type

Bug fix

Scope

Memory / storage

Linked Issue/PR

Closes [Bug]: bm25RankToScore always return same value #5767

User-visible / Behavior Changes

Hybrid search (vector + keyword) now correctly ranks BM25 matches by relevance instead of treating all matches equally.

Security Impact

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Evidence

Failing test/log before + passing after: Added test "bm25RankToScore preserves FTS5 BM25 relevance (more negative = higher score)"; all hybrid tests pass.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

…w#5767)

greptile-apps · 2026-03-04T01:59:08Z

Greptile Summary

This PR fixes a bug in bm25RankToScore where all FTS5 BM25 keyword matches were returning an identical score of 1.0, effectively reducing hybrid search to a boolean keyword signal with no relevance differentiation.

Root cause: SQLite FTS5's bm25() function returns negative values where more negative means more relevant. The old implementation used Math.max(0, rank) which clamped every negative rank to 0, then computed 1/(1+0) = 1.0 for every match regardless of relevance.

Fix: For negative ranks, use relevance = -rank and score = relevance/(1+relevance), which correctly maps to (0, 1) with the ordering preserved (more negative rank → higher score).

Key observations:

The formula change is mathematically sound: both branches now map finite ranks into (0, 1].
There is a theoretical discontinuity at rank = 0 (left-limit → 0, but bm25RankToScore(0) = 1.0), however this is not a practical issue since FTS5 only returns 0 for non-matching rows that are excluded from search results.
The toBeCloseTo(1, 1) precision relaxation in the test is necessary and correct: 100/101 ≈ 0.9901 which passes at 1-decimal precision but not at the previous default of 2.
The new test correctly verifies strict ordering for three distinct negative ranks.

Confidence Score: 5/5

This PR is safe to merge — it fixes a well-understood normalization bug with a correct mathematical transformation and adds appropriate test coverage.
The change is narrowly scoped to a single pure function (bm25RankToScore). The fix is mathematically correct, backward compatible (non-negative rank behavior is preserved, non-finite fallback unchanged), and accompanied by a focused test that verifies the strict ordering invariant. No regressions are expected.
No files require special attention.

_{Last reviewed commit: ec77563}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec77563c8b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-04T02:01:57Z

src/memory/hybrid.ts

+  }
+  if (rank < 0) {
+    const relevance = -rank;
+    return relevance / (1 + relevance);


Keep keyword fallback reachable with finite BM25 scores

The new negative-rank mapping returns textScore < 1 for every finite FTS5 match, so keyword-only hybrid scores become strictly less than textWeight. In src/memory/manager.ts:335, the relaxed fallback still uses relaxedMinScore = min(minScore, textWeight), which means with the default minScore=0.35 and textWeight=0.3 a keyword-only result can no longer satisfy either filter (score = textWeight * textScore < 0.3). This regresses the fallback path that is supposed to keep lexical hits when vector results are absent/weak.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1438069d5f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-04T02:17:42Z

pnpm-lock.yaml

@@ -876,16 +876,16 @@ packages:
    resolution: {integrity: sha512-/42G73JkuYdyWZ6m8d/CJtBrGl1Hegyc7Fy78m5Ob+jF85TOUmLR5XLce/U3LxYAw0kJ8CT5aI99RIvPHcGp/Q==}

  '@cloudflare/workers-types@4.20260120.0':
-    resolution: {integrity: sha512-B8pueG+a5S+mdK3z8oKu1ShcxloZ7qWb68IEyLLaepvdryIbNC7JVPcY0bWsjS56UQVKc5fnyRge3yZIwc9bxw==}
+    resolution: {integrity: sha1-wBnoj9UlUhhtvQ4IZL+oD7NGYzU=, tarball: https://artsz.zte.com.cn:443/artifactory/api/npm/it-npm-virtual/@cloudflare/workers-types/-/workers-types-4.20260120.0.tgz}


Remove private mirror tarballs from lockfile

This lockfile update pins package resolutions to https://artsz.zte.com.cn/... tarballs, so pnpm install will try to fetch from that private Artifactory host instead of the public registry. In any CI runner or contributor environment without access to that network, dependency installation will fail, which blocks builds and tests for this commit.

Useful? React with 👍 / 👎.

…prod CI

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2cbcd2613

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-04T02:29:42Z

src/memory/hybrid.ts

+  if (rank < 0) {
+    const relevance = -rank;
+    return relevance / (1 + relevance);


Keep finite BM25 scores able to pass keyword fallback

The new negative-rank normalization returns textScore < 1 for every finite FTS5 hit, so keyword-only hybrid scores become strictly less than textWeight. In the fallback path (src/memory/manager.ts), relaxed filtering still uses min(minScore, textWeight), which means with defaults (minScore=0.35, textWeight=0.3) keyword-only results can no longer meet the relaxed threshold when vector matches are absent/zero—the exact scenario this fallback is meant to preserve.

Useful? React with 👍 / 👎.

@lsdcc01

Land #33757 by @lsdcc01 without the unrelated dependency bump. Preserve negative FTS5 BM25 ordering in hybrid scoring and add changelog coverage for #5767. Co-authored-by: 丁春才0668000523 <ding.chuncai1@xydigit.com>

steipete · 2026-03-07T22:42:06Z

Landed on main.

What I did:

rebased the fix onto current main
kept the BM25 ordering fix from this PR
dropped the unrelated dependency-bump commit from the landing
kept the regression test coverage for negative FTS5 BM25 ordering
added a changelog entry
ran pnpm lint, pnpm build, and pnpm test before landing

Hashes:

landed commit on main: e45d62ba26ab6565cb78c47bfe5d47d163981550
original PR fix commit: ec77563c8b8df08b0747cd4445b9d2b901cf28ea
omitted unrelated dep-bump commit: b2cbcd2613756464d7da3557d9d5fbc91b695e08

Thanks @lsdcc01.

…lsdcc01