Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions src/everos/service/knowledge.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,9 @@ async def delete_document(
) -> DeleteResult:
"""Remove a document directory; cascade handles SQLite/LanceDB cleanup.

Idempotent: returns ``deleted_topics=0`` when the document does not exist.
Idempotent: returns ``deleted_topics=0`` when neither the index nor the
disk has the document, and also when the directory was removed before the
cascade had indexed its topics.

Args:
doc_id: Document primary key.
Expand All @@ -519,12 +521,28 @@ async def delete_document(
DeleteResult with the topic count that was present before deletion.
"""
row = await knowledge_document_repo.get_by_doc_id(doc_id)
memory_root = MemoryRoot.resolve()
if row is None:
return DeleteResult(doc_id=doc_id, deleted_topics=0)
# The index trails the markdown by seconds, so a document created a
# moment ago has a directory but no row yet. Deleting by the index
# alone leaves that directory behind for the cascade to index right
# back in, and the "deleted" document reappears. Markdown is the
# truth: find the directory by its name and count the topic files
# it holds, so the response says what was actually removed.
topic_count = await anyio.to_thread.run_sync(
_remove_unindexed_doc_dirs,
memory_root.knowledge_dir(app_id, project_id),
doc_id,
)
logger.info(
"document deleted",
doc_id=doc_id,
topic_count=topic_count,
indexed=False,
)
return DeleteResult(doc_id=doc_id, deleted_topics=topic_count)

topic_count = await knowledge_topic_sqlite_repo.count_by_doc_id(doc_id)

memory_root = MemoryRoot.resolve()
doc_dir = memory_root.root / Path(row.md_path).parent
if await anyio.Path(doc_dir).is_dir():
await anyio.to_thread.run_sync(shutil.rmtree, doc_dir)
Expand All @@ -537,6 +555,26 @@ async def delete_document(
return DeleteResult(doc_id=doc_id, deleted_topics=topic_count)


def _remove_unindexed_doc_dirs(knowledge_dir: Path, doc_id: str) -> int:
"""Remove every ``knowledge/<category>/<title>_<doc_id>/`` directory.

Returns the number of topic files (``N_*.md``) that were on disk. The
directory name is compared literally — ``delete_document`` is a public
service function, so a caller-supplied ``doc_id`` must not act as a glob
pattern or a path; only the HTTP route validates the id's shape.
"""
if not knowledge_dir.is_dir():
return 0
suffix = f"_{doc_id}"
topics = 0
for doc_dir in knowledge_dir.glob("*/*"):
if not doc_dir.is_dir() or not doc_dir.name.endswith(suffix):
continue
topics += sum(1 for _ in doc_dir.glob("[0-9]*.md"))
shutil.rmtree(doc_dir)
return topics


async def replace_document(
*,
extractor: KnowledgeExtractor,
Expand Down
57 changes: 56 additions & 1 deletion tests/unit/test_service/test_knowledge_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,11 @@ async def test_delete_document_success(tmp_path: Path) -> None:
mock_anyio.to_thread.run_sync.assert_awaited_once()


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

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


async def test_delete_document_removes_dir_before_the_index_has_the_row(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""A document created a moment ago has a directory but no SQLite row yet
(the cascade trails the markdown by seconds). Delete must still remove the
directory — and only that directory — and report the topic files it held.
"""
monkeypatch.setenv("EVEROS_ROOT", str(tmp_path))
kdir = tmp_path / "app1" / "proj1" / "knowledge"
doc_dir = kdir / "Technology" / "Release_checklist_d_lagging00001"
doc_dir.mkdir(parents=True)
(doc_dir / "index.md").write_text("---\ndoc_id: d_lagging00001\n---\n")
(doc_dir / "1_before.md").write_text("# before\n")
(doc_dir / "2_after.md").write_text("# after\n")
sibling = kdir / "Technology" / "Other_notes_d_sibling000001"
sibling.mkdir()
(sibling / "index.md").write_text("---\ndoc_id: d_sibling000001\n---\n")
(kdir / ".taxonomy.md").write_text("# taxonomy\n")

with patch(f"{_MOD}.knowledge_document_repo") as mock_doc_repo:
mock_doc_repo.get_by_doc_id = AsyncMock(return_value=None)
result = await delete_document("d_lagging00001", "app1", "proj1")

assert result.deleted_topics == 2
assert not doc_dir.exists()
assert sibling.is_dir()
assert (kdir / ".taxonomy.md").exists()


@pytest.mark.parametrize("doc_id", ["d_absent0000001", "*", "*/_original"])
async def test_delete_document_unindexed_id_not_on_disk_removes_nothing(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, doc_id: str
) -> None:
"""No row and no directory for the id → nothing is touched, including
when the id looks like a glob pattern or a path: the name match is literal.
"""
monkeypatch.setenv("EVEROS_ROOT", str(tmp_path))
kdir = tmp_path / "app1" / "proj1" / "knowledge"
sibling = kdir / "Technology" / "Other_notes_d_sibling000001"
sibling.mkdir(parents=True)
(sibling / "index.md").write_text("---\ndoc_id: d_sibling000001\n---\n")
(sibling / "_original").mkdir()

with patch(f"{_MOD}.knowledge_document_repo") as mock_doc_repo:
mock_doc_repo.get_by_doc_id = AsyncMock(return_value=None)
result = await delete_document(doc_id, "app1", "proj1")

assert result.deleted_topics == 0
assert (sibling / "index.md").exists()
assert (sibling / "_original").is_dir()


# ── list_documents ────────────────────────────────────────────────────────────


Expand Down
Loading