lib/segment/src/index/hnsw_index/tests/mod.rs (1)

41-44: Refactored to separate scoring and filtering

The code now retrieves both a raw scorer and a filterer using get_scorer_and_filterer, replacing the previous approach with FakeFilterContext and FilteredScorer. This change is consistent with the PR's objective of decoupling scoring and filtering mechanisms.

The clone() on added_vector might be unnecessary since the vector is already passed by value. Consider reviewing if this clone operation is required:

-        let (raw_scorer, filterer) = vector_holder
-            .get_scorer_and_filterer(added_vector.clone())
-            .unwrap();
+        let (raw_scorer, filterer) = vector_holder
+            .get_scorer_and_filterer(added_vector)
+            .unwrap();

lib/segment/src/vector_storage/dense/memmap_dense_vector_storage.rs (1)

677-681: Improved result vector preparation with new_unset constructor

The implementation now uses a more direct approach to prepare the result vector by mapping over query points and using the new ScoredPointOffset::new_unset() constructor. This eliminates the need for vector resizing and makes the code more readable and efficient.

Consider adding a comment explaining that using new_unset() initializes the score with a placeholder value (0.0) that will be updated by score_points() to help future developers understand the flow.

  let mut res = query_points
      .iter()
      .map(|&idx| ScoredPointOffset::new_unset(idx))
      .collect::<Vec<_>>();
+ // The scores are initialized with placeholder values and will be updated by score_points
  scorer.score_points(&mut res);

lib/segment/src/vector_storage/async_raw_scorer.rs (1)

73-88: Refined score_points API
Now score_points directly updates the scores array in place, simplifying the interface. The early return on is_stopped is good for responsiveness. Using Cell for in-place mutation in the closure is well-implemented.

Consider providing a fallback strategy if io_uring is not supported, e.g. logging a warning or using the synchronous path, to avoid unexpected panic in production.

lib/segment/src/index/hnsw_index/point_filterer.rs (1)

58-69: Fix spelling in documentation.

The words performace and indended are misspelled in these lines.

Please apply this diff to correct them:

-/// This method operates on [`ScoredPointOffset`] for performace reasons.
-/// The indended usage is to prepare a vector of [`ScoredPointOffset`]
+/// This method operates on [`ScoredPointOffset`] for performance reasons.
+/// The intended usage is to prepare a vector of [`ScoredPointOffset`]

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 61-61:
performace ==> performance

---

[failure] 62-62:
indended ==> intended, indented

🪛 GitHub Actions: Codespell

[warning] 61-61: codespell: 'performace' is a misspelling. Suggested correction: 'performance'.

lib/segment/benches/hnsw_build_asymptotic.rs (1)

35-35: Handle potential panic on .unwrap().

Each of these lines calls .unwrap(), which will panic if get_scorer_and_filterer returns an error. While it may be acceptable in benchmarking code, using .expect("<message>") or an error-handling strategy can provide clearer diagnostics when failures occur.

- let (raw_scorer, filterer) = vector_holder.get_scorer_and_filterer(query).unwrap();
+ let (raw_scorer, filterer) = vector_holder
+     .get_scorer_and_filterer(query)
+     .expect("Failed to retrieve scorer and filterer");

Also applies to: 56-56, 63-63, 72-72, 80-80, 94-94, 111-111, 127-127, 143-143

lib/segment/src/index/hnsw_index/graph_layers.rs (1)

425-425: Avoid potential panic on .unwrap().

Using .unwrap() can cause unwanted panics if get_scorer_and_filterer fails. Consider a more descriptive .expect() or a proper error-handling approach to make failures more explicit.

- let (raw_scorer, filterer) = vector_holder.get_scorer_and_filterer(added_vector).unwrap();
+ let (raw_scorer, filterer) = vector_holder
+     .get_scorer_and_filterer(added_vector)
+     .expect("Failed to retrieve scorer and filterer");

lib/segment/benches/hnsw_build_graph.rs (1)

31-33: 💡 Verification agent

🧩 Analysis chain

Refactored to separate scoring and filtering

The code now retrieves both a raw scorer and a filterer directly from the vector holder, replacing the previous approach of using FakeFilterContext and FilteredScorer. This change aligns with the PR's objective of decoupling scoring and filtering mechanisms.

Consider handling potential errors from get_scorer_and_filterer more gracefully instead of using unwrap(), which could cause panics in case of failure:

---

🏁 Script executed:

#!/bin/bash
# Search for other instances of error handling with get_scorer_and_filterer
rg "get_scorer_and_filterer.*unwrap" --type rust

Length of output: 2698

---

Enhance Error Handling for get_scorer_and_filterer

