parser: detect bold-as-heading + collapse letter-spacing (fixes filing parse)#12
Merged
Conversation
SEC filings have no PDF outline and use bold at body font size (not larger
fonts) for section headings, so the size-only heading heuristic missed every
real section and collapsed the entire body into one giant block. Wide
letter-tracking on cover/header rows also extracted as "U N I T E D".
Three targeted changes:
- Per-row bold detection from the glyph font name. Bold rows at >= median
font size qualify as headings, nested one level below the smallest
size-derived heading.
- collapseLetterSpacing(): rejoins letter-tracked text only on rows whose
pattern is unmistakable (majority single-char tokens), preserving word
boundaries via runs of 2+ spaces. Normal prose is untouched.
- looksLikeHeading: raise the word cap from 14 to 25 so verbose filing
headings ("Item 2. Management's Discussion and Analysis of Financial
Condition and Results of Operations") are not filtered out.
Validated on a real 10-Q (3M Q2 2023, 92 pages): one 680K-char blob became
174 retrievable sections (Item 1, Consolidated Balance Sheet, PART I, ...);
title "U N I T E D S T A T E S" became "UNITED STATES". All existing parser
tests pass; no regression.
Reviewer's GuideAdds bold-weight-aware heading detection and robust letter-spacing collapsing to the PDF parser so SEC filings without outlines are split into sensible sections instead of a single blob, while expanding the heading heuristic to allow longer titles. Flow diagram for bold-aware heading detection and letter-spacing collapsing in PDF parserflowchart TD
subgraph Extraction[Row extraction]
A[extractPDFRows] --> B[Iterate glyphs in block]
B --> C[Build text with spacing]
C --> D[collapseLetterSpacing]
B --> E[Count boldGlyphs using isBoldFont]
D --> F[Create pdfRow.text]
E --> G[Set pdfRow.bold]
end
subgraph HeadingDetection[Heading detection in Parse]
H[Parse] --> I[buildHeadingLevelMap]
I --> J[Compute boldLevel from levelForSize]
J --> K[Iterate pdfRow]
K --> L[Lookup lvl,isHeading from levelForSize]
L --> M{!isHeading
row.bold
row.fontSize >= median
looksLikeHeading}
M -->|true| N[Set isHeading=true
lvl=boldLevel]
M -->|false| O[Keep original lvl,isHeading]
N --> P{isHeading
looksLikeHeading}
O --> P
P -->|true| Q[Adjust level via numberedHeadingDepth]
end
F --> K
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds bold glyph tracking and letter-spaced normalization to PDF parsing, promotes bold rows to headings and splits oversized leaf sections into word-boundary chunks; introduces a selection LLM retry helper that aggregates usage and degrades to empty selection on persistent parse failures; updates ingest summarization prompts. ChangesBold Typography Heading Detection & Chunking
Selection LLM Retry and Integration
Ingest summarization prompts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR improves PDF parsing for SEC filings by enhancing heading detection (to handle bold, body-sized section headers) and by collapsing artificial letter-spacing in extracted rows to restore normal words.
Changes:
- Add bold-row heading detection (based on glyph font names) and assign bold headings a level nested below size-derived headings (capped at level 6).
- Collapse wide letter-tracking in extracted rows (e.g.,
"U N I T E D S T A T E S"→"UNITED STATES") while aiming to preserve word boundaries. - Relax
looksLikeHeadingword-count cap from 14 to 25 to allow verbose filing headings.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
150
to
155
| lvl, isHeading := levelForSize[roundSize(row.fontSize)] | ||
| if !isHeading && row.bold && row.fontSize >= median && looksLikeHeading(text) { | ||
| isHeading = true | ||
| lvl = boldLevel | ||
| } | ||
| if isHeading && looksLikeHeading(text) { |
Comment on lines
+624
to
+626
| // runs of 2+ spaces; within each word the single spaces between solitary glyphs | ||
| // are removed ("F O R M 1 0 - Q" → "FORM 10-Q"). Rows that aren't | ||
| // letter-spaced are returned unchanged, so normal prose is never touched. |
Comment on lines
+615
to
+619
| for _, t := range toks { | ||
| if len([]rune(t)) == 1 { | ||
| single++ | ||
| } | ||
| } |
Comment on lines
+635
to
+639
| for _, p := range parts { | ||
| if len([]rune(p)) > 1 { | ||
| allSingle = false | ||
| break | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@pkg/parser/pdf.go`:
- Around line 582-587: The PDF struct's documentation still claims headings are
"<= 14 words" but the code in function that parses headings (uses strings.Fields
and checks len(words) > 25) treats headings as up to 25 words; update the PDF
struct doc comment to reflect the current behavior (change the "<= 14 words"
phrase to "<= 25 words" or reword to match the implemented 25-word cap) so
documentation and the PDF struct comment are consistent with the heading
detection logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+582
to
+587
| // Headings are rarely > 25 words and never end with sentence punctuation | ||
| // from the middle of a paragraph. (Filing headings like "Item 2. | ||
| // Management's Discussion and Analysis of Financial Condition and Results | ||
| // of Operations" run long, so the cap is generous.) | ||
| words := strings.Fields(s) | ||
| if len(words) == 0 || len(words) > 14 { | ||
| if len(words) == 0 || len(words) > 25 { |
There was a problem hiding this comment.
Documentation mismatch: struct comment still says 14 words.
Line 26 in the PDF struct doc comment states <= 14 words but the implementation now uses 25. Update the doc comment to stay in sync.
Suggested fix
-// 3. Treat any row whose font size exceeds a threshold (1.2× median)
-// AND that is short (<= 14 words) as a heading candidate.
+// 3. Treat any row whose font size exceeds a threshold (1.2× median)
+// AND that is short (<= 25 words) as a heading candidate.🤖 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 `@pkg/parser/pdf.go` around lines 582 - 587, The PDF struct's documentation
still claims headings are "<= 14 words" but the code in function that parses
headings (uses strings.Fields and checks len(words) > 25) treats headings as up
to 25 words; update the PDF struct doc comment to reflect the current behavior
(change the "<= 14 words" phrase to "<= 25 words" or reword to match the
implemented 25-word cap) so documentation and the PDF struct comment are
consistent with the heading detection logic.
…lures The selection LLM call (chunked-tree slices and single-pass alike) sometimes returns plain text instead of the JSON the schema asks for. Most often this is Gemini briefly ignoring JSON mode. Today that surfaces as a 500 to the SDK on every blip, plus the wasted LLM cost — and the SDK's transport-level retry just repeats the same blow-up. Wrap Complete + ParseSelection in a small retry loop (2 retries by default, 3 attempts total). On retry the last user message gets an extra "ONLY JSON, no prose, no fences" reminder, which Gemini usually honors on the second try. If all attempts still fail, log a warning and return an empty selection so the HTTP request succeeds with no sections instead of erroring out — one bad LLM response can no longer take down a multi-slice retrieval. Test TestSinglePassGracefulOnNonJSON locks the behaviour: prose-only response → empty selection, nil error, 3 LLM attempts counted in usage.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/retrieval/chunked_tree.go (1)
112-115: 💤 Low valueConsider setting
ModelUsedfor consistency withSinglePassandCached.
SinglePass.SelectWithCostandCached.SelectWithCostboth populateModelUsed: budget.ModelName, butChunkedTreeleaves it empty. Downstream consumers (e.g., the query handler) may expect this field to be populated for usage reporting.Suggested diff
return &Result{ SelectedIDs: c.Merge.Merge(allIDs), + ModelUsed: budget.ModelName, Usage: totalUsage, }, nil🤖 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 `@pkg/retrieval/chunked_tree.go` around lines 112 - 115, The Result returned from ChunkedTree.SelectWithCost doesn't set ModelUsed; update the return struct in the ChunkedTree.SelectWithCost implementation to include ModelUsed: budget.ModelName (matching SinglePass.SelectWithCost and Cached.SelectWithCost) so downstream consumers get the model name for usage reporting.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@pkg/retrieval/chunked_tree.go`:
- Around line 112-115: The Result returned from ChunkedTree.SelectWithCost
doesn't set ModelUsed; update the return struct in the
ChunkedTree.SelectWithCost implementation to include ModelUsed: budget.ModelName
(matching SinglePass.SelectWithCost and Cached.SelectWithCost) so downstream
consumers get the model name for usage reporting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10de44d1-b73a-4980-aa20-e83989e1c5c1
📒 Files selected for processing (3)
pkg/retrieval/chunked_tree.gopkg/retrieval/retrieval_test.gopkg/retrieval/single_pass.go
The current summary prompt asks for "a single factual sentence" — fine for
human reading, but the resulting summaries describe sections generically
("Cover page of 3M's 10-Q with company identification") instead of naming
their concrete topics ("registered debt securities, trading symbols MMM26
/ MMM30, NYSE listings, IRS employer ID"). The downstream retrieval LLM,
given only those summaries, then can't tell which section answers a
specific question — e.g. q_00941 ("Which debt securities are registered to
trade on a national exchange under 3M's name?") picks two "Long-Term Debt"
sections instead of the cover-page section that actually contains the
registration table.
Rewrite the summary prompt for retrieval: explicitly ask the model to name
the section's concrete entities, identifiers, table contents, named items,
and key numbers. One sentence, raised cap to ≤60 words (with MaxTokens
260) so dense sections aren't truncated mid-list. The domain framings
(research / medical / default) are preserved and now include the same
retrieval rule. Existing ingest tests pass.
…evable
Filing cover pages (and any other long, mixed-topic leaf section) produce
one 2-3k-char blob under a generic title like "3M COMPANY" — mixing
registration tables, addresses, IRS IDs, contact info. A single summary
can't cover all those topics, so retrieval picks unrelated "long-term
debt" sections instead of the one that actually holds the answer.
Add chunkOversizedLeaves: any LEAF section whose Content exceeds 2400
chars is replaced by a parent (title preserved) with smaller children at
the next level. Children are sized around 900 chars and split at word
boundaries. The chunk title prefers a natural colon-terminated header
within the first 80 chars ("Securities registered pursuant to Section
12(b) of the Act:") when available — exactly the pattern in filings —
otherwise the first ~60 chars trimmed at a word boundary, falling back
to "<parent title> — part N".
Internal nodes are recursed into but never split (they're already
structured). Threshold deliberately high (2400) so most paper sub-
sections aren't affected; combined with the retrieval-friendly summary
prompt (previous commit), each chunk gets a topic-rich summary downstream
so the retrieval LLM can match it to specific questions.
Tests in chunk_test.go: oversized leaf gets split with the parent title
preserved + children at level+1; first chunk takes the colon-header
title; small sections are untouched; oversized leaves nested inside
internal nodes are still split.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/parser/pdf.go (2)
583-586: 💤 Low valueConsider applying chunking to the outline-based path for consistency.
The outline-based parsing path returns sections without
chunkOversizedLeaves, while the heuristic path (line 233) applies it. Documents with outlines could still have oversized leaf sections (e.g., a long "Appendix" with no sub-bookmarks). Applying the same post-processing would ensure uniform behavior.Suggested fix
return &ParsedDoc{ Title: title, - Sections: rootSec.Children, + Sections: chunkOversizedLeaves(rootSec.Children), }, true🤖 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 `@pkg/parser/pdf.go` around lines 583 - 586, The outline-based return in the PDF parser currently returns Sections directly (rootSec.Children) without applying chunkOversizedLeaves, causing inconsistent handling versus the heuristic path; modify the outline-based path that returns &ParsedDoc{Title: title, Sections: rootSec.Children} to first call chunkOversizedLeaves(rootSec) (or the same helper used on the heuristic path) and use the processed children when constructing ParsedDoc so oversized leaf sections are chunked consistently.
270-278: 💤 Low valueChild level may exceed the document-wide cap of 6.
When
s.Levelis already 6 (the max used elsewhere in heading detection), the children receiveLevel: 7. Other parts of the parser clamp levels to 6 (e.g.,buildHeadingLevelMap,numberedHeadingDepth). For consistency:Suggested fix
for i, piece := range pieces { fallback := fmt.Sprintf("%s — part %d", s.Title, i+1) + childLevel := s.Level + 1 + if childLevel > 6 { + childLevel = 6 + } parent.Children = append(parent.Children, Section{ - Level: s.Level + 1, + Level: childLevel, Title: deriveChunkTitle(piece, fallback), Content: piece, }) }🤖 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 `@pkg/parser/pdf.go` around lines 270 - 278, Child sections created from pieces can exceed the document cap of 6 (when s.Level == 6), so clamp the child Level to a maximum of 6 when appending to parent.Children. In the loop that builds children (referencing Section, s.Level, deriveChunkTitle, pieces and parent.Children), compute childLevel := min(s.Level+1, 6) (or otherwise clamp to 6) and set Level: childLevel for each appended Section so child headings never go above level 6.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@pkg/parser/pdf.go`:
- Around line 583-586: The outline-based return in the PDF parser currently
returns Sections directly (rootSec.Children) without applying
chunkOversizedLeaves, causing inconsistent handling versus the heuristic path;
modify the outline-based path that returns &ParsedDoc{Title: title, Sections:
rootSec.Children} to first call chunkOversizedLeaves(rootSec) (or the same
helper used on the heuristic path) and use the processed children when
constructing ParsedDoc so oversized leaf sections are chunked consistently.
- Around line 270-278: Child sections created from pieces can exceed the
document cap of 6 (when s.Level == 6), so clamp the child Level to a maximum of
6 when appending to parent.Children. In the loop that builds children
(referencing Section, s.Level, deriveChunkTitle, pieces and parent.Children),
compute childLevel := min(s.Level+1, 6) (or otherwise clamp to 6) and set Level:
childLevel for each appended Section so child headings never go above level 6.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78884a71-4a36-4414-90d9-621bf32250fb
📒 Files selected for processing (3)
pkg/ingest/ingest.gopkg/parser/chunk_test.gopkg/parser/pdf.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pkg/parser/pdf.go: detect bold-as-heading + collapse letter-spacing
SEC filings have no PDF outline and use bold at body font size (not larger
fonts) for section headings, so the size-only heading heuristic missed every
real section and collapsed the entire body into one giant block. Wide
letter-tracking on cover/header rows also extracted as "U N I T E D".
Three targeted changes:
Per-row bold detection from the glyph font name. Bold rows at >= median
font size qualify as headings, nested one level below the smallest
size-derived heading.
collapseLetterSpacing(): rejoins letter-tracked text only on rows whose
pattern is unmistakable (majority single-char tokens), preserving word
boundaries via runs of 2+ spaces. Normal prose is untouched.
looksLikeHeading: raise the word cap from 14 to 25 so verbose filing
headings ("Item 2. Management's Discussion and Analysis of Financial
Condition and Results of Operations") are not filtered out.
Validated on a real 10-Q (3M Q2 2023, 92 pages): one 680K-char blob became
174 retrievable sections (Item 1, Consolidated Balance Sheet, PART I, ...);
title "U N I T E D S T A T E S" became "UNITED STATES". All existing parser
tests pass; no regression.
Summary by Sourcery
Improve PDF filing parsing by treating bold body-size text as headings and collapsing artificial letter spacing to recover proper section structure and titles.
New Features:
Enhancements:
Summary by CodeRabbit