Skip to content

[codex] Document binding safety and release artifacts#249

Merged
Navi Bot (project-navi-bot) merged 3 commits into
mainfrom
codex/prerelease-bindings-artifact-docs
Jun 19, 2026
Merged

[codex] Document binding safety and release artifacts#249
Navi Bot (project-navi-bot) merged 3 commits into
mainfrom
codex/prerelease-bindings-artifact-docs

Conversation

@Fieldnote-Echo

Copy link
Copy Markdown
Member

Summary

  • add a consolidated Rust/Python/C/Go bindings safety and ownership contract
  • add a release-facing artifact and platform matrix for crates, wheels, and repo-local sidecars
  • link the new docs from README, C API docs, Go/Python binding docs, MSRV/features, and the release checklist

Closes #123.
Closes #141.

Validation

  • gofmt -w ordvec-go/doc.go
  • git diff --check
  • python tests/release_publish_invariants.py

Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces two new documentation files, docs/artifact-platform-matrix.md and docs/bindings-safety.md, which detail the release artifact inventory and cross-language safety contracts, and updates references to them across existing READMEs and docs. The review feedback suggests clarifying whether ordvec_last_error() is populated during Rust panics, highlighting the performance implications of duplicate candidate IDs, and recommending standard path sanitization practices to help developers avoid path traversal issues.

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.

Comment thread docs/bindings-safety.md Outdated
Comment thread docs/bindings-safety.md Outdated
Comment thread docs/bindings-safety.md
@chatgpt-codex-connector

Copy link
Copy Markdown

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Document bindings safety contract and release artifact matrix
📝 Documentation 🕐 20-40 Minutes

Grey Divider

Description

• Add consolidated cross-language binding safety/ownership contract documentation.
• Add release-facing artifact and platform matrix for crates, wheels, and sidecars.
• Link the new docs from README, binding docs, MSRV/features, and release checklist.
Diagram

