Code review surfaced two issues that matter once the migration runs
against real data, both around storage-dir removal:
1. The bib branch had no foreign-content guard — Zotero's branch
inspected d.iterdir() for non-md files before rmtree, but bib's
branch unconditionally rmtree'd the parent dir. bib's actual
storage layout is one-attachment-per-dir so this is theoretically
safe, but a defensive guard (matching the Zotero pattern) costs
nothing and prevents silent data loss if the layout ever drifts.
2. Counters incremented unconditionally after `rmtree(d, ignore_errors
=True)`. Silent rmtree failure would still bump bib_dirs_removed /
zot_dirs_removed, masking the failure in the post-run stats.
Switch to bare `rmtree` + try/except OSError, and only increment
on the success path. Failures land in two new counters
(bib_dirs_rmtree_failed, zot_dirs_rmtree_failed) so the operator
sees them in the final log.
Surfaced by code review on commit c9b8a51.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
db7ea65287