From 74163ce9635f850244cf0675f8db562ce1e5263a Mon Sep 17 00:00:00 2001 From: DrMelone <27028174+Classic298@users.noreply.github.com> Date: Sun, 12 Apr 2026 23:54:52 +0200 Subject: [PATCH] fix: purge orphan KB vectors, route sync deletes through file delete, reject duplicate sync paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - upload_and_replace_file now explicitly deletes the new file's vectors from the KB collection in the Step 3 rollback path. process_file can write embeddings before add_file_to_knowledge_by_id fails; without the association the router delete can't discover those vectors, so they would remain retrievable for a file record that no longer exists. - syncDirectoryHandler switches the removed-files loop from removeFileFromKnowledgeById (leaves storage blobs behind) to deleteFileById, which also clears the object-storage blob and the per-file vector collection. Each delete is guarded with try/catch and a truthy-response check so silent null returns are counted as failures instead of inflating the "removed" tally. - Sync summary now reports the succeeded-removed count and rolls removedFailed into totalFailed so the user sees real outcomes. - compare_files_for_sync rejects payloads with duplicate file_path entries up front, so the server no longer relies on the frontend to dedupe — duplicates would otherwise schedule the same existing file for replacement or removal twice. --- backend/open_webui/routers/knowledge.py | 30 ++++++++++++++ .../workspace/Knowledge/KnowledgeBase.svelte | 41 +++++++++++++++---- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/backend/open_webui/routers/knowledge.py b/backend/open_webui/routers/knowledge.py index 47cf8df317..4c9de51b58 100644 --- a/backend/open_webui/routers/knowledge.py +++ b/backend/open_webui/routers/knowledge.py @@ -799,6 +799,21 @@ async def upload_and_replace_file( ) except Exception as e: log.error(f'Failed to add new file to knowledge base: {e}') + # process_file may have already written embeddings into the KB + # collection before add_file_to_knowledge_by_id failed. The router + # delete iterates KB associations to clean KB vectors, so without + # the association those chunks would stay discoverable by retrieval + # even after the file record is gone. Purge them by file_id here + # before handing off to the full delete. + try: + VECTOR_DB_CLIENT.delete( + collection_name=id, filter={'file_id': new_file_id} + ) + except Exception as vector_err: + log.warning( + f'Failed to purge orphan KB embeddings for {new_file_id}: {vector_err}' + ) + # Clean up via the router-level delete so storage object and vector # collection are removed too — Files.delete_file_by_id only drops the # DB row and would orphan S3/GCS objects and the per-file vector @@ -1185,6 +1200,21 @@ async def compare_files_for_sync( detail=ERROR_MESSAGES.ACCESS_PROHIBITED, ) + # Reject payloads with duplicate file_path entries. A duplicate would plan + # the same existing file for replacement or removal twice, producing + # downstream 404s on the second pass and muddying the success/failure + # counts on the client. Public API robustness shouldn't depend on the + # frontend deduping. + incoming_paths = [incoming.file_path for incoming in form_data.files] + if len(incoming_paths) != len(set(incoming_paths)): + duplicates = sorted( + {path for path in incoming_paths if incoming_paths.count(path) > 1} + ) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f'Duplicate file_path entries in sync payload: {", ".join(duplicates)}', + ) + # Get all files currently in the knowledge base existing_files = await Knowledges.get_files_by_id(id, db=db) diff --git a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte index 6f390888f9..ef3330f0ba 100644 --- a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte +++ b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte @@ -562,9 +562,16 @@ const totalToProcess = new_files.length + changed_files.length; let processedCount = 0; - // STEP 1: Delete removed files FIRST - // If a file was incorrectly classified as both "new" and "removed" (due to filename - // matching issues), deleting first allows the subsequent upload to succeed. + // STEP 1: Delete removed files FIRST. + // If a file was incorrectly classified as both "new" and "removed" + // (due to filename matching issues), deleting first allows the + // subsequent upload to succeed. + // Use deleteFileById (DELETE /files/{id}) so the object-storage + // blob, the per-file vector collection, and KB associations are + // all removed — removeFileFromKnowledgeById leaves storage blobs + // behind and would accumulate orphans over repeated syncs. + let removedSucceeded = 0; + let removedFailed = 0; if (removed_file_ids.length > 0) { toast.info( $i18n.t('Removing {{count}} deleted files...', { @@ -572,7 +579,27 @@ }) ); for (const fileId of removed_file_ids) { - await removeFileFromKnowledgeById(localStorage.token, id, fileId); + try { + const res = await deleteFileById(localStorage.token, fileId); + if (res) { + removedSucceeded++; + } else { + // API wrapper returned null without throwing — treat + // as a silent failure so the user isn't told we + // removed something we didn't. + removedFailed++; + } + } catch (removeErr) { + removedFailed++; + console.error('Delete failed for', fileId, removeErr); + const detail = + typeof removeErr === 'string' + ? removeErr + : (removeErr?.detail ?? removeErr?.message ?? 'unknown error'); + toast.error( + $i18n.t('Failed to remove file: {{detail}}', { detail }) + ); + } } } @@ -644,7 +671,7 @@ // Show summary. Use succeeded counts — not the planned counts — // so the user sees real outcomes. Include a failure tally only // when something went wrong. - const totalFailed = newFailed + changedFailed; + const totalFailed = newFailed + changedFailed + removedFailed; if (totalFailed > 0) { toast.warning( $i18n.t( @@ -652,7 +679,7 @@ { newCount: newSucceeded, changedCount: changedSucceeded, - removedCount: removed_file_ids.length, + removedCount: removedSucceeded, unchangedCount: unchanged.length, failedCount: totalFailed } @@ -665,7 +692,7 @@ { newCount: newSucceeded, changedCount: changedSucceeded, - removedCount: removed_file_ids.length, + removedCount: removedSucceeded, unchangedCount: unchanged.length } )