Skip to content

Commit ab0822d

Browse files
committed
fix(core): address PR feedback regressions
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 83de1af commit ab0822d

15 files changed

Lines changed: 473 additions & 19 deletions

justfile

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,15 @@ index-contract-test: file-index-runner-test file-index-batch-test file-index-sem
322322
runtime-core-pytest *args:
323323
BASIC_MEMORY_ENV=test uv run pytest -p pytest_mock -q --no-cov {{args}}
324324

325+
# Focused PR #1002 Codex feedback regressions.
326+
pr-1002-feedback-test:
327+
BASIC_MEMORY_ENV=test BASIC_MEMORY_SEMANTIC_SEARCH_ENABLED=true LOGFIRE_IGNORE_NO_CONFIG=1 uv run pytest -p pytest_mock -q --no-cov \
328+
tests/runtime/test_deleted_note_response.py \
329+
tests/repository/test_accepted_note_search_repository.py \
330+
tests/indexing/test_project_index_workflow.py \
331+
tests/indexing/test_accepted_note_write_runner.py \
332+
tests/indexing/test_directory_delete_runner.py
333+
325334
runtime-core-fast-check-no-openai:
326335
OPENAI_API_KEY= just fast-check
327336

src/basic_memory/index/local_dependencies.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
BatchIndexer,
1818
CurrentMaterializedNoteEntityRepository,
1919
DefaultIndexBatchRuntime,
20+
EmbeddingIndexBatchSummary,
2021
FileIndexOperation,
2122
FileIndexResult,
2223
IndexFileBatchIndexer,
@@ -116,6 +117,11 @@ class LocalIndexSearchService(
116117
):
117118
"""Search capabilities needed by local event/project indexing."""
118119

120+
async def sync_entity_vectors_batch(
121+
self,
122+
entity_ids: list[int],
123+
) -> EmbeddingIndexBatchSummary: ...
124+
119125

120126
class LocalIndexEntityService(Protocol):
121127
"""Entity service capabilities needed by local index maintenance adapters."""

src/basic_memory/index/local_project.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
)
2929
from basic_memory.indexing import (
3030
ChangeDetector,
31+
EmbeddingBatchVectorSync,
3132
FileIndexChecker,
3233
IndexFileBatchJobResult,
3334
IndexFileBatchChecker,
@@ -165,6 +166,7 @@ class LocalProjectIndexRuntime:
165166
workflow_starter: ProjectIndexWorkflowStarter | None = None
166167
fanout_failure_recorder: ProjectIndexFanoutFailureRecorder | None = None
167168
completion_relation_runtime: RelationResolutionRuntime | None = None
169+
embedding_vector_sync: EmbeddingBatchVectorSync | None = None
168170
batch_size: int = 100
169171
coordinator_job_id: RuntimeJobId | None = None
170172

@@ -188,6 +190,16 @@ class LocalProjectIndexRuntimeProvider(Protocol):
188190
async def runtime_for_project(self, project: Project) -> LocalProjectIndexRuntime: ...
189191

190192

193+
def local_project_embedding_vector_sync(
194+
dependencies: LocalIndexProjectDependencies,
195+
) -> EmbeddingBatchVectorSync | None:
196+
"""Return the semantic vector sync backend when local config enables it."""
197+
app_config = dependencies.entity_service.app_config
198+
if app_config is None or not app_config.semantic_search_enabled:
199+
return None
200+
return dependencies.search_service
201+
202+
191203
@dataclass(frozen=True, slots=True)
192204
class LocalIndexFileBatchReader(IndexFileBatchReader[IndexInputFile]):
193205
"""Load current local files for the shared index-file batch runner."""
@@ -338,6 +350,7 @@ def runtime_from_dependencies(
338350
link_resolver=dependencies.link_resolver,
339351
entity_indexer=dependencies.search_service,
340352
),
353+
embedding_vector_sync=local_project_embedding_vector_sync(dependencies),
341354
batch_size=self.batch_size,
342355
)
343356

