chore: phase 1 -- CI, release pipeline, community files - #43
Conversation
Remove 6 unused locals and 1 ambiguous variable name to get ruff passing with zero warnings. All 151 tests still pass.
Foundation for professionalizing the project. - ci.yml: pytest on ubuntu/macOS/windows x py3.10/3.12 (+3.13 on ubuntu), ruff check as required, mypy --strict as warning-only. - release.yml: cibuildwheel for linux-x86_64/aarch64, macOS-x86_64/arm64, windows-AMD64 x py3.10-3.13; PyPI trusted publishing on tag v*. - dependabot: monthly updates for actions and pip dev deps. - Issue templates (bug, feature) + PR template + Discussions link. - SECURITY, CODE_OF_CONDUCT, CONTRIBUTING with full dev setup. - .pre-commit-config.yaml with ruff + file hygiene hooks. - README badges: PyPI, Python versions, CI, license, downloads. - CLAUDE.md with project-specific commit and code style rules.
There was a problem hiding this comment.
Code Review
This pull request establishes project governance and development infrastructure by adding issue and pull request templates, a code of conduct, contribution guidelines, and security policies. It also configures automated maintenance tools like Dependabot and pre-commit hooks. Additionally, the changes include code cleanup by removing unused imports and variables in the core library and test suite. Feedback highlights the need to remove ruff format instructions from the documentation and PR checklist, as it can destructively reformat hand-aligned constants in specific files.
| ruff check snapvec/ tests/ | ||
| ruff format snapvec/ tests/ |
There was a problem hiding this comment.
The PR description states that ruff format is intentionally omitted because it is destructive for _codebooks.py. However, the contribution guide still instructs users to run it. To avoid accidental destruction of hand-aligned constants, this instruction should be removed, or _codebooks.py should be excluded in the project's configuration (e.g., in pyproject.toml).
| ruff check snapvec/ tests/ | |
| ruff format snapvec/ tests/ | |
| ruff check snapvec/ tests/ |
| ## Checklist | ||
|
|
||
| - [ ] Tests added or updated | ||
| - [ ] `ruff check` and `ruff format` pass |
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull request overview
Adds the initial “professionalization” layer for snapvec (CI, release automation, and community health files) plus small lint-driven cleanups, without changing library behavior.
Changes:
- Add GitHub Actions CI (ruff gate + pytest matrix) and a tag-based release workflow building/publishing wheels via
cibuildwheel. - Add community/repo hygiene files: security policy, contributing/code-of-conduct, issue/PR templates, dependabot, and pre-commit.
- Clean up lint issues (unused imports/locals, ambiguous loop variable) and add README badges.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Introduces CI matrix (lint + tests + coverage artifact). |
.github/workflows/release.yml |
Adds wheel/sdist build and trusted publishing to PyPI on tags. |
.github/dependabot.yml |
Configures monthly dependency update PRs for Actions + pip. |
.pre-commit-config.yaml |
Adds pre-commit hooks (file hygiene + ruff). |
.github/PULL_REQUEST_TEMPLATE.md |
Adds PR checklist for contributor workflow consistency. |
.github/ISSUE_TEMPLATE/bug_report.yml |
Adds structured bug report template. |
.github/ISSUE_TEMPLATE/feature_request.yml |
Adds structured feature request template. |
.github/ISSUE_TEMPLATE/config.yml |
Disables blank issues; routes questions to Discussions. |
SECURITY.md |
Documents vulnerability reporting and support policy. |
CODE_OF_CONDUCT.md |
Adds Contributor Covenant CoC. |
CONTRIBUTING.md |
Documents dev setup, checks, contribution and release process. |
CLAUDE.md |
Adds tool-specific repo instructions. |
README.md |
Adds badges for package/CI/license/downloads. |
snapvec/_residual.py |
Removes unused os import. |
snapvec/_pq.py |
Removes unused os import. |
snapvec/_ivfpq.py |
Removes unused os import. |
snapvec/_kmeans.py |
Removes unused Any import. |
snapvec/_fast_fallback.py |
Removes unused local variable. |
tests/test_residual.py |
Removes unused RNG local. |
tests/test_ivfpq.py |
Removes unused locals; simplifies offset-corruption test setup. |
tests/test_ivfpq_filter.py |
Renames ambiguous loop variable; replaces em dash in comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Corrupt: scramble offsets to be non-monotone (swap first two). | ||
| buf = bytearray(path.read_bytes()) | ||
| # Offsets are stored right after coarse + codebooks. Easier to | ||
| # mutate via a fresh reload, edit, re-save. |
There was a problem hiding this comment.
The corruption comment says “swap first two” to make offsets non-monotone, but the test now only sets reloaded._offsets[0] = 99 (breaking the offsets[0]==0 invariant). Update the comment to reflect the actual corruption being performed so future readers don’t chase a mismatch.
| ```bash | ||
| ruff check snapvec/ tests/ | ||
| ruff format snapvec/ tests/ | ||
| mypy --strict snapvec/ | ||
| ``` |
There was a problem hiding this comment.
CONTRIBUTING.md instructs contributors to run ruff format snapvec/ tests/, but the PR description notes that ruff format is intentionally omitted because it rewrites the hand-aligned constants in snapvec/_codebooks.py. This is conflicting guidance; either (a) configure ruff formatting to exclude that file and then recommend/enforce formatting, or (b) remove/adjust the ruff format instruction here to match the intended workflow.
| - [ ] Tests added or updated | ||
| - [ ] `ruff check` and `ruff format` pass | ||
| - [ ] `pytest -q` passes locally |
There was a problem hiding this comment.
The PR checklist requires ruff format to pass, but the PR description states formatting checks are intentionally omitted because ruff format rewrites the hand-aligned constants in snapvec/_codebooks.py. Please align the PR template with the intended workflow (e.g., drop the ruff format item or exclude the sensitive file so formatting can be required).
| CIBW_ARCHS: ${{ matrix.cibw_archs }} | ||
| CIBW_BEFORE_ALL_MACOS: "brew install libomp" | ||
| CIBW_ENVIRONMENT_MACOS: >- | ||
| MACOSX_DEPLOYMENT_TARGET=${{ matrix.os == 'macos-13' && '13.0' || '14.0' }} |
There was a problem hiding this comment.
Setting MACOSX_DEPLOYMENT_TARGET to 14.0 for the macOS-14 wheel build will produce wheels that can’t be installed on macOS 13 (arm64). Unless you intentionally want to require macOS 14+, consider using the lowest supported deployment target (often 12.0/13.0) consistently so the published wheels are usable on older macOS versions.
| MACOSX_DEPLOYMENT_TARGET=${{ matrix.os == 'macos-13' && '13.0' || '14.0' }} | |
| MACOSX_DEPLOYMENT_TARGET=13.0 |
| MACOSX_DEPLOYMENT_TARGET=${{ matrix.os == 'macos-13' && '13.0' || '14.0' }} | ||
| CIBW_TEST_REQUIRES: "pytest" | ||
| CIBW_TEST_COMMAND: "pytest {project}/tests -q" | ||
| CIBW_TEST_SKIP: "*-macosx_arm64" |
There was a problem hiding this comment.
CIBW_TEST_SKIP: "*-macosx_arm64" means the arm64 macOS wheels are built but never tested during the release workflow. If this isn’t required for a known incompatibility, it’s safer to run the test suite on the arm64 wheel as well (or at least document the reason for skipping).
| CIBW_TEST_SKIP: "*-macosx_arm64" |
| - Keep subject under 72 chars. Present-tense imperative. | ||
|
|
||
| ## Code style | ||
| - Pure ASCII only in source and docs. No em dashes, no smart quotes, no ellipsis character. Use `-`, `--`, `"`, `'`, `...`. |
There was a problem hiding this comment.
CLAUDE.md enforces “Pure ASCII only in source and docs”, but the repository currently contains non-ASCII characters in existing source/docstrings/comments (e.g. snapvec/_residual.py uses √, snapvec/_ivfpq.py uses →/≳, and tests contain ⟨ ⟩). This makes the guidance inaccurate and likely to confuse contributors; consider either relaxing it (e.g., apply to new changes only) or aligning the codebase by removing/rewriting the existing non-ASCII text before enforcing this rule.
| - Pure ASCII only in source and docs. No em dashes, no smart quotes, no ellipsis character. Use `-`, `--`, `"`, `'`, `...`. | |
| - Use pure ASCII for new or edited source and docs. Existing non-ASCII text may remain unless you are already touching that text. No em dashes, no smart quotes, no ellipsis character. Use `-`, `--`, `"`, `'`, `...`. |
GitHub's macos-14 arm64 runners ship Xcode 15 clang, which rejects -march=native with 'unknown target CPU apple-m1'. Clang on ARM expects -mcpu=native; -march=native is x86-specific. Detect host arch in setup.py and pick the right flag per platform. MSVC gets neither since it has no equivalent knob.
- Remove 'ruff format' from CONTRIBUTING.md and PR template: conflicts with the decision to skip it (reformats hand-aligned codebooks). Added a one-liner explaining why it is not used. - Drop stale 'swap first two' comment in test_load_validates_offsets; the test sets offsets[0]=99 to break the offsets[0]==0 invariant. - release.yml: pin MACOSX_DEPLOYMENT_TARGET=13.0 for both x86_64 and arm64 wheels so published arm64 wheels install on macOS 13+ (not just macOS 14+). Also drop CIBW_TEST_SKIP so arm64 wheels are exercised on the native arm64 runner instead of going untested. - Soften CLAUDE.md ASCII rule to 'new or edited' text; the existing docstrings use sqrt/arrows/inner-product notation that should not be churned just for style.
Bumps the version to 0.10.0 and documents the landed work. No library behaviour changes beyond the surgical SnapIndex.search(k<1) validation shipped in PR #50; everything else is CI, docs, tests, and benchmarks. Adds: - CHANGELOG entry for 0.10.0 covering PRs #43, #49, #50, #51, #52, and #53 (CI matrix + wheels, MkDocs site, 40+ new tests, threading curve, forward-compat errors, competitive Pareto bench). - ROADMAP.md with scoped plans for v0.11 (streaming ingest, OPQ, strict mypy), v0.12 (file format v2, delta buffer), and v1.0 (API freeze + deprecation policy). Explicit non-goals so nobody opens a GPU-backend PR. - CITATION.cff so downstream papers can cite snapvec with a machine-readable metadata file. References TurboQuant (arXiv:2504.19874) and Jegou et al. product quantization as underlying algorithms. - README roadmap link so ROADMAP.md is discoverable from the project root.
Resolved all outstanding mypy --strict errors so the warning-only check added in PR #43 can become a required gate. Changes: - Add snapvec/_fast.pyi stub so mypy sees the compiled Cython kernels' public API without resorting to 'type: ignore[import-not-found]'. Removes both the import errors and the unused-type-ignore warnings on the fallback branch in _pq.py / _ivfpq.py. - Annotate the five save-path closures (_write(f: 'ChecksumWriter') in _index.py, _pq.py, _residual.py, _ivfpq.py) and ChecksumWriter.__exit__ in _file_format.py so they stop tripping no-untyped-def. Import ChecksumWriter where needed. - Cast numpy returns whose dtype is exact but mypy widens to Any (argmin, astype, elementwise arithmetic): _kmeans.assign_l2, _kmeans.probe_scores_l2_monotone, _index._unpack_to_indices, _pq._preprocess_single, _ivfpq._preprocess_single. Plain typing.cast, no runtime overhead. - Lock three float-dtype assignments that np.where / division widen to float64: normalisation paths in _residual.add_batch, _pq.add_batch, _ivfpq.add_batch. Explicit .astype(np.float32) at the assignment. Infra: - Drop the deprecated numpy.typing.mypy_plugin from the mypy config. - CI: mypy step now fails on error instead of '|| true'. Result: 0 mypy errors, 190 tests still pass, ruff clean.
Summary
First phase of professionalizing snapvec. Adds the infrastructure layer
(CI, wheels, community files) without touching library code.
What's in this PR
CI / Release
.github/workflows/ci.yml-- pytest matrix on ubuntu/macOS/windows x py3.10/3.12 (plus py3.13 on ubuntu).ruff checkis a hard gate;mypy --strictruns as warning-only for now (17 existing errors, to be fixed incrementally)..github/workflows/release.yml--cibuildwheelfor linux-x86_64/aarch64, macOS-13-x86_64, macOS-14-arm64, and windows-AMD64, across cp310-cp313. Publishes to PyPI via trusted publishing onv*tags (uses thepypienvironment, no API token)..github/dependabot.yml-- monthly updates for GitHub Actions and pip dev dependencies.Community
SECURITY.md,CODE_OF_CONDUCT.md,CONTRIBUTING.mdwith the full dev setup, lint/test/bench workflow, and release process.Lint cleanup
lvariable name.ruff checknow passes with zero warnings. All 151 tests still pass.README
Pre-commit
.pre-commit-config.yamlwith ruff + basic file-hygiene hooks. (Format check intentionally omitted; runningruff formatdestroys the hand-aligned codebook constants in_codebooks.py.)Manual steps required before first release
pypi(Settings -> Environments).stffns, reposnapvec, workflowrelease.yml, environmentpypi.Test plan
ruff check snapvec/ tests/passes locallypytest -qpasses locally (151 passed)cibuildwheelbuilds at least one wheel on linux (via workflow_dispatch on a throwaway tag, optional)Out of scope