feat(bib): dedupe_attachments migration + guarded unique index on (item, filename) (refs #615)

dev/scripts/dedupe_attachments.py

@@ -0,0 +1,165 @@
+#!/usr/bin/env python3
+"""One-time cleanup of duplicate bib attachments (refs #615).
+
+``Store.attach_file`` used to mint a new row + storage copy on every
+call, so re-farms produced thousands of ``(item_id, filename)``
+duplicates (33,379 groups / 66,917 of 78,017 rows on 2026-09-08). This
+keeps the oldest row of each group, deletes the others' rows and their
+storage copies, and prints a report. Dry-run by default.
+
+Zotero is not touched: ``bib.sync`` already dedupes child attachments by
+filename, so a duplicate that was synced once is a single Zotero child
+attachment and stays valid.
+
+Usage::
+
+    uv run python dev/scripts/dedupe_attachments.py            # report only
+    uv run python dev/scripts/dedupe_attachments.py --apply    # delete
+"""
+
+from __future__ import annotations
+
+import argparse
+import os
+import sqlite3
+import sys
+from dataclasses import dataclass, field
+from pathlib import Path
+
+
+@dataclass
+class Group:
+    item_id: int
+    filename: str
+    keep: str
+    remove: list[str] = field(default_factory=list)
+    remove_paths: list[str] = field(default_factory=list)
+    conflict: bool = False  # sizes differ → do not touch
+
+
+@dataclass
+class Report:
+    groups: int = 0
+    rows_removed: int = 0
+    files_removed: int = 0
+    bytes_freed: int = 0
+    skipped_conflicts: int = 0
+    removed_keys: list[str] = field(default_factory=list)
+
+
+def plan(con: sqlite3.Connection) -> list[Group]:
+    con.row_factory = sqlite3.Row
+    rows = con.execute(
+        """
+        SELECT a.item_id, a.filename, a.key, a.storage_path, a.rowid AS rid
+        FROM attachments a
+        WHERE (a.item_id, a.filename) IN (
+            SELECT item_id, filename FROM attachments
+            GROUP BY item_id, filename HAVING count(*) > 1
+        )
+        ORDER BY a.item_id, a.filename, a.rowid
+        """
+    ).fetchall()
+    groups: dict[tuple[int, str], Group] = {}
+    for r in rows:
+        gkey = (r["item_id"], r["filename"])
+        g = groups.get(gkey)
+        if g is None:
+            groups[gkey] = Group(
+                item_id=r["item_id"], filename=r["filename"], keep=r["key"]
+            )
+            continue
+        g.remove.append(r["key"])
+        g.remove_paths.append(r["storage_path"])
+    # conflict check: every copy must have the same size as the kept one
+    for g in groups.values():
+        keep_path = con.execute(
+            "SELECT storage_path FROM attachments WHERE key = ?", (g.keep,)
+        ).fetchone()["storage_path"]
+        keep_size = _size(keep_path)
+        for p in g.remove_paths:
+            if _size(p) not in (keep_size, -1):
+                g.conflict = True
+                break
+    return list(groups.values())
+
+
+def _size(path: str) -> int:
+    try:
+        return os.stat(path).st_size
+    except OSError:
+        return -1
+
+
+def apply(con: sqlite3.Connection, groups: list[Group]) -> Report:
+    rep = Report(groups=len(groups))
+    con.execute("BEGIN")
+    try:
+        for g in groups:
+            if g.conflict:
+                rep.skipped_conflicts += 1
+                continue
+            for key, path in zip(g.remove, g.remove_paths):
+                still_referenced = con.execute(
+                    "SELECT count(*) FROM attachments WHERE storage_path = ? AND key <> ?",
+                    (path, key),
+                ).fetchone()[0]
+                con.execute("DELETE FROM attachments WHERE key = ?", (key,))
+                rep.rows_removed += 1
+                rep.removed_keys.append(key)
+                if not still_referenced:
+                    p = Path(path)
+                    if p.is_file():
+                        rep.bytes_freed += p.stat().st_size
+                        p.unlink()
+                        rep.files_removed += 1
+                        try:
+                            p.parent.rmdir()  # the per-key dir, if now empty
+                        except OSError:
+                            pass
+        con.execute("COMMIT")
+    except Exception:
+        con.execute("ROLLBACK")
+        raise
+    return rep
+
+
+def main(argv: list[str] | None = None) -> int:
+    ap = argparse.ArgumentParser(
+        description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter
+    )
+    ap.add_argument(
+        "--db", default="", help="bib.sqlite path (default: stack.toml db.bib)"
+    )
+    ap.add_argument(
+        "--apply", action="store_true", help="delete duplicates (default: report only)"
+    )
+    args = ap.parse_args(argv)
+    if args.db:
+        db = args.db
+    else:
+        sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "src"))
+        from conf import path
+
+        db = str(path("db.bib"))
+    con = sqlite3.connect(db, isolation_level=None)
+    groups = plan(con)
+    conflicts = sum(1 for g in groups if g.conflict)
+    rows = sum(len(g.remove) for g in groups)
+    print(
+        f"{db}: {len(groups)} duplicate groups, {rows} rows to remove, {conflicts} conflicts (size mismatch, skipped)"
+    )
+    if not args.apply:
+        print("dry run — pass --apply to delete")
+        return 0
+    rep = apply(con, groups)
+    print(
+        f"removed rows={rep.rows_removed} files={rep.files_removed} "
+        f"freed={rep.bytes_freed / 1e6:.1f} MB skipped_conflicts={rep.skipped_conflicts}"
+    )
+    print("reopen the store once (any `stack bib` command) to create the unique index")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

