feat: high-performance HNSW and durable async indexing - #6
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds concurrent off-heap HNSW storage and search, SIMD dispatch and generated kernels, FSQ quantization, durable WAL/recovery behavior, asynchronous indexing, mutation and iteration APIs, expanded benchmarks and tests, CI validation, and extensive configuration and research documentation. ChangesHNSW and memory management
Durability and indexing
SIMD and quantization
Documentation and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/storage/singlefile/engine.go (1)
3548-3567: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Closeaborts before syncing/closing the file when arena or pool teardown fails.If
walWriteArena.Free()orwalRequests.close()returns an error,Closereturns immediately, soe.file.Sync()(for theWithWALSync(false)dirty case),e.file.Close(), ande.closed.Store(true)never run. That leaks the file descriptor and leaves the engine neither closed nor cleaned up; a retriedClosewould also re-invokeFree()on the same arena sincee.walWriteArenawas never niled. Accumulate the errors and always run sync/close.🛠️ Proposed fix
- if e.walWriteArena != nil { - if err := e.walWriteArena.Free(); err != nil { - return err - } - e.walWriteArena = nil - } - if e.walRequests != nil { - if err := e.walRequests.close(); err != nil { - return err - } - e.walRequests = nil - } - if e.dirty { - if err := e.file.Sync(); err != nil { - return err - } - } - e.closed.Store(true) - return e.file.Close() + var closeErr error + if e.walWriteArena != nil { + closeErr = errors.Join(closeErr, e.walWriteArena.Free()) + e.walWriteArena = nil + } + if e.walRequests != nil { + closeErr = errors.Join(closeErr, e.walRequests.close()) + e.walRequests = nil + } + if e.dirty { + closeErr = errors.Join(closeErr, e.file.Sync()) + } + e.closed.Store(true) + return errors.Join(closeErr, e.file.Close())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/singlefile/engine.go` around lines 3548 - 3567, Update Close to accumulate errors from walWriteArena.Free and walRequests.close instead of returning immediately, and nil each resource after its teardown attempt so retries do not repeat cleanup. Always continue through the dirty-file Sync, closed.Store(true), and file.Close steps, returning the combined cleanup error afterward.libravdb/options.go (1)
128-138: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject
m == 1before derivingML.
math.Log(1)is zero, so the current validation permits an infinite level multiplier. Requirem > 1.Proposed fix
- if m <= 0 || efConstruction <= 0 || efSearch <= 0 { - return fmt.Errorf("HNSW parameters must be positive") + if m <= 1 || efConstruction <= 0 || efSearch <= 0 { + return fmt.Errorf("HNSW m must be greater than 1 and ef parameters must be positive")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libravdb/options.go` around lines 128 - 138, Update the validation in WithHNSW so m must be greater than 1 before calculating ML, while retaining the positive-value requirements for efConstruction and efSearch.internal/storage/singlefile/codec.go (1)
339-367: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate the complete optional payload before decoding or skipping it.
A corrupt
optSizecan movedec.Offbeyonddec.Data, while truncated known fields are silently accepted. After reading the prefix, verifyoptSize <= len(dec.Data)-dec.Off, decode within that bounded section, then set the offset to its validated end.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/singlefile/codec.go` around lines 339 - 367, Update the version >= 2 optional-field parsing after ReadUint32 to validate that optSize fits within the remaining decoder data before reading any fields or skipping unknown bytes. Decode known fields only within this bounded optional section, reject truncated payloads instead of silently accepting them, then advance dec.Off to the validated section end rather than adding optSize blindly.
🟡 Minor comments (15)
docs/research/flash-hnsw-compact-codes.md-57-81 (1)
57-81: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the perpendicular-bisector definitions.
The table and Line 79 define
e/busing the wrong vectors and signs. For the stated comparison, usee = w - v,b = (||w||² - ||v||²)/2, andδ(u,v)² - δ(u,w)² = 2(e·u - b)consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/flash-hnsw-compact-codes.md` around lines 57 - 81, Correct the perpendicular-bisector definitions in the notation table and Lemma 1 proof: use e = w - v and b = (||w||² - ||v||²)/2 for comparing δ(u,v) with δ(u,w), and update the squared-distance identity to δ(u,v)² - δ(u,w)² = 2(e·u - b). Ensure the stated less-than, greater-than, and equality conditions consistently use this corrected sign convention.docs/research/hnsw-plus-plus-lid.md-35-41 (1)
35-41: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the sign in the LID estimator.
With
log(d_k/d_i), the estimator must be positive:-LID(x) = -1 / ( (1 / (k-1)) * sum_{i=1}^{k-1} log(d_k / d_i) ) +LID(x) = 1 / ( (1 / (k-1)) * sum_{i=1}^{k-1} log(d_k / d_i) )Alternatively, retain the minus sign and use
log(d_i/d_k).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/hnsw-plus-plus-lid.md` around lines 35 - 41, Correct the LID estimator equation in the documentation so its result is positive: either remove the leading minus sign while retaining log(d_k / d_i), or keep the minus sign and reverse the logarithm ratio to log(d_i / d_k). Update the displayed LID formula only and preserve the existing k−1 summation.go.mod-25-25 (1)
25-25: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winBump
google.golang.org/protobufoff 1.31.0 (known HIGH advisories).OSV flags
v1.31.0for GO-2024-2611 / GHSA-8r3f-844c-mc37 (infinite loop inprotojson.Unmarshalon invalid JSON). Since this go.mod is already being modified, bump the transitive pin (e.g. to>= v1.33.0) viago getand re-tidy.#!/bin/bash gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: GO, package: "google.golang.org/protobuf") { nodes { advisory { summary severity } vulnerableVersionRange firstPatchedVersion { identifier } } } }'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go.mod` at line 25, Update the google.golang.org/protobuf dependency in go.mod from v1.31.0 to v1.33.0 or newer, then run go get and go mod tidy so the transitive dependency metadata is consistent.Source: Linters/SAST tools
docs/research/cagra-gpu.md-30-30 (1)
30-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTypo:
CACG→CAGRA.
the locality CACG needsshould readCAGRA.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/cagra-gpu.md` at line 30, In the Stage 1 description, replace the incorrect acronym “CACG” with “CAGRA” in the sentence describing the locality requirement.internal/index/hnsw/insert.go-329-331 (1)
329-331: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate the node and level before indexing
Links.A missing node, negative level, or level above the node’s maximum currently panics before
appendWithSpinlockcan reject it.Proposed fix
func (h *Index) manualConnect(nodeID uint32, linkID uint32, level int) bool { node := h.nodes.Get(nodeID) + if node == nil || level < 0 || level > node.Level || level >= MaxLevel { + return false + } return h.appendWithSpinlock(node, node.Links[level], linkID, h.config.M, level) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/index/hnsw/insert.go` around lines 329 - 331, Update Index.manualConnect to validate that the retrieved node is non-nil and level is within the node’s Links bounds before accessing node.Links[level]. Return false for invalid node or level; otherwise preserve the existing appendWithSpinlock call.internal/storage/singlefile/wal_request.go-13-20 (1)
13-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
iotais offset by the capacity constants — state enum starts at 2, not 0.Because the state constants share this
constblock withwalRequestCapacity/walRequestWords,iotais already 2 atwalRequestFree, sowalRequestFree=2,walRequestPending=3,walRequestComplete=4. The values stay distinct, so the current state machine still works, but a zero-valuewalRequestRecordhasstate==0, which is none of these — a fragile mismatch for any future zero-value "is free" check. Split the enum into its own block so it starts at 0.♻️ Proposed fix
const ( walRequestCapacity = 4096 walRequestWords = walRequestCapacity / 64 +) +const ( walRequestFree uint32 = iota walRequestPending walRequestComplete )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/singlefile/wal_request.go` around lines 13 - 20, Split the walRequestFree, walRequestPending, and walRequestComplete constants into a separate const block so iota starts at zero and preserves the zero-value walRequestRecord as the free state; leave walRequestCapacity and walRequestWords unchanged.docs/research/freshdiskann.md-21-21 (1)
21-21: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEscape the asterisks in
p*.Markdown interprets these occurrences as emphasis delimiters, breaking the rendered algorithms. Use
p\*, inline code, or mathematical notation.Also applies to: 29-29, 33-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/freshdiskann.md` at line 21, Escape every literal asterisk in the node notation across the affected documentation lines, including each occurrence of p* in the navigability and algorithm descriptions, using p\* or an equivalent Markdown-safe notation while preserving the mathematical meaning.Source: Linters/SAST tools
docs/research/freshdiskann.md-48-48 (1)
48-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the random-read estimate.
~100 ≈ 75·l_searchcannot hold with the documented defaultl_search = 75; the product is 5,625. Replace this with the paper’s actual estimate or relationship.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/freshdiskann.md` at line 48, Correct the random-read estimate in the FreshDiskANN documentation, replacing the inconsistent “~100 ≈ 75·l_search” relationship with the paper’s actual estimate or relationship. Keep the surrounding I/O-cost and α-RNG explanation unchanged.docs/research/freshdiskann.md-17-19 (1)
17-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the recall definition.
Galready denotes the graph, whileX = |X ∩ L| / kis self-referential. Use a distinct ground-truth set, for examplerecall@k = |T ∩ L| / k.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/freshdiskann.md` around lines 17 - 19, Correct Definition 1.1 by replacing the self-referential recall notation with a distinct ground-truth k-NN set symbol, such as T, and define recall@k as |T ∩ L| / k. Preserve the existing query output L and target quality statement, and avoid reusing G for the ground-truth set because G already denotes the graph.docs/research/diskann-vamana.md-11-11 (1)
11-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDrop the machine-local absolute path. The committed
Local copy:line leaks a developer username and workstation path. Prefer a repo-relative reference.🧹 Suggested change
-- **Local copy:** `/Users/z3robit/Development/golang/src/github.com/xDarkicex/libraVDB/docs/research/diskann-vamana.pdf` (10 pages, no appendix) +- **Local copy:** `docs/research/diskann-vamana.pdf` (10 pages, no appendix)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/diskann-vamana.md` at line 11, Replace the machine-local absolute path in the “Local copy” entry of the DiskANN-Vamana research document with a repository-relative reference, preserving the document and file description without exposing developer-specific usernames or workstation locations.internal/index/hnsw/node.go-27-33 (1)
27-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid the heap-backed fallback in
internal/index/hnsw/vector_store_mmap.go. The normal raw-vector stores keep vectors off-heap, but ifcopyPoolis nil,Getfalls back tomake([]float32, s.dim), which can leave an off-heapNodepointing at a Go-heap array. Fail closed there or allocate from the same pool.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/index/hnsw/node.go` around lines 27 - 33, Update the vector retrieval path in vector_store_mmap.go, specifically the Get logic using copyPool, so a nil copyPool never creates a make([]float32, s.dim) fallback that is passed to Node.setVector. Fail closed when the pool is unavailable, or obtain the vector from the established off-heap pool before calling setVector, preserving the raw-vector off-heap invariant.docs/research/original-hnsw-malkov-yashunin.md-262-267 (1)
262-267: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEscape
|C|so the table renders correctly.Use
\|C\|in the heuristic-cost expression; the current pipes are parsed as column separators.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/original-hnsw-malkov-yashunin.md` around lines 262 - 267, Update the “Single insert” row in the operation-complexity table so the heuristic-cost expression escapes both pipe characters around C as \|C\|, preserving the intended table rendering and all other text unchanged.Source: Linters/SAST tools
docs/research/original-hnsw-malkov-yashunin.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the workstation-specific PDF path.
This absolute path exposes a local username and cannot work for other contributors. Use a repository-relative path if the PDF is tracked; otherwise omit the field.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/original-hnsw-malkov-yashunin.md` at line 7, Remove the workstation-specific absolute path from the “Local PDF” entry in the document; if the PDF is tracked, replace it with its repository-relative path, otherwise remove the field entirely.docs/research/original-hnsw-malkov-yashunin.md-51-75 (1)
51-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the level-distribution and complexity derivation.
For
l=floor(-ln(U)·mL), the distribution is:
P(l=i)=(1-exp(-1/mL))·exp(-i/mL).With
mL=1/ln(M), this becomes(1-1/M)·M^-i. The expected layers per node are constant; it is the graph’s maximum height that scales asO(log_M N)and yields logarithmic traversal depth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/original-hnsw-malkov-yashunin.md` around lines 51 - 75, Correct the level-distribution derivation around the displayed formula and equation (1): use P(l=i)=(1−exp(−1/mL))·exp(−i/mL), which becomes (1−1/M)·M^-i when mL=1/ln(M). Update the surrounding explanation to state that expected layers per node are constant, while the graph’s maximum height scales as O(log_M N) and determines logarithmic traversal depth; remove claims that per-layer connectivity or E[l+1] directly establishes this complexity.docs/research/original-hnsw-malkov-yashunin.md-88-90 (1)
88-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the HNSW memory-complexity formula.
Per-node adjacency is approximately
M_max0 + M_max·E[level], notM_max0·M_max. WithM_max0=2M, graph-link storage isO(N·M), excluding vectors and allocator overhead. Update both the prose and summary table.Also applies to: 262-267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/research/original-hnsw-malkov-yashunin.md` around lines 88 - 90, Correct the HNSW memory-cost description in section 2.6 and its corresponding summary-table entry: express per-node adjacency as approximately M_max0 + M_max·E[level], and state graph-link storage as O(N·M) when M_max0=2M, excluding vectors and allocator overhead. Remove the incorrect M_max0·M_max formulation while preserving the 60–450 bytes context only if it remains consistent with the corrected model.
🧹 Nitpick comments (4)
internal/quant/fsq.go (1)
317-324: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
denormalizeLockedor use it in decode. It has no call sites ininternal/quant/fsq.go, so it only adds maintenance overhead unless it’s meant to be the reconstruction inverse.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/quant/fsq.go` around lines 317 - 324, Remove the unused FSQQuantizer.denormalizeLocked method, or integrate it into the decode path as the reconstruction inverse if that is its intended purpose. Ensure no dead helper remains without call sites in fsq.go.libravdb/mutation_state.go (1)
103-109: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftPark contended waiters instead of spinning indefinitely.
Both acquisition paths retry with only
runtime.Gosched(). With many writers or slot collisions, all waiters remain runnable and continuously contend on the same cache lines. Use bounded spinning followed by a parking primitive or backoff, ideally with cancellation support.Also applies to: 119-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libravdb/mutation_state.go` around lines 103 - 109, Update lockMutationID and the corresponding acquisition path to avoid retrying indefinitely with only runtime.Gosched(). Add bounded spinning, then park or back off contended waiters, using cancellation support if the existing mutation-state API permits it; preserve successful token acquisition and release behavior.internal/index/hnsw/hnsw.go (1)
1181-1202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor cleanup: duplicate nil guard and shadowed loop variable.
Lines 1183-1188 nil-check
nodetwice, and the inner links loop at Line 1195 reusesi, shadowing the outer index. Same duplicate-nil pattern recurs inestimateFileSize(Lines 1835-1857) andSnapshotVectorsFromProvider(Lines 1502-1505). Drop the second check and rename the inner index for clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/index/hnsw/hnsw.go` around lines 1181 - 1202, Remove the duplicate node nil guard in the shown usage loop, and rename the inner links-loop index so it does not shadow the outer node index. Apply the same duplicate nil-check cleanup in estimateFileSize and SnapshotVectorsFromProvider, preserving their existing traversal behavior.internal/storage/singlefile/codec.go (1)
212-220: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winInclude
IDMapCapacityin the encoded-size estimate.The version-2 payload gained four bytes, but
estimateCollectionConfigSizestill adds only the length prefix. Increase its version-2 adjustment from 4 to 8 bytes to avoid under-allocation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/storage/singlefile/codec.go` around lines 212 - 220, Update estimateCollectionConfigSize so its version-2 size adjustment accounts for both the IDMapCapacity field and its length prefix, increasing the adjustment from 4 to 8 bytes while preserving the existing size calculation for earlier versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 28: Remove the local replace directive for github.com/xDarkicex/memory
from go.mod, preserving the published v1.2.1 dependency so CI and external
consumers resolve it normally. If local development still requires the sibling
checkout, move that override to an uncommitted workspace configuration.
In `@internal/index/hnsw/delete.go`:
- Around line 442-470: Update handleEntryPointReplacement to compare deletedID
with the current entry point’s ordinal and return immediately when they differ.
Only perform the existing fallback scan and replacement logic when the deleted
node is the current entry point, preserving the current behavior for replacement
failures and empty indexes.
In `@internal/index/hnsw/hnsw.go`:
- Around line 588-599: Update insertSingleMetadata so the early nil return
occurs only when updateEntryPointCAS succeeds while the index remains in the
empty state; if a concurrent higher-level insert has already established an
entry point, continue through the normal insertNode path instead of treating the
CAS success as sufficient.
- Around line 461-468: Update the insert rollback around insertNode failure and
the ID-conflict path to release the nodeSFL slot, all upper-level link/backlink
slab allocations, and the raw-vector slot before clearing registry metadata and
size. Replace the racy getEntryPoint()==nil fast path with synchronized
first-node initialization so concurrent inserts cannot leave a registered node
unconnected, including when a higher-level node wins updateEntryPointCAS.
In `@internal/index/hnsw/neighbors.go`:
- Around line 462-464: In the pruning path around maxCapacity and unsafe.Slice,
reacquire the prune lock and revalidate the node and node.Links[level] before
slicing, mirroring the under-lock checks in connectLinkWithHeuristic. If the
registry entry or level link storage is no longer valid, exit without
constructing the slice; otherwise proceed with the existing capacity calculation
and pruning logic.
In `@internal/index/hnsw/repair.go`:
- Around line 103-118: Update the repair processing loop around scanDirtyRepairs
to clear repairOverflow before scanning, not after a zero-scan result. When the
overflow flag is raised concurrently during or after the scan, preserve it and
rescan rather than returning; only return processed when no repairs were scanned
and no new overflow notification is set.
In `@internal/index/hnsw/search.go`:
- Line 1032: Update newSOACandidateQueue to handle nil soaIDs or soaDistances
without slicing them, returning a valid empty/non-SOA queue state that preserves
search behavior. Keep the existing capacity-backed initialization when both SoA
buffers are present, so heap, unsorted, and reservoir modes no longer panic when
searchLevelScratchValues constructs the queue.
In `@internal/index/hnsw/segmented_array.go`:
- Around line 22-33: Update segmentedNodeArray and its node insertion/removal
paths to retain every live *Node in a Go-heap-owned structure, rather than
relying only on off-heap directory chunks and idToIndex. Ensure the ownership
structure is updated consistently when nodes become live or are removed, while
preserving existing append-only lookup behavior.
In `@internal/index/hnsw/vector_store.go`:
- Around line 139-147: Reclaim backing allocations when vectors are deleted so
insert/delete churn does not exhaust the pool: update
InMemoryRawVectorStore.Delete in internal/index/hnsw/vector_store.go (139-147)
to defer reclamation until no readers can access the slot, then return the
allocation to the reusable pool. Apply the equivalent deferred-reclamation
change to the deleted slab slots in internal/index/hnsw/vector_store_slabby.go
(123-132), ensuring slots are not recycled before outstanding readers finish.
In `@internal/util/distance.go`:
- Around line 22-68: Add a length-mismatch guard in GetDistanceFunc’s SIMD
dispatch paths so every returned AVX2/NEON distance function validates len(a)
equals len(b) before invoking the kernel. Preserve the existing scalar helper
behavior and distance calculations, and ensure mismatched dimensions produce the
same panic behavior rather than allowing SIMD kernels to read past b.
In `@internal/util/simd/stub_arm64.go`:
- Around line 7-19: Add //go:noescape directives to every SIMD declaration in
stub_arm64.go, including DotProductNEON, L2DistanceNEON, L2Distance4NEON,
L2Distance4PtrNEON, L2Distance8PtrNEON, L2Distance8AlignedPtrNEON, and
L2AnyLessThan8AlignedPtrNEON, matching the declarations in stub_amd64.go.
In `@libravdb/async_index.go`:
- Around line 54-58: Extend the asyncIndexStorage interface to declare the
GetIDByOrdinal method required by the usage around lines 250–255, matching the
implementing storage type’s existing signature. Leave the other interface
methods unchanged.
In `@libravdb/collection.go`:
- Around line 846-857: Add both durable-range methods to the asyncIndexStorage
interface/type used by c.asyncIndex.storage: InsertDurableRange(context.Context,
*index.VectorEntry) (storage.DurableRange, error) for the path at
libravdb/collection.go:846-857, and InsertBatchDurableRange(context.Context,
[]*index.VectorEntry) (storage.DurableRange, error) for the path at
libravdb/collection.go:1006-1022. Ensure the concrete storage implementation
satisfies these methods, or retype storage to the existing implementation that
provides them.
- Around line 1155-1160: Reorder locking in Collection.Update and the
corresponding Upsert and Delete paths so the write permit from acquireWrite is
obtained before calling lockAsyncMutation. Preserve existing error handling and
deferred unlock behavior, and align async Insert’s lock order to prevent waits
between the write controller and asyncMutation barrier.
In `@libravdb/database.go`:
- Around line 219-222: Update the async-index failure path in the
collection-creation method around configureAsyncIndex to preserve and join
errors from both collection.Close and db.storage.DeleteCollection instead of
discarding them. Return the combined setup and cleanup errors so callers can
detect when rollback was incomplete; apply the same handling to the
corresponding failure path noted later in the method.
In `@libravdb/index_persistence.go`:
- Around line 84-89: Update the index persistence flow around
preciseAppliedLocked and idx.SerializeToBytes so it does not serialize idx when
the current applied LSN exceeds checkpointLSN. Fall back to the existing
record-based rebuild path in that case, preserving the checkpoint frontier and
avoiding an image containing mutations newer than the checkpoint.
In `@libravdb/migrate.go`:
- Around line 119-134: Update replaceFileDurably to report whether the rename
occurred separately from durability-sync errors, then use that state in the
migration rollback paths around replaceFileDurably calls to restore the correct
file arrangement when sync fails after a successful rename. Update
recoverMigrate to propagate activation-recovery errors instead of suppressing
them, ensuring startup cannot continue with an unresolved swap.
---
Outside diff comments:
In `@internal/storage/singlefile/codec.go`:
- Around line 339-367: Update the version >= 2 optional-field parsing after
ReadUint32 to validate that optSize fits within the remaining decoder data
before reading any fields or skipping unknown bytes. Decode known fields only
within this bounded optional section, reject truncated payloads instead of
silently accepting them, then advance dec.Off to the validated section end
rather than adding optSize blindly.
In `@internal/storage/singlefile/engine.go`:
- Around line 3548-3567: Update Close to accumulate errors from
walWriteArena.Free and walRequests.close instead of returning immediately, and
nil each resource after its teardown attempt so retries do not repeat cleanup.
Always continue through the dirty-file Sync, closed.Store(true), and file.Close
steps, returning the combined cleanup error afterward.
In `@libravdb/options.go`:
- Around line 128-138: Update the validation in WithHNSW so m must be greater
than 1 before calculating ML, while retaining the positive-value requirements
for efConstruction and efSearch.
---
Minor comments:
In `@docs/research/cagra-gpu.md`:
- Line 30: In the Stage 1 description, replace the incorrect acronym “CACG” with
“CAGRA” in the sentence describing the locality requirement.
In `@docs/research/diskann-vamana.md`:
- Line 11: Replace the machine-local absolute path in the “Local copy” entry of
the DiskANN-Vamana research document with a repository-relative reference,
preserving the document and file description without exposing developer-specific
usernames or workstation locations.
In `@docs/research/flash-hnsw-compact-codes.md`:
- Around line 57-81: Correct the perpendicular-bisector definitions in the
notation table and Lemma 1 proof: use e = w - v and b = (||w||² - ||v||²)/2 for
comparing δ(u,v) with δ(u,w), and update the squared-distance identity to
δ(u,v)² - δ(u,w)² = 2(e·u - b). Ensure the stated less-than, greater-than, and
equality conditions consistently use this corrected sign convention.
In `@docs/research/freshdiskann.md`:
- Line 21: Escape every literal asterisk in the node notation across the
affected documentation lines, including each occurrence of p* in the
navigability and algorithm descriptions, using p\* or an equivalent
Markdown-safe notation while preserving the mathematical meaning.
- Line 48: Correct the random-read estimate in the FreshDiskANN documentation,
replacing the inconsistent “~100 ≈ 75·l_search” relationship with the paper’s
actual estimate or relationship. Keep the surrounding I/O-cost and α-RNG
explanation unchanged.
- Around line 17-19: Correct Definition 1.1 by replacing the self-referential
recall notation with a distinct ground-truth k-NN set symbol, such as T, and
define recall@k as |T ∩ L| / k. Preserve the existing query output L and target
quality statement, and avoid reusing G for the ground-truth set because G
already denotes the graph.
In `@docs/research/hnsw-plus-plus-lid.md`:
- Around line 35-41: Correct the LID estimator equation in the documentation so
its result is positive: either remove the leading minus sign while retaining
log(d_k / d_i), or keep the minus sign and reverse the logarithm ratio to
log(d_i / d_k). Update the displayed LID formula only and preserve the existing
k−1 summation.
In `@docs/research/original-hnsw-malkov-yashunin.md`:
- Around line 262-267: Update the “Single insert” row in the
operation-complexity table so the heuristic-cost expression escapes both pipe
characters around C as \|C\|, preserving the intended table rendering and all
other text unchanged.
- Line 7: Remove the workstation-specific absolute path from the “Local PDF”
entry in the document; if the PDF is tracked, replace it with its
repository-relative path, otherwise remove the field entirely.
- Around line 51-75: Correct the level-distribution derivation around the
displayed formula and equation (1): use P(l=i)=(1−exp(−1/mL))·exp(−i/mL), which
becomes (1−1/M)·M^-i when mL=1/ln(M). Update the surrounding explanation to
state that expected layers per node are constant, while the graph’s maximum
height scales as O(log_M N) and determines logarithmic traversal depth; remove
claims that per-layer connectivity or E[l+1] directly establishes this
complexity.
- Around line 88-90: Correct the HNSW memory-cost description in section 2.6 and
its corresponding summary-table entry: express per-node adjacency as
approximately M_max0 + M_max·E[level], and state graph-link storage as O(N·M)
when M_max0=2M, excluding vectors and allocator overhead. Remove the incorrect
M_max0·M_max formulation while preserving the 60–450 bytes context only if it
remains consistent with the corrected model.
In `@go.mod`:
- Line 25: Update the google.golang.org/protobuf dependency in go.mod from
v1.31.0 to v1.33.0 or newer, then run go get and go mod tidy so the transitive
dependency metadata is consistent.
In `@internal/index/hnsw/insert.go`:
- Around line 329-331: Update Index.manualConnect to validate that the retrieved
node is non-nil and level is within the node’s Links bounds before accessing
node.Links[level]. Return false for invalid node or level; otherwise preserve
the existing appendWithSpinlock call.
In `@internal/index/hnsw/node.go`:
- Around line 27-33: Update the vector retrieval path in vector_store_mmap.go,
specifically the Get logic using copyPool, so a nil copyPool never creates a
make([]float32, s.dim) fallback that is passed to Node.setVector. Fail closed
when the pool is unavailable, or obtain the vector from the established off-heap
pool before calling setVector, preserving the raw-vector off-heap invariant.
In `@internal/storage/singlefile/wal_request.go`:
- Around line 13-20: Split the walRequestFree, walRequestPending, and
walRequestComplete constants into a separate const block so iota starts at zero
and preserves the zero-value walRequestRecord as the free state; leave
walRequestCapacity and walRequestWords unchanged.
---
Nitpick comments:
In `@internal/index/hnsw/hnsw.go`:
- Around line 1181-1202: Remove the duplicate node nil guard in the shown usage
loop, and rename the inner links-loop index so it does not shadow the outer node
index. Apply the same duplicate nil-check cleanup in estimateFileSize and
SnapshotVectorsFromProvider, preserving their existing traversal behavior.
In `@internal/quant/fsq.go`:
- Around line 317-324: Remove the unused FSQQuantizer.denormalizeLocked method,
or integrate it into the decode path as the reconstruction inverse if that is
its intended purpose. Ensure no dead helper remains without call sites in
fsq.go.
In `@internal/storage/singlefile/codec.go`:
- Around line 212-220: Update estimateCollectionConfigSize so its version-2 size
adjustment accounts for both the IDMapCapacity field and its length prefix,
increasing the adjustment from 4 to 8 bytes while preserving the existing size
calculation for earlier versions.
In `@libravdb/mutation_state.go`:
- Around line 103-109: Update lockMutationID and the corresponding acquisition
path to avoid retrying indefinitely with only runtime.Gosched(). Add bounded
spinning, then park or back off contended waiters, using cancellation support if
the existing mutation-state API permits it; preserve successful token
acquisition and release behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d4e7e37c-6f2d-4c38-92bc-7cddcd22abed
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (109)
.github/workflows/ci.yml.github/workflows/graph-tests.ymlREADME.mdbenchmark/hyaline_smoke_bench_test.gobenchmark/write_contention_benchmark_test.godocs/configuration/configuration.mddocs/configuration/quantization-modes.mddocs/research/async-wal-indexing-plan.mddocs/research/cagra-gpu.mddocs/research/cross-platform-simd-parity.mddocs/research/diskann-scatter-beam-libravdb-experiment.mddocs/research/diskann-soa-candidate-queue-experiment.mddocs/research/diskann-vamana.mddocs/research/exact-early-termination-and-reuse-experiments.mddocs/research/flash-hnsw-compact-codes.mddocs/research/flat-hnsw-hubs.mddocs/research/freshdiskann.mddocs/research/hnsw-plus-plus-lid.mddocs/research/nsg-spreading-graph.mddocs/research/original-hnsw-malkov-yashunin.mddocs/research/pipnn-bulk-build.mddocs/research/quiver-binary-quantization.mddocs/research/semantic-scale-validation.mddocs/scratch-hnsw-source-notes.mdgo.modinternal/graph/reverse.gointernal/graph/store.gointernal/index/flat/flat.gointernal/index/flat/flat_test.gointernal/index/hnsw/candidate_shootout_test.gointernal/index/hnsw/candidate_soa.gointernal/index/hnsw/candidate_soa_test.gointernal/index/hnsw/delete.gointernal/index/hnsw/delete_regression_test.gointernal/index/hnsw/global_state.gointernal/index/hnsw/hnsw.gointernal/index/hnsw/hnsw_test.gointernal/index/hnsw/hnsw_throughput_bench_test.gointernal/index/hnsw/insert.gointernal/index/hnsw/mmap_helper.gointernal/index/hnsw/neighbors.gointernal/index/hnsw/node.gointernal/index/hnsw/persistence.gointernal/index/hnsw/quantization_test.gointernal/index/hnsw/raw_slot_array.gointernal/index/hnsw/repair.gointernal/index/hnsw/search.gointernal/index/hnsw/search_regression_test.gointernal/index/hnsw/segmented_array.gointernal/index/hnsw/semantic_scale_bench_test.gointernal/index/hnsw/simple_quantization_test.gointernal/index/hnsw/vector_store.gointernal/index/hnsw/vector_store_mmap.gointernal/index/hnsw/vector_store_slabby.gointernal/index/hnsw/vector_store_slabby_test.gointernal/index/hnsw/vector_store_test.gointernal/index/interfaces.gointernal/index/ivfpq/ivfpq.gointernal/quant/benchmark_test.gointernal/quant/errors.gointernal/quant/fsq.gointernal/quant/fsq_test.gointernal/quant/interfaces.gointernal/quant/interfaces_test.gointernal/quant/product.gointernal/quant/registry.gointernal/storage/fsdurability/sync_test.gointernal/storage/fsdurability/sync_unix.gointernal/storage/fsdurability/sync_windows.gointernal/storage/interfaces.gointernal/storage/singlefile/codec.gointernal/storage/singlefile/engine.gointernal/storage/singlefile/engine_test.gointernal/storage/singlefile/v1compat.gointernal/storage/singlefile/wal_benchmark_test.gointernal/storage/singlefile/wal_request.gointernal/storage/singlefile/wal_request_test.gointernal/util/distance.gointernal/util/distance_test.gointernal/util/simd/distance_amd64.sinternal/util/simd/distance_arm64.sinternal/util/simd/distance_batch_amd64.gointernal/util/simd/distance_batch_arm64.gointernal/util/simd/distance_batch_other.gointernal/util/simd/distance_test.gointernal/util/simd/generate.gointernal/util/simd/generate_directive.gointernal/util/simd/prefetch8_other.gointernal/util/simd/prefetch_amd64.gointernal/util/simd/stub_amd64.gointernal/util/simd/stub_arm64.gointernal/util/simd/stub_notamd64.gointernal/util/simd/stub_notarm64.gointernal/util/simd/tools.golibravdb/async_index.golibravdb/async_index_test.golibravdb/batch.golibravdb/collection.golibravdb/collection_benchmark_test.golibravdb/collection_config_test.golibravdb/database.golibravdb/index_persistence.golibravdb/index_recovery_delta_test.golibravdb/iteration_api_test.golibravdb/migrate.golibravdb/mutation_state.golibravdb/mutation_state_test.golibravdb/options.goscripts/semanticfixture/generate.go
Comment on lines
+846
to
+857
| if c.asyncIndex != nil { | ||
| durable, err := c.asyncIndex.storage.InsertDurableRange(ctx, storageEntry) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to write to storage: %w", err) | ||
| } | ||
| c.asyncIndex.commitOne(storageEntry, durable) | ||
| asyncReserved = false | ||
| if c.metrics != nil { | ||
| c.metrics.VectorInserts.Inc() | ||
| } | ||
| return nil | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Build-blocking: asyncIndexStorage is missing the durable-range methods. Both async write paths call methods that the asyncIndexStorage type does not declare (Lint typecheck failures); the methods exist on the singlefile *Collection, so the interface/type must be extended (or storage retyped).
libravdb/collection.go#L846-L857:c.asyncIndex.storage.InsertDurableRangeis undefined — addInsertDurableRange(context.Context, *index.VectorEntry) (storage.DurableRange, error)toasyncIndexStorage.libravdb/collection.go#L1006-L1022:c.asyncIndex.storage.InsertBatchDurableRangeis undefined — addInsertBatchDurableRange(context.Context, []*index.VectorEntry) (storage.DurableRange, error)toasyncIndexStorage.
🧰 Tools
🪛 GitHub Check: Lint
[failure] 847-847:
c.asyncIndex.storage.InsertDurableRange undefined (type asyncIndexStorage has no field or method InsertDurableRange) (typecheck)
📍 Affects 1 file
libravdb/collection.go#L846-L857(this comment)libravdb/collection.go#L1006-L1022
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libravdb/collection.go` around lines 846 - 857, Add both durable-range
methods to the asyncIndexStorage interface/type used by c.asyncIndex.storage:
InsertDurableRange(context.Context, *index.VectorEntry) (storage.DurableRange,
error) for the path at libravdb/collection.go:846-857, and
InsertBatchDurableRange(context.Context, []*index.VectorEntry)
(storage.DurableRange, error) for the path at libravdb/collection.go:1006-1022.
Ensure the concrete storage implementation satisfies these methods, or retype
storage to the existing implementation that provides them.
Source: Pipeline failures
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/index/hnsw/hnsw.go (1)
447-464: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftMark nodes in-flight before publishing their ID or registry entry.
insertSingleMetadataexposes the ID, node, and size before Lines 461-464 register it as in-flight. A concurrent delete can remove the ID as “stale” beforeh.nodes.Set, or retire the node beforeinsertNodeuses its pointer. Move in-flight registration before the first publication and unwind it on every metadata error and first-node return.Add a barrier-based concurrent insert/delete regression test for this publication window.
Also applies to: 579-605
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/index/hnsw/hnsw.go` around lines 447 - 464, The Insert flow must register each newly allocated node as in-flight before insertSingleMetadata publishes its ID, node, or size. Restructure Insert and the corresponding later insertion path around insertSingleMetadata, h.inFlightNodes, and h.nodes.Set so registration occurs before publication, and ensure every metadata error and first-node return unwinds the registration. Add a barrier-based concurrent insert/delete regression test covering this publication window.
🧹 Nitpick comments (1)
internal/index/hnsw/reclamation_test.go (1)
118-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise both raw stores in the concurrent reclamation test.
This is the only test overlapping searches with delete/reinsert churn, but it omits the new in-memory pointer recycler. Table-test both
RawVectorStoreMemoryandRawVectorStoreSlabby.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/index/hnsw/reclamation_test.go` around lines 118 - 120, Update TestReclamationConcurrentSearchDeleteReinsert to run as a table-driven test covering both RawVectorStoreMemory and RawVectorStoreSlabby. Create a separate reclamation test index for each store within the subtest and preserve the existing concurrent search/delete-reinsert assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/index/hnsw/delete.go`:
- Around line 509-531: The guards in retireStoredVectorAt must not skip cleanup
when h.provider is configured; remove the provider condition while retaining the
existing nil, store, and sentinel checks, so valid slots are retired during
published-node deletion. Apply the corresponding provider-independent slot
release in internal/index/hnsw/hnsw.go lines 1986-2003 for the unpublished-node
rollback path.
In `@internal/index/hnsw/reclamation_test.go`:
- Around line 160-168: Update the mutation loop around index.Delete and
index.Insert to store any error and break instead of calling t.Fatalf
immediately. After the loop, cancel the context and wait for the search workers
to finish before reporting the saved error, ensuring index.Close runs only after
workers have stopped.
---
Outside diff comments:
In `@internal/index/hnsw/hnsw.go`:
- Around line 447-464: The Insert flow must register each newly allocated node
as in-flight before insertSingleMetadata publishes its ID, node, or size.
Restructure Insert and the corresponding later insertion path around
insertSingleMetadata, h.inFlightNodes, and h.nodes.Set so registration occurs
before publication, and ensure every metadata error and first-node return
unwinds the registration. Add a barrier-based concurrent insert/delete
regression test covering this publication window.
---
Nitpick comments:
In `@internal/index/hnsw/reclamation_test.go`:
- Around line 118-120: Update TestReclamationConcurrentSearchDeleteReinsert to
run as a table-driven test covering both RawVectorStoreMemory and
RawVectorStoreSlabby. Create a separate reclamation test index for each store
within the subtest and preserve the existing concurrent search/delete-reinsert
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6254baf4-3c05-489c-8d7e-38409dde2656
📒 Files selected for processing (23)
internal/index/hnsw/delete.gointernal/index/hnsw/global_state.gointernal/index/hnsw/hnsw.gointernal/index/hnsw/hnsw_test.gointernal/index/hnsw/insert.gointernal/index/hnsw/neighbors.gointernal/index/hnsw/reclamation.gointernal/index/hnsw/reclamation_test.gointernal/index/hnsw/repair.gointernal/index/hnsw/search.gointernal/index/hnsw/vector_store.gointernal/index/hnsw/vector_store_slabby.gointernal/index/hnsw/vector_store_slabby_test.gointernal/index/hnsw/vector_store_test.gointernal/util/distance.gointernal/util/distance_test.gointernal/util/simd/stub_arm64.golibravdb/async_index_test.golibravdb/collection.golibravdb/database.golibravdb/index_persistence.golibravdb/migrate.golibravdb/migrate_test.go
🚧 Files skipped from review as they are similar to previous changes (15)
- internal/util/simd/stub_arm64.go
- internal/index/hnsw/vector_store_slabby_test.go
- libravdb/migrate.go
- internal/index/hnsw/global_state.go
- libravdb/async_index_test.go
- internal/util/distance.go
- libravdb/database.go
- internal/index/hnsw/insert.go
- internal/index/hnsw/repair.go
- internal/index/hnsw/vector_store_slabby.go
- internal/index/hnsw/hnsw_test.go
- internal/index/hnsw/search.go
- libravdb/index_persistence.go
- libravdb/collection.go
- internal/index/hnsw/neighbors.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Measured semantic results
These are graph-ready measurements, not async accepted-write claims. The README distinguishes current results from the future 20-30k accepted-write architecture target.
Validation
go test ./...docs/research/semantic-scale-validation.mdReview focus
Automated review of
.sfiles should be treated as advisory. Assembly changes require architecture builds, parity tests, and hardware benchmarks before acceptance.Summary by CodeRabbit