Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 105 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -87,7 +101,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -207,7 +235,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -352,7 +394,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -586,7 +642,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -942,7 +1012,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down Expand Up @@ -1340,7 +1424,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

Real font metrics for the 14 standard PDF fonts. No public API change.
Real font metrics for the 14 standard PDF fonts, plus a font-coverage
test corpus. One behaviour change to a default, noted below.

### Changed (behaviour)

- **`DefaultWordOpts()` now enables `UseExplicitSpaces`.** pdfplumber's
`WordExtractor` ends a word *at* a whitespace glyph, before any gap
test runs; pdftable dropped spaces and re-inferred boundaries purely
from the gap. At small type that silently over-merges: an 8pt space is
278/1000 x 8 = 2.22pt, under the 3pt `XTolerance`, so
`Wim illegible 3,142 (16,048)` came back as one run while pdfplumber
returned four words. Body type in real documents is routinely 8-9pt, so
this was not an edge case. With it on, pdftable matches pdfplumber
word-for-word and coordinate-for-coordinate across all 12 Latin
standard fonts at 8/12/24pt.

### Fixed

Expand Down
170 changes: 170 additions & 0 deletions fonts_fixture_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
// Copyright (c) 2026 Halleluyah Oludele
// Licensed under the MIT License.

package pdftable_test

import (
"strings"
"testing"

"github.com/hallelx2/pdftable"
)

// The fixtures under testdata/fonts deliberately have NO pdfplumber
// golden. pdfplumber decodes Symbol and ZapfDingbats with
// StandardEncoding and returns Latin letters where the correct answer is
// Greek and dingbats, so a generated golden would pin the wrong answer.
// Here we assert the right answer directly.
//
// Regenerate the PDFs with: python scripts/gen_font_fixtures.py

func openPage(t *testing.T, path string, n int) pdftable.Page {
t.Helper()
doc, err := pdftable.OpenFile(path)
if err != nil {
t.Fatalf("OpenFile(%s): %v", path, err)
}
t.Cleanup(func() { doc.Close() })
p, err := doc.Page(n)
if err != nil {
t.Fatalf("Page(%d): %v", n, err)
}
return p
}

// TestSymbolFixtureDecoding is the end-to-end proof for HAL-481: a real
// PDF using Symbol and ZapfDingbats, parsed through the full pipeline.
//
// Both fonts carry their own built-in encoding and neither declares
// /Encoding, so a consumer that reaches for StandardEncoding reads Latin
// letters off a page of Greek. pdfplumber 0.11.9 still does exactly that
// — it returns "abgdep" for the Symbol page and "123" for the dingbats —
// which is why this fixture is asserted here rather than against it.
func TestSymbolFixtureDecoding(t *testing.T) {
t.Run("Symbol", func(t *testing.T) {
p := openPage(t, "testdata/fonts/symbol.pdf", 1)
text, err := p.ExtractText(pdftable.DefaultTextOpts())
if err != nil {
t.Fatalf("ExtractText: %v", err)
}
// The content stream bytes are literally "abgdep". In Symbol's
// encoding those codes are alpha beta gamma delta epsilon pi.
const want = "αβγδεπ"
if !strings.Contains(text, want) {
t.Errorf("Symbol text = %q, want it to contain %q.\n"+
"Getting \"abgdep\" back means StandardEncoding was applied "+
"to a font that ships its own encoding.", text, want)
}
if strings.Contains(text, "abgdep") {
t.Error("Symbol decoded as Latin \"abgdep\" — built-in encoding not applied")
}
})

t.Run("ZapfDingbats", func(t *testing.T) {
p := openPage(t, "testdata/fonts/symbol.pdf", 2)
text, err := p.ExtractText(pdftable.DefaultTextOpts())
if err != nil {
t.Fatalf("ExtractText: %v", err)
}
// Codes 0x31..0x33 are the a-names, not the digits 1..3.
if strings.Contains(text, "123") {
t.Errorf("ZapfDingbats text = %q, decoded as digits — built-in encoding not applied", text)
}
for _, r := range []rune{'✑', '✒', '✓'} {
if !strings.ContainsRune(text, r) {
t.Errorf("ZapfDingbats text = %q, missing %q", text, r)
}
}
})
}

// TestDifferencesFixtureDecoding covers the resolver split from HAL-481
// through a real /Differences array.
//
// Symbol's glyph names are genuine Adobe Glyph List entries, so they mean
// the same thing in any font — a Helvetica whose /Differences names
// "Alpha" really is asking for U+0391. ZapfDingbats' "aNN" names are
// font-specific and must NOT resolve here: a Latin font naming "a1" means
// its own glyph, not U+2701 SCISSORS. Resolving those globally would
// silently corrupt text in any document that happens to use the name.
func TestDifferencesFixtureDecoding(t *testing.T) {
p := openPage(t, "testdata/fonts/differences.pdf", 1)
text, err := p.ExtractText(pdftable.DefaultTextOpts())
if err != nil {
t.Fatalf("ExtractText: %v", err)
}

// A=/Alpha B=/universal C=/club E=/summation F=/partialdiff
for _, tc := range []struct {
glyph string
want rune
}{
{"Alpha", 'Α'},
{"universal", '∀'},
{"club", '♣'},
{"summation", '∑'},
{"partialdiff", '∂'},
} {
if !strings.ContainsRune(text, tc.want) {
t.Errorf("/Differences name %q did not resolve to %q; text = %q",
tc.glyph, tc.want, tc.want)
}
}

// D=/a1 — a ZapfDingbats name in a Helvetica font. It must not become
// the scissors dingbat.
if strings.ContainsRune(text, '✁') {
t.Errorf("/Differences name \"a1\" resolved to U+2701 in a Latin font; "+
"dingbat names must stay font-scoped. text = %q", text)
}
}

