perf: eliminate N+1 queries across recall, graph BFS, rooms, and edge insertion - #987
Merged
Conversation
…d edge insertion Store layer (new batch methods): - crud.rs: get_by_ids() — single SELECT ... WHERE id IN (...) using rusqlite params_from_iter - aging.rs: touch_access_batch() — single UPDATE with CASE WHEN for batch access tracking Consumer fixes (6 N+1 hotspots eliminated): - operations.rs recall(): get_by_id per candidate → batch get_by_ids + HashMap lookup - operations.rs 4× touch_access loops → single touch_access_batch call - rooms.rs room_recall(): get_by_id per missing ID → batch get_by_ids - graph.rs BFS depth traversal: 2× get_by_id per level → batch fetch frontier + batch fetch targets Edge insertion optimization: - edges.rs add_memory_edges_batch(): prepare() once outside loop, reuse stmt for all inserts Test: 476 pass, 0 fail. Clippy: 0 warnings.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
🔍 Cora AI Code Review✅ No issues found. Code looks good! Review powered by cora-code · BYOK · MIT |
… same level Cora review finding: when two source nodes in the same BFS frontier level both reference the same target, the target was added to rel_chains twice. After batch fetch, both entries would insert the target into results and next_frontier, causing duplicate entries and wasted traversal. Fix: check visited set before processing each rel_chain entry.
| visited.insert(target_id.clone()); | ||
| let decayed_score = (results[source_id].1 * 0.8).max(0.1); | ||
| results | ||
| .insert(target_id.clone(), ((*target_memory).clone(), decayed_score)); |
| return Ok(Vec::new()); | ||
| } | ||
| let placeholders = ids.iter().map(|_| "?").collect::<Vec<_>>().join(","); | ||
| let sql = format!( |
Cora review finding: SQLite has SQLITE_MAX_VARIABLE_NUMBER=999 default. If ids contains >999 entries, the WHERE id IN (?,...) query would fail. Fix: chunk IDs into batches of 900, execute separate query per chunk, merge results.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
| // Skip if already visited — multiple source nodes in the same | ||
| // frontier level can reference the same target (dedup). | ||
| if visited.contains(target_id.as_str()) { | ||
| continue; |
…e target Cora review: first-source-wins was non-deterministic and could lose a better score from a later source. Now tracks max score across all sources in the same frontier level before committing to results.
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.
What
Eliminates all N+1 SQL query patterns identified by subagent analysis across 6 hotspots in uteke-core.
Why
Every
recall(),room_recall(), and graph BFS traversal was executing 1 SQL query per candidate — a classic N+1. Withk = limit * 3 * 9 = 27×limitcandidates per recall attempt, this meant up to 27 individualSELECTqueries per search. Batch-fetching eliminates this to a single query.Changes
New batch methods (Store layer):
Store::get_by_ids(&[&str])— singleSELECT ... WHERE id IN (...)usingparams_from_iterStore::touch_access_batch(&[&str])— singleUPDATEwithCASE WHENfor batch access trackingN+1 consumers fixed (7 locations):
operations.rsrecall()get_by_id()per candidate (k up to 27×limit)get_by_ids()+ HashMap lookupoperations.rs4× touch_accessUPDATEstouch_access_batch()rooms.rsroom_recall()get_by_id()per missing IDget_by_ids()batchgraph.rsBFSget_by_id()per depth level (frontier + targets)Edge insertion:
edges.rsadd_memory_edges_batch():prepare()once outside loop → reuse prepared statement for all inserts + backlinksTesting
cargo test --workspace: 476 pass, 0 fail, 25 ignoredcargo clippy --workspace --all-targets: 0 warnings