feat: add Redis Vector Set support - #356
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds standalone Redis Vector Set support across storage, command parsing, RESP2/RESP3 encoding, lifecycle handling, cluster routing, and Rust/Python tests. It introduces vector codecs, atomic mutations, point reads, FLAT similarity search, seven commands, and a Phase 1 implementation plan. ChangesRedis Vector Set
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
happy-v587
marked this pull request as ready for review
July 20, 2026 04:12
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/resp/src/encode.rs (1)
232-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce duplication: delegate to existing
append_*helpers.These arms re-implement
append_null,append_boolean,append_double,append_big_number,append_bulk_error, andappend_verbatim_stringbyte-for-byte. Two copies of the same wire formatting (notably theDoubleNaN/inf handling and theVerbatimStringlength logic) can silently diverge on future edits. Consider calling the helpers directly.♻️ Example delegation
- RespData::Null => { - self.buffer.extend_from_slice(b"_"); - self.append_crlf() - } - RespData::Boolean(value) => { - self.buffer.extend_from_slice(b"#"); - self.buffer - .extend_from_slice(if *value { b"t" } else { b"f" }); - self.append_crlf() - } - RespData::Double(value) => { - if value.is_nan() { - self.buffer.extend_from_slice(b",nan"); - } else if value.is_infinite() { - self.buffer.extend_from_slice(if value.is_sign_negative() { - b",-inf" - } else { - b",inf" - }); - } else { - let _ = write!(self.buffer, ",{value}"); - } - self.append_crlf() - } + RespData::Null => self.append_null(), + RespData::Boolean(value) => self.append_boolean(*value), + RespData::Double(value) => self.append_double(*value),🤖 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 `@src/resp/src/encode.rs` around lines 232 - 281, Update the RespData encoding match arms for Null, Boolean, Double, BigNumber, BulkError, and VerbatimString to delegate directly to their existing append_null, append_boolean, append_double, append_big_number, append_bulk_error, and append_verbatim_string helpers. Remove the duplicated byte-formatting and validation logic while preserving each helper’s current output.
🤖 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 `@src/storage/src/meta_compaction_filter.rs`:
- Around line 364-381: Add #[allow(clippy::unwrap_used)] to the
test_vectorset_meta_value_expired function so its existing encoded_key unwrap is
explicitly permitted under the project’s Clippy settings.
In `@src/storage/tests/redis_vector_test.rs`:
- Around line 378-384: Replace direct tempfile::tempdir() usage in
test_storage_routes_all_members_of_one_vectorset_to_one_instance,
test_expired_vectorset_reads_as_missing, and
test_vector_storage_rejects_cluster_mode with unique_test_db_path(); call
safe_cleanup_test_db(&path) before opening storage, pass &path to open(), and
call safe_cleanup_test_db(&path) at each test’s end. Apply these changes at
src/storage/tests/redis_vector_test.rs:378-384, 446-452, and 506-512.
---
Nitpick comments:
In `@src/resp/src/encode.rs`:
- Around line 232-281: Update the RespData encoding match arms for Null,
Boolean, Double, BigNumber, BulkError, and VerbatimString to delegate directly
to their existing append_null, append_boolean, append_double, append_big_number,
append_bulk_error, and append_verbatim_string helpers. Remove the duplicated
byte-formatting and validation logic while preserving each helper’s current
output.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b9c8226-57d2-4f25-8bf0-c96a5a0392c3
📒 Files selected for processing (25)
docs/superpowers/plans/2026-07-19-redis-vector-set.mdsrc/cmd/src/lib.rssrc/cmd/src/table.rssrc/cmd/src/vector.rssrc/conf/src/raft_type.rssrc/raft/src/lib.rssrc/resp/src/encode.rssrc/resp/src/negotiation.rssrc/resp/tests/integration_tests.rssrc/storage/src/batch.rssrc/storage/src/data_compaction_filter.rssrc/storage/src/format_base_value.rssrc/storage/src/format_vector.rssrc/storage/src/lib.rssrc/storage/src/logindex/types.rssrc/storage/src/meta_compaction_filter.rssrc/storage/src/redis.rssrc/storage/src/redis_strings.rssrc/storage/src/redis_vectors.rssrc/storage/src/storage.rssrc/storage/src/storage_impl.rssrc/storage/src/vector.rssrc/storage/tests/redis_basic_test.rssrc/storage/tests/redis_vector_test.rstests/python/test_vector_set_commands.py
16 tasks
happy-v587
force-pushed
the
feat/redis-vector
branch
from
July 25, 2026 12:59
cb68d38 to
049f9c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/superpowers/plans/2026-07-19-redis-vector-set.md`:
- Line 423: The VectorSet recreation flow must guarantee collision-free
generations instead of relying solely on timestamps. Update the VectorMeta
generation logic around VectorMeta::new so each expired-and-recreated VectorSet
receives a monotonic or otherwise unique version, and ensure stale VectorDataCF
rows cannot be addressed by the new generation; alternatively, add a rapid
expire/recreate test that proves timestamp-based generations cannot collide.
- Line 467: Update the documented test command to avoid passing two explicit
filters to cargo test. Run the vector and format_vector test filters as separate
commands, or replace them with one broader valid filter while preserving
coverage of both test groups.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: edc0f854-de79-48e2-af78-de6e26e6b763
📒 Files selected for processing (25)
docs/superpowers/plans/2026-07-19-redis-vector-set.mdsrc/cmd/src/lib.rssrc/cmd/src/table.rssrc/cmd/src/vector.rssrc/conf/src/raft_type.rssrc/raft/src/lib.rssrc/resp/src/encode.rssrc/resp/src/negotiation.rssrc/resp/tests/integration_tests.rssrc/storage/src/batch.rssrc/storage/src/data_compaction_filter.rssrc/storage/src/format_base_value.rssrc/storage/src/format_vector.rssrc/storage/src/lib.rssrc/storage/src/logindex/types.rssrc/storage/src/meta_compaction_filter.rssrc/storage/src/redis.rssrc/storage/src/redis_strings.rssrc/storage/src/redis_vectors.rssrc/storage/src/storage.rssrc/storage/src/storage_impl.rssrc/storage/src/vector.rssrc/storage/tests/redis_basic_test.rssrc/storage/tests/redis_vector_test.rstests/python/test_vector_set_commands.py
🚧 Files skipped from review as they are similar to previous changes (23)
- src/cmd/src/lib.rs
- src/conf/src/raft_type.rs
- src/cmd/src/table.rs
- src/storage/src/storage.rs
- src/storage/src/meta_compaction_filter.rs
- src/raft/src/lib.rs
- src/storage/src/batch.rs
- src/storage/src/logindex/types.rs
- src/storage/src/format_base_value.rs
- src/storage/src/lib.rs
- src/resp/src/negotiation.rs
- src/storage/src/redis_strings.rs
- src/storage/src/data_compaction_filter.rs
- src/storage/tests/redis_basic_test.rs
- tests/python/test_vector_set_commands.py
- src/cmd/src/vector.rs
- src/storage/src/redis.rs
- src/storage/src/storage_impl.rs
- src/resp/src/encode.rs
- src/storage/src/format_vector.rs
- src/storage/src/vector.rs
- src/storage/src/redis_vectors.rs
- src/storage/tests/redis_vector_test.rs
…ions - Remove MODULE_NO_CLUSTER guard so vector commands work in Raft Group clusters (binlog replication); keep follower redirect for writes. - Split src/cmd/src/vector.rs into one module per command. - Introduce SimilarityMetric (currently Cosine) persisted in VectorMeta. - Introduce VectorSearchEngine (currently Flat) for future HNSW. - Add RocksDB encoding layout comments for vector meta/data values. - Update the Redis Vector Set superpower plan to reflect cluster support. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/storage/src/vector.rs (1)
182-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-obvious inverted
Ord— consider a clarifying comment.
Ord::cmpreverses score comparison (other.score.total_cmp(&self.score)) to makeBinaryHeap::peek()surface the worst candidate, but the tie-break (self.element.cmp(&other.element)) is not reversed. This is correct (verified againstflat_search's eviction logic and the finalsort_byat lines 257-262, which are consistent), but the asymmetry is easy to break during future edits. A short comment explaining "reversed for min-heap-of-top-k semantics" would help future maintainers avoid introducing a sign-flip bug.📝 Suggested comment
impl Ord for ScoredCandidate { + // Reversed on `score` so `BinaryHeap` (a max-heap) surfaces the *worst* + // candidate via `peek()`, enabling top-`count` retention via eviction. + // The element tie-break is intentionally NOT reversed. fn cmp(&self, other: &Self) -> Ordering {🤖 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 `@src/storage/src/vector.rs` around lines 182 - 204, Add a concise explanatory comment in ScoredCandidate::cmp documenting that the score ordering is intentionally reversed so BinaryHeap::peek() exposes the worst top-k candidate, while the element tie-break remains in its existing direction. Do not alter the comparison behavior.src/storage/src/format_vector.rs (1)
134-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing regression test for unsupported-metric rejection.
SimilarityMetric::from_u8(Line 179) is new validation logic replacing the old hardcoded cosine check, butvector_codecs_reject_malformed_bytesonly mutates theformatbyte (offset 17), not themetricbyte (offset 19). Add a case that sets an unsupported metric byte and assertsVectorMeta::decodeerrors, to directly cover the newfrom_u8rejection path.✅ Suggested additional test case
let mut bad_meta_format = encoded_meta; bad_meta_format[17] = 0; assert!(VectorMeta::decode(&bad_meta_format).is_err()); + + let mut bad_metric = bad_meta_format; + bad_metric[17] = VECTOR_META_FORMAT; + bad_metric[19] = 0xFF; + assert!(VectorMeta::decode(&bad_metric).is_err()); }🤖 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 `@src/storage/src/format_vector.rs` around lines 134 - 201, Add a regression case to the vector metadata malformed-bytes test, `vector_codecs_reject_malformed_bytes`, that mutates the metric byte at offset 19 to an unsupported value and asserts `VectorMeta::decode` returns an error. Keep the existing format-byte case and target the `SimilarityMetric::from_u8` validation path.src/storage/tests/redis_vector_test.rs (1)
458-467: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDecode
VectorMetabefore mutating its stale TTL fields.The current test slices
VectorMetabytes with hard-coded index math, even thoughVectorMeta::encode/decodedefine the field layout. Decode the CF value, mutate onlyversion/is_deleted-related TTL logic via accessor methods, then re-encode before storing; this keeps the test aligned with the meta-value format and avoids magic offsets.
🤖 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 `@src/storage/tests/redis_vector_test.rs` around lines 458 - 467, Update the test setup around the retrieved meta value to decode it with VectorMeta::decode, mutate the stale TTL-related fields through the type’s accessor methods rather than hard-coded byte offsets, then re-encode the VectorMeta before storing it with put_cf. Preserve the intended previous generation and expired TTL values while keeping the test aligned with the encode/decode format.
🤖 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 `@docs/superpowers/plans/2026-07-19-redis-vector-set.md`:
- Around line 44-51: Update Task 6 to reference the vector module layout under
src/cmd/src/vector/, using vector/mod.rs for shared parsing, helpers, and
registration tests plus separate modules for each command; remove or replace the
conflicting instruction to create src/cmd/src/vector.rs. Apply the same path
correction to the additionally affected section.
---
Nitpick comments:
In `@src/storage/src/format_vector.rs`:
- Around line 134-201: Add a regression case to the vector metadata
malformed-bytes test, `vector_codecs_reject_malformed_bytes`, that mutates the
metric byte at offset 19 to an unsupported value and asserts
`VectorMeta::decode` returns an error. Keep the existing format-byte case and
target the `SimilarityMetric::from_u8` validation path.
In `@src/storage/src/vector.rs`:
- Around line 182-204: Add a concise explanatory comment in ScoredCandidate::cmp
documenting that the score ordering is intentionally reversed so
BinaryHeap::peek() exposes the worst top-k candidate, while the element
tie-break remains in its existing direction. Do not alter the comparison
behavior.
In `@src/storage/tests/redis_vector_test.rs`:
- Around line 458-467: Update the test setup around the retrieved meta value to
decode it with VectorMeta::decode, mutate the stale TTL-related fields through
the type’s accessor methods rather than hard-coded byte offsets, then re-encode
the VectorMeta before storing it with put_cf. Preserve the intended previous
generation and expired TTL values while keeping the test aligned with the
encode/decode format.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c1b7e87-d210-4d07-b742-02e28caf3c58
📒 Files selected for processing (20)
docs/superpowers/plans/2026-07-19-redis-vector-set.mdsrc/cmd/src/lib.rssrc/cmd/src/vector/mod.rssrc/cmd/src/vector/vadd.rssrc/cmd/src/vector/vcard.rssrc/cmd/src/vector/vdim.rssrc/cmd/src/vector/vemb.rssrc/cmd/src/vector/vismember.rssrc/cmd/src/vector/vrem.rssrc/cmd/src/vector/vsim.rssrc/net/src/executor_ext.rssrc/net/tests/storage_command_e2e_tests.rssrc/resp/src/encode.rssrc/resp/tests/resp2_encoding.rssrc/storage/src/format_member_data_key.rssrc/storage/src/format_vector.rssrc/storage/src/lib.rssrc/storage/src/redis_vectors.rssrc/storage/src/vector.rssrc/storage/tests/redis_vector_test.rs
💤 Files with no reviewable changes (1)
- src/cmd/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/storage/src/lib.rs
Restore the flag constant so the bit remains reserved, even though vector commands no longer use it. Co-Authored-By: Claude <noreply@anthropic.com>
…-db#356 - Document reversed score ordering in HeapHit::cmp. - Add regression test for unsupported vector metric byte. - Expose VectorMeta accessors and use decode/encode in tests. - Delegate RESP encoding to version-aware append_* helpers. Co-Authored-By: Claude <noreply@anthropic.com>
Keep remote VectorSearchEngine / SimilarityMetric additions and resolve the outstanding CodeRabbit review fixes: - format_vector.rs: keep SimilarityMetric + metric field, keep encode/decode public for integration tests, preserve metric-byte regression test. - redis_vectors.rs: accept remote removal of HeapHit; search is now delegated to VectorSearchEngine. - vector.rs: add BinaryHeap top-k retention comment to ScoredCandidate::cmp. Co-Authored-By: Claude <noreply@anthropic.com>
- merge the private encode_resp_data_inner into the RespEncode trait method; version handling stays inline at the encoding points - translate the vector-set plan doc to Chinese and update Step 3 to match the merged encoder design - move vector value layout comments next to their struct definitions
# Conflicts: # src/net/src/executor_ext.rs
- CanonicalVector now holds quantized data (VectorData: Fp32/Binary/Int8) with to_quantized() as the single conversion entry point - score() dispatches per quantization: hamming similarity for BIN, cosine on the dequantized FP32 form otherwise - VectorDataValue format v3: quant byte, reserved flags byte (for future SETATTR attributes), per-quantization payload layouts - VectorMeta carries the set-level quantization; vadd/vsim convert members and queries to the set's quantization - unify vector meta reads via decode_vector_meta/read_vector_meta_opt
- move parse_vadd/parse_vsim/parse_vemb and their tests from vector/mod.rs into the respective command modules; mod.rs keeps only the shared helpers - parse_vadd becomes a keyword option loop matching the Redis syntax: NOQUANT/Q8/BIN are wired to quantization (default NOQUANT), while CAS/EF/SETATTR/M/REDUCE are rejected with dedicated errors - add Redis syntax comments to vector command arity definitions
AlexStocks
reviewed
Aug 3, 2026
AlexStocks
reviewed
Aug 3, 2026
AlexStocks
reviewed
Aug 3, 2026
AlexStocks
reviewed
Aug 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Redis Vector Set support to Kiwi (standalone Phase 1), spanning storage persistence (new CF + codecs), command implementations (VADD/VSIM/VREM/VCARD/VDIM/VEMB/VISMEMBER/VINFO + INFO VECTOR), RESP2/RESP3 compatibility encoding for new reply shapes, and snapshot/schema validation needed for safe restore/rolling upgrades.
Changes:
- Introduces Vector Set storage (VectorDataCF, manifests/incarnations, FLAT scan governance + metrics + fault hooks) and exposes vector APIs on
Storage. - Adds vector commands and network/cluster gating (redirects + leader linearizable-read barrier for vector reads), plus snapshot metadata schema validation and a capabilities RPC for rolling upgrades.
- Expands test coverage (Rust storage/raft/net/resp tests + Python integration/differential tests) and adjusts CI sanitizer tooling/symbolization.
Reviewed changes
Copilot reviewed 82 out of 83 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/run_python_integration.sh | Run python integration tests against an isolated Kiwi on port 7379 and export KIWI_HOST/KIWI_PORT. |
| tests/python/test_vector_set_differential.py | Differential RESP2/RESP3 tests comparing Kiwi vector sets against Redis 8 reference. |
| tests/python/test_vector_basic.py | Adds a standalone vector-index demo script (not a pytest test). |
| tests/python/test_mset.py | Allows overriding host/port via KIWI_HOST/KIWI_PORT (default 7379). |
| tests/python/conftest.py | Uses KIWI_HOST/KIWI_PORT and improves connection error messaging. |
| src/storage/tests/scan_test.rs | Extends SCAN type filtering tests to include vectorset. |
| src/storage/tests/redis_string_test.rs | Adds random_key coverage for vectorset keys. |
| src/storage/tests/redis_basic_test.rs | Updates CF count/name assertions to include VectorDataCF. |
| src/storage/tests/checkpoint_test.rs | Updates snapshot meta tests for v2 schema validation and vector schema fields. |
| src/storage/src/vector_metrics.rs | Adds per-instance counters for FLAT query execution and aggregation snapshot type. |
| src/storage/src/vector_flat.rs | Adds synchronous FLAT query gate + cancellation token + scan guard (deadline/budget). |
| src/storage/src/vector_fault.rs | Adds storage-layer fault injection hooks for vector paths. |
| src/storage/src/storage.rs | Adds validate_vector_data_sample and CF-index mapping for VectorDataCF. |
| src/storage/src/storage_scan.rs | Adds vectorset scan type parsing and meta liveness checks. |
| src/storage/src/storage_manifest.rs | Adds per-instance manifest (incarnation + generation) persisted alongside RocksDB. |
| src/storage/src/storage_impl.rs | Adds public Storage vector APIs (vadd/vsim/...) + aggregated vector metrics. |
| src/storage/src/redis.rs | Adds VectorDataCF + manifest loading/checkpoint copying + vector gates/metrics/fault hooks plumbing. |
| src/storage/src/redis_strings.rs | Treats VectorSet metas as BaseMetaValue in expiry/type checks; includes VectorDataCF in traversals. |
| src/storage/src/options.rs | Plumbs VectorConfig into StorageOptions. |
| src/storage/src/meta_compaction_filter.rs | Adds VectorSet meta expiry handling and tests. |
| src/storage/src/logindex/types.rs | Updates logindex CF metadata count/names to include vector_data_cf. |
| src/storage/src/lib.rs | Exposes vector modules/types, snapshot constants, and storage manifest file name. |
| src/storage/src/format_member_data_key.rs | Hardens member key parsing bounds checks and adds malformed-key tests. |
| src/storage/src/format_base_value.rs | Adds DataType::VectorSet + string/tag tables update. |
| src/storage/src/error.rs | Adds dedicated FLAT query governance error variants. |
| src/storage/src/checkpoint.rs | Bumps snapshot meta to v2; adds schema fields and restore validation helpers. |
| src/storage/src/batch.rs | Infers user keys for VectorDataCF via vector member key decoder. |
| src/storage/Cargo.toml | Adds proptest dev-dependency for storage tests. |
| src/server/src/main.rs | Builds command table gates from config; wires gates into storage runtime command table init. |
| src/server/Cargo.toml | Adds cmd dependency (for gates + command table). |
| src/resp/tests/resp2_encoding.rs | Adds RESP2 encoding allocation regression test with RESP3-type downgrades. |
| src/resp/tests/integration_tests.rs | Adjusts RESP3 backward-compat expectations around Null normalization. |
| src/resp/src/negotiation.rs | Updates RESP2 conversion test expectations for map/double coercions. |
| src/resp/src/encode.rs | Implements version-aware downgrades for RESP3 types when encoding RESP2. |
| src/raft/tests/snapshot_roundtrip_test.rs | Updates snapshot failure assertions to match schema validation behavior. |
| src/raft/src/state_machine.rs | Adds snapshot schema validation + restored vector sample validation; uses new meta builder. |
| src/raft/src/snapshot_archive.rs | Updates snapshot version assertion to use storage constant. |
| src/raft/src/node.rs | Implements leader linearizable-read barrier via OpenRaft ensure_linearizable. |
| src/raft/src/lib.rs | Updates exported CF name list to include vector_data_cf; adds capabilities module. |
| src/raft/src/leader_gate.rs | Adds ensure_linearizable_read API with a safe default no-op implementation. |
| src/raft/src/grpc/admin.rs | Adds GetNodeCapabilities admin RPC endpoint. |
| src/raft/src/capabilities.rs | Adds capability advertisement + cluster capability checks for rolling upgrades. |
| src/raft/proto/admin.proto | Extends admin proto with GetNodeCapabilities RPC and messages. |
| src/net/tests/storage_command_e2e_tests.rs | Adds E2E tests for vector redirect/barrier behavior and protocol-specific replies. |
| src/net/src/lib.rs | Builds network command table with gates (feature/cluster flush). |
| src/net/src/executor_ext.rs | Adds vector-read leader barrier + unified redirect message helper. |
| src/conf/src/vector_config.rs | Adds VectorConfig schema + parsing/validation defaults. |
| src/conf/src/raft_type.rs | Updates CF index enum to include VectorDataCF for raft config types. |
| src/conf/src/lib.rs | Wires vector config into config loading tests. |
| src/conf/src/config.rs | Adds vector + cluster-flush config parsing and validation plumbing. |
| src/common/runtime/storage_server.rs | Adds storage-runtime command table init with gates. |
| src/common/runtime/lib.rs | Re-exports storage command table init with gates. |
| src/common/runtime/error.rs | Classifies FLAT governance timeout as retryable; others deterministic. |
| src/cmd/src/zmscore.rs | Updates test stream scaffolding for client creation. |
| src/cmd/src/vector/vsim.rs | Implements VSIM parsing/options and WITHSCORES reply shaping (Map/Array). |
| src/cmd/src/vector/vrem.rs | Implements VREM command. |
| src/cmd/src/vector/vismember.rs | Implements VISMEMBER command. |
| src/cmd/src/vector/vinfo.rs | Implements VINFO command returning Redis-compatible fields (RESP2/RESP3 map downgrade). |
| src/cmd/src/vector/vemb.rs | Implements VEMB (RAW rejected; returns nil bulk for missing element). |
| src/cmd/src/vector/vdim.rs | Implements VDIM command. |
| src/cmd/src/vector/vcard.rs | Implements VCARD command. |
| src/cmd/src/vector/vadd.rs | Implements VADD parsing (Phase 1 requires explicit NOQUANT; rejects unsupported options). |
| src/cmd/src/vector/mod.rs | Adds shared parsing/helpers, error mapping, and vector command registration exports. |
| src/cmd/src/substr.rs | Updates tests to use shared no-requirepass provider + test stream scaffolding. |
| src/cmd/src/sscan.rs | Updates tests’ test stream scaffolding. |
| src/cmd/src/lib.rs | Registers vector module in cmd crate. |
| src/cmd/src/keys.rs | Updates tests’ test stream scaffolding. |
| src/cmd/src/hscan.rs | Updates tests’ test stream scaffolding. |
| src/cmd/src/hello.rs | Uses shared no-requirepass provider; adjusts tests accordingly. |
| src/cmd/src/auth.rs | Adds shared no-requirepass provider; adjusts defaults/tests accordingly. |
| src/cmd/src/admin.rs | Adds INFO VECTOR section with FLAT query metrics. |
| Cargo.lock | Adds cmd dep to server and proptest dep to storage. |
| .github/workflows/ci.yml | Improves sanitizer jobs (llvm-tools, symbolizer discovery, debug info for backtraces). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5 tasks
Reject vector commands before leader routing and read barriers when the cluster cannot serve them yet. node_capabilities stops advertising the unimplemented raft mutation capability while readiness still requires it, and the unsupported vector-cluster-enabled config knob is removed.
Enforce configured per-parse vector limits before large allocations while keeping storage-side checks; make VSIM error precedence match Redis (missing key first, then wrongtype, element, vector parse, dimension, options); return an Array for RESP3 missing+WITHSCORES; accept all-zero vectors as valid with a neutral score; and return WrongArity for a complete vector missing its element.
Register VADD default Q8, VDIM-missing, and VINFO sentinel values as governed known differences in the manifest. Make the vector set differential fail fast under KIWI_COMPAT_REQUIRE_ORACLE when the Redis 8.8.1 reference is unavailable instead of silently turning green.
The pre-existing cancel test slept a fixed 20ms before cancelling, which raced with the exhaustive scan on fast runners (scan could finish in under 20ms and return partial hits instead of aborting). Wait for a FLAT gate permit to be held instead, which proves the scan is in flight before cancelling, removing both race directions.
cargo audit began flagging rkyv 0.7.46 after the RustSec database refresh on 2026-08-04; it is a transitive dependency of openraft 0.9.25 with no safe in-scope upgrade. Document the exemption with a remove_when condition mirroring the deny.toml governance so the Static Analysis check can pass.
VSIM now resolves a missing key to an empty Array before validating the query and options (per Redis 8.8.1 and the cluster error-precedence fix), so the malformed-options assertion must run against an existing key to exercise the option validation path. Aligns the legacy test with the corrected semantics.
count_allocations now resets the global counting flag even when the measured closure panics, preventing leakage into later tests. Remove the shebang from test_vector_basic.py, which is executed by pytest, not run as a script.
Collaborator
Author
Redis Vector Set —— 实现方案评审（细化版）
1. 目标与范围
2. 关键设计取舍
3. 总体架构与请求流
4. 存储与数据格式
5. 集群与 Raft 约束
6. 命令语义与 Redis 8.8.1 兼容
7. 搜索与量化
8. 配置新增向量相关配置（conf/vector_config.rs）并校验范围：维度上限、元素/向量字节上限、 9. 测试与验证
10. 已知限制与后续方向
11. 评审结论以「权威数据在 RocksDB、FLAT 检索、集群显式不可用」为边界的 Phase 1 落地是自洽且可验证的：跨命令/协议/存储/集群四层完整性高，正确性归于可测的编码与生命周期逻辑，而非外部索引；对 Redis 8.8.1 的命令语义在评审轮已细致对齐并纳入机器可读兼容治理。建议按上述 §5（集群）与 §7（量化/搜索）作为主要评审关注点，后续 Phase 以索引替换与集群 mutation 正确性为优先。 |
happy-v587
commented
Aug 5, 2026
happy-v587
left a comment
happy-v587
left a comment
Collaborator
Author
There was a problem hiding this comment.
代码深度审查完成：Rust 单元测试（storage 624 例 / cmd / resp）全部通过，cargo check 通过。整体工程质量高（编码层 proptest、快照 schema 版本化、compaction filter 孤儿清理、故障注入测试均已闭环）。以下按严重程度列出问题，均落在 Files changed 对应行。
AlexStocks
reviewed
Aug 5, 2026
AlexStocks
reviewed
Aug 5, 2026
AlexStocks
reviewed
Aug 5, 2026
| // lands: physical binlog replay cannot re-encode member keys with the local | ||
| // storage incarnation, so cluster vector writes would be unreadable after a | ||
| // leader failover. | ||
| let vector_cmds: Vec<Arc<dyn Cmd>> = vec
Contributor
There was a problem hiding this comment.
[P1] 先补齐 SDD 授权链再注册生产命令
当前 exact Base 的 .planning/SDD.md 将 VectorSet 明确列为当前非目标，并把 Discussion #331 标为 Frozen；同文档还规定，没有 accepted answer 的 Discussion 不能单独定义产品行为。REQ-WORK-007 要求每个实施 PR 关联 SDD 工作包、primary Issue 和适用的 REQ-*，但本 PR body 只引用 #331，没有这条授权链。这里直接把 8 个 Vector 命令接入生产命令表，会绕过项目唯一控制面。请先在独立规划任务中解冻 VectorSet、建立工作包/primary Issue/REQ/acceptance，再以新的实施边界接入生产；完成前不能合并这项实现。
AlexStocks
reviewed
Aug 5, 2026
|
|
||
| [advisories] | ||
| ignore = [ | ||
| # RUSTSEC-2026-0235: rkyv 0.7.46 is a transitive dependency of openraft |
Contributor
There was a problem hiding this comment.
[P1] 让安全豁免随真实 feature 可达性自动失效
exact Head 上 cargo tree --locked --target all -i rkyv@0.7.46 没有依赖路径；cargo tree --locked --target all -e features -i rust_decimal@1.40.0 只显示 openraft -> byte-unit -> rust_decimal(std)，没有启用 rkyv。因此这里的“Raft wire serialization / cluster binlog 不可达”不是当前依赖事实；rkyv 只是 rust_decimal 留在 lockfile 中的可选依赖。按 advisory ID 全局 ignore 后，未来任何 feature 真正启用有漏洞的 rkyv 0.7，cargo audit 仍会继续绿色，而“等 openraft 拉取 rkyv >= 0.8.17”也没有监控真实的 rust_decimal/rkyv 可达性。请按实际 feature graph 修正理由，并增加 CI 断言使 rkyv 一旦进入 resolve/feature graph 就立即失败；做不到自动失效时不要保留这个全局豁免。
AlexStocks
reviewed
Aug 5, 2026
| .filter(|kind| kind.eq_ignore_ascii_case(b"ELE")) | ||
| .and_then(|_| argv.get(3)) | ||
| .map(Vec::as_slice); | ||
| let prepared = match storage.prepare_vsim(&client.key(), element) { |
Contributor
There was a problem hiding this comment.
[P1] 用同一个 RocksDB snapshot 完成 ELE 查询和成员扫描
prepare_vsim 在 snapshot A 中读取 meta 和查询 element 的旧向量；随后 storage.vsim 又创建 snapshot B，重新读取当前 meta 并扫描当前成员。两阶段之间没有持有与 VADD/VREM 相同的 key lock，PreparedVectorQuery 也只携带 dimension 和向量，没有 incarnation、generation 或 revision。并发 VADD 更新查询 element，或删除后重建同名集合时，本次 VSIM 会拿旧查询向量对新一代成员打分，结果既不对应更新前，也不对应更新后的任何串行时刻。请合并为同一 storage 调用/同一 snapshot，或跨两阶段持锁；若必须分阶段，则携带并校验代际/revision 后安全重试，并补带确定性 barrier 的并发回归测试。
AlexStocks
reviewed
Aug 5, 2026
|
|
||
| def test_kiwi_current_resp3_missing_key_withscores_is_array(redis_binary_client): | ||
| client = redis_binary_client | ||
| client.execute_command(b"HELLO", b"3") |
Contributor
There was a problem hiding this comment.
[P1] 不要用 HELLO 3 污染 session 级共享客户端
redis_binary_client 是默认 RESP2 的 session-scoped 连接池；这里直接发送 HELLO 3 后既不恢复也不关闭该连接。紧随其后的零向量测试却断言 WITHSCORES 返回 dict，而 Kiwi 在 RESP2 下会把 Map 编码成扁平 Array，所以该测试只有在连接池碰巧复用这条已切到 RESP3 的连接时才会通过；单独运行、调整顺序或发生重连都会得到不同结果，并可能污染后续测试。请为 RESP3 用例创建并关闭独立的 function-scoped redis.Redis(protocol=3, decode_responses=False) 客户端；零向量回归应显式覆盖 RESP2/RESP3，并分别断言或归一化 Array/Map。
AlexStocks
added a commit
that referenced
this pull request
Aug 8, 2026
Persist staged, paused, rename, reopen, Raft metadata, cleanup, and rollback boundaries so startup can select one authority before admission. Move target replacement out of checkpoint restore and bind recovery to manifest/logical digests and the configured instance count. Constraint: Keep live RocksDB handles until admission is paused and owners drain. Rejected: Delete the live target in PreparedCheckpointRestore or infer authority from directory presence alone. Confidence: High; restart/fault recovery and full Raft/server gates pass. Scope-risk: High; changes snapshot install, startup recovery, and storage replacement ordering. Directive: Execute WP8 Task 4 from the approved Option C plan. Tested: WSL make fmt-check; WSL make lint; WSL cargo test -p raft --all-features; WSL cargo test -p server --all-features. Not-tested: Physical power-loss injection and Windows CI filesystem behavior. Related: #356, #422 Co-authored-by: OmX <omx@oh-my-codex.dev> Signed-off-by: Xin.Zh <alexstocks@foxmail.com>
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
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.
变更说明
基于 Discussion #331 为 Kiwi 增加 Redis Vector Set 的 standalone Phase 1 实现。
本次实现包括：
VectorSet数据类型和独立的vector_data_cf，持久化 FP32 向量及其元数据。VADD、VSIM、VREM、VCARD、VDIM、VEMB、VISMEMBER七个命令。实现边界
NOQUANT和 cosine；尚未实现 Q8、BIN、HNSW/IVF、VINFO、INFO VECTOR及VEMB RAW。VSIM和VSIM ... TRUTH当前均使用同一套精确 FLAT 搜索。用户影响
用户可以在 standalone Kiwi 上持久化、查询、删除 Vector Set 成员，并通过 Redis RESP2 或 RESP3 客户端执行精确相似度搜索。
验证
make fmtmake lintmake buildmake testKIWI_PORT=7389 pytest -q tests/python/test_vector_set_commands.py（14 passed）Summary by CodeRabbit
New Features
VADD,VREM,VCARD,VDIM,VEMB,VISMEMBER, andVSIM.WITHSCORES.Bug Fixes
Tests