test(api): cover single-entity delete for non-markdown file entities - #1056
Conversation
|
Holding merge on one correctness gap: this test proves the endpoint no longer returns 500 and removes the database/search rows, but it does not assert that the source file is removed. On current The regression should assert |
DELETE /v2/projects/{id}/knowledge/entities/{id} used to return 500 for
note_type="file" entities (issue #1033) because the delete path did
markdown-specific cleanup. The #1002/#1054 refactor fixed this by routing
single-entity delete through the shared accepted-note delete path, but no
test covered the non-markdown case.
Add a regression test that indexes a real .csv file through the local
project indexer (note_type="file", no permalink, no note_content row),
deletes it via the v2 endpoint, and asserts the entity row and its search
index rows are gone.
Refs #1033
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
|
@codex review recheck |
1d57a9f to
d7a0cca
Compare
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Adds a regression test locking in the #1002/#1054 fix for the issue-#1033 scenario:
DELETE /v2/projects/{id}/knowledge/entities/{id}returned 500 for non-markdown (note_type="file") entities in v0.20.3 because the single-entity delete path did markdown-specific cleanup. The refactor on main fixed this by routing single-entity delete through the shared accepted-note delete path (delete_entity_by_id→note_content_mutation_service.delete_note→run_accepted_note_delete), which removes search/vector rows by entity id and tolerates entities with nonote_contentrow — but nothing covered the non-markdown case.Test
test_delete_entity_by_id_non_markdown_file_entityintests/api/v2/test_knowledge_router.py:.csvthrough the local project indexer (run_local_project_index_for_project), producing a genuine file entity:note_type="file", no permalink, nonote_contentrow — the exact shape that used to 500deleted: trueThe test passes on current
mainunmodified, proving the fix covers the #1033 scenario end-to-end.Verification
uv run pytest tests/api/v2/test_knowledge_router.py -x -q— 64 passedruff check/ruff formatclean;just typecheckdiagnostics identical to main (pre-existing)Refs #1033
🤖 Generated with Claude Code