Skip to content

fix: reject non-finite input, isolate FastScan estimator helpers, and harden release gates - #195

Merged
gouyt13 merged 6 commits into
mainfrom
fix/audit-input-isa-release-gates
Oct 31, 2026
Merged

gouyt13 merged 6 commits into
mainfrom
fix/audit-input-isa-release-gates

Conversation

@gouyt13

@gouyt13 gouyt13 commented Oct 31, 2026

Copy link
Copy Markdown
Member

Three fixes from a component audit of the library, plus one documentation note. The commits are independent and can be reviewed separately.

Reject non-finite and oversized coordinates (fix:)

IVF, HNSW and SymphonyQG accepted NaN, infinite and arbitrarily large coordinates in data, centroids and queries. The quantizer then converted NaN to int (UB, reported by UBSan float-cast-overflow), stored NaN factors, and one-bit high-accuracy IVF search returned the poisoned point first with a NaN distance, even after remove(). The stable core API and the clustering APIs already rejected the same input.

  • New utils/input_checks.hpp: finiteness scan (same integer reduction the v1 API used, which now calls it) and the clustering coordinate bound sqrt(sqrt(FLT_MAX) / (64 * dim)); static inline because ISA-flagged sources include it.
  • Called at IVF/HNSW/QG construct, add, search and batch search after the existing shape checks and before any state change; Lut rejects non-finite rotated queries (covers BatchQuery<double>).
  • RABITQ_ENABLE_SANITIZERS now adds float-cast-overflow,float-divide-by-zero; neither is part of -fsanitize=undefined, so the existing sanitizer job could not see this class of bug.
  • Contract documented in usage.md and the three index docstrings (.pyi kept in sync); C++ tests per entry point and Python tests for build/add/search.
  • QGAssignerTest.PropagatesSearchFailureFromWorkers injected its worker failure with 1e19 coordinates; the per-query entry the workers call now rejects them with invalid_argument, so the test asserts that type and message. The failure still originates inside the parallel region.

FastScan estimator Eigen isolation (fix(fastscan):)

estimator_kernels.hpp and qg_estimator.hpp instantiated Eigen assignment loops from the generic, AVX2 and AVX-512 translation units. Below -O3 those survive as identically named weak symbols with zmm/ymm/SSE bodies, and only the static-archive member order kept the generic copy in front (the -O1 sanitizer job compiles exactly that configuration). The 32-lane corrections are now plain loops that each backend vectorizes with its own flags (verified: xmm/ymm/zmm arithmetic in the respective objects at -O3). check-isa-symbols.sh passes at -O1 and -O2 -g as well as -O3; development.md updated accordingly.

Release wheel gates (ci:)

Each wheel gate was skipped whenever its build was skipped, and the build is skipped when prepare fails; a skipped required check satisfies branch protection, so a failed eligibility check on a PR waived the wheel builds, the installed-wheel suite and the typing checks. The gates now run with always() and decide from the prepare, changes and build results, like the other aggregator jobs. The contract test runs the shared verdict script over all 900 input combinations and fails on the previous workflow.

Reviewer note: on PRs without wheel-related changes the four wheel checks now show green instead of skipped.

Docs (docs(ivf):)

The hnsw initializer suits nprobe far below the cluster count: graph construction can leave centroids unreachable, so points in those clusters are not returned at any nprobe, and an nprobe near the cluster count raises instead of probing every cluster.

Verification

  • check-format.sh, check-python.sh, ruff on .github/scripts, git diff --check.
  • GCC 14 portable build: ctest --no-tests=error 434/434; check-isa-symbols.sh at -O3, -O1, -O2 -g.
  • Clang 23 ASan/UBSan build with the new checks: 434/434 (one pre-existing vendored-Eigen AVX-512 pointer-overflow diagnostic suppressed; unrelated).
  • Full check-tidy.sh build-tidy (Clang 23).
  • Reinstalled package: pytest tests/python 837 passed, 2 skipped (wheel-only); editor-support, mypy and pyright tests.
  • Search outputs on identical saved indexes are bit-identical before and after the FastScan change (IVF 1/4/7 bits × L2/IP × HACC on/off, QG 4/8 bits).
  • mkdocs build --strict; .github/scripts unittest suite 60/60.

Not verified locally: hosted-runner behaviour of the gate change (its own CI run on this PR is the check), MSVC, NEON.

…r count

Graph construction can leave centroids unreachable, so points in those clusters are never returned and an nprobe close to the cluster count raises instead of probing every cluster. State the intended regime and point at flat and flat_rabitq for exhaustive probing.
The batch correction in estimator_kernels.hpp and qg_estimator.hpp instantiated Eigen assignment loops from the generic, AVX2 and AVX-512 translation units. Below -O3 those helpers survive as identically named weak symbols with different instruction sets, and only the static-archive member order kept the generic copy in front of the ISA copies. Write the 32-lane corrections as plain loops so each backend vectorizes them with its own flags and no ISA source emits a shared Eigen helper.

check-isa-symbols.sh now passes at -O1 and -O2 -g as well as -O3; search outputs on identical saved indexes are bit-identical before and after.
Each wheel gate was skipped whenever its build job was skipped, and the build job is skipped when the prepare job fails. A skipped required check satisfies branch protection, so a failed eligibility check waived the wheel builds, the installed-wheel suite and the typing checks on a pull request. Run the gates with always() and decide from the prepare, changes and build results, matching the other aggregator jobs; the contract test now exercises every input combination of the shared verdict script.
… point

IVF, HNSW and SymphonyQG accepted NaN, infinite and arbitrarily large coordinates in data, centroids and queries. The core quantizer then converted NaN to int (undefined behaviour, reported by UBSan's float-cast-overflow check), stored NaN factors, and in one-bit high-accuracy IVF search returned a poisoned point first with a NaN distance even after remove(). The stable core API and the clustering APIs already rejected such input.

Add utils/input_checks.hpp with a finiteness scan and the clustering coordinate bound, call it at construct, add and search entry points before any state changes, and reject non-finite queries in Lut. Enable float-cast-overflow and float-divide-by-zero in the sanitizer build, since -fsanitize=undefined does not include them. Document the contract in the usage guide and the Python docstrings, and cover each entry point with C++ and Python regression tests.
@github-actions github-actions Bot added the release:fixed Release category managed from the PR title label Oct 31, 2026
lut.hpp includes utils/input_checks.hpp, so installed consumers of the FastScan headers need it too.
…orts

The installed-consumer options check hard-coded -fsanitize=address,undefined; the sanitizer build now also enables float-cast-overflow and float-divide-by-zero.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:fixed Release category managed from the PR title

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant