Skip to content

Commit 401fbf8

Browse files
authored
fix(knowledge): delete the document directory even before it is indexed
Squash merge of PR #456.
1 parent b736df6 commit 401fbf8

2 files changed

Lines changed: 98 additions & 5 deletions

File tree

‎src/everos/service/knowledge.py‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,9 @@ async def delete_document(
508508
) -> DeleteResult:
509509
"""Remove a document directory; cascade handles SQLite/LanceDB cleanup.
510510
511-
Idempotent: returns ``deleted_topics=0`` when the document does not exist.
511+
Idempotent: returns ``deleted_topics=0`` when neither the index nor the
512+
disk has the document, and also when the directory was removed before the
513+
cascade had indexed its topics.
512514
513515
Args:
514516
doc_id: Document primary key.
@@ -519,12 +521,28 @@ async def delete_document(
519521
DeleteResult with the topic count that was present before deletion.
520522
"""
521523
row = await knowledge_document_repo.get_by_doc_id(doc_id)
524+
memory_root = MemoryRoot.resolve()
522525
if row is None:
523-
return DeleteResult(doc_id=doc_id, deleted_topics=0)
526+
# The index trails the markdown by seconds, so a document created a
527+
# moment ago has a directory but no row yet. Deleting by the index
528+
# alone leaves that directory behind for the cascade to index right
529+
# back in, and the "deleted" document reappears. Markdown is the
530+
# truth: find the directory by its name and count the topic files
531+
# it holds, so the response says what was actually removed.
532+
topic_count = await anyio.to_thread.run_sync(
533+
_remove_unindexed_doc_dirs,
534+
memory_root.knowledge_dir(app_id, project_id),
535+
doc_id,
536+
)
537+
logger.info(
538+
"document deleted",
539+
doc_id=doc_id,
540+
topic_count=topic_count,
541+
indexed=False,
542+
)
543+
return DeleteResult(doc_id=doc_id, deleted_topics=topic_count)
524544

525545
topic_count = await knowledge_topic_sqlite_repo.count_by_doc_id(doc_id)
526-
527-
memory_root = MemoryRoot.resolve()
528546
doc_dir = memory_root.root / Path(row.md_path).parent
529547
if await anyio.Path(doc_dir).is_dir():
530548
await anyio.to_thread.run_sync(shutil.rmtree, doc_dir)
@@ -537,6 +555,26 @@ async def delete_document(
537555
return DeleteResult(doc_id=doc_id, deleted_topics=topic_count)
538556

539557

558+
def _remove_unindexed_doc_dirs(knowledge_dir: Path, doc_id: str) -> int:
559+
"""Remove every ``knowledge/<category>/<title>_<doc_id>/`` directory.
560+
561+
Returns the number of topic files (``N_*.md``) that were on disk. The
562+
directory name is compared literally — ``delete_document`` is a public
563+
service function, so a caller-supplied ``doc_id`` must not act as a glob
564+
pattern or a path; only the HTTP route validates the id's shape.
565+
"""
566+
if not knowledge_dir.is_dir():
567+
return 0
568+
suffix = f"_{doc_id}"
569+
topics = 0
570+
for doc_dir in knowledge_dir.glob("*/*"):
571+
if not doc_dir.is_dir() or not doc_dir.name.endswith(suffix):
572+
continue
573+
topics += sum(1 for _ in doc_dir.glob("[0-9]*.md"))
574+
shutil.rmtree(doc_dir)
575+
return topics
576+
577+
540578
async def replace_document(
541579
*,
542580
extractor: KnowledgeExtractor,

‎tests/unit/test_service/test_knowledge_crud.py‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,11 @@ async def test_delete_document_success(tmp_path: Path) -> None:
220220
mock_anyio.to_thread.run_sync.assert_awaited_once()
221221

222222

223-
async def test_delete_document_idempotent() -> None:
223+
async def test_delete_document_idempotent(
224+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
225+
) -> None:
224226
"""Returns deleted_topics=0 without error when document does not exist."""
227+
monkeypatch.setenv("EVEROS_ROOT", str(tmp_path))
225228
with patch(f"{_MOD}.knowledge_document_repo") as mock_doc_repo:
226229
mock_doc_repo.get_by_doc_id = AsyncMock(return_value=None)
227230

@@ -232,6 +235,58 @@ async def test_delete_document_idempotent() -> None:
232235
assert result.deleted_topics == 0
233236

234237

238+
async def test_delete_document_removes_dir_before_the_index_has_the_row(
239+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
240+
) -> None:
241+
"""A document created a moment ago has a directory but no SQLite row yet
242+
(the cascade trails the markdown by seconds). Delete must still remove the
243+
directory — and only that directory — and report the topic files it held.
244+
"""
245+
monkeypatch.setenv("EVEROS_ROOT", str(tmp_path))
246+
kdir = tmp_path / "app1" / "proj1" / "knowledge"
247+
doc_dir = kdir / "Technology" / "Release_checklist_d_lagging00001"
248+
doc_dir.mkdir(parents=True)
249+
(doc_dir / "index.md").write_text("---\ndoc_id: d_lagging00001\n---\n")
250+
(doc_dir / "1_before.md").write_text("# before\n")
251+
(doc_dir / "2_after.md").write_text("# after\n")
252+
sibling = kdir / "Technology" / "Other_notes_d_sibling000001"
253+
sibling.mkdir()
254+
(sibling / "index.md").write_text("---\ndoc_id: d_sibling000001\n---\n")
255+
(kdir / ".taxonomy.md").write_text("# taxonomy\n")
256+
257+
with patch(f"{_MOD}.knowledge_document_repo") as mock_doc_repo:
258+
mock_doc_repo.get_by_doc_id = AsyncMock(return_value=None)
259+
result = await delete_document("d_lagging00001", "app1", "proj1")
260+
261+
assert result.deleted_topics == 2
262+
assert not doc_dir.exists()
263+
assert sibling.is_dir()
264+
assert (kdir / ".taxonomy.md").exists()
265+
266+
267+
@pytest.mark.parametrize("doc_id", ["d_absent0000001", "*", "*/_original"])
268+
async def test_delete_document_unindexed_id_not_on_disk_removes_nothing(
269+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, doc_id: str
270+
) -> None:
271+
"""No row and no directory for the id → nothing is touched, including
272+
when the id looks like a glob pattern or a path: the name match is literal.
273+
"""
274+
monkeypatch.setenv("EVEROS_ROOT", str(tmp_path))
275+
kdir = tmp_path / "app1" / "proj1" / "knowledge"
276+
sibling = kdir / "Technology" / "Other_notes_d_sibling000001"
277+
sibling.mkdir(parents=True)
278+
(sibling / "index.md").write_text("---\ndoc_id: d_sibling000001\n---\n")
279+
(sibling / "_original").mkdir()
280+
281+
with patch(f"{_MOD}.knowledge_document_repo") as mock_doc_repo:
282+
mock_doc_repo.get_by_doc_id = AsyncMock(return_value=None)
283+
result = await delete_document(doc_id, "app1", "proj1")
284+
285+
assert result.deleted_topics == 0
286+
assert (sibling / "index.md").exists()
287+
assert (sibling / "_original").is_dir()
288+
289+
235290
# ── list_documents ────────────────────────────────────────────────────────────
236291

237292

0 commit comments

Comments
 (0)