Conversation
…rtup Migration 020 created a memory_fts FTS5 virtual table and three triggers. SQLite distributions without FTS5 support (Node.js built-in SQLite, some Bun builds) fail the migration mid-way. Because the migration runs inside the Effect setup layer that is composed with Layer.provideMerge alongside makeRuntimeSqliteLayer, a migration failure causes the entire persistence layer construction to fail — SqlClient is never registered in the Effect context, so every downstream service reports the misleading error: Service not found: effect/sql/SqlClient Fix: - Drop the memory_fts virtual table, insert/delete/update triggers from migration 020 (no schema change is needed for already-run migrations since the tables were in the same transaction and will re-run cleanly on a fresh DB, and users who ran it successfully already have those tables harmlessly present) - Replace the FTS5 MATCH query in ProjectMemoryService.search with three LIKE predicates across title, content, and tags columns - Remove the FTS5 index rebuild from ProjectMemoryService.index (forceReindex becomes a no-op, which is fine for LIKE search) bun typecheck passes (7/7 packages). https://claude.ai/code/session_01Nxa3JfS5jZVHsnJmaaHvbW
📝 WalkthroughWalkthroughThe changes migrate from Full-Text Search (FTS5) implementation to a LIKE-based search pattern. The service replaces FTS5 querying with wildcard pattern matching on title, content, and tags fields, and removes the FTS virtual table and its three synchronization triggers from the database schema. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f40ece2ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| LIMIT ?`, | ||
| queryParams, | ||
| ); | ||
| const pattern = `%${input.query}%`; |
There was a problem hiding this comment.
Building pattern as %${input.query}% means % and _ in user input are treated as SQL wildcards, not literal characters. In this memory feature, queries often include code identifiers (for example user_id or % in snippets), so searches can return unrelated entries and evict relevant matches under LIMIT. Please escape wildcard characters (and use ESCAPE) before binding the pattern so literal queries behave predictably.
Useful? React with 👍 / 👎.
| END | ||
| `); | ||
|
|
||
| // ── Presence / Session Sharing ───────────────────────────────────── |
There was a problem hiding this comment.
This commit removes FTS DDL by editing migration 020 in place, which only affects fresh databases. Existing databases that already ran the previous 020 will keep memory_fts and its triggers indefinitely, but search no longer uses them; those triggers still execute on every memory_entries write and can preserve FTS5 schema dependencies in upgraded installs. A new migration should explicitly drop memory_fts and its triggers so upgraded environments converge to the new schema.
Useful? React with 👍 / 👎.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/memory/Services/ProjectMemoryService.ts (1)
175-182:⚠️ Potential issue | 🟠 MajorDon’t silently treat
forceReindexas success.
memoryIndexis still callable fromapps/server/src/ws.tsLines 1071-1073, so callers can request a rebuild and get a success-shaped response even though nothing happened. Also, the migration snippet inapps/server/src/persistence/Migrations/020_NewFeatureTables.tsLines 201-214 only stops creating FTS objects on fresh databases; it does not remove legacymemory_ftstables/triggers from upgraded ones. Please either add a forward migration to drop the old FTS artifacts and rejectforceReindexwith a domain-specific error, or remove that flag from the contract.As per coding guidelines, "Define domain-specific error types (e.g.,
ProviderAdapterError,OrchestrationDispatchError) in the server for proper error handling".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/memory/Services/ProjectMemoryService.ts` around lines 175 - 182, The index implementation currently treats the forceReindex flag as a no-op and returns a success-shaped response; update ProjectMemoryService.ts (the index function) to explicitly reject calls that set input.forceReindex by throwing/returning a new domain-specific error (e.g., MemoryReindexNotSupportedError) so callers like memoryIndex cannot assume a rebuild occurred, and add a forward migration (referencing 020_NewFeatureTables migration and the legacy memory_fts table/triggers) to drop legacy FTS artifacts on upgraded databases; ensure the new error type follows the server's domain-error pattern (similar to ProviderAdapterError) and is used in ws.ts call sites to surface a clear error instead of a silent success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/memory/Services/ProjectMemoryService.ts`:
- Around line 59-99: Escape SQL LIKE wildcards in the user query before building
the pattern: sanitize input.query by first replacing backslashes with double
backslashes, then replacing '%' with '\%' and '_' with '\_' (e.g., const escaped
= input.query.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_');
const pattern = `%${escaped}%`), and add an explicit ESCAPE '\' clause to both
SQL statements that use pattern (the two sql`... WHERE ... (title LIKE
${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) ...` branches in
ProjectMemoryService.ts) so the backslash escapes are honored.
---
Outside diff comments:
In `@apps/server/src/memory/Services/ProjectMemoryService.ts`:
- Around line 175-182: The index implementation currently treats the
forceReindex flag as a no-op and returns a success-shaped response; update
ProjectMemoryService.ts (the index function) to explicitly reject calls that set
input.forceReindex by throwing/returning a new domain-specific error (e.g.,
MemoryReindexNotSupportedError) so callers like memoryIndex cannot assume a
rebuild occurred, and add a forward migration (referencing 020_NewFeatureTables
migration and the legacy memory_fts table/triggers) to drop legacy FTS artifacts
on upgraded databases; ensure the new error type follows the server's
domain-error pattern (similar to ProviderAdapterError) and is used in ws.ts call
sites to surface a clear error instead of a silent success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e64ae1f2-2a75-49ba-b482-7e431ba9ec8f
📒 Files selected for processing (2)
apps/server/src/memory/Services/ProjectMemoryService.tsapps/server/src/persistence/Migrations/020_NewFeatureTables.ts
💤 Files with no reviewable changes (1)
- apps/server/src/persistence/Migrations/020_NewFeatureTables.ts
| const pattern = `%${input.query}%`; | ||
| const rows = yield* (input.kind | ||
| ? sql<{ | ||
| id: string; | ||
| project_id: string; | ||
| thread_id: string | null; | ||
| kind: string; | ||
| title: string; | ||
| content: string; | ||
| tags: string; | ||
| relevance_score: number; | ||
| access_count: number; | ||
| created_at: string; | ||
| updated_at: string; | ||
| expires_at: string | null; | ||
| }>`SELECT * FROM memory_entries | ||
| WHERE project_id = ${input.projectId} | ||
| AND kind = ${input.kind} | ||
| AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) | ||
| AND (expires_at IS NULL OR expires_at > datetime('now')) | ||
| ORDER BY relevance_score DESC | ||
| LIMIT ${input.limit}` | ||
| : sql<{ | ||
| id: string; | ||
| project_id: string; | ||
| thread_id: string | null; | ||
| kind: string; | ||
| title: string; | ||
| content: string; | ||
| tags: string; | ||
| relevance_score: number; | ||
| access_count: number; | ||
| created_at: string; | ||
| updated_at: string; | ||
| expires_at: string | null; | ||
| }>`SELECT * FROM memory_entries | ||
| WHERE project_id = ${input.projectId} | ||
| AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) | ||
| AND (expires_at IS NULL OR expires_at > datetime('now')) | ||
| ORDER BY relevance_score DESC | ||
| LIMIT ${input.limit}`); |
There was a problem hiding this comment.
Escape LIKE wildcards in the user query.
input.query is now fed straight into %...%, so % and _ inside real queries become SQL wildcards. That breaks common searches like snake_case and can turn % into “match everything”.
Suggested fix
- const pattern = `%${input.query}%`;
+ const escapedQuery = input.query.replace(/[\\%_]/g, "\\$&");
+ const pattern = `%${escapedQuery}%`;
@@
- AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern})
+ AND (
+ title LIKE ${pattern} ESCAPE '\\'
+ OR content LIKE ${pattern} ESCAPE '\\'
+ OR tags LIKE ${pattern} ESCAPE '\\'
+ )
@@
- AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern})
+ AND (
+ title LIKE ${pattern} ESCAPE '\\'
+ OR content LIKE ${pattern} ESCAPE '\\'
+ OR tags LIKE ${pattern} ESCAPE '\\'
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pattern = `%${input.query}%`; | |
| const rows = yield* (input.kind | |
| ? sql<{ | |
| id: string; | |
| project_id: string; | |
| thread_id: string | null; | |
| kind: string; | |
| title: string; | |
| content: string; | |
| tags: string; | |
| relevance_score: number; | |
| access_count: number; | |
| created_at: string; | |
| updated_at: string; | |
| expires_at: string | null; | |
| }>`SELECT * FROM memory_entries | |
| WHERE project_id = ${input.projectId} | |
| AND kind = ${input.kind} | |
| AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) | |
| AND (expires_at IS NULL OR expires_at > datetime('now')) | |
| ORDER BY relevance_score DESC | |
| LIMIT ${input.limit}` | |
| : sql<{ | |
| id: string; | |
| project_id: string; | |
| thread_id: string | null; | |
| kind: string; | |
| title: string; | |
| content: string; | |
| tags: string; | |
| relevance_score: number; | |
| access_count: number; | |
| created_at: string; | |
| updated_at: string; | |
| expires_at: string | null; | |
| }>`SELECT * FROM memory_entries | |
| WHERE project_id = ${input.projectId} | |
| AND (title LIKE ${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) | |
| AND (expires_at IS NULL OR expires_at > datetime('now')) | |
| ORDER BY relevance_score DESC | |
| LIMIT ${input.limit}`); | |
| const escapedQuery = input.query.replace(/[\\%_]/g, "\\$&"); | |
| const pattern = `%${escapedQuery}%`; | |
| const rows = yield* (input.kind | |
| ? sql<{ | |
| id: string; | |
| project_id: string; | |
| thread_id: string | null; | |
| kind: string; | |
| title: string; | |
| content: string; | |
| tags: string; | |
| relevance_score: number; | |
| access_count: number; | |
| created_at: string; | |
| updated_at: string; | |
| expires_at: string | null; | |
| }>`SELECT * FROM memory_entries | |
| WHERE project_id = ${input.projectId} | |
| AND kind = ${input.kind} | |
| AND ( | |
| title LIKE ${pattern} ESCAPE '\\' | |
| OR content LIKE ${pattern} ESCAPE '\\' | |
| OR tags LIKE ${pattern} ESCAPE '\\' | |
| ) | |
| AND (expires_at IS NULL OR expires_at > datetime('now')) | |
| ORDER BY relevance_score DESC | |
| LIMIT ${input.limit}` | |
| : sql<{ | |
| id: string; | |
| project_id: string; | |
| thread_id: string | null; | |
| kind: string; | |
| title: string; | |
| content: string; | |
| tags: string; | |
| relevance_score: number; | |
| access_count: number; | |
| created_at: string; | |
| updated_at: string; | |
| expires_at: string | null; | |
| }>`SELECT * FROM memory_entries | |
| WHERE project_id = ${input.projectId} | |
| AND ( | |
| title LIKE ${pattern} ESCAPE '\\' | |
| OR content LIKE ${pattern} ESCAPE '\\' | |
| OR tags LIKE ${pattern} ESCAPE '\\' | |
| ) | |
| AND (expires_at IS NULL OR expires_at > datetime('now')) | |
| ORDER BY relevance_score DESC | |
| LIMIT ${input.limit}`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/memory/Services/ProjectMemoryService.ts` around lines 59 -
99, Escape SQL LIKE wildcards in the user query before building the pattern:
sanitize input.query by first replacing backslashes with double backslashes,
then replacing '%' with '\%' and '_' with '\_' (e.g., const escaped =
input.query.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_');
const pattern = `%${escaped}%`), and add an explicit ESCAPE '\' clause to both
SQL statements that use pattern (the two sql`... WHERE ... (title LIKE
${pattern} OR content LIKE ${pattern} OR tags LIKE ${pattern}) ...` branches in
ProjectMemoryService.ts) so the backslash escapes are honored.
2 participants
Summary
Fixes the
Service not found: effect/sql/SqlClientruntime error reported when runningbun dev:server.Root cause: Migration 020 created a
memory_ftsFTS5 virtual table and three associated triggers. SQLite distributions compiled without FTS5 (Node.js built-in SQLite, some Bun builds) fail this statement. The migration runs insideLayer.provideMerge(setup, makeRuntimeSqliteLayer(...))— a failure insetupprevents the entire combined layer from registering, so SqlClient is never put into the Effect service context. Every downstream service then reports the misleadingService not found: effect/sql/SqlClient.Changes:
020_NewFeatureTables.ts— remove theCREATE VIRTUAL TABLE memory_fts USING fts5(...)and the three insert/delete/update triggers. Thememory_entriestable is untouched.ProjectMemoryService.ts— replace the FTS5MATCHquery insearchwithLIKEpredicates ontitle,content, andtagscolumns, ordered byrelevance_score. Remove the FTS5 index rebuild fromindex(forceReindexbecomes a no-op).Test plan
bun typecheckpasses (7/7 packages)bun lint— 0 errorsbun fmt --check— all files correctly formattedbun dev:server— noService not found: SqlClienterror on startuphttps://claude.ai/code/session_01Nxa3JfS5jZVHsnJmaaHvbW
Summary by cubic
Removes the FTS5 dependency and switches memory search to simple LIKE queries, fixing the
Service not found: effect/sql/SqlClienterror onbun dev:serverstartup.memory_ftsFTS5 virtual table and related triggers from020_NewFeatureTables.tsto avoid migration failures on SQLite builds without FTS5.MATCHinProjectMemoryService.searchwithLIKEontitle,content, andtags, ordered byrelevance_score.forceReindexis now a no-op.Written for commit 2f40ece. Summary will update on new commits.
Summary by CodeRabbit