From 5adba6b4c08ba30dbd9ca39a7a8e954951b1a409 Mon Sep 17 00:00:00 2001 From: mARTin-B78 Date: Mon, 7 Sep 2026 14:00:34 +0200 Subject: [PATCH] feat: split-screen article review with numbered regions, per-page/whole-magazine "OCR again", and a real column-interleaving fix it found Split-screen review: the rendered page (left) with a numbered box per detected block, the extracted text (right) with matching numbers. Clicking either side highlights the other. Checkboxes moved from a flat list into this layout without changing their semantics (still gate what gets narrated). "OCR again": reocr_page() forces one page through the configured vision engine again, replacing only that page's blocks -- deliberately does NOT call build_articles(), so an already-approved article is not silently re-grouped. A magazine-level option re-reads every page and re-runs detection (with a confirmation, since that does reset article status). extract_pages() gained force_vision_ocr to skip its quality gate for this case. Building the numbered-box review immediately did what it was built to do -- surfaced a real bug on live production data (real magazine, real scan, not a synthetic fixture): a photo caption at 49.9% of the page width sat in the gutter between two genuinely separate columns and merged them into one, so every block on the page came back column=0 and reading order fell back to pure top-to-bottom, interleaving the two columns' sentences. Fixed two compounding causes: column boundary detection now excludes blocks much wider than the page's median width, and detect_columns' gutter threshold (3.5%) was itself narrower than some real magazine gutters (~2.1% measured) -- lowered to 1.5%. 10 new/updated tests, including one built directly from the real measured page-17 block coordinates that exposed the bug. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 27 ++++ architecture/magazine-ingestion.md | 18 +++ core/magazine.py | 92 +++++++++++++- routes/magazine.py | 78 +++++++++++- static/js/magazine.js | 191 ++++++++++++++++++++++++++--- static/sections/s-magazine.html | 3 + static/style.css | 57 ++++++++- tests/test_magazine.py | 135 ++++++++++++++++++++ 8 files changed, 573 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd2868e..feb2955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,34 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi ## [Unreleased] +### Added +- **Split-Screen Article Review, "OCR Again", Reading-Order Fixes.** Review shows the rendered page on the left with a + numbered box per detected region and the extracted text on the right with matching numbers — clicking either side + highlights the other. Building this immediately surfaced a real bug on live production data (see Fixed below). + A per-page **"OCR again"** button forces that one page through the configured vision engine again without touching + article grouping or approvals (`reocr_page`); a magazine-level **"OCR again (whole magazine)"** re-reads every page + and re-runs detection, with a confirmation when articles were already reviewed since that resets their status. + ### Fixed +- **Two Real Magazine Columns Interleaved Into One.** Found live using the new split-screen review's numbered boxes on + a real scanned c't magazine page: a photo caption at 49.9% of the page width sat in the gutter between two genuinely + separate columns and merged them into a single detected column, so every block on the page came back `column=0` and + reading order fell back to pure top-to-bottom — splicing the two columns' sentences together (a box the review + numbered 6, physically above the box numbered 1, was read after it). Column boundary detection now excludes blocks + much wider than the page's median block width; separately, the real gutter measured on that page was itself only + ~2.1% of the page width, narrower than `detect_columns`' 3.5% threshold — lowered to 1.5%. +- **Magazine "Forgets" the Open Document.** A full page reload or the browser's own Back button tears down all + in-memory state; the magazine section always landed back on the empty library with no way back to what was open. + Now persists the last-opened magazine id and reopens it automatically. The actual root cause of a reported duplicate: + Studio/Reader's book library and Library → Books both listed every document with no `kind` filter, so the magazine + was already showing up there disguised as a plain book — it had not vanished, it was misfiled, which is what led to + a re-upload and a genuine duplicate under a different kind. Both listings now exclude `kind: "magazine"`. +- **Confidence Badge Layout (Studio Characters Review).** The badge rendered on its own line far below the speaker + name because both the name and the article text were `display: block`, forcing a line break regardless of markup + order. Wrapped the name, badge and check button in a shared flex row so the badge now sits directly beside the name. +- **docker-socket-proxy Crash Loop.** `read_only: true` left the container's own entrypoint nowhere to write its + `haproxy.cfg`, crash-looping it and leaving the app without Docker access. `cap_drop: ALL`, `no-new-privileges` and + the `:ro` socket mount already provide the real hardening, so `read_only` was removed for this one service. - **Magazine Dropzone Opened the Dropped File in the Browser Instead of Accepting It.** `dragover`'s `preventDefault()` was gated on `dataTransfer.types` including `'Files'`, which Chromium does not reliably populate synchronously on every `dragover` event during a real OS file drag — the condition failed often enough that the element never diff --git a/architecture/magazine-ingestion.md b/architecture/magazine-ingestion.md index 80451be..e6ce877 100644 --- a/architecture/magazine-ingestion.md +++ b/architecture/magazine-ingestion.md @@ -78,6 +78,24 @@ magazines change column count per page — a fixed grid is wrong. Blocks spannin >60% of the page width sort ahead of both columns, so a full-width headline precedes the body it introduces. +**Column boundary detection excludes unusually wide blocks, and the gutter +threshold is narrower than it looks.** Confirmed live on a real scanned c't +page, using the split-screen review's numbered boxes (built specifically to +make exactly this kind of error visible): a photo caption at 49.9% of the +page width — under the 60% "spans the page" threshold above, so not given +special handling — sat in the whitespace gap between two genuinely separate +columns and merged them into one detected column. Every block on the page +came back `column=0`, and reading order fell back to pure top-to-bottom, +splicing the two real columns' sentences together — the box numbered 6 in +review (a caption physically above the box numbered 1) was read *after* it. +Column boundaries are now detected from blocks close to the page's median +width only, and the real gutter measured on that page was itself only ~2.1% +of the page width, narrower than the 3.5% `detect_columns` started with — now +1.5%. A single block much narrower than the surrounding column (a stray +caption fragment) can still form a spurious extra column of its own; the +main failure (two real columns interleaved) is fixed, that residual +narrower one is not. + **Furniture is identified by repetition, not position.** A running head and a headline both sit at the top of a page. What separates them is that the running head recurs on many pages at the same edge; `find_repeated_lines` needs the diff --git a/core/magazine.py b/core/magazine.py index da75e1d..d6ebe95 100644 --- a/core/magazine.py +++ b/core/magazine.py @@ -141,13 +141,20 @@ def bbox_overlaps(a: Sequence[float], b: Sequence[float], tol: float = 0.0) -> b def detect_columns(blocks: Sequence[Block], page_width: float, - min_gap_ratio: float = 0.035) -> list[tuple[float, float]]: + min_gap_ratio: float = 0.015) -> list[tuple[float, float]]: """Infer the column grid from where blocks are *absent*. Magazines vary the column count per page, so a fixed grid is wrong. The reliable signal is vertical whitespace running the height of the page: we project every block onto the x axis, then split wherever the uncovered gap is wider than a few percent of the page. Returns (left, right) spans. + + 1.5%, not the rounder 3.5% this started at: measured on a real scanned + c't magazine page, the actual gutter between two genuine columns was + only ~2.1% of the page width -- narrower than the old threshold, which + made detect_columns see one merged column and, combined with a wide + caption bridging the same gap (see assign_reading_order), produced + every block on the page landing in column=0. """ spans = sorted( ((b.bbox[0], b.bbox[0] + b.bbox[2]) for b in blocks if b.bbox[2] > 1), @@ -171,8 +178,23 @@ def assign_reading_order(blocks: list[Block], page_width: float) -> list[Block]: Sorting purely by y interleaves parallel columns, which is precisely the failure that splices one article's sentence into another's. + + Column BOUNDARIES are detected from blocks close to the page's typical + width only. Confirmed live on a real scanned magazine: a caption running + under a photo, at 49.9% of the page width -- under the "spans the whole + page" threshold below, so not given special handling -- sat squarely in + the whitespace gap between two real text columns and bridged them into + one merged span, which meant EVERY block on the page got column=0 and + reading order fell back to pure top-to-bottom, splicing the two + columns' sentences together (the exact failure this function exists to + prevent). Every block, wide ones included, is still assigned to + whichever column its centre or overlap lands in once boundaries are + known -- only the boundary DETECTION excludes them. """ - columns = detect_columns(blocks, page_width) + widths = sorted(b.bbox[2] for b in blocks if b.bbox[2] > 1) + median_w = widths[len(widths) // 2] if widths else 0 + narrow = [b for b in blocks if median_w <= 0 or b.bbox[2] <= median_w * 1.4] + columns = detect_columns(narrow if len(narrow) >= 2 else blocks, page_width) def column_of(b: Block) -> int: centre = b.bbox[0] + b.bbox[2] / 2 @@ -1003,6 +1025,7 @@ def extract_pages(pdf_path: Path, out_dir: Path, render_dpi: int = 110, vision_ocr_fallback_url: str | None = None, vision_ocr_fallback_model: str | None = None, vision_ocr_timeout: float = 150.0, + force_vision_ocr: bool = False, progress: Any = None) -> dict[str, Any]: """Run the full extraction for one PDF, writing page renders and images. @@ -1111,7 +1134,7 @@ def extract_pages(pdf_path: Path, out_dir: Path, render_dpi: int = 110, ocr_quality_score(" ".join(b.text for b in blocks)) if not needs_ocr else 0.0 ) - needs_escalation = needs_ocr or native_quality < _VISION_QUALITY_THRESHOLD + needs_escalation = force_vision_ocr or needs_ocr or native_quality < _VISION_QUALITY_THRESHOLD vision_blocks: list[Block] | None = None if vision_enabled and needs_escalation: @@ -1326,6 +1349,69 @@ def build_articles(layout: dict[str, Any]) -> list[dict[str, Any]]: return out +def reocr_page(doc_dir: Path, layout: dict[str, Any], page_no: int, + vision_ocr_url: str, vision_ocr_model: str, + vision_ocr_fallback_url: str | None = None, + vision_ocr_fallback_model: str | None = None, + timeout: float = 150.0) -> dict[str, Any] | None: + """Force one page through the vision OCR chain again with the engine the + user picked, replacing only that page's blocks in an already-ingested + layout. + + Unlike extract_pages()'s automatic escalation, this always calls the + engine regardless of the page's existing text quality -- a user pressing + "OCR again" has already judged the result unusable and is not asking for + another quality gate. Deliberately does NOT call build_articles(): an + article the user already reviewed and approved must not be silently + re-grouped and lose that approval just because one of its pages was + re-read. The caller decides whether to re-run detection separately. + + Returns the updated layout, or None if the page has no rendered PNG or + the engine returned nothing usable (network failure, empty response) -- + the caller keeps the existing blocks in that case, never discards good + data for a failed retry. + """ + fitz = _require_fitz() + png = doc_dir / "pages" / f"{page_no:04d}.png" + if not png.is_file(): + return None + + page_meta = next((p for p in layout.get("pages", []) if p["page"] == page_no), None) + if not page_meta: + return None + + engines = [(vision_ocr_url, vision_ocr_model), + (vision_ocr_fallback_url, vision_ocr_fallback_model)] + regions = vision_ocr_page_chain(png, engines, timeout=timeout) + if not regions: + return None + + scale_x = page_meta["width"] / max(1.0, _png_width(png, fitz)) + new_blocks = build_vision_blocks(regions, page_no, scale_x, id_prefix="r") + + all_blocks = [Block(**b) for b in layout.get("blocks", [])] + kept = [b for b in all_blocks if b.page != page_no] + + # The baseline must come from the rest of the document, not this page + # alone -- see classify_blocks' docstring on why a single page (which may + # have no body copy of its own) cannot be its own reference. + doc_base = body_font_size(kept) or body_font_size(all_blocks) + classify_blocks(new_blocks, page_meta["height"], page_meta["width"], base_size=doc_base) + mark_advert_pages(new_blocks) + mark_pullquotes(new_blocks) + assign_reading_order(new_blocks, page_meta["width"]) + mark_sidebars(new_blocks, page_meta["width"]) + + merged = kept + new_blocks + page_heights = {p["page"]: p["height"] for p in layout.get("pages", [])} + cluster_running_furniture(merged, page_heights) + + layout = dict(layout) + layout["blocks"] = [b.to_json() for b in merged] + layout["vision_ocr_pages"] = int(layout.get("vision_ocr_pages") or 0) + 1 + 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 7ab4cc9..3a462e8 100644 --- a/routes/magazine.py +++ b/routes/magazine.py @@ -111,7 +111,8 @@ async def magazine_create(request: Request): def _ingest_sync(doc_id: str, job_id: str, ocr_lang: str, render_dpi: int, vision_ocr_url: str | None, vision_ocr_model: str | None, vision_ocr_fallback_url: str | None = None, - vision_ocr_fallback_model: str | None = None) -> None: + vision_ocr_fallback_model: str | None = None, + force_vision_ocr: bool = False) -> None: """Run the extraction pipeline; recorded as a restart-safe job.""" d = _LIB / doc_id pdf = d / "source.pdf" @@ -131,6 +132,7 @@ def _ingest_sync(doc_id: str, job_id: str, ocr_lang: str, render_dpi: int, vision_ocr_model=vision_ocr_model, vision_ocr_fallback_url=vision_ocr_fallback_url, vision_ocr_fallback_model=vision_ocr_fallback_model, + force_vision_ocr=force_vision_ocr, progress=progress) mag.save_layout(d, layout) @@ -203,6 +205,13 @@ async def magazine_ingest(doc_id: str, request: Request): if not (vision_ocr_fallback_url and vision_ocr_fallback_model): vision_ocr_fallback_url = vision_ocr_fallback_model = None + # "OCR again" for the whole magazine: skip the quality gate and send + # every page through the vision engine regardless of how good its + # existing text already looks. Only meaningful with vision OCR actually + # enabled -- silently ignored otherwise, since there is no engine to + # force a page through. + force_vision_ocr = bool(body.get("force_vision_ocr")) and vision_ocr_url is not None + job_id = uuid.uuid4().hex background_job_create(job_id, "magazine-ingest", fingerprint=doc_id) meta["ingest"] = {"status": "running", "job_id": job_id} @@ -211,7 +220,8 @@ async def magazine_ingest(doc_id: str, request: Request): asyncio.create_task( asyncio.to_thread(_ingest_sync, doc_id, job_id, ocr_lang, render_dpi, vision_ocr_url, vision_ocr_model, - vision_ocr_fallback_url, vision_ocr_fallback_model) + vision_ocr_fallback_url, vision_ocr_fallback_model, + force_vision_ocr) ) return {"job_id": job_id} @@ -356,6 +366,70 @@ async def magazine_page_image(doc_id: str, page: int): return FileResponse(png, media_type="image/png") +def _reocr_page_sync(doc_id: str, job_id: str, page: int, + vision_ocr_url: str, vision_ocr_model: str, + vision_ocr_fallback_url: str | None, + vision_ocr_fallback_model: str | None) -> None: + """"OCR again" on one page with the engine the user picked. Restart-safe + job for the same reason ingestion is: a vision OCR call can take up to + ~40s (confirmed live), too long for a plain request/response cycle. + """ + d = _LIB / doc_id + try: + layout = mag.load_layout(d) + if not layout: + raise FileNotFoundError("No layout yet -- run ingestion first") + background_job_update(job_id, stage="reocr", detail=f"Re-reading page {page}") + updated = mag.reocr_page(d, layout, page, vision_ocr_url, vision_ocr_model, + vision_ocr_fallback_url, vision_ocr_fallback_model) + if updated is None: + raise RuntimeError("The OCR engine returned nothing usable for this page") + mag.save_layout(d, updated) + background_job_update(job_id, stage="done", done=True, + detail=f"Page {page} re-read", result={"page": page}) + except Exception as exc: # noqa: BLE001 - surfaced to UI + background_job_update(job_id, stage="failed", done=True, error=str(exc)[:1000]) + + +@router.post("/api/magazine/{doc_id}/pages/{page}/reocr") +async def magazine_reocr_page(doc_id: str, page: int, request: Request): + """Force one page through the configured vision OCR engine again, + regardless of its current text quality -- for when the user has already + judged the result unusable. Only replaces that page's blocks; article + boundaries and approvals are left untouched (call the ingest endpoint's + detection separately if the article needs re-grouping too). + """ + d, _ = _require_magazine(doc_id) + if page < 1 or page > 10000: + raise HTTPException(400, "Bad page") + if not (d / "pages" / f"{page:04d}.png").is_file(): + raise HTTPException(404, "Page not rendered -- run ingestion first") + + body = {} + try: + body = await request.json() + except Exception: + pass + settings = _load_settings() + vision_ocr_url = str(body.get("vision_ocr_url") or settings.get("vision_ocr_url") or "").strip() + vision_ocr_model = str(body.get("vision_ocr_model") or settings.get("vision_ocr_model") or "").strip() + if not (vision_ocr_url and vision_ocr_model): + raise HTTPException(400, "No vision OCR engine configured -- set one in Settings") + vision_ocr_fallback_url = str( + body.get("vision_ocr_fallback_url") or settings.get("vision_ocr_fallback_url") or "").strip() or None + vision_ocr_fallback_model = str( + body.get("vision_ocr_fallback_model") or settings.get("vision_ocr_fallback_model") or "").strip() or None + + job_id = uuid.uuid4().hex + background_job_create(job_id, "magazine-reocr-page", fingerprint=f"{doc_id}:{page}") + asyncio.create_task( + asyncio.to_thread(_reocr_page_sync, doc_id, job_id, page, + vision_ocr_url, vision_ocr_model, + vision_ocr_fallback_url, vision_ocr_fallback_model) + ) + return {"job_id": job_id} + + @router.get("/api/magazine/{doc_id}/images/{name}") async def magazine_image(doc_id: str, name: str): d, _ = _require_magazine(doc_id) diff --git a/static/js/magazine.js b/static/js/magazine.js index 0f73fdb..63e52eb 100644 --- a/static/js/magazine.js +++ b/static/js/magazine.js @@ -233,6 +233,30 @@ async function magIngest() { } } +// "OCR again" for the whole issue: forces every page through the vision +// engine regardless of its current quality, then re-detects articles -- +// which is why this asks for confirmation. A single page's "OCR again" in +// review does not touch article grouping; re-reading the whole document +// does, since detection runs again over the freshly re-read text. +async function magReocrMagazine() { + if (!state.doc) return; + const hasReviewed = state.articles.some((a) => a.status !== 'detected'); + if (hasReviewed && !confirm( + 'Re-reading the whole magazine runs article detection again. ' + + 'Any articles you already approved or rejected will need reviewing again. Continue?' + )) return; + try { + await api('/api/magazine/' + state.doc.id + '/ingest', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ force_vision_ocr: true }), + }); + pollIngest(); + } catch (e) { + say('Could not start OCR: ' + e.message, 'err'); + } +} + // "Analysing page 28 of 204" -> a real percentage, not just spinner text. function magParsePageProgress(detail) { const m = /page (\d+) of (\d+)/i.exec(detail || ''); @@ -364,6 +388,11 @@ async function magReview(articleId) { if (!article) return; state.article = article; await ensureLayout(); + // The whole review is one page at a time -- showing every page of a + // 10-page feature stacked at once was unreadable and made the boxes tiny. + state.reviewSpan = magReviewSpan(article); + state.reviewPage = state.reviewSpan[0] || null; + _magReviewChecked = null; magShow('review'); renderReview(); } @@ -385,18 +414,35 @@ function blocksForPages(pages) { .sort((a, b) => a.page - b.page || a.reading_order - b.reading_order); } +// Offer neighbouring pages too, so a boundary that started a page late (or a +// jump tail that should have been included) can be corrected without +// leaving the screen. +function magReviewSpan(article) { + if (!article.pages.length) return []; + return Array.from(new Set(article.pages.concat( + [article.pages[0] - 1, article.pages[article.pages.length - 1] + 1]))) + .filter((p) => p >= 1 && p <= (state.layout.page_count || 0)).sort((x, y) => x - y); +} + +function magReviewGoPage(page) { + if (!state.reviewSpan.includes(page)) return; + state.reviewPage = page; + renderReview(); +} + function renderReview() { const host = $('mag-review'); const a = state.article; if (!host || !a) return; const included = new Set(a.blocks || []); - // Offer neighbouring pages too, so a boundary that started a page late can be - // corrected without leaving the screen. - const span = a.pages.length - ? Array.from(new Set(a.pages.concat([a.pages[0] - 1, a.pages[a.pages.length - 1] + 1]))) - .filter((p) => p >= 1 && p <= (state.layout.page_count || 0)).sort((x, y) => x - y) - : []; const conf = confidenceLabel(a.confidence); + const page = state.reviewPage; + const pageMeta = (state.layout.pages || []).find((p) => p.page === page); + const pageBlocks = page == null ? [] : blocksForPages([page]); + + const spanIdx = state.reviewSpan.indexOf(page); + const prevPage = spanIdx > 0 ? state.reviewSpan[spanIdx - 1] : null; + const nextPage = spanIdx >= 0 && spanIdx < state.reviewSpan.length - 1 ? state.reviewSpan[spanIdx + 1] : null; let html = '
' + @@ -405,27 +451,65 @@ function renderReview() { '
' + esc(conf.text) + '
' + '
' + '

