From 2eae6eb63b04a4b2f681e978eafbbc987f201889 Mon Sep 17 00:00:00 2001 From: CCLUCKY <2226114701@qq.com> Date: Wed, 28 Oct 2026 10:44:39 +0800 Subject: [PATCH] fix(parser): carry OpenDataLoader PDF positions through to parse items (#21450) --- .../parser/pdf_parser_opendataloader.go | 201 ++++++++++++++--- .../parser/pdf_parser_opendataloader_cgo.go | 17 ++ .../pdf_parser_opendataloader_cgo_test.go | 67 ++++++ .../parser/pdf_parser_opendataloader_nocgo.go | 15 ++ .../pdf_parser_opendataloader_nocgo_test.go | 76 +++++++ ...df_parser_opendataloader_positions_test.go | 205 ++++++++++++++++++ .../parser/pdf_parser_opendataloader_test.go | 4 +- .../parser/parser/pdf_parser_remote_common.go | 2 +- .../parser/pdf_parser_table_contract_test.go | 12 +- .../parser/pdf_table_no_html_text_test.go | 3 +- 10 files changed, 569 insertions(+), 33 deletions(-) create mode 100644 internal/parser/parser/pdf_parser_opendataloader_cgo.go create mode 100644 internal/parser/parser/pdf_parser_opendataloader_cgo_test.go create mode 100644 internal/parser/parser/pdf_parser_opendataloader_nocgo.go create mode 100644 internal/parser/parser/pdf_parser_opendataloader_nocgo_test.go create mode 100644 internal/parser/parser/pdf_parser_opendataloader_positions_test.go diff --git a/internal/parser/parser/pdf_parser_opendataloader.go b/internal/parser/parser/pdf_parser_opendataloader.go index cdb448cff9..daa1084b41 100644 --- a/internal/parser/parser/pdf_parser_opendataloader.go +++ b/internal/parser/parser/pdf_parser_opendataloader.go @@ -5,10 +5,12 @@ import ( "encoding/json" "fmt" "io" + "math" "mime/multipart" "net/http" "ragflow/internal/common" "ragflow/internal/deepdoc/parser/pdf/table" + "sort" "strings" ) @@ -62,9 +64,20 @@ func parsePDFWithOpenDataLoader(ctx context.Context, filename string, data []byt return ParseResult{Err: fmt.Errorf("parser: OpenDataLoader decode: %w", err)} } if payload.JSONDoc != nil { - items := openDataLoaderItems(payload.JSONDoc) + sizes, pageCount, geometryErr := openDataLoaderPageGeometry(ctx, data, openDataLoaderPageIndexes(payload.JSONDoc)) + items, boxWarnings := openDataLoaderItemsWithGeometry(payload.JSONDoc, sizes) if len(items) > 0 { - return pdfItemsToResult(filename, items, openDataLoaderPageCount(payload.JSONDoc)) + if pageCount <= 0 { + // No pdfium geometry (nocgo, or a document it cannot load): + // fall back to the pages the service reported. + pageCount = openDataLoaderReportedPageCount(payload.JSONDoc) + } + res := pdfItemsToResult(filename, items, pageCount) + if geometryErr != nil { + res.Warnings = append(res.Warnings, "OpenDataLoader PDF geometry unavailable: "+geometryErr.Error()) + } + res.Warnings = append(res.Warnings, boxWarnings...) + return res } } if strings.TrimSpace(payload.MDText) != "" { @@ -102,15 +115,28 @@ func openDataLoaderMultipart(filename string, data []byte, parser *PDFParser) (i return strings.NewReader(body.String()), writer.FormDataContentType(), nil } -func openDataLoaderItems(root any) []map[string]any { +// openDataLoaderItemsWithGeometry flattens every typed element into a parse +// item. pageSizes maps a 0-based page index to the PDF's page size in points; +// when it is nil (nocgo, or a document pdfium cannot load) the items keep +// their text and the reported page number, but carry no positions. The second +// return value reports the pages whose boxes did not fit that page size. +func openDataLoaderItemsWithGeometry(root any, pageSizes map[int][2]float64) ([]map[string]any, []string) { items := []map[string]any{} + // One warning per page: a page whose boxes disagree with the point space + // is a coordinate-space mismatch worth surfacing, but repeating it per + // element would bury the rest of the parse result in noise. + reported := map[int]string{} var walk func(any) walk = func(node any) { switch v := node.(type) { case map[string]any: - if item := openDataLoaderNodeToItem(v); item != nil { + item, page, warning := openDataLoaderNodeToItem(v, pageSizes) + if item != nil { items = append(items, item) } + if warning != "" && reported[page] == "" { + reported[page] = warning + } for _, child := range v { walk(child) } @@ -121,14 +147,23 @@ func openDataLoaderItems(root any) []map[string]any { } } walk(root) - return items + pages := make([]int, 0, len(reported)) + for page := range reported { + pages = append(pages, page) + } + sort.Ints(pages) + warnings := make([]string, 0, len(pages)) + for _, page := range pages { + warnings = append(warnings, reported[page]) + } + return items, warnings } -func openDataLoaderNodeToItem(el map[string]any) map[string]any { +func openDataLoaderNodeToItem(el map[string]any, pageSizes map[int][2]float64) (map[string]any, int, string) { rawType, _ := el["type"].(string) t := strings.ToLower(strings.TrimSpace(rawType)) if t == "" { - return nil + return nil, 0, "" } text := strings.TrimSpace(stringValue(el["content"])) if text == "" { @@ -149,33 +184,160 @@ func openDataLoaderNodeToItem(el map[string]any) map[string]any { text = html } if text == "" { - return nil + return nil, 0, "" } item := map[string]any{"text": text, "doc_type_kwd": pdfTableDocType(text), "layout": "table"} projectPDFTable(item) - return item + return openDataLoaderWithPositions(item, el, pageSizes) case "image", "picture", "figure": if text == "" { text = "[Image]" } - return map[string]any{"text": text, "doc_type_kwd": "image", "layout": "figure"} + return openDataLoaderWithPositions(map[string]any{"text": text, "doc_type_kwd": "image", "layout": "figure"}, el, pageSizes) case "formula", "equation": if text == "" { - return nil + return nil, 0, "" } - return map[string]any{"text": text, "doc_type_kwd": "text", "layout": "equation"} + return openDataLoaderWithPositions(map[string]any{"text": text, "doc_type_kwd": "text", "layout": "equation"}, el, pageSizes) default: if text == "" { - return nil + return nil, 0, "" } layout := "text" if t == "title" || t == "heading" { layout = "title" } - return map[string]any{"text": text, "doc_type_kwd": "text", "layout": layout} + return openDataLoaderWithPositions(map[string]any{"text": text, "doc_type_kwd": "text", "layout": layout}, el, pageSizes) } } +func openDataLoaderWithPositions(item map[string]any, el map[string]any, pageSizes map[int][2]float64) (map[string]any, int, string) { + page, ok := openDataLoaderPageNumber(el) + if !ok { + return item, 0, "" + } + // The page identity survives without geometry (nocgo, or a missing box), + // matching the PaddleOCR adapter: downstream chunking and croppers can + // still attribute the item to its page. + item["page_number"] = page + positions, warning := openDataLoaderPositions(page, el, pageSizes) + if len(positions) > 0 { + item["positions"], item["_pdf_positions"] = positions, positions + } + return item, page, warning +} + +// openDataLoaderBleedTolerance is how far past the page edge a box may reach, +// as a fraction of that page dimension. Printed content bleeds slightly; a box +// further out than this is not a bleed but a disagreement about the coordinate +// space, so its positions are dropped and reported rather than clamped. +const openDataLoaderBleedTolerance = 0.1 + +// openDataLoaderPositions converts one element's geometry into RAGFlow's +// canonical [page, left, right, top, bottom] row. OpenDataLoader reports +// [left, bottom, right, top] in PDF points with the origin at the bottom-left, +// while positions are stored in screen space with the origin at the top-left, +// so the vertical edges are flipped against the page height. Without the page +// size the flip cannot be computed, and a wrong-y highlight is worse than none. +// The flip assumes both sides describe the same (post-rotation) page box, which +// holds for the page sizes pdfium reports; the point space itself is a service +// contract this adapter cannot read, so a box that cannot belong to the page is +// treated as evidence of a different space and reported through the second +// return value. +func openDataLoaderPositions(page int, el map[string]any, pageSizes map[int][2]float64) ([][]any, string) { + size, ok := pageSizes[page-1] + if !ok || size[0] <= 0 || size[1] <= 0 { + return nil, "" + } + box, ok := openDataLoaderBox(el) + if !ok { + return nil, "" + } + left, bottom, right, top := box[0], box[1], box[2], box[3] + if right <= left || top <= bottom { + return nil, "" + } + overflowRight, overflowBottom := math.Max(0, math.Max(-left, right-size[0])), math.Max(0, math.Max(-bottom, top-size[1])) + if overflowRight > size[0]*openDataLoaderBleedTolerance || overflowBottom > size[1]*openDataLoaderBleedTolerance { + return nil, fmt.Sprintf("OpenDataLoader page %d: element box [%g %g %g %g] does not fit page size %gx%g; positions omitted because the service coordinates do not match the PDF point space", page, left, bottom, right, top, size[0], size[1]) + } + left = math.Max(0, math.Min(size[0], left)) + right = math.Max(0, math.Min(size[0], right)) + screenTop := math.Max(0, math.Min(size[1], size[1]-top)) + screenBottom := math.Max(0, math.Min(size[1], size[1]-bottom)) + if right <= left || screenBottom <= screenTop { + return nil, "" + } + return [][]any{{float64(page), left, right, screenTop, screenBottom}}, "" +} + +// openDataLoaderReportedPageCount is the page-count fallback when pdfium is +// unavailable: prefer the root's page total, then the highest page number any +// element reports (a sparse document must not be counted by distinct pages), +// and finally one, because items exist. +func openDataLoaderReportedPageCount(root any) int { + if top, ok := root.(map[string]any); ok { + for _, key := range []string{"number of pages", "number_of_pages", "page count", "page_count"} { + if count := int(numberValue(top[key])); count > 0 { + return count + } + } + } + count := 0 + for page := range collectPDFPageNumbers(root) { + if page > count { + count = page + } + } + if count == 0 { + count = 1 + } + return count +} + +// openDataLoaderPageIndexes lists the zero-based pages the service reported +// content for, so the geometry lookup only opens those pages. +func openDataLoaderPageIndexes(root any) []int { + pages := collectPDFPageNumbers(root) + indexes := make([]int, 0, len(pages)) + for page := range pages { + indexes = append(indexes, page-1) + } + sort.Ints(indexes) + return indexes +} + +func openDataLoaderPageNumber(el map[string]any) (int, bool) { + for _, key := range []string{"page number", "page_number", "page"} { + if raw, ok := el[key]; ok { + n := numberValue(raw) + if page := int(n); n > 0 && float64(page) == n { + return page, true + } + } + } + return 0, false +} + +func openDataLoaderBox(el map[string]any) ([4]float64, bool) { + var box [4]float64 + for _, key := range []string{"bounding box", "bounding_box", "bbox"} { + coords, ok := el[key].([]any) + if !ok || len(coords) < 4 { + continue + } + for i := 0; i < 4; i++ { + v, ok := numericAny(coords[i]) + if !ok { + return box, false + } + box[i] = v + } + return box, true + } + return box, false +} + func openDataLoaderCellRows(raw any) [][]string { cells, ok := raw.([]any) if !ok { @@ -209,17 +371,6 @@ func openDataLoaderCellRows(raw any) [][]string { return ordered } -func openDataLoaderPageCount(root any) int { - pages := collectPDFPageNumbers(root) - if len(pages) > 0 { - return len(pages) - } - if root != nil { - return 1 - } - return 0 -} - func stringValue(v any) string { s, _ := v.(string) return s diff --git a/internal/parser/parser/pdf_parser_opendataloader_cgo.go b/internal/parser/parser/pdf_parser_opendataloader_cgo.go new file mode 100644 index 0000000000..7613da2433 --- /dev/null +++ b/internal/parser/parser/pdf_parser_opendataloader_cgo.go @@ -0,0 +1,17 @@ +//go:build cgo + +package parser + +import ( + "context" + + "ragflow/internal/deepdoc/parser/pdf/pdfium" +) + +// openDataLoaderPageGeometry returns the requested pages' sizes in PDF points +// and the document page count, which the adapter needs to flip +// OpenDataLoader's bottom-left coordinates into the screen-space rows +// positions use. One document load serves both values. +func openDataLoaderPageGeometry(ctx context.Context, data []byte, indexes []int) (map[int][2]float64, int, error) { + return pdfium.PageSizes(ctx, data, indexes) +} diff --git a/internal/parser/parser/pdf_parser_opendataloader_cgo_test.go b/internal/parser/parser/pdf_parser_opendataloader_cgo_test.go new file mode 100644 index 0000000000..4b45e77edc --- /dev/null +++ b/internal/parser/parser/pdf_parser_opendataloader_cgo_test.go @@ -0,0 +1,67 @@ +//go:build cgo + +package parser + +import ( + "io" + "math" + "net/http" + "net/http/httptest" + "os" + "testing" + + "ragflow/internal/deepdoc/parser/pdf/pdfium" +) + +func TestPDFParser_ParseWithResult_OpenDataLoaderPositions(t *testing.T) { + withSSRFBypass(t) + data, err := os.ReadFile("testdata/paddleocr_pages.pdf") + if err != nil { + t.Fatal(err) + } + sizes, pageCount, err := pdfium.PageSizes(t.Context(), data, []int{0}) + if err != nil || pageCount == 0 { + t.Fatalf("PageSizes: sizes=%v count=%d err=%v", sizes, pageCount, err) + } + pageHeight := sizes[0][1] + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, err := io.Copy(io.Discard, r.Body); err != nil { + t.Errorf("drain body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"json_doc":{"type":"paragraph","content":"ODL Body","page number":1,"bounding box":[72,650.5,540,700.2]}}`)) + })) + defer server.Close() + + pdf := NewPDFParser() + pdf.ConfigureFromSetup(map[string]any{ + "parse_method": "OpenDataLoader", + "opendataloader_apiserver": server.URL, + }) + res := pdf.ParseWithResult(t.Context(), "sample.pdf", data) + if res.Err != nil { + t.Fatalf("ParseWithResult: %v", res.Err) + } + if got, want := res.File["page_count"], pageCount; got != want { + t.Errorf("page_count = %v, want %d (pdfium)", got, want) + } + if len(res.Warnings) != 0 { + t.Errorf("warnings = %v, want none when geometry is available", res.Warnings) + } + item := res.JSON[0].ToMap() + rows, ok := item["_pdf_positions"].([][]any) + if !ok || len(rows) != 1 { + t.Fatalf("_pdf_positions = %#v, want one row", item["_pdf_positions"]) + } + if page, ok := numericAny(rows[0][0]); !ok || page != 1 { + t.Fatalf("page = %#v, want 1 (positions normalize the page to int)", rows[0][0]) + } + want := []float64{72, 540, pageHeight - 700.2, pageHeight - 650.5} + for i := range want { + got, ok := rows[0][i+1].(float64) + if !ok || math.Abs(got-want[i]) > 1e-6 { + t.Fatalf("_pdf_positions = %#v, want %v", rows, want) + } + } +} diff --git a/internal/parser/parser/pdf_parser_opendataloader_nocgo.go b/internal/parser/parser/pdf_parser_opendataloader_nocgo.go new file mode 100644 index 0000000000..efb54bdd33 --- /dev/null +++ b/internal/parser/parser/pdf_parser_opendataloader_nocgo.go @@ -0,0 +1,15 @@ +//go:build !cgo + +package parser + +import ( + "context" + "fmt" +) + +func openDataLoaderPageGeometry(ctx context.Context, _ []byte, _ []int) (map[int][2]float64, int, error) { + if err := ctx.Err(); err != nil { + return nil, 0, err + } + return nil, 0, fmt.Errorf("PDF page geometry requires cgo") +} diff --git a/internal/parser/parser/pdf_parser_opendataloader_nocgo_test.go b/internal/parser/parser/pdf_parser_opendataloader_nocgo_test.go new file mode 100644 index 0000000000..9e5ba09d43 --- /dev/null +++ b/internal/parser/parser/pdf_parser_opendataloader_nocgo_test.go @@ -0,0 +1,76 @@ +//go:build !cgo + +package parser + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" +) + +func TestPDFParser_ParseWithResult_OpenDataLoaderWithoutGeometry(t *testing.T) { + withSSRFBypass(t) + data, err := os.ReadFile("testdata/paddleocr_pages.pdf") + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, err := io.Copy(io.Discard, r.Body); err != nil { + t.Errorf("drain body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"json_doc":{"type":"paragraph","content":"ODL Body","page number":1,"bounding box":[72,650.5,540,700.2]}}`)) + })) + defer server.Close() + + pdf := NewPDFParser() + pdf.ConfigureFromSetup(map[string]any{ + "parse_method": "OpenDataLoader", + "opendataloader_apiserver": server.URL, + }) + res := pdf.ParseWithResult(t.Context(), "sample.pdf", data) + if res.Err != nil { + t.Fatalf("ParseWithResult: %v", res.Err) + } + item := res.JSON[0].ToMap() + if item["text"] != "ODL Body" { + t.Errorf("text = %v, want ODL Body", item["text"]) + } + if item["_pdf_positions"] != nil { + t.Errorf("_pdf_positions = %#v, want none without page geometry", item["_pdf_positions"]) + } + if page, ok := numericAny(item["page_number"]); !ok || page != 1 { + t.Errorf("page_number = %#v, want 1 (page identity survives without geometry)", item["page_number"]) + } + if got, want := res.File["page_count"], 1; got != want { + t.Errorf("page_count = %v, want %d (pages reported by the service)", got, want) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "requires cgo") { + t.Errorf("warnings = %v, want one geometry warning", res.Warnings) + } + + // A response without page keys still yields items; the document must not + // report zero pages because nothing pins the count down. + pageLess := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if _, err := io.Copy(io.Discard, r.Body); err != nil { + t.Errorf("drain body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"json_doc":{"type":"document","number of pages":3,"children":[{"type":"paragraph","content":"No page"}]}}`)) + })) + defer pageLess.Close() + pdf.ConfigureFromSetup(map[string]any{ + "parse_method": "OpenDataLoader", + "opendataloader_apiserver": pageLess.URL, + }) + pageLessRes := pdf.ParseWithResult(t.Context(), "sample.pdf", data) + if pageLessRes.Err != nil { + t.Fatalf("ParseWithResult (page-less): %v", pageLessRes.Err) + } + if got, want := pageLessRes.File["page_count"], 3; got != want { + t.Errorf("page_count = %v, want %d from the service's page total", got, want) + } +} diff --git a/internal/parser/parser/pdf_parser_opendataloader_positions_test.go b/internal/parser/parser/pdf_parser_opendataloader_positions_test.go new file mode 100644 index 0000000000..4a2e22fae0 --- /dev/null +++ b/internal/parser/parser/pdf_parser_opendataloader_positions_test.go @@ -0,0 +1,205 @@ +package parser + +import ( + "math" + "strings" + "testing" +) + +// OpenDataLoader reports [left, bottom, right, top] in PDF points (origin +// bottom-left); RAGFlow stores [page, left, right, top, bottom] in screen space +// (origin top-left). These tests pin the conversion that keeps citations +// highlightable (issue #21216). +func TestOpenDataLoaderItems_ScreenSpacePositions(t *testing.T) { + root := map[string]any{ + "type": "table of contents", + "page number": 1.0, + "bounding box": []any{72.0, 650.5, 540.0, 700.2}, + "children": []any{ + map[string]any{ + "type": "paragraph", + "content": "body", + "page number": 2.0, + "bbox": []any{100.0, 50.0, 200.0, 80.0}, + }, + }, + } + sizes := map[int][2]float64{0: {612, 792}, 1: {612, 792}} + items, _ := openDataLoaderItemsWithGeometry(root, sizes) + if len(items) != 1 { + t.Fatalf("items = %d, want 1 (unknown type without text is skipped): %v", len(items), items) + } + got := items[0]["_pdf_positions"].([][]any) + want := []any{2.0, 100.0, 200.0, 712.0, 742.0} + if len(got) != 1 || len(got[0]) != 5 { + t.Fatalf("positions = %#v, want one 5-column row", got) + } + for i := range want { + if math.Abs(got[0][i].(float64)-want[i].(float64)) > 1e-6 { + t.Fatalf("positions = %#v, want %#v", got, want) + } + } + legacy, ok := items[0]["positions"].([][]any) + if !ok || len(legacy) != 1 || len(legacy[0]) != 5 { + t.Fatalf("positions alias = %#v, want the same one-row matrix", items[0]["positions"]) + } + for i := range want { + if math.Abs(legacy[0][i].(float64)-want[i].(float64)) > 1e-6 { + t.Fatalf("positions alias = %#v, want %#v", legacy, want) + } + } +} + +func TestOpenDataLoaderItems_AllElementTypesCarryPositions(t *testing.T) { + root := map[string]any{ + "type": "document", "page number": 1.0, "bounding box": []any{0.0, 0.0, 612.0, 792.0}, + "children": []any{ + map[string]any{"type": "heading", "content": "Title", "page number": 1.0, "bounding box": []any{72.0, 700.0, 400.0, 730.0}}, + map[string]any{"type": "table", "html": "
a
", "page number": 1.0, "bounding box": []any{72.0, 400.0, 500.0, 600.0}}, + map[string]any{"type": "image", "page number": 1.0, "bounding box": []any{72.0, 100.0, 300.0, 300.0}}, + map[string]any{"type": "formula", "content": "a+b", "page number": 1.0, "bounding box": []any{72.0, 50.0, 200.0, 80.0}}, + }, + } + sizes := map[int][2]float64{0: {612, 792}} + items, _ := openDataLoaderItemsWithGeometry(root, sizes) + if len(items) != 4 { + t.Fatalf("items = %d, want 4: %v", len(items), items) + } + for i, item := range items { + rows, ok := item["_pdf_positions"].([][]any) + if !ok || len(rows) != 1 || rows[0][0] != 1.0 { + t.Errorf("item %d (%v) positions = %#v, want one page-1 row", i, item["layout"], item["_pdf_positions"]) + } + } +} + +func TestOpenDataLoaderItems_NoGeometryNoPositions(t *testing.T) { + root := map[string]any{"type": "paragraph", "content": "body", "page number": 1.0, "bounding box": []any{72.0, 650.5, 540.0, 700.2}} + if items, _ := openDataLoaderItemsWithGeometry(root, nil); len(items) != 1 || items[0]["_pdf_positions"] != nil { + t.Fatalf("items = %v, want one item without positions when page geometry is unavailable", items) + } else if items[0]["page_number"] != 1 { + t.Errorf("page_number = %#v, want 1 (page identity survives without geometry)", items[0]["page_number"]) + } + // A page the geometry map does not cover behaves the same way. + sizes := map[int][2]float64{2: {612, 792}} + if items, _ := openDataLoaderItemsWithGeometry(root, sizes); len(items) != 1 || items[0]["_pdf_positions"] != nil { + t.Fatalf("items = %v, want no positions for an uncovered page", items) + } +} + +func TestOpenDataLoaderItems_BadBoxesAreDropped(t *testing.T) { + for _, el := range []map[string]any{ + {"type": "paragraph", "content": "no page", "bounding box": []any{1.0, 2.0, 3.0, 4.0}}, + {"type": "paragraph", "content": "no box", "page number": 1.0}, + {"type": "paragraph", "content": "short box", "page number": 1.0, "bounding box": []any{1.0, 2.0}}, + {"type": "paragraph", "content": "zero width", "page number": 1.0, "bounding box": []any{5.0, 2.0, 5.0, 4.0}}, + {"type": "paragraph", "content": "inverted", "page number": 1.0, "bounding box": []any{200.0, 300.0, 100.0, 100.0}}, + {"type": "paragraph", "content": "page zero", "page number": 0.0, "bounding box": []any{1.0, 2.0, 3.0, 4.0}}, + } { + items, _ := openDataLoaderItemsWithGeometry(el, map[int][2]float64{0: {612, 792}}) + if len(items) != 1 { + t.Fatalf("%v: items = %d, want 1", el["content"], len(items)) + } + if items[0]["_pdf_positions"] != nil { + t.Errorf("%v: positions = %#v, want none", el["content"], items[0]["_pdf_positions"]) + } + } +} + +// TestOpenDataLoaderItems_BoxClampedToPage keeps a slightly bleeding box from +// producing a highlight outside the page rectangle. +func TestOpenDataLoaderItems_BoxClampedToPage(t *testing.T) { + el := map[string]any{ + "type": "paragraph", "content": "bleed", "page number": 1.0, + "bounding box": []any{-20.0, 700.0, 620.0, 780.0}, + } + items, warnings := openDataLoaderItemsWithGeometry(el, map[int][2]float64{0: {612, 792}}) + if len(warnings) != 0 { + t.Fatalf("warnings = %v, want none for a bleed inside the tolerance", warnings) + } + rows, ok := items[0]["_pdf_positions"].([][]any) + if !ok || len(rows) != 1 { + t.Fatalf("positions = %#v, want one row", items[0]["_pdf_positions"]) + } + want := []float64{0, 612, 12, 92} + for i, v := range want { + if got, _ := numericAny(rows[0][i+1]); math.Abs(got-v) > 1e-6 { + t.Fatalf("positions = %#v, want the clamped page box %v", rows, want) + } + } +} + +// TestOpenDataLoaderItems_ForeignCoordinateSpaceIsReported covers the failure +// the clamp used to hide: boxes in another space (image pixels, for example) +// land outside the page, and clamping them produced a full-page highlight that +// looked valid. The positions are dropped and the mismatch is surfaced instead. +func TestOpenDataLoaderItems_ForeignCoordinateSpaceIsReported(t *testing.T) { + root := []any{ + // 150 DPI pixels on a 612x792 point page. + map[string]any{"type": "paragraph", "content": "a", "page number": 1.0, "bounding box": []any{150.0, 1351.0, 550.0, 1400.0}}, + map[string]any{"type": "paragraph", "content": "b", "page number": 1.0, "bounding box": []any{0.0, 0.0, 1275.0, 1650.0}}, + } + items, warnings := openDataLoaderItemsWithGeometry(root, map[int][2]float64{0: {612, 792}}) + if len(items) != 2 { + t.Fatalf("items = %d, want 2 (text survives without positions)", len(items)) + } + for i, item := range items { + if item["_pdf_positions"] != nil { + t.Errorf("item %d positions = %#v, want none", i, item["_pdf_positions"]) + } + if item["page_number"] != 1 { + t.Errorf("item %d page_number = %#v, want 1", i, item["page_number"]) + } + } + // One warning per page, naming the offending box and the page size. + if len(warnings) != 1 { + t.Fatalf("warnings = %q, want one per page", warnings) + } + for _, want := range []string{"page 1", "612x792", "coordinate"} { + if !strings.Contains(warnings[0], want) { + t.Fatalf("warning = %q, want it to mention %q", warnings[0], want) + } + } +} + +// TestOpenDataLoaderItems_MixedPagesWarnPerPage checks that a second page with +// its own mismatch gets its own warning rather than being folded away. +func TestOpenDataLoaderItems_MixedPagesWarnPerPage(t *testing.T) { + root := []any{ + map[string]any{"type": "paragraph", "content": "ok", "page number": 1.0, "bounding box": []any{72.0, 100.0, 540.0, 140.0}}, + map[string]any{"type": "paragraph", "content": "bad", "page number": 3.0, "bounding box": []any{0.0, 0.0, 2000.0, 2000.0}}, + } + sizes := map[int][2]float64{0: {612, 792}, 2: {612, 792}} + items, warnings := openDataLoaderItemsWithGeometry(root, sizes) + if len(warnings) != 1 || !strings.Contains(warnings[0], "page 3") { + t.Fatalf("warnings = %q, want one warning for page 3", warnings) + } + if items[0]["_pdf_positions"] == nil { + t.Errorf("page 1 positions = nil, want the valid box kept") + } +} + +// TestOpenDataLoaderReportedPageCount pins the geometry-free fallback: the +// root's total wins; otherwise the HIGHEST page number, not the number of +// distinct pages (a sparse document would otherwise undercount); otherwise 1. +func TestOpenDataLoaderReportedPageCount(t *testing.T) { + cases := []struct { + name string + root any + want int + }{ + {"root total", map[string]any{"number of pages": 7, "children": []any{ + map[string]any{"type": "paragraph", "content": "a", "page number": 1.0}}}, 7}, + {"highest page", map[string]any{"children": []any{ + map[string]any{"type": "paragraph", "content": "a", "page number": 3.0}, + map[string]any{"type": "paragraph", "content": "b", "page number": 1.0}}}, 3}, + {"no page keys", map[string]any{"type": "paragraph", "content": "a"}, 1}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := openDataLoaderReportedPageCount(c.root); got != c.want { + t.Errorf("page count = %d, want %d", got, c.want) + } + }) + } +} diff --git a/internal/parser/parser/pdf_parser_opendataloader_test.go b/internal/parser/parser/pdf_parser_opendataloader_test.go index 481f57906c..e7c51693ce 100644 --- a/internal/parser/parser/pdf_parser_opendataloader_test.go +++ b/internal/parser/parser/pdf_parser_opendataloader_test.go @@ -136,7 +136,7 @@ func TestOpenDataLoaderItems_TableCellsFallback(t *testing.T) { map[string]any{"row": 0, "content": "b"}, }, } - items := openDataLoaderItems(root) + items, _ := openDataLoaderItemsWithGeometry(root, nil) if len(items) != 1 { t.Fatalf("items len = %d, want 1", len(items)) } @@ -164,7 +164,7 @@ func TestOpenDataLoaderItems_TableCellsFallbackSparseRows(t *testing.T) { map[string]any{"row": 5, "content": "z"}, }, } - items := openDataLoaderItems(root) + items, _ := openDataLoaderItemsWithGeometry(root, nil) if len(items) != 1 { t.Fatalf("items len = %d, want 1", len(items)) } diff --git a/internal/parser/parser/pdf_parser_remote_common.go b/internal/parser/parser/pdf_parser_remote_common.go index 5b39ffadac..62158f8f38 100644 --- a/internal/parser/parser/pdf_parser_remote_common.go +++ b/internal/parser/parser/pdf_parser_remote_common.go @@ -50,7 +50,7 @@ func collectPDFPageNumbers(raw any) map[int]struct{} { walk = func(node any) { switch v := node.(type) { case map[string]any: - for _, key := range []string{"page_number", "page_num", "page_no", "page_index", "page_idx", "page"} { + for _, key := range []string{"page number", "page_number", "page_num", "page_no", "page_index", "page_idx", "page"} { if page := int(numberValue(v[key])); page > 0 { pages[page] = struct{}{} } diff --git a/internal/parser/parser/pdf_parser_table_contract_test.go b/internal/parser/parser/pdf_parser_table_contract_test.go index 9a3d222df4..b8501c8d4f 100644 --- a/internal/parser/parser/pdf_parser_table_contract_test.go +++ b/internal/parser/parser/pdf_parser_table_contract_test.go @@ -49,16 +49,20 @@ func TestPDFRemoteParsers_TableItemsCarryTableMarkup(t *testing.T) { wantDocType string }{ {"opendataloader html", func() []map[string]any { - return openDataLoaderItems(map[string]any{"type": "table", "html": tableHTML}) + items, _ := openDataLoaderItemsWithGeometry(map[string]any{"type": "table", "html": tableHTML}, nil) + return items }, "table"}, {"opendataloader content only", func() []map[string]any { - return openDataLoaderItems(map[string]any{"type": "table", "content": pipeText}) + items, _ := openDataLoaderItemsWithGeometry(map[string]any{"type": "table", "content": pipeText}, nil) + return items }, "text"}, {"opendataloader cells only", func() []map[string]any { - return openDataLoaderItems(map[string]any{"type": "table", "cells": cells}) + items, _ := openDataLoaderItemsWithGeometry(map[string]any{"type": "table", "cells": cells}, nil) + return items }, "table"}, {"opendataloader blank cells keep content", func() []map[string]any { - return openDataLoaderItems(map[string]any{"type": "table", "content": pipeText, "cells": blankCells}) + items, _ := openDataLoaderItemsWithGeometry(map[string]any{"type": "table", "content": pipeText, "cells": blankCells}, nil) + return items }, "text"}, {"tcadp html content", func() []map[string]any { return tcadpAnyToItems(map[string]any{"type": "table", "content": tableHTML}) diff --git a/internal/parser/parser/pdf_table_no_html_text_test.go b/internal/parser/parser/pdf_table_no_html_text_test.go index d88deedf55..ee4c88f270 100644 --- a/internal/parser/parser/pdf_table_no_html_text_test.go +++ b/internal/parser/parser/pdf_table_no_html_text_test.go @@ -27,7 +27,8 @@ func TestPDFTableItemsOmitHTMLText(t *testing.T) { return []map[string]any{item} }}, {"opendataloader html", func() []map[string]any { - return openDataLoaderItems(map[string]any{"type": "table", "html": tableHTML}) + items, _ := openDataLoaderItemsWithGeometry(map[string]any{"type": "table", "html": tableHTML}, nil) + return items }}, {"tcadp html content", func() []map[string]any { return tcadpAnyToItems(map[string]any{"type": "table", "content": tableHTML})