profit/golangLAKEHOUSE
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

scrum fixes: 4 real findings landed, 4 false positives dismissed

Browse Source 
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>
This commit is contained in:
root
2026-04-29 19:42:39 -05:00
co-authored by Claude Opus 4.7
parent 06e71520c4
commit a730fc2016
9 changed files with 212 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/corpusingest/ingest.go
+33 -11
View File
@@ -62,14 +62,27 @@ type Config struct {
LogProgress time.Duration
}
// Stats reports run outcomes.
// Stats reports run outcomes. FailedBatches counts embed-or-add
// batches that errored out and were skipped (partial-failure
// semantics). When non-zero, Run returns ErrPartialFailure so
// callers can't accidentally treat "1 of 313 batches succeeded"
// as a successful run.
type Stats struct {
Scanned int64
Embedded int64
Added int64
Wall time.Duration
Scanned int64
Embedded int64
Added int64
Wall time.Duration
FailedBatches int64
}
// ErrPartialFailure signals that one or more batches errored during
// Run. Stats.FailedBatches has the count; the caller decides
// whether to retry / log / abort. Per 2026-04-29 cross-lineage
// scrum (Opus WARN): the original behavior returned nil even when
// 100% of batches failed silently, making "embedded=0/scanned=N"
// look like an empty corpus rather than a broken pipeline.
var ErrPartialFailure = errors.New("corpusingest: one or more batches failed")
// Run executes the ingest pipeline. Returns on source EOF after all
// in-flight jobs drain, on context cancellation, or on the first
// embed/add error (errors are logged via slog and the pipeline
@@ -90,6 +103,7 @@ func Run(ctx context.Context, cfg Config, src Source) (Stats, error) {
var (
totalEmbedded int64
totalAdded int64
failedBatches int64
)
var wg sync.WaitGroup
@@ -102,10 +116,11 @@ func Run(ctx context.Context, cfg Config, src Source) (Stats, error) {
if err != nil {
// Partial-failure semantics: log + continue. A wedged
// embed batch shouldn't kill 8 workers' worth of
// progress; the operator decides whether to re-run
// based on the final Embedded vs Scanned delta.
// progress; Run returns ErrPartialFailure on any
// failure so callers can't miss the signal.
slog.Warn("corpusingest: embed batch failed",
"index", cfg.IndexName, "items", len(j.texts), "err", err)
atomic.AddInt64(&failedBatches, 1)
continue
}
// Defense against a degraded embed backend that returns
@@ -114,12 +129,14 @@ func Run(ctx context.Context, cfg Config, src Source) (Stats, error) {
if len(vecs) != len(j.ids) {
slog.Warn("corpusingest: embed returned wrong count",
"index", cfg.IndexName, "want", len(j.ids), "got", len(vecs))
atomic.AddInt64(&failedBatches, 1)
continue
}
atomic.AddInt64(&totalEmbedded, int64(len(vecs)))
if err := addBatch(ctx, cfg, j.ids, vecs, j.metas); err != nil {
slog.Warn("corpusingest: add batch failed",
"index", cfg.IndexName, "items", len(j.ids), "err", err)
atomic.AddInt64(&failedBatches, 1)
continue
}
atomic.AddInt64(&totalAdded, int64(len(j.ids)))
@@ -159,14 +176,19 @@ func Run(ctx context.Context, cfg Config, src Source) (Stats, error) {
<-progressDone
stats := Stats{
Scanned: scanned,
Embedded: atomic.LoadInt64(&totalEmbedded),
Added: atomic.LoadInt64(&totalAdded),
Wall: time.Since(t0),
Scanned: scanned,
Embedded: atomic.LoadInt64(&totalEmbedded),
Added: atomic.LoadInt64(&totalAdded),
Wall: time.Since(t0),
FailedBatches: atomic.LoadInt64(&failedBatches),
}
if err != nil {
return stats, err
}
if stats.FailedBatches > 0 {
return stats, fmt.Errorf("%w: %d batches failed (embedded=%d added=%d scanned=%d)",
ErrPartialFailure, stats.FailedBatches, stats.Embedded, stats.Added, stats.Scanned)
}
return stats, nil
}
internal/corpusingest/ingest_test.go
+42
View File
@@ -3,6 +3,7 @@ package corpusingest
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -341,6 +342,47 @@ func TestRun_ProgressLoggerExits(t *testing.T) {
}
}
// TestRun_NonzeroFailedBatchesReturnsError guards the 2026-04-29
// scrum WARN: original behavior returned nil even when 100% of
// batches failed, making "embedded=0/scanned=N" look like an empty
// corpus rather than a broken pipeline.
func TestRun_NonzeroFailedBatchesReturnsError(t *testing.T) {
// Fake gateway that fails every embed call.
mux := http.NewServeMux()
mux.HandleFunc("/v1/vectors/index", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusCreated)
})
mux.HandleFunc("/v1/embed", func(w http.ResponseWriter, r *http.Request) {
http.Error(w, "embed failure injected", http.StatusBadGateway)
})
mux.HandleFunc("/v1/vectors/index/", func(w http.ResponseWriter, r *http.Request) {
// shouldn't reach here since embed fails first
http.Error(w, "should not be called", http.StatusInternalServerError)
})
srv := httptest.NewServer(mux)
defer srv.Close()
rows := make([]Row, 5)
for i := range rows {
rows[i] = Row{ID: fmt.Sprintf("r-%d", i), Text: "x"}
}
stats, err := Run(context.Background(), Config{
GatewayURL: srv.URL, IndexName: "fail_only", Dimension: 4,
EmbedBatch: 1, EmbedWorkers: 1, HTTPClient: srv.Client(),
}, &staticSource{rows: rows})
if !errors.Is(err, ErrPartialFailure) {
t.Errorf("want ErrPartialFailure, got %v", err)
}
if stats.FailedBatches == 0 {
t.Error("FailedBatches should be > 0 when embeds fail")
}
if stats.Added != 0 {
t.Errorf("Added: want 0 (all failed), got %d", stats.Added)
}
}
func TestRun_RequiresIndexName(t *testing.T) {
_, err := Run(context.Background(), Config{Dimension: 4},
&staticSource{rows: nil})