Skip to content

Commit 6a8badc

Browse files
authored
ingest: don't set ContentRef when no section content was written (#43)
persistTree assigned ContentRef unconditionally but only Put the content when it was non-empty after cleanForLLM. Leaves whose text cleaned to empty (heading-only sections, CID-font garbage) thus got a ref pointing at an object that was never written — so summarize, HyDE, and treewalk get_pages all failed them with 'storage: object not found'. On the whitepaper that was 24 of 93 leaves (~26%). Fix: only set ContentRef when content is actually stored; empty leaves get an empty ref, which every reader already treats as 'no stored content' (summaryFor falls back to the title; the treewalk loader returns the summary or empty). Adds a regression test asserting ref↔object parity. Closes HAL-316.
1 parent c15175a commit 6a8badc

2 files changed

Lines changed: 87 additions & 1 deletion

File tree

pkg/ingest/ingest.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,17 @@ func (p *Pipeline) persistTree(ctx context.Context, store docPersister, docID tr
628628
// time so we never persist bytes the LLM SDKs would reject
629629
// later. PDFs with CID-mapped fonts and no ToUnicode CMap
630630
// leak raw glyph IDs into extracted text.
631+
// Only assign a ContentRef when we actually wrote content. A
632+
// leaf whose text is empty after cleanForLLM (heading-only
633+
// sections, or CID-font garbage stripped to nothing) gets NO
634+
// object stored, so it must get NO ref — otherwise every later
635+
// read (summarize, HyDE, treewalk get_pages) chases a key that
636+
// was never written and fails with "storage: object not found".
637+
// Empty ContentRef is already the canonical "no stored content"
638+
// state every reader guards on (summaryFor falls back to the
639+
// title; the treewalk loader returns the summary or empty).
631640
cleanedContent := cleanForLLM(s.Content)
641+
contentRef := ""
632642
if strings.TrimSpace(cleanedContent) != "" {
633643
if err := p.Storage.Put(ctx, contentKey,
634644
bytes.NewReader([]byte(cleanedContent)),
@@ -638,6 +648,7 @@ func (p *Pipeline) persistTree(ctx context.Context, store docPersister, docID tr
638648
}); err != nil {
639649
return fmt.Errorf("store section %s: %w", id, err)
640650
}
651+
contentRef = contentKey
641652
}
642653

643654
if err := store.UpsertSection(ctx, db.Section{
@@ -647,7 +658,7 @@ func (p *Pipeline) persistTree(ctx context.Context, store docPersister, docID tr
647658
Ordinal: i,
648659
Depth: depth,
649660
Title: cleanForLLM(s.Title),
650-
ContentRef: contentKey,
661+
ContentRef: contentRef,
651662
TokenCount: approxTokens(cleanedContent),
652663
PageStart: s.PageStart,
653664
PageEnd: s.PageEnd,
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package ingest
2+
3+
import (
4+
"context"
5+
"strings"
6+
"testing"
7+
8+
"github.com/hallelx2/vectorless-engine/pkg/parser"
9+
"github.com/hallelx2/vectorless-engine/pkg/storage"
10+
)
11+
12+
// TestPersistTree_ContentRefMatchesStoredObjects is the HAL-316 regression:
13+
// a leaf only gets a ContentRef when its content was actually written. An
14+
// empty-after-clean leaf must get NO ref (and no stored object), so later
15+
// reads never chase a key that was never written ("object not found").
16+
func TestPersistTree_ContentRefMatchesStoredObjects(t *testing.T) {
17+
store, err := storage.NewLocal(t.TempDir())
18+
if err != nil {
19+
t.Fatalf("NewLocal: %v", err)
20+
}
21+
p := &Pipeline{Storage: store}
22+
fake := &fakeDocStore{}
23+
24+
doc := &parser.ParsedDoc{
25+
Title: "Doc",
26+
Sections: []parser.Section{
27+
{
28+
Level: 1, Title: "Parent", // internal node, no content
29+
Children: []parser.Section{
30+
{Level: 2, Title: "Has body", Content: "real content here", PageStart: 1, PageEnd: 1},
31+
{Level: 2, Title: "Heading only", Content: " \n\t ", PageStart: 2, PageEnd: 2}, // whitespace → empty after clean
32+
{Level: 2, Title: "Garbage glyphs", Content: "\x00\x01\x02", PageStart: 3, PageEnd: 3}, // stripped to empty
33+
},
34+
},
35+
},
36+
}
37+
38+
if err := p.persistTree(context.Background(), fake, "doc_x", doc); err != nil {
39+
t.Fatalf("persistTree: %v", err)
40+
}
41+
42+
_, _, sections := fake.snapshot()
43+
44+
// Every section that carries a ContentRef must have a readable object;
45+
// every section without one must have nothing stored under its key.
46+
withRef := 0
47+
for _, s := range sections {
48+
if s.ContentRef == "" {
49+
continue
50+
}
51+
withRef++
52+
rc, _, err := store.Get(context.Background(), s.ContentRef)
53+
if err != nil {
54+
t.Errorf("section %s has ContentRef %q but object is not readable: %v", s.ID, s.ContentRef, err)
55+
continue
56+
}
57+
_ = rc.Close()
58+
}
59+
60+
// Exactly one leaf ("Has body") had non-empty content, so exactly one
61+
// ContentRef should exist — the two empty leaves and the parent must
62+
// carry none.
63+
if withRef != 1 {
64+
t.Errorf("expected exactly 1 section with a ContentRef, got %d", withRef)
65+
}
66+
67+
// Spot-check the empty leaves explicitly carry no ref.
68+
for _, s := range sections {
69+
if strings.HasPrefix(s.Title, "Heading only") || strings.HasPrefix(s.Title, "Garbage glyphs") {
70+
if s.ContentRef != "" {
71+
t.Errorf("empty-content leaf %q must have no ContentRef, got %q", s.Title, s.ContentRef)
72+
}
73+
}
74+
}
75+
}

0 commit comments

Comments
 (0)