[codex] add Go wrapper for C ABI#103
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request introduces ordvec-go, a Go wrapper using cgo to interface with the local ordvec-ffi C ABI, along with comprehensive unit tests and documentation. The review feedback focuses on enhancing portability by adding macOS build flags, improving security by validating paths against null byte injection, and optimizing performance on the search hot path by caching immutable index metadata to avoid redundant, thread-locking CGO calls.
c0288e2 to
fe55297
Compare
f2d74e2 to
9b7106a
Compare
Review Summary by QodoAdd Go wrapper for ordvec C ABI with comprehensive testing
WalkthroughsDescription• Add Go wrapper module for C ABI with Load, Index.Close, Index.Info, Index.Search • Implement thread-safe operations with mutex protection and OS thread locking • Make Close idempotent, return ErrClosed after close, serialize Search/Info • Copy C hits/stats into Go-owned values with comprehensive error handling • Add extensive test coverage including subset search, stats, race conditions Diagramflowchart LR
A["C ABI<br/>ordvec-ffi"] -->|cgo bindings| B["ordvec.go<br/>Core wrapper"]
B -->|Load| C["Index struct<br/>Thread-safe"]
C -->|Close| D["Resource cleanup<br/>Idempotent"]
C -->|Info| E["Index metadata"]
C -->|Search| F["Hit results<br/>+ Stats"]
G["Test fixtures<br/>RankQuant/Bitmap"] -->|Load| H["ordvec_test.go<br/>14 test cases"]
H -->|Validate| I["Correctness<br/>+ Race safety"]
File Changes1. ordvec-go/README.md
|
Code Review by Qodo
1. Linux/debug-only cgo linking
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b7106a654
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a new repo-local Go module (ordvec-go) that wraps the committed ordvec-ffi C ABI via cgo, providing a Go-friendly Load, Index.Info, Index.Search, and idempotent Index.Close, plus a Go test suite covering basic functionality and edge cases.
Changes:
- Introduce
ordvec-goGo module with cgo bindings and typed status/error handling. - Implement
Indexlifecycle + concurrency control (serializeCloseagainstInfo/Search) and copy C results into Go-owned values. - Add Go tests that generate minimal fixture indexes and validate load/info/search behaviors and error semantics.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| ordvec-go/README.md | Adds build/test usage notes for the new Go wrapper. |
| ordvec-go/ordvec.go | Implements the cgo wrapper types and API surface (Load, Info, Search, Close). |
| ordvec-go/ordvec_test.go | Adds fixture-based tests covering search modes, stats, and error handling. |
| ordvec-go/go.mod | Defines the new ordvec-go Go module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fe55297 to
1c90f12
Compare
9b7106a to
96fb043
Compare
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
1c90f12 to
22cb986
Compare
96fb043 to
a79fd54
Compare
|
Addressed the remaining bot findings for this PR:
Validation run locally: |
Summary
ordvec-gomodule wrapping the committed C ABI withLoad,Index.Close,Index.Info, andIndex.SearchCloseidempotent, returnErrClosedafter close, serializeSearch/InfoagainstClose, and copy C hits/stats into Go-owned valuesErrClosed, and race-test coverageValidation
cargo fmt --all --checkcbindgen ordvec-ffi --config ordvec-ffi/cbindgen.toml --output ordvec-ffi/include/ordvec.h --verifycargo testcargo test -p ordvec-fficargo clippy --all-targets -- -D warningscargo clippy -p ordvec-ffi --all-targets -- -D warningscargo build -p ordvec-ffiGOCACHE=/tmp/go-build go test -count=1 ./...fromordvec-goGOCACHE=/tmp/go-build go test -race -count=1 ./...fromordvec-goStack base: PR #102 /
codex/c-api-docs.