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

76-76: Typo in progress bar template.

“Buildng” → “Building”.

-                ProgressStyle::with_template("{percent:>3}% Buildng HNSW {wide_bar}").unwrap(),
+                ProgressStyle::with_template("{percent:>3}% Building HNSW {wide_bar}").unwrap(),

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

519-596: Force-load/convert flow looks correct; consider elevating conversion log level.

• Order-of-preference load for None and conversion for Some(...) are implemented cleanly with appropriate Advice usage (Random for steady-state, Sequential during conversion).
• Suggest using info for the conversion summary so operators notice one-off startup conversions.

-                    log::debug!(
+                    log::info!(
                         "Converted HNSW graph links in {:.1?}: {:.1}MB -> {:.1}MB ({:.1}%)",

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

100-112: Validate base vector layout in try_new (cheap early check).

You already guard on quantized layout; also verify base layout to fail fast when storage can’t provide it.

 impl<'a> StorageGraphLinksVectors<'a> {
     pub fn try_new(
         vector_storage: &'a VectorStorageEnum,
         quantized_vectors: Option<&'a QuantizedVectors>,
     ) -> Option<Self> {
         let quantized_vectors = quantized_vectors?;
-        quantized_vectors.get_quantized_vector_layout().ok()?;
+        // Ensure both link and base layouts are obtainable
+        quantized_vectors.get_quantized_vector_layout().ok()?;
+        vector_storage.get_vector_layout().ok()?;
         Some(Self {
             vector_storage,
             quantized_vectors,
         })
     }
 }

---

187-212: Use saturating math and name “magic numbers” in format selection.

Prevents debug-overflow panics/wrap-around and documents the 3-byte id estimate and 2-page cap.

 impl<'a> GraphLinksFormatParam<'a> {
     /// Decide which format to use.
     pub fn recommended<Q: GraphLinksVectors>(
         feature_flags: &FeatureFlags,
         hnsw_m0: usize,
         vectors: Option<&'a Q>,
     ) -> Self {
-        let is_small_enough = |layout: &GraphLinksVectorsLayout| {
-            let mut node_size_bytes = 0;
-            node_size_bytes += layout.base.size();
-            node_size_bytes += hnsw_m0 * 3; // Rough estimate: 3 bytes per compressed id
-            node_size_bytes += hnsw_m0 * layout.link.size();
-            node_size_bytes <= 2 * PAGE_SIZE_BYTES
-        };
+        const COMPRESSED_ID_EST_BYTES: usize = 3;
+        const NODE_SIZE_THRESHOLD_BYTES: usize = 2 * PAGE_SIZE_BYTES;
+        let is_small_enough = |layout: &GraphLinksVectorsLayout| {
+            let node_size_bytes = layout
+                .base
+                .size()
+                .saturating_add(hnsw_m0.saturating_mul(COMPRESSED_ID_EST_BYTES))
+                .saturating_add(hnsw_m0.saturating_mul(layout.link.size()));
+            node_size_bytes <= NODE_SIZE_THRESHOLD_BYTES
+        };
 
         if feature_flags.hnsw_with_vectors
             && let Some(vectors) = vectors
             && let Ok(layout) = vectors.vectors_layout()
             && (is_small_enough(&layout) || feature_flags.hnsw_format_force)
         {
             GraphLinksFormatParam::CompressedWithVectors(vectors)
         } else {
             GraphLinksFormatParam::Compressed
         }
     }
 }

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

167-194: Warn when force-enable is requested but vectors/layout are unavailable.

Current logic silently drops forcing in this case; emit a warning to surface misconfiguration. Also split the match arm for clarity.

-            let force_format = match (
-                flags.hnsw_format_force,
-                flags.hnsw_with_vectors,
-                &graph_links_vectors,
-            ) {
-                // Normal flow - no feature flags enabled
-                (false, _, _) => None,
-
-                // No quantized vectors (or layout is not available) - can't force the format
-                (true, _, None) => None,
-
-                // Force format without vectors
-                (true, false, _) => Some(GraphLinksFormatParam::Compressed),
-
-                // Force format with vectors
-                (true, true, Some(graph_links_vectors)) => Some(
-                    GraphLinksFormatParam::CompressedWithVectors(graph_links_vectors),
-                ),
-            };
+            let force_format = match (
+                flags.hnsw_format_force,
+                flags.hnsw_with_vectors,
+                &graph_links_vectors,
+            ) {
+                // Normal flow - no feature flags enabled
+                (false, _, _) => None,
+
+                // Forcing requested but we cannot enable vector format
+                (true, true, None) => {
+                    log::warn!(
+                        "hnsw_format_force=true and hnsw_with_vectors=true, \
+                         but quantized vectors/layout are unavailable; skipping conversion."
+                    );
+                    None
+                }
+
+                // Force format without vectors
+                (true, false, _) => Some(GraphLinksFormatParam::Compressed),
+
+                // Force format with vectors
+                (true, true, Some(graph_links_vectors)) => Some(
+                    GraphLinksFormatParam::CompressedWithVectors(graph_links_vectors),
+                ),
+            };

Follow-up: Confirm this aligns with the PR’s “convert on startup” promise when forcing is set. If conversion is mandatory, consider erroring instead of warning.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

Learnt from: generall
PR: qdrant/qdrant#6479
File: lib/segment/src/index/hnsw_index/graph_layers_builder.rs:137-182
Timestamp: 2025-05-30T15:26:14.488Z
Learning: In the subgraph_connectivity method in graph_layers_builder.rs, the previous_visited_points vector and visited BitVec entry point marking are automatically re-initialized at the start of each retry loop iteration, so manual clearing is not needed.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7193
File: lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs:17-30
Timestamp: 2025-09-01T11:19:26.371Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs, the ChunkedMmapVectors underlying implementation does not validate against zero-sized vectors, so adding such validation in QuantizedChunkedMmapStorage::new is unnecessary and would be redundant.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs:46-55
Timestamp: 2025-08-11T00:37:34.100Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs`, the `ChunkedVectors` used in `QuantizedRamStorage` is intentionally designed to be non-persistent during updates. The `push_vector` method only updates the in-memory vectors without flushing to disk, and this is expected behavior rather than a bug.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

docs/redoc/master/openapi.json (2)

11757-11761: Tighten the public description of hnsw_with_vectors (avoid internal jargon).

The phrase “CompressedWithVectors format” is fine, but consider adding a 1‑liner on what it means for users (e.g., stores vectors with graph to reduce random I/O at some space cost). Also, note in docs that enabling it only affects newly written segments unless forcing is enabled. Add a “Since” note for versioned clarity.

Because this file is generated, please update lib/api/src/rest/schema.rs doc comments or #[schemars(description = "...")] accordingly.

Example in Rust (schema source), adjust wording as needed:

/// Allow writing the HNSW graph in the "CompressedWithVectors" format.
/// Stores node vectors alongside links to reduce random I/O (higher disk usage).
/// Affects only newly written segments unless `hnsw_format_force` is enabled.
/// Since: 1.xx.x
#[schemars(description = "Allow writing the HNSW graph in the `CompressedWithVectors` format. Stores node vectors alongside links to reduce random I/O (higher disk usage). Affects only newly written segments unless `hnsw_format_force` is enabled.")]
pub hnsw_with_vectors: bool,

---

11762-11766: Avoid Rust-specific link in OpenAPI and clarify conversion semantics for hnsw_format_force.

The text “based on the value of [Self::hnsw_with_vectors]” won’t render well in Redoc. Replace with plain reference “based on hnsw_with_vectors”. Also explicitly state behavior when false (no forced conversion; existing segments keep their current format) and warn about potential startup-time cost due to conversion.

Please change in schema.rs so this JSON is regenerated with cleaner wording.

Proposed source doc text:

/// Force conversion of existing HNSW graphs on startup to match `hnsw_with_vectors`.
/// - If true: convert to `CompressedWithVectors` when `hnsw_with_vectors` is true, otherwise to `Compressed`.
/// - If false: do not force-convert existing segments; only new writes follow the current format selection.
/// Conversion may increase startup time; performed atomically.
/// Since: 1.xx.x
#[schemars(description = "Force conversion of existing HNSW graphs on startup to match `hnsw_with_vectors`. If true, convert to `CompressedWithVectors` when `hnsw_with_vectors` is true, otherwise to `Compressed`. If false, existing segments keep their format; only new writes follow the selected format. Conversion may increase startup time.")]
pub hnsw_format_force: bool,

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

531-541: Be resilient if a higher-preference links file exists but fails to load

If the first existing file (e.g., CompressedWithVectors) is present but corrupted, we currently bail out instead of trying the next available format. Consider continuing to the next format on load error.

Apply this diff:

-                for format in formats {
-                    let path = GraphLayers::get_links_path(dir, format);
-                    if path.exists() {
-                        return GraphLinks::load_from_file(&path, on_disk, format, Advice::Random);
-                    }
-                }
+                for format in formats {
+                    let path = GraphLayers::get_links_path(dir, format);
+                    if path.exists() {
+                        match GraphLinks::load_from_file(&path, on_disk, format, Advice::Random) {
+                            Ok(links) => return Ok(links),
+                            Err(err) => {
+                                log::warn!(
+                                    "Failed to load HNSW links at {:?} as {:?}: {err}. Trying next format.",
+                                    path,
+                                    format
+                                );
+                            }
+                        }
+                    }
+                }

---

594-595: Improve error text with directory context

Including the directory path simplifies troubleshooting when no links file is found.

-        Err(OperationError::service_error("No links file found"))
+        Err(OperationError::service_error(format!(
+            "No HNSW links file found in {:?}",
+            dir
+        )))

lib/segment/src/index/hnsw_index/graph_links.rs (3)

187-211: Make threshold math explicit and overflow-safe

Tiny readability/robustness tweak: factor out the “3 bytes per id” estimate and use saturating math.

     pub fn recommended<Q: GraphLinksVectors>(
         feature_flags: &FeatureFlags,
         hnsw_m0: usize,
         vectors: Option<&'a Q>,
     ) -> Self {
-        let is_small_enough = |layout: &GraphLinksVectorsLayout| {
-            let mut node_size_bytes = 0;
-            node_size_bytes += layout.base.size();
-            node_size_bytes += hnsw_m0 * 3; // Rough estimate: 3 bytes per compressed id
-            node_size_bytes += hnsw_m0 * layout.link.size();
-            node_size_bytes <= 2 * PAGE_SIZE_BYTES
-        };
+        const COMPRESSED_ID_BYTES_ESTIMATE: usize = 3; // rough estimate per compressed id
+        let is_small_enough = |layout: &GraphLinksVectorsLayout| {
+            let mut node_size_bytes = layout.base.size();
+            node_size_bytes = node_size_bytes
+                .saturating_add(hnsw_m0.saturating_mul(COMPRESSED_ID_BYTES_ESTIMATE));
+            node_size_bytes = node_size_bytes
+                .saturating_add(hnsw_m0.saturating_mul(layout.link.size()));
+            node_size_bytes <= 2 * PAGE_SIZE_BYTES
+        };

---

146-159: Add a panic message for clarity in tests

Micro DX improvement when tests misconfigure vectors.

-                None => panic!(),
+                None => panic!("CompressedWithVectors requires vectors in tests"),

---

9-11: Drop unused import

Madviseable isn’t referenced in this module.

-use memory::madvise::{Advice, AdviceSetting, Madviseable};
+use memory::madvise::{Advice, AdviceSetting};

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: generall
PR: qdrant/qdrant#6479
File: lib/segment/src/index/hnsw_index/graph_layers_builder.rs:137-182
Timestamp: 2025-05-30T15:26:14.488Z
Learning: In the subgraph_connectivity method in graph_layers_builder.rs, the previous_visited_points vector and visited BitVec entry point marking are automatically re-initialized at the start of each retry loop iteration, so manual clearing is not needed.

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

Learnt from: generall
PR: qdrant/qdrant#7006
File: docs/redoc/master/openapi.json:13645-13652
Timestamp: 2025-08-10T18:26:12.443Z
Learning: In qdrant, docs/redoc/master/openapi.json is auto-generated from the REST schemas. To change field docs, edit lib/api/src/rest/schema.rs (e.g., add doc comments or #[schemars(description = ...)]). Specifically, UpdateVectors.update_filter lacked a description and should state: "If specified, only update vectors for points that match this filter; points not matching the filter are left unchanged."

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

519-597: Forced conversion path is sound; consider streaming to cut peak RAM during conversions

Current approach loads edges into memory (to_edges) before re-serializing. For very large graphs, a streaming converter (read-and-emit per node/level) would reduce memory footprint and speed up conversions. Optional, given this is primarily for force/maintenance paths.

lib/segment/src/index/hnsw_index/graph_links.rs (3)

89-93: Derive Debug for GraphLinksVectorsLayout for easier logging

Useful when tracing layout decisions and header packing.

-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub struct GraphLinksVectorsLayout {

---

97-116: StorageGraphLinksVectors::try_new: silent None can be hard to diagnose

Returning Option keeps it simple, but a debug log when layouts aren't available (or quantized vectors are absent) would aid troubleshooting without affecting release performance.

-    ) -> Option<Self> {
-        let quantized_vectors = quantized_vectors?;
-        Some(Self {
+    ) -> Option<Self> {
+        let quantized_vectors = quantized_vectors.or_else(|| {
+            log::debug!("Quantized vectors not available; CompressedWithVectors will be disabled");
+            None
+        })?;
+        Some(Self {
             vector_storage,
             quantized_vectors,
             vectors_layout: GraphLinksVectorsLayout {
                 base: vector_storage.get_vector_layout().ok()?,
                 link: quantized_vectors.get_quantized_vector_layout().ok()?,
             },
         })
     }

---

190-213: Heuristic overflow guard in recommended()

Using usize arithmetic could theoretically wrap on extreme inputs. Switch to u64 with saturating math to be robust while preserving the 8 KiB threshold (2 * PAGE_SIZE_BYTES).

-        let is_small_enough = |layout: GraphLinksVectorsLayout| {
-            let mut node_size_bytes = 0;
-            node_size_bytes += layout.base.size();
-            node_size_bytes += hnsw_m0 * 3; // Rough estimate: 3 bytes per compressed id
-            node_size_bytes += hnsw_m0 * layout.link.size();
-            node_size_bytes <= 2 * PAGE_SIZE_BYTES
-        };
+        let is_small_enough = |layout: GraphLinksVectorsLayout| {
+            let base = layout.base.size() as u64;
+            let ids  = (hnsw_m0 as u64).saturating_mul(3); // ~3 bytes per id
+            let links= (hnsw_m0 as u64).saturating_mul(layout.link.size() as u64);
+            let node_size_bytes = base.saturating_add(ids).saturating_add(links);
+            node_size_bytes <= (2 * PAGE_SIZE_BYTES) as u64
+        };

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

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

519-597: Load/convert logic is sound; good Advice usage.

• Priority order (CompWithVectors → Compressed → Plain) makes sense.
• Random advice for direct loads, Sequential for conversion is appropriate.
• Atomic write and reload path looks correct.

Optional: consider logging which source format was converted from for easier ops tracing.

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

103-116: StorageGraphLinksVectors::try_new — OK, minor ergonomics nit.

Returning Option is fine; if you want better error visibility later, consider Result<_, OperationError>. Not blocking.

---

189-214: Recommended format logic: clear, but avoid “let-chains” MSRV risk.

If your MSRV might predate stable let-chains, prefer a nested form. Functionally identical:

-        if feature_flags.hnsw_with_vectors
-            && let Some(vectors) = vectors
-            && (feature_flags.hnsw_format_force || is_on_disk && is_small_enough(vectors))
-        {
-            GraphLinksFormatParam::CompressedWithVectors(vectors)
-        } else {
-            GraphLinksFormatParam::Compressed
-        }
+        if feature_flags.hnsw_with_vectors {
+            if let Some(vectors) = vectors {
+                if feature_flags.hnsw_format_force || (is_on_disk && is_small_enough(vectors)) {
+                    return GraphLinksFormatParam::CompressedWithVectors(vectors);
+                }
+            }
+        }
+        GraphLinksFormatParam::Compressed

Would you like me to scan the repo for other let-chain usages relative to your MSRV?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs:46-55
Timestamp: 2025-08-11T00:37:34.100Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs`, the `ChunkedVectors` used in `QuantizedRamStorage` is intentionally designed to be non-persistent during updates. The `push_vector` method only updates the in-memory vectors without flushing to disk, and this is expected behavior rather than a bug.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7113
File: lib/quantization/src/encoded_vectors_u8.rs:202-211
Timestamp: 2025-08-21T13:43:52.191Z
Learning: The `quantized_vector_size()` method is available on the `EncodedVectors` trait, not on the `EncodedStorage` trait. The `EncodedStorage` trait provides methods like `get_vector_data`, `vectors_count`, `is_on_disk`, `upsert_vector`, `flusher`, `files`, and `immutable_files`.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7193
File: lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs:17-30
Timestamp: 2025-09-01T11:19:26.371Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs, the ChunkedMmapVectors underlying implementation does not validate against zero-sized vectors, so adding such validation in QuantizedChunkedMmapStorage::new is unnecessary and would be redundant.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:81-84
Timestamp: 2025-08-11T07:57:01.399Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the BitsStoreType parameter difference between single-vector and multi-vector Binary quantization is intentional: single-vector storage uses `EncodedVectorsBin<u128, ...>` to enable 128-bit SIMD/popcount optimizations for longer vectors, while multi-vector storage uses `EncodedVectorsBin<u8, ...>` because multivectors are typically shorter and benefit from byte-granular storage.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7113
File: lib/quantization/src/encoded_vectors_binary.rs:515-525
Timestamp: 2025-08-21T13:45:05.899Z
Learning: The `EncodedStorage` trait in `lib/quantization/src/encoded_storage.rs` does not have a `quantized_vector_size()` method. To validate vector sizes from storage, use `get_vector_data(index).len()` to get the actual size of stored vectors.

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

lib/common/common/src/flags.rs (1)

121-122: Re: enabling via all sets hnsw_with_vectors = true — confirm intent.
This may unexpectedly change on‑disk format in dev/test configs that set all=true. If that’s not desired, exclude it from the “all” bundle or gate it behind a dedicated dev flag.

lib/collection/src/collection_manager/optimizers/segment_optimizer.rs (1)

86-101: Avoid config drift during a single optimize run.
If HnswGlobalConfig can change at runtime (reload), temp_segment() and optimized_segment_builder() could see different configs. Consider snapshotting once per optimize() and threading it through to both calls; otherwise please confirm the config is immutable while optimization runs.

Also applies to: 326-332

lib/common/common/src/flags.rs (1)

63-66: Clarify/public surface of hnsw_with_vectors.
If this is not meant for end users, hide it from OpenAPI or docs (e.g., add a schemars skip) and state that it only applies when format selection is Auto.

Apply this diff if you want to hide it from schema:

     /// Allow writing the HNSW graph in the `CompressedWithVectors` format.
     /// Also see `GraphLinksFormatConfig`.
-    pub hnsw_with_vectors: bool,
+    #[schemars(skip)]
+    pub hnsw_with_vectors: bool,

lib/segment/tests/integration/gpu_hnsw_test.rs (1)

139-140: Bind HnswGlobalConfig locally instead of referencing a temporary.

Avoid taking a reference to a temporary in a struct literal; bind it once for clarity and lifetime safety, mirroring other tests.

Apply within this hunk:

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hnsw_global_config,

And add just above the HNSWIndex::build call:

let hnsw_global_config = HnswGlobalConfig::default();

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

379-380: Avoid &HnswGlobalConfig::default() in struct field; bind once.

This prevents any lifetime ambiguity and improves consistency with other sites.

Change in this hunk:

-        hnsw_global_config: &HnswGlobalConfig::default(),
+        hnsw_global_config: &hnsw_global_config,

Add before constructing open_args:

let hnsw_global_config = HnswGlobalConfig::default();

lib/segment/tests/integration/filtrable_hnsw_test.rs (1)

159-160: Bind default HnswGlobalConfig before use.

Keeps lifetimes explicit and aligns with the snapshot test style.

Within this hunk:

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hnsw_global_config,

Add just above the HNSWIndex::build call:

let hnsw_global_config = HnswGlobalConfig::default();

lib/segment/tests/integration/hnsw_discover_test.rs (1)

112-113: Use a named HnswGlobalConfig instead of a temporary in both tests.

Minor style/lifetime polish; keeps both tests consistent with others.

In both places:

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hnsw_global_config,

And add before each corresponding HNSWIndex::build call:

let hnsw_global_config = HnswGlobalConfig::default();

Also applies to: 238-239

lib/segment/src/segment/tests.rs (1)

250-254: Optional: also exercise forced graph-links formats in this snapshot restore.

To cover both Compressed and CompressedWithVectors in restore paths, consider parameterizing over GraphLinksFormatConfig::{ForceWithoutVectors, ForceWithVectors} in addition to the default Auto.

Example (sketch):

+    use crate::types::GraphLinksFormatConfig;
@@
-    let hnsw_global_config = HnswGlobalConfig::default();
-    let restored_segment =
-        load_segment(&entry.path(), &hnsw_global_config, &AtomicBool::new(false))
-            .unwrap()
-            .unwrap();
+    for format in [
+        GraphLinksFormatConfig::Auto,
+        GraphLinksFormatConfig::ForceWithoutVectors,
+        GraphLinksFormatConfig::ForceWithVectors,
+    ] {
+        let mut hnsw_global_config = HnswGlobalConfig::default();
+        hnsw_global_config.graph_links_format = format;
+        let restored_segment =
+            load_segment(&entry.path(), &hnsw_global_config, &AtomicBool::new(false))
+                .unwrap()
+                .unwrap();
+        // existing validations...
+    }

lib/segment/tests/integration/exact_search_test.rs (1)

139-139: Bind HnswGlobalConfig::default() to a local; avoid referencing a temporary.

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hnsw_global_config,

Add near the build call:

let hnsw_global_config = HnswGlobalConfig::default();

lib/segment/src/index/struct_payload_index.rs (1)

1241-1247: Bind HnswGlobalConfig::default() to a local; avoid borrowing a temporary.

-            load_segment(
-                &full_segment_path,
-                &HnswGlobalConfig::default(),
-                &AtomicBool::new(false),
-            )
+            let hnsw_global_config = HnswGlobalConfig::default();
+            load_segment(&full_segment_path, &hnsw_global_config, &AtomicBool::new(false))

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

77-77: Bind HnswGlobalConfig::default() to a local (don’t borrow a temporary)

Create a local HnswGlobalConfig and pass &hnsw_global_config; update all occurrences of &HnswGlobalConfig::default().

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hnsw_global_config,

Add before constructing the args:

let hnsw_global_config = HnswGlobalConfig::default();

lib/segment/tests/integration/fixtures/segment.rs (2)

175-176: Passing HnswGlobalConfig::default() into build_segment — OK.

This aligns the fixture with the new API. Consider allowing callers to inject a non-default config to exercise different graph-link formats in targeted tests.

If useful, I can add parallel helpers (keeping existing fns for BC):

+pub fn build_segment_3_with_hnsw(path: &Path, hnsw: &HnswGlobalConfig) -> Segment {
+    let mut segment3 = build_segment(
+        path,
+        /* existing SegmentConfig */,
+        hnsw,
+        true,
+    ).unwrap();
+    /* rest unchanged */
+    segment3
+}

---

271-272: Same change for sparse fixture 1 — OK.

Mirror of the above; enabling injection would make it easy to parametrize tests by format.

src/segment_inspector.rs (1)

45-47: Load with default HNSW config — OK; consider exposing a CLI toggle.

Defaulting to Auto is fine, but a flag to force WithVectors/WithoutVectors would help inspect/convert segments explicitly.

Example (sketch):

 #[derive(Parser, Debug)]
 struct Args {
   ...
+  /// Graph links format: auto|with-vectors|without-vectors
+  #[clap(long, value_parser = ["auto","with-vectors","without-vectors"])]
+  graph_links_format: Option<String>,
 }
 
 fn main() {
   ...
-  let segment = load_segment(path, &HnswGlobalConfig::default(), &AtomicBool::new(false))?
+  let cfg = match args.graph_links_format.as_deref() {
+      Some("with-vectors") => HnswGlobalConfig { graph_links_format: GraphLinksFormatConfig::ForceWithVectors, ..Default::default() },
+      Some("without-vectors") => HnswGlobalConfig { graph_links_format: GraphLinksFormatConfig::ForceWithoutVectors, ..Default::default() },
+      _ => HnswGlobalConfig::default(),
+  };
+  let segment = load_segment(path, &cfg, &AtomicBool::new(false))?

lib/segment/tests/integration/sparse_vector_index_search_tests.rs (1)

773-775: Default HNSW config for large-index test — OK.

If you later want to validate behavior under forced formats, consider parameterizing this test over GraphLinksFormatConfig.

lib/segment/src/segment_constructor/simple_segment_constructor.rs (1)

118-119: Default HNSW config in build_multivec_segment — OK; optional helper to accept custom config.

To enable targeted tests/benchmarks with forced formats, consider adding “_with_hnsw” variants instead of changing existing signatures.

Proposed additive API:

+pub fn build_simple_segment_with_hnsw(
+    path: &Path,
+    dim: usize,
+    distance: Distance,
+    hnsw: &HnswGlobalConfig,
+) -> OperationResult<Segment> {
+    build_segment(path, /* existing SegmentConfig */, hnsw, true)
+}
+
+pub fn build_multivec_segment_with_hnsw(
+    path: &Path,
+    dim1: usize,
+    dim2: usize,
+    distance: Distance,
+    hnsw: &HnswGlobalConfig,
+) -> OperationResult<Segment> {
+    build_segment(path, /* existing SegmentConfig */, hnsw, true)
+}

lib/segment/tests/integration/segment_tests.rs (1)

206-208: Nit: reuse a single HnswGlobalConfig instance.

Create one variable and pass references to reduce duplication.

Also applies to: 244-246

lib/segment/tests/integration/byte_storage_hnsw_test.rs (1)

100-106: LGTM: test updated to pass HnswGlobalConfig to build and open paths.

Minor nit: define let hnsw_global_config = HnswGlobalConfig::default(); once and pass &hnsw_global_config.

Also applies to: 203-204

lib/segment/tests/integration/multivector_filtrable_hnsw_test.rs (1)

84-86: LGTM: build/open paths supply HnswGlobalConfig.

Optional: reuse a single local HnswGlobalConfig instead of calling default() multiple times.

Also applies to: 143-144

lib/segment/benches/multi_vector_search.rs (2)

93-99: Wire-through of HnswGlobalConfig looks correct.

Consider hoisting HnswGlobalConfig::default() into a local let hgc = HnswGlobalConfig::default(); and reuse it below for HnswIndexOpenArgs to avoid duplication and make future tweaks explicit.

---

130-131: Pass the same config instance to build and open.

Use the same hgc variable here to ensure benches stay reproducible if defaults change in future.

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hgc,

lib/segment/tests/integration/byte_storage_quantization_test.rs (2)

238-244: Test wiring to new build_segment signature is fine.

Minor: create a single let hgc = HnswGlobalConfig::default(); and reuse it to reduce duplication and make the chosen graph-links behavior explicit in the test.

-    let mut segment_byte = build_segment(
+    let hgc = HnswGlobalConfig::default();
+    let mut segment_byte = build_segment(
         dir_byte.path(),
         &config_byte,
-        &HnswGlobalConfig::default(),
+        &hgc,
         true,
     )

---

356-357: Consistency: reuse the same HNSW global config instance.

Reuse &hgc here as well. Optional, but keeps tests stable if defaults evolve.

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hgc,

lib/segment/src/types.rs (1)

625-626: Defaulting behavior is sensible and backwards-compatible.

#[serde(default)] on HnswGlobalConfig plus Default impl ensures older configs deserialize fine. Consider adding a brief field doc to clarify that non‑Auto may convert-on-load.

 pub struct HnswGlobalConfig {
     /// Enable HNSW healing if the ratio of missing points is no more than this value.
     /// To disable healing completely, set this value to `0.0`.
     #[validate(range(min = 0.0, max = 1.0))]
     pub healing_threshold: f64,
-
-    pub graph_links_format: GraphLinksFormatConfig,
+    /// Controls graph-links format selection; `force_*` may convert and persist on load.
+    pub graph_links_format: GraphLinksFormatConfig,
 }

Also applies to: 642-647

lib/segment/tests/integration/multivector_quantization_test.rs (3)

231-233: Good update to the new build_segment API.

Same nit as elsewhere: hoist and reuse a single HnswGlobalConfig::default() for this test.

-    let mut segment =
-        build_segment(dir.path(), &config, &HnswGlobalConfig::default(), true).unwrap();
+    let hgc = HnswGlobalConfig::default();
+    let mut segment = build_segment(dir.path(), &config, &hgc, true).unwrap();

---

334-335: Reuse the same global config for clarity.

Pass &hgc here to keep the test’s config single-sourced.

-            hnsw_global_config: &HnswGlobalConfig::default(),
+            hnsw_global_config: &hgc,

---

231-233: Optional: parameterize this test to exercise both graph-link formats

Add an rstest #[case] over GraphLinksFormatConfig::{ForceWithoutVectors, ForceWithVectors} (guarded by the existing feature flags / vector-size preconditions) so the integration test runs with both formats and verifies identical behavior; existing HNSW tests use the same pattern under lib/segment/src/index/hnsw_index/. Location: lib/segment/tests/integration/multivector_quantization_test.rs (around the build_segment(...) call at lines 231–233).

docs/redoc/master/openapi.json (2)

11775-11779: Set default and clarify precedence for graph_links_format.

Add a default of "Auto" so client SDKs have a stable fallback, and describe interaction with feature flags to avoid ambiguity.

Apply this diff:

-          "graph_links_format": {
-            "$ref": "#/components/schemas/GraphLinksFormatConfig"
-          }
+          "graph_links_format": {
+            "allOf": [{ "$ref": "#/components/schemas/GraphLinksFormatConfig" }],
+            "default": "Auto",
+            "description": "Format policy for HNSW graph links. Defaults to Auto. When set to Force* it may trigger conversion on load; see also FeatureFlags.hnsw_with_vectors."
+          }

---

11781-11805: Simplify enum shape and document “when possible”.

• Current oneOf with three single‑value enums is verbose for generators. Prefer a single string enum.
• Spell out what “when possible” means (e.g., enabled by hnsw_with_vectors and node ≤ 8 KiB), or link to docs.

Apply this diff:

-      "GraphLinksFormatConfig": {
-        "oneOf": [
-          {
-            "description": "Decide the format based on the HNSW parameters and vector size in bytes. Do not convert during segment loading.",
-            "type": "string",
-            "enum": [
-              "Auto"
-            ]
-          },
-          {
-            "description": "Always use `Compressed`. Convert during segment loading.",
-            "type": "string",
-            "enum": [
-              "ForceWithoutVectors"
-            ]
-          },
-          {
-            "description": "Use `CompressedWithVectors` when possible. Convert during segment loading.",
-            "type": "string",
-            "enum": [
-              "ForceWithVectors"
-            ]
-          }
-        ]
-      },
+      "GraphLinksFormatConfig": {
+        "type": "string",
+        "enum": ["Auto", "ForceWithoutVectors", "ForceWithVectors"],
+        "description": "Auto: decide by HNSW params and vector size; no on-load conversion. ForceWithoutVectors: always use Compressed; convert on load. ForceWithVectors: prefer CompressedWithVectors when eligible (feature enabled; small node size), convert on load."
+      },

lib/segment/src/segment_constructor/segment_builder.rs (1)

481-481: Unused tuple fields may need extraction.

The tuple (temp_dir, destination_path) is created at line 481 but then temp_dir is referenced from line 485 onwards, suggesting the original fields are still in use. This indicates that line 481 may be redundant or the fields should be extracted from the tuple consistently.

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

90-94: Consider marking the struct as #[derive(Debug)].

The GraphLinksVectorsLayout struct could benefit from deriving Debug for easier debugging and logging.

/// Layout of base and link vectors, returned by [`GraphLinksVectors::vectors_layout`].
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
pub struct GraphLinksVectorsLayout {
    pub base: Layout,
    pub link: Layout,
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7193
File: lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs:17-30
Timestamp: 2025-09-01T11:19:26.371Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs, the ChunkedMmapVectors underlying implementation does not validate against zero-sized vectors, so adding such validation in QuantizedChunkedMmapStorage::new is unnecessary and would be redundant.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:81-84
Timestamp: 2025-08-11T07:57:01.399Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the BitsStoreType parameter difference between single-vector and multi-vector Binary quantization is intentional: single-vector storage uses `EncodedVectorsBin<u128, ...>` to enable 128-bit SIMD/popcount optimizations for longer vectors, while multi-vector storage uses `EncodedVectorsBin<u8, ...>` because multivectors are typically shorter and benefit from byte-granular storage.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs:46-55
Timestamp: 2025-08-11T00:37:34.100Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_ram_storage.rs`, the `ChunkedVectors` used in `QuantizedRamStorage` is intentionally designed to be non-persistent during updates. The `push_vector` method only updates the in-memory vectors without flushing to disk, and this is expected behavior rather than a bug.

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: generall
PR: qdrant/qdrant#6479
File: lib/segment/src/index/hnsw_index/graph_layers_builder.rs:137-182
Timestamp: 2025-05-30T15:26:14.488Z
Learning: In the subgraph_connectivity method in graph_layers_builder.rs, the previous_visited_points vector and visited BitVec entry point marking are automatically re-initialized at the start of each retry loop iteration, so manual clearing is not needed.

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

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

1-200: Re-verify all call sites of GraphLayers::load use the new 3-arg signature

Just to be safe post-rebase.

#!/bin/bash
# List call sites and flag any 2-arg usages (old API).
set -euo pipefail
echo "All call sites (context):"
rg -nC2 -P '\bGraphLayers::load\s*\(' || true
echo
echo "Potential old-call arity (2 args):"
rg -nP '\bGraphLayers::load\s*\(\s*[^,]+,\s*[^,]+\s*\)' || true

Also applies to: 630-646

docs/redoc/master/openapi.json (2)

11776-11778: Set an explicit default for graph_links_format

Many clients benefit from explicit defaults. Recommend defaulting to "auto" at the property site.

 "graph_links_format": {
+  "default": "auto",
   "$ref": "#/components/schemas/GraphLinksFormatConfig"
 }

---

11781-11805: Clarify fallback behavior in force_with_vectors description

Make it explicit that we fall back to Compressed if vectors-inclusive format isn’t applicable (e.g., node too large). Keeps behavior unambiguous.

-            "description": "Use `CompressedWithVectors` when possible. Convert during segment loading.",
+            "description": "Use `CompressedWithVectors` when possible, falling back to `Compressed` if the format is not supported for the segment (e.g., node too large). Convert during segment loading.",

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

76-76: Typo in progress template

“Buildng” → “Building”.

-                ProgressStyle::with_template("{percent:>3}% Buildng HNSW {wide_bar}").unwrap(),
+                ProgressStyle::with_template("{percent:>3}% Building HNSW {wide_bar}").unwrap(),

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

519-597: Forced format conversion flow is correct; minor polish possible

• Good: preferred-order probe, Advice::Random vs Sequential, atomic save, and size logging.
• Optional: enrich the final error with the probed filenames to aid debugging.

-        Err(OperationError::service_error(format!(
-            "No HNSW graph links file found in {dir:?}"
-        )))
+        Err(OperationError::service_error(format!(
+            "No HNSW graph links file found in {dir:?}. Searched: {:?}",
+            [
+                GraphLayers::get_links_path(dir, GraphLinksFormat::CompressedWithVectors),
+                GraphLayers::get_links_path(dir, GraphLinksFormat::Compressed),
+                GraphLayers::get_links_path(dir, GraphLinksFormat::Plain),
+            ]
+        )))

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

633-643: Overly strict gate for “on-disk” in format recommendation

recommended(..., is_on_disk) is already conservative. Requiring all storages (base, quantized) to be on disk may unnecessarily prevent CompressedWithVectors even when the index itself is on-disk and vectors are available. Consider passing just the index “on-disk” intent.

-        let format_param = GraphLinksFormatParam::recommended(
+        let format_param = GraphLinksFormatParam::recommended(
             &feature_flags,
             hnsw_global_config.graph_links_format,
             config.m0,
             graph_links_vectors.as_ref(),
-            vector_storage_ref.is_on_disk()
-                && quantized_vectors_ref
-                    .as_ref()
-                    .is_some_and(|q| q.is_on_disk())
-                && hnsw_config.on_disk.unwrap_or(false),
+            hnsw_config.on_disk.unwrap_or(false),
         );

---

170-186: Optional: log when ForceWithVectors cannot be honored

A debug/info log helps operators understand why the forced mode fell back.

-                GraphLinksFormatConfig::ForceWithVectors => {
-                    graph_links_vectors
-                        .as_ref()
-                        .map(|v| GraphLinksFormatParam::CompressedWithVectors(v))
-                        .or(Some(GraphLinksFormatParam::Compressed))
-                }
+                GraphLinksFormatConfig::ForceWithVectors => {
+                    match graph_links_vectors.as_ref() {
+                        Some(v) => Some(GraphLinksFormatParam::CompressedWithVectors(v)),
+                        None => {
+                            log::info!(
+                                "ForceWithVectors requested, but link vectors are unavailable; \
+                                 falling back to Compressed."
+                            );
+                            Some(GraphLinksFormatParam::Compressed)
+                        }
+                    }
+                }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#7232
File: lib/shard/src/segment_holder/mod.rs:1088-1094
Timestamp: 2025-09-16T21:39:09.293Z
Learning: The hnsw_format_force and hnsw_with_vectors flags in HnswGlobalConfig primarily affect existing segment conversion during load/startup, not the initial format selection for newly created empty segments. For new segments, format decisions are made by GraphLinksFormatParam::recommended() based on runtime conditions. Therefore, using HnswGlobalConfig::default() for temporary/proxy segments in SegmentHolder::build_tmp_segment is acceptable.

Learnt from: xzfc
PR: qdrant/qdrant#7232
File: lib/shard/src/segment_holder/mod.rs:1088-1094
Timestamp: 2025-09-16T21:39:09.293Z
Learning: The hnsw_format_force and hnsw_with_vectors flags in HnswGlobalConfig primarily affect existing segment conversion during load/startup, not the initial format selection for newly created empty segments. For new segments, format decisions are made by GraphLinksFormatParam::recommended() based on runtime conditions. Therefore, using HnswGlobalConfig::default() for temporary/proxy segments in SegmentHolder::build_tmp_segment is acceptable.

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: generall
PR: qdrant/qdrant#6479
File: lib/segment/src/index/hnsw_index/graph_layers_builder.rs:137-182
Timestamp: 2025-05-30T15:26:14.488Z
Learning: In the subgraph_connectivity method in graph_layers_builder.rs, the previous_visited_points vector and visited BitVec entry point marking are automatically re-initialized at the start of each retry loop iteration, so manual clearing is not needed.

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

lib/shard/src/segment_holder/mod.rs (1)

1088-1094: Use default HNSW global config for temp/proxy segments: acknowledged.

Per prior discussion, defaults are fine for brand‑new temp segments; format is decided by GraphLinksFormatParam::recommended(). Consider adding a brief comment to prevent future re‑flags.

Apply to add context:

         let mut segment = build_segment(
             segments_path,
             &config,
-            &HnswGlobalConfig::default(),
+            // Safe: temp/proxy segments start empty; format for new segments is chosen by
+            // GraphLinksFormatParam::recommended(), not by hnsw_format_force/hnsw_with_vectors.
+            // See PR #7232 discussion.
+            &HnswGlobalConfig::default(),
             save_version,
         )?;

lib/segment/tests/integration/multivector_quantization_test.rs (1)

311-318: Optional: exercise on-disk graph-with-vectors path in this test

If you want this test suite to also cover writing CompressedWithVectors when feasible, consider tying HnswConfig.on_disk to the on_disk test parameter; the current Some(false) disables that branch in recommended().

     let hnsw_config = HnswConfig {
       m,
       ef_construct,
       full_scan_threshold,
       max_indexing_threads: 2,
-      on_disk: Some(false),
+      on_disk: Some(on_disk),
       payload_m: None,
     };

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

519-597: Forced-format conversion flow is correct; consider cleanup policy for originals

The conversion path (Sequential read → serialize target → Random read) is solid. You explicitly leave the original file in place, which can double disk usage after multiple conversions. Consider an opt-in cleanup (config flag or env) or logging the presence of multiple link files during startup.

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

190-221: Threshold basis: 8 KiB vs 2*PAGE_SIZE_BYTES — clarify intent

Docs mention “no more than 8 KiB per node,” but code uses <= 2 * PAGE_SIZE_BYTES. On platforms with non‑4KiB pages this diverges. Either fix docs or pin the threshold to 8192 bytes for consistency.

-                let is_small_enough = node_size_bytes <= 2 * PAGE_SIZE_BYTES;
+                // Keep threshold stable across platforms (8 KiB), or document page-size coupling.
+                let is_small_enough = node_size_bytes <= 8 * 1024;

Alternatively, add a brief comment explaining the page-size-based heuristic.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#7232
File: lib/shard/src/segment_holder/mod.rs:1088-1094
Timestamp: 2025-09-16T21:39:09.293Z
Learning: The hnsw_format_force and hnsw_with_vectors flags in HnswGlobalConfig primarily affect existing segment conversion during load/startup, not the initial format selection for newly created empty segments. For new segments, format decisions are made by GraphLinksFormatParam::recommended() based on runtime conditions. Therefore, using HnswGlobalConfig::default() for temporary/proxy segments in SegmentHolder::build_tmp_segment is acceptable.

Learnt from: xzfc
PR: qdrant/qdrant#7232
File: lib/shard/src/segment_holder/mod.rs:1088-1094
Timestamp: 2025-09-16T21:39:09.293Z
Learning: The hnsw_format_force and hnsw_with_vectors flags in HnswGlobalConfig primarily affect existing segment conversion during load/startup, not the initial format selection for newly created empty segments. For new segments, format decisions are made by GraphLinksFormatParam::recommended() based on runtime conditions. Therefore, using HnswGlobalConfig::default() for temporary/proxy segments in SegmentHolder::build_tmp_segment is acceptable.

Learnt from: xzfc
PR: qdrant/qdrant#6982
File: lib/segment/src/index/hnsw_index/graph_links/view.rs:217-220
Timestamp: 2025-08-06T09:56:59.311Z
Learning: In Qdrant's GraphLinksView implementation (lib/segment/src/index/hnsw_index/graph_links/view.rs), methods like links() and links_with_vectors() intentionally use unwrap() and can panic for performance reasons, maintaining consistency across similar methods. Error handling improvements are considered secondary to feature completion and require benchmarking before implementation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7193
File: lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs:17-30
Timestamp: 2025-09-01T11:19:26.371Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs, the ChunkedMmapVectors underlying implementation does not validate against zero-sized vectors, so adding such validation in QuantizedChunkedMmapStorage::new is unnecessary and would be redundant.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#6986
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:81-84
Timestamp: 2025-08-11T07:57:01.399Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the BitsStoreType parameter difference between single-vector and multi-vector Binary quantization is intentional: single-vector storage uses `EncodedVectorsBin<u128, ...>` to enable 128-bit SIMD/popcount optimizations for longer vectors, while multi-vector storage uses `EncodedVectorsBin<u8, ...>` because multivectors are typically shorter and benefit from byte-granular storage.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.

Learnt from: generall
PR: qdrant/qdrant#6479
File: lib/segment/src/index/hnsw_index/graph_layers_builder.rs:137-182
Timestamp: 2025-05-30T15:26:14.488Z
Learning: In the subgraph_connectivity method in graph_layers_builder.rs, the previous_visited_points vector and visited BitVec entry point marking are automatically re-initialized at the start of each retry loop iteration, so manual clearing is not needed.

Learnt from: xzfc
PR: qdrant/qdrant#7118
File: lib/segment/src/index/hnsw_index/graph_links/serializer.rs:50-54
Timestamp: 2025-08-22T13:49:39.261Z
Learning: In Qdrant's graph serialization code (lib/segment/src/index/hnsw_index/graph_links/serializer.rs), the team prefers to keep the code simple rather than adding defensive input validation for edge cases that don't occur in practice, relying on unit tests to catch any unexpected issues early.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

lib/segment/src/types.rs (2)

593-599: Doc wording nit: clarify Option semantics.

Docs say “Default: false” but the field is Option<bool>. Suggest “If unset, defaults to false” to avoid ambiguity in generated docs.

Apply:

-/// Store copies of original and quantized vectors within the HNSW index file. Default: false.
+/// Store copies of original and quantized vectors within the HNSW index file. If unset, defaults to `false`.

---

611-630: Avoid unnecessary clone in mismatch check.

All fields here are Copy; destructure by value and drop the clone().

Apply:

-        let HnswConfig {
-            m,
-            ef_construct,
-            full_scan_threshold,
-            max_indexing_threads: _,
-            payload_m,
-            on_disk,
-            copy_vectors,
-        } = self.clone();
+        let HnswConfig {
+            m,
+            ef_construct,
+            full_scan_threshold,
+            max_indexing_threads: _,
+            payload_m,
+            on_disk,
+            copy_vectors,
+        } = *self;

lib/collection/src/operations/config_diff.rs (1)

93-99: REST diff field present and exposed in OpenAPI — add optional cross-field validation.

Verified: pub copy_vectors: Option<bool> exists in lib/collection/src/operations/config_diff.rs and is present in the generated OpenAPI (docs/redoc/master/openapi.json). Add a higher-level validation to error when copy_vectors = true without quantization enabled or when multivectors are used.

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

94-98: Consider deriving Debug for improved diagnostics.

The struct would benefit from deriving Debug to aid in troubleshooting and logging.

+#[derive(Debug)]
 pub struct StorageGraphLinksVectors<'a> {
     vector_storage: &'a VectorStorageEnum,   // base vectors
     quantized_vectors: &'a QuantizedVectors, // link vectors
     vectors_layout: GraphLinksVectorsLayout,
 }

lib/segment/tests/integration/multivector_filtrable_hnsw_test.rs (4)

145-152: Optional: parameterize FeatureFlags to exercise both code paths.

Consider rstest parameters to run with hnsw_with_vectors off/on, ensuring coverage of both gated behaviors without changing test intent.

Example (sketch):

-            feature_flags: FeatureFlags::default(),
+            feature_flags: {
+                let mut flags = FeatureFlags::default();
+                // add a new rstest bool case to toggle this
+                flags.hnsw_with_vectors = enable_hnsw_with_vectors;
+                flags
+            },

and add an #[case(false)]/#[case(true)] enable_hnsw_with_vectors: bool to the test signature.

---

55-55: Fix misleading comment for full_scan_threshold.

The “KB” unit in the comment appears incorrect for this field. Clarify or remove the unit to avoid confusion.

-    let full_scan_threshold = 8; // KB
+    let full_scan_threshold = 8; // threshold value (not a size unit)

---

124-132: Align max_indexing_threads with the single‑thread permit.

You set permit_cpu_count = 1 for determinism but max_indexing_threads: 2. Make them consistent to avoid surprises.

-        max_indexing_threads: 2,
+        max_indexing_threads: 1,

---

145-152: Follow‑up test to build+load both link formats.

Per the PR discussion, add a small smoke test that builds segments in both Compressed and CompressedWithVectors and then loads them, avoiding conversion during load. This test can live separately and use GraphLinksFormatConfig::{ForceWithoutVectors,ForceWithVectors} plus the feature flag.

If helpful, I can draft that test skeleton.

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

615-625: Consider logging when StorageGraphLinksVectors::try_new fails

When try_new fails, the configuration is silently ignored (as noted in the comment). Consider adding a debug or trace log to help diagnose configuration issues.

        let graph_links_vectors = hnsw_config
            .copy_vectors
            .unwrap_or_default()
            .then(|| {
                // NOTE: the configuration is silently ignored if try_new fails.
-                StorageGraphLinksVectors::try_new(
+                let result = StorageGraphLinksVectors::try_new(
                    &vector_storage_ref,
                    quantized_vectors_ref.as_ref(),
-                )
+                );
+                if result.is_none() {
+                    log::debug!("Failed to create StorageGraphLinksVectors - using fallback format");
+                }
+                result
            })
            .flatten();

lib/api/src/grpc/qdrant.rs (1)

410-415: Clarify “Default: false” semantics for a Diff field

HnswConfigDiff uses Option; “Default: false” can mislead API users during updates where None means “leave unchanged.” Please reword the docs to distinguish create vs. update behavior and tighten phrasing.

Suggested wording (update in the proto so it regenerates here):

-    /// Store copies of original and quantized vectors within the HNSW index file. Default: false.
-    /// Enabling this option will trade the search speed for disk usage by reducing amount of
-    /// random seeks during the search.
-    /// Requires quantized vectors to be enabled. Multi-vectors are not supported.
+    /// Store copies of original and quantized vectors within the HNSW index file.
+    /// Create-time default: false. When used in UpdateCollection/VectorParamsDiff, omit this field to leave it unchanged.
+    /// Enabling this option trades disk space for search speed by reducing random seeks.
+    /// Requires quantized vectors to be enabled. Not supported for multi-vectors.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: xzfc
PR: qdrant/qdrant#7232
File: lib/shard/src/segment_holder/mod.rs:1088-1094
Timestamp: 2025-09-16T21:39:09.353Z
Learning: The hnsw_format_force and hnsw_with_vectors flags in HnswGlobalConfig primarily affect existing segment conversion during load/startup, not the initial format selection for newly created empty segments. For new segments, format decisions are made by GraphLinksFormatParam::recommended() based on runtime conditions. Therefore, using HnswGlobalConfig::default() for temporary/proxy segments in SegmentHolder::build_tmp_segment is acceptable.

Learnt from: generall
PR: qdrant/qdrant#7006
File: docs/redoc/master/openapi.json:13645-13652
Timestamp: 2025-08-10T18:26:12.443Z
Learning: In qdrant, docs/redoc/master/openapi.json is auto-generated from the REST schemas. To change field docs, edit lib/api/src/rest/schema.rs (e.g., add doc comments or #[schemars(description = ...)]). Specifically, UpdateVectors.update_filter lacked a description and should state: "If specified, only update vectors for points that match this filter; points not matching the filter are left unchanged."