fix(magazine): adding a region crashed after a byline split; undo could roll back later changes

original_text is now a Block field, so paths that rebuild blocks keep it
instead of crashing. Undo snapshots are sealed with a digest of the saved
state and refused once anything else changed; actions without their own undo
point clear the Undo button. v1.41.1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

CHANGELOG.md

@@ -7,6 +7,12 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
 
 ## [Unreleased]
 
+## [1.41.1] — 2026-09-22
+
+### Fixed
+- **Magazine: "Could not add region: … unexpected keyword argument 'original_text'".** After *Clean up boxes* split an author line, drawing a region (and re-OCR of a page) crashed on the extra field kept with that author line. The field is now part of the region model and survives every path.
+- **Undo could roll back more than the change it belonged to.** An Undo offered for a delete stayed valid while other changes that record no undo point of their own (drawing a region, fixing the reading order, clean-up, approving) happened afterwards — pressing it would have reverted those too. Undo now refuses as soon as anything else has changed, and those actions clear the Undo button.
+
 ## [1.41.0] — 2026-09-21
 
 ### Changed

VERSION

@@ -1 +1 @@
-1.41.0
+1.41.1

architecture/magazine-ingestion.md

@@ -347,7 +347,10 @@ membership like the single-region routes. Every destructive route (delete, type
 split, bulk) writes `undo.json` = {token, label, layout, articles} *after* validating and *before* saving
 (the files on disk still hold the old state), and returns the token; `POST .../undo {token}` restores both
 files only if that token is still the latest (409 otherwise), so an old Undo can never roll back a newer
-change. One level. UI: *Select boxes* mode (click or drag a rectangle; a box ≥ 50% inside is selected),
+change. One level. After the change is saved the snapshot is sealed with a digest of layout.json +
+articles.json (`_seal_undo`); undo also requires the current state to match that digest, so a later change that
+records no undo point (drawing, fix reading order, clean-up, approve) makes the old Undo invalid instead of being
+rolled back with it. UI: *Select boxes* mode (click or drag a rectangle; a box ≥ 50% inside is selected),
 a bulk bar, and an Undo bar replacing the per-delete confirmation dialog (per NN/g-style guidance: frequent,
 reversible actions get Undo, not "are you sure?").
 

core/magazine.py

@@ -116,9 +116,16 @@ class Block:
     region_label: str = ""
     continues_from_previous_page: bool = False
     source: str = "native"          # "native" | "ocr" | "vision_ocr" | "vision_label"
+    # The author line's text before an author line that ran into the first sentence was split
+    # (clean_up_regions). Part of the model so every path that rebuilds blocks keeps it -- and does
+    # not crash on it (drawing a region did, "unexpected keyword argument 'original_text'").
+    original_text: str = ""
 
     def to_json(self) -> dict[str, Any]:
-        return asdict(self)
+        data = asdict(self)
+        if not data["original_text"]:
+            del data["original_text"]
+        return data
 
 
 @dataclass

routes/magazine.py

@@ -19,6 +19,7 @@ from __future__ import annotations
 import asyncio
 import base64
 import html
+import hashlib
 import json
 import logging
 import re
@@ -756,13 +757,36 @@ def _add_split_paragraphs_to_articles(articles: list[dict], report: dict) -> boo
 
 def _snapshot_for_undo(d: Path, label: str) -> str:
     """Keep the layout and articles as they are now, so the change about to happen can be undone.
-    One level: the next snapshot replaces this one. Returns the token the client undoes with."""
+    One level: the next snapshot replaces this one. Returns the token the client undoes with.
+    The caller seals it with _seal_undo once the change is saved."""
     token = uuid.uuid4().hex
     snapshot = {"token": token, "label": label, "layout": mag.load_layout(d), "articles": mag.load_articles(d)}
     (d / "undo.json").write_text(json.dumps(snapshot, ensure_ascii=False), "utf-8")
     return token
 
 
+def _state_digest(d: Path) -> str:
+    """A fingerprint of the saved layout and articles."""
+    h = hashlib.sha256()
+    for name in ("layout.json", "articles.json"):
+        path = d / name
+        h.update(path.read_bytes() if path.is_file() else b"-")
+    return h.hexdigest()
+
+
+def _seal_undo(d: Path | None, token: str | None) -> None:
+    """Record what the state looks like right after the undoable change. Undo is refused once the
+    state differs from that -- any later change (drawing a region, fixing the reading order,
+    approving) would otherwise be rolled back silently with it."""
+    path = d / "undo.json" if d is not None and token else None
+    if path is None or not path.is_file():
+        return
+    snapshot = json.loads(path.read_text("utf-8"))
+    if snapshot.get("token") == token:
+        snapshot["after"] = _state_digest(d)
+        path.write_text(json.dumps(snapshot, ensure_ascii=False), "utf-8")
+
+
 @router.post("/api/magazine/{doc_id}/undo")
 async def magazine_undo(doc_id: str, request: Request):
     """Undo the last region change (delete, type change, merge, split, bulk). Body: {token}.
