feat(ivf): support bucket-aligned precise codes

[jac0626]

Change Type

• Bug fix
• New feature
• Improvement/Refactor
• Documentation
• CI/Build/Infra

Linked Issue

• Closes: [feat](ivf): support bucket-aligned precise codes #2519

What Changed

• Add precise_codes_layout: "flat" | "bucket" and keep flat as the compatibility default.
• Store bucket-layout precise codes with the same posting id, bucket, and offset as the corresponding basic posting.
• Require buckets_per_data: 1 for the first bucket-layout version and reject multi-assignment explicitly.
• Cover train, add, reorder, distance-by-id, serialization, streaming load, model export, merge, and alignment validation.
• Add a dedicated critical streaming tag for the bucket payload.
• Add a runnable C++ example for disk-backed bucket-aligned precise codes.
• Document the new layout and supported IO backends in English and Chinese.

The first implementation intentionally keeps per-candidate QueryOneById reads. Grouped batch
reads and shared CacheIO remain independent follow-up optimizations.

The shared disk-backed BucketDataCell memory-accounting bug found during review is tracked
separately in #2573; it also affects basic IVF buckets on main.

Test Evidence

• clang-format 15 dry-run
• make lint
• make test
• make cov, run tests, and collect coverage
• Other (describe below)

Test details:

cmake --build build --target functests unittests 326_feature_ivf_precise_bucket -j 3
Build completed successfully.

./build/tests/unittests '[IVFParameter]' -d yes -a --order rand \
  --rng-seed 185372933 --colour-mode none
All tests passed (65 assertions in 5 test cases).

./build/tests/functests '[ivf][reorder][pr]' -d yes -a --order rand \
  --rng-seed 185372933 --colour-mode none
All tests passed (136697 assertions in 5 test cases).

./build/tests/functests -d yes -a --order rand --rng-seed 185372933 \
  --allow-running-no-tests '[ivf]' '~[daily]' --colour-mode none
All tests passed (374627 assertions in 43 test cases).

./build/examples/cpp/326_feature_ivf_precise_bucket
The example completed successfully; the self-query returned id=0 with distance=0.

Coverage includes buckets_per_data=1 plus rejection coverage for >1, buffer_io disk files,
ContinueAdd, four-thread in-memory build, BinarySet and streaming round trips, Index::Load,
merge, and scalar/batch distance-by-id.

Compatibility Impact

• API/ABI compatibility: additive public constants and one optional IVF parameter.
• Behavior changes: none for existing configurations. Missing precise_codes_layout defaults to
  flat, so existing flat indexes remain readable. New bucket indexes persist their layout and
  use a distinct streaming block.

Performance and Concurrency Impact

• Performance impact: the bucket layout enables precise-code locality for disk-backed IVF, but
  performance measurement is deferred to a dedicated POC. The first version rejects
  buckets_per_data > 1, so reranking always reads the unique aligned posting.
• Concurrency/thread-safety impact: precise postings are written before publishing their basic
  mirrors. The shared NonContinuousIO allocation path is synchronized, and four-thread
  in-memory construction is covered.

Documentation Impact

• No docs update needed
• Updated docs:
  • README.md
  • DEVELOPMENT.md
  • CONTRIBUTING.md
  • Other: IVF website docs and examples/cpp/README.md

Risk and Rollback

• Risk level: medium; the new layout touches IVF persistence and merge paths but is opt-in.
• Rollback plan: revert this PR. Existing indexes continue to use the unchanged default flat
  layout.

Checklist

• I have linked the relevant issue.
• I have added/updated tests for new behavior.
• I have considered API compatibility impact.
• I have updated docs for the new behavior.
• My commit message follows project conventions.

[vsag-bot]

/label status/waiting-for-review
/waiting-on reviewer
/request-review @jiaweizone
/request-review @wxyucs
/request-review @inabao

[vsag-bot]

Automated pull request review completed.

Review effort: high (941 changed lines across 20 files).

Submitted 1 inline comment.
Review: #2520 (review)

[mergify]

