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

100-104: ⚠️ Potential issue

Casting the sampled f32 to u8 collapses the distribution to {0,1}

x as u8 saturates negatives to 0 and maps the (0.0,1.0] range to 0‒1, so the generated values are almost exclusively 0 or 1.
That defeats the purpose of providing a balanced [-1.0, 1.0] input for BQ tests and can mask real-world errors.

-        Box::new(
-            rng.sample_iter(rand::distr::Uniform::new_inclusive(-1.0, 1.0).unwrap())
-                .map(|x| f32::from(x as u8)),
-        )
+        Box::new(
+            rng.sample_iter(rand::distr::Uniform::new_inclusive(-1.0, 1.0).unwrap())
+        )

lib/quantization/benches/binary.rs (1)

22-43: ⚠️ Potential issue

VectorParameters.count no longer matches the vector set – potential UB in benchmarks

You create 100 K vectors, then push another 100 K, but keep count at 100 K.
encode() relies on that field for bounds checks and may panic or silently process the wrong slice.

-    let vectors_count = 100_000;
+    let vectors_count = 200_000;           // or remove the second push

@@
-        &VectorParameters {
-            dim: vector_dim,
-            count: vectors_count,
+        &VectorParameters {
+            dim: vector_dim,
+            count: vectors.len() as u32,

Update the same block for the u8 run below.

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

939-950: Same mapping block repeated – DRY violation

This second match block is identical to the one above. Once the conversion helper suggested earlier is in place, replace this entire block with the single-line conversion to avoid errors of omission when variants are extended.

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

95-99: Consider exercising the new encodings in this test suite

encoding: None preserves the previous 1-bit default, but the PR adds TwoBits and OneAndHalfBits. Extending the test matrix to cover those variants would protect the new code paths and prevent silent regressions.

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

268-270: Parameterise the test over the new encoding enum

Hard-coding encoding: None means the multivector pipeline is never validated for TwoBits or OneAndHalfBits. Adding a #[values(None, Some(OneBit), Some(TwoBits), …)] style matrix (or looping inside the test) will give end-to-end coverage at negligible runtime cost.

lib/api/src/grpc/proto/collections.proto (1)

298-302: Introduce BinaryQuantizationEncoding enum
The new enum defines all supported binary quantization schemes. Consider adding inline comments for each variant to improve readability of the proto.

docs/grpc/docs.md (2)

398-398: Approve encoding field addition
The encoding field for BinaryQuantization is documented with the correct type and anchor. Optionally, you may specify the default encoding value to aid users.

---

1835-1845: Approve BinaryQuantizationEncoding enum documentation
The new enum section aligns with the existing format. All anchors and markdown syntax are consistent. Consider adding brief descriptions for each enum member to enhance clarity.

lib/segment/src/index/hnsw_index/gpu/gpu_vector_storage/tests.rs (2)

147-176: Consider adding a “legacy / default‐encoding” case

All cases in this block pass an explicit BinaryQuantizationEncoding value.
A quick sanity-check that encoding: None still behaves the same as the original one-bit path is missing. Adding a single case (e.g. case::cosine_f32_default_encoding) that passes None would guard against silent regressions in fallback handling.

This could look like:

 #[case::cosine_f32_default_encoding(
     Distance::Cosine,
     TestStorageType::Dense(TestElementType::Float32),
     273,
     2057,
-    BinaryQuantizationEncoding::OneBit
+    /* encoding = */ { /* special marker – see below */ }
 )]

You would then switch the parameter type of the test to Option<BinaryQuantizationEncoding> and feed it into the config unchanged:

encoding: Option<BinaryQuantizationEncoding>
…
encoding, // instead of Some(encoding)

This keeps coverage high while exercising the “no encoding specified” path.

---

223-228: Unnecessary clone() of quantization_config

quantization_config is moved only once into test_gpu_vector_storage_impl; cloning here allocates needlessly in every test run.

