From b9c353c41af43df186c93946bff1d84b96cda1be Mon Sep 17 00:00:00 2001 From: kert Date: Tue, 8 Sep 2026 13:54:20 -0400 Subject: [PATCH] fix(llm): rule fingerprint includes updated_at; Zotero snapshot honours the WAL (refs #615) --- src/llm/source.py | 39 ++++++++++++++---- tests/llm/test_source_refs.py | 78 +++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/llm/source.py b/src/llm/source.py index 4694452..3fcd51b 100644 --- a/src/llm/source.py +++ b/src/llm/source.py @@ -23,6 +23,7 @@ so parsing the comment id is the reliable path to a comment's docket. from __future__ import annotations import logging +import os import shutil import sqlite3 from dataclasses import dataclass @@ -295,7 +296,9 @@ def iter_rule_refs( if keys and key not in keys: continue if row["anchor_sha"]: - fp = f"anchors:{row['anchor_sha']}" + # updated_at too: the anchor sha covers the FR body, not the + # item's own metadata (title, cms-rule: tag, date). + fp = f"anchors:{row['anchor_sha']}|{row['updated_at']}" else: fp = ( fingerprint_files(_attachment_paths(store, key)) @@ -355,6 +358,20 @@ def iter_rule_docs( yield doc +def _source_mtime_ns(src: Path) -> int: + """Newest mtime across zotero.sqlite and its ``-wal`` sidecar. + + Zotero commits into the WAL and only touches the main database at a + checkpoint, so ``src.stat()`` alone reports a database that has not + changed for weeks while the library is being edited daily. + """ + newest = src.stat().st_mtime_ns if src.exists() else 0 + wal = src.with_name(src.name + "-wal") + if wal.exists(): + newest = max(newest, wal.stat().st_mtime_ns) + return newest + + class ZoteroPdfIndex: """bib/Zotero item key → storage PDFs, read from a *snapshot copy* of zotero.sqlite (the live file is locked by the Zotero desktop and its @@ -376,13 +393,19 @@ class ZoteroPdfIndex: def _load() -> dict[str, list[Path]]: snap = Path(tmp_dir) / "zotero.sqlite" src = Path(sqlite_path) - if ( - snap.exists() - and src.exists() - and snap.stat().st_mtime_ns >= src.stat().st_mtime_ns - ): + # Zotero writes to zotero.sqlite-wal and only touches the main + # file at a checkpoint, so the main mtime alone would leave a + # stale snapshot in place indefinitely — take the newer of the two. + newest = _source_mtime_ns(src) + if snap.exists() and src.exists() and snap.stat().st_mtime_ns >= newest: return cls._read(snap, Path(storage_dir)) - return cls.snapshot(src, storage_dir, Path(tmp_dir))._by_key + index = cls.snapshot(src, storage_dir, Path(tmp_dir)) + if snap.exists() and newest: + # copy2 carries the *main* file's mtime, which is older + # than the WAL; stamp what we actually captured so the + # next run doesn't re-copy 1.95 GB for nothing. + os.utime(snap, ns=(newest, newest)) + return index._by_key inst._loader = _load return inst @@ -397,8 +420,6 @@ class ZoteroPdfIndex: tmp_dir.mkdir(parents=True, exist_ok=True) snap = tmp_dir / "zotero.sqlite" try: - # copy2 preserves the source mtime, so a later lazy() sees - # snap.mtime >= src.mtime until Zotero writes again. shutil.copy2(sqlite_path, snap) except OSError as e: log.warning("zotero snapshot failed (%s) — no Zotero PDF fallback", e) diff --git a/tests/llm/test_source_refs.py b/tests/llm/test_source_refs.py index 00bfed3..9540aa0 100644 --- a/tests/llm/test_source_refs.py +++ b/tests/llm/test_source_refs.py @@ -113,25 +113,44 @@ class TestCommentRefs: assert len(selects) == 1 +def _anchored_rule(store) -> str: + """A rule with one grabbed FR anchor paragraph (sha256 "abc").""" + key = store.create(Rule(title="R", url="https://fr/1", document_number="2019-1")) + store._con().execute( + "INSERT INTO fr_anchor_docs (item_key, document_number, html_url, start_page, end_page, fr_volume, sha256) VALUES (?,?,?,?,?,?,?)", + (key, "2019-1", "https://fr/1", 1, 2, 84, "abc"), + ) + store._con().execute( + "INSERT INTO fr_anchors (item_key, p_id, page, ordinal, text) VALUES (?,?,?,?,?)", + (key, 1, 1, 1, "Para one."), + ) + return key + + class TestRuleRefs: def test_fingerprint_from_anchor_sha(self, store): - key = store.create( - Rule(title="R", url="https://fr/1", document_number="2019-1") - ) - store._con().execute( - "INSERT INTO fr_anchor_docs (item_key, document_number, html_url, start_page, end_page, fr_volume, sha256) VALUES (?,?,?,?,?,?,?)", - (key, "2019-1", "https://fr/1", 1, 2, 84, "abc"), - ) - store._con().execute( - "INSERT INTO fr_anchors (item_key, p_id, page, ordinal, text) VALUES (?,?,?,?,?)", - (key, 1, 1, 1, "Para one."), - ) + key = _anchored_rule(store) refs = list(iter_rule_refs(store)) assert [r.key for r in refs] == [key] - assert refs[0].fingerprint == "anchors:abc" + assert refs[0].fingerprint.startswith("anchors:abc|") assert refs[0].collection == "rules" and refs[0].docket is None assert refs[0].load().text == "Para one." + def test_anchor_fingerprint_changes_with_updated_at(self, store): + """The sha covers the FR body, not the item's own metadata.""" + key = _anchored_rule(store) + f1 = next(iter_rule_refs(store)).fingerprint + store._con().execute( + "UPDATE items SET updated_at='2030-01-01T00:00:00Z' WHERE key=?", (key,) + ) + assert next(iter_rule_refs(store)).fingerprint != f1 + + def test_tag_filter_selects_only_tagged_rules(self, store): + tagged = store.create(Rule(title="Tagged", document_number="2019-2")) + store.add_tag(tagged, "project:pfs") + store.create(Rule(title="Untagged", document_number="2019-3")) + assert [r.key for r in iter_rule_refs(store, tag="project:pfs")] == [tagged] + class TestCorpusRefs: def test_excludes_comments_and_is_lazy(self, store): @@ -163,6 +182,12 @@ class TestCorpusRefs: refs[0].load() assert calls + def test_tag_filter_selects_only_tagged_items(self, store): + tagged = store.create(Item(item_type="report", title="T", abstract="Body.")) + store.add_tag(tagged, "project:pfs") + store.create(Item(item_type="report", title="U", abstract="Body.")) + assert [r.key for r in iter_corpus_refs(store, tag="project:pfs")] == [tagged] + class TestLazyZotero: def test_no_copy_until_first_lookup(self, tmp_path): @@ -190,3 +215,32 @@ class TestLazyZotero: z3 = ZoteroPdfIndex.lazy(src, tmp_path / "storage", snap_dir) z3.pdfs_for("ABCD1234") assert (snap_dir / "zotero.sqlite").stat().st_mtime_ns != m1 + + def test_newer_wal_forces_a_recopy(self, tmp_path): + """Zotero commits into zotero.sqlite-wal and only touches the main + file at a checkpoint — the WAL's mtime has to count too.""" + src = tmp_path / "zotero.sqlite" + import sqlite3 + + con = sqlite3.connect(src) + con.executescript( + "CREATE TABLE items(itemID INTEGER, key TEXT); CREATE TABLE itemAttachments(itemID INTEGER, parentItemID INTEGER, path TEXT); CREATE TABLE deletedItems(itemID INTEGER);" + ) + con.close() + snap_dir = tmp_path / "snap" + ZoteroPdfIndex.lazy(src, tmp_path / "storage", snap_dir).pdfs_for("ABCD1234") + snap = snap_dir / "zotero.sqlite" + m1 = snap.stat().st_mtime_ns + + wal = tmp_path / "zotero.sqlite-wal" + wal.write_bytes(b"wal") + bump = m1 + 10**9 + os.utime(wal, ns=(bump, bump)) # main file untouched, WAL is newer + + ZoteroPdfIndex.lazy(src, tmp_path / "storage", snap_dir).pdfs_for("ABCD1234") + m2 = snap.stat().st_mtime_ns + assert m2 != m1 # re-copied + # and the snapshot now records what it captured, so the next run + # doesn't re-copy 1.95 GB for the same unchanged WAL + ZoteroPdfIndex.lazy(src, tmp_path / "storage", snap_dir).pdfs_for("ABCD1234") + assert snap.stat().st_mtime_ns == m2