Skip to content

Conversation

@aponcedeleonch
Copy link
Member

@aponcedeleonch aponcedeleonch commented Feb 11, 2026

Closes: #3731

Add an SQLite FTS5 ToolStore implementation as an alternative to the existing InMemoryToolStore, and wire it into the vMCP server when optimizer config is present.

This adds:

  • pkg/vmcp/optimizer/fts5store: SQLiteToolStore implementation using modernc.org/sqlite (pure Go, no CGO) with FTS5 virtual tables
  • BM25 ranking for search results with LIKE-based fallback
  • FTS5 query sanitization for safe handling of special characters
  • Thread-safe concurrent access with sync.RWMutex
  • Close() method on the ToolStore interface for resource cleanup
  • OptimizerStoreCloser on server.Config for store lifecycle management
  • OptimizerConfig.FTSDBPath for configurable database location (defaults to in-memory, use emptyDir in Kubernetes)
  • Wire FTS5 store into vMCP serve command when optimizer is configured
  • Comprehensive tests for search, upsert, and concurrency scenarios

Part 2 of the optimizer FTS5 migration (issue #3731).

Large PR Justification

  • Multiple related changes that would break if separated. It just introduces 1 new concept

@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Feb 11, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 78.43137% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.86%. Comparing base (471f878) to head (85a4c84).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...cp/optimizer/internal/sqlite_store/sqlite_store.go 75.94% 10 Missing and 9 partials ⚠️
pkg/vmcp/server/server.go 75.00% 1 Missing and 1 partial ⚠️
cmd/vmcp/app/commands.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3786      +/-   ##
==========================================
+ Coverage   66.81%   66.86%   +0.04%     
==========================================
  Files         438      439       +1     
  Lines       43321    43404      +83     
==========================================
+ Hits        28946    29020      +74     
- Misses      12128    12132       +4     
- Partials     2247     2252       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a closer look at the sqlite_store_test when they're in a more condensed form. Everything else looks mostly good aside for some blocking comments which should be easy to address.

@aponcedeleonch aponcedeleonch force-pushed the issue-3731/sqlite-fts5-store branch from 86a4829 to 27c5e5c Compare February 12, 2026 13:34
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Feb 12, 2026
@aponcedeleonch aponcedeleonch force-pushed the issue-3731/sqlite-fts5-store branch from 27c5e5c to 76c494b Compare February 12, 2026 17:12
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Feb 12, 2026
@github-actions github-actions bot dismissed their stale review February 12, 2026 17:13

Large PR justification has been provided. Thank you!

@github-actions
Copy link
Contributor

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

Copy link
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor blocker, but otherwise LGTM

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Feb 12, 2026
aponcedeleonch and others added 2 commits February 12, 2026 22:00
Add an SQLite FTS5 ToolStore implementation as an alternative to the
existing InMemoryToolStore, and wire it into the vMCP server when
optimizer config is present.

This adds:
- pkg/vmcp/optimizer/fts5store: SQLiteToolStore implementation using
  modernc.org/sqlite (pure Go, no CGO) with FTS5 virtual tables
- BM25 ranking for search results with LIKE-based fallback
- FTS5 query sanitization for safe handling of special characters
- Thread-safe concurrent access with sync.RWMutex
- Close() method on the ToolStore interface for resource cleanup
- OptimizerStoreCloser on server.Config for store lifecycle management
- OptimizerConfig.FTSDBPath for configurable database location
  (defaults to in-memory, use emptyDir in Kubernetes)
- Wire FTS5 store into vMCP serve command when optimizer is configured
- Comprehensive tests for search, upsert, and concurrency scenarios

Part 2 of the optimizer FTS5 migration (issue #3731).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aponcedeleonch aponcedeleonch force-pushed the issue-3731/sqlite-fts5-store branch from 631f1d6 to 85a4c84 Compare February 12, 2026 20:00
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Feb 12, 2026
Copy link
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feel free to address the nit in a later PR

@aponcedeleonch aponcedeleonch merged commit 02496a2 into main Feb 13, 2026
36 of 39 checks passed
@aponcedeleonch aponcedeleonch deleted the issue-3731/sqlite-fts5-store branch February 13, 2026 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Back the dummyOptimizer with a shared SQLite instance and basic similarity search

2 participants