fix: keep failed knowledge-base document deletions retryable

[beemines]

Deleting a knowledge-base document currently commits removal of its KBDocument and KBMedia rows before deleting its chunks and FAISS vectors. If index persistence or chunk deletion then fails, the request raises an error but the document has already disappeared from the document list. The remaining stored chunks have lost their metadata and the user cannot select the document in the dashboard to retry deletion.

Modifications

Move vec_db.delete_documents() before the metadata transaction. A storage failure keeps the document and media records discoverable; retrying completes the existing idempotent deletion. A later metadata-transaction failure also retains these records for retry, even when the chunks have already been removed.

This is an ordering fix in the knowledge-base application workflow. It does not make the separate stores atomic, restore vectors after a partial failure, add automatic retries, or change FAISS internals. API schemas and normal successful deletion are unchanged.

Add parameterized tests using real temporary SQLite metadata/chunk databases and a real FAISS index. The embedding provider returns a fixed vector; faults are injected at index save, chunk deletion, and the document-row deletion inside the metadata transaction. The tests reopen the metadata database, retry deletion, check the other document's data remains, and reopen the vector store to verify persistence.

This was found by inspecting the current deletion path. Related #9120 handles media cleanup on successful single-document deletion; #9303 handles whole-knowledge-base cascades. Neither changes this single-document failure ordering.

• This is NOT a breaking change.

Verification steps and test results

Windows, Python 3.12; base bd046ed29914ee559e9bf47676ccb71a84f747ba.

Before the production change, the index-save and chunk-delete regression cases both failed because the target document was absent after reopening the metadata database:

2 failed, 1 warning

After the fix, including the metadata rollback/retry case:

python -m pytest tests/unit/test_kb_delete_retry.py tests/unit/test_kb_document_cleanup.py tests/unit/test_kb_upload_atomicity.py tests/unit/test_kb_manager_resilience.py tests/unit/test_knowledge_base_service_contract.py tests/unit/test_faiss_vec_db.py tests/unit/test_document_storage_fts.py -q
56 passed, 3 warnings

The warnings include the existing audioop deprecation and aiosqlite worker-thread Event loop is closed warnings. They are not failed assertions. The full repository suite, Linux/macOS runs, live model calls, and browser-level interactions were not run locally.

ruff format --check ., ruff check ., and git diff --cached --check pass. The new test file was also explicitly formatted and linted.

Checklist

• New-feature discussion: not applicable; this is a bug fix.
• Verification steps and actual test output are provided above.
• No new dependencies.
• No malicious code.

AI assistance

Codex assisted with code inspection, reproduction, implementation, validation, and this PR description. The tests and results above were executed locally; the change does not claim end-to-end atomicity across independent stores.

CI follow-up: synchronize watchdog tests

The first remote run passed on Ubuntu and Windows. On macOS, all three new knowledge-base tests passed, but test_event_loop_watchdog_writes_rotating_log failed because the sampled stack did not include the test coroutine (2886 passed, 1 failed). Its fixed 50ms blocking window can end before log opening and stack capture complete.

A separate test-only commit replaces the fixed sleeps in the two watchdog dump tests with bounded threading.Event waits and finally cleanup. The rotation test still writes real stack traces and checks the original rotated file contents; it now specifically checks the event-loop thread's stack contains the test coroutine. Production watchdog behavior is unchanged.

Local fault injection adding 80ms to the real log-open operation reproduces the original missing-test-frame assertion; the same probe passes with synchronization. The watchdog module plus the new knowledge-base tests pass together (8 passed), and full Ruff formatting/lint checks pass. Remote CI on commit c3102950ffd4b919bf1a206e522a5dfbcfdb6dcc is now complete: all 23 checks passed. The actual test steps on Ubuntu, Windows, and macOS all succeeded. Formatting, CodeQL, dashboard CI, and smoke tests also passed. Unit-test run.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the deletion ordering or retry behavior is wrong, vector chunks and metadata can be removed in separate stores, leaving a partially deleted document after a failure. Reverting the change cannot restore records already deleted, although a later retry can clean up the remaining metadata.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook