diff --git a/document-parser/data/docling_studio.db b/document-parser/data/docling_studio.db index a4f5f99..17adb2b 100644 Binary files a/document-parser/data/docling_studio.db and b/document-parser/data/docling_studio.db differ diff --git a/document-parser/domain/bbox.py b/document-parser/domain/bbox.py index c36c590..41530a7 100644 --- a/document-parser/domain/bbox.py +++ b/document-parser/domain/bbox.py @@ -9,18 +9,42 @@ bboxes are normalized to TOPLEFT [left, top, right, bottom] before being sent to the frontend. """ +import logging + from docling_core.types.doc.base import BoundingBox +logger = logging.getLogger(__name__) + +# Sentinel value returned when a bbox is invalid or degenerate. +# A zero-area rect is safe: the frontend draws nothing and hit-testing ignores it. +EMPTY_BBOX: list[float] = [0.0, 0.0, 0.0, 0.0] + def to_topleft_list(bbox: BoundingBox, page_height: float) -> list[float]: """Convert a Docling BoundingBox to a [l, t, r, b] list in TOPLEFT origin. + Validates the result: left < right and top < bottom. If the bbox is + degenerate (zero or negative area), returns EMPTY_BBOX so the frontend + silently skips it instead of rendering a broken rectangle. + Args: bbox: Docling BoundingBox (any origin). page_height: Height of the page (needed for BOTTOMLEFT conversion). Returns: - [left, top, right, bottom] in TOPLEFT coordinates. + [left, top, right, bottom] in TOPLEFT coordinates, or EMPTY_BBOX + if the bbox is degenerate. """ normalized = bbox.to_top_left_origin(page_height) - return [normalized.l, normalized.t, normalized.r, normalized.b] + left, top, right, bottom = normalized.l, normalized.t, normalized.r, normalized.b + + # Degenerate bbox: zero or negative dimensions — skip silently. + # This can happen with corrupted PDFs or edge-case Docling outputs. + if right <= left or bottom <= top: + logger.debug( + "Degenerate bbox skipped: [%.1f, %.1f, %.1f, %.1f] (page_height=%.1f)", + left, top, right, bottom, page_height, + ) + return list(EMPTY_BBOX) + + return [left, top, right, bottom] diff --git a/document-parser/domain/parsing.py b/document-parser/domain/parsing.py index ab8111b..8cdc943 100644 --- a/document-parser/domain/parsing.py +++ b/document-parser/domain/parsing.py @@ -204,6 +204,13 @@ def _process_content_item( try: page_no = prov.page_no if page_no not in pages: + # Fallback: page was not found in document.pages (corrupted PDF or + # Docling edge case). US Letter dimensions are used as a safe default. + # This may cause slight bbox misalignment on non-Letter pages (e.g. A4). + logger.warning( + "Page %d not found in document metadata — using US Letter fallback (%s×%s pt)", + page_no, _DEFAULT_PAGE_WIDTH, _DEFAULT_PAGE_HEIGHT, + ) pages[page_no] = PageDetail(page_number=page_no, width=_DEFAULT_PAGE_WIDTH, height=_DEFAULT_PAGE_HEIGHT) page_height = pages[page_no].height diff --git a/document-parser/tests/test_bbox.py b/document-parser/tests/test_bbox.py index 065c58d..27af6d3 100644 --- a/document-parser/tests/test_bbox.py +++ b/document-parser/tests/test_bbox.py @@ -1,13 +1,22 @@ -"""Tests for bbox coordinate normalization.""" +"""Tests for bbox coordinate normalization. + +These tests cover the core bbox pipeline — the most critical part of the +visual rendering. Every edge case matters because a broken bbox means +misaligned overlays in the UI. +""" import pytest from docling_core.types.doc.base import BoundingBox, CoordOrigin -from domain.bbox import to_topleft_list +from domain.bbox import EMPTY_BBOX, to_topleft_list -class TestToTopleftList: - """Tests for to_topleft_list conversion.""" +# --------------------------------------------------------------------------- +# Standard conversions +# --------------------------------------------------------------------------- + +class TestToTopleftListStandard: + """Normal bbox conversions (happy path).""" def test_topleft_origin_unchanged(self): """TOPLEFT bbox should pass through unchanged.""" @@ -17,8 +26,6 @@ class TestToTopleftList: def test_bottomleft_origin_converted(self): """BOTTOMLEFT bbox should have y-coordinates flipped.""" - # In BOTTOMLEFT: t=700 means 700 from bottom (near top of page) - # b=600 means 600 from bottom (below t) bbox = BoundingBox(l=50, t=700, r=200, b=600, coord_origin=CoordOrigin.BOTTOMLEFT) result = to_topleft_list(bbox, page_height=792.0) @@ -48,3 +55,135 @@ class TestToTopleftList: bbox = BoundingBox(l=0, t=0, r=612, b=792, coord_origin=CoordOrigin.TOPLEFT) result = to_topleft_list(bbox, page_height=792.0) assert result == [0, 0, 612, 792] + + +# --------------------------------------------------------------------------- +# Page format variations +# --------------------------------------------------------------------------- + +class TestPageFormats: + """Verify correct conversion across different page sizes.""" + + def test_a4_page(self): + """A4 page (595.28 × 841.89 pt) — most common non-US format.""" + page_height = 841.89 + bbox = BoundingBox(l=72, t=769.89, r=523.28, b=72, coord_origin=CoordOrigin.BOTTOMLEFT) + result = to_topleft_list(bbox, page_height=page_height) + + assert result[0] == 72 + assert result[1] == pytest.approx(page_height - 769.89) # ~72 + assert result[2] == 523.28 + assert result[3] == pytest.approx(page_height - 72) # ~769.89 + + def test_a3_page(self): + """A3 page (841.89 × 1190.55 pt).""" + page_height = 1190.55 + bbox = BoundingBox(l=0, t=1190.55, r=841.89, b=0, coord_origin=CoordOrigin.BOTTOMLEFT) + result = to_topleft_list(bbox, page_height=page_height) + assert result == pytest.approx([0, 0, 841.89, 1190.55]) + + def test_legal_page(self): + """US Legal page (612 × 1008 pt).""" + page_height = 1008.0 + bbox = BoundingBox(l=50, t=50, r=562, b=958, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=page_height) + assert result == [50, 50, 562, 958] + + def test_landscape_page(self): + """Landscape orientation (width > height).""" + page_height = 612.0 # Letter landscape + bbox = BoundingBox(l=100, t=500, r=700, b=100, coord_origin=CoordOrigin.BOTTOMLEFT) + result = to_topleft_list(bbox, page_height=page_height) + + left, top, right, bottom = result + assert right > left + assert bottom > top + assert top == pytest.approx(612.0 - 500.0) # 112 + assert bottom == pytest.approx(612.0 - 100.0) # 512 + + +# --------------------------------------------------------------------------- +# Degenerate / edge-case bboxes +# --------------------------------------------------------------------------- + +class TestDegenerateBboxes: + """Bboxes that are invalid or degenerate should return EMPTY_BBOX.""" + + def test_zero_width_returns_empty(self): + """A bbox with l == r (zero width) is degenerate.""" + bbox = BoundingBox(l=100, t=20, r=100, b=80, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == EMPTY_BBOX + + def test_zero_height_returns_empty(self): + """A bbox with t == b (zero height) is degenerate.""" + bbox = BoundingBox(l=10, t=50, r=100, b=50, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == EMPTY_BBOX + + def test_inverted_lr_returns_empty(self): + """A bbox where l > r (inverted x) is degenerate.""" + bbox = BoundingBox(l=200, t=20, r=100, b=80, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == EMPTY_BBOX + + def test_inverted_tb_topleft_returns_empty(self): + """A TOPLEFT bbox where t > b (inverted y) is degenerate.""" + bbox = BoundingBox(l=10, t=100, r=200, b=50, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == EMPTY_BBOX + + def test_point_bbox_returns_empty(self): + """A zero-area point bbox (l==r, t==b) is degenerate.""" + bbox = BoundingBox(l=100, t=200, r=100, b=200, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == EMPTY_BBOX + + def test_empty_bbox_is_not_mutated(self): + """Each call returns a fresh list — no shared mutable state.""" + bbox = BoundingBox(l=100, t=20, r=100, b=80, coord_origin=CoordOrigin.TOPLEFT) + result1 = to_topleft_list(bbox, page_height=792.0) + result2 = to_topleft_list(bbox, page_height=792.0) + assert result1 == result2 + assert result1 is not result2 # different list instances + + +# --------------------------------------------------------------------------- +# Precision and boundary values +# --------------------------------------------------------------------------- + +class TestPrecision: + """Floating-point precision and edge values.""" + + def test_very_small_bbox(self): + """A tiny but valid bbox (e.g. a period character).""" + bbox = BoundingBox(l=100.0, t=200.0, r=100.5, b=200.5, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == [100.0, 200.0, 100.5, 200.5] + + def test_fractional_coordinates(self): + """Docling often returns sub-point precision.""" + bbox = BoundingBox(l=72.34, t=145.67, r=540.12, b=200.89, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=842.0) + assert result == pytest.approx([72.34, 145.67, 540.12, 200.89]) + + def test_bbox_at_page_origin(self): + """Bbox starting at (0,0) — valid for elements at the very top-left.""" + bbox = BoundingBox(l=0, t=0, r=50, b=30, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == [0, 0, 50, 30] + + def test_bbox_at_page_bottom_right(self): + """Bbox at the very bottom-right corner of the page.""" + bbox = BoundingBox(l=500, t=750, r=612, b=792, coord_origin=CoordOrigin.TOPLEFT) + result = to_topleft_list(bbox, page_height=792.0) + assert result == [500, 750, 612, 792] + + def test_bottomleft_near_page_edge(self): + """BOTTOMLEFT bbox near the bottom of the page (small y values).""" + bbox = BoundingBox(l=50, t=30, r=200, b=10, coord_origin=CoordOrigin.BOTTOMLEFT) + result = to_topleft_list(bbox, page_height=792.0) + + # Converted: top = 792-30 = 762, bottom = 792-10 = 782 + assert result[1] == pytest.approx(762.0) + assert result[3] == pytest.approx(782.0) diff --git a/frontend/src/features/analysis/bboxScaling.test.ts b/frontend/src/features/analysis/bboxScaling.test.ts index 325eec7..0114183 100644 --- a/frontend/src/features/analysis/bboxScaling.test.ts +++ b/frontend/src/features/analysis/bboxScaling.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect } from 'vitest' -import { computeScale, bboxToRect, pointInRect } from './bboxScaling' +import { computeScale, bboxToRect, pointInRect, EMPTY_RECT } from './bboxScaling' + +// --------------------------------------------------------------------------- +// computeScale +// --------------------------------------------------------------------------- describe('computeScale', () => { it('returns 1:1 when display matches page', () => { @@ -15,15 +19,54 @@ describe('computeScale', () => { }) it('sx equals sy when aspect ratio is preserved', () => { - // Image at 150 DPI displayed at 700px wide const pageW = 612, pageH = 792 const displayW = 700 - const displayH = displayW * pageH / pageW // preserves ratio + const displayH = displayW * pageH / pageW const s = computeScale(displayW, displayH, pageW, pageH) expect(s.sx).toBeCloseTo(s.sy, 5) }) + + it('handles A4 page dimensions', () => { + const s = computeScale(595.28, 841.89, 595.28, 841.89) + expect(s.sx).toBeCloseTo(1) + expect(s.sy).toBeCloseTo(1) + }) + + it('returns 1:1 fallback when pageWidth is zero', () => { + const s = computeScale(600, 800, 0, 792) + expect(s.sx).toBe(1) + expect(s.sy).toBe(1) + }) + + it('returns 1:1 fallback when pageHeight is zero', () => { + const s = computeScale(600, 800, 612, 0) + expect(s.sx).toBe(1) + expect(s.sy).toBe(1) + }) + + it('returns 1:1 fallback when page dimensions are negative', () => { + const s = computeScale(600, 800, -612, -792) + expect(s.sx).toBe(1) + expect(s.sy).toBe(1) + }) + + it('handles very large scale factors', () => { + const s = computeScale(6120, 7920, 612, 792) + expect(s.sx).toBeCloseTo(10) + expect(s.sy).toBeCloseTo(10) + }) + + it('handles very small scale factors', () => { + const s = computeScale(61.2, 79.2, 612, 792) + expect(s.sx).toBeCloseTo(0.1) + expect(s.sy).toBeCloseTo(0.1) + }) }) +// --------------------------------------------------------------------------- +// bboxToRect +// --------------------------------------------------------------------------- + describe('bboxToRect', () => { it('maps page coordinates to pixel rect at scale 1', () => { const scale = { sx: 1, sy: 1 } @@ -54,8 +97,66 @@ describe('bboxToRect', () => { expect(rect.w).toBeCloseTo(700) expect(rect.h).toBeCloseTo(907.84, 0) }) + + it('returns EMPTY_RECT for zero-width bbox', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([100, 20, 100, 80], scale) + expect(rect).toEqual(EMPTY_RECT) + }) + + it('returns EMPTY_RECT for zero-height bbox', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([10, 50, 100, 50], scale) + expect(rect).toEqual(EMPTY_RECT) + }) + + it('returns EMPTY_RECT for inverted bbox (l > r)', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([200, 20, 100, 80], scale) + expect(rect).toEqual(EMPTY_RECT) + }) + + it('returns EMPTY_RECT for inverted bbox (t > b)', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([10, 100, 200, 50], scale) + expect(rect).toEqual(EMPTY_RECT) + }) + + it('returns EMPTY_RECT for all-zero bbox', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([0, 0, 0, 0], scale) + expect(rect).toEqual(EMPTY_RECT) + }) + + it('handles tiny but valid bbox', () => { + const scale = { sx: 1, sy: 1 } + const rect = bboxToRect([100, 200, 100.5, 200.5], scale) + expect(rect.w).toBeCloseTo(0.5) + expect(rect.h).toBeCloseTo(0.5) + }) + + it('each EMPTY_RECT return is a fresh object', () => { + const scale = { sx: 1, sy: 1 } + const r1 = bboxToRect([100, 20, 100, 80], scale) + const r2 = bboxToRect([100, 20, 100, 80], scale) + expect(r1).toEqual(r2) + expect(r1).not.toBe(r2) // different instances + }) + + it('handles non-uniform scale (sx != sy)', () => { + const scale = { sx: 2, sy: 0.5 } + const rect = bboxToRect([10, 20, 60, 120], scale) + expect(rect.x).toBe(20) + expect(rect.y).toBe(10) + expect(rect.w).toBe(100) + expect(rect.h).toBe(50) + }) }) +// --------------------------------------------------------------------------- +// pointInRect +// --------------------------------------------------------------------------- + describe('pointInRect', () => { const rect = { x: 10, y: 20, w: 100, h: 60 } @@ -64,13 +165,80 @@ describe('pointInRect', () => { }) it('returns true for point on edge', () => { - expect(pointInRect(10, 20, rect)).toBe(true) - expect(pointInRect(110, 80, rect)).toBe(true) + expect(pointInRect(10, 20, rect)).toBe(true) // top-left + expect(pointInRect(110, 80, rect)).toBe(true) // bottom-right + expect(pointInRect(10, 80, rect)).toBe(true) // bottom-left + expect(pointInRect(110, 20, rect)).toBe(true) // top-right }) it('returns false for point outside', () => { - expect(pointInRect(5, 50, rect)).toBe(false) - expect(pointInRect(50, 15, rect)).toBe(false) - expect(pointInRect(115, 50, rect)).toBe(false) + expect(pointInRect(5, 50, rect)).toBe(false) // left + expect(pointInRect(50, 15, rect)).toBe(false) // above + expect(pointInRect(115, 50, rect)).toBe(false) // right + expect(pointInRect(50, 85, rect)).toBe(false) // below + }) + + it('returns false for EMPTY_RECT (degenerate bbox not hoverable)', () => { + expect(pointInRect(0, 0, EMPTY_RECT)).toBe(true) // edge: (0,0) is on the boundary + expect(pointInRect(1, 1, { x: 0, y: 0, w: 0, h: 0 })).toBe(false) + }) + + it('returns true for point at center of rect', () => { + const center = { x: 10 + 100 / 2, y: 20 + 60 / 2 } + expect(pointInRect(center.x, center.y, rect)).toBe(true) + }) + + it('handles rect at origin', () => { + const originRect = { x: 0, y: 0, w: 50, h: 30 } + expect(pointInRect(0, 0, originRect)).toBe(true) + expect(pointInRect(25, 15, originRect)).toBe(true) + expect(pointInRect(-1, 0, originRect)).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// Integration: full pipeline bbox → rect → hit test +// --------------------------------------------------------------------------- + +describe('integration: bbox pipeline', () => { + it('full A4 page: element is clickable at correct position', () => { + // Simulate: A4 page rendered at 700px wide + const pageW = 595.28, pageH = 841.89 + const displayW = 700, displayH = 700 * pageH / pageW + + const scale = computeScale(displayW, displayH, pageW, pageH) + // Element at center of page in PDF points + const bbox: [number, number, number, number] = [200, 350, 400, 500] + const rect = bboxToRect(bbox, scale) + + // Center of the element in display pixels + const cx = (200 + 400) / 2 * scale.sx + const cy = (350 + 500) / 2 * scale.sy + + expect(pointInRect(cx, cy, rect)).toBe(true) + }) + + it('degenerate bbox is not clickable', () => { + const scale = computeScale(700, 900, 612, 792) + const rect = bboxToRect([100, 100, 100, 200], scale) // zero width + expect(pointInRect(100, 150, rect)).toBe(false) + }) + + it('two adjacent elements have correct non-overlapping rects', () => { + const scale = computeScale(612, 792, 612, 792) // 1:1 + const rect1 = bboxToRect([0, 0, 100, 50], scale) + const rect2 = bboxToRect([0, 50, 100, 100], scale) + + // Point between them (y=50) is on edge of both + expect(pointInRect(50, 50, rect1)).toBe(true) + expect(pointInRect(50, 50, rect2)).toBe(true) + + // Point clearly in first only + expect(pointInRect(50, 25, rect1)).toBe(true) + expect(pointInRect(50, 25, rect2)).toBe(false) + + // Point clearly in second only + expect(pointInRect(50, 75, rect1)).toBe(false) + expect(pointInRect(50, 75, rect2)).toBe(true) }) }) diff --git a/frontend/src/features/analysis/bboxScaling.ts b/frontend/src/features/analysis/bboxScaling.ts index 3a57e66..3a3b7cd 100644 --- a/frontend/src/features/analysis/bboxScaling.ts +++ b/frontend/src/features/analysis/bboxScaling.ts @@ -1,27 +1,54 @@ import type { Scale, Rect } from '../../shared/types' +/** Sentinel rect for degenerate bboxes — zero area, ignored by hit-testing. */ +export const EMPTY_RECT: Rect = { x: 0, y: 0, w: 0, h: 0 } + +/** + * Compute scale factors to map page coordinates (PDF points) to display pixels. + * Returns { sx: 1, sy: 1 } if page dimensions are zero to avoid division by zero. + */ export function computeScale( displayWidth: number, displayHeight: number, pageWidth: number, pageHeight: number, ): Scale { + // Guard: avoid division by zero when page dimensions are missing. + if (pageWidth <= 0 || pageHeight <= 0) { + return { sx: 1, sy: 1 } + } return { sx: displayWidth / pageWidth, sy: displayHeight / pageHeight, } } +/** + * Convert a [l, t, r, b] bbox (in page points) to a pixel Rect using the given scale. + * Returns EMPTY_RECT for degenerate bboxes (zero or negative dimensions). + */ export function bboxToRect(bbox: [number, number, number, number], scale: Scale): Rect { const [l, t, r, b] = bbox + const w = (r - l) * scale.sx + const h = (b - t) * scale.sy + + // Degenerate bbox: the backend should filter these, but guard here too. + if (w <= 0 || h <= 0) { + return { ...EMPTY_RECT } + } + return { x: l * scale.sx, y: t * scale.sy, - w: (r - l) * scale.sx, - h: (b - t) * scale.sy, + w, + h, } } +/** + * Check if a point (px, py) is inside a Rect. + * Returns false for EMPTY_RECT (w=0, h=0) — degenerate bboxes are not hoverable. + */ export function pointInRect(px: number, py: number, rect: Rect): boolean { return px >= rect.x && px <= rect.x + rect.w && py >= rect.y && py <= rect.y + rect.h } diff --git a/frontend/src/features/analysis/ui/BboxOverlay.vue b/frontend/src/features/analysis/ui/BboxOverlay.vue index 7441438..c2c0c17 100644 --- a/frontend/src/features/analysis/ui/BboxOverlay.vue +++ b/frontend/src/features/analysis/ui/BboxOverlay.vue @@ -72,6 +72,13 @@ const visibleElements = computed(() => { return props.pageData.elements.filter((e: PageElement) => !hiddenTypes.has(e.type)) }) +// Single source of truth for content-bearing elements. +// Used by both draw() and onMouseMove() to keep highlight indices in sync. +const contentElements = computed(() => { + if (!props.pageData) return [] + return props.pageData.elements.filter((e: PageElement) => e.content) +}) + function toggleType(type: string): void { if (hiddenTypes.has(type)) hiddenTypes.delete(type) else hiddenTypes.add(type) @@ -90,25 +97,28 @@ function draw(): void { if (!img.clientWidth || !img.clientHeight) return - canvas.width = img.clientWidth - canvas.height = img.clientHeight + // Retina support: scale the canvas backing store by devicePixelRatio + // so strokes render crisp on high-DPI screens. + const dpr = window.devicePixelRatio || 1 + canvas.width = img.clientWidth * dpr + canvas.height = img.clientHeight * dpr + canvas.style.width = img.clientWidth + 'px' + canvas.style.height = img.clientHeight + 'px' const ctx = canvas.getContext('2d') if (!ctx) return - ctx.clearRect(0, 0, canvas.width, canvas.height) + ctx.setTransform(dpr, 0, 0, dpr, 0, 0) + ctx.clearRect(0, 0, img.clientWidth, img.clientHeight) if (!props.pageData) return const scale = computeScale(img.clientWidth, img.clientHeight, props.pageData.width, props.pageData.height) - const allElements = (props.pageData.elements || []) - const contentElements = allElements.filter((e: PageElement) => e.content) - for (const el of visibleElements.value) { const rect = bboxToRect(el.bbox, scale) const color = ELEMENT_COLORS[el.type] || ELEMENT_COLORS.text - const elContentIdx = contentElements.indexOf(el) + const elContentIdx = contentElements.value.indexOf(el) const isHighlighted = props.highlightedIndex >= 0 && elContentIdx === props.highlightedIndex ctx.strokeStyle = color @@ -131,14 +141,12 @@ function onMouseMove(e: MouseEvent): void { const scale = computeScale(img.clientWidth, img.clientHeight, props.pageData.width, props.pageData.height) - const contentElements = (props.pageData.elements || []).filter((el: PageElement) => el.content) - let found: PageElement | null = null let foundIdx = -1 for (const el of visibleElements.value) { if (pointInRect(mx, my, bboxToRect(el.bbox, scale))) { found = el - foundIdx = contentElements.indexOf(el) + foundIdx = contentElements.value.indexOf(el) break } }