src/bib/store.py

@@ -83,6 +83,19 @@ class Store:
     def _init_schema(self) -> None:
         ddl = resources.files("bib").joinpath("schema.sql").read_text()
         self._con().executescript(ddl)
+        # The (item_id, filename) unique index can only exist once the
+        # dedupe migration has run (dev/scripts/dedupe_attachments.py);
+        # until then we leave it off rather than fail to open the store.
+        con = self._con()
+        dup = con.execute(
+            "SELECT 1 FROM attachments GROUP BY item_id, filename HAVING count(*) > 1 LIMIT 1"
+        ).fetchone()
+        if dup is None:
+            con.execute(
+                "CREATE UNIQUE INDEX IF NOT EXISTS idx_attachments_item_filename "
+                "ON attachments(item_id, filename)"
+            )
+            con.commit()
 
     def close(self) -> None:
         if self._connection is not None:

tests/dev/test_dedupe_attachments.py

@@ -0,0 +1,117 @@
+from __future__ import annotations
+
+import importlib.util
+import sqlite3
+import sys
+from importlib import resources
+from pathlib import Path
+
+from bib.item import Source
+from bib.store import Store
+
+_SCRIPT = (
+    Path(__file__).resolve().parents[2] / "dev" / "scripts" / "dedupe_attachments.py"
+)
+spec = importlib.util.spec_from_file_location("dedupe_attachments", _SCRIPT)
+mod = importlib.util.module_from_spec(spec)
+sys.modules["dedupe_attachments"] = mod
+spec.loader.exec_module(mod)
+
+
+def _seed(tmp_path: Path) -> tuple[Store, str]:
+    # Seed the item + duplicate attachment rows via a raw connection,
+    # *before* any Store ever opens this file. Store's schema guard adds
+    # the (item_id, filename) unique index the moment it sees zero
+    # duplicate groups — true the instant the attachments table exists —
+    # so seeding duplicates through a Store-opened connection can never
+    # succeed. This mirrors the real migration scenario: the duplicates
+    # were written by pre-Task-4 code that predates this guard.
+    db_path = tmp_path / "bib.sqlite"
+    storage = tmp_path / "storage"
+    ddl = resources.files("bib").joinpath("schema.sql").read_text()
+    raw = sqlite3.connect(str(db_path))
+    raw.executescript(ddl)
+    item = Source(title="T", url="https://x/1")
+    item.stamp_access()
+    row = item.to_row()
+    key = row["key"] or "SEEDKEY1"
+    row["key"] = key
+    raw.execute(
+        """INSERT INTO items
+           (key, item_type, title, url, date_published,
+            access_date, abstract, institution, extra, extra_json)
+           VALUES (:key, :item_type, :title, :url,
+                   :date_published, :access_date, :abstract,
+                   :institution, :extra, :extra_json)""",
+        row,
+    )
+    item_id = raw.execute("SELECT id FROM items WHERE key=?", (key,)).fetchone()[0]
+    # Simulate the old non-idempotent attach: three rows, three copies.
+    for k in ("AAAAAAAA", "BBBBBBBB", "CCCCCCCC"):
+        d = storage / k
+        d.mkdir(parents=True)
+        (d / "attachment_1.pdf").write_bytes(b"%PDF-dup")
+        raw.execute(
+            "INSERT INTO attachments (item_id, key, filename, content_type, storage_path) VALUES (?,?,?,?,?)",
+            (
+                item_id,
+                k,
+                "attachment_1.pdf",
+                "application/pdf",
+                str(d / "attachment_1.pdf"),
+            ),
+        )
+    raw.commit()
+    raw.close()
+    s = Store(str(db_path), storage_dir=storage)
+    return s, key
+
+
+def test_plan_finds_group_and_keeps_oldest(tmp_path: Path):
+    s, _ = _seed(tmp_path)
+    groups = mod.plan(s._con())
+    assert len(groups) == 1
+    g = groups[0]
+    assert g.keep == "AAAAAAAA"
+    assert sorted(g.remove) == ["BBBBBBBB", "CCCCCCCC"]
+
+
+def test_apply_removes_rows_and_files(tmp_path: Path):
+    s, _ = _seed(tmp_path)
+    rep = mod.apply(s._con(), mod.plan(s._con()))
+    assert rep.rows_removed == 2
+    assert rep.files_removed == 2
+    assert s._con().execute("SELECT count(*) FROM attachments").fetchone()[0] == 1
+    assert (tmp_path / "storage" / "AAAAAAAA" / "attachment_1.pdf").is_file()
+    assert not (tmp_path / "storage" / "BBBBBBBB").exists()
+    assert mod.plan(s._con()) == []
+
+
+def test_refuses_group_with_differing_sizes(tmp_path: Path):
+    s, _ = _seed(tmp_path)
+    (tmp_path / "storage" / "CCCCCCCC" / "attachment_1.pdf").write_bytes(
+        b"different-longer"
+    )
+    groups = mod.plan(s._con())
+    assert groups[0].conflict is True
+    rep = mod.apply(s._con(), groups)
+    assert rep.rows_removed == 0 and rep.skipped_conflicts == 1
+
+
+def test_unique_index_created_only_when_clean(tmp_path: Path):
+    s, _ = _seed(tmp_path)
+    s.close()
+    s2 = Store(str(tmp_path / "bib.sqlite"), storage_dir=tmp_path / "storage")
+    idx = {
+        r[0]
+        for r in s2._con().execute("SELECT name FROM sqlite_master WHERE type='index'")
+    }
+    assert "idx_attachments_item_filename" not in idx
+    mod.apply(s2._con(), mod.plan(s2._con()))
+    s2.close()
+    s3 = Store(str(tmp_path / "bib.sqlite"), storage_dir=tmp_path / "storage")
+    idx = {
+        r[0]
+        for r in s3._con().execute("SELECT name FROM sqlite_master WHERE type='index'")
+    }
+    assert "idx_attachments_item_filename" in idx