Repository navigation
fix: add ZeroDBClient.delete_file, and make three verify scripts really clean up - #181
Merged
Merged
Conversation
ZeroDBClient has no delete_file, so stories 9.1, 5.4 and 9.2 each had to
document 'cleanup cannot remove uploaded file objects' as a limitation
instead of cleaning up. These tests pin the behaviour the method must have,
measured live against the real project on 2026-10-20:
* it addresses DELETE /projects/{id}/files/{file_id} -- the one of the two
live delete routes that names the storage question, rather than
/database/files/{file_id} which hides it;
* it never sends delete_from_storage=false, which deletes the metadata row
and leaves the bytes behind as an unreachable orphan forever;
* both the metadata and the stored bytes are gone afterwards;
* a second delete answers a clean 404 (not the wrapped 500 delete_memory
has to reinterpret), so a cleanup path can treat it as already done;
* a metadata-only row -- no bytes ever stored -- is refused unless the
caller opts in, because those are datamodel.md's unmendable citations.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds ZeroDBClient.delete_file, closing the gap stories 9.1, 5.4 and 9.2 each had to document as a limitation. Endpoint, measured live rather than assumed -- the /database/files vs /files split here is the same one that produced the upload bug in PR #157: DELETE /projects/{id}/files/{file_id} Both that route and /database/files/{file_id} do genuinely remove the MinIO object. That is not inferred from the 200: a presigned URL minted before the delete, still within its signature window afterwards, was fetched straight from object storage and answered NoSuchKey for either route. /files/{file_id} is used because it is the only one whose contract names storage. Two live findings shaped the method: * ?delete_from_storage=false returns 200, drops the metadata row, and leaves the object in MinIO addressable by nothing -- a byte leak with a friendly name. The method sends the parameter never and exposes no argument for it. * deleted_from_storage in the response reads true even for a metadata-only row that never had an object, so it means attempted, not removed. It is not treated as proof anywhere. A second delete answers a clean 404, so unlike delete_memory no reinterpretation is needed and no retry is burned -- a cleanup path can treat already-gone as done. delete_file also refuses a row with no stored bytes unless the caller passes allow_missing_content=True: those are datamodel.md's unmendable citations, and retiring them is a human decision, not something a cleanup sweep does in passing. FakeZeroDBTransport serves the route with the same semantics, including the unconditional deleted_from_storage so no caller can rely on it as proof. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…he leak Three merged stories each had to write down 'cleanup cannot remove uploaded file objects' as an honest limitation. delete_file exists now, so the two scripts that have a cleanup path use it, and the notes that described the gap are replaced by a delete that is re-checked -- not by a quieter skip. verify_bill_text_live.py: deletes each file it uploads (orphaned_file_ids is now uploaded_file_ids) and folds the result into the same cleanup_failures list the rows use, so a file left behind fails the run. It no longer leaks H.R. 1's 2.8 MB public-law text on every run. verify_debates_questionnaires_live.py: --cleanup deletes both files per capture. Files go last, after their promise_sources rows, so a failure there is a true orphan and is counted as a failure rather than logged as a note. Each deletion is confirmed against the CONTENT route, not the metadata row. That distinction is story 9.1's whole lesson: a file row and the bytes behind it can disagree, so only a failed download proves the object actually left the store. verify_campaign_sites_live.py (story 9.1) is deliberately NOT changed: it has no cleanup path by design -- its captures are permanent reference data its own output says not to clean up. The 9.1-era limitation note lives in repair_unretrievable_archives.py instead, whose conclusion still stands (retiring unmendable citations is a human decision) but whose stated blocker was half plumbing that now exists; it says so accurately. Live runs of both scripts pass with real file cleanup: 2 files for bill text, 10 for debates/questionnaires, every one confirmed unreadable afterwards. datamodel.md: the story 5.4 open question is closed, with the three live findings recorded, and both orphans it named -- 583d2231 (4,233 bytes) and 0cebf653 (2,787,974 bytes) -- were confirmed unreferenced by any bills row and then deleted live, reclaiming 2.79 MB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cross-cutting infrastructure fix, not a numbered backlog story — same category as the scheduled-job runner.
ZeroDBClienthad nodelete_file, and three separately merged stories each had to document "cleanup cannot remove uploaded file objects" as an honest limitation instead of actually cleaning up: 9.1 (campaign sites), 5.4 (bill text), 9.2 (debates/questionnaires). Story 5.4's verify script leaked H.R. 1's 2.8 MB public-law text on every single run. Built once, here, rather than a fourth time.The endpoint, measured rather than assumed
The
/database/filesvs/filessplit is exactly the one that caused the upload bug in PR #157 (metadata row written, no bytes stored), so this was verified live against the real project before committing to a shape. Both DELETE routes exist:DELETE /projects/{id}/database/files/{file_id}{message, file_name}DELETE /projects/{id}/files/{file_id}{success, file_id, deleted_from_storage, message}Both genuinely remove the MinIO object. That is not inferred from the 200 — a presigned URL minted before the delete, still inside its signature window afterwards, was fetched straight from object storage and answered
NoSuchKeyfor either route.delete_fileuses/files/{file_id}because it is the only one whose contract names storage, which is what makes the trap below refusable instead of invisible.Two live findings that shaped the method
?delete_from_storage=falseis a byte leak with a friendly name. It returns 200, drops the metadata row, and leaves the object in MinIO addressable by nothing — every delete route addresses a row that no longer exists. Recovering one takes re-uploading under the identicalfile_keyand deleting that properly (confirmed; that is how the one I created while probing was reclaimed). The method sends the parameter never and exposes no argument for it.deleted_from_storageis not proof. It readstrueeven for a metadata-only row that never had an object, so it means "attempted", not "removed". Nothing treats it as proof; the verify scripts prove removal with a faileddownload_fileinstead.Unlike
delete_memory, a second delete answers a clean 404, not a 500 wrapping a 404 — so noreinterprethook is needed and no retry is burned. A cleanup path can treat already-gone as done.The one judgment call
delete_filerefuses a row whose bytes were never stored unless the caller passesallow_missing_content=True. Those rows are datamodel.md's unmendable citations — story 9.1's threepromise_sourcesarchives whose pages have since changed, so the bytes are provably unrecoverable by anyone. Retiring them is a deliberate human decision, not something a cleanup sweep does on its way past, and the opt-in is the gate that keeps it deliberate. Live-verified in both directions: the guard fires, and the opt-in works.Verified live
get_file,download_fileand a seconddelete_fileall raiseZeroDBNotFoundError.verify_bill_text_live.py— all live checks passed, 2 bills, 4 provenance_log rows and 2 archived files deleted, each re-checked. No more 2.8 MB leak per run.verify_debates_questionnaires_live.py --cleanup— VERIFICATION PASSED, all 10 stored files (5 captures × document + text) deleted and each confirmed unreadable.583d2231…(4,233 bytes) and0cebf653…(2,787,974 bytes), each first confirmed matching its documented name/size and unreferenced by anybillsrow, then deleted and confirmed unreadable. 2.79 MB reclaimed.Scope note
verify_campaign_sites_live.py(9.1) is deliberately not changed — it has no cleanup path by design; its own output says "These are REAL captures … Do NOT clean these up." Adding deletion there would be wrong. The 9.1-era limitation note lives inrepair_unretrievable_archives.py, whose conclusion still stands but whose stated blocker was half plumbing that now exists; it now says so accurately.Tests
Red first, confirmed failing for the right reason (
'ZeroDBClient' object has no attribute 'delete_file'), committed, then green. Five unit tests againstFakeZeroDBTransport, consistent with every other client method; the fake serves the route with the same semantics, including the unconditionaldeleted_from_storageso no caller can come to rely on it. No Gherkin — internal infrastructure with no user-facing behaviour, following this session's scheduled-job-runner precedent.Full backend suite: 3918 passed, 4 xfailed, 0 failed.
ruff checkandruff format --checkclean.🤖 Generated with Claude Code