@@ -416,6 +429,7 @@ async def run_local_project_index(
416429
batch_enqueuer=runtime.batch_enqueuer,
417430
fanout_failure_recorder=runtime.fanout_failure_recorder,
418431
batch_size=runtime.batch_size,
432+
embedding_vector_sync=runtime.embedding_vector_sync,
419433
)
420434
if runtime.completion_relation_runtime is not None:
421435
await resolve_project_index_completion_relations(

src/basic_memory/indexing/accepted_note_write_runner.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,12 @@ async def delete_entity(
252252
entity_id: RuntimeEntityId,
253253
) -> None: ...
254254

255+
async def delete_entity_vectors(
256+
self,
257+
session: AsyncSession,
258+
entity_id: RuntimeEntityId,
259+
) -> None: ...
260+
255261

256262
class AcceptedNoteWriteRepositories(Protocol):
257263
"""Repository capability set needed by accepted-note DB-first writes."""
@@ -646,6 +652,19 @@ async def delete_accepted_note_search_index(
646652
await repository.delete_entity(session, entity_id)
647653

648654

655+
async def delete_accepted_note_vectors(
656+
session: AsyncSession,
657+
*,
658+
project_id: ProjectId,
659+
entity_id: RuntimeEntityId,
660+
repositories: AcceptedNoteWriteRepositories | None = None,
661+
) -> None:
662+
"""Remove semantic vectors for an accepted-note entity inside the caller's transaction."""
663+
write_repositories = repositories or build_default_accepted_note_write_repositories()
664+
repository = write_repositories.search_repository(project_id)
665+
await repository.delete_entity_vectors(session, entity_id)
666+
667+
649668
async def persist_accepted_note_write(
650669
session: AsyncSession,
651670
*,
@@ -721,5 +740,11 @@ async def delete_accepted_note(
721740
entity_id=entity.id,
722741
repositories=repositories,
723742
)
743+
await delete_accepted_note_vectors(
744+
session,
745+
project_id=project_id,
746+
entity_id=entity.id,
747+
repositories=repositories,
748+
)
724749
await session.delete(entity)
725750
return accepted

src/basic_memory/indexing/directory_delete_runner.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from sqlalchemy import bindparam, delete, select, text
1111
from sqlalchemy.ext.asyncio import AsyncSession
1212

13+
from basic_memory.indexing.project_index_workflow import delete_project_index_vector_rows
1314
from basic_memory.models import Entity, NoteContent, Project
1415
from basic_memory.runtime import (
1516
RuntimeDirectoryFileSnapshot,
@@ -196,6 +197,11 @@ async def delete_directory_entities(
196197
).bindparams(bindparam("entity_ids", expanding=True)),
197198
{"project_id": project_id, "entity_ids": tuple(entity_ids)},
198199
)
200+
await delete_project_index_vector_rows(
201+
session,
202+
project_id=project_id,
203+
entity_ids=tuple(entity_ids),
204+
)
199205
await session.execute(delete(Entity).where(Entity.id.in_(entity_ids)))
200206

201207

src/basic_memory/indexing/project_index_workflow.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
project_index_file_outcome_from_job_result,
1818
)
1919
from basic_memory.indexing.change_planning import ChangeReport
20+
from basic_memory.indexing.embedding_index_planning import (
21+
EmbeddingBatchVectorSync,
22+
EmbeddingIndexBatchJobRequest,
23+
run_embedding_index_batch,
24+
)
2025
from basic_memory.indexing.project_index_progress import (
2126
ProjectIndexCompletion,
2227
ProjectIndexCounters,
@@ -1466,6 +1471,7 @@ async def run_project_index_coordinator(
14661471
batch_enqueuer: ProjectIndexBatchEnqueuer,
14671472
fanout_failure_recorder: ProjectIndexFanoutFailureRecorder | None,
14681473
batch_size: int,
1474+
embedding_vector_sync: EmbeddingBatchVectorSync | None = None,
14691475
) -> ProjectIndexCoordinatorResult:
14701476
"""Run the storage-neutral project-index coordinator fan-out."""
14711477
if not request.search:
@@ -1537,6 +1543,12 @@ async def run_project_index_coordinator(
15371543
)
15381544
raise
15391545

1546+
await sync_project_index_vector_targets(
1547+
request=request,
1548+
batch_results=batch_results,
1549+
embedding_vector_sync=embedding_vector_sync,
1550+
)
1551+
15401552
return ProjectIndexCoordinatorResult(
15411553
total_files=len(observed_files),
15421554
enqueued_files=enqueued_files,
@@ -1551,6 +1563,34 @@ async def run_project_index_coordinator(
15511563
)
15521564

15531565

1566+
async def sync_project_index_vector_targets(
1567+
*,
1568+
request: RuntimeProjectIndexJobRequest,
1569+
batch_results: Sequence[IndexFileBatchJobResult],
1570+
embedding_vector_sync: EmbeddingBatchVectorSync | None,
1571+
) -> None:
1572+
"""Refresh vectors produced by inline project-index batch execution."""
1573+
if not request.embeddings or embedding_vector_sync is None:
1574+
return
1575+
1576+
vector_targets = tuple(
1577+
target for batch_result in batch_results for target in batch_result.vector_targets
1578+
)
1579+
if not vector_targets:
1580+
return
1581+
1582+
await run_embedding_index_batch(
1583+
EmbeddingIndexBatchJobRequest(
1584+
tenant_id=request.tenant_id,
1585+
project_id=request.project.project_id,
1586+
project_path=request.project.project_path,
1587+
entities=vector_targets,
1588+
workflow_id=request.workflow_id,
1589+
),
1590+
vector_sync=embedding_vector_sync,
1591+
)
1592+
1593+
15541594
def build_project_index_batch_activity_update(
15551595
*,
15561596
metadata: Mapping[str, object],

src/basic_memory/repository/accepted_note_search_repository.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from sqlalchemy.ext.asyncio import AsyncSession
1010

1111
from basic_memory.indexing.accepted_note_search import AcceptedNoteSearchRow
12+
from basic_memory.indexing.project_index_workflow import delete_project_index_vector_rows
1213

1314
type SearchIndexSqlValue = str | int | datetime | None
1415
type SearchIndexSqlParams = dict[str, SearchIndexSqlValue]
@@ -31,7 +32,7 @@
3132
project_id
3233
) VALUES (
3334
:id, :title, :content_stems, :content_snippet, :permalink, :file_path, :type,
34-
CAST(:metadata AS jsonb),
35+
:metadata,
3536
NULL, NULL, NULL,
3637
:entity_id, NULL,
3738
:created_at, :updated_at,
@@ -139,3 +140,15 @@ async def delete_entity(
139140
DELETE_ACCEPTED_NOTE_SEARCH_SQL,
140141
{"entity_id": entity_id, "project_id": self.project_id},
141142
)
143+
144+
async def delete_entity_vectors(
145+
self,
146+
session: AsyncSession,
147+
entity_id: int,
148+
) -> None:
149+
"""Delete semantic vector rows for one accepted-note entity."""
150+
await delete_project_index_vector_rows(
151+
session,
152+
project_id=self.project_id,
153+
entity_ids=(entity_id,),
154+
)

src/basic_memory/runtime/contracts.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,9 +1017,8 @@ def from_entity(
10171017
field_name="title",
10181018
file_path=file_path,
10191019
),
1020-
permalink=required_runtime_deleted_note_text(
1020+
permalink=runtime_deleted_note_permalink(
10211021
entity.permalink,
1022-
field_name="permalink",
10231022
file_path=file_path,
10241023
),
10251024
)
@@ -1108,6 +1107,21 @@ def required_runtime_deleted_note_text(
11081107
return value.strip()
11091108

11101109

1110+
def runtime_deleted_note_permalink(
1111+
value: object,
1112+
*,
1113+
file_path: RuntimeFilePath,
1114+
) -> str:
1115+
"""Return deleted-note permalink text, falling back to the file path."""
1116+
if isinstance(value, str) and value.strip():
1117+
return value.strip()
1118+
1119+
fallback = str(file_path).strip()
1120+
if not fallback:
1121+
raise RuntimeError(f"Deleted entity for {file_path} is missing permalink")
1122+
return fallback
1123+
1124+
11111125
def runtime_deleted_note_reference_for_entity(
11121126
entity: RuntimeDeletedNoteEntitySource,
11131127
*,

tests/index/test_local_project_index.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,6 +2106,20 @@ async def handle_delete(self, entity: Entity) -> None:
21062106
async def index_entity(self, entity: Entity) -> None:
21072107
return None
21082108

2109+
async def sync_entity_vectors_batch(
2110+
self,
2111+
entity_ids: list[int],
2112+
) -> "RuntimeFactoryVectorBatchResult":
2113+
return RuntimeFactoryVectorBatchResult(entities_synced=len(entity_ids))
2114+
2115+
2116+
@dataclass(frozen=True, slots=True)
2117+
class RuntimeFactoryVectorBatchResult:
2118+
entities_synced: int
2119+
entities_skipped: int = 0
2120+
entities_failed: int = 0
2121+
entities_deferred: int = 0
2122+
21092123

21102124
class RuntimeFactoryRelationRepository:
21112125
async def find_unresolved_relations(

tests/indexing/test_accepted_note_mutation_runner.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ class _SearchRepository:
285285
def __init__(self) -> None:
286286
self.calls: list[tuple[AsyncSession, AcceptedNoteSearchRow]] = []
287287
self.deleted_entity_ids: list[int] = []
288+
self.deleted_vector_entity_ids: list[int] = []
288289

289290
async def refresh_entity(
290291
self,
@@ -301,6 +302,14 @@ async def delete_entity(
301302
_ = session
302303
self.deleted_entity_ids.append(entity_id)
303304

305+
async def delete_entity_vectors(
306+
self,
307+
session: AsyncSession,
308+
entity_id: int,
309+
) -> None:
310+
_ = session
311+
self.deleted_vector_entity_ids.append(entity_id)
312+
304313

305314
@dataclass(frozen=True, slots=True)
306315
class _MutationLookupRepositories:
@@ -758,6 +767,7 @@ async def test_run_accepted_note_delete_removes_entity_and_returns_cleanup() ->
758767

759768
assert session.deleted == [entity]
760769
assert search_repository.deleted_entity_ids == [entity.id]
770+
assert search_repository.deleted_vector_entity_ids == [entity.id]
761771
assert change.status_code == 200
762772
assert change.file_delete is not None
763773
assert change.file_delete.file_path == "notes/accepted.md"

0 commit comments

Comments
 (0)