Items from PR #301 critic review
1. Arena growth invalidation (MEDIUM)
SearchArena uses Cell<Vec<u8>>. If the slab grows via resize(), existing pointers from other ArenaVec instances become dangling. Mitigated in practice (1MiB pre-alloc, search uses ~10KiB, arena currently unused on hot path), but should be hardened:
- Add
debug_assert! that no growth occurs after first allocation
- Consider linked-list-of-chunks design
2. Arena module unused on search hot path
pub mod arena is public but HnswSearchContext uses std::collections::BinaryHeap and Vec, not ArenaBinaryHeap. Consider gating behind a feature flag or pub(crate) until wired in.
3. INT8 pre-filter assumption
Near-zero vectors get scale = 1.0, making quantization error unbounded relative to true distance. Document the assumption that input vectors have non-trivial magnitude, or skip INT8 pre-filter when query_norm < threshold.
4. ulid dependency may be unused
Verify and remove if not needed.
Source
Critic review of PR #301.
Items from PR #301 critic review
1. Arena growth invalidation (MEDIUM)
SearchArenausesCell<Vec<u8>>. If the slab grows viaresize(), existing pointers from otherArenaVecinstances become dangling. Mitigated in practice (1MiB pre-alloc, search uses ~10KiB, arena currently unused on hot path), but should be hardened:debug_assert!that no growth occurs after first allocation2. Arena module unused on search hot path
pub mod arenais public butHnswSearchContextusesstd::collections::BinaryHeapandVec, notArenaBinaryHeap. Consider gating behind a feature flag orpub(crate)until wired in.3. INT8 pre-filter assumption
Near-zero vectors get
scale = 1.0, making quantization error unbounded relative to true distance. Document the assumption that input vectors have non-trivial magnitude, or skip INT8 pre-filter whenquery_norm < threshold.4.
uliddependency may be unusedVerify and remove if not needed.
Source
Critic review of PR #301.