Merge Protections

🟢 All 3 merge protections satisfied — ready to merge.

Show 3 satisfied protections

🟢 Require kind label

• label~=^kind/

🟢 Require version label

• label~=^version/

🟢 Require linked issue for feature/bug PRs

• body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

[Copilot]

Pull request overview

Adds an opt-in IVF feature to store “precise” (reorder) codes in a bucket-aligned layout to improve locality for bucket-scoped reads (especially for disk-backed IVF), while preserving the legacy flat layout as the default for backward compatibility.

Changes:

• Introduces precise_codes_layout: "flat" | "bucket" (default flat) and wires it through IVF parameter parsing/compatibility checks.
• Implements bucket-aligned precise-code storage, streaming/serialization support via a new critical streaming tag (ivf_precise_bucket), and merge/model-export handling.
• Adds functional/unit coverage for bucket precise codes (train/add/reorder/distance-by-id/serialization/streaming/merge) and updates English/Chinese docs.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_ivf.cpp	Adds persistent functional tests covering bucket-layout precise codes across IO backends and streaming/merge scenarios.
src/storage/serialization_tags.h	Adds IVF_PRECISE_BUCKET streaming tag, name mapping, criticality, and version routing.
src/inner_string_params.h	Adds internal parameter keys/values for precise_codes_layout and exposes them via DEFAULT_MAP.
src/constants.cpp	Adds public constant strings for IVF_PRECISE_CODES_LAYOUT and its values.
include/vsag/constants.h	Exposes the new public IVF constants in the public header.
src/algorithm/ivf/ivf_parameter.h	Adds precise_codes_layout to IVF parameters with default flat.
src/algorithm/ivf/ivf_parameter.cpp	Parses/validates precise_codes_layout, persists it, and includes it in compatibility checks.
src/algorithm/ivf/ivf_parameter_test.cpp	Adds unit tests for defaulting/validation/compatibility of precise_codes_layout.
src/algorithm/ivf/ivf.h	Adds bucket-precise reorder helper and new member for precise_bucket_.
src/algorithm/ivf/ivf.cpp	Implements bucket-aligned precise bucket initialization, insert/mirror logic, streaming/serialization handling, reorder path, distance-by-id selection, and merge validation.
docs/docs/en/src/indexes/ivf.md	Documents precise_codes_layout, supported backends, and behavior with buckets_per_data > 1.
docs/docs/zh/src/indexes/ivf.md	Chinese docs update for precise_codes_layout and supported IO backends.
docs/docs/en/src/advanced/new_serialization.md	Documents mutual exclusivity of high_precision_codes vs ivf_precise_bucket streaming blocks.
docs/docs/zh/src/advanced/new_serialization.md	Same streaming documentation updates in Chinese.

[jac0626]

/retest

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

[vsag-bot]

Automated inline review completed.

Review effort: high (673 changed lines across 17 files).
Submitted 2 inline comments.

Reviewed commit e23e117.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[LHT129]

Summary

This PR adds a precise_codes_layout: "bucket" option to IVF indexes, enabling bucket-aligned precise codes for disk-backed IVF. The core idea is to mirror each basic posting with a corresponding precise-code posting in the same bucket at the same offset, so that precise-code reads during reorder are colocated with the basic codes they belong to — avoiding random seeks when the precise codes are on disk.

What's Changed (880 additions, 38 deletions across 20 files)

• Core IVF logic (ivf.cpp, ivf.h): New precise_bucket_ member (BucketInterfacePtr), reorder_with_precise_bucket() method, disk_backed_precise_bucket_ flag, and updated Add/Search/Serialize/Deserialize/CalDistanceById paths.
• Parameter handling (ivf_parameter.cpp, ivf_parameter.h): New precise_codes_layout parameter ("flat"/"bucket"), validation rules (rejects pqfs quantizer, mmap_io, requires use_reorder).
• Serialization (serialization_tags.h): New IVF_PRECISE_BUCKET = 17 stream tag.
• Factory (factory.cpp): Rejects Index::Load for disk-backed precise buckets.
• IO (noncontinuous_allocator.h): Changed last_offset_ to std::atomic<uint64_t> with fetch_add for thread-safe concurrent allocation.
• Tests (test_ivf.cpp): Added GenerateBucketPreciseParameters(), CheckBucketPreciseIndex(), and multiple test cases covering build, search, serialize, merge, concurrent add, and CalDistanceById.
• Documentation: Updated IVF docs and serialization docs in both EN/ZH.
• Example: New 326_feature_ivf_precise_bucket.cpp runnable example.

