docs: pre-release comment & docstring cleanup#73
Conversation
Conservative multi-agent pass over all code comments/docstrings (comments only — no code changes). Notable accuracy fixes: - quant.rs: select_simd_tier doc + the search_asymmetric CRITICAL block claimed a mis-dispatched kernel "silently drops its trailing chunk -> wrong top-k in release". The kernels were since hardened to a real assert! (not debug_assert) on the lane invariant, so a mis-dispatch panics loudly. Corrected to match. - bitmap.rs: build_query_bitmap_fp32 comment named the sort key backwards (it sorts by raw q[j] desc, not |q[j]|); "828 KiB" temp -> ~808 KiB (207k x 4 B = 808.6 KiB). - multi_bucket.rs: b=2 storage "matches RankQuant b=2" -> "2x" (512 vs 256 B). - rank.rs: "see tests/rank.rs" -> the inline tests module (no such file). - rank_io.rs: "MAX_DIM * MAX_VECTORS ~8 TiB" was an element count -> "* 2 bytes"; noted sign bitmaps are bound by MAX_SIGN_BITMAP_DIM. - util.rs: module doc "Both items" -> lists all pub(crate) shared helpers. - quant.rs: search_asymmetric_byte_lut noted as re-exported #[doc(hidden)]. - tests/index/main.rs: "Three substrate types" -> "Three kinds of check" (the list is test categories); moved the experimental-feature comment above the module it describes (rustfmt then re-sorted the mod group). - tests/redteam_alpha.rs: "public contract requires sorted ids" -> performance preference (the Rust API accepts unsorted ids). Gate green: fmt, clippy -D warnings, tests (incl. doctests + experimental), strict rustdoc (no broken intra-doc links). Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Review Summary by QodoFix critical SIMD safety docs and accuracy of comments across codebase
WalkthroughsDescription• Corrected critical SIMD kernel safety documentation in quant.rs - Changed from "silently drops trailing chunk" to "panics loudly" with real assert! • Fixed accuracy issues in comments across multiple files - Bitmap sort key direction, memory calculations, storage comparisons • Clarified module documentation and cross-references - Updated util.rs module doc to list all helpers, fixed rank.rs test reference • Improved comment precision for API contracts and implementation details - Clarified performance vs correctness requirements, re-export visibility Diagramflowchart LR
A["Comment/Docstring Review"] --> B["SIMD Safety Fixes"]
A --> C["Accuracy Corrections"]
A --> D["Documentation Clarity"]
B --> E["quant.rs: assert! behavior"]
C --> F["bitmap.rs: sort key & memory"]
C --> G["multi_bucket.rs: storage ratio"]
C --> H["rank_io.rs: byte calculations"]
D --> I["util.rs: module scope"]
D --> J["rank.rs: test reference"]
D --> K["redteam_alpha.rs: API contract"]
File Changes1. src/quant.rs
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request primarily updates documentation, comments, and module-level descriptions across several files (including bitmap.rs, multi_bucket.rs, quant.rs, rank.rs, rank_io.rs, util.rs, and integration tests) to improve clarity, correct outdated metrics, and accurately describe internal behaviors such as SIMD assertions and document ID sorting preferences. There are no functional code changes, and no review comments were provided to evaluate.
There was a problem hiding this comment.
Pull request overview
Documentation-only polish across the ordvec crate and its test suite to improve comment/docstring accuracy ahead of release, including correcting stale explanations of SIMD dispatch failure behavior and various storage/sorting details.
Changes:
- Corrected/clarified docstrings and “CRITICAL”/contract comments to match current runtime behavior (e.g., SIMD mis-dispatch now panics via
assert!). - Fixed several inaccurate or misleading explanatory comments (sorting key description, temporary buffer sizing in KiB, storage comparisons, test references).
- Minor non-functional test-module ordering tweak in
tests/index/main.rsafter comment movement (rustfmt-resortedmodgroup).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/redteam_alpha.rs |
Clarifies that sorted doc_ids are a performance recommendation, not a correctness requirement. |
tests/index/main.rs |
Updates module-level test-doc narrative; moves the experimental gating comment above mod multi_bucket and reorders mod rank. |
src/util.rs |
Updates module docs to reflect the full set of shared pub(crate) helpers now present. |
src/rank.rs |
Fixes doc reference to point to the inline tests module rather than a non-existent tests/rank.rs. |
src/rank_io.rs |
Corrects loader docs to account for byte sizing (* 2 bytes) and the sign-bitmap dimension cap. |
src/quant.rs |
Updates docs to reflect assert!-enforced SIMD lane invariants and clarifies the #[doc(hidden)] re-export intent. |
src/multi_bucket.rs |
Corrects storage comparison wording (MultiBucketBitmap b=2 is 2× RankQuant b=2 at D=1024). |
src/bitmap.rs |
Fixes comment about query sort key (raw value vs absolute value) and corrects KiB estimates (~808 KiB). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pre-release comment & docstring cleanup
A final conservative polish of all code comments and docstrings before release, run as a 6-agent parallel Opus sweep (rank/quant · bitmap family · SIMD kernels · persistence+crate-root+example · Python binding · tests). Each agent fixed only unambiguous issues and flagged anything touching
// SAFETY:blocks or meaning; every edit was reviewed and the gate re-run centrally. Comments/docstrings only — no code changes (the one structural diff is rustfmt re-sorting amodgroup after a comment was moved).Most important fix
quant.rs— bothselect_simd_tier's doc and thesearch_asymmetricCRITICALblock claimed a mis-dispatched SIMD kernel "silently drops its trailing chunk → wrong top-k in release." That is stale and misleading: the kernels were since hardened to a realassert!(notdebug_assert!) on the lane invariant (verified atquant_kernels.rs171/250/349/437), so a mis-dispatch panics loudly in release. Corrected both comments.Other accuracy fixes
bitmap.rsbuild_query_bitmap_fp32named the sort key backwards (sorts rawq[j]desc, not|q[j]|);828 KiB→~808 KiB(207k × 4 B)multi_bucket.rsrank.rstests/rank.rs" → the inlinetestsmodule (no such file exists)rank_io.rsMAX_DIM * MAX_VECTORS~8 TiB" was an element count → "* 2bytes"; noted sign bitmaps are bound byMAX_SIGN_BITMAP_DIMutil.rspub(crate)shared helpers (the module grew past 2)quant.rssearch_asymmetric_byte_lutnoted as re-exported#[doc(hidden)]tests/index/main.rsmod multi_buckettests/redteam_alpha.rsLeft for your call (not edited)
examples/bench_rank.rs(~L1322) — asign-headlinemode comment carries cryptic internal shorthand ("prompted by Todd's schema.org-typed result, validated at the Harrier scale and recall regime"). Reads as private context for a soon-public sanctioned benchmark; not touched (named-person / voice call). Suggest trimming to the technical content if you want.rank_io.rs(L15-17) — one sentence lightly restates the line above. Pure style; left as-is.Verification (all green)
fmt --check·clippy --all-targets --all-features -D warnings·cargo test(default +--features experimental, incl. doctests) · strictRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features(no broken intra-doc links).