fix(magazine): re-check for duplicate boxes after box-fitting/seam-mending grows them
The duplicate detector only ran once, before geometry fixes could merge or grow a box into one that now duplicates another's text -- narrating it twice. Now re-run after the geometry loop at ingest and after recover_missing_text in the whole-issue quality run. Confirmed live: 13 such duplicates in a fresh import, 6 in the already-reviewed magazine. TODO: F decided unnecessary per user; G updated with both universal fixes from this pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
1fb5528112
commit
ac1153b782
@ -15,6 +15,7 @@ Follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) · versioned wi
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
- Vision OCR: a refusal sentence the engine hallucinates over an unreadable photo or screenshot ("The image is too blurry to recognize...") is no longer narrated as page text -- discarded when the same sentence repeats across 3+ regions, confirmed live on c't KI Wissen 2026 pages 82, 87 and 124 (29 and 122 hallucinated regions dropped on re-OCR).
|
- Vision OCR: a refusal sentence the engine hallucinates over an unreadable photo or screenshot ("The image is too blurry to recognize...") is no longer narrated as page text -- discarded when the same sentence repeats across 3+ regions, confirmed live on c't KI Wissen 2026 pages 82, 87 and 124 (29 and 122 hallucinated regions dropped on re-OCR).
|
||||||
|
- A duplicate box (its text already held by a wider box) that only becomes a duplicate once a later step in the same run grows or merges a box was left unmarked, so its text was narrated twice -- the duplicate check now also runs after the per-page box-fitting loop (ingest) and after finding missed text (the whole-issue quality run), not only before either. Confirmed live on 20 real pages across two runs of the same issue.
|
||||||
|
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
38
TODO.md
38
TODO.md
@ -99,19 +99,31 @@ rules beat what was built up by hand, and to feed every difference back into the
|
|||||||
- [x] **E — Compare the new result with the backup.** Ran `scripts/compare_magazines.py`; report saved at
|
- [x] **E — Compare the new result with the backup.** Ran `scripts/compare_magazines.py`; report saved at
|
||||||
`architecture/evidence/compare-345b2a84-vs-backup-20260928.{md,json}`. Headline: text similarity 0.937,
|
`architecture/evidence/compare-345b2a84-vs-backup-20260928.{md,json}`. Headline: text similarity 0.937,
|
||||||
box recall/precision 0.88/0.85, reading-order agreement 0.967, 51/56 articles matched.
|
box recall/precision 0.88/0.85, reading-order agreement 0.967, 51/56 articles matched.
|
||||||
- [ ] **F — Show the differences and decide what is better.** Not built: a review view listing every
|
- [x] **F — Show the differences and decide what is better.** Decided not needed: a side-by-side review UI
|
||||||
difference with both versions side by side (page render, boxes, text) and a verdict per difference (new
|
adds no value the markdown/JSON report from E doesn't already give for this workflow.
|
||||||
better / backup better / equal). The markdown report is a stand-in for now; a proper UI is still open.
|
- [~] **G — Adjust the rules so the next magazine needs less work.** Two universal fixes landed 2026-09-28/29,
|
||||||
- [~] **G — Adjust the rules so the next magazine needs less work.** First fix landed 2026-09-28: the worst
|
each traced from the worst-differing pages E flagged, not from one page in isolation:
|
||||||
differing pages (82, 87, 124, 202) all traced to one root cause -- the vision OCR engine hallucinating a
|
1. **Vision refusal hallucinations** (pages 82, 87, 124, 202): the vision OCR engine sometimes answers
|
||||||
refusal sentence ("The image is too blurry to recognize...", "The Ground Truth image displays...") tiled
|
with a sentence describing its own failure ("The image is too blurry to recognize...", "The Ground
|
||||||
across dozens of regions over a photo/screenshot it could not read, narrated as if it were real page text.
|
Truth image displays...") instead of leaving an unreadable photo/screenshot region empty, tiled across
|
||||||
Fixed in `core/magazine.py` (`_drop_repeated_region_text`/`_looks_like_vision_refusal`): text repeated
|
dozens of regions and narrated as real page text. Fixed in `core/magazine.py`
|
||||||
verbatim across 3+ regions that also reads like a refusal is blanked before it becomes blocks. Verified
|
(`_drop_repeated_region_text`/`_looks_like_vision_refusal`): text repeated verbatim across 3+ regions
|
||||||
live: re-OCR of page 82 discarded 29 hallucinated regions, page 87 discarded 122. Regression tests in
|
that also reads like a refusal is blanked before it becomes blocks. Verified live: re-OCR of page 82
|
||||||
`tests/test_magazine.py::VisionOcrRefusalHallucinationTests`. **Still open:** re-run D→E→F with this fix
|
discarded 29 hallucinated regions, page 87 discarded 122.
|
||||||
and keep going through the remaining worst-page list (3, 44, 75, 124, 136, 148, 152, 164) until the new
|
2. **Duplicate check runs too early** (pages 31, 42, 103, 151, 152, and 6 more on the already-reviewed
|
||||||
import matches or beats the backup; then record final numbers in `PROJECT_MAP.md`.
|
backup): the duplicate-box detector (`clean_up_regions`) only ran once, before the box-fitting/seam-
|
||||||
|
mending loop that can grow or merge a box afterward -- a box that becomes a duplicate only because of
|
||||||
|
that growth was left unmarked, so its text was narrated twice. Fixed by re-running the check after that
|
||||||
|
loop at ingest, and again after `recover_missing_text` in the whole-issue quality run
|
||||||
|
(`routes/magazine.py`). Verified live: re-running the check on the current layout alone found 13 such
|
||||||
|
duplicates in the fresh import and 6 in the already-reviewed backup (the bug predates this fix, so the
|
||||||
|
backup itself is not clean either -- its next Auto-fix/quality run will self-heal now).
|
||||||
|
Regression tests: `tests/test_magazine.py::VisionOcrRefusalHallucinationTests`,
|
||||||
|
`tests/test_magazine_routes.py::QualityRunJobTests::test_a_duplicate_created_by_recovering_missing_text_is_still_caught`,
|
||||||
|
`tests/test_magazine_routes.py::IngestSyncDuplicateRecheckTests`. **Still open:** re-run D→E with both
|
||||||
|
fixes in place and see whether the gap to the backup narrows; the remaining worst-page cases not yet
|
||||||
|
root-caused (e.g. page 44's screenshot-in-article OCR) need a closer look before deciding whether they are
|
||||||
|
general bugs or page-specific noise not worth a rule for.
|
||||||
|
|
||||||
### Voice Studio (github.com/debpalash/voicestudio) — evaluation, container, comparison
|
### Voice Studio (github.com/debpalash/voicestudio) — evaluation, container, comparison
|
||||||
|
|
||||||
|
|||||||
@ -363,6 +363,13 @@ def _ingest_sync(doc_id: str, job_id: str, ocr_lang: str, render_dpi: int,
|
|||||||
layout, _, _ = _fix_page_geometry(d, layout, [], page_meta["page"], _load_settings())
|
layout, _, _ = _fix_page_geometry(d, layout, [], page_meta["page"], _load_settings())
|
||||||
except Exception: # noqa: BLE001 - never fail an import over a box fix
|
except Exception: # noqa: BLE001 - never fail an import over a box fix
|
||||||
logging.exception("Box fixes failed for page %s", page_meta.get("page"))
|
logging.exception("Box fixes failed for page %s", page_meta.get("page"))
|
||||||
|
# The mend/reread/grow steps inside _fix_page_geometry can merge a wider box out of
|
||||||
|
# smaller fragments (e.g. an infographic's caption absorbed into one wide text block)
|
||||||
|
# -- after the run above already found no duplicates. Re-running the duplicate check
|
||||||
|
# now, against the geometry it actually produced, is what catches those (confirmed live
|
||||||
|
# on c't KI Wissen 2026 page 152: an infographic caption fragment was narrated twice
|
||||||
|
# because the one-shot check above ran before the wide block existed).
|
||||||
|
layout, _ = mag.clean_up_regions(layout)
|
||||||
mag.save_layout(d, layout)
|
mag.save_layout(d, layout)
|
||||||
|
|
||||||
background_job_update(job_id, stage="articles", detail="Detecting articles")
|
background_job_update(job_id, stage="articles", detail="Detecting articles")
|
||||||
@ -1201,6 +1208,11 @@ def _quality_run_sync(doc_id: str, job_id: str, settings: dict[str, Any]) -> Non
|
|||||||
outcome = spelling_agent_pass(d, layout, settings, page=page, language=issue_language)
|
outcome = spelling_agent_pass(d, layout, settings, page=page, language=issue_language)
|
||||||
layout, spell_fixed = outcome["layout"], outcome["fixed"]
|
layout, spell_fixed = outcome["layout"], outcome["fixed"]
|
||||||
spell_left = sorted({i["token"] for i in _spelling_findings(d, layout, page=page, language=issue_language)})
|
spell_left = sorted({i["token"] for i in _spelling_findings(d, layout, page=page, language=issue_language)})
|
||||||
|
# recover_missing_text (and the geometry fix above) can grow or add a box whose
|
||||||
|
# text now duplicates a smaller box's -- neither step re-checks for that itself.
|
||||||
|
# Catching it here, right before the audit reads the final boxes, is what finds
|
||||||
|
# it (see the ingest-time call for the case this closes).
|
||||||
|
layout, _ = mag.clean_up_regions(layout, pages=[page])
|
||||||
step("measuring box coverage")
|
step("measuring box coverage")
|
||||||
row = mag.audit_page_coverage(d, layout, page)
|
row = mag.audit_page_coverage(d, layout, page)
|
||||||
except Exception as exc: # noqa: BLE001 - one bad page must not stop the run
|
except Exception as exc: # noqa: BLE001 - one bad page must not stop the run
|
||||||
|
|||||||
@ -1490,6 +1490,42 @@ class IngestSyncVisualRefreshTests(unittest.TestCase):
|
|||||||
shutil.rmtree(lib, ignore_errors=True)
|
shutil.rmtree(lib, ignore_errors=True)
|
||||||
|
|
||||||
|
|
||||||
|
class IngestSyncDuplicateRecheckTests(unittest.TestCase):
|
||||||
|
"""The per-page geometry loop (seam mending, region growth) can merge a wider box out of
|
||||||
|
fragments clean_up_regions's one earlier call had no reason to flag yet -- confirmed live
|
||||||
|
on c't KI Wissen 2026 page 152, where an infographic caption fragment was left narrated
|
||||||
|
twice this way. Confirms clean_up_regions runs again after that loop, not only before it."""
|
||||||
|
|
||||||
|
def test_clean_up_regions_runs_again_after_the_geometry_loop(self):
|
||||||
|
from unittest.mock import patch, call
|
||||||
|
import routes.magazine as rm
|
||||||
|
with patch.object(rm, "_LIB", Path(tempfile.mkdtemp(prefix="ttvc-lib-"))) as lib:
|
||||||
|
d = lib / "doc1"
|
||||||
|
d.mkdir(parents=True)
|
||||||
|
(d / "source.pdf").write_bytes(b"x")
|
||||||
|
layout = {"pages": [{"page": 1, "width": 100, "height": 100}], "blocks": [], "images": []}
|
||||||
|
|
||||||
|
def semantic(_layout, _settings, page=None, doc_dir=None, on_page=None):
|
||||||
|
on_page(1)
|
||||||
|
|
||||||
|
with patch.object(rm.mag, "extract_pages", return_value=layout), \
|
||||||
|
patch.object(rm, "_recognise_advertisements"), \
|
||||||
|
patch.object(rm, "_recognise_semantic_layout", side_effect=semantic), \
|
||||||
|
patch.object(rm.mag, "refresh_visual_regions"), \
|
||||||
|
patch.object(rm.mag, "clean_up_regions", return_value=(layout, {})) as cleanup, \
|
||||||
|
patch.object(rm, "_fix_page_geometry", return_value=(layout, {}, False)) as geometry, \
|
||||||
|
patch.object(rm.mag, "save_layout"), \
|
||||||
|
patch.object(rm.mag, "build_articles", return_value=[]), \
|
||||||
|
patch.object(rm.mag, "save_articles"), \
|
||||||
|
patch.object(rm, "background_job_update"), \
|
||||||
|
patch.object(rm, "_write_meta"):
|
||||||
|
rm._ingest_sync("doc1", "job1", "eng", 150, None, None)
|
||||||
|
# Once before the geometry loop, once after it -- geometry itself ran in between.
|
||||||
|
self.assertEqual(cleanup.call_args_list, [call(layout), call(layout)])
|
||||||
|
geometry.assert_called_once()
|
||||||
|
shutil.rmtree(lib, ignore_errors=True)
|
||||||
|
|
||||||
|
|
||||||
class InterruptedIngestTests(unittest.TestCase):
|
class InterruptedIngestTests(unittest.TestCase):
|
||||||
def test_an_ingest_killed_by_a_restart_is_reported_as_failed_not_running_for_ever(self):
|
def test_an_ingest_killed_by_a_restart_is_reported_as_failed_not_running_for_ever(self):
|
||||||
import asyncio
|
import asyncio
|
||||||
@ -1562,6 +1598,47 @@ class QualityRunJobTests(unittest.TestCase):
|
|||||||
self.assertFalse(done['result']['verified_with_vision'])
|
self.assertFalse(done['result']['verified_with_vision'])
|
||||||
self.assertIn('skipped', done['detail'])
|
self.assertIn('skipped', done['detail'])
|
||||||
|
|
||||||
|
def test_a_duplicate_created_by_recovering_missing_text_is_still_caught(self):
|
||||||
|
"""clean_up_regions runs at the start of _fix_page_geometry -- but a later step, in the
|
||||||
|
same function or in recover_missing_text right after it, can still grow or add a box
|
||||||
|
whose text now duplicates another's (confirmed live on c't KI Wissen 2026 page 152: an
|
||||||
|
infographic caption fragment was left narrated twice this way). _fix_page_geometry is
|
||||||
|
stubbed out here so the test isolates the specific gap the fix closes: a duplicate that
|
||||||
|
appears only once recover_missing_text has run, after _fix_page_geometry's own check."""
|
||||||
|
from unittest.mock import patch
|
||||||
|
from routes.magazine import _quality_run_sync
|
||||||
|
text = "Die Plattformen bilden das Zentrum der Kreisläufe."
|
||||||
|
big = {'id': 'wide', 'page': 1, 'type': 'body', 'bbox': [0, 0, 500, 100],
|
||||||
|
'text': "Intro. More text after it.", 'reading_order': 0} # not yet a duplicate
|
||||||
|
small = {'id': 'frag', 'page': 1, 'type': 'body', 'bbox': [10, 10, 200, 20],
|
||||||
|
'text': text, 'reading_order': 1}
|
||||||
|
before = {'pages': [{'page': 1}], 'blocks': [big, small]}
|
||||||
|
grown = {'pages': [{'page': 1}], 'blocks': [
|
||||||
|
{**big, 'text': f"Intro. {text} More text after it."}, small]} # now contains it
|
||||||
|
seen_types: list[str] = []
|
||||||
|
|
||||||
|
def fake_audit(d, layout, page):
|
||||||
|
seen_types.append(next(b['type'] for b in layout['blocks'] if b['id'] == 'frag'))
|
||||||
|
return self._row(page)
|
||||||
|
|
||||||
|
lib = Path(tempfile.mkdtemp(prefix='ttvc-qrun-dup-'))
|
||||||
|
self.addCleanup(shutil.rmtree, lib, True)
|
||||||
|
(lib / ('a' * 32)).mkdir()
|
||||||
|
with patch('routes.magazine.mag.load_layout', return_value=before), \
|
||||||
|
patch('routes.magazine._fix_page_geometry', return_value=(before, self._geom_counts(), False)), \
|
||||||
|
patch('routes.magazine.mag.recover_missing_text', return_value=(grown, 1)), \
|
||||||
|
patch('routes.magazine.mag.audit_page_coverage', side_effect=fake_audit), \
|
||||||
|
patch('routes.magazine.mag.save_layout'), \
|
||||||
|
patch('routes.magazine._LIB', lib), \
|
||||||
|
patch('routes.magazine.background_job_update'):
|
||||||
|
_quality_run_sync('a' * 32, 'job', {})
|
||||||
|
self.assertEqual(seen_types, ['furniture'])
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _geom_counts():
|
||||||
|
return {k: 0 for k in ('boxes_fitted', 'paragraphs_cut', 'pullquotes_found',
|
||||||
|
'pictures_added', 'overlaps_fixed', 'seams_mended')}
|
||||||
|
|
||||||
|
|
||||||
class PageAdvertisementRouteTests(unittest.IsolatedAsyncioTestCase):
|
class PageAdvertisementRouteTests(unittest.IsolatedAsyncioTestCase):
|
||||||
async def test_mark_and_unmark_round_trip_and_404(self):
|
async def test_mark_and_unmark_round_trip_and_404(self):
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user