graph TD
  A["README.md"] --> B["docs/bindings-safety.md"] --> C["docs/c-api.md"]
  A --> D["docs/artifact-platform-matrix.md"] --> E["RELEASING.md"]
  F["ordvec-python/README.md"] --> B
  G["ordvec-go/README.md"] --> B
  H["docs/msrv-and-features.md"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep per-language safety notes only
  • ➕ Keeps docs closest to the implementation surface (Rust/Python/C/Go)
  • ➕ Avoids having to maintain a central contract document
  • ➖ Contract details drift across languages over time
  • ➖ Harder for embedders to find the authoritative threading/ownership rules
  • ➖ Release reviewers must chase multiple docs to validate invariants
2. Generate artifact/platform matrix from CI/workflow
  • ➕ Single source of truth directly derived from build/release jobs
  • ➕ Reduces risk of docs diverging from the workflow
  • ➖ More engineering overhead (parsing workflow + rendering docs)
  • ➖ Harder to include the explanatory context reviewers need (what/why/how to verify)

Recommendation: The chosen approach (two canonical, human-maintained docs with broad cross-linking) is the best tradeoff for reviewer/consumer clarity and low maintenance overhead. If the artifact matrix begins to drift, consider a future follow-up to partially generate the table from the release workflow while keeping the narrative guidance human-authored.

Files changed (9) +154 / -2

Documentation (9) +154 / -2
README.mdLink consolidated safety contract and artifact matrix from README +6/-0

Link consolidated safety contract and artifact matrix from README

• Adds a pointer to the new cross-language bindings safety/ownership contract and highlights the new artifact/platform matrix in the documentation index section.

README.md

RELEASING.mdAdd artifact/platform matrix check to release verification steps +2/-0

Add artifact/platform matrix check to release verification steps

• Extends the release verification checklist to compare observed release assets against the documented artifact/platform matrix.

RELEASING.md

artifact-platform-matrix.mdAdd release-facing artifact and platform inventory +44/-0

Add release-facing artifact and platform inventory

• Introduces a new document enumerating published artifacts (Rust crates, Python wheels/sdists), repo-local sidecars, and platform expectations plus suggested verification steps.

docs/artifact-platform-matrix.md

bindings-safety.mdAdd cross-language bindings safety and ownership contract +84/-0

Add cross-language bindings safety and ownership contract

• Introduces a consolidated contract covering concurrency, borrowed inputs, row ordinals vs external IDs, path trust, and error/panic behavior across Rust/Python/C/Go. Includes a short release review checklist for binding-impacting changes.

docs/bindings-safety.md

c-api.mdPoint C API documentation to consolidated bindings safety contract +3/-1

Point C API documentation to consolidated bindings safety contract

• Augments the ABI policy section with a link to the new cross-language ownership/threading/path-trust contract document.

docs/c-api.md

msrv-and-features.mdLink MSRV/features doc to artifact/platform matrix +2/-1

Link MSRV/features doc to artifact/platform matrix

• Adds a reference to the artifact/platform matrix as the inventory of release artifacts and wheel targets.

docs/msrv-and-features.md

README.mdLink Go wrapper docs to consolidated bindings safety contract +2/-0

Link Go wrapper docs to consolidated bindings safety contract

• Adds a short note directing readers to the shared ownership/lifetime contract document.

ordvec-go/README.md

doc.goReference consolidated bindings safety contract in Go package docs +3/-0

Reference consolidated bindings safety contract in Go package docs

• Adds a package-level documentation pointer to the cross-language bindings safety/ownership contract.

ordvec-go/doc.go

README.mdAdd Python safety contract section and link to consolidated doc +8/-0

Add Python safety contract section and link to consolidated doc

• Documents the GIL-release and borrowed-NumPy-array implications for thread safety, and links to the consolidated bindings safety/ownership contract.

ordvec-python/README.md

@qodo-code-review

qodo-code-review Bot commented Jun 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

Context used

Grey Divider


Action required

1. Matrix omits SBOM expectations ✓ Resolved 📎 Requirement gap ⛨ Security
Description
The matrix mentions provenance/attestations but does not state SBOM expectations for release
artifacts. This leaves supply-chain metadata requirements unclear for downstream compliance and
audit workflows.
Code

docs/artifact-platform-matrix.md[R10-15]

+| Surface | Published where | Platform/build contract | Release verification |
+| --- | --- | --- | --- |
+| `ordvec` Rust crate | crates.io package `ordvec`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; pure Rust, no BLAS/LAPACK/system numeric dependency | `cargo package --locked`; GitHub/Sigstore/SLSA provenance; pre-publish and post-publish byte identity against crates.io |
+| `ordvec-manifest` Rust crate | crates.io package `ordvec-manifest`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; optional `cli`, `sqlite`, and `sqlite-bundled` features | Built after matching `ordvec` exists; GitHub/Sigstore/SLSA provenance; byte identity against crates.io |
+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
Relevance

⭐⭐⭐ High

Release process already generates/guards SBOMs; SBOM/provenance work merged in PRs #43/#50.

PR-#43
PR-#50

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 5 requires SBOM/provenance expectations to be mentioned in the matrix. The table
entries include provenance language (e.g., GitHub/Sigstore/SLSA provenance) but do not state any
SBOM expectation (format, presence, or attachment) for crates/wheels.

Support matrix includes SBOM/provenance expectations
docs/artifact-platform-matrix.md[10-15]

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

## Issue description
The artifact/platform matrix does not mention SBOM expectations/policy for published artifacts.

## Issue Context
Compliance requires SBOM/provenance expectations to be stated at least at a documented policy/expectation level (e.g., whether SBOMs are generated, format, where attached, and for which artifacts).

## Fix Focus Areas
- docs/artifact-platform-matrix.md[10-15]

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


2. No native library loading rules ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
The matrix includes SIMD notes but does not document native library naming/loading/version alignment
across artifacts. This can cause packaging/runtime breakage when embedding via C/Go/Python and when
aligning versions across release assets.
Code

docs/artifact-platform-matrix.md[R30-35]

+## Platform Notes
+
+- SIMD dispatch in the core crate is not feature-gated. x86_64 dispatches
+  AVX-512 and AVX2 at runtime where available, aarch64 uses NEON, wasm32 can
+  use `simd128` when built with that target feature, and unsupported targets
+  use scalar fallback paths.
Relevance

⭐⭐ Medium

No historical evidence found for native library naming/loading/version alignment rules in docs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 4 requires CPU baseline/SIMD expectations and also native library
naming/loading/versioning strategy in the matrix. The added Platform Notes discuss SIMD dispatch and
targets, but provide no naming/loading/version alignment rules for the C ABI library and how
Python/Go relate to it.

Support matrix documents CPU baseline/SIMD expectations and native library naming/loading/versioning
docs/artifact-platform-matrix.md[30-35]

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

## Issue description
The artifact/platform matrix lacks required documentation for (1) native library naming and loading/linking strategy and (2) version alignment rules across artifacts.

## Issue Context
Compliance requires the matrix to state how native libraries are named on each OS, how each binding loads/links them (or embeds them), and how versions/ABI compatibility are aligned across Rust crate(s), wheels, and repo-local sidecars.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[30-35]

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


3. Matrix missing Node/JVM surfaces ✓ Resolved 📎 Requirement gap ⚙ Maintainability
Description
docs/artifact-platform-matrix.md does not include entries (even placeholders) for future Node/WASM
and JVM distribution surfaces. This violates the requirement to document all required artifact
surfaces so packaging expectations don’t drift.
Code

docs/artifact-platform-matrix.md[R8-29]

+## Published Artifacts
+
+| Surface | Published where | Platform/build contract | Release verification |
+| --- | --- | --- | --- |
+| `ordvec` Rust crate | crates.io package `ordvec`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; pure Rust, no BLAS/LAPACK/system numeric dependency | `cargo package --locked`; GitHub/Sigstore/SLSA provenance; pre-publish and post-publish byte identity against crates.io |
+| `ordvec-manifest` Rust crate | crates.io package `ordvec-manifest`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; optional `cli`, `sqlite`, and `sqlite-bundled` features | Built after matching `ordvec` exists; GitHub/Sigstore/SLSA provenance; byte identity against crates.io |
+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+
+The Python release currently expects exactly four wheels plus one sdist for
+each Python package. There is no macOS x86_64 wheel leg in the current release
+workflow.
+
+## Repo-Local Sidecars
+
+| Surface | Published where | Platform/build contract | Release role |
+| --- | --- | --- | --- |
+| `ordvec-ffi` | Not published to crates.io; built from the repository | Rust 1.89; emits `rlib`, `cdylib`, and `staticlib`; ABI v1 header is committed under `ordvec-ffi/include/` | C ABI compatibility surface for embedders; CI checks header drift and C link smoke |
+| `ordvec-go` | Not published as a Go module release from this repo | Thin cgo wrapper over `ordvec-ffi`; links the local Rust library | Binding smoke and race/cgocheck coverage for the C ABI contract |
+| `benchmarks/beir-bench` | Not shipped in the published crate or wheels | Workspace benchmark crate with `publish = false` | Release-adjacent benchmark harness only; not a shipped user dependency |
+| `fuzz/` | Not a workspace member and not published | `cargo-fuzz` crate with its own lockfile | Loader and parser hardening gate; release workflow runs smoke fuzz jobs |
+
Relevance

⭐⭐ Medium

No historical evidence on requiring future Node/WASM or JVM placeholder entries in matrices.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2 requires the matrix to include entries for future Node/WASM and JVM packaging
surfaces (placeholders allowed). The matrix tables list Rust crates, Python packages, and repo-local
sidecars, but contain no Node/WASM or JVM rows.

Support matrix covers required artifact surfaces (current and future-facing placeholders)
docs/artifact-platform-matrix.md[8-29]

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

## Issue description
The artifact/platform matrix is missing the required future-facing placeholder entries for Node/WASM and JVM packaging surfaces.

## Issue Context
Compliance requires the matrix to cover Rust, Python, C ABI, Go, and also include clearly marked placeholders for planned Node/WASM and JVM surfaces.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[8-29]

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


View more (2)
4. Python Linux libc unspecified ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
The matrix lists Linux wheel platforms but does not distinguish glibc vs musl coverage. This leaves
Linux platform expectations ambiguous for downstream packagers and users.
Code

docs/artifact-platform-matrix.md[R14-19]

+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+
+The Python release currently expects exactly four wheels plus one sdist for
+each Python package. There is no macOS x86_64 wheel leg in the current release
+workflow.
Relevance

⭐⭐ Medium

No prior accepted/rejected guidance found on documenting manylinux vs musllinux (glibc vs musl)
wheels.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 3 requires Linux libc variants to be called out where applicable. The Python wheel
rows state Linux x86_64 and Linux aarch64 but do not specify glibc (manylinux) vs musl
(musllinux), and the nearby note does not add that distinction.

Support matrix covers required platforms/architectures and libc variants where applicable
docs/artifact-platform-matrix.md[14-19]

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

## Issue description
The support matrix lists Linux wheel targets but does not state whether they are glibc-only (manylinux) and/or musl (musllinux), which is required where applicable.

## Issue Context
Compliance requires Linux libc variants (glibc vs musl) to be explicitly documented per artifact where applicable.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[14-19]

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


5. No C/Go freeing rules ✓ Resolved 📎 Requirement gap ⛨ Security
Description
docs/bindings-safety.md describes borrowed inputs and panic/error boundaries but does not document
how memory returned across C/Go boundaries must be freed and by whom. This risks leaks or
double-frees for embedders consuming returned allocations.
Code

docs/bindings-safety.md[R24-33]

+## Borrowed Inputs
+
+Caller-provided buffers are borrowed for the duration of the call and are not
+retained after the function returns.
+
+- Do not mutate Rust slices, NumPy arrays, C buffers, or Go slices while a call
+  that received them is in progress.
+- Query, corpus, candidate, output, hit, and stats buffers remain caller-owned
+  unless a specific API says otherwise.
+- Candidate lists are entry lists, not sets. Duplicate candidate IDs are scored
Relevance

⭐⭐ Medium

No historical evidence found for adding returned-memory freeing rules to binding safety docs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 15 requires returned-memory freeing rules to be documented for C/Go-style bindings.
The new canonical safety doc covers borrowing rules and error/panic behavior, but contains no
section describing how to free any memory returned from the C ABI/Go wrapper (allocator/free
function/responsibility).

Returned-memory freeing rules are documented for C/Go-style bindings
docs/bindings-safety.md[24-33]
docs/bindings-safety.md[60-72]

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

## Issue description
The canonical bindings safety contract does not document returned-memory freeing rules for C/Go-style bindings (allocator choice, which side frees, and which function to call).

## Issue Context
Compliance requires the freeing rules to be explicitly documented to prevent leaks/double-frees when crossing FFI boundaries.

## Fix Focus Areas
- docs/bindings-safety.md[24-33]
- docs/bindings-safety.md[60-72]

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



Remediation recommended

6. Matrix scope not explicitly limited ✓ Resolved 📎 Requirement gap ⚙ Maintainability
Description
The matrix describes release inventory/build contracts but does not explicitly clarify it is
packaging/distribution compatibility (not service/runtime support). This can lead to
misinterpretation of the commitments being made.
Code

docs/artifact-platform-matrix.md[R3-6]

+This matrix is the release-facing inventory for what `ordvec` publishes, what
+is repo-local, and which platform expectations are checked by the release
+workflow. It complements `RELEASING.md` and `docs/msrv-and-features.md`; the
+workflow remains the source of truth for the exact build jobs.
Relevance

⭐⭐⭐ High

Team accepts doc scope/contract clarifications; multiple docs clarifications merged in PR #156.

PR-#156

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 9 requires documentation around the matrix to clearly state it is about
packaging/distribution compatibility, not service/runtime support. The intro describes it as a
release-facing inventory and build contract but does not explicitly disclaim service/runtime support
scope.

Documentation clarifies the matrix is packaging compatibility (not service support)
docs/artifact-platform-matrix.md[3-6]

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

## Issue description
The matrix does not explicitly state that it is a packaging/distribution compatibility matrix and not a service/runtime support commitment.

## Issue Context
Compliance requires an explicit scope clarification to avoid downstream readers interpreting the matrix as runtime/service support guarantees.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[3-6]

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


7. No panic-boundary tests 📎 Requirement gap ☼ Reliability
Description
docs/bindings-safety.md documents that the C ABI catches Rust panics and returns
ORDVEC_STATUS_PANIC, but this behavior is not covered by a binding-level test, so regressions
could silently reintroduce unwinding/host crashes.
Code

docs/bindings-safety.md[R77-93]

+## Errors and Panics
+
+- The Rust crate keeps fail-loud panicking constructors and methods where that
+  is the documented API. Existing `try_*` helpers return `OrdvecError` only
+  where explicitly provided.
+- Python validates dimensions, dtypes, contiguity where required, finite
+  values, candidate ranges, and capacities at the boundary so common invalid
+  inputs raise typed Python exceptions instead of surfacing opaque Rust panics.
+- The C ABI catches Rust panics at status-returning FFI boundaries and returns
+  `ORDVEC_STATUS_PANIC`; no Rust unwind crosses the ABI boundary. The same
+  thread's `ordvec_last_error()` is set to the panic payload when it is a string
+  or to fallback panic text otherwise. Successful status-returning calls clear
+  that thread-local error. Non-status helpers such as `ordvec_last_error()`,
+  version/status accessors, and `ordvec_index_free` do not report fallible
+  status.
+- The Go wrapper maps C status values to Go errors and preserves the C ABI
+  pointer and lifetime rules.
Relevance

⭐⭐ Medium

Team values FFI panic containment, but no prior accepted request to add explicit ORDVEC_STATUS_PANIC
regression test.

PR-#101
PR-#107

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 11 requires panic boundary behavior to be both documented and tested where
practical. The new canonical contract explicitly specifies the C ABI panic translation behavior, but
the PR does not add a corresponding test to validate it.

Panic boundary behavior is documented and tested where practical
docs/bindings-safety.md[77-93]
ordvec-ffi/src/lib.rs[221-236]

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

## Issue description
The canonical bindings safety contract documents that panics are caught at the C ABI boundary and translated into `ORDVEC_STATUS_PANIC`, but there is no automated test validating this behavior.

## Issue Context
PR Compliance ID 11 requires panic boundary behavior to be documented and tested where practical. The contract is now documented in `docs/bindings-safety.md`, so adding at least one focused test helps prevent future regressions that could allow unwinding across FFI.

## Fix Focus Areas
- ordvec-ffi/src/lib.rs[221-236]
- docs/bindings-safety.md[77-93]

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



Informational

8. Unversioned safety doc link ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
ordvec-python/README.md links the safety contract to the repository’s main branch, so the README
rendered for older PyPI releases will always point to whatever the current main contains rather
than the contract that matched that release.
Code

ordvec-python/README.md[R65-71]

+## Safety contract
+
+The Python binding releases the GIL while Rust searches, scores, and mutates
+indexes. NumPy arrays passed to those methods are read in place while the call
+is active; do not mutate them from another thread until the method returns.
+The cross-language ownership and lifetime contract is maintained in
+[`docs/bindings-safety.md`](https://github.com/Fieldnote-Echo/ordvec/blob/main/docs/bindings-safety.md).
Relevance

⭐ Low

Similar request to avoid blob/main links was definitely rejected in PR #76.

PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The README’s new safety section explicitly links to blob/main, which is not version-specific and
will drift relative to published artifacts over time.

ordvec-python/README.md[65-72]

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

### Issue description
The PyPI-facing README links `docs/bindings-safety.md` via a `blob/main/...` URL, which is a moving target. For users reading an older release’s README, this can show a different safety/ownership contract than the one that applied to that released version.

### Issue Context
This link appears in the newly added “Safety contract” section of `ordvec-python/README.md`.

### Fix
Change the URL to a version-pinned permalink (release tag or commit SHA). Example options:
- `https://github.com/Fieldnote-Echo/ordvec/blob/vX.Y.Z/docs/bindings-safety.md`
- `https://github.com/Fieldnote-Echo/ordvec/blob/<commit-sha>/docs/bindings-safety.md`

If you want this to stay automatic, update it as part of the release process when the tag/version is known.

### Fix Focus Areas
- ordvec-python/README.md[65-71]

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


Grey Divider

Previous review results

Review updated until commit 518cd04

Results up to commit cdc87f5


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


Action required
1. Matrix omits SBOM expectations ✓ Resolved 📎 Requirement gap ⛨ Security
Description
The matrix mentions provenance/attestations but does not state SBOM expectations for release
artifacts. This leaves supply-chain metadata requirements unclear for downstream compliance and
audit workflows.
Code

docs/artifact-platform-matrix.md[R10-15]

+| Surface | Published where | Platform/build contract | Release verification |
+| --- | --- | --- | --- |
+| `ordvec` Rust crate | crates.io package `ordvec`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; pure Rust, no BLAS/LAPACK/system numeric dependency | `cargo package --locked`; GitHub/Sigstore/SLSA provenance; pre-publish and post-publish byte identity against crates.io |
+| `ordvec-manifest` Rust crate | crates.io package `ordvec-manifest`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; optional `cli`, `sqlite`, and `sqlite-bundled` features | Built after matching `ordvec` exists; GitHub/Sigstore/SLSA provenance; byte identity against crates.io |
+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
Relevance

⭐⭐⭐ High

Release process already generates/guards SBOMs; SBOM/provenance work merged in PRs #43/#50.

PR-#43
PR-#50

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 5 requires SBOM/provenance expectations to be mentioned in the matrix. The table
entries include provenance language (e.g., GitHub/Sigstore/SLSA provenance) but do not state any
SBOM expectation (format, presence, or attachment) for crates/wheels.

Support matrix includes SBOM/provenance expectations
docs/artifact-platform-matrix.md[10-15]

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

## Issue description
The artifact/platform matrix does not mention SBOM expectations/policy for published artifacts.

## Issue Context
Compliance requires SBOM/provenance expectations to be stated at least at a documented policy/expectation level (e.g., whether SBOMs are generated, format, where attached, and for which artifacts).

## Fix Focus Areas
- docs/artifact-platform-matrix.md[10-15]

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


2. Matrix missing Node/JVM surfaces ✓ Resolved 📎 Requirement gap ⚙ Maintainability
Description
docs/artifact-platform-matrix.md does not include entries (even placeholders) for future Node/WASM
and JVM distribution surfaces. This violates the requirement to document all required artifact
surfaces so packaging expectations don’t drift.
Code

docs/artifact-platform-matrix.md[R8-29]

+## Published Artifacts
+
+| Surface | Published where | Platform/build contract | Release verification |
+| --- | --- | --- | --- |
+| `ordvec` Rust crate | crates.io package `ordvec`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; pure Rust, no BLAS/LAPACK/system numeric dependency | `cargo package --locked`; GitHub/Sigstore/SLSA provenance; pre-publish and post-publish byte identity against crates.io |
+| `ordvec-manifest` Rust crate | crates.io package `ordvec-manifest`; GitHub Release `.crate` asset | Rust 1.89 MSRV; default features empty; optional `cli`, `sqlite`, and `sqlite-bundled` features | Built after matching `ordvec` exists; GitHub/Sigstore/SLSA provenance; byte identity against crates.io |
+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+
+The Python release currently expects exactly four wheels plus one sdist for
+each Python package. There is no macOS x86_64 wheel leg in the current release
+workflow.
+
+## Repo-Local Sidecars
+
+| Surface | Published where | Platform/build contract | Release role |
+| --- | --- | --- | --- |
+| `ordvec-ffi` | Not published to crates.io; built from the repository | Rust 1.89; emits `rlib`, `cdylib`, and `staticlib`; ABI v1 header is committed under `ordvec-ffi/include/` | C ABI compatibility surface for embedders; CI checks header drift and C link smoke |
+| `ordvec-go` | Not published as a Go module release from this repo | Thin cgo wrapper over `ordvec-ffi`; links the local Rust library | Binding smoke and race/cgocheck coverage for the C ABI contract |
+| `benchmarks/beir-bench` | Not shipped in the published crate or wheels | Workspace benchmark crate with `publish = false` | Release-adjacent benchmark harness only; not a shipped user dependency |
+| `fuzz/` | Not a workspace member and not published | `cargo-fuzz` crate with its own lockfile | Loader and parser hardening gate; release workflow runs smoke fuzz jobs |
+
Relevance

⭐⭐ Medium

No historical evidence on requiring future Node/WASM or JVM placeholder entries in matrices.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2 requires the matrix to include entries for future Node/WASM and JVM packaging
surfaces (placeholders allowed). The matrix tables list Rust crates, Python packages, and repo-local
sidecars, but contain no Node/WASM or JVM rows.

Support matrix covers required artifact surfaces (current and future-facing placeholders)
docs/artifact-platform-matrix.md[8-29]

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

## Issue description
The artifact/platform matrix is missing the required future-facing placeholder entries for Node/WASM and JVM packaging surfaces.

## Issue Context
Compliance requires the matrix to cover Rust, Python, C ABI, Go, and also include clearly marked placeholders for planned Node/WASM and JVM surfaces.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[8-29]

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


3. No native library loading rules ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
The matrix includes SIMD notes but does not document native library naming/loading/version alignment
across artifacts. This can cause packaging/runtime breakage when embedding via C/Go/Python and when
aligning versions across release assets.
Code

docs/artifact-platform-matrix.md[R30-35]

+## Platform Notes
+
+- SIMD dispatch in the core crate is not feature-gated. x86_64 dispatches
+  AVX-512 and AVX2 at runtime where available, aarch64 uses NEON, wasm32 can
+  use `simd128` when built with that target feature, and unsupported targets
+  use scalar fallback paths.
Relevance

⭐⭐ Medium

No historical evidence found for native library naming/loading/version alignment rules in docs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 4 requires CPU baseline/SIMD expectations and also native library
naming/loading/versioning strategy in the matrix. The added Platform Notes discuss SIMD dispatch and
targets, but provide no naming/loading/version alignment rules for the C ABI library and how
Python/Go relate to it.

Support matrix documents CPU baseline/SIMD expectations and native library naming/loading/versioning
docs/artifact-platform-matrix.md[30-35]

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

## Issue description
The artifact/platform matrix lacks required documentation for (1) native library naming and loading/linking strategy and (2) version alignment rules across artifacts.

## Issue Context
Compliance requires the matrix to state how native libraries are named on each OS, how each binding loads/links them (or embeds them), and how versions/ABI compatibility are aligned across Rust crate(s), wheels, and repo-local sidecars.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[30-35]

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


View more (2)
4. Python Linux libc unspecified ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
The matrix lists Linux wheel platforms but does not distinguish glibc vs musl coverage. This leaves
Linux platform expectations ambiguous for downstream packagers and users.
Code

docs/artifact-platform-matrix.md[R14-19]

+| Python `ordvec` | PyPI package `ordvec`; GitHub Release wheels and sdist | CPython 3.10+ abi3; `numpy>=2.2`; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+| Python `ordvec-manifest` | PyPI package `ordvec-manifest`; GitHub Release wheels and sdist | CPython 3.10+ abi3; wheels for Linux x86_64, Linux aarch64, macOS aarch64, and Windows x64 | Canonical wheel/sdist selection; linux/aarch64 native smoke; PyPI hash verification; PEP 740 attestation on fresh upload |
+
+The Python release currently expects exactly four wheels plus one sdist for
+each Python package. There is no macOS x86_64 wheel leg in the current release
+workflow.
Relevance

⭐⭐ Medium

No prior accepted/rejected guidance found on documenting manylinux vs musllinux (glibc vs musl)
wheels.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 3 requires Linux libc variants to be called out where applicable. The Python wheel
rows state Linux x86_64 and Linux aarch64 but do not specify glibc (manylinux) vs musl
(musllinux), and the nearby note does not add that distinction.

Support matrix covers required platforms/architectures and libc variants where applicable
docs/artifact-platform-matrix.md[14-19]

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

## Issue description
The support matrix lists Linux wheel targets but does not state whether they are glibc-only (manylinux) and/or musl (musllinux), which is required where applicable.

## Issue Context
Compliance requires Linux libc variants (glibc vs musl) to be explicitly documented per artifact where applicable.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[14-19]

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


5. No C/Go freeing rules ✓ Resolved 📎 Requirement gap ⛨ Security
Description
docs/bindings-safety.md describes borrowed inputs and panic/error boundaries but does not document
how memory returned across C/Go boundaries must be freed and by whom. This risks leaks or
double-frees for embedders consuming returned allocations.
Code

docs/bindings-safety.md[R24-33]

+## Borrowed Inputs
+
+Caller-provided buffers are borrowed for the duration of the call and are not
+retained after the function returns.
+
+- Do not mutate Rust slices, NumPy arrays, C buffers, or Go slices while a call
+  that received them is in progress.
+- Query, corpus, candidate, output, hit, and stats buffers remain caller-owned
+  unless a specific API says otherwise.
+- Candidate lists are entry lists, not sets. Duplicate candidate IDs are scored
Relevance

⭐⭐ Medium

No historical evidence found for adding returned-memory freeing rules to binding safety docs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 15 requires returned-memory freeing rules to be documented for C/Go-style bindings.
The new canonical safety doc covers borrowing rules and error/panic behavior, but contains no
section describing how to free any memory returned from the C ABI/Go wrapper (allocator/free
function/responsibility).

Returned-memory freeing rules are documented for C/Go-style bindings
docs/bindings-safety.md[24-33]
docs/bindings-safety.md[60-72]

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

## Issue description
The canonical bindings safety contract does not document returned-memory freeing rules for C/Go-style bindings (allocator choice, which side frees, and which function to call).

## Issue Context
Compliance requires the freeing rules to be explicitly documented to prevent leaks/double-frees when crossing FFI boundaries.

## Fix Focus Areas
- docs/bindings-safety.md[24-33]
- docs/bindings-safety.md[60-72]

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



Remediation recommended
6. Matrix scope not explicitly limited ✓ Resolved 📎 Requirement gap ⚙ Maintainability
Description
The matrix describes release inventory/build contracts but does not explicitly clarify it is
packaging/distribution compatibility (not service/runtime support). This can lead to
misinterpretation of the commitments being made.
Code

docs/artifact-platform-matrix.md[R3-6]

+This matrix is the release-facing inventory for what `ordvec` publishes, what
+is repo-local, and which platform expectations are checked by the release
+workflow. It complements `RELEASING.md` and `docs/msrv-and-features.md`; the
+workflow remains the source of truth for the exact build jobs.
Relevance

⭐⭐⭐ High

Team accepts doc scope/contract clarifications; multiple docs clarifications merged in PR #156.

PR-#156

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 9 requires documentation around the matrix to clearly state it is about
packaging/distribution compatibility, not service/runtime support. The intro describes it as a
release-facing inventory and build contract but does not explicitly disclaim service/runtime support
scope.

Documentation clarifies the matrix is packaging compatibility (not service support)
docs/artifact-platform-matrix.md[3-6]

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

## Issue description
The matrix does not explicitly state that it is a packaging/distribution compatibility matrix and not a service/runtime support commitment.

## Issue Context
Compliance requires an explicit scope clarification to avoid downstream readers interpreting the matrix as runtime/service support guarantees.

## Fix Focus Areas
- docs/artifact-platform-matrix.md[3-6]

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



Informational
7. Unversioned safety doc link ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
ordvec-python/README.md links the safety contract to the repository’s main branch, so the README
rendered for older PyPI releases will always point to whatever the current main contains rather
than the contract that matched that release.
Code

ordvec-python/README.md[R65-71]

+## Safety contract
+
+The Python binding releases the GIL while Rust searches, scores, and mutates
+indexes. NumPy arrays passed to those methods are read in place while the call
+is active; do not mutate them from another thread until the method returns.
+The cross-language ownership and lifetime contract is maintained in
+[`docs/bindings-safety.md`](https://github.com/Fieldnote-Echo/ordvec/blob/main/docs/bindings-safety.md).
Relevance

⭐ Low

Similar request to avoid blob/main links was definitely rejected in PR #76.

PR-#76

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The README’s new safety section explicitly links to blob/main, which is not version-specific and
will drift relative to published artifacts over time.

ordvec-python/README.md[65-72]

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

### Issue description
The PyPI-facing README links `docs/bindings-safety.md` via a `blob/main/...` URL, which is a moving target. For users reading an older release’s README, this can show a different safety/ownership contract than the one that applied to that released version.

### Issue Context
This link appears in the newly added “Safety contract” section of `ordvec-python/README.md`.

### Fix
Change the URL to a version-pinned permalink (release tag or commit SHA). Example options:
- `https://github.com/Fieldnote-Echo/ordvec/blob/vX.Y.Z/docs/bindings-safety.md`
- `https://github.com/Fieldnote-Echo/ordvec/blob/<commit-sha>/docs/bindings-safety.md`

If you want this to stay automatic, update it as part of the release process when the tag/version is known.

### Fix Focus Areas
- ordvec-python/README.md[65-71]

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


Results up to commit 4c2faa5


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


Remediation recommended
1. No panic-boundary tests 📎 Requirement gap ☼ Reliability
Description
docs/bindings-safety.md documents that the C ABI catches Rust panics and returns
ORDVEC_STATUS_PANIC, but this behavior is not covered by a binding-level test, so regressions
could silently reintroduce unwinding/host crashes.
Code

docs/bindings-safety.md[R77-93]

+## Errors and Panics
+
+- The Rust crate keeps fail-loud panicking constructors and methods where that
+  is the documented API. Existing `try_*` helpers return `OrdvecError` only
+  where explicitly provided.
+- Python validates dimensions, dtypes, contiguity where required, finite
+  values, candidate ranges, and capacities at the boundary so common invalid
+  inputs raise typed Python exceptions instead of surfacing opaque Rust panics.
+- The C ABI catches Rust panics at status-returning FFI boundaries and returns
+  `ORDVEC_STATUS_PANIC`; no Rust unwind crosses the ABI boundary. The same
+  thread's `ordvec_last_error()` is set to the panic payload when it is a string
+  or to fallback panic text otherwise. Successful status-returning calls clear
+  that thread-local error. Non-status helpers such as `ordvec_last_error()`,
+  version/status accessors, and `ordvec_index_free` do not report fallible
+  status.
+- The Go wrapper maps C status values to Go errors and preserves the C ABI
+  pointer and lifetime rules.
Relevance

⭐⭐ Medium

Team values FFI panic containment, but no prior accepted request to add explicit ORDVEC_STATUS_PANIC
regression test.

PR-#101
PR-#107

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 11 requires panic boundary behavior to be both documented and tested where
practical. The new canonical contract explicitly specifies the C ABI panic translation behavior, but
the PR does not add a corresponding test to validate it.

Panic boundary behavior is documented and tested where practical
docs/bindings-safety.md[77-93]
ordvec-ffi/src/lib.rs[221-236]

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

## Issue description
The canonical bindings safety contract documents that panics are caught at the C ABI boundary and translated into `ORDVEC_STATUS_PANIC`, but there is no automated test validating this behavior.

## Issue Context
PR Compliance ID 11 requires panic boundary behavior to be documented and tested where practical. The contract is now documented in `docs/bindings-safety.md`, so adding at least one focused test helps prevent future regressions that could allow unwinding across FFI.

## Fix Focus Areas
- ordvec-ffi/src/lib.rs[221-236]
- docs/bindings-safety.md[77-93]

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


Qodo Logo

Comment thread docs/artifact-platform-matrix.md
Comment thread docs/artifact-platform-matrix.md Outdated
Comment thread docs/artifact-platform-matrix.md
Comment thread docs/artifact-platform-matrix.md Outdated
Comment thread docs/bindings-safety.md

Copy link
Copy Markdown
Member Author

/agentic_review

@Fieldnote-Echo Nelson Spence (Fieldnote-Echo) force-pushed the codex/prerelease-bindings-artifact-docs branch from 51ff824 to a59f036 Compare June 19, 2026 16:35

Copy link
Copy Markdown
Member Author

/agentic_review

@Fieldnote-Echo Nelson Spence (Fieldnote-Echo) force-pushed the codex/prerelease-bindings-artifact-docs branch from a59f036 to 5bb5220 Compare June 19, 2026 16:39

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5bb5220

@Fieldnote-Echo Nelson Spence (Fieldnote-Echo) force-pushed the codex/prerelease-bindings-artifact-docs branch from 5bb5220 to 4c2faa5 Compare June 19, 2026 16:47
@Fieldnote-Echo

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 4c2faa5

Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
@Fieldnote-Echo Nelson Spence (Fieldnote-Echo) force-pushed the codex/prerelease-bindings-artifact-docs branch from 4c2faa5 to 518cd04 Compare June 19, 2026 16:57
@Fieldnote-Echo

Copy link
Copy Markdown
Member Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 518cd04

@project-navi-bot Navi Bot (project-navi-bot) merged commit 8a03e68 into main Jun 19, 2026
38 checks passed
@project-navi-bot Navi Bot (project-navi-bot) deleted the codex/prerelease-bindings-artifact-docs branch June 19, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dist: cross-language artifact and platform support matrix docs: bindings-facing safety and ownership contract

2 participants