09299a27b7
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>
62 lines
3.5 KiB
Markdown
62 lines
3.5 KiB
Markdown
# 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.
|