Skip to content

Conversation

@cryo-zd
Copy link
Contributor

@cryo-zd cryo-zd commented Nov 4, 2025

What type of PR is this?

fix: correct HNSW frontier comparisons in hybrid cache

What this PR does / why we need it:
  This PR fixes the sign bug in both func (h *HybridCache) searchLayerHybrid and func (h *HybridCache) searchLayerHybridWithEarlyStop: the code compared currentDist against -results.data[0].dist even though distances were already stored as negatives, so saturated frontiers kept enqueuing worse neighbors instead of pruning them.
  ( the stopping condition (currentDist > -results.data[0].dist) almost never fires, and the “is this better than the frontier?” check (dist < -results.data[0].dist) is nearly always true.)
  This prevents needless graph traversal, keeps lookup latency under control, and avoids returning responses routed through poor matches.

// searchLayerHybrid searches for nearest neighbors at a specific layer
func (h *HybridCache) searchLayerHybrid(query []float32, ef int, layer int, entryPoints []int) []int {
// Reuse buffers from pool to reduce allocations
buf := getSearchBuffers()
defer putSearchBuffers(buf)
visited := buf.visited
candidates := buf.candidates
results := buf.results
for _, ep := range entryPoints {
if ep < 0 || ep >= len(h.embeddings) {
continue
}
dist := -dotProduct(query, h.embeddings[ep])
candidates.push(ep, dist)
results.push(ep, dist)
visited[ep] = true
}
for len(candidates.data) > 0 {
currentIdx, currentDist := candidates.pop()
if len(results.data) > 0 && currentDist > -results.data[0].dist {
break
}

Which issue(s) this PR fixes:

Fixes #

Release Notes: Yes/No

Signed-off-by: cryo <zdtna412@gmail.com>
@netlify
Copy link

netlify bot commented Nov 4, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit df9014c
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/690a1a302147ce0007e07ac8
😎 Deploy Preview https://deploy-preview-587--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 src

Owners: @rootfs, @Xunzhuo, @wangchen615
Files changed:

  • src/semantic-router/pkg/cache/cache_test.go
  • src/semantic-router/pkg/cache/hybrid_cache.go

vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

@rootfs rootfs requested a review from Copilot November 5, 2025 23:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical bug in the HNSW (Hierarchical Navigable Small World) search algorithm for the hybrid cache. The issue was with incorrect distance comparisons that caused the search to accept weaker candidates and fail to properly prune branches, leading to inefficient searches and potentially incorrect results.

Key changes:

  • Removed redundant negations in distance comparisons (lines 949, 967, 1078, 1101)
  • Added clarifying comments explaining the negative dot product convention (lines 941, 1065)
  • Added comprehensive test coverage to verify proper pruning behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/semantic-router/pkg/cache/hybrid_cache.go Fixed distance comparison logic by removing double negations in searchLayerHybrid and searchLayerHybridWithEarlyStop functions; added explanatory comments
src/semantic-router/pkg/cache/cache_test.go Added regression test to ensure weak branches are properly pruned during hybrid layer search

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rootfs rootfs merged commit 2d0338d into vllm-project:main Nov 5, 2025
22 checks passed
@rootfs
Copy link
Collaborator

rootfs commented Nov 5, 2025

@cryo-zd thank you for testing this!

@cryo-zd cryo-zd deleted the fix/hnsw branch November 5, 2025 23:44
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.

4 participants