-
Notifications
You must be signed in to change notification settings - Fork 260
fix(cli): surface reindex embedding failures and index identity #1240
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
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 |
|---|---|---|
|
|
@@ -26,7 +26,10 @@ | |
| from basic_memory.repository.search_query import relaxed_query_words | ||
| from basic_memory.schemas.base import normalize_note_type | ||
| from basic_memory.schemas.search import SearchQuery, SearchItemType, SearchRetrievalMode | ||
| from basic_memory.runtime.vector_sync import VectorSyncBatchResult | ||
| from basic_memory.runtime.vector_sync import ( | ||
| VECTOR_SYNC_SAMPLE_ERROR_LIMIT, | ||
| VectorSyncBatchResult, | ||
| ) | ||
| from basic_memory.services import FileService | ||
|
|
||
| # Maximum size for content_stems field to stay under Postgres's 8KB index row limit. | ||
|
|
@@ -516,11 +519,7 @@ async def sync_entity_vectors_batch( | |
| ) -> VectorSyncBatchResult: | ||
| """Refresh vector chunks for a batch of entities.""" | ||
| if not entity_ids: | ||
| return VectorSyncBatchResult( | ||
| entities_total=0, | ||
| entities_synced=0, | ||
| entities_failed=0, | ||
| ) | ||
| return await self.repository.sync_entity_vectors_batch([]) | ||
|
Comment on lines
521
to
+522
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.
When Useful? React with 👍 / 👎. |
||
|
|
||
| async with db.scoped_session(self.session_maker) as session: | ||
| entities_by_id = { | ||
|
|
@@ -550,31 +549,19 @@ async def sync_entity_vectors_batch( | |
| cleanup_task = ( | ||
| self.repository.sync_entity_vectors_batch(unknown_ids) if unknown_ids else None | ||
| ) | ||
| eligible_task = ( | ||
| self.repository.sync_entity_vectors_batch( | ||
| eligible_entity_ids, | ||
| progress_callback=progress_callback, | ||
| ) | ||
| if eligible_entity_ids | ||
| else None | ||
| eligible_task = self.repository.sync_entity_vectors_batch( | ||
| eligible_entity_ids, | ||
| progress_callback=progress_callback, | ||
| ) | ||
| repository_results = [ | ||
| result | ||
| for result in await asyncio.gather( | ||
| cleanup_task if cleanup_task is not None else asyncio.sleep(0, result=None), | ||
| eligible_task if eligible_task is not None else asyncio.sleep(0, result=None), | ||
| eligible_task, | ||
| ) | ||
| if result is not None | ||
| ] | ||
|
|
||
| if not repository_results: | ||
| return VectorSyncBatchResult( | ||
| entities_total=len(entity_ids), | ||
| entities_synced=0, | ||
| entities_failed=0, | ||
| entities_skipped=len(opted_out_ids), | ||
| ) | ||
|
|
||
| batch_result = VectorSyncBatchResult( | ||
| entities_total=len(entity_ids), | ||
| entities_synced=sum(result.entities_synced for result in repository_results), | ||
|
|
@@ -590,6 +577,25 @@ async def sync_entity_vectors_batch( | |
| for result in repository_results | ||
| for failed_entity_id in result.failed_entity_ids | ||
| ), | ||
| sample_errors=tuple( | ||
| dict.fromkeys( | ||
| error | ||
| for result in repository_results | ||
| for error in result.sample_errors | ||
| ) | ||
| )[:VECTOR_SYNC_SAMPLE_ERROR_LIMIT], | ||
| vector_index=next( | ||
| (result.vector_index for result in repository_results if result.vector_index), | ||
| "", | ||
| ), | ||
| embedding_model=next( | ||
| ( | ||
| result.embedding_model | ||
| for result in repository_results | ||
| if result.embedding_model | ||
| ), | ||
| "", | ||
| ), | ||
| chunks_total=sum(result.chunks_total for result in repository_results), | ||
| chunks_skipped=sum(result.chunks_skipped for result in repository_results), | ||
| embedding_jobs_total=sum(result.embedding_jobs_total for result in repository_results), | ||
|
|
@@ -616,7 +622,7 @@ async def reindex_vectors( | |
| eligible entity re-embeds from scratch. | ||
|
|
||
| Returns: | ||
| dict with stats: total_entities, embedded, skipped, errors | ||
| dict with counts, sampled errors, and the active vector index/model identity | ||
| """ | ||
| async with db.scoped_session(self.session_maker) as session: | ||
| entities = await self.entity_repository.find_all(session) | ||
|
|
@@ -638,6 +644,9 @@ async def reindex_vectors( | |
| "embedded": batch_result.entities_synced, | ||
| "skipped": batch_result.entities_skipped, | ||
| "errors": batch_result.entities_failed, | ||
| "sample_errors": batch_result.sample_errors, | ||
| "vector_index": batch_result.vector_index, | ||
| "embedding_model": batch_result.embedding_model, | ||
| } | ||
|
|
||
| for failed_entity_id in batch_result.failed_entity_ids: | ||
|
|
||
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.
When a selected project contains any opted-out or already-current entity,
total_entitiesincludes it whileerrorsdoes not, so every entity that actually required embedding can fail and this comparison still exits 0. For example, oneembed: falsenote plus one eligible note that hits the manifest-ownership guard yieldstotal_entities=2,skipped=1, anderrors=1, followed byReindex complete!; base the failure decision on entities that required vector work rather than all database entities.AGENTS.md reference: AGENTS.md:L132-L133
Useful? React with 👍 / 👎.