🔍 Comprehensive Codebase Optimization & Efficiency Review#10
Merged
Conversation
|
Warning
|
| Layer / File(s) | Summary |
|---|---|
Review Configuration Settings .coderabbit.yaml |
Introduces complete CodeRabbit configuration with language (en-US), early_access: false, assertive review profile with request-changes workflow, high-level summaries, visible review status, collapsed walkthroughs; path_filters excluding build/output and lockfiles; path_instructions for src/**/*.ts (efficiency, async correctness, error-handling, memory leak checks, LOC reduction) and tests/**/*.ts (isolation, mock cleanup, behavior vs implementation guidance, parallelization); auto_review enabled for drafts targeting main; tools: markdownlint and github-checks enabled (90s timeout), shellcheck and ruff disabled; chat auto-reply enabled and Jira/Linear integrations disabled. |
🎯 1 (Trivial) | ⏱️ ~3 minutes
"I nibble on configs, tidy and bright,
A yaml carrot gleams in the night.
Rules set clean, filters in place,
Reviews awake with a prudent pace.
Hooray — small hops toward clearer sight!"
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The description is detailed and explains the PR's objective well, but does not follow the repository's required template structure with sections like Type of Change, Checklist, and Testing. | Use the repository's PR description template, including Type of Change, Checklist items, and Testing sections to align with project standards. | |
| Title check | ❓ Inconclusive | The title uses a vague, generic phrase ('Comprehensive Codebase Optimization & Efficiency Review') that doesn't clearly convey what the PR actually does: adding a .coderabbit.yaml configuration file. |
Consider a more specific title like 'Add CodeRabbit configuration for optimization review' that directly describes the changeset. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
refactor/efficiency-optimization
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Pull request overview
Adds a repository-level CodeRabbit configuration to enable automated, deep optimization-focused reviews (primarily targeting src/**/*.ts) while excluding build artifacts and lockfiles from review scope.
Changes:
- Introduces
.coderabbit.yamlwith review profile/settings (summary/status, request-changes workflow, collapsed walkthrough). - Adds path filters to exclude
dist/,node_modules/, lockfiles, and other non-source paths. - Adds per-path instructions for deep optimization review on
src/**/*.tsand test-efficiency review ontests/**/*.ts, plus enables relevant tooling checks (e.g.,markdownlint,github-checks).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.coderabbit.yaml:
- Around line 57-60: The yaml uses invalid enum values for the integration
"usage" keys: change the jira.usage and linear.usage entries (the "jira" and
"linear" objects and their "usage" properties) from "ignore" to a valid value
such as "auto" (or "disabled" if you want to explicitly turn them off); update
both jira.usage and linear.usage to fix the schema validation error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Owner
Author
|
@coderabbitai Please perform a comprehensive full-repository review of all source files in
Focus on:
Please provide:
Review the ENTIRE codebase, not just files in this PR diff. |
ZeR020
force-pushed
the
refactor/efficiency-optimization
branch
from
May 7, 2026 11:22
f98df15 to
d3d13f6
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
1-61: 🧹 Nitpick | 🔵 TrivialConfiguration is valid and well-structured for optimization reviews.
The
.coderabbit.yamlconfiguration successfully enables comprehensive optimization analysis with:
- Assertive profile for thorough feedback
- Request-changes workflow for enforcement
- Targeted path instructions covering efficiency, performance, and code quality
- Appropriate tool selection for TypeScript projects
- Path filters excluding build artifacts and dependencies
Note on review scope: This configuration applies when files matching the patterns are changed in a PR. To get a comprehensive review of the entire codebase as mentioned in the PR objectives, you can:
- Create PRs that touch the files you want reviewed (e.g., add/remove whitespace or comments), or
- Use CodeRabbit chat commands like
@coderabbitai review src/to trigger on-demand reviews of specific directories🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yaml around lines 1 - 61, The config is valid but only runs on changed files; to enable comprehensive reviews add an explicit path instruction for the whole codebase under path_instructions (e.g. add an entry with path: "src/**" or "src/**, tests/**" or a catch-all "path: '**/*'") so auto_review.enabled will evaluate those files even if they are not changed, by updating the path_instructions array in .coderabbit.yaml (adjust or append an entry) while keeping auto_review.enabled: true and your existing path_filters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.coderabbit.yaml:
- Around line 1-61: The config is valid but only runs on changed files; to
enable comprehensive reviews add an explicit path instruction for the whole
codebase under path_instructions (e.g. add an entry with path: "src/**" or
"src/**, tests/**" or a catch-all "path: '**/*'") so auto_review.enabled will
evaluate those files even if they are not changed, by updating the
path_instructions array in .coderabbit.yaml (adjust or append an entry) while
keeping auto_review.enabled: true and your existing path_filters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7223f90-1cd8-4249-bb47-0a4da813c5e1
📒 Files selected for processing (1)
.coderabbit.yaml
🧩 Analysis chainLet me first explore the full codebase structure before providing the review. 🏁 Script executed: Length of output: 53877 Now let me read the remaining service files. 🏁 Script executed: Length of output: 115115 🏁 Script executed: Length of output: 50499 🏁 Script executed: Length of output: 60287 🏁 Script executed: Length of output: 85192
Here’s a full-repository optimization and reliability review across the requested paths. I read all TypeScript sources under src/, including the hot paths you listed. Executive summary
Findings and recommendations P0. Web server can be exposed without auth (security)
Before
After - const requiresAuth = !localHosts.has(this.config.host) && !!this.config.enabled;
+ const requiresAuth = !localHosts.has(this.config.host);
+ if (requiresAuth && !this.config.apiKey) {
+ return this.jsonResponse({ success: false, error: "Server misconfigured: apiKey required when host is not localhost." }, 503);
+ }Estimated impact: High security improvement; 0 LOC net (≈+6). P0. Duplicate timeout paths in embeddings can leak work and stall
Before (embedWithTimeout) async embedWithTimeout(text: string): Promise<Float32Array> {
return withTimeout(this.embed(text), TIMEOUT_MS, undefined);
}After async embedWithTimeout(text: string, ms = TIMEOUT_MS, signal?: AbortSignal): Promise<Float32Array> {
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), ms);
try {
return await this.embed(text, controller.signal);
} finally {
clearTimeout(timeout);
}
}And adapt embed to accept a signal and pass it to fetch/pipeline once. P0. Vector search rebuilds indexes on every search path
Before await backend.rebuildFromShard({ db, shard, kind: "content" });
await backend.rebuildFromShard({ db, shard, kind: "tags" });After // Pseudo: guard by shard.vectorCount (or a hash)
if (!this._ready?.has(shard.id)) {
await backend.rebuildFromShard({ db, shard, kind: "content" });
await backend.rebuildFromShard({ db, shard, kind: "tags" });
(this._ready ||= new Set()).add(shard.id);
}Estimated impact: 5–20 ms shaved per search/shard after warmup (more on cold-start-heavy workloads). LOC: +12. Risk: Low (ensure updateVector/insertVector keep backend in sync, which they already do). P0. Global lock can silently skip conflict detection
Before if (isConflictCheckRunning) {
log("detectConflicts: skipping, another check is running");
return [];
}
isConflictCheckRunning = true;After (simple queue) const conflictQueue: Array<() => Promise<void>> = [];
let conflictBusy = false;
async function enqueueConflictCheck(job: () => Promise<void>) {
conflictQueue.push(job);
if (conflictBusy) return;
conflictBusy = true;
try {
while (conflictQueue.length) {
const j = conflictQueue.shift()!;
await j();
}
} finally {
conflictBusy = false;
}
}And wrap body of detectConflicts with enqueueConflictCheck. P0. Cleanup may delete active memories (uses updated_at only)
Before const oldMemories = db.prepare(`
SELECT id, container_tag, is_pinned FROM memories
WHERE updated_at < ?
`).all(cutoffTime);After -- Prefer last_accessed if present, else updated_at, else created_at
SELECT id, container_tag, is_pinned
FROM memories
WHERE COALESCE(last_accessed, updated_at, created_at) < ?
AND is_deprecated = 0And rename linkedMemoriesDeleted to linkedMemoriesSkipped in counters/return. P0. Batch DB writes for access_count updates
Before const updateAccessStmt = db.prepare(`UPDATE memories SET access_count = access_count + 1, last_accessed = ? WHERE id = ?`);
const now = Date.now();
for (const result of diverseResults) {
updateAccessStmt.run(now, result.id);
}After const now = Date.now();
db.run("BEGIN");
try {
const stmt = db.prepare("UPDATE memories SET access_count = access_count + 1, last_accessed = ? WHERE id = ?");
for (const r of diverseResults) stmt.run(now, r.id);
db.run("COMMIT");
} catch { db.run("ROLLBACK"); }Estimated impact: 3–10x fewer fsyncs per search set. Risk: Low. P1. Add FTS5 index for memories and fix FTS query
Schema addition (in initShardDb) CREATE VIRTUAL TABLE IF NOT EXISTS memories_fts USING fts5(
content, content='memories'
);
CREATE TRIGGER IF NOT EXISTS memories_fts_ai AFTER INSERT ON memories BEGIN
INSERT INTO memories_fts(rowid, content) VALUES (new.rowid, new.content);
END;
CREATE TRIGGER IF NOT EXISTS memories_fts_ad AFTER DELETE ON memories BEGIN
INSERT INTO memories_fts(memories_fts, rowid, content) VALUES ('delete', old.rowid, old.content);
END;
CREATE TRIGGER IF NOT EXISTS memories_fts_au AFTER UPDATE ON memories BEGIN
INSERT INTO memories_fts(memories_fts, rowid, content) VALUES ('delete', old.rowid, old.content);
INSERT INTO memories_fts(rowid, content) VALUES (new.rowid, new.content);
END;And fix the query in vector-search.ts: - SELECT id FROM memories_fts WHERE memories_fts MATCH ? ORDER BY rank LIMIT ?
+ SELECT m.id FROM memories_fts fts
+ JOIN memories m ON fts.rowid = m.rowid
+ WHERE memories_fts MATCH ?
+ ORDER BY rank LIMIT ?Estimated impact: 1.5–5x faster keyword recall vs LIKE; better hybrid search quality. LOC: +40. Risk: Low (FTS5 availability required). P1. Replace FIFO embed cache with true LRU and increase size
After (sketch) class LRU<K,V> { /* simple list+map or array splice for small sizes */ }
private cache = new LRU<string, Float32Array>(512);Or reinsert key on hit: const cached = this.cache.get(text);
if (cached) { this.cache.delete(text); this.cache.set(text, cached); return cached; }Estimated impact: Lower embed calls on repetitive patterns; + few LOC. Risk: Low. P1. Pass retrieval context end-to-end
Estimated impact: Better result ranking; LOC: +15. Risk: Low. P1. Clear timers on shutdown
After, inside shutdownHandler: for (const t of sessionIdleTimers.values()) clearTimeout(t);
sessionIdleTimers.clear();Estimated impact: Avoids dangling work after stop; LOC: +4. Risk: Low. P1. Deduplication: replace O(n^2) scanning with ANN-assisted candidate search
Before: nested for i..j. After for (const mem of uniqueMemories) {
const neighbors = await this.backend.search({ db, shard, kind: "content", queryVector: v1, limit: 50 });
for (const nb of neighbors) {
if (sameContainer && nb.id !== mem.id && sim >= threshold) { ... }
}
}Estimated impact: Significant CPU reduction on large shards; Risk: Medium (needs backend injection). P2. Remove unused/duplicated server worker
P2. Avoid warming embeddings for non-embed routes
P2. Modern micro-optimizations and cleanup
Database indexes (audit)
CREATE INDEX IF NOT EXISTS idx_memories_last_accessed ON memories(last_accessed);Impact: Faster cleanup; Low risk. Async patterns
Before/after snippets grouped by file
- async embedWithTimeout(text: string): Promise<Float32Array> {
- return withTimeout(this.embed(text), TIMEOUT_MS, undefined);
- }
+ async embedWithTimeout(text: string, ms = TIMEOUT_MS, signal?: AbortSignal): Promise<Float32Array> {
+ const controller = new AbortController();
+ const timeout = setTimeout(() => controller.abort(), ms);
+ try {
+ return await this.embed(text, controller.signal);
+ } finally {
+ clearTimeout(timeout);
+ }
+ }
- const cached = this.cache.get(text);
- if (cached) return cached;
+ const cached = this.cache.get(text);
+ if (cached) { this.cache.delete(text); this.cache.set(text, cached); return cached; }
- await backend.rebuildFromShard({ db, shard, kind: "content" });
- await backend.rebuildFromShard({ db, shard, kind: "tags" });
+ if (!this._ready?.has(shard.id)) {
+ await backend.rebuildFromShard({ db, shard, kind: "content" });
+ await backend.rebuildFromShard({ db, shard, kind: "tags" });
+ (this._ready ||= new Set()).add(shard.id);
+ }
...
- const ftsRows = ftsStmt.all(queryText, limit * 2) as any[];
- ftsResults = ftsRows.map((r: any) => r.id);
+ const ftsRows = db.prepare(`
+ SELECT m.id FROM memories_fts fts
+ JOIN memories m ON fts.rowid = m.rowid
+ WHERE memories_fts MATCH ?
+ ORDER BY rank LIMIT ?
+ `).all(queryText, limit * 2) as any[];
+ ftsResults = ftsRows.map((r: any) => r.id);
...
- const updateAccessStmt = db.prepare(
- `UPDATE memories SET access_count = access_count + 1, last_accessed = ? WHERE id = ?`
- );
- const now = Date.now();
- for (const result of diverseResults) {
- updateAccessStmt.run(now, result.id);
- }
+ const now = Date.now();
+ db.run("BEGIN");
+ try {
+ const updateAccessStmt = db.prepare(
+ `UPDATE memories SET access_count = access_count + 1, last_accessed = ? WHERE id = ?`
+ );
+ for (const r of diverseResults) updateAccessStmt.run(now, r.id);
+ db.run("COMMIT");
+ } catch { db.run("ROLLBACK"); }
- const oldMemories = db.prepare(`
- SELECT id, container_tag, is_pinned FROM memories
- WHERE updated_at < ?
- `).all(cutoffTime) as any[];
+ const oldMemories = db.prepare(`
+ SELECT id, container_tag, is_pinned FROM memories
+ WHERE COALESCE(last_accessed, updated_at, created_at) < ? AND is_deprecated = 0
+ `).all(cutoffTime) as any[];
...
- let linkedMemoriesDeleted = 0;
+ let linkedMemoriesSkipped = 0;
...
- if (protectedMemoryIds.has(memory.id)) {
- linkedMemoriesDeleted++;
- continue;
- }
+ if (protectedMemoryIds.has(memory.id)) {
+ linkedMemoriesSkipped++;
+ continue;
+ }
...
- const result = {
+ const result = {
deletedCount: totalDeleted,
userCount: userDeleted,
projectCount: projectDeleted,
promptsDeleted,
- linkedMemoriesDeleted,
+ linkedMemoriesSkipped,
pinnedMemoriesSkipped: pinnedSkipped,
};
const shutdownHandler = async () => {
try {
+ for (const t of sessionIdleTimers.values()) clearTimeout(t);
+ sessionIdleTimers.clear();
stopScoringRecalculation();
stopLifecycleJob();
AIProviderFactory.stopCleanupSchedule();Estimated impacts (rough)
Heavy dependencies and size
Dead code and LOC reduction opportunities
Async/await anti-patterns
Error handling gaps
Database query inefficiencies
Modern ES patterns
Would you like me to prepare a patch plan PR for the P0/P1 items above? I can break it into small, reviewable commits per subsystem (web, embeddings, vector search, sqlite schema, cleanup, conflicts, timers), with smoke tests for the changed SQL paths. ✅ Actions performedFull review triggered. |
Owner
Author
|
@coderabbitai Please perform a full-repository optimization and efficiency review, not just a PR diff review. The goal is to find optimization opportunities across the entire opencode-mem0 codebase before additional fixes are made. Please prioritize findings as P0/P1/P2 and include exact file/line references where possible. Focus areas:
Important: Please separate true optimization wins from risky micro-optimizations. Flag anything that could change behavior, data retention, privacy/security, memory lifecycle, or persisted schema semantics. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3227 🏁 Script executed: Length of output: 43188 🏁 Script executed: Length of output: 56496 🏁 Script executed: Length of output: 53217 🏁 Script executed: Length of output: 85696 🏁 Script executed: Length of output: 41274 🏁 Script executed: Length of output: 53938
Below is a full-repository optimization and reliability review across the requested paths with concrete file/line pointers, P0/P1/P2 priorities, before/after diffs, estimated impact, and risk notes. I focused on true wins first; micro-optimizations are called out separately. P0 — Fix now (security/correctness/hot-path reliability)
Before (Lines 231–244) After // src/services/web-server.ts
const localHosts = new Set(["127.0.0.1", "localhost", "::1", "[::1]"]);
const requiresAuth = !localHosts.has(this.config.host);
if (requiresAuth && !this.config.apiKey) {
return this.jsonResponse(
{ success: false, error: "Server misconfigured: apiKey required for non-local host" },
503
);
}
if (requiresAuth) {
const apiKey = req.headers.get("x-opencode-mem-key");
if (apiKey !== this.config.apiKey) return this.jsonResponse({ success: false, error: "Unauthorized" }, 401);
}
Before (key parts)
After // src/services/embedding.ts
- async embed(text: string): Promise<Float32Array> {
+ async embed(text: string, signal?: AbortSignal): Promise<Float32Array> {
...
- const abortController = new AbortController();
- const timeoutId = setTimeout(() => abortController.abort(), TIMEOUT_MS);
+ const abortController = new AbortController();
+ const onAbort = () => abortController.abort();
+ if (signal) signal.addEventListener("abort", onAbort);
try {
if (CONFIG.embeddingApiUrl && CONFIG.embeddingApiKey) {
const response = await fetch(`${CONFIG.embeddingApiUrl}/embeddings`, {
...
- signal: abortController.signal,
+ signal: abortController.signal,
});
...
} else {
const output = await this.pipe(text, { pooling: "mean", normalize: true });
result = new Float32Array(output.data);
}
} finally {
- clearTimeout(timeoutId);
+ if (signal) signal.removeEventListener("abort", onAbort);
}
- async embedWithTimeout(text: string): Promise<Float32Array> {
- return withTimeout(this.embed(text), TIMEOUT_MS, undefined);
- }
+ async embedWithTimeout(text: string, ms = TIMEOUT_MS): Promise<Float32Array> {
+ const controller = new AbortController();
+ const t = setTimeout(() => controller.abort(), ms);
+ try {
+ return await this.embed(text, controller.signal);
+ } finally {
+ clearTimeout(t);
+ }
+ }
After // src/services/sqlite/vector-search.ts
export class VectorSearch {
private readonly backendPromise = ...;
private readonly fallbackBackend = ...;
+ private readyByShard = new Map<number, number>(); // shardId -> vectorCount
async searchInShard( ... ) {
const db = connectionManager.getConnection(shard.dbPath);
const backend = await this.getBackend();
- await backend.rebuildFromShard({ db, shard, kind: "content" });
- await backend.rebuildFromShard({ db, shard, kind: "tags" });
+ const last = this.readyByShard.get(shard.id);
+ if (last !== shard.vectorCount) {
+ await backend.rebuildFromShard({ db, shard, kind: "content" });
+ await backend.rebuildFromShard({ db, shard, kind: "tags" });
+ this.readyByShard.set(shard.id, shard.vectorCount);
+ }
...
}
}
After const now = Date.now();
db.run("BEGIN");
try {
const updateAccessStmt = db.prepare(
`UPDATE memories SET access_count = access_count + 1, last_accessed = ? WHERE id = ?`
);
for (const r of diverseResults) updateAccessStmt.run(now, r.id);
db.run("COMMIT");
} catch (e) {
db.run("ROLLBACK");
log("Failed to update access count", { error: String(e) });
}
After // Select candidates using best-available recency field
const oldMemories = db.prepare(`
SELECT id, container_tag, is_pinned FROM memories
WHERE COALESCE(last_accessed, updated_at, created_at) < ?
AND is_deprecated = 0
`).all(cutoffTime) as any[];
...
- let linkedMemoriesDeleted = 0;
+ let linkedMemoriesSkipped = 0;
...
- if (protectedMemoryIds.has(memory.id)) { linkedMemoriesDeleted++; continue; }
+ if (protectedMemoryIds.has(memory.id)) { linkedMemoriesSkipped++; continue; }
...
return {
...,
- linkedMemoriesDeleted,
+ linkedMemoriesSkipped,
pinnedMemoriesSkipped: pinnedSkipped,
};
After // Top-level
- let isConflictCheckRunning = false;
+ const conflictQueue: Array<() => Promise<MemoryConflict[]>> = [];
+ let conflictBusy = false;
export async function detectConflicts(...) {
- if (isConflictCheckRunning) { log("...skipping..."); return []; }
- isConflictCheckRunning = true;
- try { ... } finally { isConflictCheckRunning = false; }
+ return new Promise<MemoryConflict[]>((resolve) => {
+ conflictQueue.push(async () => {
+ try { resolve(await runDetect()); } catch { resolve([]); }
+ });
+ if (!conflictBusy) drainQueue();
+ });
}
+
+ async function drainQueue() {
+ conflictBusy = true;
+ try { while (conflictQueue.length) await conflictQueue.shift()!(); }
+ finally { conflictBusy = false; }
+ }
+
+ async function runDetect(): Promise<MemoryConflict[]> {
+ // existing body of detectConflicts moved here (minus old guard)
+ }
P1 — Important improvements
Add to initShardDb (after memories table and indexes) CREATE VIRTUAL TABLE IF NOT EXISTS memories_fts USING fts5(
content, content='memories', tokenize='unicode61'
);
CREATE TRIGGER IF NOT EXISTS memories_fts_ai AFTER INSERT ON memories BEGIN
INSERT INTO memories_fts(rowid, content) VALUES (new.rowid, new.content);
END;
CREATE TRIGGER IF NOT EXISTS memories_fts_ad AFTER DELETE ON memories BEGIN
INSERT INTO memories_fts(memories_fts, rowid, content) VALUES ('delete', old.rowid, old.content);
END;
CREATE TRIGGER IF NOT EXISTS memories_fts_au AFTER UPDATE ON memories BEGIN
INSERT INTO memories_fts(memories_fts, rowid, content) VALUES ('delete', old.rowid, old.content);
INSERT INTO memories_fts(rowid, content) VALUES (new.rowid, new.content);
END;Fix query in vector-search.ts Lines 202–210 const ftsRows = db.prepare(`
SELECT m.id
FROM memories_fts fts
JOIN memories m ON fts.rowid = m.rowid
WHERE memories_fts MATCH ?
ORDER BY rank
LIMIT ?
`).all(queryText, limit * 2) as any[];
Before (excerpt) After // src/services/api-handlers.ts
// keep row and update vectors in-place
await vectorSearch.updateVector(db, id, vector, foundShard, tagsVector);
db.prepare(`UPDATE memories SET content = ?, tags = ?, type = ?, updated_at = ? WHERE id = ?`)
.run(newContent, tags.length ? tags.join(",") : null, data.type || existingMemory.type, Date.now(), id);
After (sketch) // inside for (const mem1 of uniqueMemories)
const buf = new Uint8Array(mem1.vector);
const v1 = new Float32Array(buf.buffer);
const neighbors = await vectorSearch.searchInShard(
shard, v1, mem1.container_tag, /*limit*/ 50
);
for (const nb of neighbors) {
if (nb.id === mem1.id) continue;
if (nb.similarity >= CONFIG.deduplicationSimilarityThreshold && nb.similarity < 1.0) {
similarGroup.duplicates.push({ id: nb.id, content: nb.memory, similarity: nb.similarity });
processedIds.add(nb.id);
}
}
After // src/services/tags.ts
const _gitCache = new Map<string, { email?: string; name?: string; url?: string; common?: string; top?: string }>();
export function getGitEmail(): string | null {
const k = "__global__"; const c = _gitCache.get(k) || {};
if (c.email) return c.email;
try { c.email = execSync("git config user.email", {...}).trim(); _gitCache.set(k, c); return c.email; } catch { return null; }
}
export function getGitRepoUrl(directory: string): string | null {
const c = _gitCache.get(directory) || {};
if (c.url) return c.url;
try { c.url = execSync("git config --get remote.origin.url", { cwd: directory, ... }).trim(); _gitCache.set(directory, c); return c.url; } catch { return null; }
}
After // src/index.ts in shutdownHandler
for (const t of sessionIdleTimers.values()) clearTimeout(t);
sessionIdleTimers.clear();
CREATE INDEX IF NOT EXISTS idx_last_accessed ON memories(last_accessed);
P2 — Nice-to-have and micro-optimizations (separate from true wins)
// top-level
const _TECH_PATTERNS = TECHNICAL_KEYWORDS.map(k => new RegExp(`\\b${k.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`, "gi"));
// inside calculateImportance:
for (const re of _TECH_PATTERNS) {
re.lastIndex = 0;
const matches = lowerContent.match(re);
if (matches) keywordMatches += matches.length;
}
Notes on dynamic SQL vs. prepared
Summary of expected impact
Would you like me to craft a step-by-step patch plan (split by subsystem: web, embeddings, vector/SQLite, lifecycle/cleanup, conflicts, dedupe) and stage it as follow-up commits? I can keep API changes minimal (e.g., keep both linkedMemoriesDeleted and linkedMemoriesSkipped fields for one release to avoid breaking consumers). ✏️ Learnings added
|
Owner
Author
|
@coderabbitai Please do another full-repository scan, not only this PR diff. Focus on whether opencode-mem0 still works as promised and delivers its documented behavior: persistent memory, STM/LTM lifecycle, hybrid search, transcript storage, web UI, cross-platform support, AI provider/session handling, migration safety, privacy/security, and cleanup/deduplication. Prioritize bugs, correctness regressions, concurrency/race issues, data-loss risks, security/auth risks, and meaningful efficiency optimizations. Please classify findings as P0/P1/P2 with exact file:line references and explain whether each is a correctness fix, security fix, or performance optimization. Avoid speculative micro-optimizations unless there is clear runtime impact or reduced risk. |
|
Oops, something went wrong! Please try again later. 🐰 💔 |
This was referenced May 7, 2026
ZeR020
added a commit
that referenced
this pull request
Jun 30, 2026
…late dedup
Dead code removed (graph-confirmed zero callers):
- ensureShardTables, getShardByPath (shard-manager.ts)
- countVectors(containerTag) (vector-search.ts)
- getMessagesByRoleStmt prepared statement + field (ai-session-manager.ts)
- maxProfileItems config field + docs + test ref
- 6 duplicate FTS5 triggers in migrate-v1-to-v2.ts
- 6 dead i18n keys (en + zh)
- sessionIdleSweep speculative 10K-entry LRU guard (index.ts)
Stdlib replacements:
- Hand-rolled UserProfileValidator (107ln) → zod schema with matching error messages
- 3× dynamic `await import("zod")` → static top-level imports
Shrinks:
- fetchWithTimeout() + apiErrorResponse() hoisted to BaseAIProvider
- All 4 providers refactored to use shared HTTP boilerplate
- plugin.ts IIFE wrapper flattened
- Obsolete marked.setOptions headerIds/mangle removed (marked v8+)
- Broken lint-deepsource.sh rules #1 (awk) and #10 (complexity) fixed
22 files changed, net -92 lines (-434/+342)
All 730 tests + typecheck + build + format:check + lint-deepsource pass
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.
Objective
Requesting a full-repository optimization scan by CodeRabbit to identify opportunities for making this codebase more efficient, faster, cleaner, and maintainable while reducing overall lines of code.
Focus Areas
Performance & Efficiency
Code Quality & Maintainability
Architecture & Design
Bundle & Runtime Size
Scope
Please review the entire
src/directory including:index.ts)Exclude:
dist/,node_modules/,tests/(unless test optimization suggestions), generated files.Definition of Done
CodeRabbit should provide:
Note: This PR adds
.coderabbit.yamlto configure deep review settings. The actual optimization changes will be implemented in follow-up PRs based on CodeRabbit's recommendations.Summary by CodeRabbit