fix(magazine): snap and crop pictures per page during import, not only at the end

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

CHANGELOG.md

@@ -7,6 +7,11 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
 
 ## [Unreleased]
 
+## [1.42.3] — 2026-09-22
+
+### Fixed
+- **Magazine import preview: picture, chart and table boxes were drawn in the wrong place, and the recognised-text panel never showed them.** Both came from the same cause: the boxes were only snapped to the real picture edges and cropped to actual files once, after the whole magazine had finished importing — during the import itself the preview drew the vision model's raw, often-imprecise guess, and the text panel could only show a generic "Figure" placeholder because no crop file existed yet. Pictures, charts and tables are now snapped and cropped as soon as each page is labelled, so the live preview's boxes are accurate and the recognised-text panel shows the actual picture instead of a placeholder, while the import is still running.
+
 ## [1.42.2] — 2026-09-22
 
 ### Fixed

Manual.md

@@ -107,7 +107,12 @@ turned into a podcast discussion between two or three voices you pick.
    tables and code are only recognised in this second pass (the first pass
    only reads text, and says so under the page); they are drawn as dashed,
    filled boxes in their own colours — picture orange, chart green, table
-   yellow, code grey — so they stand out from the text boxes. The page
+   yellow, code grey — so they stand out from the text boxes, and fitted
+   to the actual picture as soon as that page is labelled (not only once
+   the whole magazine has finished), so the box does not lag behind the
+   real edges of the picture for long. The recognised-text panel shows the
+   actual cropped picture, chart or table for the same reason, rather than
+   a plain "Figure" placeholder, as soon as it has been cropped. The page
    scan itself (a scanned magazine is one picture per page) is not framed. If a run is interrupted
    (for example when the server restarts), the status now says so instead
    of spinning forever; start it again with *Re-run detection*.

VERSION

@@ -1 +1 @@
-1.42.2
+1.42.3

routes/magazine.py

@@ -123,6 +123,11 @@ _INGEST_PREVIEW_BUDGET = 8000
 _SCAN_IMAGE_SHARE = 0.85   # an embedded image covering this much of the page is the page scan itself
 
 
+def _is_scan_sized_image(bbox, w: float, h: float) -> bool:
+    _, _, bw, bh = (float(v) for v in bbox)
+    return bw * bh >= _SCAN_IMAGE_SHARE * w * h
+
+
 def _ingest_preview_boxes(blocks: list, images: list | None, size: tuple[float, float] | None) -> list[dict]:
     """The page's regions as boxes in percent of the page, in reading order, for the live preview."""
     if not size or not size[0] or not size[1]:
