Skip to content

Commit db327d3

Browse files
authored
@ (#53)
Fix empty section content + surface richer citations PDF sectioning: multi-line display headings (a hero title wrapped across several lines, e.g. "THE SYSTEM, END-TO-END" / "How Vectorless" / "actually works.") were each detected as a separate heading, producing "purely structural" leaf Sections with empty Content. Those sections returned empty content to the section browser and /v1/sections/{id}. Two-layer fix in pkg/parser/pdf.go: - Merge consecutive same-level heading rows on the same page with no body between them into one heading, so the following body attaches to the whole title (root cause). - foldEmptyLeafSections: a lossless post-parse pass that removes any remaining empty-content leaf and carries its title onto the sibling body it heads (bold markdown heading line), unioning page ranges. Internal nodes (empty headings with real children) are untouched. Citations: raise defaultTreeWalkMaxCitations 3 -> 6. The cap is a ceiling that only bites on over-emission; the prompt + confidence gating still drive the minimal sufficient set, so precision-critical single-pick answers are unchanged, while genuinely multi-location answers now surface every location they rely on instead of being truncated at three. @
1 parent 8d074fa commit db327d3

6 files changed

Lines changed: 257 additions & 23 deletions

File tree

internal/config/config_treewalk_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ func TestForwardTreeWalkMaxCitations(t *testing.T) {
1818
os.Setenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS", prevVLE)
1919
}()
2020

21-
// Engine default is 3 with no env set.
21+
// Engine default is 6 with no env set.
2222
os.Unsetenv("VLS_RETRIEVAL_TREEWALK_MAX_CITATIONS")
2323
os.Unsetenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS")
2424
cfg := Default()
2525
applyEnvOverrides(&cfg)
26-
if cfg.Engine.Retrieval.TreeWalk.MaxCitations != 3 {
27-
t.Errorf("default max_citations = %d, want 3", cfg.Engine.Retrieval.TreeWalk.MaxCitations)
26+
if cfg.Engine.Retrieval.TreeWalk.MaxCitations != 6 {
27+
t.Errorf("default max_citations = %d, want 6", cfg.Engine.Retrieval.TreeWalk.MaxCitations)
2828
}
2929

3030
// VLE_ alone forwards through.
@@ -43,12 +43,12 @@ func TestForwardTreeWalkMaxCitations(t *testing.T) {
4343
t.Errorf("VLS_ should win: max_citations = %d, want 1", cfg3.Engine.Retrieval.TreeWalk.MaxCitations)
4444
}
4545

46-
// Garbled value preserves the engine default (3), does not zero it.
46+
// Garbled value preserves the engine default (6), does not zero it.
4747
os.Unsetenv("VLS_RETRIEVAL_TREEWALK_MAX_CITATIONS")
4848
os.Setenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS", "heaps")
4949
cfg4 := Default()
5050
applyEnvOverrides(&cfg4)
51-
if cfg4.Engine.Retrieval.TreeWalk.MaxCitations != 3 {
52-
t.Errorf("garbage value should preserve default 3, got %d", cfg4.Engine.Retrieval.TreeWalk.MaxCitations)
51+
if cfg4.Engine.Retrieval.TreeWalk.MaxCitations != 6 {
52+
t.Errorf("garbage value should preserve default 6, got %d", cfg4.Engine.Retrieval.TreeWalk.MaxCitations)
5353
}
5454
}

pkg/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ func Default() Config {
760760
Enabled: true,
761761
MaxHops: 8,
762762
PageContentLimit: 16000,
763-
MaxCitations: 3,
763+
MaxCitations: 6,
764764
},
765765
},
766766
Ingest: IngestConfig{

pkg/config/config_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -684,8 +684,8 @@ func TestTreeWalkDefaults(t *testing.T) {
684684
if cfg.Retrieval.TreeWalk.PageContentLimit != 16000 {
685685
t.Errorf("page_content_limit = %d, want 16000", cfg.Retrieval.TreeWalk.PageContentLimit)
686686
}
687-
if cfg.Retrieval.TreeWalk.MaxCitations != 3 {
688-
t.Errorf("max_citations = %d, want 3", cfg.Retrieval.TreeWalk.MaxCitations)
687+
if cfg.Retrieval.TreeWalk.MaxCitations != 6 {
688+
t.Errorf("max_citations = %d, want 6", cfg.Retrieval.TreeWalk.MaxCitations)
689689
}
690690
if cfg.Retrieval.TreeWalk.Model != "" {
691691
t.Errorf("model default should be empty (inherit), got %q", cfg.Retrieval.TreeWalk.Model)
@@ -802,8 +802,8 @@ func TestTreeWalkEnvOverrideRejectsBad(t *testing.T) {
802802
if cfg.Retrieval.TreeWalk.PageContentLimit != 16000 {
803803
t.Errorf("garbage page_content_limit env should preserve default, got %d", cfg.Retrieval.TreeWalk.PageContentLimit)
804804
}
805-
if cfg.Retrieval.TreeWalk.MaxCitations != 3 {
806-
t.Errorf("garbage max_citations env should preserve default 3, got %d", cfg.Retrieval.TreeWalk.MaxCitations)
805+
if cfg.Retrieval.TreeWalk.MaxCitations != 6 {
806+
t.Errorf("garbage max_citations env should preserve default 6, got %d", cfg.Retrieval.TreeWalk.MaxCitations)
807807
}
808808
}
809809

pkg/parser/empty_sections_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package parser
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
// TestFoldEmptyLeafSections proves the post-parse pass eliminates empty
9+
// "structural" leaf fragments — the multi-line display-title remnants and
10+
// running headers that otherwise ship as sections returning empty content
11+
// to the browser / API — while preserving their title text on the sibling
12+
// body they head, and never touching internal nodes (empty headings that
13+
// carry real sub-sections).
14+
func TestFoldEmptyLeafSections(t *testing.T) {
15+
t.Parallel()
16+
17+
t.Run("empty leaf folds its title into the following sibling", func(t *testing.T) {
18+
in := []Section{
19+
{Level: 1, Title: "How Vectorless", Content: "", PageStart: 3, PageEnd: 3},
20+
{Level: 1, Title: "Body", Content: "Vectorless walks the document tree.", PageStart: 3, PageEnd: 4},
21+
}
22+
out := foldEmptyLeafSections(in)
23+
if len(out) != 1 {
24+
t.Fatalf("want 1 section (empty leaf folded away), got %d: %+v", len(out), out)
25+
}
26+
if strings.Contains(out[0].Content, "walks the document tree") == false {
27+
t.Errorf("body content lost: %q", out[0].Content)
28+
}
29+
if !strings.Contains(out[0].Content, "How Vectorless") {
30+
t.Errorf("folded title not preserved in content: %q", out[0].Content)
31+
}
32+
if out[0].PageStart != 3 {
33+
t.Errorf("PageStart should union to 3, got %d", out[0].PageStart)
34+
}
35+
})
36+
37+
t.Run("consecutive empty fragments accumulate onto one body", func(t *testing.T) {
38+
secs := []Section{
39+
{Level: 1, Title: "THE SYSTEM, END-TO-END", Content: "", PageStart: 2, PageEnd: 2},
40+
{Level: 1, Title: "How Vectorless", Content: "", PageStart: 2, PageEnd: 2},
41+
{Level: 1, Title: "actually works.", Content: "", PageStart: 2, PageEnd: 2},
42+
{Level: 1, Title: "Intro", Content: "The system reads structure first.", PageStart: 2, PageEnd: 3},
43+
}
44+
out := foldEmptyLeafSections(secs)
45+
if len(out) != 1 {
46+
t.Fatalf("want 1 section, got %d: %+v", len(out), out)
47+
}
48+
for _, frag := range []string{"THE SYSTEM", "How Vectorless", "actually works", "reads structure first"} {
49+
if !strings.Contains(out[0].Content, frag) {
50+
t.Errorf("fragment %q missing from folded content: %q", frag, out[0].Content)
51+
}
52+
}
53+
})
54+
55+
t.Run("empty internal node (heading with children) is preserved", func(t *testing.T) {
56+
in := []Section{
57+
{Level: 1, Title: "Chapter", Content: "", Children: []Section{
58+
{Level: 2, Title: "Sub", Content: "real body", PageStart: 5, PageEnd: 5},
59+
}},
60+
}
61+
out := foldEmptyLeafSections(in)
62+
if len(out) != 1 || len(out[0].Children) != 1 {
63+
t.Fatalf("internal node must survive with its child, got %+v", out)
64+
}
65+
})
66+
67+
t.Run("trailing empty fragment with no following body attaches to previous", func(t *testing.T) {
68+
in := []Section{
69+
{Level: 1, Title: "Body", Content: "content here", PageStart: 1, PageEnd: 1},
70+
{Level: 1, Title: "Dangling Header", Content: "", PageStart: 2, PageEnd: 2},
71+
}
72+
out := foldEmptyLeafSections(in)
73+
if len(out) != 1 {
74+
t.Fatalf("want 1 section, got %d", len(out))
75+
}
76+
if !strings.Contains(out[0].Content, "Dangling Header") {
77+
t.Errorf("trailing title lost: %q", out[0].Content)
78+
}
79+
})
80+
81+
t.Run("lone empty leaf with no siblings is kept (nothing to fold into)", func(t *testing.T) {
82+
in := []Section{{Level: 1, Title: "Only", Content: ""}}
83+
out := foldEmptyLeafSections(in)
84+
if len(out) != 1 {
85+
t.Fatalf("lone empty leaf should be kept, got %d", len(out))
86+
}
87+
})
88+
}

pkg/parser/pdf.go

Lines changed: 141 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,10 +424,32 @@ func (p *PDF) parseDoc(_ context.Context, buf []byte) (*ParsedDoc, error) {
424424
// depth relative to the font-derived level. We only ever DEEPEN
425425
// (never change the base level), so top-level headings — numbered
426426
// "3" or not ("Abstract") — stay siblings at the same font level.
427+
newLevel := lvl
427428
if nd, ok := numberedHeadingDepth(text); ok && nd > 1 {
428-
lvl += nd - 1
429+
newLevel += nd - 1
429430
}
430-
current = &flat{level: lvl, title: text}
431+
// Multi-line display heading: a run of heading rows at the SAME
432+
// level, on the SAME page, with NO body text accrued between them
433+
// is one visual heading wrapped across lines — e.g. a hero title
434+
// "THE SYSTEM, END-TO-END" / "How Vectorless" / "actually works."
435+
// Left un-merged, each line becomes its own heading flat whose
436+
// body is empty, shipping as a "purely structural" Section that
437+
// returns empty content to the section browser / API. Fold the
438+
// continuation line into the current heading instead so the body
439+
// that follows attaches to the whole title and no empty fragment
440+
// section is emitted. We only merge when the current flat is
441+
// itself an as-yet-empty heading at the same level on the same
442+
// page — never across a level change (real nesting) or once body
443+
// text has begun.
444+
if current.level > 0 && newLevel == current.level &&
445+
current.title != "" &&
446+
strings.TrimSpace(current.body.String()) == "" &&
447+
(current.pageEnd == 0 || row.page == 0 || row.page == current.pageEnd) {
448+
current.title = strings.TrimSpace(current.title + " " + text)
449+
touch(current, row.page)
450+
continue
451+
}
452+
current = &flat{level: newLevel, title: text}
431453
touch(current, row.page)
432454
flats = append(flats, current)
433455
continue
@@ -509,6 +531,11 @@ func (p *PDF) parseDoc(_ context.Context, buf []byte) (*ParsedDoc, error) {
509531
}}
510532
}
511533

534+
// Drop empty "structural" leaf fragments (multi-line title remnants,
535+
// running headers) so no section returns empty content to the browser /
536+
// API; their titles ride onto the sibling body they head.
537+
rootSec.Children = foldEmptyLeafSections(rootSec.Children)
538+
512539
// Internal sections inherit the union of their children's page ranges
513540
// so callers reading the outline can still cite a page span.
514541
propagateSectionPages(rootSec.Children)
@@ -550,6 +577,115 @@ func (p *PDF) resolvedParseTimeout() time.Duration {
550577
return p.ParseTimeout
551578
}
552579

580+
// foldEmptyLeafSections eliminates leaf sections (no children) whose
581+
// content is empty. A PDF whose heading detector fires on a fragment that
582+
// carries no body — a lone display-title line the row-merge didn't catch,
583+
// a repeated running header, a stray bold word — otherwise ships as a
584+
// "purely structural" Section that returns empty content to the section
585+
// browser and the /v1/sections/{id} API, which reads as a broken/empty
586+
// part of the document.
587+
//
588+
// The fold is lossless: an empty leaf's TITLE is preserved by attaching it
589+
// as a bold markdown heading line to the content of the sibling it heads —
590+
// the NEXT sibling when one exists (the body it introduces), else the
591+
// PREVIOUS sibling. Consecutive empty leaves accumulate and flush together
592+
// into the first sibling that can carry them, so a multi-line title that
593+
// escaped the row-merge still lands as one heading block above its body.
594+
// Page ranges union so citations stay correct. Internal nodes (sections
595+
// WITH children) are recursed into but never dropped — an empty heading
596+
// with real sub-sections is legitimate structure whose content lives in
597+
// its children.
598+
//
599+
// A lone empty leaf with no siblings to carry its title is kept as-is
600+
// (dropping it would erase the only trace of that heading); this is the
601+
// rare degenerate case and harms nothing.
602+
func foldEmptyLeafSections(sections []Section) []Section {
603+
for i := range sections {
604+
if len(sections[i].Children) > 0 {
605+
sections[i].Children = foldEmptyLeafSections(sections[i].Children)
606+
}
607+
}
608+
609+
// Collect the indices of empty leaves eligible to fold. A section is an
610+
// empty leaf when it has no children and no non-whitespace content.
611+
emptyLeaf := func(s *Section) bool {
612+
return len(s.Children) == 0 && strings.TrimSpace(s.Content) == ""
613+
}
614+
// Nothing to do unless there is at least one empty leaf AND at least one
615+
// sibling that can carry a folded title.
616+
empties, carriers := 0, 0
617+
for i := range sections {
618+
if emptyLeaf(&sections[i]) {
619+
empties++
620+
} else {
621+
carriers++
622+
}
623+
}
624+
if empties == 0 || carriers == 0 {
625+
return sections
626+
}
627+
628+
out := make([]Section, 0, len(sections))
629+
var pendingTitles []string
630+
var pendStart, pendEnd int
631+
takePending := func(target *Section) {
632+
if len(pendingTitles) == 0 {
633+
return
634+
}
635+
var b strings.Builder
636+
for _, t := range pendingTitles {
637+
b.WriteString("**")
638+
b.WriteString(t)
639+
b.WriteString("**\n\n")
640+
}
641+
b.WriteString(target.Content)
642+
target.Content = strings.TrimSpace(b.String())
643+
target.PageStart = minNonZero(pendStart, target.PageStart)
644+
if pendEnd > target.PageEnd {
645+
target.PageEnd = pendEnd
646+
}
647+
pendingTitles = nil
648+
pendStart, pendEnd = 0, 0
649+
}
650+
651+
for i := range sections {
652+
s := sections[i]
653+
if emptyLeaf(&s) {
654+
if t := strings.TrimSpace(s.Title); t != "" {
655+
pendingTitles = append(pendingTitles, t)
656+
pendStart = minNonZero(pendStart, s.PageStart)
657+
if s.PageEnd > pendEnd {
658+
pendEnd = s.PageEnd
659+
}
660+
}
661+
continue // drop the empty leaf; its title rides on pendingTitles
662+
}
663+
// A real section: it carries any pending titles as heading prefix
664+
// (these are the fragments it follows).
665+
takePending(&s)
666+
out = append(out, s)
667+
}
668+
669+
// Any titles still pending had no following carrier — attach them to the
670+
// last emitted sibling as a trailing heading block so nothing is lost.
671+
if len(pendingTitles) > 0 && len(out) > 0 {
672+
last := &out[len(out)-1]
673+
var b strings.Builder
674+
b.WriteString(last.Content)
675+
for _, t := range pendingTitles {
676+
b.WriteString("\n\n**")
677+
b.WriteString(t)
678+
b.WriteString("**")
679+
}
680+
last.Content = strings.TrimSpace(b.String())
681+
if pendEnd > last.PageEnd {
682+
last.PageEnd = pendEnd
683+
}
684+
last.PageStart = minNonZero(pendStart, last.PageStart)
685+
}
686+
return out
687+
}
688+
553689
// propagateSectionPages fills internal-node PageStart/PageEnd from the union
554690
// of descendant leaf ranges where the internal node didn't have its own
555691
// (because its body was empty / hoisted into children). Leaves keep their
@@ -1268,7 +1404,9 @@ func parsePDFWithOutline(outline pdflib.Outline, rows []pdfRow) (*ParsedDoc, boo
12681404
title = rootSec.Children[0].Title
12691405
}
12701406

1271-
// Propagate page ranges so internal nodes span their children.
1407+
// Fold empty leaf fragments, then propagate page ranges so internal
1408+
// nodes span their children.
1409+
rootSec.Children = foldEmptyLeafSections(rootSec.Children)
12721410
propagateSectionPages(rootSec.Children)
12731411

12741412
return &ParsedDoc{

pkg/retrieval/treewalk_strategy.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,23 @@ const defaultTreeWalkMaxHops = 8
140140
// excerpt. Matches TreeWalk's reference behaviour.
141141
const defaultPageContentLimit = 16000
142142

143-
// defaultTreeWalkMaxCitations bounds the FINAL cited-range set. Three
144-
// is generous for the answer-spans-one-place common case (where ONE is
145-
// ideal) while still allowing a genuinely multi-location answer (e.g. a
146-
// 10-K figure cross-referenced between the income statement and a
147-
// footnote) to cite two or three distinct ranges. The FinanceBench
148-
// signal that motivated the cap: confident single-pick = f1 1.0,
149-
// 5-range spray = f1 0. Capping at 3 keeps the legitimate multi-range
150-
// case while removing the long tail of low-confidence noise.
151-
const defaultTreeWalkMaxCitations = 3
143+
// defaultTreeWalkMaxCitations bounds the FINAL cited-range set. The cap
144+
// is a CEILING, not a target: it only bites when the model emits MORE
145+
// distinct ranges than this. The prompt still drives the model to the
146+
// minimal sufficient set (usually one), so a confident single-pick answer
147+
// still yields ONE citation and the FinanceBench precision behaviour is
148+
// unchanged — the signal that motivated a cap at all (confident single =
149+
// f1 1.0, 5-range spray = f1 0) is enforced by the prompt + confidence
150+
// gating, not by clamping legitimate multi-location answers.
151+
//
152+
// It is raised from 3 to 6 so richer, genuinely multi-location answers —
153+
// a synthesis question over a whitepaper that draws on the intro, a
154+
// method section and a results table; a comparison spanning several parts
155+
// of a report — surface every location they actually rely on rather than
156+
// being truncated to three. Six is still well below the "spray" regime the
157+
// prompt/confidence machinery suppresses, so precision-critical single-pick
158+
// answers are untouched while multi-source answers become fully traceable.
159+
const defaultTreeWalkMaxCitations = 6
152160

153161
// strategyNameTreeWalk is the stable identifier for config
154162
// (retrieval.strategy: treewalk) and telemetry.

0 commit comments

Comments
 (0)