fix: purge orphan KB vectors, route sync deletes through file delete, reject duplicate sync paths

- 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.
This commit is contained in:
DrMelone 2026-04-12 23:54:52 +02:00
parent 86e6e239a4
commit 74163ce963
2 changed files with 64 additions and 7 deletions

View file

@ -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)

View file

@ -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
}
)