Skip to content

Commit bcb2635

Browse files
committed
fix(tree+ingest): show all sections + drop bad bytes before LLM
Three related bugs from inspecting the GINA PDF upload that reached 'ready' with an empty ToC: 1. buildTree silently dropped multiple top-level sections. It picked the first row with NULL parent_id as the tree root and ignored the rest, so BuildView only walked one subtree. This PDF had three top-level siblings (the title splits across pages) and ~280 sections under the other two were invisible to /toc. Now we wrap multiple top-level sections in a synthetic root so BuildView walks them all. 2. Section content was written to storage without UTF-8 validation. PDFs with CID-mapped fonts and no ToUnicode CMap leak raw glyph IDs into extracted text, which the Gemini SDK then rejects with "google.ai.generativelanguage.v1beta.Part.text contains invalid UTF-8". cleanForLLM strips the bad bytes and disallowed C0 controls (keeping \t \n \r) at storage-write time AND on the read-back path that feeds the LLM. 3. CountSections + plumbing through HandleGetDocument so the dashboard metadata panel can render the actual section count instead of a blank field. The doc list endpoint still doesn't include it (one query per row is a footgun for large libraries) — only the detail endpoint.
1 parent 95c4e18 commit bcb2635

3 files changed

Lines changed: 128 additions & 13 deletions

File tree

pkg/db/sections.go

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,22 @@ func (p *Pool) UpdateSectionSummary(ctx context.Context, id tree.SectionID, summ
6262
return mapErr(err)
6363
}
6464

65+
// CountSections returns the number of sections persisted for a
66+
// document. Used by the dashboard metadata panel; not org-scoped on
67+
// purpose — pair with a prior GetDocument(orgID) check to ensure the
68+
// caller is allowed to see the document at all.
69+
func (p *Pool) CountSections(ctx context.Context, docID tree.DocumentID) (int, error) {
70+
var n int
71+
err := p.QueryRow(ctx,
72+
`SELECT count(*) FROM sections WHERE document_id = $1`,
73+
string(docID),
74+
).Scan(&n)
75+
if err != nil {
76+
return 0, mapErr(err)
77+
}
78+
return n, nil
79+
}
80+
6581
// GetSection fetches a single section, scoped to an org via JOIN on
6682
// documents.org_id. Cross-org reads return ErrNotFound rather than
6783
// the row, so section IDs from other tenants can't be probed.
@@ -214,13 +230,16 @@ func buildTree(doc *Document, rows []Section) *tree.Tree {
214230
}
215231
}
216232