-    test_gpu_vector_storage_impl(
+    test_gpu_vector_storage_impl(
         …
-        Some(quantization_config.clone()),
+        Some(quantization_config),

(A similar clone appears in the SQ/PQ helpers and can be removed in a follow-up sweep.)

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

293-297: Leverage the struct’s Default to keep the diff minimal

Now that encoding is an Option, the config can be expressed more concisely and keep future additions painless:

-        QuantizationVariant::Binary => BinaryQuantizationConfig {
-            always_ram: None,
-            encoding: None,
-        }
+        QuantizationVariant::Binary => BinaryQuantizationConfig {
+            encoding: None,
+            ..Default::default()
+        }

This avoids repeating fields once defaults evolve.

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

7032-7039: Enhance BinaryQuantizationEncoding schema with documentation.

Adding a description (and optionally a default) to the enum schema will improve generated API docs and clarify intended defaults:

"BinaryQuantizationEncoding": {
  "type": "string",
  "enum": [
    "one_bit",
    "two_bits",
    "one_and_half_bits"
  ],
+ "description": "Binary quantization encoding options. Defaults to `one_bit` when unset.",
+ "default": "one_bit"
}

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

663-677: Consider deriving Copy (and optionally EnumIter) for the new enum

BinaryQuantizationEncoding is a small, C-like enum with no payload.
Deriving Copy lets the compiler move values by bit-copy instead of requiring an explicit clone and reduces boiler-plate at call-sites. Everywhere else in the file we derive Copy for similar enums (Distance, Order, …) so this would keep the API consistent.

Optional quality-of-life: if the variants are ever iterated over (e.g. in tests or CLI validation) adding EnumIter (already in Cargo.toml) avoids handwritten arrays.

-#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone, PartialEq, Eq, Hash, Default)]
+#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone, Copy, PartialEq, Eq, Hash, Default, EnumIter)]

No behaviour changes – only compile-time conveniences.

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

879-890: Duplicate BinaryQuantizationEncoding → Encoding mapping – extract a helper or From impl

The same match block translating BinaryQuantizationEncoding to quantization::encoded_vectors_binary::Encoding appears several times in this file (see also 939-950). Duplicating this logic risks future divergence when new enum variants are added.

-let encoding = match binary_config.encoding {
-    Some(BinaryQuantizationEncoding::OneBit) => Encoding::OneBit,
-    Some(BinaryQuantizationEncoding::TwoBits) => Encoding::TwoBits,
-    Some(BinaryQuantizationEncoding::OneAndHalfBits) => Encoding::OneAndHalfBits,
-    None => Encoding::OneBit,
-};
+let encoding = binary_config
+    .encoding
+    .unwrap_or(BinaryQuantizationEncoding::OneBit)
+    .into();               // requires `impl From<BinaryQuantizationEncoding> for Encoding`

Implementing From (or TryFrom for potential fall-backs) in encoded_vectors_binary.rs removes the boilerplate here and elsewhere, improving maintainability.

lib/quantization/tests/integration/test_binary_encodings.rs (1)

110-118: Test assertion may produce false negatives

The test assumes monotonically non-decreasing accuracy from OneBit → 1.5Bits → 2Bits. For some random seeds that assumption can be violated by statistical noise even when encodings are correct, causing flaky tests.

Consider comparing each encoding against a fixed threshold or running multiple seeds and averaging instead of asserting strict ordering.

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

1056-1059: Consider providing more context in the error message.

While the error handling is correct, consider including the invalid encoding value in the error message for better debugging.

-            .map_err(|_| Status::invalid_argument("Unknown binary quantization encoding"))?;
+            .map_err(|_| Status::invalid_argument(format!("Unknown binary quantization encoding: {:?}", encoding)))?;

lib/quantization/src/encoded_vectors_binary.rs (4)

269-271: Document the standard deviation fallback behavior.

The fallback to 1.0 for near-zero standard deviations is reasonable but should be documented for clarity.

Add a comment explaining this behavior:

         sds.iter_mut()
-            .for_each(|sd| *sd = if *sd < f32::EPSILON { 1.0 } else { sd.sqrt() });
+            .for_each(|sd| {
+                // Use 1.0 as fallback for near-zero variance to avoid division by zero in normalization
+                *sd = if *sd < f32::EPSILON { 1.0 } else { sd.sqrt() }
+            });

---

330-333: Document the hardcoded normalization parameters.

The z-score normalization uses hardcoded values that should be documented for clarity.

Consider extracting these as named constants or adding explanatory comments:

             let mean = means[i];
             let sd = standard_deviations[i];
-            let ranges = 3;
-            let v_z = (v - mean) / sd;
-            let index = (v_z + 2.0) / (4.0 / ranges as f32);
+            // Map z-scores from [-2, 2] range to [0, 3] for 3 quantization levels
+            const RANGES: f32 = 3.0;
+            const Z_OFFSET: f32 = 2.0;  // Maps z=-2 to index=0
+            const Z_RANGE: f32 = 4.0;   // Total z-score range covered
+            let v_z = (v - mean) / sd;
+            let index = (v_z + Z_OFFSET) / (Z_RANGE / RANGES);

---

345-371: Add documentation for the complex bit packing strategy.

The one-and-a-half bits encoding uses a non-trivial bit packing strategy that needs clear documentation.

Add a comment explaining the bit layout:

     fn encode_one_and_half_bits_vector(
         vector: &[f32],
         encoded_vector: &mut [TBitsStoreType],
         standard_deviations: &[f32],
         means: &[f32],
     ) {
+        // Bit packing strategy for 1.5 bits per dimension:
+        // - First n bits: one bit per dimension (indicates if value > first threshold)
+        // - Next n/2 bits: one bit per two dimensions (indicates if value > second threshold)
+        // This gives us 3 possible states per dimension using 1.5 bits on average
         let bits_count = u8::BITS as usize * std::mem::size_of::<TBitsStoreType>();

---

138-138: Use u8::BITS consistently for bit calculations.

For consistency with line 181, consider using u8::BITS here as well.

-        let bits_count = u8::BITS as usize * bytes_count;
+        let bits_count = u8::BITS as usize * bytes_count;

Wait, this line is already using u8::BITS. However, the logic above could be simplified:

     fn get_storage_size(size: usize) -> usize {
-        let bytes_count = if size > 128 {
-            std::mem::size_of::<u128>()
-        } else if size > 64 {
-            std::mem::size_of::<u64>()
-        } else if size > 32 {
-            std::mem::size_of::<u32>()
-        } else {
-            std::mem::size_of::<u8>()
-        };
-
-        let bits_count = u8::BITS as usize * bytes_count;
+        let (bytes_count, bits_count) = if size > 128 {
+            (std::mem::size_of::<u128>(), u128::BITS as usize)
+        } else if size > 64 {
+            (std::mem::size_of::<u64>(), u64::BITS as usize)
+        } else if size > 32 {
+            (std::mem::size_of::<u32>(), u32::BITS as usize)
+        } else {
+            (std::mem::size_of::<u8>(), u8::BITS as usize)
+        };

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

lib/quantization/src/encoded_vectors_binary.rs (5)

22-28: Add an explicit representation to avoid ABI / forward-compat pitfalls

serde guarantees deterministic (de)serialisation of enums, but the underlying discriminant is not stabilised across compilers/platforms.
Declaring the enum as #[repr(u8)] locks the layout and makes FFI (and potential on-disk bincode / C-bindings) safer at virtually zero cost.

-#[derive(Clone, Copy, Eq, PartialEq, Debug, Serialize, Deserialize, Default)]
+#[repr(u8)]
+#[derive(Clone, Copy, Eq, PartialEq, Debug, Serialize, Deserialize, Default)]
 pub enum Encoding {

---

201-209: Avoid unnecessary mean/σ computations for OneBit encoding

Mean and standard-deviation vectors are never used by encode_one_bit_vector, yet they are computed and persisted for every collection.
That is O(N·dim) extra work and extra disk I/O.

-        let means = Self::means(orig_data.clone(), count);
-        let standard_deviations = Self::standard_deviations(orig_data.clone(), &means, count);
+        let (means, standard_deviations) = if encoding == Encoding::OneBit {
+            (Vec::new(), Vec::new())
+        } else {
+            let m = Self::means(orig_data.clone(), count);
+            let sd = Self::standard_deviations(orig_data.clone(), &m, count);
+            (m, sd)
+        };

You can then pass empty slices to encode_vector for the OneBit path.
Reduces encode time and shrinks metadata considerably.

---

234-250: Three passes over the dataset – consider streaming mean/variance

means and standard_deviations walk the entire iterator twice after the initial count() pass (total = 3 × N).
A single streaming Welford’s algorithm can compute both mean and variance in one pass, cutting memory traffic by ~66 %.

Not critical for small collections, but noticeable on multi-GB datasets.

Also applies to: 262-272

---

319-333: Hard-coded ranges = 3 deserves a named constant / comment

Two-bit encoding currently splits the Z-score interval [-2,2] into three buckets, mapping to 0/1/2 set bits.
Readers unfamiliar with the academic paper will wonder why “3” and why “±2”. A short comment or a const BINS: usize = 3 would clarify intent and ease future tuning.

---

180-187: Minor semantic inconsistency in get_storage_size return units

For u8 the function returns bytes, for u128 it returns elements (converted to bytes later).
The wrapper currently compensates, but aligning semantics (either always bytes or always elements) would make future refactors safer.

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

lib/quantization/src/encoded_vectors_binary.rs (1)

459-465: Errors from create_dir_all silently ignored

parent().map(std::fs::create_dir_all); discards both the Option and the Result, so failures (e.g. permission denied) propagate as “file not found” later, making debugging painful.

-if let Some(dir) = meta_path.parent() {
-    std::fs::create_dir_all(dir)?;
-}
+if let Some(dir) = meta_path.parent() {
+    std::fs::create_dir_all(dir)?;
+}

Repeat for the data path and vector_stats_path.

lib/quantization/src/vector_stats.rs (1)

18-27: Use f32::INFINITY / NEG_INFINITY for clearer intent

f32::MAX / f32::MIN are technically correct, but INFINITY / NEG_INFINITY convey intent more explicitly and avoid the cognitive overhead of remembering which of the two M-prefixed constants is the “large positive” one.

-            min: f32::MAX,
-            max: f32::MIN,
+            min: f32::INFINITY,
+            max: f32::NEG_INFINITY,

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

lib/quantization/src/encoded_vectors_binary.rs (1)

472-474: Fix misuse of std::fs::exists – it returns bool, not Result

std::fs::exists already yields a boolean. Pattern-matching on Ok(true) will not compile and re-introduces the compile error called out in a prior review.
Replace the check with the idiomatic .exists() call on the Path.

-        } else if let Ok(true) = std::fs::exists(&vector_stats_path) {
-            std::fs::remove_file(&vector_stats_path)?;
+        } else if vector_stats_path.exists() {
+            std::fs::remove_file(&vector_stats_path)?;
         }

lib/quantization/src/encoded_vectors_binary.rs (1)

202-208: Avoid double pass over orig_data to build stats

VectorStats::build consumes a full clone of orig_data, and the subsequent
encode loop consumes the original iterator – two complete reads.
Streaming the statistics while encoding (or re-using the same iterator with
peekable buffering) would cut I/O by half for large on-disk collections.

Consider refactoring VectorStats::build into an online updater that is fed
inside the main loop.

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

lib/quantization/src/encoded_vectors_binary.rs (1)

463-476: Critical: Fix std::fs::exists usage

The code on line 473 treats std::fs::exists as returning a Result, but it returns a plain bool. This will cause a compilation error.

Apply this fix:

-        } else if let Ok(true) = std::fs::exists(&vector_stats_path) {
+        } else if vector_stats_path.exists() {

docs/grpc/docs.md (2)

96-96: Validate TOC indentation
The new nested list item is indented by 4 spaces, which violates markdownlint MD007 (expected 2 spaces for nested lists).

Apply this diff to normalize the indent:

-    - [BinaryQuantizationEncoding](#qdrant-BinaryQuantizationEncoding)
+  - [BinaryQuantizationEncoding](#qdrant-BinaryQuantizationEncoding)

---

1853-1863: Enhance enum variant descriptions
The BinaryQuantizationEncoding table omits descriptions for each variant, reducing clarity. Consider adding concise descriptions for each entry.

For example:

| Name              | Number | Description                     |
| ----------------- | ------ | ------------------------------- |
| OneBit            | 0      | Single-bit binary quantization  |
| TwoBits           | 1      | Two-bit binary quantization     |
| OneAndHalfBits    | 2      | One-and-a-half bit quantization |

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