// TestStandard14FixtureCoversEveryLatinFont guards the corpus itself.
// The suite previously ran on 26 words in a single font, which is how two
// font-metric bugs survived four releases. If someone trims this fixture,
// the coverage silently collapses again — so assert the breadth, not just
// the output.
func TestStandard14FixtureCoversEveryLatinFont(t *testing.T) {
doc, err := pdftable.OpenFile("testdata/golden/fonts-standard14.pdf")
if err != nil {
t.Fatalf("OpenFile: %v", err)
}
defer doc.Close()

want := []string{
"Helvetica", "Helvetica-Bold", "Helvetica-Oblique", "Helvetica-BoldOblique",
"Times-Roman", "Times-Bold", "Times-Italic", "Times-BoldItalic",
"Courier", "Courier-Bold", "Courier-Oblique", "Courier-BoldOblique",
}
if doc.NumPages() != len(want) {
t.Fatalf("fixture has %d pages, want %d (one per Latin standard font)",
doc.NumPages(), len(want))
}

seen := map[string]bool{}
sizes := map[float64]bool{}
for n := 1; n <= doc.NumPages(); n++ {
p, err := doc.Page(n)
if err != nil {
t.Fatalf("Page(%d): %v", n, err)
}
chars, err := p.Chars()
if err != nil || len(chars) == 0 {
t.Fatalf("page %d: no chars (%v)", n, err)
}
for _, c := range chars {
seen[c.FontName] = true
sizes[c.FontSize] = true
}
}
for _, f := range want {
if !seen[f] {
t.Errorf("fixture never renders %s", f)
}
}
// Multiple sizes matter: a descent scaled by the wrong factor is
// indistinguishable from a wrong constant at a single size.
if len(sizes) < 3 {
t.Errorf("fixture uses %d font sizes, want >= 3", len(sizes))
}
Comment on lines +165 to +169

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the required font sizes.

The cardinality check passes if a future fixture removes 8pt and adds another size. That removes the small-space regression coverage. Check for 8pt, 12pt, and 24pt explicitly.

Proposed fix
-	if len(sizes) < 3 {
-		t.Errorf("fixture uses %d font sizes, want >= 3", len(sizes))
+	for _, size := range []float64{8, 12, 24} {
+		if !sizes[size] {
+			t.Errorf("fixture does not render the required %gpt size", size)
+		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Multiple sizes matter: a descent scaled by the wrong factor is
// indistinguishable from a wrong constant at a single size.
if len(sizes) < 3 {
t.Errorf("fixture uses %d font sizes, want >= 3", len(sizes))
}
// Multiple sizes matter: a descent scaled by the wrong factor is
// indistinguishable from a wrong constant at a single size.
for _, size := range []float64{8, 12, 24} {
if !sizes[size] {
t.Errorf("fixture does not render the required %gpt size", size)
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fonts_fixture_test.go` around lines 165 - 169, Update the font-size
validation around the sizes fixture check to require the explicit presence of
8pt, 12pt, and 24pt, rather than only checking that at least three sizes exist.
Preserve the existing error-reporting style and ensure the test fails if any
required size is missing.

}
36 changes: 35 additions & 1 deletion merge_split_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,40 @@ func TestMergeSplitTokensLeavesRealColumnsAlone(t *testing.T) {
}
}

// TestBoundarySplitsTokenRespectsExplicitSpaces guards against welding
// real words together.
//
// The space glyph is itself a Char, sitting flush against its neighbours.
// Measuring gaps without excluding it makes every space look like an
// intra-word join — on a real 10-K that turned "(Dollars in millions,"
// plus "except per share amount)" into "millions,except". A whitespace
// glyph across the boundary is the PDF stating outright that a word ends
// there, and it outranks any geometry.
func TestBoundarySplitsTokenRespectsExplicitSpaces(t *testing.T) {
left := BBox{X0: 90, X1: 118, Y0: 559, Y1: 569}
right := BBox{X0: 118, X1: 180, Y0: 559, Y1: 569}

// No space: adjacent glyphs really are one token.
noSpace := []Char{
glyph(",", 110, 116),
glyph("e", 116.02, 124),
}
if !boundarySplitsToken(noSpace, left, right, 3) {
t.Error("adjacent glyphs with no space should read as a split token")
}

// Same geometry, but with the space glyph the producer actually
// emitted. Now it is two words and must not be merged.
withSpace := []Char{
glyph(",", 110, 116),
glyph(" ", 116, 118.5),
glyph("e", 118.52, 126),
}
if boundarySplitsToken(withSpace, left, right, 3) {
t.Error("an explicit space across the boundary means two words — must not merge")
}
}

// TestMergeSplitTokensKeepsTableRectangular is the property that makes
// the feature safe to use on a real table.
//
Expand Down Expand Up @@ -155,7 +189,7 @@ func TestMergeSplitTokensKeepsTableRectangular(t *testing.T) {
}
// The boundary split a token in row 0, so it goes for the whole table —
// row 1 merges too, keeping the grid rectangular.
want := [][]string{{"Dec31,"}, {"A9"}}
want := [][]string{{"Dec31,"}, {"A 9"}}
if !reflect.DeepEqual(gotRows, want) {
t.Errorf("rows = %q, want %q", gotRows, want)
}
Expand Down
Loading
Loading