Skip to content

fix(dedup): filter duplicate advisories on cosine similarity, not display rank - #1721

Open
ebarkhordar wants to merge 1 commit into
agentic-community:mainfrom
ebarkhordar:fix/1696-dedup-threshold-vs-display-score
Open

fix(dedup): filter duplicate advisories on cosine similarity, not display rank#1721
ebarkhordar wants to merge 1 commit into
agentic-community:mainfrom
ebarkhordar:fix/1696-dedup-threshold-vs-display-score

Conversation

@ebarkhordar

Copy link
Copy Markdown
Contributor

DuplicateCheckService compares dedup_score_threshold against relevance_score, but under the default rrf fusion that value is a position in a result list, not a similarity. _normalize_scores min-max rescales the fused scores, and all three of its return paths hand exactly 1.0 to the highest scorer (search_repository.py:290). Since 1.0 always clears 0.7, the filter at duplicate_check_service.py:342 cannot reject the top candidate whatever the registry holds.

The normalization is doing its job. d97a24b6 added it so the UI would stop rendering RRF scores as "1-3% match"; the advisory is just reading a display number as if it were a similarity.

That makes the shared -mcp-server suffix incidental to this report. Against a registry holding only payroll-mcp-server and weather-mcp-server, registering topic-mcp-server still returns an advisory at 1.0, on true cosines of 0.2648 and 0.2313.

Fix

search() now carries similarity_score next to relevance_score: the cosine of the hit against the query embedding, stamped by _attach_similarity_scores on the hybrid and client-side paths. relevance_score and the UI are untouched. The advisory ranks and filters on similarity_score, and a hit arriving without one is treated as not comparable rather than admitted.

This also takes the non-default linear branch's (cosine + 1) / 2 mapping out of the threshold's meaning, where a configured 0.7 was really asking for cosine 0.4.

The existing advisory fixtures set only relevance_score, which is no longer the field that gates, so I renamed it there to keep them testing what they were testing.

Verification

  • Four new behavioural tests in test_duplicate_check_service.py fail on main and pass here, plus test_search_similarity_scores.py for the helper.
  • Full tests/unit suite on Python 3.14: 6594 passed, 8 skipped.
  • ruff check, ruff format and mypy clean at the pinned pre-commit versions.
  • Two gaps. The matching call on the MongoDB CE client-side fallback is not covered, since that method has no test harness in the repo. And a registration with an empty description still embeds the bare name, where topic-mcp-server and category-mcp-server sit at 0.7621, so the boilerplate effect described in the report survives that case.

Stripping catalog tokens before embedding would close that second one, but it changes what gets indexed rather than how a score is read, so it seemed worth keeping separate.

Refs #1696

…play rank

Under the default rrf fusion, relevance_score is a position in a result
list rather than a similarity: _normalize_scores min-max rescales the
fused scores so the top hit is always exactly 1.0. The duplicate-check
advisory compared that against dedup_score_threshold, so the highest
scorer cleared the filter whatever the registry held.

search() now carries similarity_score, the cosine of the hit against the
query embedding, on both paths that have one. DuplicateCheckService ranks
and filters on it and skips hits that arrive without one. relevance_score
keeps its display meaning.

Refs agentic-community#1696
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...istry/repositories/documentdb/search_repository.py 86.66% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

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.

2 participants