diff --git a/docs/tickets/README.md b/docs/tickets/README.md index 5e51c22..1f038f7 100644 --- a/docs/tickets/README.md +++ b/docs/tickets/README.md @@ -81,9 +81,9 @@ Open bugs and improvements for figmark, numbered `T-NNN`. | [T-075](T-075-truncated-descriptions-shared-as-complete.md) | **Closed** | Low | A truncated figure description is stored in the shared cross-request cache as if complete | | [T-076](T-076-cache-operational-envelope-unenforced.md) | **Closed** | Medium | The cache's operational envelope is unenforced — no schema version, disk use beyond the cap, world-readable directory, undocumented scaling assumptions | | [T-077](T-077-doc-drift-guard.md) | **Closed** | Medium | Hand-maintained docs drift from code — no guard catches a stale index, module map, or metadata | -| [T-078](T-078-vector-bar-charts-dropped.md) | Open | High | Vector bar and combo charts are silently dropped by the diagram detector (line charts only) | +| [T-078](T-078-vector-bar-charts-dropped.md) | **Closed** | High | Vector bar and combo charts are silently dropped by the diagram detector — resolved by T-080, verified live on FSR 28/28 | | [T-079](T-079-raster-charts-get-alt-text-prompt.md) | Open | Medium | Charts embedded as raster images are described with the alt-text prompt, losing most of their information | -| [T-080](T-080-capture-regions-stop-classifying.md) | Open | High | figmark misses figures because the code tries to guess which regions are charts | +| [T-080](T-080-capture-regions-stop-classifying.md) | **Closed** | High | figmark misses figures because the code tries to guess which regions are charts — one-box-per-page, coverage 58/79→100 % | | [T-081](T-081-structured-describe-output.md) | Open | High | The skip/keep decision rides on a free-text "[SKIP]" string the model can fail to emit | | [T-082](T-082-description-quality-judge.md) | Open | Medium | No way to measure figure-description quality, only whether a figure was captured | | [T-083](T-083-image-cap-should-be-config-model-validated.md) | Open | Medium | The image payload cap (1500 px / 500 KB) is a hardcoded guess, not validated against the model | diff --git a/docs/tickets/T-078-vector-bar-charts-dropped.md b/docs/tickets/T-078-vector-bar-charts-dropped.md index 6d686d1..a7c6327 100644 --- a/docs/tickets/T-078-vector-bar-charts-dropped.md +++ b/docs/tickets/T-078-vector-bar-charts-dropped.md @@ -1,6 +1,6 @@ # T-078: Vector bar and combo charts are silently dropped by the diagram detector (line charts only) -**Status:** Open +**Status:** Closed — resolved by T-080's one-box-per-page extraction; resolution below. **Priority:** High — figure interpretation is figmark's differentiator, and this silently loses a whole class of charts. A dropped bar chart leaves only its caption text; the data is gone, with no warning — the exact "silent degradation" @@ -81,3 +81,31 @@ rescuing it. not mis-detected as bar regions) — measured, not assumed. - A dropped chart is never silent: at minimum a loud warning when a captioned chart has no captured region. + +## Resolution (2026-07-22, via T-080) + +The root cause was the chart-classification gates themselves, and T-080 removed +them entirely: the detector no longer judges what geometry "looks like a chart" +— it unions each page's vector content into one box per band and lets the +vision model decide. There is no line-vs-bar distinction left to fail. + +**Verified live** (Riksbank FSR 2025:1, gemma-4-31B via Berget, 2026-07-22): +all three repro charts are captured *and described correctly, in Swedish*: + +| Chart | Page | Outcome | +|---|---|---| +| Diagram 10 — Svensk varuexport (stacked bar) | 27 | captured; export split per varugrupp described | +| Diagram 20 — Likviditetskvoter (grouped bar) | 37 | captured; LCR/NSFR bars described | +| Diagram 24 — Nettofondflöden (bar + line, dual axis) | 41 | captured; both axes and the time span described | + +Coverage on the full document: **28/28 captions (100 %)**, no misses; cost +0.019 EUR for the full conversion. The controlled contrast is closed: bar +charts are now handled identically however the publisher embedded them. + +**On the "loud gap warning" criterion (option 3):** not implemented, by +decision. The failure mode it guarded against — a classification gate silently +rejecting a chart — no longer exists, coverage is measured at 100 % across six +corpus documents (`scripts/coverage_bench/BASELINE.md`), and implementing the +warning would require caption parsing, which T-080 deliberately removed from +the extraction path. The coverage bench is the ongoing guard: run it when the +extraction changes. diff --git a/docs/tickets/T-080-capture-regions-stop-classifying.md b/docs/tickets/T-080-capture-regions-stop-classifying.md index 3f09334..05bc439 100644 --- a/docs/tickets/T-080-capture-regions-stop-classifying.md +++ b/docs/tickets/T-080-capture-regions-stop-classifying.md @@ -1,9 +1,9 @@ # T-080: figmark misses figures because the code tries to guess which regions are charts -**Status:** Open — refactor validated on branch `feat/simpler-extraction-t080` -(commit saved), **blocked on T-081** before merge. +**Status:** Closed — one-box-per-page shipped (with T-081's structured skip in +place). Resolution below. **Priority:** High — figure interpretation is the product's differentiator, and -the coverage bench shows we silently drop ~1 in 4 figures. +the coverage bench showed we silently dropped ~1 in 4 figures. ## Symptom @@ -82,3 +82,39 @@ That is why this is blocked on **T-081** (structured describe) before merge. - `diagrams.py` is *simpler* (fewer constants, no `solid`/classification code), not merely detuned. - Cost increase measured and documented. + +## Resolution (2026-07-22) + +Shipped as **one box per page-band** (Option 3), with the open split question +resolved by bench: + +- **Banding:** a page's filtered drawings group into maximal vertically + overlapping runs; adjacent bands merge unless a **body paragraph** sits in + the gap. Captions/source lines between two charts do not split — they belong + to the figure and land inside the rendered box. +- **Paragraph vs furniture** (the open question): word counts alone do NOT + separate them — BIS panel-title rows run 15–44 words. Bench-validated rule: + ≥ 20 words AND ≥ 2 lines AND sentence punctuation (`.`/`,` — tick-label runs + like "RO CL MX …" never have it; junk leakage 107→0 words on BIS). +- **Paragraph guard (new, found by the bench):** a one-box band can + legitimately cover flowing text (column layouts, bridged bands) — measured + 29/156/538 swallowed paragraph-words on BoC/BoJ/BIS even with per-band + boxes. `text_block_in_region` therefore never claims a body paragraph: mild + duplication beats silent deletion. Rescued prose: 831/2016/9350 words on + BoC/BoJ/BIS, junk 0. +- **Page furniture:** a drawing spanning ≥ 90 % of a page dimension (margin + rules, borders) is filtered — one 5 px margin bar otherwise bridged every + band on a table page into a full-page box. +- **Tables:** drawings the table path owns are excluded *before* banding. A + ruled table `find_tables` misses (the report's p70) is now captured for the + model instead of dropped — it addresses the T-050 symptom instead of + flattening to loose text. + +**Geometric caption coverage** (extraction-side, `scripts/coverage_bench` +ground truth): BoC **58→100 %**, BoJ **79→100 %**, BIS/Fed/Riksbank PPR/ +Riksbank FSR **100 %** — nothing missed on six docs. Region counts bounded: +35/54/81/64/44/28 — aggregate **301→284 vs main over seven docs** (BIS halves +as panels merge; BoC/Fed roughly double, exactly where main dropped figures). +T-078's three dropped FSR bar/combo charts (p27/37/41) all capture. + +Live before→after numbers recorded in `scripts/coverage_bench/BASELINE.md`. diff --git a/scripts/coverage_bench/BASELINE.md b/scripts/coverage_bench/BASELINE.md index 77abc8a..078ee91 100644 --- a/scripts/coverage_bench/BASELINE.md +++ b/scripts/coverage_bench/BASELINE.md @@ -53,3 +53,32 @@ produced 41 placements that dedupe to 6; one repeated header is correctly skipped). The docx's open questions are *quality* (two logos over-described), not coverage — so it belongs to the LLM-judge track, but it now has a real coverage number instead of N/A. + +## After T-080 — one-box-per-page extraction (2026-07-22, gemma-4-31B via Berget) + +Same yardstick, same documents, live end-to-end runs with the T-080 band +extraction + T-081 structured skip: + +| Document | Mode | Ground truth | Covered | Coverage | Captured | Skipped by model | +|---|---|---|---|---|---|---| +| boc-mpr-202410.pdf | caption | 26 captions | 26 | **100 %** (was 58 %) | 32 figs | 3 (cover/decoration) | +| boj-outlook-2410.pdf | caption | 58 captions | 58 | **100 %** (was 79 %) | 55 figs | 4 | +| riksbank-fsr-202505.pdf | caption | 28 captions | 28 | **100 %** (3 charts dropped before, T-078) | 38 figs | — | +| **Aggregate (caption)** | | **112** | **112** | **100 %** | | | + +**Missed figures: none.** + +**Cost** (per full conversion, live): BoC 37 calls / 46.8k tokens ≈ 0.016 EUR, +BoJ 61 calls / 86.1k tokens ≈ 0.030 EUR, FSR 40 calls / 56.7k ≈ 0.019 EUR. Aggregate regions across seven corpus +docs are 301→284 vs main (BIS AR halves as multi-panel figures merge into one +box; BoC/Fed roughly double — exactly where main dropped 1 in 4 figures), so +capture-100 % costs about the same as the old gated detector overall. + +**Skip behaviour (T-081):** over-captured non-figures (cover decorations, +title-page rules) return `is_figure=false` and are cleanly skipped — 3/35 on +BoC, 4/59 on BoJ, no junk in the Markdown. + +Geometric (extraction-side) coverage is also 100 % on bis-ar-2024, +fed-mpr-202407 and riksbank-ppr-202503. The three FSR bar/combo charts T-078 +reported dropped (pages 27/37/41) are captured *and described correctly* in +the live FSR run — T-078 closes with this baseline. diff --git a/src/figmark/diagrams.py b/src/figmark/diagrams.py index 48a1b9e..7046fef 100644 --- a/src/figmark/diagrams.py +++ b/src/figmark/diagrams.py @@ -1,9 +1,13 @@ -"""Diagram extraction via PyMuPDF drawing clustering. +"""Diagram extraction: one box per page-band of vector content (T-080). Central-bank and similar agency reports embed vector charts (matplotlib exported -to PDF path commands) that page.get_images() does not catch. This module finds -diagram regions by clustering drawings spatially and expands the bbox to capture -axis titles and source lines. +to PDF path commands) that page.get_images() does not catch. Earlier versions +tried to *classify* which drawing clusters are charts (solid-fill counts, cluster +gates) and silently dropped ~1 in 4 figures. The premise was wrong: only a vision +model can tell what a region is. So this module now unions a page's vector +content into one box per vertical band, splitting bands only where a body-text +paragraph sits between them, and lets the model decide what each box is +(T-081's structured is_figure decision skips the non-figures). """ from __future__ import annotations @@ -27,29 +31,43 @@ # report (vector charts from matplotlib). # ============================================================================ -# Pages with fewer drawings than this are not examined (text-heavy pages) -MIN_DRAWINGS_PER_PAGE = 30 +# Cheap early-exit for pure-text pages only. Kept deliberately low: the code does +# NOT judge whether a region is a chart — it captures any drawing cluster and lets +# the vision model + significance gate decide (T-080). A real figure page has far +# more than this; the size/cluster floors below do the actual noise filtering. +MIN_DRAWINGS_PER_PAGE = 5 # Drawings below this pixel size in BOTH dimensions do not count (specks). # A drawing below it in exactly ONE dimension is an axis-aligned hairline — # LibreOffice/TikZ draw chart axes and gridlines as zero-thickness strokes -# (T-055). Lines join clusters (they carry the chart's connectivity) but do -# not gate them: see MIN_SOLID_DRAWINGS_PER_CLUSTER. +# (T-055). These count as ordinary cluster members: line charts are mostly +# hairlines, and the code no longer tries to tell a chart from a grid (T-080). MIN_DRAWING_DIM = 2 # Drawings larger than X% of the page area are skipped (background boxes) MAX_DRAWING_AREA_RATIO = 0.4 -# Drawings within this pixel distance are joined into the same cluster -MERGE_DISTANCE = 3 -# A cluster must contain at least this many drawings -MIN_DRAWINGS_PER_CLUSTER = 8 -# ... of which at least this many must be non-line (solid) members. Bar/pie/ -# area charts pass (bars, wedges, legend chips are fills); a pure ruled grid -# (only hairlines) fails. Bench-derived (T-055): the smallest corpus positive, -# a 4-bar LO chart, has exactly 4 solids; line-only "charts" (an axis frame -# with no data marks) are indistinguishable from ruled grids and stay out. -MIN_SOLID_DRAWINGS_PER_CLUSTER = 4 -# A candidate region is NOT a chart if it substantially overlaps a table-like -# find_tables candidate — ruled/zebra slide tables cluster exactly like charts -# (row fills = solids), and T-031's table path owns them. "Table-like" is +# A drawing spanning nearly a full page dimension is page furniture (margin +# rules, section frames, header-to-footer borders), not figure content — and +# because banding is vertical-overlap based, one full-height margin rule would +# bridge every band on the page into a single full-page box (seen on ruled +# table pages: a 5 px margin bar unioned the page). No corpus figure spans +# edge to edge; coverage stays 100 % on the T-080 bench with this filter. +MAX_DRAWING_SPAN_RATIO = 0.9 +# A text block is a *body paragraph* (not figure furniture — captions, source +# lines, axis notes) at this many words and lines. Bench on BoC MPR 2024-10 + +# BoJ Outlook 2024-10 + BIS AR 2024 (T-080): furniture and body word counts +# overlap heavily (BIS panel-title rows run 15–44 words), so no clean threshold +# exists — 20 words / 2 lines is the point where caption coverage stays 100 % +# on all three docs while region counts stay bounded (34/56/81). Used for two +# decisions: a paragraph between two visual bands splits them, and a paragraph +# inside a region is never suppressed from the body text (see +# ``text_block_in_region``). +PARA_MIN_WORDS = 20 +PARA_MIN_LINES = 2 +# A gap block must lie at least this fraction inside the gap to count as +# *between* the bands (rather than belonging to one of them). +GAP_CONTAINMENT = 0.7 +# A drawing mostly inside a table-like find_tables candidate is the table +# path's content, not visual-band material — ruled/zebra tables draw row fills +# exactly like chart bars, and T-031's table path owns them. "Table-like" is # deliberately laxer than tables.py's keep gates (suppression needs less # evidence than emission): >= 3 rows, >= 2 cols, and a cell fill ratio the # bench separated cleanly (charts' grid-junk candidates measured <= 36% @@ -69,12 +87,11 @@ # 0 sloped members too — no such page exists in the corpus (bar-chart pages # produce no table candidate), noted here for honesty. TABLE_SUPPRESS_MAX_SLOPED = 3 -# Clusters must be at least this large (px) +# Size floor: a band smaller than this (px) is stray marks (rules, decorations), +# not a renderable figure. MIN_CLUSTER_WIDTH = 80 MIN_CLUSTER_HEIGHT = 60 -# Internal y-gaps larger than this split the cluster (stacked charts) -INTERNAL_Y_GAP_SPLIT = 40 -# Extra px around the clustering bbox before text expansion +# Extra px around the band bbox before text expansion PADDING = 6 # Neighbouring text blocks (axis title/source line) within this many px are included TEXT_EXPAND_DISTANCE = 30 @@ -95,87 +112,72 @@ class DiagramRegion: path: Path | None = None # filled in when the image is saved -def _close(r1: fitz.Rect, r2: fitz.Rect, slack: float) -> bool: - return not ( - r1.x1 + slack < r2.x0 - or r2.x1 + slack < r1.x0 - or r1.y1 + slack < r2.y0 - or r2.y1 + slack < r1.y0 - ) +def _is_paragraph(text: str, n_lines: int | None = None) -> bool: + """Body paragraph vs figure furniture (caption/source line/axis note) — + the bench-validated word/line floor (see PARA_MIN_WORDS). Prose always + carries sentence punctuation; tick-label runs ("RO CL MX …", "23 22 21 …") + can exceed the word floor but never do — measured junk leakage drops from + 107 to ~0 words on BIS AR 2024 with this test. ``n_lines`` is checked only + when the caller has it (page dicts do; pipeline TextBlocks don't).""" + if len(text.split()) < PARA_MIN_WORDS: + return False + if n_lines is not None and n_lines < PARA_MIN_LINES: + return False + return "." in text or "," in text -def _cluster_rects(rects: list[fitz.Rect], merge_distance: float) -> list[list[int]]: - """Union-find clustering of rectangles by spatial proximity. +def _page_text_blocks(page: fitz.Page) -> list[tuple[fitz.Rect, str, int]]: + """(bbox, text, line count) for every non-empty text block on the page.""" + out: list[tuple[fitz.Rect, str, int]] = [] + for block in page.get_text("dict").get("blocks", []): + if block.get("type") != 0: + continue + tb = fitz.Rect(block.get("bbox", (0, 0, 0, 0))) + if tb.width <= 0 or tb.height <= 0: + continue + text = " ".join( + span["text"] for line in block.get("lines", []) for span in line.get("spans", []) + ).strip() + if text: + out.append((tb, text, len(block.get("lines", [])))) + return out + - Connectivity is the ``_close`` relation (bboxes within ``merge_distance``). - Rather than test all O(n²) pairs, sweep left to right by ``x0`` keeping an - "active" set of rects whose right edge is still within reach; a rect can only - be close to an active one. This yields the *identical* connected components as - the brute-force pairing (a pair pruned by the sweep can never satisfy - ``_close``), but is near-linear when drawings are spatially spread — the common - case on a chart page — instead of quadratic. (T-037) +def _vertical_bands(rects: list[fitz.Rect]) -> list[list[fitz.Rect]]: + """Maximal runs of vertically overlapping/touching drawings. + + No gap threshold: any vertical whitespace is a *potential* split point, but + a split only happens when a body paragraph sits in the gap (see caller). """ - n = len(rects) - parent = list(range(n)) - - def find(x: int) -> int: - while parent[x] != x: - parent[x] = parent[parent[x]] - x = parent[x] - return x - - def union(x: int, y: int) -> None: - px, py = find(x), find(y) - if px != py: - parent[px] = py - - order = sorted(range(n), key=lambda i: rects[i].x0) - active: list[int] = [] # indices whose right edge may still reach a later rect - for idx in order: - r = rects[idx] - still_active: list[int] = [] - for j in active: - # Once a rect's right edge + slack is left of r.x0, it (and every later - # rect, all with larger x0) can never be close again — drop it. - if rects[j].x1 + merge_distance < r.x0: - continue - still_active.append(j) - if _close(rects[j], r, merge_distance): - union(j, idx) - still_active.append(idx) - active = still_active + by_y = sorted(rects, key=lambda r: r.y0) + bands: list[list[fitz.Rect]] = [[by_y[0]]] + y_max = by_y[0].y1 + for r in by_y[1:]: + if r.y0 <= y_max: + bands[-1].append(r) + else: + bands.append([r]) + y_max = max(y_max, r.y1) + return bands - groups: dict[int, list[int]] = {} - for i in range(n): - groups.setdefault(find(i), []).append(i) - return list(groups.values()) +def _union(rects: list[fitz.Rect]) -> fitz.Rect: + box = fitz.Rect(rects[0]) + for r in rects[1:]: + box |= r + return box -def _split_by_y_gap( - group_rects: list[tuple[fitz.Rect, bool]], min_gap: float -) -> list[list[tuple[fitz.Rect, bool]]]: - """Split a cluster if it has an internal y-gap larger than min_gap. - Items are ``(rect, is_solid)`` pairs so the solid count survives the split. - """ - if len(group_rects) < 2: - return [group_rects] - by_y = sorted(group_rects, key=lambda item: item[0].y0) - splits: list[list[tuple[fitz.Rect, bool]]] = [] - current = [by_y[0]] - current_max_y = by_y[0][0].y1 - for item in by_y[1:]: - r = item[0] - gap = r.y0 - current_max_y - if gap > min_gap: - splits.append(current) - current = [item] - current_max_y = r.y1 - else: - current.append(item) - current_max_y = max(current_max_y, r.y1) - splits.append(current) - return splits +def _paragraph_in_gap( + blocks: list[tuple[fitz.Rect, str, int]], y_top: float, y_bot: float +) -> bool: + """True if a body paragraph lies (mostly) between y_top and y_bot — + the signal that two visual bands are separate figures, not one.""" + for tb, text, n_lines in blocks: + inside = min(tb.y1, y_bot) - max(tb.y0, y_top) + if inside / tb.height >= GAP_CONTAINMENT and _is_paragraph(text, n_lines): + return True + return False def _has_sloped_content(drawing: dict) -> bool: @@ -251,10 +253,13 @@ def _expand_with_neighboring_text( def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: - """Find diagram regions on a page. + """Find visual regions on a page — one box per band of vector content. - Pipeline: filter drawings → cluster spatially → split on internal y-gap → - expand bbox with neighbouring text → return sorted in reading order. + Pipeline: filter drawings (specks, background) → drop table-owned drawings → + group into vertical bands, splitting only where a body paragraph sits between + two bands → union each band, expand with neighbouring text → return sorted in + reading order. No chart/not-chart judgement happens here: the vision model + decides what each box is (T-080/T-081). """ drawings = page.get_drawings() if len(drawings) < MIN_DRAWINGS_PER_PAGE: @@ -264,7 +269,6 @@ def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: max_drawing_area = MAX_DRAWING_AREA_RATIO * page_area rects: list[fitz.Rect] = [] - solid: list[bool] = [] sloped: list[bool] = [] for d in drawings: r = d.get("rect") @@ -276,53 +280,88 @@ def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: continue # speck/decoration if r.width * r.height > max_drawing_area: continue # page background/frame - # Axis-aligned hairlines (LO/TikZ axes, gridlines) join clusters but - # don't gate them (T-055). + if ( + r.height >= MAX_DRAWING_SPAN_RATIO * page.rect.height + or r.width >= MAX_DRAWING_SPAN_RATIO * page.rect.width + ): + continue # page furniture (margin rule/border) — would bridge all bands + # Axis-aligned hairlines (LO/TikZ axes, gridlines) are ordinary band + # members (T-055). rects.append(fitz.Rect(r)) - solid.append(not (thin_w or thin_h)) sloped.append(_has_sloped_content(d)) if not rects: return [] - groups = _cluster_rects(rects, MERGE_DISTANCE) + # A drawing on a real data table is the table path's business (T-031/T-055) + # — zebra row fills draw exactly like chart bars, and describing a table as + # a picture double-represents it. Dropped *before* banding so table rows + # cannot bridge two genuine figures into one band. + suppressors = [] + for s in _table_like_rects(page): + sloped_inside = sum( + 1 + for i, rect in enumerate(rects) + if sloped[i] and (rect & s).get_area() >= 0.5 * max(rect.get_area(), 1e-9) + ) + if sloped_inside < TABLE_SUPPRESS_MAX_SLOPED: + suppressors.append(s) + if suppressors: + before = len(rects) + rects = [ + r + for r in rects + if not any( + (r & s).get_area() >= TABLE_SUPPRESS_MIN_OVERLAP * max(r.get_area(), 1e-9) + for s in suppressors + ) + ] + if len(rects) < before: + logger.info( + "page %d: %d drawing(s) on a data table left to the table path", + page_num, + before - len(rects), + ) + if not rects: + return [] + + # Band the remaining drawings vertically; merge adjacent bands unless a body + # paragraph sits in the gap — the only non-arbitrary "these are separate + # figures" signal (T-080). Captions/source lines between two charts do NOT + # split: they belong to the figure and land inside the rendered box. + text_blocks = _page_text_blocks(page) + bands = _vertical_bands(rects) + merged: list[list[fitz.Rect]] = [bands[0]] + for band in bands[1:]: + gap_top = _union(merged[-1]).y1 + gap_bot = _union(band).y0 + if _paragraph_in_gap(text_blocks, gap_top, gap_bot): + merged.append(band) + else: + merged[-1].extend(band) raw_bboxes: list[tuple[fitz.Rect, int]] = [] - for group in groups: - if len(group) < MIN_DRAWINGS_PER_CLUSTER: - continue - cluster_items = [(rects[i], solid[i]) for i in group] - for sub_items in _split_by_y_gap(cluster_items, INTERNAL_Y_GAP_SPLIT): - if len(sub_items) < MIN_DRAWINGS_PER_CLUSTER: - continue - if sum(1 for _, s in sub_items if s) < MIN_SOLID_DRAWINGS_PER_CLUSTER: - continue # hairlines only ≈ ruled grid, not a chart - sub = [r for r, _ in sub_items] - x0 = min(r.x0 for r in sub) - y0 = min(r.y0 for r in sub) - x1 = max(r.x1 for r in sub) - y1 = max(r.y1 for r in sub) - if x1 - x0 < MIN_CLUSTER_WIDTH or y1 - y0 < MIN_CLUSTER_HEIGHT: - continue - bbox = fitz.Rect( - max(0, x0 - PADDING), - max(0, y0 - PADDING), - min(page.rect.width, x1 + PADDING), - min(page.rect.height, y1 + PADDING), - ) - raw_bboxes.append((bbox, len(sub))) + for band in merged: + box = _union(band) + if box.width < MIN_CLUSTER_WIDTH or box.height < MIN_CLUSTER_HEIGHT: + continue # stray marks (rules, decorations) — below the size floor + bbox = fitz.Rect( + max(0, box.x0 - PADDING), + max(0, box.y0 - PADDING), + min(page.rect.width, box.x1 + PADDING), + min(page.rect.height, box.y1 + PADDING), + ) + raw_bboxes.append((bbox, len(band))) - raw_bboxes.sort(key=lambda b: b[0].y0) regions: list[DiagramRegion] = [] for i, (bbox, n_draw) in enumerate(raw_bboxes): + # Text expansion stops at the neighbouring band so two figures never + # merge through a shared caption. y_min_limit = 0.0 y_max_limit = page.rect.height for j, (other, _) in enumerate(raw_bboxes): if j == i: continue - horiz_overlap = min(bbox.x1, other.x1) - max(bbox.x0, other.x0) - if horiz_overlap < 0.3 * min(bbox.width, other.width): - continue if other.y1 <= bbox.y0: y_min_limit = max(y_min_limit, other.y1 + 4) elif other.y0 >= bbox.y1: @@ -332,9 +371,9 @@ def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: page, bbox, TEXT_EXPAND_DISTANCE, y_min_limit, y_max_limit ) expanded &= page.rect - # Clusters can lie (partly) outside the page — crop marks, spill-over + # Bands can lie (partly) outside the page — crop marks, spill-over # content with negative coordinates. After clipping to the page, a - # degenerate or tiny remnant is not a chart: rendering it crashes the + # degenerate or tiny remnant is not a figure: rendering it crashes the # PNG writer ("Invalid bandwriter header dimensions/setup"). if expanded.is_empty or expanded.width < 1 or expanded.height < 1: continue @@ -347,35 +386,6 @@ def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: ) ) - # A region sitting on a real data table is the table path's business - # (T-031/T-055) — zebra row fills cluster exactly like chart bars, and - # describing a table as a picture double-represents it. - if regions: - suppressors = [] - for s in _table_like_rects(page): - sloped_inside = sum( - 1 - for i, rect in enumerate(rects) - if sloped[i] and (rect & s).get_area() >= 0.5 * max(rect.get_area(), 1e-9) - ) - if sloped_inside < TABLE_SUPPRESS_MAX_SLOPED: - suppressors.append(s) - if suppressors: - kept: list[DiagramRegion] = [] - for r in regions: - region_rect = fitz.Rect(r.bbox) - area = region_rect.get_area() - overlap = max((region_rect & s).get_area() for s in suppressors) - if area > 0 and overlap / area >= TABLE_SUPPRESS_MIN_OVERLAP: - logger.info( - "page %d: diagram candidate %s suppressed — overlaps a data table", - page_num, - tuple(round(v) for v in r.bbox), - ) - continue - kept.append(r) - regions = kept - regions.sort(key=lambda r: (round(r.bbox[1] / 10), r.bbox[0])) for i, r in enumerate(regions, start=1): r.index = i @@ -390,11 +400,23 @@ def find_diagram_regions(page: fitz.Page, page_num: int) -> list[DiagramRegion]: def text_block_in_region( - bbox, regions: list[DiagramRegion], min_overlap: float = TEXT_IN_REGION_OVERLAP + bbox, + regions: list[DiagramRegion], + min_overlap: float = TEXT_IN_REGION_OVERLAP, + text: str | None = None, ) -> bool: """True if a text block lies mostly inside a diagram region — its content is the diagram's internal labels, which leak into the body otherwise. (T-008) + + A body *paragraph* is never claimed, even when fully inside a region: with + one-box-per-page bands (T-080), a box can legitimately cover flowing text + (column layouts, bridged bands — measured 29/156/538 swallowed paragraph-words + on BoC/BoJ/BIS even with per-band boxes). Keeping the paragraph in the body + costs mild duplication with the rendered image; deleting it silently loses + content — and figmark never silently degrades. """ + if text is not None and _is_paragraph(text): + return False r = fitz.Rect(bbox) area = abs(r.width * r.height) if area <= 0: diff --git a/src/figmark/pipeline.py b/src/figmark/pipeline.py index a687c9c..899c437 100644 --- a/src/figmark/pipeline.py +++ b/src/figmark/pipeline.py @@ -358,7 +358,10 @@ def emit_loud(msg: str) -> None: page_data.blocks = [ b for b in page_data.blocks - if not (isinstance(b, TextBlock) and text_block_in_region(b.bbox, regions)) + if not ( + isinstance(b, TextBlock) + and text_block_in_region(b.bbox, regions, text=b.text) + ) ] dropped = before - len(page_data.blocks) if dropped: diff --git a/tests/test_diagrams.py b/tests/test_diagrams.py index 66e5dc5..022740e 100644 --- a/tests/test_diagrams.py +++ b/tests/test_diagrams.py @@ -2,7 +2,6 @@ from __future__ import annotations -import random from pathlib import Path import fitz @@ -10,10 +9,7 @@ from figmark.diagrams import ( MIN_CLUSTER_HEIGHT, MIN_CLUSTER_WIDTH, - MIN_DRAWINGS_PER_CLUSTER, DiagramRegion, - _close, - _cluster_rects, find_diagram_regions, render_and_save_region, text_block_in_region, @@ -32,41 +28,78 @@ def test_text_block_in_region_drops_internal_keeps_adjacent(): assert text_block_in_region((100, 500, 400, 520), [region]) is False -def test_find_regions_in_report_known_diagram_page(report_pdf: Path): - """Page 11 of the monetary-policy report has two known charts side by side.""" +def test_paragraph_never_suppressed_even_inside_region(): + """T-080: a one-box band can legitimately cover flowing text (column + layouts, bridged bands). A body paragraph is kept in the body even when + fully inside a region — mild duplication beats silent deletion — while + short label text and punctuation-free tick-label runs are still claimed.""" + region = DiagramRegion(page_num=1, index=1, bbox=(100, 100, 400, 400), n_drawings=20) + inside = (150, 150, 350, 250) + paragraph = ( + "Global inflation continued to recede from the peak it reached in 2022, " + "and monetary policy stayed restrictive across most advanced economies." + ) + assert text_block_in_region(inside, [region], text=paragraph) is False + # A short axis label inside the region is still suppressed. + assert text_block_in_region(inside, [region], text="Index, 2019 = 100") is True + # A long punctuation-free tick-label run is furniture, not prose. + tick_run = "RO CL MX CZ IL ID BR PE IN CN NO AT US NZ NL IE DE GB PT CA DK CO" + assert text_block_in_region(inside, [region], text=tick_run) is True + + +def test_find_regions_splits_on_body_paragraph(report_pdf: Path): + """Page 11 has two charts with a body paragraph between them — the only + signal that splits two visual bands into separate regions (T-080).""" doc = fitz.open(report_pdf) try: page = doc.load_page(10) regions = find_diagram_regions(page, 11) - assert len(regions) == 2, f"Expected 2 diagrams on page 11, got {len(regions)}" + assert len(regions) == 2, f"Expected 2 regions on page 11, got {len(regions)}" + regions_sorted = sorted(regions, key=lambda r: r.bbox[1]) + assert regions_sorted[0].bbox[3] < regions_sorted[1].bbox[1] for r in regions: assert r.bbox[2] - r.bbox[0] >= MIN_CLUSTER_WIDTH assert r.bbox[3] - r.bbox[1] >= MIN_CLUSTER_HEIGHT - assert r.n_drawings >= MIN_DRAWINGS_PER_CLUSTER finally: doc.close() -def test_find_regions_splits_stacked_diagrams(report_pdf: Path): - """Page 68 has two charts stacked vertically.""" +def test_find_regions_merges_stacked_charts_without_paragraph_between(report_pdf: Path): + """Page 68 has two charts stacked vertically with only figure furniture + (captions/source lines) between them: one box — the model gets the whole + visual block with its shared context (T-080). Under the old detector this + page split in two; the merge is the intended design change.""" doc = fitz.open(report_pdf) try: page = doc.load_page(67) regions = find_diagram_regions(page, 68) - assert len(regions) == 2, f"Expected 2 stacked diagrams on page 68, got {len(regions)}" - regions_sorted = sorted(regions, key=lambda r: r.bbox[1]) - assert regions_sorted[0].bbox[3] < regions_sorted[1].bbox[1] + assert len(regions) == 1, f"Expected 1 merged region on page 68, got {len(regions)}" + # The box must span both charts, not just one. + assert regions[0].bbox[3] - regions[0].bbox[1] > 300 finally: doc.close() -def test_find_regions_skips_table_page(report_pdf: Path): - """Page 70 is a data table, not a chart — should yield 0 regions.""" +def test_find_regions_captures_table_the_table_path_misses(report_pdf: Path): + """Page 70 is a ruled data table that ``find_tables`` does not detect — the + table path emits nothing and the cells flatten to loose text (the T-050 + symptom). The band capture hands it to the vision model instead of dropping + it (T-080); tables the table path DOES own are excluded before banding. The + box must cover the table area, not the whole page (the full-height margin + rule is filtered as page furniture).""" doc = fitz.open(report_pdf) try: page = doc.load_page(69) + from figmark.tables import find_table_blocks + + assert find_table_blocks(page, 70) == [], ( + "find_tables now detects this table — restore the 0-region " + "expectation and let the table path own the page" + ) regions = find_diagram_regions(page, 70) - assert regions == [], f"A table page should yield no regions, got {len(regions)}" + assert len(regions) == 1, f"Expected 1 captured region on page 70, got {len(regions)}" + x0, y0, x1, y1 = regions[0].bbox + assert y1 - y0 < 0.8 * page.rect.height, "region must not swallow the full page" finally: doc.close() @@ -168,44 +201,41 @@ def create(model, max_tokens, messages, **kw): ) -def _rand_rect(rng: random.Random) -> fitz.Rect: - x = rng.uniform(0, 500) - y = rng.uniform(0, 500) - return fitz.Rect(x, y, x + rng.uniform(1, 60), y + rng.uniform(1, 60)) - - -def _bruteforce_partition(rects: list[fitz.Rect], slack: float) -> set[frozenset[int]]: - """All-pairs union-find — the reference the x-sweep must reproduce exactly.""" - n = len(rects) - parent = list(range(n)) - - def find(x: int) -> int: - while parent[x] != x: - parent[x] = parent[parent[x]] - x = parent[x] - return x - - for i in range(n): - for j in range(i + 1, n): - if _close(rects[i], rects[j], slack): - a, b = find(i), find(j) - if a != b: - parent[a] = b - groups: dict[int, list[int]] = {} - for i in range(n): - groups.setdefault(find(i), []).append(i) - return {frozenset(g) for g in groups.values()} - - -def test_cluster_rects_matches_bruteforce_partition(): - """T-037: the near-linear x-sweep clustering must yield the identical partition - as the original O(n²) all-pairs version, across many random layouts.""" - rng = random.Random(20260624) - for _ in range(25): - rects = [_rand_rect(rng) for _ in range(90)] - slack = rng.uniform(0, 12) - swept = {frozenset(g) for g in _cluster_rects(rects, slack)} - assert swept == _bruteforce_partition(rects, slack) +def _two_chart_page(gap_text: list[str]) -> fitz.Document: + """A synthetic page with two drawing groups and the given text lines in the + vertical gap between them.""" + doc = fitz.open() + page = doc.new_page(width=595, height=842) + for y0 in (100, 500): # two chart-like groups of stacked bars + for i in range(12): + y = y0 + i * 12 + page.draw_rect(fitz.Rect(100, y, 400, y + 8), fill=(0.5, 0.5, 0.9)) + for i, line in enumerate(gap_text): + page.insert_text((100, 320 + i * 14), line) + return doc + + +def test_body_paragraph_between_bands_splits_them(tmp_path: Path): + """T-080's split signal: a body paragraph between two visual bands means + they are separate figures; caption-like furniture (or nothing) means one.""" + para = [ + "Economic growth is forecast to pick up gradually over the projection", + "horizon, supported by lower interest rates and rising real incomes in", + "most households, while inflation returns towards the two per cent target.", + ] + doc = _two_chart_page(para) + try: + assert len(find_diagram_regions(doc[0], 1)) == 2 + finally: + doc.close() + + for furniture in ([], ["Chart 4: GDP growth and contributions"]): + doc = _two_chart_page(furniture) + try: + regions = find_diagram_regions(doc[0], 1) + assert len(regions) == 1, f"gap text {furniture!r} must not split" + finally: + doc.close() def test_render_region_produces_png(report_pdf: Path, tmp_path: Path): diff --git a/tests/test_office_extraction_quality.py b/tests/test_office_extraction_quality.py index 255b52c..50aef45 100644 --- a/tests/test_office_extraction_quality.py +++ b/tests/test_office_extraction_quality.py @@ -108,9 +108,12 @@ def test_docx_footnote_text_survives(run_office): def test_docx_pictures_counted_once_each(run_office): """poi-pictures: 2 raster images + 3 vector-rendered pictures (wmf/emf/pict - become drawing clusters → diagram regions) — 5 figures, 5 description calls, - none double-counted. (Originally asserted 2: a pipeline regression had - silently stopped scheduling diagram-description jobs; fixed with T-061.)""" + become drawings). With one-box banding (T-080) the 3 vector pictures sit in + one visual band — no body paragraph between them — so they render as ONE + region the model describes together: 3 figures, 3 description calls, none + dropped, none double-counted. (Originally asserted 2: a pipeline regression + had silently stopped scheduling diagram-description jobs; fixed with T-061. + Asserted 5 before T-080 merged the vector band.)""" result, client = run_office("poi-pictures.docx") - assert result.figure_count == 5 - assert len(client.describe_prompts) == 5 + assert result.figure_count == 3 + assert len(client.describe_prompts) == 3 diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 27622b7..3cd89c8 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -183,12 +183,13 @@ def test_pipeline_diagrams_extracted_from_report( diagram_files = sorted(diagrams_dir.iterdir()) if diagrams_dir.exists() else [] desc_files = sorted(diagram_desc_dir.iterdir()) if diagram_desc_dir.exists() else [] - # Page 11 = 2 diagrams, page 68 = 2 diagrams → 4 total - assert len(diagram_files) == 4, f"Expected 4 diagram images, got {len(diagram_files)}" - assert len(desc_files) == 4 + # Page 11 = 2 regions (body paragraph between the charts splits them); + # page 68 = 1 region (only captions between the stacked charts → one box, T-080) + assert len(diagram_files) == 3, f"Expected 3 diagram images, got {len(diagram_files)}" + assert len(desc_files) == 3 markdown = (pdf_out / f"{mini_path.stem}.md").read_text(encoding="utf-8") - assert markdown.count("](diagrams/") == 4 + assert markdown.count("](diagrams/") == 3 # Verify Swedish myndighetssvenska in each description. for desc_file in desc_files: @@ -207,7 +208,7 @@ def test_pipeline_determinism_workers_1_vs_4( ): """Output must be identical regardless of worker count (1 vs 4). - We run a mini PDF (2 pages, 4 diagrams) twice: once with max_workers=1, once + We run a mini PDF (2 pages, 3 diagram regions) twice: once with max_workers=1, once with max_workers=4. The Markdown output must be identical byte-for-byte. """ import fitz