@@ -146,11 +151,11 @@ def _ingest_preview_boxes(blocks: list, images: list | None, size: tuple[float,
         # A crop of a region already drawn above would be the same box twice.
         if not bbox or get(img, "source_block") in drawn - {None}:
             continue
-        x, y, bw, bh = (float(v) for v in bbox)
         # A scanned magazine is one embedded image per page: framing the whole page as a
         # "picture" hides the real pictures, charts and tables found on it.
-        if bw * bh >= _SCAN_IMAGE_SHARE * w * h:
+        if _is_scan_sized_image(bbox, w, h):
             continue
+        x, y, bw, bh = (float(v) for v in bbox)
         boxes.append({"x": round(100 * x / w, 2), "y": round(100 * y / h, 2), "w": round(100 * bw / w, 2),
                       "h": round(100 * bh / h, 2), "type": "image", "n": None})
     return boxes[:400]
@@ -165,16 +170,27 @@ def _write_ingest_preview(d: Path, page: int, blocks: list, images: list | None,
     as_objects = [b if not isinstance(b, dict) else SimpleNamespace(
         bbox=b.get("bbox") or [0, 0, 0, 0], type=b.get("type") or mag.BLOCK_BODY,
         text=b.get("text") or "", font_size=float(b.get("font_size") or 0.0)) for b in blocks or []]
+    # The page scan itself (a scanned magazine is one embedded image per page) is not a
+    # picture on the page -- shown as a "Figure" it would swamp the real ones.
+    shown_images = images or []
+    if size and size[0] and size[1]:
+        w, h = float(size[0]), float(size[1])
+        shown_images = [i for i in shown_images if not _is_scan_sized_image(i.get("bbox") or [0, 0, 0, 0], w, h)]
     try:
         (d / "ingest_preview.json").write_text(json.dumps({
-            "page": page, "stage": stage, "text": _render_ingest_preview_html(as_objects, images),
+            "page": page, "stage": stage,
-            "boxes": _ingest_preview_boxes(blocks, images, size),
+            "text": _render_ingest_preview_html(as_objects, shown_images, doc_id=d.name),
+            "boxes": _ingest_preview_boxes(blocks, shown_images, size),
         }, ensure_ascii=False), "utf-8")
     except Exception:
         logging.exception("Failed to write live ingest preview for page %s", page)
 
 
-def _render_ingest_preview_html(blocks: list, images: list | None) -> str:
+_INGEST_PREVIEW_FIGURE_LABELS = {"figure": "Picture", "image": "Picture", "chart": "Chart",
+                                 "table": "Table", "code": "Code"}
+
+
+def _render_ingest_preview_html(blocks: list, images: list | None, doc_id: str = "") -> str:
     """Format one page's just-read blocks as simple HTML, in reading order.
 
     Best-effort presentation only, for the live "here is what was just
@@ -195,6 +211,11 @@ def _render_ingest_preview_html(blocks: list, images: list | None) -> str:
     that vision labelling is working -- but never promoted to a heading,
     regardless of its font size.
 
+    Once a picture, chart or table has been cropped to its own file (see
+    refresh_visual_regions, run per page as labelling finishes it) the
+    placeholder becomes the actual thumbnail instead of a bare icon, so a
+    reader can tell what was found without switching to the page image.
+
     Every text node is escaped; the caller writes the result straight into
     the page as innerHTML.
     """
@@ -210,9 +231,16 @@ def _render_ingest_preview_html(blocks: list, images: list | None) -> str:
     budget_used = 0
     for _, kind, item in items:
         if kind == "image":
+            label = _INGEST_PREVIEW_FIGURE_LABELS.get(str(item.get("type") or "").strip(), "Picture")
+            filename = str(item.get("filename") or "")
+            if doc_id and re.fullmatch(r"[A-Za-z0-9_-]{1,64}\.png", filename):
+                src = f"/api/magazine/{doc_id}/images/{filename}"
+                line = (f'<figure class="mag-ingest-preview-figure">'
+                        f'<img src="{html.escape(src)}" alt="{html.escape(label)} on this page" loading="lazy">'
+                        f'<figcaption>{html.escape(label)}</figcaption></figure>')
+            else:
                 line = ('<div class="mag-ingest-preview-figure">'
-                    '<span class="mdi mdi-image-outline" aria-hidden="true"></span>'
+                        f'<span class="mdi mdi-image-outline" aria-hidden="true"></span>{html.escape(label)}</div>')
-                    'Figure</div>')
         else:
             block = item
             if block.type in mag.VISUAL_TYPES:
@@ -267,10 +295,21 @@ def _ingest_sync(doc_id: str, job_id: str, ocr_lang: str, render_dpi: int,
         page_sizes = {p["page"]: (p.get("width"), p.get("height")) for p in layout.get("pages", [])}
         total_pages = len(page_sizes)
 
-        def labelled(step: str):
+        def labelled(step: str, *, refresh: bool = False):
             def on_page(page_no: int) -> None:
                 background_job_update(job_id, stage="advertisements",
                                       detail=f"{step} — page {page_no} of {total_pages}")
+                if refresh:
+                    # Snap this page's picture/chart/table boxes to the ink and crop them to
+                    # real files now, instead of waiting for the end-of-document pass below --
+                    # so the live preview shows accurate boxes and real thumbnails as each page
+                    # is labelled, not only once the whole magazine has finished. Idempotent
+                    # (see refresh_visual_regions), so the later whole-document call is still
+                    # safe to run as a fallback for any page this raised on.
+                    try:
+                        mag.refresh_visual_regions(d, layout, page_no)
+                    except Exception:
+                        logging.exception("Failed to refresh visual regions for page %s", page_no)
                 _write_ingest_preview(d, page_no, [b for b in layout.get("blocks", []) if b["page"] == page_no],
                                       [i for i in layout.get("images", []) if i["page"] == page_no],
                                       page_sizes.get(page_no), "labelling")
@@ -278,8 +317,12 @@ def _ingest_sync(doc_id: str, job_id: str, ocr_lang: str, render_dpi: int,
 
         background_job_update(job_id, stage="advertisements", detail="Recognising paragraphs, headings and advertisements")
         _recognise_advertisements(layout, _load_settings(), on_page=labelled("Finding advertisements"))
-        _recognise_semantic_layout(layout, _load_settings(), doc_dir=d, on_page=labelled("Recognising paragraphs and headings"))
+        _recognise_semantic_layout(layout, _load_settings(), doc_dir=d,
+                                   on_page=labelled("Recognising paragraphs and headings", refresh=True))
+        try:
             mag.refresh_visual_regions(d, layout)
+        except Exception:
+            logging.exception("Failed to refresh visual regions for %s", doc_id)
         layout, _ = mag.clean_up_regions(layout)
         mag.save_layout(d, layout)
 

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.42.2">
+<link rel="stylesheet" href="/static/style.css?v=1.42.3">
-<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.42.2">
+<link rel="stylesheet" href="/static/css/resource-diagnostics.css?v=1.42.3">
-<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.42.2">
+<link rel="stylesheet" href="/static/css/background-jobs.css?v=1.42.3">
-<link rel="stylesheet" href="/static/css/voice-design.css?v=1.42.2">
+<link rel="stylesheet" href="/static/css/voice-design.css?v=1.42.3">
-<link rel="stylesheet" href="/static/css/rehearser.css?v=1.42.2">
+<link rel="stylesheet" href="/static/css/rehearser.css?v=1.42.3">
-<link rel="stylesheet" href="/static/css/audiobook.css?v=1.42.2">
+<link rel="stylesheet" href="/static/css/audiobook.css?v=1.42.3">
 
 
   <!-- ── Flag icons — non-blocking (loaded async, icons appear after JS) ── -->

static/style.css

@@ -4883,6 +4883,9 @@ input[list="lang-codes"] {
   background: rgba(255,255,255,.04); color: #8a8f98; font: 500 11px/1.3 -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
   text-transform: uppercase; letter-spacing: .05em; }
 .mag-ingest-preview-text .mag-ingest-preview-figure .mdi { font-size: 22px; }
+.mag-ingest-preview-text .mag-ingest-preview-figure { margin-block: 12px; }   /* <figure> has UA defaults to undo */
+.mag-ingest-preview-text .mag-ingest-preview-figure img { display: block; max-width: 100%; max-height: 240px;
+  border-radius: 4px; object-fit: contain; }
 .mag-ingest-preview-text .mag-ingest-preview-excluded { color: #6b7078; font-style: italic; }
 .mag-ingest-preview-text .mag-ingest-preview-more { color: #6b7078; text-align: center; padding: 4px 0 0; }
 .mag-ingest-preview-resize { background: var(--border); cursor: col-resize; position: relative; touch-action: none; }

tests/test_magazine_routes.py

@@ -272,6 +272,23 @@ class IngestPreviewHtmlTests(unittest.TestCase):
         from routes.magazine import _render_ingest_preview_html
         html = _render_ingest_preview_html([], [{"bbox": [0, 5, 50, 50]}])
         self.assertIn("mag-ingest-preview-figure", html)
+        self.assertIn("Picture", html)
+        self.assertNotIn("<img", html)   # no filename yet -- not cropped, so no thumbnail to show
+
+    def test_a_cropped_region_shows_its_actual_thumbnail(self):
+        from routes.magazine import _render_ingest_preview_html
+        chart = {"bbox": [0, 5, 50, 50], "type": "chart", "filename": "p0001-region-b1.png"}
+        html = _render_ingest_preview_html([], [chart], doc_id="doc1")
+        self.assertIn('<img src="/api/magazine/doc1/images/p0001-region-b1.png"', html)
+        self.assertIn("Chart", html)
+        self.assertIn("<figcaption>Chart</figcaption>", html)
+
+    def test_a_bad_filename_falls_back_to_the_placeholder_rather_than_a_bad_src(self):
+        from routes.magazine import _render_ingest_preview_html
+        bad = {"bbox": [0, 5, 50, 50], "type": "table", "filename": "../../etc/passwd"}
+        html = _render_ingest_preview_html([], [bad], doc_id="doc1")
+        self.assertNotIn("<img", html)
+        self.assertIn("Table", html)
 
     def test_text_is_escaped(self):
         from routes.magazine import _render_ingest_preview_html
@@ -1220,6 +1237,22 @@ class LivePreviewBoxTests(unittest.TestCase):
         finally:
             shutil.rmtree(d, ignore_errors=True)
 
+    def test_the_preview_file_drops_the_page_scan_and_can_show_a_real_thumbnail(self):
+        import json
+        from pathlib import Path
+        from routes.magazine import _write_ingest_preview
+        d = Path(tempfile.mkdtemp(prefix="ttvc-preview-"))
+        try:
+            _write_ingest_preview(d, 3, [], [
+                {"bbox": [0, 0, 100, 100]},   # the page scan -- must not appear
+                {"bbox": [10, 10, 40, 40], "type": "figure", "filename": "p0003-region-b1.png"},
+            ], (100, 100), "labelling")
+            data = json.loads((d / "ingest_preview.json").read_text("utf-8"))
+            self.assertEqual(len(data["boxes"]), 1)
+            self.assertIn(f"/api/magazine/{d.name}/images/p0003-region-b1.png", data["text"])
+        finally:
+            shutil.rmtree(d, ignore_errors=True)
+
     def test_the_labelling_pass_reports_every_page_even_when_its_model_call_fails(self):
         from unittest import mock
         import routes.magazine as rm
@@ -1232,6 +1265,70 @@ class LivePreviewBoxTests(unittest.TestCase):
         self.assertEqual(seen, [1, 2])
 
 
+class IngestSyncVisualRefreshTests(unittest.TestCase):
+    """Pictures, charts and tables are snapped and cropped to real files per page, as each
+    page is labelled -- not only once at the very end -- so the live preview is accurate
+    and can show an actual thumbnail while the import is still running."""
+
+    def test_refresh_is_called_per_page_during_semantic_labelling(self):
+        from unittest.mock import patch, call
+        import routes.magazine as rm
+        with patch.object(rm, "_LIB", Path(tempfile.mkdtemp(prefix="ttvc-lib-"))) as lib:
+            d = lib / "doc1"
+            d.mkdir(parents=True)
+            (d / "source.pdf").write_bytes(b"x")
+            layout = {"pages": [{"page": 1, "width": 100, "height": 100},
+                                {"page": 2, "width": 100, "height": 100}], "blocks": [], "images": []}
+
+            def semantic(_layout, _settings, page=None, doc_dir=None, on_page=None):
+                on_page(1)
+                on_page(2)
+
+            with patch.object(rm.mag, "extract_pages", return_value=layout), \
+                 patch.object(rm, "_recognise_advertisements"), \
+                 patch.object(rm, "_recognise_semantic_layout", side_effect=semantic), \
+                 patch.object(rm.mag, "refresh_visual_regions") as refresh, \
+                 patch.object(rm.mag, "clean_up_regions", return_value=(layout, {})), \
+                 patch.object(rm.mag, "save_layout"), \
+                 patch.object(rm.mag, "build_articles", return_value=[]), \
+                 patch.object(rm.mag, "save_articles"), \
+                 patch.object(rm, "background_job_update"), \
+                 patch.object(rm, "_write_meta"):
+                rm._ingest_sync("doc1", "job1", "eng", 150, None, None)
+            # Once per page during labelling, plus the final whole-document safety-net call.
+            self.assertEqual(refresh.call_args_list,
+                             [call(d, layout, 1), call(d, layout, 2), call(d, layout)])
+            shutil.rmtree(lib, ignore_errors=True)
+
+    def test_a_refresh_failure_on_one_page_does_not_abort_ingestion(self):
+        from unittest.mock import patch, call
+        import routes.magazine as rm
+        with patch.object(rm, "_LIB", Path(tempfile.mkdtemp(prefix="ttvc-lib-"))) as lib:
+            d = lib / "doc1"
+            d.mkdir(parents=True)
+            (d / "source.pdf").write_bytes(b"x")
+            layout = {"pages": [{"page": 1, "width": 100, "height": 100}], "blocks": [], "images": []}
+
+            def semantic(_layout, _settings, page=None, doc_dir=None, on_page=None):
+                on_page(1)
+
+            with patch.object(rm.mag, "extract_pages", return_value=layout), \
+                 patch.object(rm, "_recognise_advertisements"), \
+                 patch.object(rm, "_recognise_semantic_layout", side_effect=semantic), \
+                 patch.object(rm.mag, "refresh_visual_regions", side_effect=RuntimeError("boom")), \
+                 patch.object(rm.mag, "clean_up_regions", return_value=(layout, {})), \
+                 patch.object(rm.mag, "save_layout") as save_layout, \
+                 patch.object(rm.mag, "build_articles", return_value=[]), \
+                 patch.object(rm.mag, "save_articles"), \
+                 patch.object(rm, "background_job_update") as update, \
+                 patch.object(rm, "_write_meta"):
+                rm._ingest_sync("doc1", "job1", "eng", 150, None, None)
+                save_layout.assert_called_once()
+            done = [c.kwargs for c in update.call_args_list if c.kwargs.get("done")]
+            self.assertEqual(done[-1]["stage"], "done")   # never surfaced as an ingest failure
+            shutil.rmtree(lib, ignore_errors=True)
+
+
 class InterruptedIngestTests(unittest.TestCase):
     def test_an_ingest_killed_by_a_restart_is_reported_as_failed_not_running_for_ever(self):
         import asyncio