feat(comments): attach combined.md as a Zotero note (HTML), not files
Replaces the per-attachment .md attach_file loop with a single attach_note call per comment item: combined.md rendered to HTML via rex.comments.render. Sibling .md files keep being written to disk for non-Zotero corpus consumers. Idempotency cache shifts from (item_key, filename) to a per-item "Comment text" title check. Migration of the 33,351 existing .md attachments lands in a follow-up one-shot script. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -31,9 +31,10 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
||||
once at startup — bib has 164k+ rows and the URL → comment_id parse
|
||||
can't use any index, so we materialize the whole map up front.
|
||||
|
||||
``attach_callback(comment_id, md_path) -> None`` runs on the main
|
||||
thread and registers combined.md as a bib attachment, skipping items
|
||||
that already have one. Returns None when --no-bib or --no-attach.
|
||||
``attach_callback(comment_id, comment_dir) -> None`` runs on the main
|
||||
thread and registers a single ``"Comment text"`` note on the bib item
|
||||
containing combined.md rendered to HTML. Skips items that already have
|
||||
one. Returns None when --no-bib or --no-attach.
|
||||
"""
|
||||
if not use_bib:
|
||||
return lambda _cid: "", None
|
||||
@@ -61,17 +62,18 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
||||
return body_lookup, None
|
||||
|
||||
from bib import connect
|
||||
from rex.comments.render import render_combined_md
|
||||
|
||||
store = connect()
|
||||
# Pre-load every existing markdown attachment so re-runs are no-ops
|
||||
# at the per-(item, filename) level. Both combined.md and the per-
|
||||
# attachment <name>.md siblings live in this set.
|
||||
existing: set[tuple[str, str]] = {
|
||||
(row[0], row[1])
|
||||
# Items that already have a "Comment text" note — keeps re-runs idempotent.
|
||||
NOTE_TITLE = "Comment text"
|
||||
items_with_note: set[str] = {
|
||||
row[0]
|
||||
for row in store._con().execute(
|
||||
"SELECT i.key, a.filename FROM attachments a "
|
||||
"JOIN items i ON a.item_id = i.id "
|
||||
"WHERE a.filename LIKE '%.md'"
|
||||
"SELECT i.key FROM notes n "
|
||||
"JOIN items i ON n.item_id = i.id "
|
||||
"WHERE n.title = ?",
|
||||
(NOTE_TITLE,),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -80,22 +82,17 @@ def _build_bib_helpers(use_bib: bool, attach: bool):
|
||||
if not info:
|
||||
return # comment not in bib
|
||||
item_key = info[0]
|
||||
for md_path in sorted(comment_dir.glob("*.md")):
|
||||
# Don't attach the aggregated combined.md — it duplicates the
|
||||
# text already in the per-attachment siblings, doubles the
|
||||
# row/file count in Zotero, and slows the library down.
|
||||
# combined.md still lives on disk for `stack comments stats`
|
||||
# to index it.
|
||||
if md_path.name == "combined.md":
|
||||
continue
|
||||
key = (item_key, md_path.name)
|
||||
if key in existing:
|
||||
continue
|
||||
try:
|
||||
store.attach_file(item_key, md_path, title=md_path.name)
|
||||
existing.add(key)
|
||||
except Exception as e: # noqa: BLE001
|
||||
log.warning("attach %s failed for %s: %s", md_path.name, item_key, e)
|
||||
if item_key in items_with_note:
|
||||
return
|
||||
combined = comment_dir / "combined.md"
|
||||
if not combined.is_file():
|
||||
return # extraction must have failed; nothing to attach
|
||||
try:
|
||||
html = render_combined_md(combined.read_text(encoding="utf-8"))
|
||||
store.attach_note(item_key, html, title=NOTE_TITLE)
|
||||
items_with_note.add(item_key)
|
||||
except Exception as e: # noqa: BLE001
|
||||
log.warning("attach_note failed for %s: %s", item_key, e)
|
||||
|
||||
return body_lookup, attach_callback
|
||||
|
||||
|
||||
@@ -59,11 +59,10 @@ def test_stats_after_extract(tmp_path: Path):
|
||||
assert "1" in result.output # at least one comment counted
|
||||
|
||||
|
||||
def test_extract_attaches_only_sibling_mds_to_bib(tmp_path: Path, monkeypatch):
|
||||
"""End-to-end: extract writes combined.md (for `stats` indexing) plus
|
||||
per-attachment sibling MDs, but only the siblings get attached to the
|
||||
bib item — combined.md is intentionally skipped to avoid duplicating
|
||||
the same text twice in Zotero. Idempotent on re-run."""
|
||||
def test_extract_attaches_combined_md_as_note(tmp_path: Path, monkeypatch):
|
||||
"""Extract writes combined.md and per-attachment sibling MDs to disk.
|
||||
Only one Zotero-bound artifact: a single bib note titled 'Comment text'
|
||||
whose content is the rendered-HTML of combined.md. No file attachments."""
|
||||
from bib import connect
|
||||
from bib.item import Source
|
||||
|
||||
@@ -83,14 +82,15 @@ def test_extract_attaches_only_sibling_mds_to_bib(tmp_path: Path, monkeypatch):
|
||||
|
||||
result = runner.invoke(app, ["extract", "--root", str(tmp_path), "--workers", "1"])
|
||||
assert result.exit_code == 0, result.output
|
||||
assert (cdir / "combined.md").is_file() # still on disk for stats
|
||||
|
||||
# Disk siblings still written (kept for non-Zotero corpus consumers)
|
||||
assert (cdir / "combined.md").is_file()
|
||||
assert (cdir / "attachment_1.pdf.md").is_file()
|
||||
|
||||
def md_attachments():
|
||||
store = connect(str(bib_db))
|
||||
rows = sorted(
|
||||
r[0]
|
||||
for r in store._con().execute(
|
||||
rows = list(
|
||||
store._con().execute(
|
||||
"SELECT a.filename FROM attachments a "
|
||||
"JOIN items i ON a.item_id=i.id "
|
||||
"WHERE i.key=? AND a.filename LIKE '%.md'",
|
||||
@@ -100,8 +100,30 @@ def test_extract_attaches_only_sibling_mds_to_bib(tmp_path: Path, monkeypatch):
|
||||
store.close()
|
||||
return rows
|
||||
|
||||
assert md_attachments() == ["attachment_1.pdf.md"]
|
||||
def notes():
|
||||
store = connect(str(bib_db))
|
||||
rows = list(
|
||||
store._con().execute(
|
||||
"SELECT n.title, n.content FROM notes n "
|
||||
"JOIN items i ON n.item_id=i.id WHERE i.key=?",
|
||||
(item_key,),
|
||||
)
|
||||
)
|
||||
store.close()
|
||||
return rows
|
||||
|
||||
# Re-running must not duplicate attachments
|
||||
# No file attachments — old behavior is gone.
|
||||
assert md_attachments() == []
|
||||
|
||||
# Exactly one note, rendered HTML.
|
||||
rows = notes()
|
||||
assert len(rows) == 1
|
||||
title, content = rows[0]
|
||||
assert title == "Comment text"
|
||||
assert "<h1>" in content or "<h2>" in content
|
||||
assert "Long body content" in content # the seeded PDF text shows up
|
||||
|
||||
# Idempotent on re-run
|
||||
runner.invoke(app, ["extract", "--root", str(tmp_path), "--workers", "1"])
|
||||
assert md_attachments() == ["attachment_1.pdf.md"]
|
||||
assert len(notes()) == 1
|
||||
assert md_attachments() == []
|
||||
|
||||
Reference in New Issue
Block a user