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
12 changes: 6 additions & 6 deletions internal/config/config_treewalk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ func TestForwardTreeWalkMaxCitations(t *testing.T) {
os.Setenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS", prevVLE)
}()

// Engine default is 3 with no env set.
// Engine default is 6 with no env set.
os.Unsetenv("VLS_RETRIEVAL_TREEWALK_MAX_CITATIONS")
os.Unsetenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS")
cfg := Default()
applyEnvOverrides(&cfg)
if cfg.Engine.Retrieval.TreeWalk.MaxCitations != 3 {
t.Errorf("default max_citations = %d, want 3", cfg.Engine.Retrieval.TreeWalk.MaxCitations)
if cfg.Engine.Retrieval.TreeWalk.MaxCitations != 6 {
t.Errorf("default max_citations = %d, want 6", cfg.Engine.Retrieval.TreeWalk.MaxCitations)
}

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

// Garbled value preserves the engine default (3), does not zero it.
// Garbled value preserves the engine default (6), does not zero it.
os.Unsetenv("VLS_RETRIEVAL_TREEWALK_MAX_CITATIONS")
os.Setenv("VLE_RETRIEVAL_TREEWALK_MAX_CITATIONS", "heaps")
cfg4 := Default()
applyEnvOverrides(&cfg4)
if cfg4.Engine.Retrieval.TreeWalk.MaxCitations != 3 {
t.Errorf("garbage value should preserve default 3, got %d", cfg4.Engine.Retrieval.TreeWalk.MaxCitations)
if cfg4.Engine.Retrieval.TreeWalk.MaxCitations != 6 {
t.Errorf("garbage value should preserve default 6, got %d", cfg4.Engine.Retrieval.TreeWalk.MaxCitations)
}
}
2 changes: 1 addition & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ func Default() Config {
Enabled: true,
MaxHops: 8,
PageContentLimit: 16000,
MaxCitations: 3,
MaxCitations: 6,
},
},
Ingest: IngestConfig{
Expand Down
8 changes: 4 additions & 4 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,8 +684,8 @@ func TestTreeWalkDefaults(t *testing.T) {
if cfg.Retrieval.TreeWalk.PageContentLimit != 16000 {
t.Errorf("page_content_limit = %d, want 16000", cfg.Retrieval.TreeWalk.PageContentLimit)
}
if cfg.Retrieval.TreeWalk.MaxCitations != 3 {
t.Errorf("max_citations = %d, want 3", cfg.Retrieval.TreeWalk.MaxCitations)
if cfg.Retrieval.TreeWalk.MaxCitations != 6 {
t.Errorf("max_citations = %d, want 6", cfg.Retrieval.TreeWalk.MaxCitations)
}
if cfg.Retrieval.TreeWalk.Model != "" {
t.Errorf("model default should be empty (inherit), got %q", cfg.Retrieval.TreeWalk.Model)
Expand Down Expand Up @@ -802,8 +802,8 @@ func TestTreeWalkEnvOverrideRejectsBad(t *testing.T) {
if cfg.Retrieval.TreeWalk.PageContentLimit != 16000 {
t.Errorf("garbage page_content_limit env should preserve default, got %d", cfg.Retrieval.TreeWalk.PageContentLimit)
}
if cfg.Retrieval.TreeWalk.MaxCitations != 3 {
t.Errorf("garbage max_citations env should preserve default 3, got %d", cfg.Retrieval.TreeWalk.MaxCitations)
if cfg.Retrieval.TreeWalk.MaxCitations != 6 {
t.Errorf("garbage max_citations env should preserve default 6, got %d", cfg.Retrieval.TreeWalk.MaxCitations)
}
}

Expand Down
88 changes: 88 additions & 0 deletions pkg/parser/empty_sections_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package parser

import (
"strings"
"testing"
)

// TestFoldEmptyLeafSections proves the post-parse pass eliminates empty
// "structural" leaf fragments — the multi-line display-title remnants and
// running headers that otherwise ship as sections returning empty content
// to the browser / API — while preserving their title text on the sibling
// body they head, and never touching internal nodes (empty headings that
// carry real sub-sections).
func TestFoldEmptyLeafSections(t *testing.T) {
t.Parallel()

t.Run("empty leaf folds its title into the following sibling", func(t *testing.T) {
in := []Section{
{Level: 1, Title: "How Vectorless", Content: "", PageStart: 3, PageEnd: 3},
{Level: 1, Title: "Body", Content: "Vectorless walks the document tree.", PageStart: 3, PageEnd: 4},
}
out := foldEmptyLeafSections(in)
if len(out) != 1 {
t.Fatalf("want 1 section (empty leaf folded away), got %d: %+v", len(out), out)
}
if strings.Contains(out[0].Content, "walks the document tree") == false {
t.Errorf("body content lost: %q", out[0].Content)
}
if !strings.Contains(out[0].Content, "How Vectorless") {
t.Errorf("folded title not preserved in content: %q", out[0].Content)
}
if out[0].PageStart != 3 {
t.Errorf("PageStart should union to 3, got %d", out[0].PageStart)
}
})

t.Run("consecutive empty fragments accumulate onto one body", func(t *testing.T) {
secs := []Section{
{Level: 1, Title: "THE SYSTEM, END-TO-END", Content: "", PageStart: 2, PageEnd: 2},
{Level: 1, Title: "How Vectorless", Content: "", PageStart: 2, PageEnd: 2},
{Level: 1, Title: "actually works.", Content: "", PageStart: 2, PageEnd: 2},
{Level: 1, Title: "Intro", Content: "The system reads structure first.", PageStart: 2, PageEnd: 3},
}
out := foldEmptyLeafSections(secs)
if len(out) != 1 {
t.Fatalf("want 1 section, got %d: %+v", len(out), out)
}
for _, frag := range []string{"THE SYSTEM", "How Vectorless", "actually works", "reads structure first"} {
if !strings.Contains(out[0].Content, frag) {
t.Errorf("fragment %q missing from folded content: %q", frag, out[0].Content)
}
}
})

t.Run("empty internal node (heading with children) is preserved", func(t *testing.T) {
in := []Section{
{Level: 1, Title: "Chapter", Content: "", Children: []Section{
{Level: 2, Title: "Sub", Content: "real body", PageStart: 5, PageEnd: 5},
}},
}
out := foldEmptyLeafSections(in)
if len(out) != 1 || len(out[0].Children) != 1 {
t.Fatalf("internal node must survive with its child, got %+v", out)
}
})

t.Run("trailing empty fragment with no following body attaches to previous", func(t *testing.T) {
in := []Section{
{Level: 1, Title: "Body", Content: "content here", PageStart: 1, PageEnd: 1},
{Level: 1, Title: "Dangling Header", Content: "", PageStart: 2, PageEnd: 2},
}
out := foldEmptyLeafSections(in)
if len(out) != 1 {
t.Fatalf("want 1 section, got %d", len(out))
}
if !strings.Contains(out[0].Content, "Dangling Header") {
t.Errorf("trailing title lost: %q", out[0].Content)
}
})

t.Run("lone empty leaf with no siblings is kept (nothing to fold into)", func(t *testing.T) {
in := []Section{{Level: 1, Title: "Only", Content: ""}}
out := foldEmptyLeafSections(in)
if len(out) != 1 {
t.Fatalf("lone empty leaf should be kept, got %d", len(out))
}
})
}
144 changes: 141 additions & 3 deletions pkg/parser/pdf.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,32 @@ func (p *PDF) parseDoc(_ context.Context, buf []byte) (*ParsedDoc, error) {
// depth relative to the font-derived level. We only ever DEEPEN
// (never change the base level), so top-level headings — numbered
// "3" or not ("Abstract") — stay siblings at the same font level.
newLevel := lvl
if nd, ok := numberedHeadingDepth(text); ok && nd > 1 {
lvl += nd - 1
newLevel += nd - 1
}
current = &flat{level: lvl, title: text}
// Multi-line display heading: a run of heading rows at the SAME
// level, on the SAME page, with NO body text accrued between them
// is one visual heading wrapped across lines — e.g. a hero title
// "THE SYSTEM, END-TO-END" / "How Vectorless" / "actually works."
// Left un-merged, each line becomes its own heading flat whose
// body is empty, shipping as a "purely structural" Section that
// returns empty content to the section browser / API. Fold the
// continuation line into the current heading instead so the body
// that follows attaches to the whole title and no empty fragment
// section is emitted. We only merge when the current flat is
// itself an as-yet-empty heading at the same level on the same
// page — never across a level change (real nesting) or once body
// text has begun.
if current.level > 0 && newLevel == current.level &&
current.title != "" &&
strings.TrimSpace(current.body.String()) == "" &&
(current.pageEnd == 0 || row.page == 0 || row.page == current.pageEnd) {
current.title = strings.TrimSpace(current.title + " " + text)
touch(current, row.page)
continue
}
current = &flat{level: newLevel, title: text}
touch(current, row.page)
flats = append(flats, current)
continue
Expand Down Expand Up @@ -509,6 +531,11 @@ func (p *PDF) parseDoc(_ context.Context, buf []byte) (*ParsedDoc, error) {
}}
}

// Drop empty "structural" leaf fragments (multi-line title remnants,
// running headers) so no section returns empty content to the browser /
// API; their titles ride onto the sibling body they head.
rootSec.Children = foldEmptyLeafSections(rootSec.Children)

// Internal sections inherit the union of their children's page ranges
// so callers reading the outline can still cite a page span.
propagateSectionPages(rootSec.Children)
Expand Down Expand Up @@ -550,6 +577,115 @@ func (p *PDF) resolvedParseTimeout() time.Duration {
return p.ParseTimeout
}

// foldEmptyLeafSections eliminates leaf sections (no children) whose
// content is empty. A PDF whose heading detector fires on a fragment that
// carries no body — a lone display-title line the row-merge didn't catch,
// a repeated running header, a stray bold word — otherwise ships as a
// "purely structural" Section that returns empty content to the section
// browser and the /v1/sections/{id} API, which reads as a broken/empty
// part of the document.
//
// The fold is lossless: an empty leaf's TITLE is preserved by attaching it
// as a bold markdown heading line to the content of the sibling it heads —
// the NEXT sibling when one exists (the body it introduces), else the
// PREVIOUS sibling. Consecutive empty leaves accumulate and flush together
// into the first sibling that can carry them, so a multi-line title that
// escaped the row-merge still lands as one heading block above its body.
// Page ranges union so citations stay correct. Internal nodes (sections
// WITH children) are recursed into but never dropped — an empty heading
// with real sub-sections is legitimate structure whose content lives in
// its children.
//
// A lone empty leaf with no siblings to carry its title is kept as-is
// (dropping it would erase the only trace of that heading); this is the
// rare degenerate case and harms nothing.
func foldEmptyLeafSections(sections []Section) []Section {
for i := range sections {
if len(sections[i].Children) > 0 {
sections[i].Children = foldEmptyLeafSections(sections[i].Children)
}
}

// Collect the indices of empty leaves eligible to fold. A section is an
// empty leaf when it has no children and no non-whitespace content.
emptyLeaf := func(s *Section) bool {
return len(s.Children) == 0 && strings.TrimSpace(s.Content) == ""
}
// Nothing to do unless there is at least one empty leaf AND at least one
// sibling that can carry a folded title.
empties, carriers := 0, 0
for i := range sections {
if emptyLeaf(&sections[i]) {
empties++
} else {
carriers++
}
}
if empties == 0 || carriers == 0 {
return sections
}

out := make([]Section, 0, len(sections))
var pendingTitles []string
var pendStart, pendEnd int
takePending := func(target *Section) {
if len(pendingTitles) == 0 {
return
}
var b strings.Builder
for _, t := range pendingTitles {
b.WriteString("**")
b.WriteString(t)
b.WriteString("**\n\n")
}
b.WriteString(target.Content)
target.Content = strings.TrimSpace(b.String())
target.PageStart = minNonZero(pendStart, target.PageStart)
if pendEnd > target.PageEnd {
target.PageEnd = pendEnd
}
pendingTitles = nil
pendStart, pendEnd = 0, 0
}

for i := range sections {
s := sections[i]
if emptyLeaf(&s) {
if t := strings.TrimSpace(s.Title); t != "" {
pendingTitles = append(pendingTitles, t)
pendStart = minNonZero(pendStart, s.PageStart)
if s.PageEnd > pendEnd {
pendEnd = s.PageEnd
}
}
continue // drop the empty leaf; its title rides on pendingTitles
}
// A real section: it carries any pending titles as heading prefix
// (these are the fragments it follows).
takePending(&s)
out = append(out, s)
}

// Any titles still pending had no following carrier — attach them to the
// last emitted sibling as a trailing heading block so nothing is lost.
if len(pendingTitles) > 0 && len(out) > 0 {
last := &out[len(out)-1]
var b strings.Builder
b.WriteString(last.Content)
for _, t := range pendingTitles {
b.WriteString("\n\n**")
b.WriteString(t)
b.WriteString("**")
}
last.Content = strings.TrimSpace(b.String())
if pendEnd > last.PageEnd {
last.PageEnd = pendEnd
}
last.PageStart = minNonZero(pendStart, last.PageStart)
}
return out
}

// propagateSectionPages fills internal-node PageStart/PageEnd from the union
// of descendant leaf ranges where the internal node didn't have its own
// (because its body was empty / hoisted into children). Leaves keep their
Expand Down Expand Up @@ -1268,7 +1404,9 @@ func parsePDFWithOutline(outline pdflib.Outline, rows []pdfRow) (*ParsedDoc, boo
title = rootSec.Children[0].Title
}

// Propagate page ranges so internal nodes span their children.
// Fold empty leaf fragments, then propagate page ranges so internal
// nodes span their children.
rootSec.Children = foldEmptyLeafSections(rootSec.Children)
propagateSectionPages(rootSec.Children)

return &ParsedDoc{
Expand Down
26 changes: 17 additions & 9 deletions pkg/retrieval/treewalk_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,23 @@ const defaultTreeWalkMaxHops = 8
// excerpt. Matches TreeWalk's reference behaviour.
const defaultPageContentLimit = 16000

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

// strategyNameTreeWalk is the stable identifier for config
// (retrieval.strategy: treewalk) and telemetry.
Expand Down
Loading