Overall Assessment

The design is sound and well-scoped. The bucket-aligned layout is a natural extension of the existing IVF architecture, and the implementation correctly mirrors the basic bucket's structure for precise codes. The PR includes thorough test coverage and handles edge cases like buckets_per_data > 1, disk-backed IO types, and compatibility checks for Merge/ExportModel/Clone/Fork.

Issues Already Identified (from existing comments)

Several issues have been raised by previous reviewers and are still present in this commit:

1. Fork() error message copy-paste (ivf.h:121): Says "Clone does not support..." instead of "Fork does not support...". [LHT129]

2. cal_memory_usage() inflates memory for disk-backed IOs (ivf.cpp:2281): precise_bucket_->GetMemoryUsage() reports full on-disk size even for disk-backed IOs. [vsag-bot, LHT129]

3. reorder_with_precise_bucket uses j==0 posting only (ivf.cpp:1663): get_location() returns only the first posting's bucket/offset. For buckets_per_data > 1, the precise code read may not be colocated with the winning posting. [vsag-bot]

4. fill_location_map() over-allocation (ivf.cpp:2050): Resizes to total_elements_ * buckets_per_data_ but indexed by origin id. Pre-existing on main. [Copilot]

5. Flat path FlattenInterface::MakeInstance null check (ivf.cpp:404): No null check before passing to FlattenReorder. Pre-existing on main. [Copilot]

New Observations

Beyond the issues already identified, I reviewed the full diff and found the implementation to be consistent and correct. The NonContinuousAllocator thread-safety fix (atomic fetch_add) is a good improvement. The rejection of disk-backed precise buckets in Clone/Fork/ExportModel/Index::Load is appropriate and complete.

The test coverage is comprehensive, covering:

• Basic build and search with bucket precise
• Serialization round-trip
• Merge of bucket-precise indexes
• Concurrent add
• CalDistanceById with bucket precise
• Compatibility validation (rejecting incompatible merges)
• Various IO types (memory, buffer, async)

Recommendation

The PR is in good shape. The remaining issues are either pre-existing on main or have been acknowledged by the author. I recommend addressing the Fork() error message fix (trivial) and the cal_memory_usage() inflation before merging, but neither is a correctness blocker.

[vsag-bot]

Automated inline review completed.

Review effort: high (918 changed lines across 20 files).
No actionable inline findings were found.

Reviewed commit 7ed1b37.

[jac0626]

/retest

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[LHT129]

Review Summary (supplemental)

Reviewed commit dbc8fe6d (880 additions, 38 deletions across 20 files). This is a supplemental review on top of the 30+ existing inline comments and several prior review summaries that have already covered the PR thoroughly.

Issues already covered by existing comments

The following have been raised and addressed in prior review rounds:

• Fork() error message — Fixed (now says "Fork" instead of "Clone")
• NonContinuousAllocator thread safety — Fixed (atomic fetch_add)
• cal_memory_usage() inflation for disk IOs — Tracked separately in #2573 (pre-existing on main)
• buckets_per_data > 1 reorder posting — Resolved by requiring buckets_per_data=1 for bucket layout
• Per-candidate QueryOneById in reorder/CalDistanceById — Acknowledged as baseline; grouped reads are follow-up work
• ExportModel/Load file sharing — Pre-existing on main for basic bucket; acknowledged
• Batch async writes — Acknowledged as existing behavior
• MMap truncation — Pre-existing on main
• fill_location_map over-allocation — Pre-existing on main
• Flat path null check — Pre-existing on main

