diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dfb3a8..89feb56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi --- +## [1.30.0] — 2026-09-15 + +### Added +- **Edit boxes in Magazine review.** In article review, a new "Edit boxes" mode lets you drag an already-detected region to move it, drag its corner handle to resize it, or press its × to delete it — for correcting a box the layout engine got wrong, not for one it missed entirely ("Draw it" still covers that). Moving or resizing opens a small confirm panel: "Save" keeps the region at its new position, and for text regions "Save & re-OCR" also reads the corrected area again and replaces its text. Deleting a region removes it from the page entirely. +- New `core/magazine.py` `update_region()`/`delete_region()` and `PATCH`/`DELETE /api/magazine/{doc}/pages/{page}/regions/{block_id}` routes. An image region is re-cropped from the page render whenever its bbox changes, since its saved file is exactly that crop. + +### Verified +- Live against the real ingested c't KI Wissen 2026 magazine: added a throwaway region, moved it (bbox persisted, text unchanged), moved it again with re-OCR (correctly preserved the old text when the corrected area had no readable text — the same known engine limitation from 1.29.0, not a new bug), deleted it (confirmed gone; repeat delete correctly 404s). + +--- + ## [1.29.1] — 2026-09-14 ### Fixed diff --git a/Manual.md b/Manual.md index 6649c86..b1c2892 100644 --- a/Manual.md +++ b/Manual.md @@ -62,7 +62,16 @@ turned into a podcast discussion between two or three voices you pick. place confidently are left exactly as they were. If the layout engine missed a region entirely, **"Draw it"** lets you click-drag a box directly on the page image and add it as text (typed or OCRed) or as a - new figure. Confirm the boundaries, correct the title, then **Approve**. + new figure. If a region was found but its box is misplaced, the wrong + size, or shouldn't exist at all, **"Edit boxes"** lets you drag an + existing box to move it, drag its corner handle to resize it, or press + its **×** to delete it outright. Moving or resizing opens a small confirm + panel — **Save** keeps the region's existing text at its new position; + for a text region, **Save & re-OCR** also reads the corrected area again + and replaces the text with that fresh reading (useful when the box was + too small or too large to read cleanly the first time). Deleting a + region removes it from the page entirely, not just from the article + currently under review. Confirm the boundaries, correct the title, then **Approve**. Optionally, click **"Describe"** under a figure, chart, table or code block to have the configured vision engine generate a one- or two-sentence description for listeners who cannot see the image — e.g. what a chart @@ -127,6 +136,7 @@ unreviewed boundaries may include a neighbouring article or an advert. | "Fix figure boundaries" reports nothing corrected | Every box already matched its page, or none could be placed with confidence. | Nothing is wrong; correct any remaining box by drawing it. | | "No vision OCR engine configured" (Describe) | `vision_ocr_url`/`vision_ocr_model` are empty. | Set a vision engine in Settings. | | "The vision engine returned nothing usable for this figure" | The configured engine (and fallback, if set) returned an empty or degenerate response for this image. Some vision models are transcription-only and cannot produce open-ended descriptions at all, however the prompt is phrased — confirmed with a pure-OCR model that answers its own "Text Recognition:" prompt but returns nothing for any descriptive one. | Configure a genuinely caption-capable vision model as the fallback (`vision_ocr_fallback_url`/`vision_ocr_fallback_model`), or write the description by hand. | +| "Save & re-OCR" on an edited box leaves the old text in place | The corrected area produced an empty or unreadable OCR result — the same engine limitation that can affect figure descriptions applies here too. | Press **Save** to keep the corrected box position with the existing text, then correct the text by hand if needed, or try "OCR again (this page)" once the box position is saved. | ### Privacy diff --git a/TODO.md b/TODO.md index 2d59384..940e086 100644 --- a/TODO.md +++ b/TODO.md @@ -365,6 +365,9 @@ casting. The original PDF remains the visual source of truth while approved arti - [x] Add OCR-engine selection from Engines > OCR & Document Parsing, with page-image input and structured output. - [x] Detect contents/index pages and propose article titles, authors, and page ranges. - [x] Build a review screen where users approve or correct article boundaries before TTS. + *(includes "Edit boxes" — user-requested 2026-09-14: drag an already-detected region to move or resize it, + or delete it outright, with an optional re-OCR of the corrected area for text regions. "Draw it" remains the + separate tool for a region detection missed entirely.)* - [x] Preserve source coordinates for every extracted block, image, caption, and page reference. - [x] Generate per-article podcast scripts from a connected LLM with two or three user-picked host voices, diff --git a/VERSION b/VERSION index eaeb236..ff6d09b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.29.1 \ No newline at end of file +1.30.0 \ No newline at end of file diff --git a/core/magazine.py b/core/magazine.py index 36193a8..6868477 100644 --- a/core/magazine.py +++ b/core/magazine.py @@ -2542,6 +2542,77 @@ def extract_region_text(doc_dir: Path, layout: dict[str, Any], page_no: int, crop_path.unlink(missing_ok=True) +def update_region(doc_dir: Path, layout: dict[str, Any], page_no: int, block_id: str, + bbox: Sequence[float] | None, reocr: bool = False, + vision_ocr_url: str | None = None, + vision_ocr_model: str | None = None, + vision_ocr_fallback_url: str | None = None, + vision_ocr_fallback_model: str | None = None, + timeout: float = 150.0) -> tuple[dict[str, Any], str | None]: + """Move/resize a region the layout engine already found, after the user + dragged its box to correct a misplaced or mis-sized detection. + + Not for a region detection missed entirely (add_manual_region covers + that) -- block_id must already exist in this page's blocks or images. + A text block can also be re-OCRed at its corrected geometry (`reocr`); + an image region is always re-cropped from the page render when its bbox + changes, since its saved file is exactly that crop. Returns the updated + layout and, when reocr produced text, that text (None otherwise, so the + caller can tell "no re-OCR requested" apart from "re-OCR found nothing"). + """ + page_meta = next((p for p in layout.get("pages", []) if p["page"] == page_no), None) + if not page_meta: + raise ValueError("Unknown page") + if bbox is not None and (len(bbox) != 4 or bbox[2] <= 0 or bbox[3] <= 0): + raise ValueError("bbox must be [x, y, width, height] with a positive size") + layout = dict(layout) + + blocks = list(layout.get("blocks", [])) + block_idx = next((i for i, b in enumerate(blocks) if b["id"] == block_id), None) + if block_idx is not None: + block = dict(blocks[block_idx]) + if bbox is not None: + block["bbox"] = list(bbox) + new_text = None + if reocr: + new_text = extract_region_text( + doc_dir, layout, page_no, block["bbox"], + vision_ocr_url, vision_ocr_model, + vision_ocr_fallback_url, vision_ocr_fallback_model, timeout) + if new_text: + block["text"] = new_text + blocks[block_idx] = block + layout["blocks"] = blocks + return layout, new_text + + images = list(layout.get("images", [])) + image_idx = next((i for i, im in enumerate(images) if im["id"] == block_id), None) + if image_idx is None: + raise ValueError("Region not found on this page") + image = dict(images[image_idx]) + if bbox is not None: + image["bbox"] = list(bbox) + filename = image.get("filename") or f"{block_id}.png" + (doc_dir / "images").mkdir(parents=True, exist_ok=True) + _crop_page_png(doc_dir, page_no, page_meta, bbox, doc_dir / "images" / filename) + image["filename"] = filename + images[image_idx] = image + layout["images"] = images + return layout, None + + +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.""" + 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] + return layout + + def save_layout(doc_dir: Path, layout: dict[str, Any]) -> None: (doc_dir / "layout.json").write_text( json.dumps(layout, ensure_ascii=False), "utf-8") diff --git a/routes/magazine.py b/routes/magazine.py index acf6f46..414cfaf 100644 --- a/routes/magazine.py +++ b/routes/magazine.py @@ -811,6 +811,59 @@ async def magazine_add_region(doc_id: str, page: int, request: Request): return {"job_id": job_id} +@router.patch("/api/magazine/{doc_id}/pages/{page}/regions/{block_id}") +async def magazine_update_region(doc_id: str, page: int, block_id: str, request: Request): + """Move/resize a region the layout engine already detected -- for + correcting a box that's misplaced or mis-sized, not for adding one + that's missing entirely (POST .../regions covers that). + """ + d, _ = _require_magazine(doc_id) + layout = mag.load_layout(d) + if not layout: + raise HTTPException(404, "No layout yet -- run ingestion first") + + body = await request.json() + bbox = body.get("bbox") + if bbox is not None and not (isinstance(bbox, list) and len(bbox) == 4 + and all(isinstance(v, (int, float)) for v in bbox)): + raise HTTPException(400, "bbox must be [x, y, width, height]") + reocr = bool(body.get("reocr")) + + settings = _load_settings() + vision_ocr_url = str(settings.get("vision_ocr_url") or "").strip() or None + vision_ocr_model = str(settings.get("vision_ocr_model") or "").strip() or None + vision_ocr_fallback_url = str(settings.get("vision_ocr_fallback_url") or "").strip() or None + vision_ocr_fallback_model = str(settings.get("vision_ocr_fallback_model") or "").strip() or None + + try: + layout, text = await asyncio.to_thread( + mag.update_region, d, layout, page, block_id, + [float(v) for v in bbox] if bbox is not None else None, reocr, + vision_ocr_url, vision_ocr_model, vision_ocr_fallback_url, vision_ocr_fallback_model, + ) + except ValueError as exc: + raise HTTPException(400, str(exc)) + mag.save_layout(d, layout) + return {"ok": True, "text": text} + + +@router.delete("/api/magazine/{doc_id}/pages/{page}/regions/{block_id}") +async def magazine_delete_region(doc_id: str, page: int, block_id: str): + """Remove a region the layout engine detected wrongly -- an artefact, + a duplicate, or a box that doesn't correspond to real content at all. + """ + d, _ = _require_magazine(doc_id) + layout = mag.load_layout(d) + if not layout: + raise HTTPException(404, "No layout yet -- run ingestion first") + if not any(b["id"] == block_id for b in layout.get("blocks", [])) and \ + not any(im["id"] == block_id for im in layout.get("images", [])): + raise HTTPException(404, "Region not found") + layout = mag.delete_region(layout, block_id) + mag.save_layout(d, layout) + return {"ok": True} + + @router.post("/api/magazine/{doc_id}/pages/{page}/extract") async def magazine_extract_region(doc_id: str, page: int, request: Request): """OCR a selected page rectangle for a metadata field without saving it.""" diff --git a/static/js/magazine.js b/static/js/magazine.js index 8185918..a8a0393 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -741,6 +741,9 @@ function renderReview() { ' Source / footer' + '' + + '' + '' + '' + '' + + '' + ''; }); html += ''; @@ -851,6 +860,7 @@ function renderReview() { ''; host.innerHTML = html; setupMagDrawing(); + setupMagBoxEdit(); } // Tracks every ticked block across ALL reviewed pages of this article, not @@ -966,7 +976,7 @@ function magReviewClearCurrentPage() { function magReviewFocusBlock(blockId) { const cb = document.querySelector('#mag-review input[data-block="' + CSS.escape(blockId) + '"]'); - if (state.drawMode) return; + if (state.drawMode || state.editMode) return; if (cb) { cb.checked = !cb.checked; magReviewToggleBox(cb); } const row = document.querySelector('#mag-review [data-block-row="' + CSS.escape(blockId) + '"]'); if (row) row.scrollIntoView({ block: 'nearest', behavior: prefersReducedMotion() ? 'auto' : 'smooth' }); @@ -988,6 +998,7 @@ function magReviewHighlightBox(blockId) { function magReviewToggleDraw(kind = 'text') { state.drawMode = state.drawMode && state.drawKind === kind ? false : true; state.drawKind = kind; + if (state.drawMode && state.editMode) magReviewToggleEdit(); const hint = $('mag-draw-hint'); const field = kind === 'title' ? 'title' : kind === 'author' ? 'author' : ''; const drawLabels = { text: 'text region', image: 'image, chart, or graphic', heading: 'headline', page_number: 'page number', source: 'source or footer', ad: 'advertisement' }; @@ -1003,6 +1014,180 @@ function magReviewToggleDraw(kind = 'text') { magReviewCancelDrawPanel(); } +// ── Edit an already-detected region ────────────────────────────────────── +// Correcting a box the layout engine already found -- drag its body to +// move it, its corner handle to resize it, or its × to delete it entirely. +// Mutually exclusive with draw mode (adding a region that's missing) for +// the same reason those drawing tools already dim every existing box while +// active: dragging and click-to-toggle-inclusion would otherwise fight over +// the same mousedown. +function magReviewToggleEdit() { + state.editMode = !state.editMode; + if (state.editMode && state.drawMode) magReviewToggleDraw(state.drawKind); + const wrap = $('mag-review-page-wrap'); + if (wrap) wrap.classList.toggle('is-editing', state.editMode); + const btn = $('mag-edit-toggle'); + if (btn) { btn.classList.toggle('active', state.editMode); btn.setAttribute('aria-pressed', String(state.editMode)); } + const hint = $('mag-draw-hint'); + if (hint) hint.textContent = state.editMode + ? ' Drag a region to move it, its corner to resize it, or press × to delete it.' : ''; + magReviewCancelEditPanel(); +} + +let _magEditState = null; // { blockId, mode: 'move'|'resize', box, startXPct, startYPct, origLeft, origTop, origW, origH } + +function setupMagBoxEdit() { + const wrap = $('mag-review-page-wrap'); + if (!wrap) return; + if (state.editMode) wrap.classList.add('is-editing'); + // wrap is rebuilt fresh by every renderReview(), so per-wrap listeners + // never accumulate -- only the one-time window-level listener below needs + // the install guard, matching setupMagDrawing's own pattern. + wrap.addEventListener('mousedown', (e) => { + if (!state.editMode || e.button !== 0) return; + if (e.target.closest('.mag-edit-panel')) return; + const resizeHandle = e.target.closest('.mag-review-box-resize'); + const box = e.target.closest('.mag-review-box'); + if (!box || box.id === 'mag-draw-rect') return; + magReviewCancelEditPanel(); + const rect = wrap.getBoundingClientRect(); + _magEditState = { + blockId: box.dataset.block, + mode: resizeHandle ? 'resize' : 'move', + box, + startXPct: (e.clientX - rect.left) / rect.width * 100, + startYPct: (e.clientY - rect.top) / rect.height * 100, + origLeft: parseFloat(box.style.left) || 0, + origTop: parseFloat(box.style.top) || 0, + origW: parseFloat(box.style.width) || 0, + origH: parseFloat(box.style.height) || 0, + }; + e.preventDefault(); + }); + wrap.addEventListener('mousemove', (e) => { + if (!_magEditState) return; + const rect = wrap.getBoundingClientRect(); + const xPct = Math.max(0, Math.min(100, (e.clientX - rect.left) / rect.width * 100)); + const yPct = Math.max(0, Math.min(100, (e.clientY - rect.top) / rect.height * 100)); + const dx = xPct - _magEditState.startXPct; + const dy = yPct - _magEditState.startYPct; + const { box, origLeft, origTop, origW, origH } = _magEditState; + if (_magEditState.mode === 'move') { + const left = Math.max(0, Math.min(100 - origW, origLeft + dx)); + const top = Math.max(0, Math.min(100 - origH, origTop + dy)); + box.style.left = left + '%'; box.style.top = top + '%'; + } else { + const w = Math.max(1, Math.min(100 - origLeft, origW + dx)); + const h = Math.max(1, Math.min(100 - origTop, origH + dy)); + box.style.width = w + '%'; box.style.height = h + '%'; + } + }); + if (!_magEditMouseupInstalled) { + _magEditMouseupInstalled = true; + window.addEventListener('mouseup', () => { + if (!_magEditState) return; + const { box, origLeft, origTop, origW, origH } = _magEditState; + const moved = Math.abs((parseFloat(box.style.left) || 0) - origLeft) > .1 || + Math.abs((parseFloat(box.style.top) || 0) - origTop) > .1 || + Math.abs((parseFloat(box.style.width) || 0) - origW) > .1 || + Math.abs((parseFloat(box.style.height) || 0) - origH) > .1; + const state_ = _magEditState; + _magEditState = null; + if (!moved) return; // a click, not a drag -- nothing to confirm + magReviewOpenEditPanel(state_.blockId, box, { origLeft, origTop, origW, origH }); + }); + } +} +let _magEditMouseupInstalled = false; + +function magReviewCancelEditPanel(revert = false) { + const panel = document.querySelector('.mag-edit-panel'); + if (panel) { + if (revert && panel.dataset.origLeft !== undefined) { + const box = document.querySelector('.mag-review-box[data-block="' + CSS.escape(panel.dataset.blockId) + '"]'); + if (box) { + box.style.left = panel.dataset.origLeft + '%'; box.style.top = panel.dataset.origTop + '%'; + box.style.width = panel.dataset.origW + '%'; box.style.height = panel.dataset.origH + '%'; + } + } + panel.remove(); + } +} + +function magReviewOpenEditPanel(blockId, box, orig) { + const wrap = $('mag-review-page-wrap'); + if (!wrap) return; + const kind = box.dataset.kind; + const panel = document.createElement('div'); + panel.className = 'mag-draw-panel mag-edit-panel'; + panel.dataset.blockId = blockId; + panel.dataset.origLeft = orig.origLeft; panel.dataset.origTop = orig.origTop; + panel.dataset.origW = orig.origW; panel.dataset.origH = orig.origH; + panel.innerHTML = + '
Save this region’s new position?
' + + (kind === 'text' + ? '