Untick anything that is not part of this article — a sidebar, an advert, ' + - 'or a neighbouring piece. Only ticked text is read aloud.

'; + 'or a neighbouring piece. Only ticked text is read aloud. Click a numbered region on the page, ' + + 'or its matching entry on the right, to line the two up.

'; - let currentPage = null; + html += '
' + + '' + + 'Page ' + (page ?? '—') + '' + + '' + + '' + + '' + + '' + + '
'; + + html += '
'; + + // Left: the rendered page with a numbered box per block. + html += '
'; + if (page != null) { + html += '
' + + 'Page ' + page + ''; + pageBlocks.forEach((b, i) => { + if (!pageMeta) return; + const on = included.has(b.id); + const left = (b.bbox[0] / pageMeta.width * 100).toFixed(2); + const top = (b.bbox[1] / pageMeta.height * 100).toFixed(2); + const w = (b.bbox[2] / pageMeta.width * 100).toFixed(2); + const h = (b.bbox[3] / pageMeta.height * 100).toFixed(2); + html += '
' + + '' + (i + 1) + '
'; + }); + html += '
'; + } else { + html += '

No page to show.

'; + } + html += '
'; + + // Right: the same blocks as text, numbered to match. html += '
'; - blocksForPages(span).forEach((b) => { - if (b.page !== currentPage) { - currentPage = b.page; - html += '
Page ' + b.page + - '
'; - } + pageBlocks.forEach((b, i) => { const narratable = ['title', 'deck', 'byline', 'body'].includes(b.type); const on = included.has(b.id); - html += '
' + - '
' + + if (!pageBlocks.length) html += '