Additional observations

After a thorough pass over the full implementation, I did not identify new correctness or security issues beyond what has already been raised. A few notes:

1. Deserialize/read_streaming_body symmetry: The serialization format correctly writes "precise_bucket" or "reorder_codes" based on layout, and the deserialization paths correctly read the corresponding block. The parameter compatibility check in CheckCompatibility (checking precise_codes_layout) prevents cross-layout deserialization mismatches.

2. Concurrent Add correctness: The InsertVector → InsertVectorWithOffset pattern in the concurrent add_func lambda is sound. Each thread gets a unique offset from the precise bucket (protected by per-bucket mutexes), and InsertVectorWithOffset uses std::max for bucket_sizes_ update, correctly handling out-of-order inserts.

3. Merge safety: check_merge_illegal verifies precise_bucket_ consistency between indexes and calls ExportModel for model comparison. Since Merge rejects disk_backed_precise_bucket_ upfront, the ExportModel calls within check_merge_illegal are safe.

4. Streaming tag handling: Both IVF_PRECISE_BUCKET and HIGH_PRECISION_CODES tags are guarded by the appropriate null checks in read_streaming_body, and the loaded_precise_codes flag is correctly validated after the loop. Cross-layout mismatches are caught by parameter compatibility checks.

Conclusion

The PR is in good shape. The implementation is consistent, the serialization is symmetric, and the disk-backed safety guards (Clone/Fork/ExportModel/Merge/Load rejection) are complete. No new blocking issues found.

[LHT129]

Review Summary (supplemental)

Reviewed commit dbc8fe6d (880 additions, 38 deletions across 20 files). This is a supplemental review on top of the 30+ existing inline comments and several prior review summaries that have already covered the PR thoroughly.

Issues already covered by existing comments

The following have been raised and addressed in prior review rounds:

• Fork() error message — Fixed (now says "Fork" instead of "Clone")
• NonContinuousAllocator thread safety — Fixed (atomic fetch_add)
• cal_memory_usage() inflation for disk IOs — Tracked separately in #2573 (pre-existing on main)
• buckets_per_data > 1 reorder posting — Resolved by requiring buckets_per_data=1 for bucket layout
• Per-candidate QueryOneById in reorder/CalDistanceById — Acknowledged as baseline; grouped reads are follow-up work
• ExportModel/Load file sharing — Pre-existing on main for basic bucket; acknowledged
• Batch async writes — Acknowledged as existing behavior
• MMap truncation — Pre-existing on main
• fill_location_map over-allocation — Pre-existing on main
• Flat path null check — Pre-existing on main

Additional observations

After a thorough pass over the full implementation, I did not identify new correctness or security issues beyond what has already been raised. A few notes:

1. Deserialize/read_streaming_body symmetry: The serialization format correctly writes "precise_bucket" or "reorder_codes" based on layout, and the deserialization paths correctly read the corresponding block. The parameter compatibility check in CheckCompatibility (checking precise_codes_layout) prevents cross-layout deserialization mismatches.

2. Concurrent Add correctness: The InsertVector → InsertVectorWithOffset pattern in the concurrent add_func lambda is sound. Each thread gets a unique offset from the precise bucket (protected by per-bucket mutexes), and InsertVectorWithOffset uses std::max for bucket_sizes_ update, correctly handling out-of-order inserts.

3. Merge safety: check_merge_illegal verifies precise_bucket_ consistency between indexes and calls ExportModel for model comparison. Since Merge rejects disk_backed_precise_bucket_ upfront, the ExportModel calls within check_merge_illegal are safe.

4. Streaming tag handling: Both IVF_PRECISE_BUCKET and HIGH_PRECISION_CODES tags are guarded by the appropriate null checks in read_streaming_body, and the loaded_precise_codes flag is correctly validated after the loop. Cross-layout mismatches are caught by parameter compatibility checks.

Conclusion

The PR is in good shape. The implementation is consistent, the serialization is symmetric, and the disk-backed safety guards (Clone/Fork/ExportModel/Merge/Load rejection) are complete. No new blocking issues found.