Save keeps the existing text. Save & re-OCR reads the corrected area again and replaces it.

' + : '

The saved figure image will be re-cropped to the new box.

') + + '
' + + '' + + (kind === 'text' ? '' + + '' + : '') + + '
'; + wrap.appendChild(panel); + const top = parseFloat(box.style.top) || 0, left = parseFloat(box.style.left) || 0, height = parseFloat(box.style.height) || 0; + panel.style.top = Math.min(80, top + height + 1) + '%'; + panel.style.left = Math.min(55, left) + '%'; +} + +async function magReviewSubmitEdit(reocr) { + const panel = document.querySelector('.mag-edit-panel'); + if (!panel) return; + const blockId = panel.dataset.blockId; + const box = document.querySelector('.mag-review-box[data-block="' + CSS.escape(blockId) + '"]'); + const pageMeta = (state.layout.pages || []).find((p) => p.page === state.reviewPage); + if (!box || !pageMeta) { magReviewCancelEditPanel(); return; } + const bbox = [ + parseFloat(box.style.left) / 100 * pageMeta.width, + parseFloat(box.style.top) / 100 * pageMeta.height, + parseFloat(box.style.width) / 100 * pageMeta.width, + parseFloat(box.style.height) / 100 * pageMeta.height, + ]; + panel.remove(); + say(reocr ? 'Saving and re-reading region…' : 'Saving region…', ''); + try { + await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(blockId), { + method: 'PATCH', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ bbox, reocr }), + }); + } catch (e) { + say('Could not save region: ' + e.message, 'err'); + magReviewCancelEditPanel(true); + return; + } + say('Region saved', 'ok'); + state.layout = null; + await ensureLayout(); + renderReview(); +} + +async function magReviewDeleteRegion(blockId) { + const ok = await confirmDialog('Delete this region? This removes it from the page entirely, not just from this article.', + { title: 'Delete region?', okLabel: 'Delete', danger: true }); + if (!ok) return; + try { + await api('/api/magazine/' + state.doc.id + '/pages/' + state.reviewPage + '/regions/' + encodeURIComponent(blockId), { + method: 'DELETE', + }); + } catch (e) { + return say('Could not delete region: ' + e.message, 'err'); + } + say('Region deleted', 'ok'); + state.layout = null; + await ensureLayout(); + renderReview(); +} + let _magDrawState = null; // { startXPct, startYPct } while dragging let _magDrawMouseupInstalled = false; @@ -1044,6 +1229,7 @@ function setupMagDrawing() { _magDrawMouseupInstalled = true; window.addEventListener('keydown', (e) => { if (e.key === 'Escape' && state.drawMode) magReviewToggleDraw(state.drawKind); + if (e.key === 'Escape' && state.editMode) magReviewToggleEdit(); }); // $('mag-draw-rect') is looked up fresh on every mouseup, so this one // listener keeps working correctly across every renderReview() rebuild diff --git a/static/style.css b/static/style.css index d218c85..98331d6 100644 --- a/static/style.css +++ b/static/style.css @@ -4982,6 +4982,21 @@ input[list="lang-codes"] { padding: 4px 6px; border: 1px solid var(--border); border-radius: 4px; background: var(--surface); color: var(--text); } +/* Editing an already-detected region: move by dragging the box body, resize + from its corner handle, delete with its ×. Handles and the delete button + only render usably in edit mode -- kept in the DOM at all times (rather + than added/removed) so toggling edit mode doesn't cost a re-render. */ +#mag-review-page-wrap.is-editing { cursor: default; } +#mag-review-page-wrap.is-editing .mag-review-box { cursor: move; } +.mag-review-box-delete { display: none; position: absolute; top: -9px; right: -9px; width: 18px; height: 18px; + border-radius: 50%; border: 1px solid var(--border); background: var(--red); color: #fff; + font-size: 11px; line-height: 1; align-items: center; justify-content: center; padding: 0; cursor: pointer; z-index: 3; } +#mag-review-page-wrap.is-editing .mag-review-box-delete { display: flex; } +.mag-review-box-resize { display: none; position: absolute; bottom: -5px; right: -5px; width: 12px; height: 12px; + border-radius: 3px; border: 1px solid var(--accent); background: var(--panel); cursor: nwse-resize; z-index: 3; } +#mag-review-page-wrap.is-editing .mag-review-box-resize { display: block; } +#mag-edit-toggle.active { background: var(--accent); color: var(--on-accent); border-color: var(--accent); } + .mag-block { display: grid; grid-template-columns: auto auto 90px 1fr; gap: 10px; align-items: start; padding: 8px 10px; border: 1px solid var(--border); border-radius: var(--radius); background: var(--surface); margin-bottom: 4px; cursor: pointer; } diff --git a/tests/test_magazine.py b/tests/test_magazine.py index 7f4967c..a40d280 100644 --- a/tests/test_magazine.py +++ b/tests/test_magazine.py @@ -46,6 +46,8 @@ from core.magazine import ( classify_blocks, reocr_page, add_manual_region, + update_region, + delete_region, detect_columns, estimate_font_size_from_area, estimate_listen_seconds, @@ -1642,6 +1644,108 @@ class ManualRegionTests(unittest.TestCase): self.assertEqual(len(names), 2) +class UpdateRegionTests(unittest.TestCase): + """Correcting a box the layout engine already found -- moving/resizing + it, or deleting it entirely. A different job from add_manual_region + (which is for a region detection missed outright): here block_id must + already exist.""" + + def setUp(self): + from PIL import Image + self.tmp = tempfile.mkdtemp(prefix="ttvc-updateregion-test-") + self.doc_dir = pathlib.Path(self.tmp) + (self.doc_dir / "pages").mkdir() + Image.new("RGB", (595, 842), "white").save(self.doc_dir / "pages" / "0003.png") + self.layout = { + "pages": [{"page": 3, "width": 595.0, "height": 842.0}], + "blocks": [ + {"id": "b0003-000", "page": 3, "bbox": [50, 60, 300, 40], + "type": BLOCK_TITLE, "text": "Existing title", "reading_order": 0}, + ], + "images": [ + {"id": "img0003-000", "page": 3, "bbox": [20, 30, 120, 80], + "filename": "img0003-000.png", "type": "chart"}, + ], + } + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_unknown_page_raises(self): + with self.assertRaises(ValueError): + update_region(self.doc_dir, self.layout, 99, "b0003-000", [10, 10, 50, 50]) + + def test_unknown_block_id_raises(self): + with self.assertRaises(ValueError): + update_region(self.doc_dir, self.layout, 3, "does-not-exist", [10, 10, 50, 50]) + + def test_degenerate_bbox_raises(self): + with self.assertRaises(ValueError): + update_region(self.doc_dir, self.layout, 3, "b0003-000", [10, 10, 0, 0]) + + def test_moves_a_text_block_without_reocr(self): + out, text = update_region(self.doc_dir, self.layout, 3, "b0003-000", [70, 90, 300, 40]) + moved = next(b for b in out["blocks"] if b["id"] == "b0003-000") + self.assertEqual(moved["bbox"], [70, 90, 300, 40]) + self.assertEqual(moved["text"], "Existing title") # unchanged + self.assertIsNone(text) + + def test_reocr_replaces_the_blocks_text(self): + with mock.patch("core.magazine.extract_region_text", return_value="Freshly read title"): + out, text = update_region(self.doc_dir, self.layout, 3, "b0003-000", + [70, 90, 300, 40], reocr=True) + moved = next(b for b in out["blocks"] if b["id"] == "b0003-000") + self.assertEqual(moved["text"], "Freshly read title") + self.assertEqual(text, "Freshly read title") + + def test_reocr_finding_nothing_keeps_the_old_text(self): + with mock.patch("core.magazine.extract_region_text", return_value=""): + out, text = update_region(self.doc_dir, self.layout, 3, "b0003-000", + [70, 90, 300, 40], reocr=True) + moved = next(b for b in out["blocks"] if b["id"] == "b0003-000") + self.assertEqual(moved["text"], "Existing title") + self.assertEqual(text, "") + + def test_moving_an_image_recrops_its_file(self): + from PIL import Image + out, text = update_region(self.doc_dir, self.layout, 3, "img0003-000", [0, 0, 60, 40]) + moved = next(i for i in out["images"] if i["id"] == "img0003-000") + self.assertEqual(moved["bbox"], [0, 0, 60, 40]) + self.assertIsNone(text) + with Image.open(self.doc_dir / "images" / moved["filename"]) as im: + self.assertEqual(im.size, (60, 40)) + + def test_bbox_none_leaves_geometry_untouched_for_reocr_only(self): + with mock.patch("core.magazine.extract_region_text", return_value="Reread text"): + out, text = update_region(self.doc_dir, self.layout, 3, "b0003-000", None, reocr=True) + moved = next(b for b in out["blocks"] if b["id"] == "b0003-000") + self.assertEqual(moved["bbox"], [50, 60, 300, 40]) + self.assertEqual(text, "Reread text") + + +class DeleteRegionTests(unittest.TestCase): + def test_removes_a_block(self): + layout = {"blocks": [{"id": "b1"}, {"id": "b2"}], "images": []} + out = delete_region(layout, "b1") + self.assertEqual([b["id"] for b in out["blocks"]], ["b2"]) + + def test_removes_an_image(self): + layout = {"blocks": [], "images": [{"id": "img1"}, {"id": "img2"}]} + out = delete_region(layout, "img1") + self.assertEqual([i["id"] for i in out["images"]], ["img2"]) + + def test_unknown_id_is_a_harmless_no_op(self): + layout = {"blocks": [{"id": "b1"}], "images": [{"id": "img1"}]} + out = delete_region(layout, "does-not-exist") + self.assertEqual(len(out["blocks"]), 1) + self.assertEqual(len(out["images"]), 1) + + def test_does_not_mutate_the_original_layout(self): + layout = {"blocks": [{"id": "b1"}], "images": []} + delete_region(layout, "b1") + self.assertEqual(len(layout["blocks"]), 1) + + class SnapVisualBboxesTests(unittest.TestCase): """A vision OCR engine grounds a figure loosely; the render is the truth. diff --git a/tests/test_magazine_routes.py b/tests/test_magazine_routes.py index be951f0..a0b05cb 100644 --- a/tests/test_magazine_routes.py +++ b/tests/test_magazine_routes.py @@ -15,6 +15,13 @@ from fastapi import HTTPException from routes.magazine import _article_id, _doc_dir, _parse_script +async def _run_sync(fn, *args, **kwargs): + """Stand-in for asyncio.to_thread that runs the callable inline and + stays awaitable, for routes that await its result directly rather than + firing it as a background job.""" + return fn(*args, **kwargs) + + class PathSafetyTests(unittest.TestCase): def test_traversal_and_junk_ids_are_rejected(self): for bad in ("../../etc", "a/b", "", "..", "x" * 41, "id-with-dash", "a.b"): @@ -455,3 +462,118 @@ class UpdateImageDescriptionRouteTests(unittest.IsolatedAsyncioTestCase): with self.assertRaises(HTTPException) as err: await magazine_update_image_description('a' * 32, 'img1', request) self.assertEqual(err.exception.status_code, 404) + + +class UpdateRegionRouteTests(unittest.IsolatedAsyncioTestCase): + async def test_moves_a_block_and_returns_no_text_without_reocr(self): + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_update_region + layout = {"pages": [{"page": 1, "width": 600, "height": 800}], + "blocks": [{"id": "b1", "page": 1, "bbox": [10, 10, 100, 20], "type": "body", "text": "hi"}]} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout), \ + patch('routes.magazine.mag.save_layout') as save, \ + patch('routes.magazine._load_settings', return_value={}), \ + patch('routes.magazine.asyncio.to_thread', new=_run_sync): + request = AsyncMock() + request.json.return_value = {'bbox': [20, 20, 100, 20]} + result = await magazine_update_region('a' * 32, 1, 'b1', request) + self.assertTrue(result['ok']) + self.assertIsNone(result['text']) + save.assert_called_once() + + async def test_reocr_replaces_text_when_extraction_succeeds(self): + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_update_region + layout = {"pages": [{"page": 1, "width": 600, "height": 800}], + "blocks": [{"id": "b1", "page": 1, "bbox": [10, 10, 100, 20], "type": "body", "text": "stale"}]} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout), \ + patch('routes.magazine.mag.save_layout'), \ + patch('routes.magazine.mag.extract_region_text', return_value='fresh text'), \ + patch('routes.magazine._load_settings', + return_value={'vision_ocr_url': 'http://e/v1', 'vision_ocr_model': 'm'}), \ + patch('routes.magazine.asyncio.to_thread', new=_run_sync): + request = AsyncMock() + request.json.return_value = {'bbox': [10, 10, 100, 20], 'reocr': True} + result = await magazine_update_region('a' * 32, 1, 'b1', request) + self.assertEqual(result['text'], 'fresh text') + + async def test_bad_bbox_is_rejected(self): + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_update_region + layout = {"pages": [{"page": 1, "width": 600, "height": 800}], "blocks": []} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout): + request = AsyncMock() + request.json.return_value = {'bbox': [1, 2, 3]} + with self.assertRaises(HTTPException) as err: + await magazine_update_region('a' * 32, 1, 'b1', request) + self.assertEqual(err.exception.status_code, 400) + + async def test_unknown_region_is_a_400(self): + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_update_region + layout = {"pages": [{"page": 1, "width": 600, "height": 800}], "blocks": [], "images": []} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout), \ + patch('routes.magazine._load_settings', return_value={}), \ + patch('routes.magazine.asyncio.to_thread', new=_run_sync): + request = AsyncMock() + request.json.return_value = {'bbox': [1, 2, 3, 4]} + with self.assertRaises(HTTPException) as err: + await magazine_update_region('a' * 32, 1, 'missing', request) + self.assertEqual(err.exception.status_code, 400) + + async def test_no_layout_is_a_404(self): + from unittest.mock import AsyncMock, patch + from routes.magazine import magazine_update_region + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=None): + request = AsyncMock() + request.json.return_value = {'bbox': [1, 2, 3, 4]} + with self.assertRaises(HTTPException) as err: + await magazine_update_region('a' * 32, 1, 'b1', request) + self.assertEqual(err.exception.status_code, 404) + + +class DeleteRegionRouteTests(unittest.IsolatedAsyncioTestCase): + async def test_removes_a_known_block(self): + from unittest.mock import patch + from routes.magazine import magazine_delete_region + layout = {"blocks": [{"id": "b1", "page": 1}], "images": []} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout), \ + patch('routes.magazine.mag.save_layout') as save: + result = await magazine_delete_region('a' * 32, 1, 'b1') + self.assertTrue(result['ok']) + save.assert_called_once() + + async def test_removes_a_known_image(self): + from unittest.mock import patch + from routes.magazine import magazine_delete_region + layout = {"blocks": [], "images": [{"id": "img1", "page": 1}]} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout), \ + patch('routes.magazine.mag.save_layout'): + result = await magazine_delete_region('a' * 32, 1, 'img1') + self.assertTrue(result['ok']) + + async def test_unknown_id_is_a_404(self): + from unittest.mock import patch + from routes.magazine import magazine_delete_region + layout = {"blocks": [], "images": []} + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=layout): + with self.assertRaises(HTTPException) as err: + await magazine_delete_region('a' * 32, 1, 'missing') + self.assertEqual(err.exception.status_code, 404) + + async def test_no_layout_is_a_404(self): + from unittest.mock import patch + from routes.magazine import magazine_delete_region + with patch('routes.magazine._require_magazine', return_value=(None, {})), \ + patch('routes.magazine.mag.load_layout', return_value=None): + with self.assertRaises(HTTPException) as err: + await magazine_delete_region('a' * 32, 1, 'b1') + self.assertEqual(err.exception.status_code, 404)