test+bench+docs: release-harden caller-owned batched two-stage primitives

[Fieldnote-Echo]

Summary

The caller-owned serial two-stage primitives added in 0.5.0 —
SignBitmap::top_m_candidates_batched_serial_csr → CandidateBatch →
RankQuant::search_asymmetric_subset_batched_serial[_into] with a reusable
SubsetScratch — are present, shape-conformant, and memory-safe. A downstream
(OrdinalDB) reviewer flagged the original PR as "only half the work"; an
adversarial trust/memory-safety audit confirms that gap is release-readiness +
proof-of-value, not a functional or safety defect. No source/guard change.

This PR lands the missing half: the explicit, tested memory-safety net + the
perf evidence + the integration docs.

Memory-safety verdict (the reason this was gated)

SOUND — no out-of-bounds reachable. The audit traced every unsafe SIMD
load/store from the two public rerank entries through validate_csr_batch into
all four AVX kernels + the scalar fallback and could not construct a
malformed-but-accepted CSR/query that reaches an OOB gather. Order is:
queries % dim → assert_all_finite → validate_csr_batch (offsets len/start/
final/monotonic/row-len≤n_vectors/candidate-id<n_vectors) → out-buffer lengths,
all before the per-row gather. Heap sizing uses checked_mul. The guards are
correct and complete; the residual risk was purely untested — which this PR
closes.

API status

Present + exact shape match; no API change. Design constraints verified in
code: no internal rayon, _into allocation-free after warmup, exact
single-query semantics + tie policy (score desc, global row-id asc),
-1/NEG_INFINITY sentinels, unsorted-rows-OK + deterministic.

Tests (tests/index/two_stage.rs, +11; tests/alloc_free.rs, new)

• Rejection-path net on the rerank entry points: overlong row (the guard
  bounding the unsafe gather), non-monotonic / wrong-final / non-zero-first
  offsets, non-finite + ragged queries (_into, the CSR gen, and the allocating
  wrapper), wrong out_scores/out_indices length — each #[should_panic],
  pinning that a bad input panics before the SIMD scan.
• Mixed sentinel test: full + underfull + empty rows in one batch; filled
  slots equal the single-query reference, trailing slots are -1/NEG_INFINITY.