@@ -774,7 +798,7 @@ async def magazine_undo(doc_id: str, request: Request):
     if not isinstance(token, str) or not path.is_file():
         raise HTTPException(409, "Nothing to undo")
     snapshot = json.loads(path.read_text("utf-8"))
-    if snapshot.get("token") != token:
+    if snapshot.get("token") != token or snapshot.get("after") != _state_digest(d):
         raise HTTPException(409, "Something else has changed since -- this can no longer be undone")
     mag.save_layout(d, snapshot["layout"])
     mag.save_articles(d, snapshot["articles"])
@@ -840,6 +864,7 @@ async def magazine_bulk_regions(doc_id: str, page: int, request: Request):
                     changed = True
     if changed:
         mag.save_articles(d, articles)
+    _seal_undo(d, token)
     return {"ok": True, "undo": token, "remap": remap, "count": len(ids)}
 
 
@@ -866,6 +891,7 @@ async def magazine_merge_region(doc_id: str, page: int, block_id: str):
             changed = True
     if changed:
         mag.save_articles(d, articles)
+    _seal_undo(d, token)
     return {"ok": True, "id": block_id, "removed": removed, "undo": token}
 
 
@@ -890,6 +916,7 @@ async def magazine_split_region(doc_id: str, page: int, block_id: str, request:
     articles = mag.load_articles(d)
     if _add_split_paragraphs_to_articles(articles, {"byline_split": [{"id": block_id, "new_id": new_id}]}):
         mag.save_articles(d, articles)
+    _seal_undo(d, token)
     return {"ok": True, "id": block_id, "new_id": new_id, "undo": token}
 
 
@@ -1095,6 +1122,7 @@ async def magazine_update_region(doc_id: str, page: int, block_id: str, request:
     mag.save_layout(d, layout)
     if region_id != block_id:
         _move_article_membership(d, block_id, region_id, to_image=new_type == "image")
+    _seal_undo(d, token)
     return {"ok": True, "text": text, "id": region_id, "type": new_type, "undo": token}
 
 
@@ -1142,6 +1170,7 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str):
                 changed = True
     if changed:
         mag.save_articles(d, articles)
+    _seal_undo(d, token)
     return {"ok": True, "undo": token}
 
 

static/index.html

@@ -27,12 +27,12 @@
 
   <!-- ── Core styles (local — no CDN dependency for first paint) ────────── -->
   <link rel="stylesheet" href="/static/vendor/mdi/materialdesignicons.min.css">
-<link rel="stylesheet" href="/static/style.css?v=1.41.0">
-<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.41.0">
-<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.41.0">
-<link rel="stylesheet" href="/static/css/voice-design.css?v=1.41.0">
-<link rel="stylesheet" href="/static/css/rehearser.css?v=1.41.0">
-<link rel="stylesheet" href="/static/css/audiobook.css?v=1.41.0">
+<link rel="stylesheet" href="/static/style.css?v=1.41.1">
+<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.41.1">
+<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.41.1">
+<link rel="stylesheet" href="/static/css/voice-design.css?v=1.41.1">
+<link rel="stylesheet" href="/static/css/rehearser.css?v=1.41.1">
+<link rel="stylesheet" href="/static/css/audiobook.css?v=1.41.1">
 
 
   <!-- ── Flag icons — non-blocking (loaded async, icons appear after JS) ── -->

static/js/magazine.js

@@ -1217,6 +1217,7 @@ async function magOrderPut(order) {
 }
 
 async function magOrderSave() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const seq = (state.orderSeq || []).slice();
   if (!seq.length) return;
   try {
@@ -1231,6 +1232,7 @@ async function magOrderSave() {
 }
 
 async function magOrderMove(id, value) {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const order = (state.reviewRegionOrder || []).filter((x) => x !== id);
   const to = Math.min(Math.max(1, parseInt(value, 10) || 1), order.length + 1) - 1;
   order.splice(to, 0, id);
@@ -1245,6 +1247,7 @@ async function magOrderMove(id, value) {
 }
 
 async function magOrderReset() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   try {
     await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/reading-order', { method: 'DELETE' });
   } catch (e) {
@@ -1596,6 +1599,7 @@ async function magReviewSubmitEdit(reocr) {
 }
 
 async function magCleanUpBoxes() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const ok = await confirmDialog('Clean up the boxes of this whole issue: text inside pictures, OCR noise (boxes of one or two characters) ' +
     'and boxes that repeat a bigger box\'s text stop being read aloud, and author lines that run into the first sentence are split. ' +
     'Nothing is deleted — every change shows in the review list and can be undone with the type picker — and a backup of the layout is kept.',
@@ -1620,6 +1624,7 @@ async function magCleanUpBoxes() {
 }
 
 async function magFixReadingOrder() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const ok = await confirmDialog('Re-read the reading order of every page in this issue, column by column. ' +
     'Text, boxes and approvals stay as they are, but the order in which approved articles are read aloud can change. ' +
     'Pages you ordered by hand are kept.', { title: 'Fix reading order?', okLabel: 'Fix reading order' });
@@ -1911,6 +1916,7 @@ function magReviewOpenDrawPanel(box) {
 }
 
 async function magReviewSubmitDraw() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const box = $('mag-draw-rect');
   const pageMeta = (state.layout.pages || []).find((p) => p.page === state.reviewPage);
   if (!box || !pageMeta) return;
@@ -2020,6 +2026,7 @@ function selectedBlockIds() {
 }
 
 async function magReocrPage() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const page = state.reviewPage;
   if (page == null) return;
   const btn = $('mag-reocr-btn');
@@ -2102,6 +2109,7 @@ async function magRefineFigures() {
 }
 
 async function magApprove() {
+  magOfferUndo(null);   // this change records no undo point, so an older Undo no longer applies
   const a = state.article;
   if (!a) return;
   const blocks = selectedBlockIds();

tests/magazine.spec.js

@@ -629,3 +629,15 @@ test('clicking boxes in select mode builds a selection whose type changes at onc
   await page.locator('#mag-bulk-type').selectOption('ocr_noise');
   await expect.poll(() => sent).toEqual({ ids: ['b1', 'b2'], action: 'type', type: 'ocr_noise' });
 });
+
+test('an action without its own undo point clears the Undo button', async ({ page }) => {
+  await mockBackend(page);
+  await page.route(`**/api/magazine/${DOC_ID}/pages/3/regions/b3`, route => route.fulfill({ json: { ok: true, undo: 'tok-9' } }));
+  await page.route(`**/api/magazine/${DOC_ID}/reading-order`, route => route.fulfill({ json: { ok: true, pages_changed: [], pages_locked: 0 } }));
+  await openReview(page);
+  await page.locator('[data-block-row="b3"]').getByRole('button', { name: /Delete region/ }).click();
+  await expect(page.locator('#mag-undo-bar')).toContainText('Region deleted.');
+  await page.getByRole('button', { name: 'Fix reading order', exact: true }).click();
+  await page.getByRole('alertdialog').getByRole('button', { name: 'Fix reading order', exact: true }).click();
+  await expect(page.locator('#mag-undo-bar')).toBeEmpty();
+});

tests/test_magazine_routes.py

@@ -1123,3 +1123,45 @@ class BulkUndoFoldTests(unittest.TestCase):
                 self.assertEqual(err.exception.status_code, 400)
         self.assertEqual(len(mag.load_layout(self.dir)["blocks"]), 4)
         self.assertFalse((self.dir / "undo.json").exists())
+
+
+class UndoSafetyAndExtraFieldTests(BulkUndoFoldTests):
+    """Undo never rolls back a later change; a split author line's extra field survives every path."""
+
+    def test_undo_is_refused_once_anything_else_changed(self):
+        from core import magazine as mag
+        from routes.magazine import magazine_bulk_regions, magazine_undo
+        out = self.run_route(magazine_bulk_regions, 1, body={"ids": ["lg1"], "action": "delete"})
+        # A change that records no undo point of its own, e.g. drawing a region or fixing the order.
+        layout = mag.load_layout(self.dir)
+        layout["blocks"][0]["text"] = "edited later"
+        mag.save_layout(self.dir, layout)
+        with self.assertRaises(HTTPException) as err:
+            self.run_route(magazine_undo, body={"token": out["undo"]})
+        self.assertEqual(err.exception.status_code, 409)
+        self.assertEqual(mag.load_layout(self.dir)["blocks"][0]["text"], "edited later")
+
+    def test_an_unsealed_old_snapshot_is_refused(self):
+        import json
+        from routes.magazine import magazine_undo
+        from core import magazine as mag
+        (self.dir / "undo.json").write_text(json.dumps({"token": "t", "label": "x", "layout": mag.load_layout(self.dir),
+                                                        "articles": mag.load_articles(self.dir)}), "utf-8")
+        with self.assertRaises(HTTPException):
+            self.run_route(magazine_undo, body={"token": "t"})
+
+    def test_drawing_a_region_on_a_layout_with_a_split_author_line_works(self):
+        """"Could not add region: Block.__init__() got an unexpected keyword argument 'original_text'"."""
+        from core.magazine import add_manual_region, clean_up_regions
+        self.layout["blocks"].append({"id": "by", "page": 1, "bbox": [50, 20, 300, 30], "type": "byline",
+                                      "text": "Von Jo Bager KI gilt als Antwort", "reading_order": 9})
+        cleaned, report = clean_up_regions(self.layout)
+        self.assertEqual(len(report["byline_split"]), 1)
+        after = add_manual_region(self.dir, cleaned, 1, [400, 700, 100, 40], "text", "New text")
+        byline = next(b for b in after["blocks"] if b["id"] == "by")
+        self.assertIn("KI gilt als Antwort", byline["original_text"])
+        self.assertTrue(any(b["text"] == "New text" for b in after["blocks"]))
+
+    def test_blocks_without_the_field_do_not_gain_it(self):
+        from core.magazine import Block
+        self.assertNotIn("original_text", Block(id="x", page=1, bbox=[0, 0, 1, 1], type="body", text="t").to_json())