chore(ci): production release gate (CI matrix, SDE AVX-512, deny, MSRV)#3
Conversation
All 26 clippy errors fixed across src/, tests/, and examples/: - manual_is_multiple_of (13×): x % n == 0 → x.is_multiple_of(n), stable since 1.87, safe on MSRV 1.89 - manual_range_contains (2×): negated range comparisons → !(a..=b).contains(&x) - manual_repeat_n (1×): repeat(v).take(n) → repeat_n(v, n), stable 1.82 - too_many_arguments (7×): #[allow] with justifying comment on scan_b2_fastscan_avx512, scan_b2_fastscan_scalar, scan_via_lut_scalar (src), finalise_row, bench_two_stage, bench_two_stage_batched, bench_sign_two_stage_batched (examples) - needless_range_loop (9×): #[allow] on all SIMD kernel loops (bitmap.rs AVX-512 kernels, sign_bitmap.rs AVX-512 kernel, fastscan.rs scalar finalize) plus two clear mechanical rewrites in tests/rank_index/quant.rs and two #[allow] in tests/rank_index/index.rs (raw index used in assertion message) cargo fmt --all run; reformatted bitmap.rs, fastscan.rs and several test/example files. No behavior change.
Old lockfile was version 3 and still listed serde as a transitive dependency that no longer exists in the dependency tree. Regenerated with `cargo generate-lockfile`; new lockfile is version 4, contains no serde entries, and passes `cargo build --locked` and `cargo test --locked`.
Review Summary by QodoProduction release gate: CI workflow, MSRV, AVX-512 SDE, deny, and code hygiene
WalkthroughsDescription• Implement production release gate CI workflow with multi-OS testing matrix • Add AVX-512 coverage via Intel SDE emulation on Sapphire Rapids • Enforce MSRV 1.89, dependency hygiene, and packaging validation • Apply cargo fmt and clippy fixes across codebase (13× manual_is_multiple_of, 9× needless_range_loop, 7× too_many_arguments) • Add project hygiene: deny.toml (licenses/advisories/bans), CHANGELOG.md, dual MIT/Apache-2.0 license, PR template Diagramflowchart LR
A["Code Formatting<br/>cargo fmt + clippy"] --> B["CI Workflow<br/>lint/test/msrv/deps/avx512"]
C["Cargo.toml<br/>dual license + docs.rs"] --> B
D["deny.toml<br/>licenses/advisories/bans"] --> B
E["CHANGELOG.md<br/>Keep a Changelog"] --> B
F["PR Template<br/>checklist"] --> B
B --> G["Release Gate<br/>Production Ready"]
File Changes1. .github/workflows/ci.yml
|
Code Review by Qodo
1. Deps gate regex bypass
|
There was a problem hiding this comment.
Code Review
This pull request prepares the ordvec crate for its initial release (v0.1.0) by adding a PR template, a detailed changelog, and a cargo-deny configuration for license and advisory checks. The codebase has been extensively refactored for better idiomaticity and formatting, including the use of is_multiple_of for alignment checks and the addition of clippy allow attributes for specialized SIMD kernels. Additionally, the serde dependency was removed, the license was updated to MIT OR Apache-2.0, and various tests were updated for clarity. I have no feedback to provide.
The x86 SIMD dispatch (select_simd_tier + the SimdTier match arms, the AVX kernels, BATCHED_AVX512_CHUNK) is cfg(target_arch=x86_64)-gated, but the glue it references — the SimdTier::Avx2/Avx512 variants, the batched-chunk consts, and the simd_tier / centre_drop_used bindings — was defined unconditionally. On non-x86 (aarch64 / macos-latest CI) those are dead/unused and, under RUSTFLAGS=-D warnings, fail the build with 7 dead_code/unused errors. Add cfg_attr(not(target_arch=x86_64), allow(...)) to each so the crate builds clean on aarch64 (scalar path) while x86 is untouched. Verified: aarch64 lib + tests + examples compile clean under -D warnings; x86 fmt/clippy/test 80/86.
…al license)
CI workflow (.github/workflows/ci.yml) — Grumpy's release gate:
- lint: fmt --check + clippy --all-targets --all-features -D warnings
- test: ubuntu/macos/windows matrix; default, experimental,
no-default-features; release build of bench_rank example
- msrv: rust 1.89.0 build + test (proves the declared AVX-512 floor)
- deps: cargo tree gate failing on blas|openblas|faer|ndarray|statrs
+ cargo publish --dry-run
- avx512: runs the suite under Intel SDE (-spr / Sapphire Rapids) so the
runtime-dispatched AVX-512 kernels are actually covered on
GitHub-hosted runners that lack AVX-512. Includes an inline
is_x86_feature_detected! probe asserting SDE exposes avx512f +
avx512vpopcntdq. Pattern adapted from microsoft/DiskANN.
Actions pinned: dtolnay/rust-toolchain, Swatinem/rust-cache@v2,
actions/checkout@v4.
Cargo.toml:
- license = "MIT OR Apache-2.0" (was "MIT")
- [package.metadata.docs.rs]: all-features = false,
rustdoc-args = ["--cfg", "docsrs"] (keeps experimental + doc-hidden
items off the published docs page)
Hygiene:
- deny.toml (cargo-deny 0.19 schema): license allow-list
MIT/Apache-2.0/Unicode-3.0/BSD-2-Clause/Apache-2.0-WITH-LLVM-exception;
advisories + bans (multiple-versions warn, wildcards deny); sources
locked to crates.io. `cargo deny check` passes clean.
- CHANGELOG.md (Keep a Changelog) with Unreleased + 0.1.0 extraction notes
- .github/PULL_REQUEST_TEMPLATE.md
rust-toolchain.toml intentionally omitted: a committed toolchain file
takes rustup precedence over dtolnay/rust-toolchain@1.89.0 and would
silently defeat the MSRV job (verified empirically).
The previous comment claimed CI overrides a contributor rust-toolchain.toml. In fact dtolnay/rust-toolchain neither removes nor overrides one — a committed toolchain file takes rustup directory-override precedence and would silently change the MSRV floor. The repo intentionally has no such file; say so, and pin the floor in the workflow as the source of truth.
There was a problem hiding this comment.
Pull request overview
Adds a “production release gate” for the ordvec Rust crate by introducing a comprehensive CI workflow (lint/test/MSRV/deps/AVX-512-under-SDE), plus release hygiene artifacts (cargo-deny config, changelog, PR template) and some accompanying formatting/lint-driven refactors across core code, examples, and tests.
Changes:
- Add GitHub Actions CI with OS matrix testing, MSRV enforcement, dependency-tree gate + publish dry-run, and AVX-512 coverage via Intel SDE.
- Add release hygiene files (
deny.toml,CHANGELOG.md, PR template) and update crate metadata (dual license expression, docs.rs config, refreshed lockfile). - Apply clippy-/rustfmt-driven refactors and targeted
#[allow]s to keep-D warningsclean across platforms/SIMD cfgs.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
New CI workflow covering lint, OS matrix tests, MSRV, deps/publish gate, and AVX-512 coverage under Intel SDE. |
.github/PULL_REQUEST_TEMPLATE.md |
Adds a PR checklist aligned with the new CI/release gates. |
deny.toml |
Introduces cargo-deny policy for licenses/advisories/bans/sources. |
CHANGELOG.md |
Adds Keep-a-Changelog formatted changelog with initial 0.1.0 entry. |
Cargo.toml |
Updates license expression and configures docs.rs build options. |
Cargo.lock |
Regenerated lockfile (v4), removing stale transitive entries. |
src/rank_io.rs |
Refactors validation checks (range/is_multiple_of) and minor signature formatting. |
src/rank.rs |
Adds a targeted clippy allow for indexed loop; minor formatting in tests. |
src/sign_bitmap.rs |
Uses is_multiple_of, adds clippy allows for kernel-style loops, minor formatting. |
src/rank_index/index.rs |
Formatting-only struct construction change. |
src/rank_index/util.rs |
Formatting-only changes in TopK utilities. |
src/rank_index/multi_bucket.rs |
Minor formatting of parallel scoring loop. |
src/rank_index/bitmap.rs |
Uses is_multiple_of, adds clippy allows for kernel loops, minor formatting. |
src/rank_index/fastscan.rs |
Adds clippy allow for argument-heavy kernels; minor formatting. |
src/rank_index/quant.rs |
Non-x86 warning hygiene via cfg_attr allows; is_multiple_of and formatting tweaks. |
src/rank_index/quant_kernels.rs |
Adds clippy allow for argument-heavy scalar kernel; formatting in asserts. |
examples/bench_rank.rs |
Formatting and clippy allows for argument-heavy bench helpers. |
tests/redteam_alpha.rs |
Formatting-only refactors in tests. |
tests/redteam_beta.rs |
Formatting-only refactors in tests. |
tests/redteam_delta.rs |
Formatting-only refactor of temp-file forge helper. |
tests/rank_index/main.rs |
Formatting refactors; uses repeat_n; minor assertion formatting. |
tests/rank_index/index.rs |
Import ordering + targeted clippy allow for indexed loop in tests. |
tests/rank_index/quant.rs |
Import ordering + small iterator refactors and formatting. |
tests/rank_index/bitmap.rs |
Formatting-only refactors in tests. |
tests/rank_index/fastscan.rs |
Import ordering + formatting-only refactors. |
tests/rank_index/multi_bucket.rs |
Import ordering changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address bot review on PR #3: - deps gate: the word-boundary class [^[:alnum:]_-] treated '-'/'_' as part of the crate name, so blas-src / openblas-src / ndarray-linalg / faer-core / blas_sys slipped through the no-system-deps check. Drop '_' and '-' from the boundaries so those variants are caught (verified: still clean on ordvec, no false positive on names like 'myblaster'). - avx512 (SDE): pin and verify the Intel SDE tarball SHA256 before unpacking and executing it — the job previously ran remote network content unverified.
… review) Address the bot review wave (gemini/Codex/qodo) — all "convert core panics to clean Python errors", completing the binding's boundary-guard design: - Width validation (check_width): every f32 input now checks ncols == dim (2-D) / len == dim (1-D). The core derives n = len/dim and only asserts divisibility, so a wrong-but-divisible shape (e.g. (1,128) into a dim-64 index) was silently reinterpreted as a different vector count, or panicked on the result reshape. Now a clean ValueError. (gemini x3 critical, Codex x2 P1, qodo #3) - Constructor validation: Rank/RankQuant/Bitmap/SignBitmap `new` return PyResult and validate against the EXACT core asserts (dim in [2, u16::MAX]; bits in {1,2,4} + dim multiple of 8/bits and 2^bits; dim % 64 + 0 < n_top < dim; dim % 64 + <= MAX_SIGN_BITMAP_DIM) -> ValueError instead of panic. (gemini x4) - swap_remove (Rank, RankQuant): bounds-check -> IndexError, not panic. (gemini high, qodo #4) - README provenance tightened to the canonical "developed within turbovec, factored out" phrasing. (qodo #2) Tests: +9 (width-mismatch x6, swap_remove OOB x3); constructor-rejection tests tightened from BaseException to ValueError. Suite now 117 passed + 1 xfail. clippy -D warnings + fmt clean; MSRV 1.89 builds core + binding. Not changed: qodo #1 (ndarray via numpy) is a deliberate, documented core-vs- binding split (deps grep + publish scoped to -p ordvec; the core's published lock is clean; the binding is publish = false, PyPI-only) -- explained on-thread.
Production release gate — CI, deny, Cargo metadata
CI (
.github/workflows/ci.yml)cargo fmt --check+clippy --all-targets --all-features -D warnings.--features experimental,--no-default-features, + release build ofbench_rank.dtolnay/rust-toolchain@1.89.0build + test. Norust-toolchain.tomlby design — the action does not override one, so a committed file would take rustup precedence and silently change the floor.blas|openblas|faer|ndarray|statrsenters the tree (--all-features --edges normal,build,dev), thencargo publish --dry-run.RUSTFLAGS: -D warnings, concurrency cancel-in-progress, least-privilegepermissions, pinned actions.Project hygiene
deny.toml(cargo-deny: license allow-list, advisories, bans, crates.io-only sources) — passes clean.CHANGELOG.md(Keep a Changelog) +.github/PULL_REQUEST_TEMPLATE.md.Cargo.toml:license = "MIT OR Apache-2.0"+[package.metadata.docs.rs].Note
The
lint/ SDE jobs can only be fully validated on GitHub's runners; locally I verified the deps-gate regex (catches a plantedndarray, passes clean on ordvec),cargo deny check,cargo publish --dry-runpackaging, and the SDE heredoc/probe generation.