• Allocation-counter test (own binary, counting #[global_allocator]):
  search_asymmetric_subset_batched_serial_into does 0 heap allocations on a
  warmed steady-state call — strong form of the prior capacity proxy.
• (Existing coverage already had: per-query parity across dims/bits/k/nq, empty
  batch, k=0, duplicates, unsorted order-independence, cross-tier scalar
  reference, offsets-len + OOB-candidate rejection.)

Benchmark — Harrier-1024 (examples/two_stage_bench.rs, SYNTHETIC)

Reproduce: cargo run --release --example two_stage_bench -- --dim 1024 --n 50000 --queries 200 --m 256 --k 10
(committed capture: benchmarks/two_stage_caller_owned_dim1024.txt; Zen5 / AVX-512, single-thread):

phase	µs/query
1. stage-1 candidate gen (CSR)	159.6
2. single-query rerank loop	10.43
3. batched rerank _into	10.16
4. full two-stage	172.4
rerank: batched _into vs single-query loop	1.03×

Honest framing (no-fiction): at dim=1024 the rerank stage is a small slice of
an already stage-1-dominated cost, and the batched _into is on par with the
single-query loop single-threaded. The primitives' value is (a) allocation-free
steady state (proven: 0 allocs) and (b) caller-owned parallelism (no internal
rayon → a DB drives _into across its own pool, GIL released, one query-range per
worker) — not a single-thread speedup. This result is not explained by the
SignBitmap AVX-tail (dim=768) win — different shape, different mechanism.

Docs

README "Caller-owned serial two-stage (DB / runtime integration)" section (CSR
shape, sentinel padding, scratch reuse, no-rayon); a no_run rustdoc example on the
alloc-free _into hot path + a buffer-sizing porting callout; an example on the
CSR candidate-gen; CHANGELOG.

Validation (local, Zen5 / AVX-512)

• cargo fmt --all --check ✅
• cargo clippy --workspace --all-targets --locked -- -D warnings ✅ (incl. ordvec-python)
• core + manifest + ffi tests --locked ✅ (index 85→96; alloc_free; 4 doctests; FFI C-link)
• cargo +1.89.0 build (MSRV) ✅ · cargo build --locked ✅
• examples/two_stage_bench runs at --dim 1024 ✅

Note: cargo test --workspace cannot link ordvec-python (pyo3 extension-module
has no libpython for a standalone cargo test binary) — a pre-existing property of
this workspace; the binding is gated via maturin develop + pytest in python.yml
and is untouched here. The FFI C-link test needs an absolute target dir (it
resolves CARGO_TARGET_DIR against the package CWD); green under CI's default target/.

Follow-up (not in this PR)

A csr_batched_twostage cargo-fuzz target over the CSR offset arithmetic + gather
(the audit's one remaining "optional" residual-risk item) — best reviewed with its
own fuzz.yml wiring. The guards are proven sound and now rejection-tested, so this
is additive continuous coverage.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

1. Hidden API surface added ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

subset_rerank_uses_simd is re-exported as a pub crate-root symbol, even though its own doc calls
it “not a stability surface”; downstream code can still depend on it, turning future removal/renames
into semver-breaking changes. This undermines the intended “tests-only diagnostic” boundary.

Code

src/lib.rs[R83-88]

+// `subset_rerank_uses_simd` reports whether the asymmetric subset rerank takes a
+// SIMD kernel (vs the allocating scalar LUT fallback) for a `(dim, bits)` on
+// this CPU. `#[doc(hidden)]` — reachable for the allocation-free test so its
+// skip-gate cannot drift from the actual rerank dispatch.
+#[doc(hidden)]
+pub use quant::subset_rerank_uses_simd;

Evidence

The crate root re-exports the helper publicly, while the helper’s own docs explicitly frame it as
“not a stability surface”; the alloc-free test depends on it via the crate root.

src/lib.rs[83-88]
src/quant.rs[231-244]
tests/alloc_free.rs[55-70]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`subset_rerank_uses_simd` is documented as a tests-only diagnostic (“not a stability surface”), but it is exported as a crate-root `pub` symbol. That effectively commits the crate to supporting it long-term.

## Issue Context
The symbol is currently used by the counting-allocator integration test as a skip gate. We want the test to keep working, but without unintentionally expanding the stable public API surface.

## Fix Focus Areas
- src/lib.rs[83-88]
- src/quant.rs[231-244]
- tests/alloc_free.rs[55-70]
- Cargo.toml[69-76]

## Suggested fix
- Introduce an opt-in cargo feature (e.g. `test-utils` / `diagnostics`) with `default = false`.
- Export `subset_rerank_uses_simd` only when that feature is enabled (or move it under a feature-gated module), e.g.:
 - `#[cfg(feature = "test-utils")] pub use quant::subset_rerank_uses_simd;`
- Gate the integration test accordingly:
 - either `#[cfg(feature = "test-utils")]` on the whole test, or
 - use `cfg!(feature = "test-utils")` to decide whether to run the strict assertion.
This preserves the test and keeps the helper out of the default public surface.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. SIMD probe ignores validation ✓ Resolved 🐞 Bug ≡ Correctness

Description

subset_rerank_uses_simd reports SIMD eligibility using only select_simd_tier, so it can return
true for (dim,bits) pairs that RankQuant::validate_params/new would reject (e.g., bits=4 and
dim multiple-of-8 but not divisible by 2^bits). This makes the probe’s semantics ambiguous and
can mislead future tests or callers that treat it as an “allocation-free is possible” predicate.

Code

src/quant.rs[R231-244]

+/// Whether the asymmetric subset rerank takes a SIMD kernel (vs the scalar LUT
+/// fallback) for `(dim, bits)` on this CPU. The scalar fallback allocates a
+/// per-query LUT, so the allocation-free steady-state guarantee of
+/// [`RankQuant::search_asymmetric_subset_batched_serial_into`] holds exactly
+/// when this is `true`.
+///
+/// `#[doc(hidden)]` — a diagnostic for tests, not a stability surface. It reads
+/// the same [`select_simd_tier`] the rerank dispatch reads, so it cannot drift
+/// from the actual dispatch.
+#[doc(hidden)]
+#[must_use]
+pub fn subset_rerank_uses_simd(dim: usize, bits: u8) -> bool {
+    !matches!(select_simd_tier(dim, bits), SimdTier::None)
+}

Evidence

The probe delegates solely to select_simd_tier (lane/feature gating), while
RankQuant::validate_params enforces additional invariants (e.g., divisibility constraints) that
the probe does not check.

src/quant.rs[231-244]
src/quant.rs[246-282]
src/quant.rs[201-223]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`subset_rerank_uses_simd(dim, bits)` can return `true` even when `(dim,bits)` is not a valid `RankQuant` configuration, because it does not incorporate `RankQuant`’s parameter validation rules.

## Issue Context
Today it’s used as a skip gate in `tests/alloc_free.rs`. Tightening semantics avoids future confusion (and makes the helper more robust if reused elsewhere).

## Fix Focus Areas
- src/quant.rs[231-244]
- src/quant.rs[246-282]

## Suggested fix
- Make `subset_rerank_uses_simd` return `false` if `RankQuant::validate_params(dim, bits)` is `Err`.
 - Example:
   - `if RankQuant::validate_params(dim, bits).is_err() { return false; }`
   - then run `select_simd_tier`.
- Alternatively (if you don’t want to call `validate_params`), clarify in the doc comment that inputs must already satisfy `RankQuant`’s constructor invariants; but the stricter behavior is safer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

PR Summary by Qodo

Release-harden caller-owned two-stage API with tests, docs, and benchmark example
🧪 Tests 📝 Documentation ✨ Enhancement 🕐 20-40 Minutes

Walkthroughs

Description

• Add rejection-path regression tests ensuring invalid CSR/query/buffers panic before SIMD rerank.
• Prove batched _into rerank is allocation-free after warmup via counting allocator test.
• Document and benchmark caller-owned serial two-stage pipeline (README/rustdoc/example + reference
  capture).

Diagram

graph TD
  A(["Caller runtime/DB"]) --> B["SignBitmap CSR"] --> C[("CandidateBatch (CSR)")] --> D["RankQuant rerank _into"] --> E[("Caller out buffers")]
  D --> F[("SubsetScratch")]
  G["Tests"] --> B --> D
  H["two_stage_bench example"] --> B --> D
  subgraph Legend
    direction LR
    _actor(["Caller"]) ~~~ _mod["Module/API"] ~~~ _data[("Data/buffers")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Add property-based/fuzz coverage for CSR edge cases

• ➕ Explores a larger space of malformed CSR/queries beyond hand-picked regressions
• ➕ Good at finding unexpected guard gaps around offset arithmetic and gather sizing
• ➖ More CI wiring and runtime cost (especially fuzz)
• ➖ Harder to keep deterministic/reproducible compared to targeted #[should_panic] tests

2. Use Criterion for the decomposition benchmark

• ➕ More robust statistics (noise filtering, confidence intervals)
• ➕ Standardized benchmark harness and reporting
• ➖ Adds dev dependency and increases benchmark complexity
• ➖ Less direct as an integration example runnable via cargo run --example

Recommendation: Keep the PR’s approach: targeted rejection-path tests + an allocator-counting test directly validate the safety/contract claims without changing core code, and the example-based benchmark doubles as an integration guide. Consider adding a dedicated fuzz target later as continuous coverage, but it’s additive rather than a replacement for these explicit regressions.

File Changes

Enhancement (1)

two_stage_bench.rs Add two-stage decomposition benchmark/integration example +177/-0

Add two-stage decomposition benchmark/integration example

• Adds a synthetic example that builds an index, warms scratch/buffers, and times four phases (CSR gen, single-query rerank loop, batched '_into', full pipeline). Prints per-phase ms/q and the rerank speedup headline for the caller-owned contract.

examples/two_stage_bench.rs

---

Tests (2)

alloc_free.rs Add counting-allocator test proving steady-state '_into' has zero allocations +101/-0

Add counting-allocator test proving steady-state '_into' has zero allocations

• Introduces a dedicated test binary with a custom global allocator that counts alloc/realloc/zeroed calls. Warms 'SubsetScratch' once, then asserts the second batched '_into' rerank call performs zero heap allocations.

tests/alloc_free.rs

---

two_stage.rs Add rejection-path regression net and mixed-sentinel padding test +254/-0

Add rejection-path regression net and mixed-sentinel padding test

• Adds multiple '#[should_panic]' tests ensuring malformed CSR offsets, overlong rows, ragged/non-finite queries, and wrong output-buffer lengths are rejected before any unsafe SIMD gather. Adds a mixed full/underfull/empty-row batch test validating sentinel padding and per-row parity with the single-query reference.

tests/index/two_stage.rs

---

Documentation (5)

CHANGELOG.md Document release-hardening of caller-owned two-stage primitives +19/-0

Document release-hardening of caller-owned two-stage primitives

• Adds an Unreleased changelog entry describing the new rejection-path tests, allocation-free proof, benchmark example, and integration docs. Explicitly states no API changes and frames the trust-model hardening.

CHANGELOG.md

---

README.md Add caller-owned serial two-stage integration section +34/-0

Add caller-owned serial two-stage integration section

• Introduces a README section explaining the no-rayon, allocation-free two-stage workflow for DB/runtime integration. Provides a minimal example and clarifies CSR shape, sentinel padding, buffer sizing, and scratch reuse.

README.md

---

two_stage_caller_owned_dim1024.txt Add reference benchmark capture for Harrier-1024 decomposition +21/-0

Add reference benchmark capture for Harrier-1024 decomposition

• Commits a reproducible benchmark output capture decomposing stage-1 CSR generation vs rerank costs and comparing batched '_into' to a single-query loop. Includes honest interpretation notes and reproduction command.

benchmarks/two_stage_caller_owned_dim1024.txt

---

quant.rs Expand rustdoc for batched subset rerank '_into' contract +30/-0

Expand rustdoc for batched subset rerank '_into' contract

• Extends the API docs for 'search_asymmetric_subset_batched_serial_into' with explicit buffer sizing guidance and a no_run usage example. Emphasizes rectangular outputs, sentinel padding, and fail-loud length asserts to avoid under-writes.

src/quant.rs

---

sign_bitmap.rs Add rustdoc example for CSR candidate batch generation +14/-0

Add rustdoc example for CSR candidate batch generation

• Adds a no_run example demonstrating how to call 'top_m_candidates_batched_serial_csr' and interpret the returned CSR layout. Points readers at piping offsets/candidates into the RankQuant batched rerank entry point.

src/sign_bitmap.rs

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[gemini-code-assist]

Code Review

This pull request release-hardens the caller-owned serial two-stage primitives by adding extensive documentation, benchmarks, and tests. Specifically, it introduces a new benchmark example (two_stage_bench.rs), a counting-allocator test (alloc_free.rs) verifying zero heap allocations in steady state, and a comprehensive suite of rejection-path regression tests to ensure robust validation of inputs before SIMD scanning. Additionally, user-facing documentation and rustdoc examples have been added to clarify the integration contract for the allocation-free serial path. No review comments were provided, so there is no feedback to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[Fieldnote-Echo]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 1967c9a

[Fieldnote-Echo]

Triage of the qodo review (head 013cbc6):

Bug 2 — SIMD probe ignores validation ✅ Fixed (013cbc6). subset_rerank_uses_simd now gates on RankQuant::validate_params before reading select_simd_tier, so it can no longer report SIMD eligibility for a (dim, bits) that RankQuant::new would reject (e.g. bits=4 with dim % 8 == 0 but dim % 16 != 0). Added a regression test asserting the probe is false for constructor-invalid fixtures and never claims SIMD where new() rejects.

Bug 1 — hidden API surface — Working as intended. subset_rerank_uses_simd is deliberately #[doc(hidden)] and stays a test-only dispatch diagnostic, consistent with the crate's existing #[doc(hidden)] convention (search_asymmetric_byte_lut, RankQuantFastscan). Keeping it #[doc(hidden)] rather than feature-gating avoids dropping the allocation-free guarantee out of the default-build test coverage. The broader hidden/gated public surface is being tidied deliberately in a separate post-0.5.0 effort (e.g. RankQuantFastscan promotion; test/bench helpers stay hidden).

[Fieldnote-Echo]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 013cbc6