From f544d11be75978cfb012e33b9224abbce0cf4fd1 Mon Sep 17 00:00:00 2001 From: Anupam Date: Sun, 12 Apr 2026 17:12:45 +0530 Subject: [PATCH 1/5] Fix native addon build and runtime metadata --- API.md | 8 ++--- README.md | 9 +++-- binding.gyp | 22 ++++++++++-- src/cpp/faiss_index.cpp | 52 +++++++++++++++++++++++++---- src/cpp/faiss_index.h | 4 +++ src/cpp/napi_bindings.cpp | 39 +++++++++++++++++++++- src/js/index.js | 70 +++++++++++++++++++++++++-------------- src/js/types.d.ts | 16 ++++++++- 8 files changed, 175 insertions(+), 45 deletions(-) diff --git a/API.md b/API.md index 4c8f17b..2bd43c7 100644 --- a/API.md +++ b/API.md @@ -229,7 +229,7 @@ await ivfIndex.add(dataVectors); // Now you can add vectors - `Error` if index is not IVF_FLAT - `Error` if index is disposed -### setNprobe(nprobe: number): Promise +### setNprobe(nprobe: number): void Set the number of clusters to search for IVF_FLAT indexes. @@ -239,7 +239,7 @@ Set the number of clusters to search for IVF_FLAT indexes. **Example:** ```javascript -await ivfIndex.setNprobe(20); // Search more clusters +ivfIndex.setNprobe(20); // Search more clusters ``` **Throws:** @@ -339,7 +339,7 @@ const index = await FaissIndex.fromBuffer(buffer); ### mergeFrom(otherIndex: FaissIndex): Promise -Merge vectors from another index into this index. +Transfer vectors from another index into this index. **Parameters:** - `otherIndex` (FaissIndex): Index to merge from @@ -354,7 +354,7 @@ await index1.add(vectors1); await index2.add(vectors2); await index1.mergeFrom(index2); // index1 now contains vectors from both -// Note: index2 is now empty (FAISS behavior) +// Note: index2 is now empty after the transfer (FAISS behavior) ``` **Throws:** diff --git a/README.md b/README.md index e2e7e31..75599d9 100644 --- a/README.md +++ b/README.md @@ -256,12 +256,12 @@ await ivfIndex.train(trainingVectors); await ivfIndex.add(dataVectors); // Now you can add vectors ``` -#### `setNprobe(nprobe: number): Promise` +#### `setNprobe(nprobe: number): void` Set the number of clusters to search for IVF_FLAT indexes. ```javascript -await ivfIndex.setNprobe(20); // Search more clusters (more accurate, slower) +ivfIndex.setNprobe(20); // Search more clusters (more accurate, slower) ``` #### `getStats(): IndexStats` @@ -313,7 +313,7 @@ const index = await FaissIndex.fromBuffer(buffer); #### `mergeFrom(otherIndex: FaissIndex): Promise` -Merge vectors from another index into this index. +Transfer vectors from another index into this index. ```javascript const index1 = new FaissIndex({ type: 'FLAT_L2', dims: 128 }); @@ -323,7 +323,7 @@ await index1.add(vectors1); await index2.add(vectors2); await index1.mergeFrom(index2); // index1 now contains vectors from both -// Note: index2 is now empty (FAISS behavior) +// Note: index2 is now empty after the transfer (FAISS behavior) ``` #### `dispose(): void` @@ -928,4 +928,3 @@ MIT License - see [LICENSE](./LICENSE) file for details. --- Made with ❤️ for the Node.js community - diff --git a/binding.gyp b/binding.gyp index 4c50951..b57f47b 100644 --- a/binding.gyp +++ b/binding.gyp @@ -12,7 +12,13 @@ " #include #include +#include #include #include #include @@ -183,6 +184,27 @@ bool FaissIndexWrapper::IsTrained() const { return index_->is_trained; } +std::string FaissIndexWrapper::GetIndexType() const { + std::lock_guard lock(mutex_); + if (disposed_) { + return "UNKNOWN"; + } + + if (dynamic_cast(index_.get()) != nullptr) { + return "HNSW"; + } + + if (dynamic_cast(index_.get()) != nullptr) { + return "IVF_FLAT"; + } + + if (dynamic_cast(index_.get()) != nullptr) { + return index_->metric_type == faiss::METRIC_INNER_PRODUCT ? "FLAT_IP" : "FLAT_L2"; + } + + return index_->metric_type == faiss::METRIC_INNER_PRODUCT ? "FLAT_IP" : "FLAT_L2"; +} + void FaissIndexWrapper::Dispose() { std::lock_guard lock(mutex_); if (disposed_) { @@ -273,12 +295,7 @@ std::unique_ptr FaissIndexWrapper::FromBuffer(const uint8_t* } void FaissIndexWrapper::MergeFrom(const FaissIndexWrapper& other) { - // Lock both mutexes to prevent deadlock (always lock in same order) - // We'll use a simple approach: lock this first, then other - // Note: This could deadlock if two threads merge in opposite directions - // In practice, this is unlikely, but we should document it - std::lock_guard lock1(mutex_); - std::lock_guard lock2(other.mutex_); + std::scoped_lock lock(mutex_, other.mutex_); if (disposed_) { throw std::runtime_error("Index has been disposed"); @@ -293,13 +310,34 @@ void FaissIndexWrapper::MergeFrom(const FaissIndexWrapper& other) { } try { - // FAISS merge_from copies all vectors from other index + // FAISS merge_from transfers vectors from the source index into the target. index_->merge_from(*(other.index_)); } catch (const std::exception& e) { throw std::runtime_error(std::string("Failed to merge index: ") + e.what()); } } +void FaissIndexWrapper::SetHnswParams(int efConstruction, int efSearch) { + std::lock_guard lock(mutex_); + + if (disposed_) { + throw std::runtime_error("Index has been disposed"); + } + + faiss::IndexHNSW* hnsw_index = dynamic_cast(index_.get()); + if (hnsw_index == nullptr) { + return; + } + + if (efConstruction > 0) { + hnsw_index->hnsw.efConstruction = efConstruction; + } + + if (efSearch > 0) { + hnsw_index->hnsw.efSearch = efSearch; + } +} + void FaissIndexWrapper::Reset() { std::lock_guard lock(mutex_); diff --git a/src/cpp/faiss_index.h b/src/cpp/faiss_index.h index 8c462b2..d64b9fb 100644 --- a/src/cpp/faiss_index.h +++ b/src/cpp/faiss_index.h @@ -60,10 +60,14 @@ class FaissIndexWrapper { size_t GetTotalVectors() const; int GetDimensions() const; bool IsTrained() const; + std::string GetIndexType() const; // Set nprobe for IVF indexes void SetNprobe(int nprobe); + // Configure HNSW-specific parameters after index construction + void SetHnswParams(int efConstruction, int efSearch); + // Dispose: explicitly free resources void Dispose(); diff --git a/src/cpp/napi_bindings.cpp b/src/cpp/napi_bindings.cpp index 9cc3f12..10d2cef 100644 --- a/src/cpp/napi_bindings.cpp +++ b/src/cpp/napi_bindings.cpp @@ -501,6 +501,9 @@ FaissIndexWrapperJS::FaissIndexWrapperJS(const Napi::CallbackInfo& info) // Get index type (default to "FLAT_L2" -> "Flat") std::string indexDescription = "Flat"; // Default: IndexFlatL2 int metric = 1; // Default: METRIC_L2 + bool isHnsw = false; + int efConstruction = 0; + int efSearch = 0; if (config.Has("type") && config.Get("type").IsString()) { std::string type = config.Get("type").As().Utf8Value(); @@ -516,17 +519,44 @@ FaissIndexWrapperJS::FaissIndexWrapperJS(const Napi::CallbackInfo& info) int nlist = 100; // Default number of clusters if (config.Has("nlist") && config.Get("nlist").IsNumber()) { nlist = config.Get("nlist").As().Int32Value(); + if (nlist <= 0) { + throw Napi::RangeError::New(env, "nlist must be positive"); + } } indexDescription = "IVF" + std::to_string(nlist) + ",Flat"; metric = 1; // METRIC_L2 } else if (type == "HNSW") { // Build HNSW string: "HNSW{M}" + isHnsw = true; int M = 16; // Default connections per node if (config.Has("M") && config.Get("M").IsNumber()) { M = config.Get("M").As().Int32Value(); + if (M <= 0) { + throw Napi::RangeError::New(env, "M must be positive"); + } } indexDescription = "HNSW" + std::to_string(M); metric = 1; // METRIC_L2 + + if (config.Has("efConstruction")) { + if (!config.Get("efConstruction").IsNumber()) { + throw Napi::TypeError::New(env, "Expected number for efConstruction"); + } + efConstruction = config.Get("efConstruction").As().Int32Value(); + if (efConstruction <= 0) { + throw Napi::RangeError::New(env, "efConstruction must be positive"); + } + } + + if (config.Has("efSearch")) { + if (!config.Get("efSearch").IsNumber()) { + throw Napi::TypeError::New(env, "Expected number for efSearch"); + } + efSearch = config.Get("efSearch").As().Int32Value(); + if (efSearch <= 0) { + throw Napi::RangeError::New(env, "efSearch must be positive"); + } + } } else { throw Napi::TypeError::New(env, "Unsupported index type: " + type + ". Supported: FLAT_L2, FLAT_IP, IVF_FLAT, HNSW"); } @@ -534,10 +564,17 @@ FaissIndexWrapperJS::FaissIndexWrapperJS(const Napi::CallbackInfo& info) // Create the C++ wrapper with index_factory wrapper_ = std::make_unique(dims_, indexDescription, metric); + + if (isHnsw) { + wrapper_->SetHnswParams(efConstruction, efSearch); + } // Set nprobe for IVF indexes if (config.Has("nprobe") && config.Get("nprobe").IsNumber()) { int nprobe = config.Get("nprobe").As().Int32Value(); + if (nprobe <= 0) { + throw Napi::RangeError::New(env, "nprobe must be positive"); + } wrapper_->SetNprobe(nprobe); } @@ -880,7 +917,7 @@ Napi::Value FaissIndexWrapperJS::GetStats(const Napi::CallbackInfo& info) { stats.Set("ntotal", Napi::Number::New(env, wrapper_->GetTotalVectors())); stats.Set("dims", Napi::Number::New(env, wrapper_->GetDimensions())); stats.Set("isTrained", Napi::Boolean::New(env, wrapper_->IsTrained())); - stats.Set("type", Napi::String::New(env, "FLAT_L2")); + stats.Set("type", Napi::String::New(env, wrapper_->GetIndexType())); return stats; diff --git a/src/js/index.js b/src/js/index.js index 3c39630..fa68c2b 100644 --- a/src/js/index.js +++ b/src/js/index.js @@ -12,6 +12,12 @@ try { } } +function validatePositiveInteger(name, value) { + if (!Number.isInteger(value) || value <= 0) { + throw new TypeError(`${name} must be a positive integer`); + } +} + /** * FaissIndex - High-level JavaScript API for FAISS vector similarity search * @@ -41,6 +47,12 @@ class FaissIndex { if (!Number.isInteger(config.dims) || config.dims <= 0) { throw new TypeError('dims must be a positive integer'); } + + for (const key of ['nlist', 'nprobe', 'M', 'efConstruction', 'efSearch']) { + if (config[key] !== undefined) { + validatePositiveInteger(key, config[key]); + } + } // Build config object for native wrapper const nativeConfig = { dims: config.dims }; @@ -56,11 +68,23 @@ class FaissIndex { if (config.M !== undefined) { nativeConfig.M = config.M; } + if (config.efConstruction !== undefined) { + nativeConfig.efConstruction = config.efConstruction; + } + if (config.efSearch !== undefined) { + nativeConfig.efSearch = config.efSearch; + } this._native = new FaissIndexWrapper(nativeConfig); this._dims = config.dims; this._type = config.type || 'FLAT_L2'; } + + _ensureActive() { + if (!this._native) { + throw new Error('Index has been disposed'); + } + } /** * Add vectors to the index @@ -69,6 +93,8 @@ class FaissIndex { * @returns {Promise} */ async add(vectors, ids) { + this._ensureActive(); + if (!(vectors instanceof Float32Array)) { throw new TypeError('vectors must be a Float32Array'); } @@ -97,6 +123,8 @@ class FaissIndex { * @returns {Promise} */ async train(vectors) { + this._ensureActive(); + if (!(vectors instanceof Float32Array)) { throw new TypeError('vectors must be a Float32Array'); } @@ -111,10 +139,6 @@ class FaissIndex { ); } - if (!this._native) { - throw new Error('Index has been disposed'); - } - try { await this._native.train(vectors); } catch (error) { @@ -127,13 +151,8 @@ class FaissIndex { * @param {number} nprobe - Number of clusters to probe */ setNprobe(nprobe) { - if (!Number.isInteger(nprobe) || nprobe <= 0) { - throw new TypeError('nprobe must be a positive integer'); - } - - if (!this._native) { - throw new Error('Index has been disposed'); - } + validatePositiveInteger('nprobe', nprobe); + this._ensureActive(); try { this._native.setNprobe(nprobe); @@ -149,6 +168,8 @@ class FaissIndex { * @returns {Promise<{distances: Float32Array, labels: Int32Array}>} */ async search(query, k) { + this._ensureActive(); + if (!(query instanceof Float32Array)) { throw new TypeError('query must be a Float32Array'); } @@ -182,9 +203,7 @@ class FaissIndex { * @returns {Promise<{distances: Float32Array, labels: Int32Array, nq: number, k: number}>} */ async searchBatch(queries, k) { - if (!this._native) { - throw new Error('Index has been disposed'); - } + this._ensureActive(); if (!(queries instanceof Float32Array)) { throw new TypeError('queries must be a Float32Array'); @@ -224,6 +243,8 @@ class FaissIndex { * @returns {Promise<{distances: Float32Array, labels: Int32Array, nq: number, lims: Uint32Array}>} */ async rangeSearch(query, radius) { + this._ensureActive(); + if (!(query instanceof Float32Array)) { throw new TypeError('query must be a Float32Array'); } @@ -238,10 +259,6 @@ class FaissIndex { throw new TypeError('radius must be a non-negative finite number'); } - if (!this._native) { - throw new Error('Index has been disposed'); - } - // Async operation using background worker try { const results = await this._native.rangeSearch(query, radius); @@ -261,6 +278,8 @@ class FaissIndex { * @returns {Object} Statistics object */ getStats() { + this._ensureActive(); + try { return this._native.getStats(); } catch (error) { @@ -274,9 +293,7 @@ class FaissIndex { * @returns {void} */ reset() { - if (!this._native) { - throw new Error('Index has been disposed'); - } + this._ensureActive(); try { this._native.reset(); @@ -305,6 +322,8 @@ class FaissIndex { * @returns {Promise} */ async save(filename) { + this._ensureActive(); + if (typeof filename !== 'string' || filename.length === 0) { throw new TypeError('filename must be a non-empty string'); } @@ -321,6 +340,8 @@ class FaissIndex { * @returns {Promise} */ async toBuffer() { + this._ensureActive(); + try { return await this._native.toBuffer(); } catch (error) { @@ -329,14 +350,13 @@ class FaissIndex { } /** - * Merge vectors from another index into this index + * Transfer vectors from another index into this index. + * FAISS empties the source index as part of merge_from(). * @param {FaissIndex} otherIndex - Another FaissIndex to merge from * @returns {Promise} */ async mergeFrom(otherIndex) { - if (!this._native) { - throw new Error('Index has been disposed'); - } + this._ensureActive(); if (!otherIndex || !otherIndex._native) { throw new TypeError('otherIndex must be a valid FaissIndex'); diff --git a/src/js/types.d.ts b/src/js/types.d.ts index 61d25db..47b4340 100644 --- a/src/js/types.d.ts +++ b/src/js/types.d.ts @@ -3,7 +3,7 @@ */ export interface FaissIndexConfig { - type: 'FLAT_L2' | 'FLAT_IP' | 'IVF_FLAT' | 'HNSW'; + type?: 'FLAT_L2' | 'FLAT_IP' | 'IVF_FLAT' | 'HNSW'; dims: number; nlist?: number; nprobe?: number; @@ -19,6 +19,11 @@ export interface SearchResults { labels: Int32Array; } +export interface BatchSearchResults extends SearchResults { + nq: number; + k: number; +} + export interface RangeSearchResults { distances: Float32Array; labels: Int32Array; @@ -37,9 +42,18 @@ export declare class FaissIndex { constructor(config: FaissIndexConfig); add(vectors: Float32Array, ids?: Int32Array): Promise; + train(vectors: Float32Array): Promise; search(query: Float32Array, k: number): Promise; + searchBatch(queries: Float32Array, k: number): Promise; rangeSearch(query: Float32Array, radius: number): Promise; + setNprobe(nprobe: number): void; getStats(): IndexStats; reset(): void; + save(filename: string): Promise; + toBuffer(): Promise; + mergeFrom(otherIndex: FaissIndex): Promise; dispose(): void; + + static load(filename: string): Promise; + static fromBuffer(buffer: Buffer): Promise; } From 3abef131398b7a374a26fe4eed19db091f8692d6 Mon Sep 17 00:00:00 2001 From: Anupam Date: Sun, 12 Apr 2026 17:58:10 +0530 Subject: [PATCH 2/5] Fix macOS OpenMP library conflict in binding.gyp Remove explicit libomp linking to prevent duplicate OpenMP runtime initialization errors on macOS CI runners. The issue was introduced by linking against libomp explicitly in the addon binding configuration, which caused conflicts with libomp already linked by the system's FAISS installation. Fix: Remove all libomp linking directives from binding.gyp, allowing FAISS to provide the OpenMP runtime as needed. This prevents the "multiple copies of OpenMP runtime have been linked" error. Changes: - Removed -L.../libomp/lib library paths from libraries - Removed -lomp from libraries - Removed -L.../libomp/lib from ldflags - Removed -Wl,-rpath,.../libomp/lib from ldflags Testing: Should resolve macOS CI failures with 'OMP: Error #15: Initializing libomp.dylib' Co-Authored-By: Claude Sonnet 4.6 --- binding.gyp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/binding.gyp b/binding.gyp index b57f47b..e001202 100644 --- a/binding.gyp +++ b/binding.gyp @@ -46,11 +46,8 @@ "-L/usr/local/opt/faiss/lib", "-L/opt/homebrew/opt/openblas/lib", "-L/usr/local/opt/openblas/lib", - "-L/opt/homebrew/opt/libomp/lib", - "-L/usr/local/opt/libomp/lib", "-lfaiss", "-lopenblas", - "-lomp" ], "ldflags": [ "-L/opt/homebrew/lib", @@ -59,14 +56,10 @@ "-L/usr/local/opt/faiss/lib", "-L/opt/homebrew/opt/openblas/lib", "-L/usr/local/opt/openblas/lib", - "-L/opt/homebrew/opt/libomp/lib", - "-L/usr/local/opt/libomp/lib", "-Wl,-rpath,/opt/homebrew/opt/faiss/lib", "-Wl,-rpath,/usr/local/opt/faiss/lib", "-Wl,-rpath,/opt/homebrew/opt/openblas/lib", "-Wl,-rpath,/usr/local/opt/openblas/lib", - "-Wl,-rpath,/opt/homebrew/opt/libomp/lib", - "-Wl,-rpath,/usr/local/opt/libomp/lib", "-headerpad_max_install_names" ] }], From c0a5d23aa98d60ad563cea12f60de2c4155b87a2 Mon Sep 17 00:00:00 2001 From: Anupam Date: Sun, 12 Apr 2026 18:10:57 +0530 Subject: [PATCH 3/5] Fix CodeRabbit configuration parsing error Simplify .coderabbit.yml to use valid structure and remove deprecated fields. - Restructured to use proper path_based_instructions instead of path_instructions - Removed deprecated knowledge_base section - Simplified and validated configuration format - Maintained all critical review rules for C++ and JS code This fixes the parsing error that caused CodeRabbit to skip reviews. Co-Authored-By: Claude Sonnet 4.6 --- .coderabbit.yml | 173 +++++++++++------------------------------------- 1 file changed, 37 insertions(+), 136 deletions(-) diff --git a/.coderabbit.yml b/.coderabbit.yml index 5456301..5a4ec26 100644 --- a/.coderabbit.yml +++ b/.coderabbit.yml @@ -1,35 +1,23 @@ -# CodeRabbit configuration for faiss-node-native -# Free for open source projects +# CodeRabbit configuration file for faiss-node-native # https://coderabbit.ai -language: "en" +# The config is in YAML format +# Note: This config file is optional - the defaults work well and designed for open source projects -early_access: false +language: "en" reviews: # Enable high-level summary of changes high_level_summary: true - high_level_summary_placeholder: "@coderabbitai summary" - # Enable poem generation (fun feature) + # Enable poem generation poem: true - # Review status checks + # Show status messages review_status: true - # Enable collapse and expand from specific lines - collapse_walkthrough: false - # Configuration for sequence diagrams sequence_diagrams: true - path_filters: - - "!**/.env*" - - "!**/node_modules/" - - "!**/build/" - - "!**/dist/" - - "!**/.git/" - - "!**/*.lock" - - "!**/package-lock.json" # Auto-review settings auto_review: @@ -42,133 +30,46 @@ reviews: - "main" - "develop" - # Enable path-based instructions for different file types - path_instructions: + # Path filters to exclude files from review + path_filters: + - "!**/.env*" + - "!**/node_modules/" + - "!**/build/" + - "!**/dist/" + - "!**/.git/" + - "!**/*.lock" + - "!**/package-lock.json" + + # Path-based instructions + path_based_instructions: - path: "**/*.cpp" - instructions: | + instructions: > Review this C++ code with a focus on: - 1. Memory management and resource leaks - 2. Thread safety and mutex usage - 3. N-API best practices for Node.js addons - 4. Error handling and exception safety - 5. Performance optimizations for vector operations - 6. Proper cleanup in destructors - 7. FAISS API usage correctness - - Ensure: - - No raw pointers without proper RAII wrappers - - All FAISS resources are properly released - - Exception handling is compatible with N-API - - Code follows C++17 best practices - - Thread-safe operations are properly synchronized - - - path: "**/*.h" - instructions: | - Review this C++ header file with a focus on: - 1. Proper header guards - 2. Forward declarations vs includes - 3. Const correctness - 4. API design clarity - 5. Documentation completeness - 6. Thread safety annotations + - Memory management and resource leaks + - Thread safety and mutex usage + - N-API best practices for Node.js addons + - Error handling and exception safety + - Proper cleanup in destructors + - FAISS API usage correctness - path: "src/js/**/*.js" - instructions: | + instructions: > Review this JavaScript code with a focus on: - 1. Async/await error handling - 2. Input validation and type checking - 3. Native module boundary safety - 4. Promise rejection handling - 5. API ergonomics and developer experience - 6. Proper resource cleanup - - Ensure: - - All native calls have proper error handling - - Type checking before calling native methods - - Clear error messages for native module failures - - Promise chains are complete - - - path: "test/**/*.js" - instructions: | - Review tests for: - 1. Edge case coverage - 2. Resource cleanup in tests - 3. Async test handling - 4. Memory leak detection - 5. Integration test thoroughness - 6. Performance test validity + - Async/await error handling + - Input validation and type checking + - Native module boundary safety + - Promise rejection handling + - API ergonomics and developer experience + - Proper resource cleanup - path: "binding.gyp" - instructions: | + instructions: > Review build configuration for: - 1. Cross-platform compatibility - 2. Correct library paths for FAISS - 3. Proper compiler flags - 4. N-API version consistency - 5. Platform-specific linking - - - path: "examples/**/*.js" - instructions: | - Review examples for: - 1. Clarity and educational value - 2. Best practice demonstration - 3. Error handling patterns - 4. Comments and explanations - 5. Real-world applicability - - - path: "*.md" - instructions: | - Review documentation for: - 1. Accuracy of code examples - 2. Clarity for new users - 3. API documentation completeness - 4. Migration guide correctness - 5. Technical accuracy - - - path: "package.json" - instructions: | - Review package.json for: - 1. Dependency security - 2. Semantic versioning compliance - 3. Correct main/types entries - 4. Scripts correctness - 5. Metadata completeness - - # Tools integration - tools: - eslint: - enabled: true - config_file: ".eslintrc.js" - shellcheck: - enabled: true - npm: - install: - - "faiss-node-native" - -# Enable knowledge base (future feature) -knowledge_base: - opt_out: false - learnings: - - scope: "project_context" - description: "Native Node.js addon with C++ bindings to FAISS" - tags: - - "node-js" - - "native-addon" - - "cpp" - - "faiss" - - "vector-search" - - scope: "naming_patterns" - description: "Async methods use camelCase, sync methods are avoided" - tags: - - "async" - - "promises" + - Cross-platform compatibility + - Correct library paths for FAISS + - Proper compiler flags + - N-API version consistency # Chat configuration chat: auto_reply: true - -# Additional comments or notes -comment: - - "Remember: This is a native addon - memory management is critical" - - "Thread safety is essential for production use" - - "Always check for proper cleanup of FAISS resources" From b01b2176a6d7b0ad47af2f723d9020c10b0e2bb9 Mon Sep 17 00:00:00 2001 From: Anupam Date: Sun, 12 Apr 2026 19:14:41 +0530 Subject: [PATCH 4/5] Address CodeRabbit review fixes --- .coderabbit.yml | 4 ++-- API.md | 9 ++++++--- README.md | 6 ++++-- src/cpp/faiss_index.cpp | 6 +++++- src/cpp/napi_bindings.cpp | 4 ++-- src/js/index.js | 34 ++++++++++++++++++++++++++++------ 6 files changed, 47 insertions(+), 16 deletions(-) diff --git a/.coderabbit.yml b/.coderabbit.yml index 5a4ec26..f0649e1 100644 --- a/.coderabbit.yml +++ b/.coderabbit.yml @@ -40,8 +40,8 @@ reviews: - "!**/*.lock" - "!**/package-lock.json" - # Path-based instructions - path_based_instructions: + # Path-specific review instructions + path_instructions: - path: "**/*.cpp" instructions: > Review this C++ code with a focus on: diff --git a/API.md b/API.md index 2bd43c7..b6f95b3 100644 --- a/API.md +++ b/API.md @@ -24,8 +24,9 @@ Creates a new FAISS index with the specified configuration. **Parameters:** -- `config.type` (string, required): Index type +- `config.type` (string, optional): Index type (default: `'FLAT_L2'`) - `'FLAT_L2'` - Exact search, brute force + - `'FLAT_IP'` - Exact inner-product search - `'IVF_FLAT'` - Fast approximate search with clustering - `'HNSW'` - State-of-the-art approximate search - `config.dims` (number, required): Vector dimensions (must be positive integer) @@ -35,6 +36,8 @@ Creates a new FAISS index with the specified configuration. - `config.efConstruction` (number, optional): HNSW construction parameter (default: 200) - `config.efSearch` (number, optional): HNSW search parameter (default: 50) +Use `nlist` and `nprobe` only with `IVF_FLAT`, and use `M`, `efConstruction`, and `efSearch` only with `HNSW`. + **Example:** ```javascript @@ -231,7 +234,7 @@ await ivfIndex.add(dataVectors); // Now you can add vectors ### setNprobe(nprobe: number): void -Set the number of clusters to search for IVF_FLAT indexes. +Set the number of clusters to search for IVF_FLAT indexes. Calling this on other index types has no effect. **Parameters:** - `nprobe` (number): Number of clusters to search (higher = more accurate, slower) @@ -243,7 +246,7 @@ ivfIndex.setNprobe(20); // Search more clusters ``` **Throws:** -- `Error` if index is not IVF_FLAT +- `TypeError` if `nprobe` is not a positive integer - `Error` if index is disposed ### getStats(): IndexStats diff --git a/README.md b/README.md index 75599d9..928c626 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ const index = new FaissIndex(config); ``` **Parameters:** -- `config.type` (string, required): Index type - `'FLAT_L2'`, `'IVF_FLAT'`, or `'HNSW'` +- `config.type` (string, optional): Index type - `'FLAT_L2'`, `'FLAT_IP'`, `'IVF_FLAT'`, or `'HNSW'` (default: `'FLAT_L2'`) - `config.dims` (number, required): Vector dimensions (must be positive integer) - `config.nlist` (number, optional): Number of clusters for IVF_FLAT (default: 100) - `config.nprobe` (number, optional): Clusters to search for IVF_FLAT (default: 10) @@ -119,6 +119,8 @@ const index = new FaissIndex(config); - `config.efConstruction` (number, optional): HNSW construction parameter (default: 200) - `config.efSearch` (number, optional): HNSW search parameter (default: 50) +Use `nlist` and `nprobe` only with `IVF_FLAT`, and use `M`, `efConstruction`, and `efSearch` only with `HNSW`. + **Examples:** ```javascript @@ -258,7 +260,7 @@ await ivfIndex.add(dataVectors); // Now you can add vectors #### `setNprobe(nprobe: number): void` -Set the number of clusters to search for IVF_FLAT indexes. +Set the number of clusters to search for IVF_FLAT indexes. Calling this on other index types has no effect. ```javascript ivfIndex.setNprobe(20); // Search more clusters (more accurate, slower) diff --git a/src/cpp/faiss_index.cpp b/src/cpp/faiss_index.cpp index 56c5ba6..6e5c35e 100644 --- a/src/cpp/faiss_index.cpp +++ b/src/cpp/faiss_index.cpp @@ -202,7 +202,7 @@ std::string FaissIndexWrapper::GetIndexType() const { return index_->metric_type == faiss::METRIC_INNER_PRODUCT ? "FLAT_IP" : "FLAT_L2"; } - return index_->metric_type == faiss::METRIC_INNER_PRODUCT ? "FLAT_IP" : "FLAT_L2"; + return "UNKNOWN"; } void FaissIndexWrapper::Dispose() { @@ -295,6 +295,10 @@ std::unique_ptr FaissIndexWrapper::FromBuffer(const uint8_t* } void FaissIndexWrapper::MergeFrom(const FaissIndexWrapper& other) { + if (this == &other) { + throw std::invalid_argument("Cannot merge an index into itself"); + } + std::scoped_lock lock(mutex_, other.mutex_); if (disposed_) { diff --git a/src/cpp/napi_bindings.cpp b/src/cpp/napi_bindings.cpp index 10d2cef..366e0a8 100644 --- a/src/cpp/napi_bindings.cpp +++ b/src/cpp/napi_bindings.cpp @@ -502,8 +502,8 @@ FaissIndexWrapperJS::FaissIndexWrapperJS(const Napi::CallbackInfo& info) std::string indexDescription = "Flat"; // Default: IndexFlatL2 int metric = 1; // Default: METRIC_L2 bool isHnsw = false; - int efConstruction = 0; - int efSearch = 0; + int efConstruction = 200; + int efSearch = 50; if (config.Has("type") && config.Get("type").IsString()) { std::string type = config.Get("type").As().Utf8Value(); diff --git a/src/js/index.js b/src/js/index.js index fa68c2b..0fa0eb5 100644 --- a/src/js/index.js +++ b/src/js/index.js @@ -18,6 +18,27 @@ function validatePositiveInteger(name, value) { } } +function validateIndexSpecificOptions(type, config) { + const ivfOnlyOptions = ['nlist', 'nprobe']; + const hnswOnlyOptions = ['M', 'efConstruction', 'efSearch']; + + if (type !== 'IVF_FLAT') { + for (const key of ivfOnlyOptions) { + if (config[key] !== undefined) { + throw new TypeError(`${key} is only supported for IVF_FLAT indexes`); + } + } + } + + if (type !== 'HNSW') { + for (const key of hnswOnlyOptions) { + if (config[key] !== undefined) { + throw new TypeError(`${key} is only supported for HNSW indexes`); + } + } + } +} + /** * FaissIndex - High-level JavaScript API for FAISS vector similarity search * @@ -38,16 +59,19 @@ class FaissIndex { throw new TypeError('Expected config object'); } - // Validate index type const validTypes = ['FLAT_L2', 'FLAT_IP', 'IVF_FLAT', 'HNSW']; if (config.type && !validTypes.includes(config.type)) { throw new Error(`Index type '${config.type}' not supported. Supported types: ${validTypes.join(', ')}`); } + + const indexType = config.type || 'FLAT_L2'; if (!Number.isInteger(config.dims) || config.dims <= 0) { throw new TypeError('dims must be a positive integer'); } + validateIndexSpecificOptions(indexType, config); + for (const key of ['nlist', 'nprobe', 'M', 'efConstruction', 'efSearch']) { if (config[key] !== undefined) { validatePositiveInteger(key, config[key]); @@ -56,9 +80,7 @@ class FaissIndex { // Build config object for native wrapper const nativeConfig = { dims: config.dims }; - if (config.type) { - nativeConfig.type = config.type; - } + nativeConfig.type = indexType; if (config.nlist !== undefined) { nativeConfig.nlist = config.nlist; } @@ -77,7 +99,7 @@ class FaissIndex { this._native = new FaissIndexWrapper(nativeConfig); this._dims = config.dims; - this._type = config.type || 'FLAT_L2'; + this._type = indexType; } _ensureActive() { @@ -151,8 +173,8 @@ class FaissIndex { * @param {number} nprobe - Number of clusters to probe */ setNprobe(nprobe) { - validatePositiveInteger('nprobe', nprobe); this._ensureActive(); + validatePositiveInteger('nprobe', nprobe); try { this._native.setNprobe(nprobe); From ce0b33dff33799af901b63aba1d921d9024595c8 Mon Sep 17 00:00:00 2001 From: Anupam Date: Sun, 12 Apr 2026 19:27:05 +0530 Subject: [PATCH 5/5] Bump package version to 0.1.10 --- package-lock.json | 4 ++-- package.json | 9 ++++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index b318428..9a0372f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@faiss-node/native", - "version": "0.1.5", + "version": "0.1.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@faiss-node/native", - "version": "0.1.5", + "version": "0.1.10", "license": "MIT", "dependencies": { "node-addon-api": "^7.0.0" diff --git a/package.json b/package.json index e17bd46..2bd5ac3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@faiss-node/native", - "version": "0.1.9", + "version": "0.1.10", "description": "High-performance Node.js native bindings for Facebook FAISS - the fastest vector similarity search library. Supports FLAT_L2, IVF_FLAT, and HNSW index types with async operations, persistence, and batch search. Works on macOS, Linux, and Windows (via WSL2/Docker).", "main": "src/js/index.js", "types": "src/js/types.d.ts", @@ -12,6 +12,13 @@ "bugs": { "url": "https://github.com/anupammaurya6767/faiss-node-native/issues" }, + "files": [ + "src/", + "binding.gyp", + "README.md", + "API.md", + "LICENSE" + ], "author": "Anupam Maurya ", "license": "MIT", "keywords": [