-
Notifications
You must be signed in to change notification settings - Fork 2
fix(sqlite): transactional integrity, FTS5 for memories, decay rotation #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
77e8b13
e927768
0f5ac44
b4c186d
c1ab06d
680d590
4dc59c3
86585c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,12 +176,15 @@ class MigrationService { | |
| total: mismatch.shardMismatches.length, | ||
| }); | ||
|
|
||
| const expected = mismatch.shardMismatches.length; | ||
| const success = deletedShards === expected; | ||
| return { | ||
| success: true, | ||
| success, | ||
| strategy: "fresh-start", | ||
| deletedShards, | ||
| reEmbeddedMemories: 0, | ||
| duration: Date.now() - startTime, | ||
| ...(success ? {} : { error: "Failed to delete one or more shards" }), | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -223,41 +226,12 @@ class MigrationService { | |
| memory: any, | ||
| processedCount: number, | ||
| totalMemories: number, | ||
| shardId: string | ||
| shardId: string, | ||
| db: ReturnType<typeof connectionManager.getConnection> | ||
| ): Promise<{ success: boolean; processedCount: number }> { | ||
| try { | ||
| const vector = await embeddingService.embedWithTimeout(memory.content); | ||
| const scope = memory.containerTag.includes("_user_") ? "user" : "project"; | ||
| const hash = memory.containerTag.split("_").slice(2).join("_"); | ||
| const newShard = shardManager.getWriteShard(scope, hash); | ||
| const newDb = connectionManager.getConnection(newShard.dbPath); | ||
|
|
||
| await vectorSearch.insertVector( | ||
| newDb, | ||
| { | ||
| id: memory.id, | ||
| content: memory.content, | ||
| vector, | ||
| containerTag: memory.containerTag, | ||
| type: memory.type || undefined, | ||
| createdAt: memory.createdAt, | ||
| updatedAt: memory.updatedAt, | ||
| metadata: memory.metadata || undefined, | ||
| displayName: memory.displayName || undefined, | ||
| userName: memory.userName || undefined, | ||
| userEmail: memory.userEmail || undefined, | ||
| projectPath: memory.projectPath || undefined, | ||
| projectName: memory.projectName || undefined, | ||
| gitRepoUrl: memory.gitRepoUrl || undefined, | ||
| }, | ||
| newShard | ||
| ); | ||
|
|
||
| if (memory.isPinned === 1) { | ||
| vectorSearch.pinMemory(newDb, memory.id); | ||
| } | ||
|
|
||
| shardManager.incrementVectorCount(newShard.id); | ||
| await vectorSearch.updateVector(db, memory.id, vector); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Re-embedding leaves live searches stale After an index serves a search, re-embedding calls Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above — fixed in #66 (
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Re-embedding deletes tag embeddings Each migrated memory calls Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in #66 ( |
||
| const nextCount = processedCount + 1; | ||
|
Comment on lines
232
to
235
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate of the Codex/Devin finding — fixed in #66 ( |
||
|
|
||
| this.reportProgress({ | ||
|
|
@@ -291,7 +265,7 @@ class MigrationService { | |
|
|
||
| let reEmbeddedCount = 0; | ||
| let processedCount = 0; | ||
| let deletedShards = 0; | ||
| let shardHadFailures = false; | ||
|
|
||
| for (const shardInfo of mismatch.shardMismatches) { | ||
| this.reportProgress({ | ||
|
|
@@ -307,32 +281,41 @@ class MigrationService { | |
| // Default 10000 is too low for shards with large memory counts. | ||
| const memories = vectorSearch.getAllMemories(db, 1_000_000); | ||
| const tempMemories = this._backupMemories(memories); | ||
| let shardHadFailures = false; | ||
| let thisShardFailed = false; | ||
|
|
||
| for (const memory of tempMemories) { | ||
| const result = await this._reEmbedSingleMemory( | ||
| memory, | ||
| processedCount, | ||
| totalMemories, | ||
| String(shardInfo.shardId) | ||
| String(shardInfo.shardId), | ||
| db | ||
| ); | ||
| processedCount = result.processedCount; | ||
| if (result.success) { | ||
| reEmbeddedCount++; | ||
| } else { | ||
| thisShardFailed = true; | ||
| shardHadFailures = true; | ||
| } | ||
| } | ||
|
|
||
| if (!shardHadFailures) { | ||
| await shardManager.deleteShard(shardInfo.shardId); | ||
| deletedShards++; | ||
| if (!thisShardFailed) { | ||
| db.run("INSERT OR REPLACE INTO shard_metadata (key, value) VALUES (?, ?)", [ | ||
| "embedding_dimensions", | ||
| String(CONFIG.embeddingDimensions), | ||
| ]); | ||
| db.run("INSERT OR REPLACE INTO shard_metadata (key, value) VALUES (?, ?)", [ | ||
| "embedding_model", | ||
| CONFIG.embeddingModel, | ||
| ]); | ||
| } else { | ||
| log("Migration: keeping original shard due to re-embedding failures", { | ||
| shardId: shardInfo.shardId, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| shardHadFailures = true; | ||
| log("Migration: error processing shard", { | ||
| shardId: shardInfo.shardId, | ||
| error: String(error), | ||
|
|
@@ -346,12 +329,14 @@ class MigrationService { | |
| total: totalMemories, | ||
| }); | ||
|
|
||
| const success = !shardHadFailures && reEmbeddedCount === totalMemories; | ||
| return { | ||
| success: true, | ||
| success, | ||
| strategy: "re-embed", | ||
| deletedShards, | ||
| deletedShards: 0, | ||
| reEmbeddedMemories: reEmbeddedCount, | ||
| duration: Date.now() - startTime, | ||
| ...(success ? {} : { error: "One or more memories failed to re-embed" }), | ||
| }; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a live migration to a different model with the same embedding dimensions, this call omits
shard, soupdateVectorupdates only SQLite and never marks the shard rebuild-dirty or updates the native index. If the default USearch index was initialized by an earlier search,maybeRebuildskips it andUSearchBackend.rebuildFromShardalso preserves initialized indexes, so subsequent searches use the old embeddings even though the shard metadata reports the new model.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #66 (
1ec9af5): the migration now resolves full ShardInfo per dbPath and passes it toupdateVector, re-embeds the tags text with the capture-time prompt shape, and marks the shard dirty so the next search force-replaces any initialized index with new-dimension data. Regression-asserted via amarkShardDirtyspy intests/reembed-honesty.test.ts.