Nothing detected on this page.

'; + html += '
'; + + html += '
'; // /mag-review-split + + html += '
' + '' + '' + '' + @@ -433,11 +517,74 @@ function renderReview() { host.innerHTML = html; } +// Tracks every ticked block across ALL reviewed pages of this article, not +// just the one currently on screen -- switching pages must not forget a +// change made on a previous one. +let _magReviewChecked = null; + +function magReviewToggleBox(input) { + if (!_magReviewChecked) _magReviewChecked = new Set(state.article.blocks || []); + if (input.checked) _magReviewChecked.add(input.dataset.block); + else _magReviewChecked.delete(input.dataset.block); + const box = document.querySelector('.mag-review-box[data-block="' + CSS.escape(input.dataset.block) + '"]'); + if (box) box.classList.toggle('is-excluded', !input.checked); +} + +function magReviewFocusBlock(blockId) { + const cb = document.querySelector('#mag-review input[data-block="' + CSS.escape(blockId) + '"]'); + 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' }); +} + +function magReviewHighlightBox(blockId) { + document.querySelectorAll('.mag-review-box.is-linked').forEach((el) => el.classList.remove('is-linked')); + if (!blockId) return; + const box = document.querySelector('.mag-review-box[data-block="' + CSS.escape(blockId) + '"]'); + if (box) box.classList.add('is-linked'); +} + function selectedBlockIds() { + if (_magReviewChecked) return Array.from(_magReviewChecked); return Array.from(document.querySelectorAll('#mag-review input[data-block]:checked')) .map((el) => el.dataset.block); } +async function magReocrPage() { + const page = state.reviewPage; + if (page == null) return; + const btn = $('mag-reocr-btn'); + const statusEl = $('mag-reocr-status'); + if (btn) btn.disabled = true; + if (statusEl) statusEl.textContent = 'Reading page ' + page + '…'; + let jobId; + try { + const r = await api('/api/magazine/' + state.doc.id + '/pages/' + page + '/reocr', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, body: '{}', + }); + jobId = r.job_id; + } catch (e) { + if (statusEl) statusEl.textContent = ''; + if (btn) btn.disabled = false; + return say('Could not start OCR: ' + e.message, 'err'); + } + for (;;) { + let job; + try { job = await api('/api/jobs/' + jobId); } catch (_) { break; } + if (job.done) { + if (job.error) say('OCR failed: ' + job.error, 'err'); + else say('Page ' + page + ' re-read', 'ok'); + break; + } + await new Promise((r) => setTimeout(r, 2000)); + } + if (btn) btn.disabled = false; + if (statusEl) statusEl.textContent = ''; + state.layout = null; + await ensureLayout(); + renderReview(); +} + async function magApprove() { const a = state.article; if (!a) return; @@ -1036,6 +1183,12 @@ root.magOpen = magOpen; root.magCreate = magCreate; root.magIngest = magIngest; root.magReview = magReview; +root.magReviewGoPage = magReviewGoPage; +root.magReviewFocusBlock = magReviewFocusBlock; +root.magReviewHighlightBox = magReviewHighlightBox; +root.magReviewToggleBox = magReviewToggleBox; +root.magReocrPage = magReocrPage; +root.magReocrMagazine = magReocrMagazine; root.magApprove = magApprove; root.magSetStatus = magSetStatus; root.magRead = magRead; diff --git a/static/sections/s-magazine.html b/static/sections/s-magazine.html index 4d586dc..9cfd6e3 100644 --- a/static/sections/s-magazine.html +++ b/static/sections/s-magazine.html @@ -52,6 +52,9 @@ +
diff --git a/static/style.css b/static/style.css index 37561d7..e72c78b 100644 --- a/static/style.css +++ b/static/style.css @@ -4765,8 +4765,29 @@ input[list="lang-codes"] { .mag-bar { display: flex; align-items: center; gap: 12px; margin-bottom: 14px; flex-wrap: wrap; } .mag-bar h3 { margin: 0; } .mag-bar-actions { margin-left: auto; } -.mag-status { padding: 8px 12px; border-radius: var(--radius); background: var(--panel); - color: var(--subtext); margin-bottom: 12px; } +.mag-status { padding: 12px 14px; border-radius: var(--radius); background: var(--panel); + color: var(--subtext); margin-bottom: 12px; border-left: 3px solid var(--border); } +.mag-status.is-running { border-left-color: var(--accent); } +.mag-status.is-done { border-left-color: var(--green); } +.mag-status.is-failed { border-left-color: var(--red); } +.mag-ingest-row { display: flex; align-items: center; gap: 10px; } +.mag-ingest-icon { font-size: 20px; flex-shrink: 0; } +.mag-status.is-running .mag-ingest-icon { color: var(--accent); animation: mag-spin 1.6s linear infinite; } +.mag-status.is-done .mag-ingest-icon { color: var(--green); } +.mag-status.is-failed .mag-ingest-icon { color: var(--red); } +.mag-ingest-text { flex: 1; min-width: 0; display: flex; flex-direction: column; gap: 6px; } +.mag-ingest-text strong { font-weight: 500; color: var(--text); font-size: 13px; + overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } +.mag-ingest-track { height: 6px; border-radius: 3px; background: var(--border); overflow: hidden; } +.mag-ingest-fill { height: 100%; background: var(--accent); border-radius: 3px; + transition: width .4s ease; } +.mag-ingest-pct { font-size: 13px; font-weight: 600; color: var(--accent); flex-shrink: 0; + min-width: 3.2em; text-align: right; } +@keyframes mag-spin { from { transform: rotate(0deg); } to { transform: rotate(360deg); } } +@media (prefers-reduced-motion: reduce) { + .mag-status.is-running .mag-ingest-icon { animation: none; } + .mag-ingest-fill { transition: none; } +} .mag-pill { display: inline-block; padding: 2px 8px; border-radius: 999px; font-size: 12px; background: var(--panel); color: var(--subtext); border: 1px solid var(--border); } @@ -4791,15 +4812,43 @@ input[list="lang-codes"] { .mag-review-head input { padding: 8px 10px; border: 1px solid var(--border); border-radius: var(--radius); background: var(--surface); color: var(--text); } .mag-page-head { margin: 16px 0 6px; font-size: 13px; color: var(--subtext); } -.mag-block { display: grid; grid-template-columns: auto 90px 1fr; gap: 10px; align-items: start; + +/* ── Review page navigator + "OCR again" ─────────────────────────────────── */ +.mag-review-pagenav { display: flex; align-items: center; gap: 10px; margin-bottom: 12px; } +.mag-review-pagenav span { font-size: 13px; color: var(--subtext); min-width: 60px; text-align: center; } +.mag-review-pagenav-spacer { flex: 1; min-width: 0; } + +/* ── Split screen: rendered page (left) + extracted blocks (right) ───────── */ +.mag-review-split { display: grid; grid-template-columns: 1.1fr 1fr; gap: 18px; align-items: start; } +.mag-review-page { position: sticky; top: 12px; } +.mag-review-page-wrap { position: relative; border: 1px solid var(--border); border-radius: var(--radius); + overflow: hidden; background: var(--panel); } +.mag-review-page-wrap img { width: 100%; height: auto; display: block; } +.mag-review-box { position: absolute; border: 2px solid var(--accent); background: rgba(37,99,235,.10); + cursor: pointer; transition: background .12s, border-color .12s; } +.mag-review-box:hover { background: rgba(37,99,235,.22); } +.mag-review-box.is-excluded { border-color: var(--subtext); background: rgba(120,120,120,.10); } +.mag-review-box.is-excluded:hover { background: rgba(120,120,120,.22); } +.mag-review-box.is-linked { border-color: var(--yellow); background: rgba(217,119,6,.22); z-index: 2; } +.mag-review-box-num { position: absolute; top: -1px; left: -1px; min-width: 16px; padding: 0 3px; + background: var(--accent); color: var(--on-accent); font-size: 10px; font-weight: 700; + line-height: 16px; text-align: center; border-radius: 0 0 4px 0; } +.mag-review-box.is-excluded .mag-review-box-num { background: var(--subtext); } + +.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; } .mag-block.is-aux { opacity: .68; } .mag-block:focus-within { border-color: var(--accent); box-shadow: 0 0 0 3px rgba(37,99,235,.14); } +.mag-block-num { font-size: 11px; font-weight: 700; color: var(--accent); min-width: 1.4em; } .mag-block-type { font-size: 11px; text-transform: uppercase; letter-spacing: .05em; color: var(--subtext); } .mag-block-text { font-size: 14px; line-height: 1.45; } .mag-review-foot { display: flex; gap: 8px; justify-content: flex-end; margin-top: 16px; position: sticky; bottom: 0; padding: 12px 0; background: var(--bg); } +@media (max-width: 980px) { + .mag-review-split { grid-template-columns: 1fr; } + .mag-review-page { position: static; } +} .mag-controls { display: flex; gap: 12px; align-items: center; flex-wrap: wrap; margin-bottom: 14px; padding: 10px 12px; border: 1px solid var(--border); border-radius: var(--radius); background: var(--surface); } @@ -4874,7 +4923,7 @@ input[list="lang-codes"] { .mag-new-grid, .mag-review-head { grid-template-columns: 1fr; } .mag-toc-row { grid-template-columns: 1fr; } .mag-toc-actions { justify-content: flex-start; } - .mag-block { grid-template-columns: auto 1fr; } + .mag-block { grid-template-columns: auto auto 1fr; } .mag-block-type { display: none; } .mag-article { font-size: 18px; } .mag-driving-controls { gap: 20px; } diff --git a/tests/test_magazine.py b/tests/test_magazine.py index 551d34e..157203a 100644 --- a/tests/test_magazine.py +++ b/tests/test_magazine.py @@ -9,6 +9,7 @@ from __future__ import annotations import os import tempfile +import shutil import pathlib import unittest from unittest import mock @@ -37,6 +38,7 @@ from core.magazine import ( build_articles, build_vision_blocks, classify_blocks, + reocr_page, detect_columns, estimate_font_size_from_area, estimate_listen_seconds, @@ -103,6 +105,36 @@ class ColumnGeometryTests(unittest.TestCase): order = [b.id for b in assign_reading_order(blocks, PAGE_W)] self.assertEqual(order[0], "head") + def test_a_wide_caption_does_not_bridge_two_real_columns(self): + """Grounded in real data from a live scanned c't magazine page: + every block came back column=0, and reading order fell back to pure + top-to-bottom, splicing the two columns' sentences together. Cause: + a caption running under a photo at 49.9% of the page width -- under + the "spans the whole page" threshold, so not given special handling + -- sat in the whitespace gap between two real columns and merged + them into one. Page width and block widths below are the real + measured values (in points), simplified to two blocks per column.""" + page_w = 583.2 + blocks = [ + block("caption", 1, 31.3, 268.0, 291.3, 23.4, "Caption under a photo, spanning most of the page."), + block("l1", 1, 138.4, 345.4, 201.7, 55.8, "Left column body copy."), + block("l2", 1, 138.9, 401.8, 198.9, 21.4, "Left column continues."), + block("r1", 1, 352.4, 58.0, 199.2, 10.6, "Right column body copy."), + block("r2", 1, 352.7, 79.8, 201.0, 44.0, "Right column continues."), + ] + ordered = assign_reading_order(blocks, page_w) + cols = {b.id: b.column for b in ordered} + self.assertNotEqual(cols["l1"], cols["r1"], + "left and right column blocks landed in the same detected column") + # The left column (l1, l2) must not be interleaved with the right + # column (r1, r2) by y-position -- confirmed live as the actual + # symptom: an earlier left-column block sorted after a later + # right-column one purely because both were column=0. + order = [b.id for b in ordered] + self.assertLess(order.index("l1"), order.index("r2")) + self.assertEqual(cols["l1"], cols["l2"]) + self.assertEqual(cols["r1"], cols["r2"]) + class ClassificationTests(unittest.TestCase): def test_body_size_is_not_hijacked_by_display_type(self): @@ -1019,5 +1051,108 @@ class VisionEngineChainTests(unittest.TestCase): self.assertIsNone(vision_ocr_page_chain(pathlib.Path("/fake.png"), [])) +class ReocrPageTests(unittest.TestCase): + """User-triggered "OCR again" on one page -- must replace only that + page's blocks, never touch build_articles() (an already-approved + article's boundaries must not silently change), and never discard good + existing blocks on a failed retry.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="ttvc-reocr-test-") + self.doc_dir = pathlib.Path(self.tmp) + (self.doc_dir / "pages").mkdir() + (self.doc_dir / "pages" / "0005.png").write_bytes(b"fake-png") + self.layout = { + "page_count": 10, + "pages": [{"page": 5, "width": 595.0, "height": 842.0}], + "blocks": [ + {"id": "b0005-000", "page": 5, "bbox": [50, 60, 470, 50], + "type": BLOCK_TITLE, "text": "Old garbled title", "reading_order": 0, + "confidence": 0.5, "font_size": 26.0, "font_name": "", "bold": True, + "column": 0, "source": "ocr"}, + {"id": "b0009-000", "page": 9, "bbox": [50, 60, 300, 400], + "type": BLOCK_BODY, "text": "Untouched other page copy. " * 40, + "reading_order": 0, "confidence": 0.85, "font_size": 10.0, + "font_name": "", "bold": False, "column": 0, "source": "native"}, + ], + } + self._fitz_patch = mock.patch("core.magazine._require_fitz", return_value=mock.Mock()) + self._pngw_patch = mock.patch("core.magazine._png_width", return_value=595.0) + self._fitz_patch.start() + self._pngw_patch.start() + + def tearDown(self): + self._fitz_patch.stop() + self._pngw_patch.stop() + shutil.rmtree(self.tmp, ignore_errors=True) + + def _mock_chain(self, regions): + return mock.patch("core.magazine.vision_ocr_page_chain", return_value=regions) + + def test_missing_png_returns_none(self): + (self.doc_dir / "pages" / "0005.png").unlink() + out = reocr_page(self.doc_dir, self.layout, 5, "http://e", "m") + self.assertIsNone(out) + + def test_page_not_in_layout_returns_none(self): + out = reocr_page(self.doc_dir, self.layout, 99, "http://e", "m") + self.assertIsNone(out) + + def test_engine_returning_nothing_returns_none_and_keeps_old_blocks(self): + with self._mock_chain(None): + out = reocr_page(self.doc_dir, self.layout, 5, "http://e", "m") + self.assertIsNone(out) + # The original (unsuccessful) caller-visible layout is untouched -- + # reocr_page never mutates its input on failure. + self.assertEqual(len(self.layout["blocks"]), 2) + + def test_successful_reocr_replaces_only_the_target_pages_blocks(self): + regions = [{"label": "title", "bbox": [605, 294, 914, 336], + "text": "1. Irrtum: KI ist ein allgemeiner Jobkiller"}] + with self._mock_chain(regions): + out = reocr_page(self.doc_dir, self.layout, 5, "http://e", "m") + self.assertIsNotNone(out) + pages = {b["page"] for b in out["blocks"]} + self.assertEqual(pages, {5, 9}) + page5 = [b for b in out["blocks"] if b["page"] == 5] + self.assertEqual(len(page5), 1) + self.assertEqual(page5[0]["text"], "1. Irrtum: KI ist ein allgemeiner Jobkiller") + self.assertEqual(page5[0]["type"], BLOCK_TITLE) + # Page 9's block is byte-for-byte the original -- never regrouped or + # reclassified just because a different page was re-OCR'd. + page9 = [b for b in out["blocks"] if b["page"] == 9] + self.assertEqual(page9[0]["text"], self.layout["blocks"][1]["text"]) + + def test_vision_ocr_pages_counter_increments(self): + regions = [{"label": "text", "bbox": [50, 60, 500, 400], "text": "Some real text. " * 10}] + with self._mock_chain(regions): + out = reocr_page(self.doc_dir, self.layout, 5, "http://e", "m") + self.assertEqual(out["vision_ocr_pages"], 1) + + def test_baseline_uses_the_rest_of_the_document_not_the_new_page_alone(self): + """A page reduced to one block would otherwise become its own + font-size reference (see classify_blocks' docstring): with only this + page's own text to measure against, its ratio is always exactly 1.0, + which would make it read as body regardless of what it actually is. + Judged against page 9's real ~10pt body baseline instead, a + similarly-sized estimate for ordinary paragraph text should also + land as body -- not because the ratio is trivially 1, but because + the two independently-measured sizes genuinely agree.""" + text = "Body copy text repeated many times to fill the box. " * 4 + regions = [{"label": "text", "bbox": [50, 60, 250, 60], "text": text}] + with self._mock_chain(regions): + out = reocr_page(self.doc_dir, self.layout, 5, "http://e", "m") + page5 = [b for b in out["blocks"] if b["page"] == 5][0] + self.assertEqual(page5["type"], BLOCK_BODY) + + def test_fallback_engine_is_passed_through_the_chain(self): + with mock.patch("core.magazine.vision_ocr_page_chain") as mocked: + mocked.return_value = [{"label": "text", "bbox": [0, 0, 10, 10], "text": "x y z real words"}] + reocr_page(self.doc_dir, self.layout, 5, "http://primary", "m1", + vision_ocr_fallback_url="http://fallback", vision_ocr_fallback_model="m2") + engines_arg = mocked.call_args[0][1] + self.assertEqual(engines_arg, [("http://primary", "m1"), ("http://fallback", "m2")]) + + if __name__ == "__main__": unittest.main()