Skip to content

fix(storage): prevent full-text scores from contributing as vector scores - #260

Merged
mergify[bot] merged 1 commit into
mainfrom
codex/fix-hybrid-vector-score
Sep 28, 2026
Merged

mergify[bot] merged 1 commit into
mainfrom
codex/fix-hybrid-vector-score

Conversation

@muuuuuuushroom

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • fix (bug fix)

Which issue(s) this PR fixes

Fixes #259

What this PR does / why we need it

Full-text-only candidates retained their raw full-text score in retrieval_score, which the hybrid scorer then interpreted as a vector score. This gave those candidates an unintended additional ranking contribution.

Preserve full-text scores in the existing keyword-score map and clear the vector-facing score only when appending full-text-only candidates. Candidates returned by vector search, including overlapping candidates, keep their vector scores. The merge is extracted into a private helper so its score provenance can be tested without a database.

Validation

  • Passed: cargo test --manifest-path memoria/Cargo.toml -p memoria-storage hybrid_merge_keeps_fulltext_scores_out_of_vector_score --lib (1 test). This calls the production merge helper with mocked vector-only, overlapping, and full-text-only memories and checks candidate order, deduplication, and score provenance.
  • Red/green verified: removing only memory.retrieval_score = None makes the same test fail (F: Some(2.0) instead of None); restoring it makes the test pass.
  • git diff --check passes. Package formatting check reports pre-existing formatting differences outside this change; those were left untouched.

Database-backed integration tests have not been run.

Copilot AI lite review requested due to automatic review settings September 23, 2026 10:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@aptend aptend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review of exact head a936812 against base/merge-base 9962999. The change only clears retrieval_score when a full-text result is appended as a full-text-only candidate; the score is first retained in ft_map, and overlapping candidates keep their vector-origin score. I traced the downstream hybrid scorer, which reads retrieval_score solely as vec_score and ft_map as kw_score, and checked deduplication/order and error paths. The focused regression and all 49 runnable memoria-storage library tests pass; one DB-dependent test is intentionally ignored, and database-backed integration was not rerun locally. CI checks are green and git diff --check is clean. No blocking issue found.

@mergify

mergify Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Merge Queue Status

  • ✅ Entered queue — 2026-09-28 03:44 UTC · Rule: main · triggered by rule autoqueue
  • ✅ Checks skipped · PR is already up-to-date
  • ✅ Merged — 2026-09-28 03:45 UTC · at 9101313ba91c06e731d5f1d168de81692137d3d7 · squash

This pull request spent 9 seconds in the queue, including 2 seconds running CI.

Required conditions to merge

@mergify
mergify Bot merged commit 9101313 into main Sep 28, 2026
5 checks passed
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.

[Bug]: Full-text-only candidates incorrectly receive a vector contribution in hybrid scoring

3 participants