root b8c072cf0b G1: vectord — HNSW vector search via coder/hnsw · 6 scrum fixes applied
First G1+ piece. Standalone vectord service with in-memory HNSW
indexes keyed by string IDs and optional opaque JSON metadata.
Wraps github.com/coder/hnsw v0.6.1 (pure Go, no cgo). New port
:3215 with /v1/vectors/* routed through gateway.

API:
  POST   /v1/vectors/index            create
  GET    /v1/vectors/index            list
  GET    /v1/vectors/index/{name}     get info
  DELETE /v1/vectors/index/{name}
  POST   /v1/vectors/index/{name}/add (batch)
  POST   /v1/vectors/index/{name}/search

Acceptance smoke 7/7 PASS — including recall=1 on inserted vector
w-042 (cosine distance 5.96e-8, float32 precision noise), 200-
vector batch round-trip, dim mismatch → 400, missing index → 404,
duplicate create → 409.

Two upstream library quirks worked around in the wrapper:
  1. coder/hnsw.Add panics with "node not added" on re-adding an
     existing key (length-invariant fires because internal
     delete+re-add doesn't change Len). Pre-Delete fixes for n>1.
  2. Delete of the LAST node leaves layers[0] non-empty but
     entryless; next Add SIGSEGVs in Dims(). Workaround: when
     re-adding to a 1-node graph, recreate the underlying graph
     fresh via resetGraphLocked().

Cross-lineage scrum on shipped code:
  - Opus 4.7 (opencode):                 0 BLOCK + 4 WARN + 3 INFO
  - Kimi K2-0905 (openrouter):           2 BLOCK + 2 WARN + 1 INFO
  - Qwen3-coder (openrouter):            "No BLOCKs" (4 tokens)

Fixed (4 real + 2 cleanup):
  O-W1: Lookup returned the raw []float32 from coder/hnsw — caller
    mutation would corrupt index. Now copies before return.
  O-W3: NaN/Inf vectors poison HNSW (distance comparisons return
    false for both < and >, breaking heap invariants). Zero-norm
    under cosine produces NaN. Now validated at Add time.
  K-B1: Re-adding with nil metadata silently cleared the existing
    entry — JSON-omitted "metadata" field deserializes as nil,
    making upsert non-idempotent. Now nil = "leave alone"; explicit
    {} or Delete to clear.
  O-W4: Batch Add with mid-batch failure left items 0..N-1
    committed and item N rejected. Now pre-validates all IDs+dims
    before any Add.
  O-I1: jsonItoa hand-roll replaced with strconv.Itoa — no
    measured allocation win.
  O-I2: distanceFn re-resolved per Search → use stored i.g.Distance.

Dismissed (2 false positives):
  K-B2 "MaxBytesReader applied after full read" — false, applied
    BEFORE Decode in decodeJSON
  K-W1 "Search distances under read lock might see invalidated
    slices from concurrent Add" — false, RWMutex serializes
    write-lock during Add against read-lock during Search

Deferred (3): HTTP server timeouts (consistent G0 punt),
Content-Type validation (internal service behind gateway), Lookup
dim assertion (in-memory state can't drift).

The K-B1 finding is worth pausing on: nil metadata on re-add is
the kind of API ergonomics bug only a code-reading reviewer
catches — smoke would never detect it because the smoke always
sends explicit metadata. Three lines changed in Add; the resulting
API matches what callers actually expect.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 00:50:28 -05:00

278 lines
8.6 KiB
Go

// Package vectord owns the vector-search surface — HNSW indexes
// keyed by string IDs with optional opaque JSON metadata. The
// underlying library is github.com/coder/hnsw (pure Go, no cgo).
//
// G1 scope: in-memory only. Persistence to storaged + rehydrate
// across restart is the next piece — keeping it out of this layer
// makes the index API easier to test and keeps the storaged
// dependency optional for downstream tooling.
package vectord
import (
"encoding/json"
"errors"
"fmt"
"math"
"sync"
"github.com/coder/hnsw"
)
// Distance names accepted by IndexParams.Distance.
const (
DistanceCosine = "cosine"
DistanceEuclidean = "euclidean"
)
// Default HNSW parameters — match coder/hnsw's NewGraph defaults
// which are tuned for OpenAI-shaped embeddings (1536-d, but the
// hyperparameters generalize).
const (
DefaultM = 16
DefaultEfSearch = 20
)
// IndexParams describes one vector index. Once an Index is built,
// these are fixed — changing M / dimension / distance requires a
// rebuild.
type IndexParams struct {
Name string `json:"name"`
Dimension int `json:"dimension"`
M int `json:"m"`
EfSearch int `json:"ef_search"`
Distance string `json:"distance"`
}
// Result is one search hit. Distance semantics depend on the
// configured distance function — for cosine it's `1 - cos(a,b)`
// where smaller = closer; for euclidean it's the L2 norm of
// `a - b`. Either way, smaller = closer and the result list is
// sorted ascending.
type Result struct {
ID string `json:"id"`
Distance float32 `json:"distance"`
Metadata json.RawMessage `json:"metadata,omitempty"`
}
// Index wraps a coder/hnsw graph plus a side map of opaque JSON
// metadata per ID. Concurrency: read-heavy via Search (read-lock);
// Add and Delete take the write lock.
type Index struct {
params IndexParams
g *hnsw.Graph[string]
meta map[string]json.RawMessage
mu sync.RWMutex
}
// Errors surfaced to HTTP handlers. Sentinel-based so the wire
// layer can map to status codes via errors.Is.
var (
ErrDimensionMismatch = errors.New("vectord: vector dimension mismatch")
ErrUnknownDistance = errors.New("vectord: unknown distance function")
ErrInvalidParams = errors.New("vectord: invalid index params")
)
// NewIndex builds a fresh index from validated params.
func NewIndex(p IndexParams) (*Index, error) {
if p.Name == "" {
return nil, fmt.Errorf("%w: empty name", ErrInvalidParams)
}
if p.Dimension <= 0 {
return nil, fmt.Errorf("%w: dimension must be > 0 (got %d)", ErrInvalidParams, p.Dimension)
}
if p.M <= 0 {
p.M = DefaultM
}
if p.EfSearch <= 0 {
p.EfSearch = DefaultEfSearch
}
if p.Distance == "" {
p.Distance = DistanceCosine
}
dist, err := distanceFn(p.Distance)
if err != nil {
return nil, err
}
g := hnsw.NewGraph[string]()
g.M = p.M
g.EfSearch = p.EfSearch
g.Distance = dist
// Ml stays at the library default (0.25); exposing it as a knob
// is a G2 concern when we have real tuning data.
return &Index{
params: p,
g: g,
meta: make(map[string]json.RawMessage),
}, nil
}
// distanceFn maps the string name to the underlying function.
// Easier to unit-test than calling out to coder/hnsw's registry.
func distanceFn(name string) (hnsw.DistanceFunc, error) {
switch name {
case DistanceCosine, "":
return hnsw.CosineDistance, nil
case DistanceEuclidean:
return hnsw.EuclideanDistance, nil
}
return nil, fmt.Errorf("%w: %q (want cosine or euclidean)", ErrUnknownDistance, name)
}
// Params returns a copy of the immutable index params.
func (i *Index) Params() IndexParams { return i.params }
// Len returns the number of vectors currently in the index.
func (i *Index) Len() int {
i.mu.RLock()
defer i.mu.RUnlock()
return i.g.Len()
}
// Add inserts a vector with optional metadata, with replace
// semantics for the vector: if id already exists, the prior
// vector is removed first. Dim must match the index dim or
// ErrDimensionMismatch is returned.
//
// Metadata semantics (post-scrum K-B1): nil meta is "leave
// existing alone" (upsert-style); to clear metadata, pass an
// empty `{}` or Delete+Add. This avoids silent metadata loss
// when the JSON `metadata` field is omitted on re-add.
//
// Validates that all vector components are finite (post-scrum
// O-W3). NaN/Inf in any component poisons HNSW: distance
// comparisons return false for both `<` and `>`, breaking the
// search heap invariants. Zero-norm vectors are also rejected
// under cosine distance — cos(0,x) = NaN.
//
// Note: coder/hnsw's Graph.Add panics on re-adding an existing
// key (internal "node not added" length-invariant check). We
// pre-Delete to make Add idempotent on re-insert.
func (i *Index) Add(id string, vec []float32, meta json.RawMessage) error {
if id == "" {
return errors.New("vectord: empty id")
}
if len(vec) != i.params.Dimension {
return fmt.Errorf("%w: index dim=%d, got=%d", ErrDimensionMismatch, i.params.Dimension, len(vec))
}
if err := validateVector(vec, i.params.Distance); err != nil {
return err
}
i.mu.Lock()
defer i.mu.Unlock()
// coder/hnsw has two sharp edges on re-add:
// 1. Add of an existing key panics with "node not added"
// (length-invariant fires because internal delete+re-add
// doesn't change Len). Pre-Delete fixes this for n>1.
// 2. Delete of the LAST node leaves layers[0] non-empty but
// entryless; the next Add SIGSEGVs in Dims() because
// entry().Value is nil. We rebuild the graph in that case.
_, exists := i.g.Lookup(id)
if exists {
if i.g.Len() == 1 {
i.resetGraphLocked()
} else {
i.g.Delete(id)
}
}
i.g.Add(hnsw.MakeNode(id, vec))
if meta != nil {
// Per scrum K-B1 (Kimi): only OVERWRITE on explicit non-nil.
// nil = "leave existing meta alone" (upsert). To clear, the
// caller should send an empty `{}` body or Delete the id.
i.meta[id] = meta
}
return nil
}
// resetGraphLocked recreates the underlying coder/hnsw Graph with
// the same params. Caller MUST hold i.mu (write-lock). Used to
// dodge the library's "delete the last node, then segfault on
// next Add" bug — see Add for details. Metadata map is preserved
// because the only entry it could affect is the one being
// re-added, which Add overwrites.
func (i *Index) resetGraphLocked() {
g := hnsw.NewGraph[string]()
g.M = i.params.M
g.EfSearch = i.params.EfSearch
g.Distance = i.g.Distance
i.g = g
}
// validateVector rejects vectors that would poison the HNSW
// graph or produce NaN distances. Per scrum O-W3 (Opus).
func validateVector(vec []float32, distance string) error {
var sumSq float64
for j, v := range vec {
f := float64(v)
if math.IsNaN(f) || math.IsInf(f, 0) {
return fmt.Errorf("vectord: vec[%d] is non-finite (got %v)", j, v)
}
sumSq += f * f
}
if distance == DistanceCosine && sumSq == 0 {
return errors.New("vectord: zero-norm vector under cosine distance")
}
return nil
}
// Delete removes id from the index. Returns true if present.
func (i *Index) Delete(id string) bool {
i.mu.Lock()
defer i.mu.Unlock()
delete(i.meta, id)
return i.g.Delete(id)
}
// Search returns the k nearest neighbors of query, sorted
// ascending by distance.
//
// Note: coder/hnsw's Search returns `[]Node[K]` without distances —
// they're computed internally in the search candidate heap but
// dropped from the public API. We recompute distance from the
// returned vectors. O(k·dim) per search, negligible at typical
// k=10 / dim<2048.
func (i *Index) Search(query []float32, k int) ([]Result, error) {
if len(query) != i.params.Dimension {
return nil, fmt.Errorf("%w: index dim=%d, got=%d", ErrDimensionMismatch, i.params.Dimension, len(query))
}
if k <= 0 {
return nil, errors.New("vectord: k must be > 0")
}
i.mu.RLock()
defer i.mu.RUnlock()
// Per scrum O-I2 (Opus): use the stored distance function
// directly rather than re-resolving the string name on every
// search. The graph's Distance is set once at NewIndex.
dist := i.g.Distance
hits := i.g.Search(query, k)
out := make([]Result, len(hits))
for j, n := range hits {
out[j] = Result{
ID: n.Key,
Distance: dist(query, n.Value),
Metadata: i.meta[n.Key],
}
}
return out, nil
}
// Lookup returns the stored vector + metadata for an id.
//
// Per scrum O-W1 (Opus): the vector is COPIED before return.
// coder/hnsw's Lookup hands back the underlying graph slice;
// caller mutation would corrupt the index without locking.
func (i *Index) Lookup(id string) (vec []float32, meta json.RawMessage, ok bool) {
i.mu.RLock()
defer i.mu.RUnlock()
v, found := i.g.Lookup(id)
if !found {
return nil, nil, false
}
out := make([]float32, len(v))
copy(out, v)
return out, i.meta[id], true
}