Found while testing a fix for #183 against production. This is a safety-mechanism gap, not a capture bug — it applies to every event, not just the one that exposed it.
What happened
Re-capturing Doc5713434353 (which already held a good 37-of-39 bundle) produced only 20 documents. capture_files accepted it and wrote a 347 MB bundle over the existing 826 MB one. 18 documents lost.
_MIN_CAPTURE_RATIO (#182) did not fire: 20/39 = 51%, just above the 50% floor.
It was caught only because the original had been moved aside rather than deleted before the test. On an unattended nightly there would have been no signal at all — Doc<n>.zip existing is the whole of what capture_attachments reads as "archived", and Respond dies when the posting closes, so no later run could ever repair it.
Why the existing guard can't catch this
_MIN_CAPTURE_RATIO compares this run's downloads against this run's own traversal listing. That is the right check for what it was built for (#182: a broken run that captured 1 of 39), and it should stay.
But it is blind to the one thing that matters when a bundle already exists: is this capture better or worse than what it is about to replace? Nothing in capture_files reads the existing Doc<n>.zip before overwriting it. A run that legitimately traverses 39 files and legitimately downloads 20 of them looks healthy by every check present.
Why a re-capture happens at all
Normally it does not — capture_attachments skips any event whose Doc<n>.zip exists, which is why this went unnoticed. It becomes reachable whenever the bundle is removed deliberately: recovering documents from a known gap record (exactly what #183's test was doing), a manual re-run after a parser fix, or any future retry-the-gaps mechanism. The moment such a mechanism exists, this becomes a live hazard rather than a latent one.
Options
- Refuse to shrink a bundle. Before
build_bundle overwrites an existing Doc<n>.zip, read its entry count (cheap — central directory only, index_zip already does this) and refuse if the new capture holds fewer, keeping partials and leaving the event pending. Strictly additive to the existing floor.
- Merge rather than replace. Take the union of what is on disk and what this run captured, since both are real bytes that may be unrepeatable. More faithful to the archive's "never lose bytes" ethos, but changes bundle identity in a way that needs thought.
- Never overwrite; write beside and promote. Heavier, but makes the replace step explicitly reversible.
Option 1 is the smallest change that closes the hole and is consistent with how every other guard here is framed (refuse, keep partials, stay pending, say so).
Note
Whatever is chosen should be verified against a large event, not a small one — that is precisely the generalisation error that produced the regression in the first place (see the #183 re-scope).
Found while testing a fix for #183 against production. This is a safety-mechanism gap, not a capture bug — it applies to every event, not just the one that exposed it.
What happened
Re-capturing
Doc5713434353(which already held a good 37-of-39 bundle) produced only 20 documents.capture_filesaccepted it and wrote a 347 MB bundle over the existing 826 MB one. 18 documents lost._MIN_CAPTURE_RATIO(#182) did not fire: 20/39 = 51%, just above the 50% floor.It was caught only because the original had been moved aside rather than deleted before the test. On an unattended nightly there would have been no signal at all —
Doc<n>.zipexisting is the whole of whatcapture_attachmentsreads as "archived", and Respond dies when the posting closes, so no later run could ever repair it.Why the existing guard can't catch this
_MIN_CAPTURE_RATIOcompares this run's downloads against this run's own traversal listing. That is the right check for what it was built for (#182: a broken run that captured 1 of 39), and it should stay.But it is blind to the one thing that matters when a bundle already exists: is this capture better or worse than what it is about to replace? Nothing in
capture_filesreads the existingDoc<n>.zipbefore overwriting it. A run that legitimately traverses 39 files and legitimately downloads 20 of them looks healthy by every check present.Why a re-capture happens at all
Normally it does not —
capture_attachmentsskips any event whoseDoc<n>.zipexists, which is why this went unnoticed. It becomes reachable whenever the bundle is removed deliberately: recovering documents from a known gap record (exactly what #183's test was doing), a manual re-run after a parser fix, or any future retry-the-gaps mechanism. The moment such a mechanism exists, this becomes a live hazard rather than a latent one.Options
build_bundleoverwrites an existingDoc<n>.zip, read its entry count (cheap — central directory only,index_zipalready does this) and refuse if the new capture holds fewer, keeping partials and leaving the event pending. Strictly additive to the existing floor.Option 1 is the smallest change that closes the hole and is consistent with how every other guard here is framed (refuse, keep partials, stay pending, say so).
Note
Whatever is chosen should be verified against a large event, not a small one — that is precisely the generalisation error that produced the regression in the first place (see the #183 re-scope).