feat: sync wrapper for ONNX embeddings function - #46
Merged
Conversation
vkozyura
marked this pull request as draft
July 30, 2026 15:35
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
vkozyura
marked this pull request as ready for review
July 31, 2026 09:53
Contributor
There was a problem hiding this comment.
The PR introduces a useful SQLite VECTOR_EMBEDDING shim for local development, but has several substantive correctness issues: a race condition in prototype patching, repeated model initialization per connection, missing null-guards before synchronous inference, a chunking off-by-one that silently drops tokens, and unsafe file download handling that can leave corrupt cached models. Please address the comments above (including those from the prior review round) before merging.
PR Bot Information
Version: 1.29.9
- Correlation ID:
adee7d50-8cc5-11f1-8581-44e300807992 - Event Trigger:
pull_request.ready_for_review - Experiment Variant:
control - File Content Strategy: Full file content
- LLM:
anthropic--claude-4.6-sonnet
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
vkozyura
changed the title
sjvans
reviewed
Aug 6, 2026
| "srv" | ||
| ], | ||
| "dependencies": { | ||
| "onnxruntime-node": "^1.20.1" |
Contributor
There was a problem hiding this comment.
adds almost 300 MB to build even though not needed in production
sjvans
reviewed
Aug 25, 2026
Comment on lines
+11
to
+18
| "exports": { | ||
| ".": "./cds-plugin.js", | ||
| "./cds-plugin": "./cds-plugin.js", | ||
| "./cds-plugin.js": "./cds-plugin.js", | ||
| "./vector_embedding": "./lib/vector_embedding/index.js", | ||
| "./srv/*": "./srv/*", | ||
| "./lib/*": "./lib/*" | ||
| }, |
sjvans
reviewed
Aug 25, 2026
| "format:check": "npx -y prettier@3 --check . && format-cds --check" | ||
| }, | ||
| "files": [ | ||
| "cds-plugin.js", |
Contributor
There was a problem hiding this comment.
Suggested change
| "cds-plugin.js", |
already defined as main
The exports field was blocking CDS from loading: - cds-plugin.js (plugin registration) - srv/* (AICoreService, MockAICoreService) - lib/* (internal modules) Without these exports, Node.js blocks access to these paths, causing "Navigation property SAP_Recommendations is not defined" errors because the CSN enhancement never registers.
Without this, npm pack excludes cds-plugin.js from the tarball, breaking plugin auto-registration when installed as a dependency. This caused MTX integration tests to fail with 'ResourceGroup undefined' because the plugin never loaded.
sjvans
force-pushed
the
sqlite-embeddings
branch
from
August 25, 2026 20:35
b0d7f8b to
e6cf3dd
Compare
sjvans
force-pushed
the
sqlite-embeddings
branch
from
August 25, 2026 20:37
e6cf3dd to
447f21c
Compare
This was referenced Aug 25, 2026
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
2 participants
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.
Local
VECTOR_EMBEDDINGthroughai-sqliteBased on #53.
This PR adds local semantic embeddings backed by ONNX Runtime and integrates them through
AISQLiteServiceinstead of modifying or being imported by@cap-js/sqlite.ai-sqliteservice starts.VECTOR_EMBEDDINGfor each SQLite connection.remote_sourceargument.NULLsemantics.ai-sqlitekind replaces SQLite's hash embedding implementation.The ONNX dependency is optional for regular
@cap-js/aiconsumers and is required only whenai-sqliteis selected.