[LHT129]

Review Summary (supplemental)

Reviewed commit dbc8fe6d (880 additions, 38 deletions across 20 files). This is a supplemental review on top of the 30+ existing inline comments and several prior review summaries that have already covered the PR thoroughly.

Issues already covered by existing comments

The following have been raised and addressed in prior review rounds:

• Fork() error message — Fixed (now says "Fork" instead of "Clone")
• NonContinuousAllocator thread safety — Fixed (atomic fetch_add)
• cal_memory_usage() inflation for disk IOs — Tracked separately in #2573 (pre-existing on main)
• buckets_per_data > 1 reorder posting — Resolved by requiring buckets_per_data=1 for bucket layout
• Per-candidate QueryOneById in reorder/CalDistanceById — Acknowledged as baseline; grouped reads are follow-up work
• ExportModel/Load file sharing — Pre-existing on main for basic bucket; acknowledged
• Batch async writes — Acknowledged as existing behavior
• MMap truncation — Pre-existing on main
• fill_location_map over-allocation — Pre-existing on main
• Flat path null check — Pre-existing on main

Additional observations

After a thorough pass over the full implementation, I did not identify new correctness or security issues beyond what has already been raised. A few notes:

1. Deserialize/read_streaming_body symmetry: The serialization format correctly writes "precise_bucket" or "reorder_codes" based on layout, and the deserialization paths correctly read the corresponding block. The parameter compatibility check in CheckCompatibility (checking precise_codes_layout) prevents cross-layout deserialization mismatches.

2. Concurrent Add correctness: The InsertVector → InsertVectorWithOffset pattern in the concurrent add_func lambda is sound. Each thread gets a unique offset from the precise bucket (protected by per-bucket mutexes), and InsertVectorWithOffset uses std::max for bucket_sizes_ update, correctly handling out-of-order inserts.

3. Merge safety: check_merge_illegal verifies precise_bucket_ consistency between indexes and calls ExportModel for model comparison. Since Merge rejects disk_backed_precise_bucket_ upfront, the ExportModel calls within check_merge_illegal are safe.

4. Streaming tag handling: Both IVF_PRECISE_BUCKET and HIGH_PRECISION_CODES tags are guarded by the appropriate null checks in read_streaming_body, and the loaded_precise_codes flag is correctly validated after the loop. Cross-layout mismatches are caught by parameter compatibility checks.

Conclusion

The PR is in good shape. The implementation is consistent, the serialization is symmetric, and the disk-backed safety guards (Clone/Fork/ExportModel/Merge/Load rejection) are complete. No new blocking issues found.

[vsag-bot]

Automated inline review completed.

Review effort: high (941 changed lines across 20 files).
Submitted 1 inline comment.

Reviewed commit dbc8fe6.

[vsag-bot]

[P2] Validate the precise inner-ID vector length before indexing

BucketDataCell::Deserialize reads inner_ids_ and bucket_sizes_ as independent serialized vectors. A crafted IVF_PRECISE_BUCKET can therefore declare a bucket size equal to the valid basic bucket while supplying a shorter, even empty, precise inner-ID vector; this check then evaluates precise_ids[j] out of bounds, causing undefined behavior instead of rejecting the index. Validate that each precise inner-ID vector covers bucket_size before dereferencing it, preferably during datacell deserialization.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[vsag-bot]

Automated inline review completed.

Review effort: high (941 changed lines across 20 files).
Submitted 1 inline comment.

Reviewed commit 7e14d50.

[vsag-bot]

[P2] [P2] Validate precise ID storage before indexing

BucketDataCell::Deserialize reads bucket_sizes_ and each inner_ids_ vector independently, so a malformed IVF_PRECISE_BUCKET can report the same size as the basic bucket while supplying fewer precise IDs. This line then indexes past precise_ids, causing undefined behavior instead of rejecting the serialized index. Validate that the precise inner-ID vector contains at least bucket_size entries before entering this loop.