feat(magazine): let review screen boxes be moved, resized, deleted, and re-OCRed in place
Some checks failed
Test and SonarQube / quality (push) Has been cancelled
Some checks failed
Test and SonarQube / quality (push) Has been cancelled
Adds "Edit boxes" mode to article review: drag an already-detected
region to move it, its corner handle to resize it, or press its x to
delete it -- for correcting a box the layout engine got wrong, not one
it missed entirely ("Draw it" still covers that case). Moving or
resizing opens a confirm panel; "Save" keeps the region's existing
text at its new position, and for text regions "Save & re-OCR" reads
the corrected area again and replaces the text.
New core/magazine.py update_region()/delete_region() and PATCH/DELETE
/api/magazine/{doc}/pages/{page}/regions/{block_id}. An image region
is re-cropped from the page render whenever its bbox changes, since
its saved file is exactly that crop.
Live-verified against the real ingested c't KI Wissen 2026 magazine:
add -> move -> move+re-OCR (correctly kept old text when the corrected
area had nothing readable) -> delete -> confirmed gone, repeat delete
404s.
275 tests passing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
53624d470b
commit
2dd5cea7b7
11
CHANGELOG.md
11
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
|
||||
|
||||
12
Manual.md
12
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
|
||||
|
||||
|
||||
3
TODO.md
3
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,
|
||||
|
||||
@ -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")
|
||||
|
||||
@ -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."""
|
||||
|
||||
@ -741,6 +741,9 @@ function renderReview() {
|
||||
'<span class="mdi mdi-book-open-page-variant-outline" aria-hidden="true"></span> Source / footer</button>' +
|
||||
'<button type="button" class="btn-sm" onclick="magReviewToggleDraw(\'ad\')">' +
|
||||
'<span class="mdi mdi-advertisements-outline" aria-hidden="true"></span> Advertisement</button>' +
|
||||
'<button type="button" class="btn-sm" id="mag-edit-toggle" aria-pressed="false" ' +
|
||||
'title="Drag a region to move or resize it, or delete one that was detected wrongly" ' +
|
||||
'onclick="magReviewToggleEdit()"><span class="mdi mdi-vector-polygon" aria-hidden="true"></span> Edit boxes</button>' +
|
||||
'<button type="button" class="btn-sm" onclick="magReviewClearCurrentPage()"><span class="mdi mdi-checkbox-multiple-blank-outline" aria-hidden="true"></span> Clear all fields</button></div>' +
|
||||
'<span class="mag-review-pagenav-spacer"></span>' +
|
||||
'<button type="button" class="btn-sm" id="mag-refine-figures-btn" onclick="magRefineFigures()" ' +
|
||||
@ -790,11 +793,17 @@ function renderReview() {
|
||||
const owner = magBlockOwner(r.id);
|
||||
const icon = magBlockIcon(isImage ? r.data.type || 'image' : r.data.region_label === 'page_number' ? 'page_number' : r.data.type);
|
||||
html += '<div class="mag-review-box' + (on ? '' : ' is-excluded') + (owner ? ' has-owner' : '') + (r.data.type === 'ad' ? ' is-advertisement' : '') +
|
||||
'" data-block="' + esc(r.id) + '" style="left:' + left + '%;top:' + top + '%;width:' + w + '%;height:' + h +
|
||||
'" data-block="' + esc(r.id) + '" data-kind="' + (isImage ? 'image' : 'text') +
|
||||
'" style="left:' + left + '%;top:' + top + '%;width:' + w + '%;height:' + h +
|
||||
'%" onclick="magReviewFocusBlock(\'' + esc(r.id) + '\')" title="' + esc(isImage ? 'figure' : r.data.type) + '">' +
|
||||
'<span class="mag-review-box-num">' + (i + 1) + '</span>' +
|
||||
'<span class="mag-review-box-icon mdi ' + icon + '"></span>' +
|
||||
(owner ? '<span class="mag-review-box-owner">Art. ' + owner.index + '</span>' : '') +
|
||||
'<button type="button" class="mag-review-box-delete" tabindex="-1" ' +
|
||||
'aria-label="Delete this region" title="Delete this region" ' +
|
||||
'onclick="event.stopPropagation(); magReviewDeleteRegion(\'' + esc(r.id) + '\')">' +
|
||||
'<span class="mdi mdi-close" aria-hidden="true"></span></button>' +
|
||||
'<span class="mag-review-box-resize" aria-hidden="true"></span>' +
|
||||
'</div>';
|
||||
});
|
||||
html += '</div>';
|
||||
@ -851,6 +860,7 @@ function renderReview() {
|
||||
'</div>';
|
||||
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 =
|
||||
'<div class="mag-draw-panel-row"><strong>Save this region’s new position?</strong></div>' +
|
||||
(kind === 'text'
|
||||
? '<p class="muted">Save keeps the existing text. Save & re-OCR reads the corrected area again and replaces it.</p>'
|
||||
: '<p class="muted">The saved figure image will be re-cropped to the new box.</p>') +
|
||||
'<div class="mag-draw-panel-row">' +
|
||||
'<button type="button" class="btn-sm" onclick="magReviewCancelEditPanel(true)">Cancel</button>' +
|
||||
(kind === 'text' ? '<button type="button" class="btn-sm" onclick="magReviewSubmitEdit(false)">Save</button>' +
|
||||
'<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(true)"><span class="mdi mdi-text-recognition"></span> Save & re-OCR</button>'
|
||||
: '<button type="button" class="btn-sm primary" onclick="magReviewSubmitEdit(false)">Save</button>') +
|
||||
'</div>';
|
||||
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
|
||||
|
||||
@ -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; }
|
||||
|
||||
@ -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.
|
||||
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user