217-
var root *tree.Section
233+
// Collect every section whose parent_id is empty — these are
234+
// "top-level" sections. Older code picked just the first one as
235+
// the tree root and silently dropped the other siblings + their
236+
// subtrees. Now we wrap them in a synthetic root so callers see
237+
// the whole document.
238+
var topLevel []*tree.Section
218239
for i := range rows {
219240
s := byID[rows[i].ID]
220241
if s.ParentID == "" {
221-
if root == nil {
222-
root = s
223-
}
242+
topLevel = append(topLevel, s)
224243
continue
225244
}
226245
parent, ok := byID[s.ParentID]
@@ -230,6 +249,25 @@ func buildTree(doc *Document, rows []Section) *tree.Tree {
230249
parent.Children = append(parent.Children, s)
231250
}
232251

252+
var root *tree.Section
253+
switch len(topLevel) {
254+
case 0:
255+
root = nil // empty doc
256+
case 1:
257+
root = topLevel[0]
258+
default:
259+
// Multiple top-level sections — wrap in a synthetic root so
260+
// BuildView walks all of them. The synthetic root carries the
261+
// document's title and an empty ID so consumers can distinguish
262+
// it from a real section.
263+
root = &tree.Section{
264+
ID: "",
265+
ParentID: "",
266+
Title: doc.Title,
267+
Children: topLevel,
268+
}
269+
}
270+
233271
return &tree.Tree{
234272
DocumentID: doc.ID,
235273
Title: doc.Title,

pkg/ingest/ingest.go

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"strings"
2323
"sync"
2424
"time"
25+
"unicode/utf8"
2526

2627
"github.com/google/uuid"
2728
"golang.org/x/sync/errgroup"
@@ -159,12 +160,17 @@ func (p *Pipeline) persistTree(ctx context.Context, docID tree.DocumentID, doc *
159160
id := tree.SectionID("sec_" + uuid.New().String())
160161
contentKey := path.Join("documents", string(docID), "sections", string(id)+".txt")
161162

162-
if strings.TrimSpace(s.Content) != "" {
163+
// Strip invalid UTF-8 / disallowed control chars at storage
164+
// time so we never persist bytes the LLM SDKs would reject
165+
// later. PDFs with CID-mapped fonts and no ToUnicode CMap
166+
// leak raw glyph IDs into extracted text.
167+
cleanedContent := cleanForLLM(s.Content)
168+
if strings.TrimSpace(cleanedContent) != "" {
163169
if err := p.Storage.Put(ctx, contentKey,
164-
bytes.NewReader([]byte(s.Content)),
170+
bytes.NewReader([]byte(cleanedContent)),
165171
storage.Metadata{
166172
ContentType: "text/plain; charset=utf-8",
167-
Size: int64(len(s.Content)),
173+
Size: int64(len(cleanedContent)),
168174
}); err != nil {
169175
return fmt.Errorf("store section %s: %w", id, err)
170176
}
@@ -176,9 +182,9 @@ func (p *Pipeline) persistTree(ctx context.Context, docID tree.DocumentID, doc *
176182
ParentID: parent,
177183
Ordinal: i,
178184
Depth: depth,
179-
Title: s.Title,
185+
Title: cleanForLLM(s.Title),
180186
ContentRef: contentKey,
181-
TokenCount: approxTokens(s.Content),
187+
TokenCount: approxTokens(cleanedContent),
182188
Metadata: s.Metadata,
183189
}); err != nil {
184190
return err
@@ -296,7 +302,7 @@ func (p *Pipeline) summaryFor(ctx context.Context, s db.Section, kids []db.Secti
296302
if len(kids) == 0 {
297303
// Leaf: fetch the stored text.
298304
if s.ContentRef == "" {
299-
return s.Title, nil
305+
return cleanForLLM(s.Title), nil
300306
}
301307
rc, _, err := p.Storage.Get(ctx, s.ContentRef)
302308
if err != nil {
@@ -307,7 +313,7 @@ func (p *Pipeline) summaryFor(ctx context.Context, s db.Section, kids []db.Secti
307313
if err != nil {
308314
return "", err
309315
}
310-
body = string(raw)
316+
body = cleanForLLM(string(raw))
311317
} else {
312318
// Internal: compose from children's titles so we have SOMETHING to
313319
// summarize even without bringing all children content into memory.
@@ -327,7 +333,7 @@ func (p *Pipeline) summaryFor(ctx context.Context, s db.Section, kids []db.Secti
327333
{Role: llmgate.RoleSystem, Content: "You write short, factual section summaries. One sentence, no preamble, no quotes."},
328334
{Role: llmgate.RoleUser, Content: fmt.Sprintf(
329335
"Summarize this section titled %q in a single sentence (max 40 words):\n\n%s",
330-
s.Title, body)},
336+
cleanForLLM(s.Title), body)},
331337
},
332338
})
333339
if err != nil {
@@ -370,6 +376,46 @@ func (p *Pipeline) fail(ctx context.Context, id tree.DocumentID, stage string, c
370376
}
371377
}
372378

379+
// cleanForLLM strips invalid-UTF-8 bytes and a couple of control
380+
// characters that some LLM SDKs reject at the proto layer (the
381+
// gemini-go SDK is strict about this — it errors with
382+
// "google.ai.generativelanguage.v1beta.Part.text contains invalid UTF-8"
383+
// the moment any byte sequence isn't a complete UTF-8 codepoint).
384+
//
385+
// PDFs with custom CID-mapped fonts and no ToUnicode CMap leak raw
386+
// glyph IDs into our extracted text, which look like garbage bytes.
387+
// We drop them rather than fail the whole summarization.
388+
func cleanForLLM(s string) string {
389+
if utf8.ValidString(s) && !hasBadControlChars(s) {
390+
return s
391+
}
392+
var b strings.Builder
393+
b.Grow(len(s))
394+
for i := 0; i < len(s); {
395+
r, size := utf8.DecodeRuneInString(s[i:])
396+
i += size
397+
if r == utf8.RuneError && size == 1 {
398+
b.WriteRune('�')
399+
continue
400+
}
401+
// Drop NUL + most C0 control chars; keep tab/newline/CR.
402+
if r < 0x20 && r != '\t' && r != '\n' && r != '\r' {
403+
continue
404+
}
405+
b.WriteRune(r)
406+
}
407+
return b.String()
408+
}
409+
410+
func hasBadControlChars(s string) bool {
411+
for _, r := range s {
412+
if r < 0x20 && r != '\t' && r != '\n' && r != '\r' {
413+
return true
414+
}
415+
}
416+
return false
417+
}
418+
373419
// isLikelyMojibakeTitle returns true when s shows the doubled-glyph
374420
// signature of two-layer PDFs (an overlay watermark drawn over real
375421
// text at the same Y coordinate, so chars from both layers interleave

pkg/ingest/mojibake_test.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,37 @@
11
package ingest
22

3-
import "testing"
3+
import (
4+
"testing"
5+
"unicode/utf8"
6+
)
7+
8+
func TestCleanForLLM(t *testing.T) {
9+
cases := []struct {
10+
in string
11+
want string
12+
}{
13+
// Valid text passes through unchanged.
14+
{"Hello world", "Hello world"},
15+
{"Section title — with em dash", "Section title — with em dash"},
16+
{"Multi-line\nbody\nstays intact", "Multi-line\nbody\nstays intact"},
17+
// Invalid bytes get replaced with U+FFFD.
18+
{"bad \xff byte", "bad � byte"},
19+
// NUL + most C0 controls get dropped; tab/newline/CR are kept.
20+
{"keep\ttabs\nand\rcrs but drop\x00nul", "keep\ttabs\nand\rcrs but dropnul"},
21+
{"text with bell\x07char", "text with bellchar"},
22+
// Empty input survives.
23+
{"", ""},
24+
}
25+
for _, tc := range cases {
26+
got := cleanForLLM(tc.in)
27+
if got != tc.want {
28+
t.Errorf("cleanForLLM(%q) = %q, want %q", tc.in, got, tc.want)
29+
}
30+
if !utf8.ValidString(got) {
31+
t.Errorf("cleanForLLM(%q) returned invalid UTF-8", tc.in)
32+
}
33+
}
34+
}
435

536
func TestIsLikelyMojibakeTitle(t *testing.T) {
637
cases := []struct {

0 commit comments

Comments
 (0)