feat(store): add zero-dependency local fallback stores and make native deps optional#92
feat(store): add zero-dependency local fallback stores and make native deps optional#92
Conversation
- Add LocalVectorStore: JSON file-based vector store with cosine similarity and no native dependencies as the new default in DefaultContextStore - Move @lancedb/lancedb to optionalDependencies in both rpg-store and rpg-utils - Remove NullVectorStore (replaced by LocalVectorStore) - Remove vectorEnabled field from ContextStoreConfig - Add ./null-vector-store and ./local sub-path exports to rpg-store - Add 10 unit tests for LocalVectorStore
Summary of ChangesHello @amondnet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new, lightweight Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The introduction of LocalVectorStore is a great architectural improvement to make LanceDB an optional dependency, simplifying the setup for environments where native binaries are difficult to manage. However, the LocalVectorStore implementation has several issues regarding resource management and performance that should be addressed. Specifically, it leaks temporary directories in memory mode and uses blocking synchronous I/O operations in asynchronous methods. There is also a broken export in packages/store/package.json for a file that was reportedly removed.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/utils/package.json">
<violation number="1" location="packages/utils/package.json:34">
P2: @lancedb/lancedb is imported directly by @pleaseai/rpg-utils, so it must remain a required dependency. Keeping it optional can break runtime module resolution when vector.ts is used. Move it back to dependencies or remove/guard the import.
(Based on your team's feedback about keeping directly imported native DB modules in dependencies.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/store/src/local/vector-store.ts">
<violation number="1" location="packages/store/src/local/vector-store.ts:31">
P2: Temporary directories created for memory mode are never cleaned up in close(), which can leak disk space over time. Track the auto-created path and remove it during close().
(Based on your team's feedback about cleaning up mkdtempSync-created directories.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/store/src/local/vector-store.ts:107">
P2: `Math.min(a.length, b.length)` silently truncates the longer vector when dimensions differ, producing incorrect cosine similarity scores. Vectors with mismatched dimensions should either throw an error (to surface embedding bugs early) or at minimum treat missing dimensions as 0 by using `Math.max` instead of `Math.min`.</violation>
</file>
<file name="packages/store/package.json">
<violation number="1" location="packages/store/package.json:19">
P2: The new export "./null-vector-store" points to ./src/null-vector-store.ts, but that file no longer exists. This will break package exports resolution for consumers trying to import the sub-path. Remove the export or restore the file.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:18">
P2: Native module better-sqlite3 is still imported in the codebase, but the build script no longer marks it as external. Bun build will try to bundle the native bindings, which typically breaks native modules at build or runtime. Keep better-sqlite3 external to avoid bundling native binaries.</violation>
</file>
<file name="packages/store/src/default-context-store.ts">
<violation number="1" location="packages/store/src/default-context-store.ts:52">
P2: If _vector.close() throws, _text and _graph are never closed, which can leak resources. Ensure all stores attempt to close even if one fails (e.g., use Promise.allSettled and rethrow).</violation>
</file>
<file name="tests/store/local-vector-store.test.ts">
<violation number="1" location="tests/store/local-vector-store.test.ts:65">
P2: Add a non-empty assertion before using `.every()` so the test doesn't pass vacuously when `results` is empty.
(Based on your team's feedback about asserting non-empty collections before using Array.prototype.every() in tests.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
… SemanticSearch - Delete packages/utils/src/vector.ts (LanceDB-based VectorStore implementation) - Remove @lancedb/lancedb dependency and ./vector export from packages/utils/package.json - Remove vector type re-exports from packages/utils/src/index.ts - Rewrite SemanticSearch to accept VectorStore interface via constructor injection (removed dbPath) - Update MCP server to create LocalVectorStore and inject it into SemanticSearch - Update all encoder and tools tests to use LocalVectorStore from @pleaseai/rpg-store
There was a problem hiding this comment.
1 issue found across 10 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/encoder/src/semantic-search.ts">
<violation number="1" location="packages/encoder/src/semantic-search.ts:88">
P2: `upsertBatch` always falls through to the per-document upsert because `await ...` resolves to `undefined`. This duplicates work and can create duplicate writes when `upsertBatch` exists.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ee-sitter deps - Add --external better-sqlite3 to build script - Move @lancedb/lancedb back to optionalDependencies in rpg-store - Remove stale ./null-vector-store export from rpg-store package.json - Add tree-sitter-python and tree-sitter-typescript to root devDependencies so Node.js can resolve them when running the built CLI - Delete obsolete tests/utils/vector-hybrid.test.ts (imports deleted module)
- Add direct clear() unit test (count and search return empty) - Add clear() persistence test (data absent after reopen) - Add SemanticSearch.clear() no-op test when VectorStore lacks clear - Strengthen searchFts fallback assertion: exact count instead of >=0
- Validate config.path in LocalVectorStore.open() with clear TypeError - Narrow catch to ENOENT only; corrupt vectors.json now throws instead of silently resetting - Wrap flush() in close() with try/finally so index/filePath always reset - Add clear() to VectorStore interface (optional) and LocalVectorStore - Fix upsertBatch dispatch: use explicit if/else instead of ?. + ?? to avoid void/undefined confusion - Add SemanticSearch.clear() guarded by VectorStore.clear? - Add log.debug() in searchHybrid and searchFts to surface degraded-mode fallback - Aggregate errors in DefaultContextStore.close() so all stores close even if one fails - Fix existsSync check in MCP server to target vectors.json, not the directory
…-sqlite3 optional - Add LocalGraphStore: zero-dependency in-memory+JSON-file GraphStore fallback - Add LocalTextSearchStore: in-memory term-frequency TextSearchStore fallback - DefaultContextStore falls back to local stores when better-sqlite3 is unavailable - Move better-sqlite3 to optionalDependencies in @pleaseai/rpg-store - Add 38 unit tests covering all store operations and persistence
amondnet
changed the title
There was a problem hiding this comment.
3 issues found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/store/src/local/graph-store.ts">
<violation number="1" location="packages/store/src/local/graph-store.ts:55">
P2: Temp directories created for memory mode are never cleaned up, so each open/close leaves a directory behind. Track the auto-created path and remove it in close() to avoid accumulating temp files.
(Based on your team's feedback about cleaning up mkdtempSync directories.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/store/src/default-context-store.ts">
<violation number="1" location="packages/store/src/default-context-store.ts:43">
P2: This bare `catch` swallows any SQLite initialization/open error and silently falls back to the local stores, which can mask real DB failures and lead to writing data to the wrong backend. Only fall back when the optional SQLite module is missing; otherwise rethrow.</violation>
</file>
<file name="packages/store/package.json">
<violation number="1" location="packages/store/package.json:26">
P2: better-sqlite3 is imported directly by SQLiteGraphStore/SQLiteTextSearchStore, so making it optional can break runtime when those modules load. Keep it in dependencies (or make the imports truly optional/lazy).
(Based on your team's feedback about declaring native database modules as dependencies when imported directly.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ismatch - Track auto-created temp directory in LocalVectorStore and remove it in close() to prevent orphaned directories from accumulating - Replace Math.min dimension truncation with a strict dimension check that throws on mismatch to surface embedding bugs early - Add non-empty assertion before .every() in remove test to prevent vacuous truth pass when results array is empty
…catch - LocalGraphStore.close() now removes auto-created mkdtemp dirs (same pattern as LocalVectorStore._tempDir) - DefaultContextStore.open() catch block now only falls back to local stores on MODULE_NOT_FOUND errors; other errors are rethrown to surface real SQLite failures
|
1 participant
Summary
LocalVectorStore: a JSON file-based vector store with cosine similarity search and zero native dependencies, replacingNullVectorStoreas the default inDefaultContextStore@lancedb/lancedbtooptionalDependenciesin both@pleaseai/rpg-storeand@pleaseai/rpg-utils, making LanceDB an opt-in dependencyNullVectorStore(packages/store/src/null-vector-store.ts) and thevectorEnabledfield fromContextStoreConfigChanges
packages/store/src/local/vector-store.ts— newLocalVectorStoreimplementation (JSON persistence, cosine similarity)packages/store/src/local/index.ts— barrel export for./localsub-pathpackages/store/src/default-context-store.ts— switch default vector store from LanceDB toLocalVectorStorepackages/store/src/types.ts— removevectorEnabledfield fromContextStoreConfigpackages/store/package.json—@lancedb/lancedbmoved tooptionalDependencies; added./null-vector-storeand./localsub-path exportspackages/utils/package.json—@lancedb/lancedbmoved tooptionalDependenciespackages/graph/src/rpg.ts— removevectorEnabled: falsefromstore.open()calltests/store/local-vector-store.test.ts— 10 unit tests forLocalVectorStoreTest Plan
bun run test tests/store/local-vector-store.test.ts— all 10 unit tests passbun run build— build succeeds without LanceDB native bindings requiredbun run typecheck— no type errorsDefaultContextStoreworks without@lancedb/lancedbinstalledSummary by cubic
Adds LocalVectorStore and local graph/text stores so the app runs without native deps. DefaultContextStore uses the local vector store and falls back to local graph/text when SQLite isn’t available; SemanticSearch injects a VectorStore and LanceDB is optional.
New Features
Migration
Written for commit 1f20881. Summary will update on new commits.