a730fc2016
Cross-lineage scrum review on the 12 commits of this session
(afbb506..06e7152) via Rust gateway :3100 with Opus + Kimi +
Qwen3-coder. Results:
Real findings landed:
1. Opus BLOCK — vectord BatchAdd intra-batch duplicates panic
coder/hnsw's "node not added" length-invariant. Fixed with
last-write-wins dedup inside BatchAdd before the pre-pass.
Regression test TestBatchAdd_IntraBatchDedup added.
2. Opus + Kimi convergent WARN — strings.Contains(err.Error(),
"status 404") was brittle string-matching to detect cold-
start playbook state. Fixed: ErrCorpusNotFound sentinel
returned by searchCorpus on HTTP 404; fetchPlaybookHits
uses errors.Is.
3. Opus WARN — corpusingest.Run returned nil on total batch
failure, masking broken pipelines as "empty corpora." Fixed:
Stats.FailedBatches counter, ErrPartialFailure sentinel
returned when nonzero. New regression test
TestRun_NonzeroFailedBatchesReturnsError.
4. Opus WARN — dead var _ = io.EOF in staffing_500k/main.go
was justified by a fictional comment. Removed.
Drivers (staffing_500k, staffing_candidates, staffing_workers)
updated to handle ErrPartialFailure gracefully — print warn, keep
running queries — rather than fatal'ing on transient hiccups
while still surfacing the failure clearly in the output.
Documented (no code change):
- Opus WARN: matrixd /matrix/downgrade reads
LH_FORCE_FULL_ENRICHMENT from process env when body omits
it. Comment now explains the opinionated default and points
callers wanting deterministic behavior to pass the field
explicitly.
False positives dismissed (caught and verified, NOT acted on):
A. Kimi BLOCK on errors.Is + wrapped error in cmd/matrixd:223.
Verified false: Search wraps with %w (fmt.Errorf("%w: %v",
ErrEmbed, err)), so errors.Is matches the chain correctly.
B. Kimi INFO "BatchAdd has no unit tests." Verified false:
batch_bench_test.go has BenchmarkBatchAdd; the new dedup
test TestBatchAdd_IntraBatchDedup adds another.
C. Opus BLOCK on missing finite/zero-norm pre-validation in
cmd/vectord:280-291. Verified false: line 272 already calls
vectord.ValidateVector before BatchAdd, so finite + zero-
norm IS checked. Pre-validation is exhaustive.
D. Opus WARN on relevance.go tokenRe (Opus self-corrected
mid-finding when realizing leading char counts toward token
length).
Qwen3-coder returned NO FINDINGS — known issue with very long
diffs through the OpenRouter free tier; lineage rotation worked
as designed (Opus + Kimi between them caught everything Qwen
would have).
15-smoke regression sweep all green (D1-D6, G1, G1P, G2,
storaged_cap, pathway, matrix, relevance, downgrade, playbook).
Unit tests all green (corpusingest +1, vectord +1).
Per feedback_cross_lineage_review.md: convergent finding #2 (404
detection) is the highest-signal one — both Opus and Kimi
flagged it independently. The other Opus findings stand on
single-reviewer signal but each one verified against the actual
code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
105 lines
2.6 KiB
Go
105 lines
2.6 KiB
Go
package vectord
|
|
|
|
import (
|
|
"fmt"
|
|
"math/rand"
|
|
"testing"
|
|
)
|
|
|
|
// BenchmarkSingleAdd vs BenchmarkBatchAdd quantifies the lock-amortization
|
|
// win for the HTTP-batch shape. Same N items, same vectors; one path
|
|
// takes the lock N times, the other takes it once. Run with:
|
|
// go test ./internal/vectord/ -bench=. -benchmem -benchtime=1x
|
|
func BenchmarkSingleAdd(b *testing.B) {
|
|
for _, n := range []int{16, 128, 1024} {
|
|
b.Run(fmt.Sprintf("N=%d", n), func(b *testing.B) {
|
|
items := makeBatch(n, 768)
|
|
for i := 0; i < b.N; i++ {
|
|
idx := mustIndex(b)
|
|
for _, it := range items {
|
|
if err := idx.Add(it.ID, it.Vector, it.Metadata); err != nil {
|
|
b.Fatalf("Add: %v", err)
|
|
}
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func BenchmarkBatchAdd(b *testing.B) {
|
|
for _, n := range []int{16, 128, 1024} {
|
|
b.Run(fmt.Sprintf("N=%d", n), func(b *testing.B) {
|
|
items := makeBatch(n, 768)
|
|
for i := 0; i < b.N; i++ {
|
|
idx := mustIndex(b)
|
|
if err := idx.BatchAdd(items); err != nil {
|
|
b.Fatalf("BatchAdd: %v", err)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestBatchAdd_IntraBatchDedup guards the 2026-04-29 scrum BLOCK:
|
|
// without dedup, coder/hnsw's "node not added" length-invariant
|
|
// panics when the same ID appears twice in one batch. Last-write-
|
|
// wins semantics; the second vector for a duplicate ID replaces the
|
|
// first.
|
|
func TestBatchAdd_IntraBatchDedup(t *testing.T) {
|
|
idx := mustIndex(t)
|
|
items := []BatchItem{
|
|
{ID: "a", Vector: makeVec(768, 1)},
|
|
{ID: "b", Vector: makeVec(768, 2)},
|
|
{ID: "a", Vector: makeVec(768, 99)}, // duplicate — should win
|
|
}
|
|
if err := idx.BatchAdd(items); err != nil {
|
|
t.Fatalf("BatchAdd: %v", err)
|
|
}
|
|
if idx.Len() != 2 {
|
|
t.Errorf("Len: want 2, got %d", idx.Len())
|
|
}
|
|
// "a" should hold the LATER vector (the 99 one), not the first.
|
|
v, _, ok := idx.Lookup("a")
|
|
if !ok {
|
|
t.Fatal("a not found")
|
|
}
|
|
if v[0] != 99 {
|
|
t.Errorf("last-write-wins: want vec[0]=99, got %v", v[0])
|
|
}
|
|
}
|
|
|
|
func makeVec(dim int, val float32) []float32 {
|
|
v := make([]float32, dim)
|
|
v[0] = val
|
|
v[1] = 1 // non-zero-norm under cosine
|
|
return v
|
|
}
|
|
|
|
func mustIndex(tb testing.TB) *Index {
|
|
tb.Helper()
|
|
idx, err := NewIndex(IndexParams{
|
|
Name: "bench",
|
|
Dimension: 768,
|
|
M: DefaultM,
|
|
EfSearch: DefaultEfSearch,
|
|
Distance: DistanceCosine,
|
|
})
|
|
if err != nil {
|
|
tb.Fatalf("NewIndex: %v", err)
|
|
}
|
|
return idx
|
|
}
|
|
|
|
func makeBatch(n, dim int) []BatchItem {
|
|
rng := rand.New(rand.NewSource(int64(n)))
|
|
out := make([]BatchItem, n)
|
|
for i := range out {
|
|
v := make([]float32, dim)
|
|
for j := range v {
|
|
v[j] = rng.Float32()*2 - 1
|
|
}
|
|
out[i] = BatchItem{ID: fmt.Sprintf("k-%06d", i), Vector: v}
|
|
}
|
|
return out
|
|
}
|