diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5e5398d..2b0019c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
---
## [Unreleased]
+### Fixed
+- **Deleting a picture left an empty paragraph in its place (seen on c't p. 157 and 159).** A picture region is stored as two records, a visual block and an image entry linked by `source_block`, and the review list shows either one as a row. Delete removed only the row's own record, so the twin stayed at the same spot as an empty, unticked "Paragraph" that could not be told from a real one. Deleting either half now removes both (single delete, bulk delete and the type changes), the articles forget both ids, and Undo brings both back; a hand-drawn picture (no twin) and a text block that was retyped since are untouched. Copies of a magazine that already carry leftovers: *Clean up boxes* re-types an empty picture block that has no picture (not narrated, still listed, reversible; your production issue has 1 such, on page 158). Verified on your real page 159 through the real route on a scratch copy: delete removes both, Undo restores both.
+- **A turn in progress now says so.** While a turn is being read the picture and text on screen are still the old ones for 30 to 60 seconds, which looked like a failed turn (a screenshot showed the stale upside-down picture and its gibberish text beside "Re-reading page 156"). The review now shows a notice with the elapsed time saying the picture and text below are from before the turn, dims them and blocks clicks into them, and removes both when the read finishes.
+
---
diff --git a/Manual.md b/Manual.md
index cbdd30b..7224038 100644
--- a/Manual.md
+++ b/Manual.md
@@ -407,6 +407,9 @@ of the two pages the buttons act on, as with the other page tools.
are re-measured. On a test page, reading it sideways scored word-F1 0.02 and reading it after turning 0.88 (an ordinary page scores
about 0.88). The turn is remembered: re-running detection or re-importing does not flip the page back.
+**While a turn is being read** the picture and text you see are the old ones: the notice above the page says so, shows the time spent, and
+dims them until the read finishes (usually 30 to 60 seconds). The text is never turned itself; it is read again from the turned picture.
+
**Keeping track of turns.** Turns add up: a right turn plus a half turn is three quarters. The toolbar says so in words (for example
"Turned 270° clockwise from the scan") and offers **Undo turns**, which returns the page to the orientation the PDF itself gives it. Only one
turn or re-read of a page can run at a time: pressing a button again while one is still working is refused with a message, instead of stacking
diff --git a/architecture/magazine-ingestion.md b/architecture/magazine-ingestion.md
index 2dd46c5..6ed769d 100644
--- a/architecture/magazine-ingestion.md
+++ b/architecture/magazine-ingestion.md
@@ -477,6 +477,15 @@ Verified end-to-end against a synthetic 8-page issue (contents page, two-column
feature with a byline and a page-8 jump, full-page advert, second feature,
reviews) and against an image-only rendering of the same file to exercise OCR.
+## A picture is two records
+
+A picture region is stored as a visual block (`figure`/`chart`/`table`/`code`, empty or with OCR text) and an image entry whose `source_block` names
+it (`crop_visual_regions`); hand-drawn pictures have only the image entry. `region_ids_with` returns the ids that belong together, `delete_region` removes
+them all, and the delete and bulk routes drop every one of them from the articles. Only a *visual* block is taken with its image, so a text block that was
+retyped after the image was made is never deleted by mistake. Before this, deleting the image row left the block as an empty paragraph-looking box in the
+same place (real c't pages 157 and 159). `clean_up_regions` re-types an empty visual block that no image entry owns as `ocr_noise`, to tidy leftovers from
+before the fix.
+
## Endless-token regions and unbreakable text
A recogniser stuck in a loop can emit one token with no spaces at all (c't p. 176: `Giro + 2.1.1.1.1…`, 3,408 characters). `_looks_like_repetition_loop`
diff --git a/core/magazine.py b/core/magazine.py
index 7bf0622..fecaf60 100644
--- a/core/magazine.py
+++ b/core/magazine.py
@@ -4768,7 +4768,13 @@ def clean_up_regions(layout: dict[str, Any], pages: Iterable[int] | None = None)
b["type"], b["region_label"], b["confidence"] = BLOCK_FURNITURE, label, 0.9
report[label].append({"page": b["page"], "id": b["id"], "text": " ".join((b.get("text") or "").split())[:80]})
+ owned = {im.get("source_block") for im in layout.get("images", []) if im.get("source_block")}
for page_no, page_blocks in by_page.items():
+ # What deleting a picture used to leave behind: an empty figure block whose image entry is gone. It is nothing
+ # (no text, no picture to play), so it is re-typed as noise -- still listed, reversible, not narrated.
+ for b in page_blocks:
+ if b["type"] in _VISUAL_TYPES and not (b.get("text") or "").strip() and b["id"] not in owned:
+ mark(b, "ocr_noise")
text_boxes = [b for b in page_blocks if b["type"] not in _VISUAL_TYPES and b["type"] != BLOCK_FURNITURE]
for b in text_boxes:
text = " ".join((b.get("text") or "").split())
@@ -5095,15 +5101,32 @@ def reorder_page(layout: dict[str, Any], page_no: int) -> dict[str, Any]:
return layout
+def region_ids_with(layout: dict[str, Any], region_id: str) -> set[str]:
+ """The ids that go when `region_id` is deleted: the region itself and, for a picture, its other half.
+
+ A picture region is stored twice: a visual block (figure/chart/table/code, empty or carrying OCR text) and an
+ image entry whose `source_block` names it. The review list shows either one as a row, so deleting "the image"
+ used to leave the block behind as a box with an empty paragraph in the same place (and the other way round).
+ """
+ ids = {region_id}
+ for im in layout.get("images", []):
+ if im["id"] == region_id and im.get("source_block"):
+ twin = next((b for b in layout.get("blocks", []) if b["id"] == im["source_block"]), None)
+ if twin is not None and twin.get("type") in VISUAL_TYPES: # never take a text block that was retyped since
+ ids.add(twin["id"])
+ ids |= {im["id"] for im in layout.get("images", []) if im.get("source_block") == region_id}
+ return ids
+
+
def delete_region(layout: dict[str, Any], block_id: str) -> dict[str, Any]:
- """Remove a region the layout engine got wrong entirely (not just
- misplaced). An article referencing this id in its own blocks/images
- list is left as-is -- article_text() and the review screen already
- treat an id no longer present in the layout as simply absent, the same
- way a page re-OCR can already drop ids an article still lists."""
+ """Remove a region the layout engine got wrong entirely (not just misplaced) -- both halves of a picture region
+ (see region_ids_with). An article referencing a removed id in its own blocks/images list is left as-is here:
+ article_text() and the review screen treat an id no longer in the layout as absent, and the routes drop the
+ ids from the articles themselves."""
+ gone = region_ids_with(layout, block_id)
layout = dict(layout)
- layout["blocks"] = [b for b in layout.get("blocks", []) if b["id"] != block_id]
- layout["images"] = [im for im in layout.get("images", []) if im["id"] != block_id]
+ layout["blocks"] = [b for b in layout.get("blocks", []) if b["id"] not in gone]
+ layout["images"] = [im for im in layout.get("images", []) if im["id"] not in gone]
return layout
diff --git a/routes/magazine.py b/routes/magazine.py
index b72f810..8f9ea72 100644
--- a/routes/magazine.py
+++ b/routes/magazine.py
@@ -1760,8 +1760,9 @@ async def magazine_bulk_regions(doc_id: str, page: int, request: Request):
remap: dict[str, str | None] = {}
if action == "delete":
for rid in ids:
+ for gone_id in mag.region_ids_with(layout, rid):
+ remap[gone_id] = None # the twin half of a picture is forgotten by the articles too
layout = mag.delete_region(layout, rid)
- remap[rid] = None
else:
settings = _load_settings()
ocr = [str(settings.get(k) or "").strip() or None for k in
@@ -2148,6 +2149,7 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str):
not any(im["id"] == block_id for im in layout.get("images", [])):
raise HTTPException(404, "Region not found")
token = _snapshot_for_undo(d, "Deleted a region") if d is not None else None
+ gone = mag.region_ids_with(layout, block_id) # a picture goes with its twin record
layout = mag.delete_region(layout, block_id)
mag.save_layout(d, layout)
# Also drop it from articles: an article still listing a deleted id made Approve fail with
@@ -2156,8 +2158,8 @@ async def magazine_delete_region(doc_id: str, page: int, block_id: str):
changed = False
for article in articles:
for key in ("blocks", "images"):
- if block_id in (article.get(key) or []):
- article[key] = [x for x in article[key] if x != block_id]
+ if any(x in gone for x in (article.get(key) or [])):
+ article[key] = [x for x in article[key] if x not in gone]
changed = True
if changed:
mag.save_articles(d, articles)
diff --git a/static/js/magazine.js b/static/js/magazine.js
index 2804dd9..bbe4ed5 100644
--- a/static/js/magazine.js
+++ b/static/js/magazine.js
@@ -3628,6 +3628,29 @@ async function magReocrPage() {
renderReview();
}
+// While a turn is being read, the picture and the text on screen are still the OLD ones (the page is replaced as a whole
+// when the read finishes, 30 s to a few minutes). Say so plainly, with the time spent, and dim what is about to be replaced,
+// so nobody mistakes the stale, sideways picture and its gibberish text for the result.
+function magTurnBusyNote(on, page, detail, startedAt) {
+ const review = $('mag-review');
+ const split = review?.querySelector('.mag-review-split');
+ let note = $('mag-turn-busy');
+ if (!on) { note?.remove(); split?.classList.remove('is-stale'); return; }
+ if (!split) return;
+ if (!note) {
+ note = document.createElement('div');
+ note.id = 'mag-turn-busy';
+ note.className = 'mag-turn-busy';
+ note.setAttribute('role', 'status');
+ split.before(note);
+ }
+ const secs = Math.round((Date.now() - startedAt) / 1000);
+ note.innerHTML = ' Turning page ' + Number(page) +
+ ' and reading it again — ' + esc(detail || 'working') + ' (' + secs + ' s). The picture and the text below are from before ' +
+ 'the turn and are replaced when this finishes; it usually takes 30 to 60 seconds.';
+ split.classList.add('is-stale');
+}
+
// How far this page has been turned from the PDF's own orientation, in words, with a way back. Turns add up (a right
// turn and a half turn is three quarters), so pressing the buttons more than once is easy to lose track of.
function magTurnNow(page) {
@@ -3662,7 +3685,12 @@ async function magRotatePage(degrees) {
magOfferUndo(null); // an older Undo would put old-orientation boxes on the turned page
const buttons = ['mag-turn-left', 'mag-turn-right', 'mag-turn-flip', 'mag-turn-undo', 'mag-reocr-btn'].map($).filter(Boolean);
const statusEl = $('mag-reocr-status');
- const busy = (on, text) => { buttons.forEach((b) => { b.disabled = on; }); if (statusEl) statusEl.textContent = text || ''; };
+ const startedAt = Date.now();
+ const busy = (on, text) => {
+ buttons.forEach((b) => { b.disabled = on; });
+ if (statusEl) statusEl.textContent = text || '';
+ magTurnBusyNote(on, page, text, startedAt);
+ };
busy(true, 'Turning page ' + page + '…');
let jobId;
try {
diff --git a/static/style.css b/static/style.css
index b445678..1cf2b01 100644
--- a/static/style.css
+++ b/static/style.css
@@ -5655,3 +5655,7 @@ details.mag-report-group summary { cursor: pointer; margin-bottom: 6px; }
.mag-review-split > *, .mag-blocks, .mag-block { min-width: 0; }
.mag-block-text, .mag-diff, .mag-article, .mag-driving-lyrics, .mag-page-article-title, .mag-toc-title { overflow-wrap: anywhere; }
.mag-turn-state { align-self: center; padding: 0 6px; font-size: 12px; font-weight: 600; color: var(--subtext); }
+.mag-turn-busy { margin: 8px 0; padding: 10px 14px; border-radius: var(--radius); border: 2px solid var(--accent);
+ background: color-mix(in srgb, var(--accent) 12%, var(--surface)); font-size: 14px; line-height: 1.4; }
+.mag-review-split.is-stale { opacity: .4; pointer-events: none; filter: grayscale(.6); transition: opacity .15s; }
+@media (prefers-reduced-motion: reduce) { .mag-review-split.is-stale { transition: none; } }
diff --git a/tests/magazine.spec.js b/tests/magazine.spec.js
index a69712c..1162879 100644
--- a/tests/magazine.spec.js
+++ b/tests/magazine.spec.js
@@ -2036,6 +2036,25 @@ test.describe('Turning a page scanned sideways', () => {
await expect(page.getByRole('button', { name: /Turn right/ })).toBeEnabled();
});
+ test('while a turn is read the old picture and text are dimmed and labelled as old, then the notice goes', async ({ page }) => {
+ const calls = await open(page);
+ let polls = 0;
+ await page.route(`**/api/magazine/${DOC_ID}/pages/3/rotate`, route => { calls.rotate.push(route.request().postDataJSON()); return route.fulfill({ json: { job_id: 'rot1', degrees: 90 } }); });
+ await page.route('**/api/jobs/rot1', route => { polls++; return route.fulfill({ json: polls < 3 ? { done: false, detail: 'Re-reading page 3' } : { done: true, error: null } }); });
+ await page.getByRole('button', { name: /Turn right/ }).click();
+ await page.getByRole('alertdialog').getByRole('button', { name: 'Turn and read again' }).click();
+ const note = page.locator('#mag-turn-busy');
+ await expect(note).toBeVisible();
+ await expect(note).toContainText('Turning page 3 and reading it again');
+ await expect(note).toContainText('from before');
+ await expect(note).toHaveAttribute('role', 'status');
+ await expect(page.locator('.mag-review-split')).toHaveClass(/is-stale/);
+ // what is stale cannot be clicked into by accident
+ expect(await page.locator('.mag-review-split').evaluate(el => getComputedStyle(el).pointerEvents)).toBe('none');
+ await expect(page.locator('#mag-turn-busy')).toHaveCount(0, { timeout: 15_000 });
+ await expect(page.locator('.mag-review-split')).not.toHaveClass(/is-stale/);
+ });
+
test('cancelling the question turns nothing', async ({ page }) => {
const calls = await open(page);
await routeRotate(page, calls);
diff --git a/tests/test_magazine_routes.py b/tests/test_magazine_routes.py
index b6b02fe..36fced5 100644
--- a/tests/test_magazine_routes.py
+++ b/tests/test_magazine_routes.py
@@ -2176,3 +2176,145 @@ class FastLlmCallTests(unittest.TestCase):
with patch('routes.conversation._post_llm_chat_completion', return_value=MagicMock(status_code=200)) as post:
_llm_chat_fast({"llm_model": "m"}, [])
self.assertEqual(post.call_count, 1)
+
+
+class DeletePictureTests(unittest.TestCase):
+ """A picture is a visual block plus an image entry; deleting it must take both, or an empty paragraph is left."""
+
+ def layout(self):
+ return {"pages": [{"page": 1, "width": 595.0, "height": 842.0}], "blocks": [
+ {"id": "fig", "page": 1, "bbox": [50, 50, 200, 150], "type": "figure", "text": "", "reading_order": 0},
+ {"id": "tbl", "page": 1, "bbox": [50, 250, 200, 80], "type": "table", "text": "a | b", "reading_order": 1},
+ {"id": "txt", "page": 1, "bbox": [50, 350, 200, 40], "type": "body", "text": "Real text.", "reading_order": 2},
+ {"id": "retyped", "page": 1, "bbox": [50, 400, 200, 40], "type": "body", "text": "", "reading_order": 3},
+ ], "images": [
+ {"id": "p1-region-fig", "page": 1, "bbox": [50, 50, 200, 150], "source_block": "fig", "type": "figure", "filename": "f.png"},
+ {"id": "p1-region-tbl", "page": 1, "bbox": [50, 250, 200, 80], "source_block": "tbl", "type": "table", "filename": "t.png"},
+ {"id": "p1-manual-000", "page": 1, "bbox": [300, 50, 100, 100], "filename": "m.png"},
+ {"id": "p1-stale", "page": 1, "bbox": [50, 400, 200, 40], "source_block": "retyped", "type": "figure", "filename": "s.png"},
+ ]}
+
+ def ids(self, layout):
+ return sorted(b["id"] for b in layout["blocks"]), sorted(i["id"] for i in layout["images"])
+
+ def test_deleting_the_image_takes_its_block_too(self):
+ from core.magazine import delete_region
+ blocks, images = self.ids(delete_region(self.layout(), "p1-region-fig"))
+ self.assertNotIn("fig", blocks)
+ self.assertNotIn("p1-region-fig", images)
+
+ def test_deleting_the_block_takes_its_image_too(self):
+ from core.magazine import delete_region
+ blocks, images = self.ids(delete_region(self.layout(), "tbl"))
+ self.assertNotIn("tbl", blocks)
+ self.assertNotIn("p1-region-tbl", images)
+
+ def test_nothing_else_is_touched(self):
+ from core.magazine import delete_region
+ blocks, images = self.ids(delete_region(self.layout(), "p1-region-fig"))
+ self.assertEqual(blocks, ["retyped", "tbl", "txt"])
+ self.assertEqual(images, ["p1-manual-000", "p1-region-tbl", "p1-stale"])
+
+ def test_a_hand_drawn_picture_has_no_twin_and_a_text_block_has_none(self):
+ from core.magazine import delete_region, region_ids_with
+ self.assertEqual(region_ids_with(self.layout(), "p1-manual-000"), {"p1-manual-000"})
+ self.assertEqual(region_ids_with(self.layout(), "txt"), {"txt"})
+ self.assertEqual(self.ids(delete_region(self.layout(), "txt"))[0], ["fig", "retyped", "tbl"])
+
+ def test_an_image_whose_block_was_retyped_to_text_does_not_take_that_text_block(self):
+ from core.magazine import delete_region
+ blocks, images = self.ids(delete_region(self.layout(), "p1-stale"))
+ self.assertIn("retyped", blocks)
+ self.assertNotIn("p1-stale", images)
+
+ def test_the_input_layout_is_not_modified_and_an_unknown_id_changes_nothing(self):
+ from core.magazine import delete_region
+ layout = self.layout()
+ snapshot = json.dumps(layout, sort_keys=True)
+ delete_region(layout, "p1-region-fig")
+ self.assertEqual(json.dumps(layout, sort_keys=True), snapshot)
+ self.assertEqual(self.ids(delete_region(layout, "nope")), self.ids(layout))
+
+ def test_the_real_page_159_pictures_leave_nothing_behind(self):
+ """Measured on the production c't issue: deleting either picture of page 159 used to leave its empty figure block."""
+ import copy
+ from core.magazine import delete_region
+ path = Path(__file__).resolve().parents[1] / "config" / "reader_library" / "bd871c7c22f0449984d3b0e3d603d6d6" / "layout.json"
+ if not path.is_file():
+ self.skipTest("the production layout is not on this machine")
+ real = json.loads(path.read_text("utf-8"))
+ for image_id, block_id in (("p0159-region-b0159-v008", "b0159-v008"), ("p0159-region-b0159-v011", "b0159-v011")):
+ after = delete_region(copy.deepcopy(real), image_id)
+ self.assertNotIn(block_id, {b["id"] for b in after["blocks"]})
+ self.assertNotIn(image_id, {i["id"] for i in after["images"]})
+ self.assertEqual(len(after["blocks"]), len(real["blocks"]) - 1)
+ self.assertEqual(len(after["images"]), len(real["images"]) - 1)
+
+ def test_clean_up_retypes_what_earlier_deletes_left_behind_but_not_real_pictures(self):
+ from core.magazine import clean_up_regions
+ layout = self.layout()
+ layout["images"] = [i for i in layout["images"] if i["id"] != "p1-region-fig"] # the old bug's leftover: empty figure, no image
+ out, report = clean_up_regions(layout)
+ types = {b["id"]: (b["type"], b.get("region_label", "")) for b in out["blocks"]}
+ self.assertEqual(types["fig"], ("furniture", "ocr_noise"))
+ self.assertEqual(types["tbl"][0], "table") # has an image entry
+ self.assertEqual(types["txt"][0], "body")
+ layout = self.layout() # an empty figure that still has its picture is a real picture
+ self.assertEqual({b["id"]: b["type"] for b in clean_up_regions(layout)[0]["blocks"]}["fig"], "figure")
+
+
+class DeletePictureRouteTests(unittest.TestCase):
+ def setUp(self):
+ from fastapi import FastAPI
+ from fastapi.testclient import TestClient
+ import routes.magazine as routes
+ self.lib = Path(tempfile.mkdtemp(prefix="ttvc-lib-"))
+ from unittest import mock
+ p = mock.patch.object(routes, "_LIB", self.lib)
+ p.start(); self.addCleanup(p.stop)
+ app = FastAPI(); app.include_router(routes.router)
+ self.client = TestClient(app)
+ self.doc = self.client.post("/api/magazine/docs", json={"title": "T"}).json()["id"]
+ self.d = self.lib / self.doc
+ base = DeletePictureTests().layout()
+ (self.d / "layout.json").write_text(json.dumps(base))
+ (self.d / "articles.json").write_text(json.dumps({"articles": [
+ {"id": "a1", "title": "T", "pages": [1], "blocks": ["fig", "txt"], "images": ["p1-region-fig", "p1-manual-000"], "status": "detected"},
+ {"id": "a2", "title": "U", "pages": [1], "blocks": ["tbl"], "images": ["p1-region-tbl"], "status": "detected"}]}))
+
+ def state(self):
+ layout = json.loads((self.d / "layout.json").read_text())
+ arts = {a["id"]: a for a in json.loads((self.d / "articles.json").read_text())["articles"]}
+ return layout, arts
+
+ def test_deleting_an_image_row_leaves_no_twin_in_the_layout_or_the_articles(self):
+ r = self.client.delete(f"/api/magazine/{self.doc}/pages/1/regions/p1-region-fig")
+ self.assertEqual(r.status_code, 200, r.text)
+ layout, arts = self.state()
+ self.assertNotIn("fig", {b["id"] for b in layout["blocks"]})
+ self.assertEqual(arts["a1"]["blocks"], ["txt"]) # the block half is forgotten as well
+ self.assertEqual(arts["a1"]["images"], ["p1-manual-000"])
+ self.assertEqual(arts["a2"]["blocks"], ["tbl"]) # another article is untouched
+
+ def test_deleting_the_block_row_takes_the_image_too(self):
+ self.assertEqual(self.client.delete(f"/api/magazine/{self.doc}/pages/1/regions/tbl").status_code, 200)
+ layout, arts = self.state()
+ self.assertNotIn("p1-region-tbl", {i["id"] for i in layout["images"]})
+ self.assertEqual((arts["a2"]["blocks"], arts["a2"]["images"]), ([], []))
+
+ def test_bulk_delete_removes_both_halves_and_forgets_both_in_the_articles(self):
+ r = self.client.post(f"/api/magazine/{self.doc}/pages/1/regions/bulk", json={"ids": ["p1-region-fig", "tbl"], "action": "delete"})
+ self.assertEqual(r.status_code, 200, r.text)
+ layout, arts = self.state()
+ self.assertEqual(sorted(b["id"] for b in layout["blocks"]), ["retyped", "txt"])
+ self.assertEqual(sorted(i["id"] for i in layout["images"]), ["p1-manual-000", "p1-stale"])
+ self.assertEqual((arts["a1"]["blocks"], arts["a1"]["images"]), (["txt"], ["p1-manual-000"]))
+ self.assertEqual((arts["a2"]["blocks"], arts["a2"]["images"]), ([], []))
+
+ def test_undo_after_deleting_a_picture_brings_both_halves_back(self):
+ token = self.client.delete(f"/api/magazine/{self.doc}/pages/1/regions/p1-region-fig").json()["undo"]
+ self.assertEqual(self.client.post(f"/api/magazine/{self.doc}/undo", json={"token": token}).status_code, 200)
+ layout, arts = self.state()
+ self.assertIn("fig", {b["id"] for b in layout["blocks"]})
+ self.assertIn("p1-region-fig", {i["id"] for i in layout["images"]})
+ self.assertEqual(arts["a1"]["blocks"], ["fig", "txt"])