The refactor effectively decouples scoring and filtering by retrieving both components directly from the vector holder. However, our search indicates that using unwrap() with get_scorer_and_filterer is pervasive across the codebase (including in this file, e.g., in lib/segment/benches/hnsw_build_graph.rs on lines 31–33, and elsewhere). This approach could lead to panics if the function ever fails.

• Action Items:
  • Review and refactor error handling for get_scorer_and_filterer both here and in other affected files (e.g., in lib/segment/src/index/hnsw_index/graph_layers.rs and lib/segment/src/index/hnsw_index/graph_layers_builder.rs).
  • Consider returning more informative errors or using methods like expect() with a clear message, or refactor to propagate errors gracefully.

lib/segment/src/vector_storage/raw_scorer.rs:941

• [nitpick] Consider renaming this method (or updating its documentation) to emphasize that it fills an existing scores buffer in-place instead of returning a new vector; this would clarify that the scores slice must be pre-initialized (e.g. using ScoredPointOffset::new_unset).

fn score_points(&self, scores: &mut [ScoredPointOffset]) {

lib/segment/src/fixtures/index_fixtures.rs:170

• [nitpick] Consider refactoring the repeated pattern of obtaining a RawScorer and its corresponding PointsFilterer into a dedicated helper function or utility to reduce boilerplate at call sites.

pub fn get_scorer_and_filterer(

lib/common/common/src/iterator_ext/mod.rs (1)

47-55: Well-implemented benchmarking utility!

The black_box method is a good addition for benchmarking purposes. It correctly applies std::hint::black_box to each item in the iterator to prevent compiler optimizations during benchmarks.

If you prefer a more concise style, the closure could be simplified to:

-    self.for_each(|p| {
-        std::hint::black_box(p);
-    });
+    self.for_each(std::hint::black_box);

However, the current implementation is explicit and clear, so this is purely optional.

lib/segment/src/index/sparse_index/sparse_vector_index.rs (2)

302-309: Leverage quantized vectors if present for cheaper scoring

FilteredScorer::new accepts an optional quantized_vectors parameter, but we always pass None.
When the underlying storage has a pre‑built quantized representation, using it typically reduces RAM traffic and SIMD cost during scoring.

-        let scorer = FilteredScorer::new(
-            query_vector.clone(),
-            &vector_storage,
-            None,
-            None,
-            deleted_point_bitslice,
-            vector_query_context.hardware_counter(),
-        )?;
+        let scorer = FilteredScorer::new(
+            query_vector.clone(),
+            &vector_storage,
+            vector_storage.quantized_vectors(),   // <- returns `Option<&QuantizedVectors>`
+            None,
+            deleted_point_bitslice,
+            vector_query_context.hardware_counter(),
+        )?;

(If quantized_vectors() is not available on VectorStorageEnum, you can expose it behind a helper.)
This is a pure optimisation; functional behaviour remains identical.

---

318-323: Passing the iterator by reference is unnecessary

peek_top_iter takes its iterator by value, and &mut I implements Iterator only indirectly.
Simply move the iterator to the call‑site for slightly cleaner code and avoid the extra reference indirection:

-                let res = scorer.peek_top_iter(&mut filtered_points, top, &is_stopped)?;
+                let res = scorer.peek_top_iter(filtered_points, top, &is_stopped)?;

lib/segment/src/vector_storage/quantized/quantized_scorer_builder.rs (1)

52-83: Heavy match‑cascade could be centralised

The triple‑nested match on (datatype, distance) duplicates very similar branches 18 times.
Consider a helper such as

fn route<TElement, TMetric>(self, builder: fn(Self) -> OperationResult<Box<dyn RawScorer<'a>>>)

or a small macro to collapse repetition.
Less duplication eases future distance additions (e.g. CosineSquared) and reduces the chance of forgetting one of the 18 arms.

lib/segment/src/index/hnsw_index/point_scorer.rs (2)

143-152: Slice scores_buffer to avoid scanning unused entries

zip(&*point_ids, &self.scores_buffer) iterates the entire buffer, although only the first point_ids.len() slots are freshly written.
Use the same slice you gave to score_points:

-        std::iter::zip(&*point_ids, &self.scores_buffer)
+        std::iter::zip(&*point_ids, &self.scores_buffer[..point_ids.len()])

Saves needless cache traffic when the buffer had previously grown large.

---

147-152: scores_buffer never shrinks – potential long‑lived memory spike

The buffer only ever grows; if one request with 1 M points arrives, every subsequent tiny request will still carry a 1 M‑entry buffer.

Consider resetting it when point_ids.len() falls under a fraction of current capacity, or allocate fresh per call and reuse via thread‑local pool.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro