Fix Last Changed updates during ePub rebuilds

Update Last Changed only when Bindery stores a new chapter or adds a new
chapter revision. Manual ePub generation, rebuilds, metadata edits and
rule changes leave it unchanged.

Save the timestamp in the same transaction as the chapter or revision.
This preserves the date if a later build fails and prevents retries from
moving it forward without another chapter change.

Add regression tests for new chapters, revisions, forced rebuilds, failed
builds, retries and transaction rollback. Adjust OPDS test data setup and
document what Last Changed records.

README.md

@@ -321,6 +321,9 @@ The catalog has three lists, with 50 books per page:
 - By Author, in surname order. Each author leads to their books. A book shows under each of its authors.
 - By Last Changed, with the newest change first.
 
+Last Changed records when Bindery stores a new chapter or a new revision of an existing chapter.
+Rebuilds, metadata edits and rule changes do not update this date.
+
 A book appears when it has a stored, included chapter. It shows its authors, series, description, tags,
 language and cover, with your kept values in place of the published values. A reader can search by title,
 author, series and tag. A download uses the stored chapters, the current ePub rules and the download

bindery/orchestrator.py

@@ -1200,9 +1200,8 @@ def _run_rebuild(job: Job) -> str:
     force = bool(job.options.get("force"))
     if not force and key == _done_key(story["id"]):
         return "No changes"
-    message, changed = _send(story, overrides, key, story["title"], story["author"], force)
+    message, _ = _send(story, overrides, key, story["title"], story["author"], force)
-    changes: dict[str, Any] = {"last_changed": now()} if changed or force else {}
+    store.update_story(story["id"], pinned=pinned, status="ok", message=message)
-    store.update_story(story["id"], pinned=pinned, status="ok", message=message, **changes)
     return message
 
 
@@ -1454,9 +1453,7 @@ def _run_site_job(job: Job) -> str:
         return f"No changes. {summary}"
 
     _progress(job, "Building epub", 1.0)
-    message, book_changed = _send(story, overrides, key, title, snap.author)
+    message, _ = _send(story, overrides, key, title, snap.author)
-    if book_changed:
-        changes["last_changed"] = now()
     store.update_story(story_id, **changes, message=message, status="error" if failed else "ok")
     return f"{summary}. {message}"
 

bindery/store.py

@@ -273,15 +273,17 @@ class Store:
         conn: sqlite3.Connection, story_id: int, chapter: dict[str, str], source: str, position: int
     ) -> int:
         """Store a chapter with its first revision. The chapter needs url, title, html and text_hash."""
+        changed_at = now()
         cur = conn.execute(
             "INSERT INTO chapters (story_id, position, url, title, added_at) VALUES (?, ?, ?, ?, ?)",
-            (story_id, position, chapter["url"], chapter["title"], now()),
+            (story_id, position, chapter["url"], chapter["title"], changed_at),
         )
         chapter_id = int(cur.lastrowid)
         conn.execute(
             "INSERT INTO revisions (chapter_id, rev, html, text_hash, source, fetched_at) VALUES (?, 1, ?, ?, ?, ?)",
-            (chapter_id, chapter["html"], chapter["text_hash"], source, now()),
+            (chapter_id, chapter["html"], chapter["text_hash"], source, changed_at),
         )
+        conn.execute("UPDATE stories SET last_changed = ? WHERE id = ?", (changed_at, story_id))
         return chapter_id
 
     def add_chapter(
@@ -326,6 +328,7 @@ class Store:
 
     def add_revision(self, chapter_id: int, html: str, text_hash: str, source: str) -> int:
         """Store a new revision and make it the current one."""
+        changed_at = now()
         with self._connect() as conn:
             rev = conn.execute(
                 "SELECT COALESCE(MAX(rev), 0) + 1 FROM revisions WHERE chapter_id = ?", (chapter_id,)
@@ -333,9 +336,15 @@ class Store:
             conn.execute(
                 "INSERT INTO revisions (chapter_id, rev, html, text_hash, source, fetched_at) "
                 "VALUES (?, ?, ?, ?, ?, ?)",
-                (chapter_id, rev, html, text_hash, source, now()),
+                (chapter_id, rev, html, text_hash, source, changed_at),
+            )
+            conn.execute(
+                "UPDATE chapters SET current_rev = ?, changed_at = ? WHERE id = ?", (rev, changed_at, chapter_id)
+            )
+            conn.execute(
+                "UPDATE stories SET last_changed = ? WHERE id = (SELECT story_id FROM chapters WHERE id = ?)",
+                (changed_at, chapter_id),
             )
-            conn.execute("UPDATE chapters SET current_rev = ?, changed_at = ? WHERE id = ?", (rev, now(), chapter_id))
             return int(rev)
 
     def update_chapter(self, chapter_id: int, **fields: Any) -> None:

tests/test_build_keys.py

@@ -12,6 +12,7 @@ from test_deliver import jobs as jobs
 from test_deliver import story_epub as story_epub
 
 from bindery import config, orchestrator, pages
+from bindery import store as storage
 from bindery.jobs import DownloadMode, Job
 
 ONE = [(1, "One", "a")]
@@ -44,13 +45,21 @@ def test_an_unchanged_story_is_not_built_or_sent_again(builds, monkeypatch):
     assert len(ingest_files()) == 2  # one epub and its sidecar
 
 
-@pytest.mark.parametrize("change", ["chapter", "rule", "global rule", "paragraphs", "metadata", "included"])
+@pytest.mark.parametrize(
+    "change", ["chapter", "revision", "renamed", "rule", "global rule", "paragraphs", "metadata", "included"]
+)
 def test_each_change_builds_and_sends_the_book(builds, monkeypatch, change):
     story_id = linked_story([(1, "One", "a"), (2, "Two", "b")])
     site(monkeypatch, [(1, "One", "a"), (2, "Two", "b")])
     update(story_id)
+    previous = "2020-01-01T00:00:00"
+    orchestrator.store.update_story(story_id, last_changed=previous)
     if change == "chapter":
         site(monkeypatch, [(1, "One", "a"), (2, "Two", "b"), (3, "Three", "c")])
+    elif change == "revision":
+        site(monkeypatch, [(1, "One", "a"), (2, "Two", "new text")])
+    elif change == "renamed":
+        site(monkeypatch, [(1, "One", "a"), (2, "Renamed", "b")])
     elif change == "rule":
         orchestrator.save_rules(story_id, [{"pattern": "a", "replacement": "b"}])
     elif change == "global rule":
@@ -62,10 +71,14 @@ def test_each_change_builds_and_sends_the_book(builds, monkeypatch, change):
     else:
         chapter = orchestrator.store.list_chapters(story_id)[0]
         orchestrator.store.update_chapter(chapter["id"], included=0)
-    assert "library book 1" in update(story_id)
+    mode = DownloadMode.RECHECK_ALL if change == "revision" else DownloadMode.NEW
+    assert "library book 1" in update(story_id, mode)
     assert builds.call_count == 2
     assert len(ingest_files()) == 4
+    changed_at = story(story_id)["last_changed"]
+    assert (changed_at != previous) == (change in ("chapter", "revision"))
     assert update(story_id).startswith("No changes")
+    assert story(story_id)["last_changed"] == changed_at
 
 
 def test_a_held_book_that_did_not_change_is_not_built_again(builds, monkeypatch):
@@ -110,13 +123,14 @@ def test_a_lost_file_is_built_again_only_to_send_it(builds):
     assert builds.call_count == 2
 
 
-def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(builds, client, jobs):
+@pytest.mark.parametrize("last_changed", [None, "2020-01-01T00:00:00"])
+def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(builds, client, jobs, last_changed):
     story_id = add_story(ingested_at="2026-01-01T00:00:00", calibre_id=1, calibre_uuid="uuid-1")
     job = Job(mode=DownloadMode.REBUILD, story_id=story_id)
     assert "library book 1" in orchestrator._run_rebuild(job)
-    orchestrator.store.update_story(story_id, last_changed=None, message="before")
+    orchestrator.store.update_story(story_id, last_changed=last_changed, message="before")
     assert orchestrator._run_rebuild(job) == "No changes"
-    assert (story(story_id)["last_changed"], story(story_id)["message"]) == (None, "before")
+    assert (story(story_id)["last_changed"], story(story_id)["message"]) == (last_changed, "before")
     assert builds.call_count == 1
     # The Rebuild button of the story page.
     queued = client.post(f"/api/stories/{story_id}/download", json={"mode": "rebuild"}, headers=H).json
@@ -124,7 +138,7 @@ def test_a_rebuild_without_a_change_does_nothing_unless_the_user_asks_for_it(bui
     assert "library book 1" in orchestrator._run_rebuild(jobs.get_next())
     assert builds.call_count == 2
     assert len(ingest_files()) == 4
-    assert story(story_id)["last_changed"]
+    assert story(story_id)["last_changed"] == last_changed
     # Other modes take no such option.
     queued = client.post(f"/api/stories/{story_id}/download", json={"mode": "new"}, headers=H).json
     assert queued["options"] == {}
@@ -152,6 +166,31 @@ def test_a_failed_delivery_is_tried_again_without_a_build(builds, fake, monkeypa
     assert builds.call_count == 1
 
 
+@pytest.mark.parametrize("change", ["chapter", "revision"])
+def test_chapter_timestamp_survives_a_failed_build_and_retry(builds, monkeypatch, change):
+    story_id = linked_story(ONE)
+    site(monkeypatch, ONE)
+    update(story_id)
+    previous = "2020-01-01T00:00:00"
+    orchestrator.store.update_story(story_id, last_changed=previous)
+    clock = ["2024-01-01T00:00:00"]
+    monkeypatch.setattr(storage, "now", lambda: clock[0])
+    site(monkeypatch, [*ONE, (2, "Two", "b")] if change == "chapter" else [(1, "One", "revised")])
+    job = Job(mode=DownloadMode.RECHECK_ALL, story_id=story_id)
+    build = orchestrator._build
+    monkeypatch.setattr(orchestrator, "_build", Mock(side_effect=OSError("disk full")))
+    with pytest.raises(OSError, match="disk full"):
+        orchestrator._run_site_job(job)
+    assert story(story_id)["last_changed"] == clock[0]
+    assert job.result["new"] or job.result["changed"]
+    clock[0] = "2024-02-01T00:00:00"
+    monkeypatch.setattr(orchestrator, "_build", build)
+    assert "library book 1" in orchestrator._run_site_job(job)
+    assert story(story_id)["last_changed"] == "2024-01-01T00:00:00"
+    assert update(story_id).startswith("No changes")
+    assert story(story_id)["last_changed"] == "2024-01-01T00:00:00"
+
+
 def test_a_new_build_removes_the_older_file(builds, monkeypatch):
     story_id = linked_story(ONE)
     site(monkeypatch, ONE)
@@ -227,6 +266,7 @@ def test_a_download_uses_the_built_book(builds, client, monkeypatch):
 
 def test_a_download_builds_a_book_that_has_no_file_or_that_changed(builds, client, monkeypatch):
     story_id = add_story()
+    changed_at = story(story_id)["last_changed"]
     first = download(client, story_id)
     download(client, story_id)
     assert builds.call_count == 1
@@ -234,12 +274,12 @@ def test_a_download_builds_a_book_that_has_no_file_or_that_changed(builds, clien
     assert download(client, story_id) != first
     assert builds.call_count == 2
     assert len(list(config.TMP_DIR.iterdir())) == 1
-    # The build of a download is not the build of a job. The job sends the book, and marks the change.
+    # The build of a download is not the build of a job. The job sends the book.
     assert orchestrator.store.get_setting(story_id, "built_key") is None
     job = Job(mode=DownloadMode.REBUILD, story_id=story_id)
     assert "library book 1" in orchestrator._run_rebuild(job)
     assert builds.call_count == 2
-    assert story(story_id)["last_changed"]
+    assert story(story_id)["last_changed"] == changed_at
 
 
 # Page counts

tests/test_last_changed.py

@@ -0,0 +1,66 @@
+"""Last Changed records stored chapters and revisions, not changes to the built book."""
+
+import sqlite3
+
+import pytest
+
+from bindery import store as storage
+
+
+@pytest.fixture
+def state(tmp_path, monkeypatch):
+    clock = ["2024-01-01T00:00:00"]
+    monkeypatch.setattr(storage, "now", lambda: clock[0])
+    store = storage.Store(tmp_path / "stories.db")
+    sid = store.create_story("story", "site", "Title", "Author", [])
+    return store, sid, clock
+
+
+@pytest.mark.parametrize("source", ["new", "import"])
+def test_new_chapters_and_revisions_set_the_timestamp_in_the_store(state, source):
+    store, sid, clock = state
+    other = store.create_story("other", "site", "Other", "Author", [])
+    assert store.get_story(sid)["last_changed"] is None
+    first = store.add_chapter(sid, "one", "One", "first", "h1", source, 0)
+    assert store.get_story(sid)["last_changed"] == clock[0]
+    assert store.get_chapter(first)["added_at"] == clock[0]
+    clock[0] = "2024-02-01T00:00:00"
+    store.add_chapters(sid, [{"url": "two", "title": "Two", "html": "second", "text_hash": "h2"}], source, [])
+    assert store.get_story(sid)["last_changed"] == clock[0]
+    clock[0] = "2024-03-01T00:00:00"
+    store.add_revision(first, "revised", "h3", "recheck")
+    assert store.get_story(sid)["last_changed"] == clock[0]
+    assert store.get_chapter(first)["changed_at"] == clock[0]
+    assert store.list_revisions(first)[-1]["fetched_at"] == clock[0]
+    assert storage.Store(store.path).get_story(sid)["last_changed"] == clock[0]
+    assert store.get_story(other)["last_changed"] is None
+
+
+def test_changes_without_new_chapters_or_revisions_keep_the_timestamp(state):
+    store, sid, clock = state
+    first = store.add_chapter(sid, "one", "One", "first", "h1", "new", 0)
+    second = store.add_chapter(sid, "two", "Two", "second", "h2", "new", 1)
+    store.add_revision(first, "revised", "h3", "recheck")
+    previous = clock[0]
+    clock[0] = "2024-02-01T00:00:00"
+    store.update_story(sid, title="Edited", pinned={"title": "Edited"})
+    store.set_rules(sid, [{"pattern": "first", "replacement": "1st"}])
+    store.update_chapter(first, title="Renamed", included=False, on_site=False, current_rev=1)
+    store.replace_revision(first, 1, "original site text", "h4")
+    store.reorder_chapters(sid, lambda order: list(reversed(order)))
+    store.add_chapters(sid, [], "import", ["one", "two"])
+    store.delete_revision(first, 2)
+    store.delete_chapters(sid, [second])
+    assert store.get_story(sid)["last_changed"] == previous
+
+
+def test_a_failed_chapter_transaction_does_not_change_the_timestamp(state):
+    store, sid, clock = state
+    store.add_chapter(sid, "one", "One", "first", "h1", "new", 0)
+    previous = clock[0]
+    clock[0] = "2024-02-01T00:00:00"
+    chapters = [{"url": url, "title": url, "html": "text", "text_hash": "h"} for url in ("two", "one")]
+    with pytest.raises(sqlite3.IntegrityError):
+        store.add_chapters(sid, chapters, "import", [])
+    assert store.get_story(sid)["last_changed"] == previous
+    assert [chapter["url"] for chapter in store.list_chapters(sid)] == ["one"]

tests/test_opds.py

@@ -48,10 +48,10 @@ def add_book(title="A book", author="Writer", changed=None, url=None, included=T
     store = orchestrator.store
     url = url or f"https://example.com/books/{len(store.list_stories()) + 1}"
     sid = store.create_story(url, "example.com", title, author, [])
-    store.update_story(sid, last_changed=changed, site_values=meta or {})
     if included:
         html = "<p>Stored text.</p>"
         store.add_chapter(sid, url + "/1", "One", html, diffing.text_hash(html), "site", 0)
+    store.update_story(sid, last_changed=changed, site_values=meta or {})
     return sid