-
Notifications
You must be signed in to change notification settings - Fork 19
feat(core): add QMD-inspired hybrid search pipeline #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
ea3ce01
feat(core): add QMD-inspired hybrid search pipeline
rohitg00 eb1275e
fix(search): address CodeRabbit review findings
rohitg00 02c36c2
fix(search): address CodeRabbit review round 2
rohitg00 fb9c8a1
fix(search): use @ts-ignore for optional node-llama-cpp imports
rohitg00 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: rohitg00/skillkit
Length of output: 1553
🏁 Script executed:
Repository: rohitg00/skillkit
Length of output: 5201
🏁 Script executed:
Repository: rohitg00/skillkit
Length of output: 2141
🏁 Script executed:
Repository: rohitg00/skillkit
Length of output: 669
🏁 Script executed:
Repository: rohitg00/skillkit
Length of output: 1235
🌐 Web query:
better-sqlite3 Node.js version requirements engines compatibility💡 Result:
better-sqlite3(v12.4.6) Node.js engine constraint (npmengines):node: "20.x || 22.x || 23.x || 24.x || 25.x"— notably Node 21.x is not included. [1]package.json/ native ABI). [2]package.jsonon unpkg shows noenginesfield, so npm won’t enforce a Node version viaenginesfor that release line (though it still may or may not build/run on your Node due to native ABI). [3]Sources:
[1]
better-sqlite3@12.4.6package.json(engines) (app.unpkg.com)[2]
better-sqlite3README / install notes (github.com)[3]
better-sqlite3@11.9.0package.json(no engines) (app.unpkg.com)Citations:
🌐 Web query:
sqlite-vec Node.js version requirements engines💡 Result:
The
sqlite-vecnpm distribution does not declare a Node.js version requirement viapackage.json#engines(at least in the published0.1.7-alpha.2artifacts). For example, the platform packages’package.jsonfiles contain noenginesfield—only metadata likeos/cpuand entrypoints. [1], [2]Practically, that means npm won’t enforce a minimum Node version for
sqlite-vecitself; your effective Node version requirement will come from whichever SQLite driver you use to load the extension (e.g.,better-sqlite3/sqlite3). The upstream project documents installing vianpm install sqlite-vecfor Node.js. [3]Sources: [1], [2], [3]
Tighten
node-llama-cppversion constraint and ensure consistent fallback behavior.The
node-llama-cpprange^3.0.0can resolve to versions 3.7.0+ which require Node >=20.0.0, but the package declares engines Node >=18.0.0. This creates an incompatibility: installation will fail on Node 18.x.Additionally, fallback behavior is inconsistent—
better-sqlite3andsqlite-vecgracefully degrade when missing, butnode-llama-cppthrows an error inexpansion.tsandembeddings.tsrather than following the same fallback pattern used invector-store.ts.Either:
node-llama-cppto^3.0.0 <3.7.0to maintain Node 18 compatibility, or>=20.0.0and document the breaking change.🤖 Prompt for AI Agents