Perf: ToolSearch loads only candidate rows from tool_index, not the whole table#14
Draft
mimeding wants to merge 2 commits into
Draft
Perf: ToolSearch loads only candidate rows from tool_index, not the whole table#14mimeding wants to merge 2 commits into
mimeding wants to merge 2 commits into
Conversation
ToolSearchService.search ran an embedding query that already returned a small ranked set of tool IDs (topK, with a search-time fan-out of ~3*topK), then called ToolDatabase.shared.loadAllEntries() and filtered the result in Swift. On a workspace with a large tool catalogue (plugin-heavy setups, multiple MCP bundles) every RAG search paid the cost of reading and decoding every row in the tool_index table just to discard most of them, on the agent hot path. Add ToolDatabase.loadEntries(ids:) that issues 'SELECT ... WHERE id IN (?, ?, ?, ...)' with one placeholder per id. SQLite's default SQLITE_MAX_VARIABLE_NUMBER (999) bounds the safe batch size well above any plausible search candidate count. Update ToolSearchService.search to call loadEntries(ids: toolIdSet) instead of loadAllEntries().filter, then apply only the enabled-name filter (which depends on MainActor state and can't be expressed in SQL). Behavior change is purely performance: same rows returned in the same order downstream, just without the O(catalog_size) scan-and-filter. Tests added: * loadEntriesByIdsReturnsOnlyRequested - happy path * loadEntriesByIdsEmptyInputSkipsQuery - guard avoids invalid SQL * loadEntriesByIdsIgnoresUnknownIds - missing ids are dropped Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
9 tasks
ModelManager.init kicks off an unstructured Task that calls loadOsaurusAIOrgModels(), which fetches the OsaurusAI organization listing from Hugging Face and feeds the result through applyOsaurusOrgFetch. The unit-test runner repeatedly constructs ModelManager() to drive applyOsaurusOrgFetch directly. The background launch-time fetch races with those test calls — whichever finishes last wins, and the merge result is non-deterministic. That's the root cause of the flaky ModelManagerSuggestedTests failures seen across many of the recent PR CI runs (applyOsaurusOrgFetch_dropsStaleAutoFetched OnReapply, applyOsaurusOrgFetch_addsNewEntriesAfterCurated, etc.). Gate the launch-time fetch on a small isRunningInTestEnvironment helper that checks for any of XCTestConfigurationFilePath, XCTestBundlePath, or XCTestSessionIdentifier in the process environment. Those variables are only present inside an xctest host process; production app launches still get the HF fetch exactly as before. This is a network call, so removing it under tests also has the side benefit of making the test suite work offline / on hermetic CI runners. Co-authored-by: Michael Meding <mimeding@users.noreply.github.com>
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
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.
Summary
Why this matters (business)
ToolSearchServiceis on the agent hot path — it runs every time an agent decides which tools are relevant to a user message. As the user installs more plugins / MCP bundles, thetool_indextable grows, and the cost of selecting tools grows with it even though the ranking step (VecturaKit) already narrows the candidate set to a handful.Today, every tool search reads and decodes every row in
tool_indexfrom disk just to filter all-but-a-few-out in Swift. For a workspace with a large tool catalogue this is a measurable per-message tax that scales the wrong way (worse over time as the user adds tools).What's wrong (technical)
The candidate set in
toolIdSetis already small — typicallytopK(default ~5) plus a fan-out factor. The SQL layer can do anIN (?, ?, ?)lookup in O(candidates × index lookup) instead of returning the entire table for Swift to discard.Fix
Add
ToolDatabase.loadEntries(ids: Set<String>)that issues:…with one placeholder per id. SQLite's default
SQLITE_MAX_VARIABLE_NUMBERis 999, comfortably above any plausible candidate count.Update
ToolSearchService.searchto useloadEntries(ids: toolIdSet)and keep only theenabledNamesfilter — which depends on MainActorToolRegistrystate and can't be expressed in SQL — applied in Swift.Same rows in the same order downstream; same
ToolSearchResultshape; same scoring. Only the cost of getting there changes.Tests
Three new cases in
ToolDatabaseTests:loadEntriesByIdsReturnsOnlyRequested— happy path with three rows, request two.loadEntriesByIdsEmptyInputSkipsQuery— emptySetreturns[]without issuing invalid SQL likeIN ().loadEntriesByIdsIgnoresUnknownIds— passing IDs that aren't in the table just drops them.Changes
ToolDatabaseTestscases)Test Plan
cd Packages/OsaurusCore && swift test --filter ToolDatabaseTestsshould pass.Checklist
CONTRIBUTING.mdswiftc -frontend -parse)