T&E re-upload that fails every row still overwrites the earlier load's registry record #215

What happens

Re-uploading a file in T&E mode where every row fails still replaces the registry record of the earlier successful load. The rows from that earlier load stay in the table, but the registry (what "Migrated files" shows) now says nothing was loaded, and the API returns status: ok.

This is the gap left by #170. Its guard covers first-time uploads only.

Reproduction (clean database, main at ad39ddf, real API + PostgreSQL 16)

Upload the same 10-row organisations CSV twice, mode: "te", targetTable: "organisations" (6 rows valid, 4 invalid on the first run: duplicate name, bad org_type, country too long, null name):

After upload 2 the registry contradicts the table: it says 0 rows were loaded from a file whose rows are sitting in the table. The only surviving record of the 6-row load is the audit log.

Root cause

api/services/te_loader.py:

• :217-231 detects a re-upload and runs _cleanup_prior_registry_entries() before any row is inserted, deleting the prior csv_files row.
• :304 the RV-F3 (BR-34): all-failed T&E import runs registry cleanup and removes prior dataset #170 all-failed rollback is if data_rows and inserted == 0 and not is_reupload, so it is skipped for exactly this case.
• :318-321 then registers the new, all-failed load.

The comment at :217 says this ordering is deliberate ("does not undo the registry cleanup … reupload compat"). That trade-off is what loses the record.

Why it matters

The registry is the ledger a verification or reconciliation step reads: "file X, N rows loaded, M failed". After this sequence it misreports a successful migration as a total failure. For T&E mode the earlier rows cannot be dropped with the registry entry (the table is shared), so the two diverge permanently.

Proposed

Defer _cleanup_prior_registry_entries() until inserted > 0; apply the all-failed rollback to re-uploads as well, returning the same clean status: error response and leaving the prior registry row alone. Keep the permission check (_require_delete) where it is.

Done when

• A test uploads a partly valid T&E CSV, then re-uploads it unchanged, and asserts the registry row from the first load survives and the response is status: error (fails on main at ad39ddf, passes with the fix).
• A re-upload that does insert rows still replaces the registry row (existing reupload behaviour, Orphaned csv_<hash> tables leak when a dynamic-mode file is re-uploaded in T&E mode #25/upload_dynamic overwrite cleanup silently no-ops for TE-origin duplicates #27 tests unchanged).
• Dynamic-mode behaviour unchanged.

Related

#170 (closed, first-upload case), #175 (closed, overwrite permission), #109 (M5: per-tenant dedup will change re-upload semantics, so fix the guard now rather than wait).

Found while producing the sample Migration Verification Report for #59.

Generated by Claude Code