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
36 changes: 36 additions & 0 deletions internal/handler/documents.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
"io"
"log/slog"
"net/http"
"regexp"
"strconv"
"strings"
"time"
"unicode/utf8"

"github.com/go-chi/chi/v5"

Expand Down Expand Up @@ -128,6 +130,34 @@ func (h *DocumentsHandler) HandleListDocuments(w http.ResponseWriter, r *http.Re
writeJSON(w, http.StatusOK, resp)
}

// sanitizeTitle coerces a document title into a value safe for the
// Postgres text column: valid UTF-8, no C0 control chars (except none —
// titles are single-line), whitespace-collapsed and trimmed. Invalid
// byte sequences (a client that mangled a non-ASCII char in the multipart
// field) are dropped rather than replaced so the title stays clean. An
// all-garbage title collapses to "" and the caller falls back to the doc id.
func sanitizeTitle(s string) string {
if !utf8.ValidString(s) {
s = strings.ToValidUTF8(s, "")
}
var b strings.Builder
b.Grow(len(s))
for _, r := range s {
if r == utf8.RuneError {
continue
}
if r < 0x20 { // drop control chars incl. tab/newline — titles are one line
b.WriteRune(' ')
continue
}
b.WriteRune(r)
}
return strings.TrimSpace(multiSpaceRe.ReplaceAllString(b.String(), " "))
}
Comment on lines +139 to +156

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

RuneError check drops legitimate U+FFFD characters.

utf8.RuneError and '\uFFFD' are the same value. Since s was already coerced via strings.ToValidUTF8(s, "") above, for _, r := range s can no longer decode an actual invalid byte to RuneError here — any r == utf8.RuneError at this point is a legitimately-encoded "�" character in the input, not a decode failure. The current check silently strips these valid characters instead of a decode error.

🐛 Proposed fix
 	var b strings.Builder
 	b.Grow(len(s))
-	for _, r := range s {
-		if r == utf8.RuneError {
-			continue
-		}
-		if r < 0x20 { // drop control chars incl. tab/newline — titles are one line
+	for _, r := range s {
+		if r < 0x20 { // drop control chars incl. tab/newline — titles are one line
 			b.WriteRune(' ')
 			continue
 		}
 		b.WriteRune(r)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func sanitizeTitle(s string) string {
if !utf8.ValidString(s) {
s = strings.ToValidUTF8(s, "")
}
var b strings.Builder
b.Grow(len(s))
for _, r := range s {
if r == utf8.RuneError {
continue
}
if r < 0x20 { // drop control chars incl. tab/newline — titles are one line
b.WriteRune(' ')
continue
}
b.WriteRune(r)
}
return strings.TrimSpace(multiSpaceRe.ReplaceAllString(b.String(), " "))
}
func sanitizeTitle(s string) string {
if !utf8.ValidString(s) {
s = strings.ToValidUTF8(s, "")
}
var b strings.Builder
b.Grow(len(s))
for _, r := range s {
if r < 0x20 { // drop control chars incl. tab/newline — titles are one line
b.WriteRune(' ')
continue
}
b.WriteRune(r)
}
return strings.TrimSpace(multiSpaceRe.ReplaceAllString(b.String(), " "))
}
🤖 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 `@internal/handler/documents.go` around lines 139 - 156, The sanitizeTitle
function is incorrectly removing valid U+FFFD replacement characters by checking
r == utf8.RuneError after strings.ToValidUTF8 has already normalized invalid
bytes. Update sanitizeTitle so it only strips actual invalid input during the
ToValidUTF8 step and does not discard legitimate “�” runes during the rune loop;
keep the control-character handling and whitespace normalization intact.


// multiSpaceRe collapses runs of whitespace in a sanitized title.
var multiSpaceRe = regexp.MustCompile(`\s{2,}`)

// sourceTypeFromContentType collapses an HTTP Content-Type to the
// short tag the dashboard's source-type badge expects.
func sourceTypeFromContentType(ct string) string {
Expand Down Expand Up @@ -249,6 +279,12 @@ func (h *DocumentsHandler) HandleIngestDocument(w http.ResponseWriter, r *http.R
if title == "" {
title = filename
}
// Sanitize before the DB write: a title with invalid UTF-8 bytes
// (e.g. a client that mangled an em-dash) would otherwise make the
// Postgres insert fail with "db write failed" and 500 the whole
// ingest. Coerce to valid UTF-8 and drop control chars so a bad
// title byte can never fail the upload.
title = sanitizeTitle(title)
if title == "" {
title = string(docID)
}
Expand Down
20 changes: 20 additions & 0 deletions internal/handler/sanitize_title_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package handler

import "testing"

func TestSanitizeTitle(t *testing.T) {
cases := []struct{ in, want string }{
{"Attention Is All You Need", "Attention Is All You Need"},
{" spaced out title ", "spaced out title"},
{"Berkshire — 2023", "Berkshire — 2023"}, // valid UTF-8 em-dash is kept
{"bad\xff\xfebyte", "badbyte"}, // invalid UTF-8 bytes dropped
{"line\nbreak\ttab", "line break tab"}, // control chars → space, collapsed
{"\xff\xfe", ""}, // all-garbage collapses to empty
{"", ""},
}
for _, c := range cases {
if got := sanitizeTitle(c.in); got != c.want {
t.Errorf("sanitizeTitle(%q) = %q, want %q", c.in, got, c.want)
}
}
}
4 changes: 3 additions & 1 deletion pkg/ingest/ingest.go
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,9 @@ func (p *Pipeline) persistTree(ctx context.Context, store docPersister, docID tr
// Watermarked PDFs whose overlay text shares a Y coordinate with
// the real title produce mojibake like "GGlloobbaall SSttrraatteeggyy"
// — we'd rather keep the original filename than show that to a user.
if err := store.SetDocumentTitle(ctx, docID, doc.Title); err != nil {
// cleanForLLM strips any invalid-UTF-8 bytes so the title write can
// never fail the persist.
if err := store.SetDocumentTitle(ctx, docID, cleanForLLM(doc.Title)); err != nil {
return err
}
}
Expand Down
34 changes: 32 additions & 2 deletions pkg/parser/pdf.go
Original file line number Diff line number Diff line change
Expand Up @@ -1477,12 +1477,42 @@ var boilerplateFragments = []string{
"or scholarly",
}

// isRepeatedMarkerLine reports whether a row is a run of the same short
// token repeated — the artifact of an icon glyph rendered as text and
// stamped once per item, e.g. the ORCID "iD" logo repeated once per author
// on a paper's byline ("iD iD iD iD"). Real prose never repeats an
// identical ≤2-character token as the majority of a ≥3-token line, so
// dropping these is safe and removes the junk author-block "headings"
// (e.g. PRISMA's contributor page) the font heuristic would otherwise
// surface as sections.
func isRepeatedMarkerLine(s string) bool {
fields := strings.Fields(s)
if len(fields) < 3 {
return false
}
counts := map[string]int{}
for _, f := range fields {
if len([]rune(f)) <= 2 {
counts[f]++
}
}
for _, c := range counts {
if c*100 >= len(fields)*60 { // ≥60% of the line is one short token
return true
}
}
return false
}

// isBoilerplateLine reports whether a row is publisher/license noise.
// Matches the curated signature list, the bare arXiv id stamp
// ("arXiv:2401.01234v2 [cs.CL] 5 Jan 2024"), and short license-tail
// fragments.
// ("arXiv:2401.01234v2 [cs.CL] 5 Jan 2024"), short license-tail
// fragments, and repeated icon-glyph marker rows (ORCID "iD iD iD").
func isBoilerplateLine(s string) bool {
low := strings.ToLower(strings.TrimSpace(s))
if isRepeatedMarkerLine(low) {
return true
}
for _, sig := range boilerplateSignatures {
if strings.Contains(low, sig) {
return true
Expand Down
32 changes: 32 additions & 0 deletions pkg/parser/repeated_marker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package parser

import "testing"

// TestIsRepeatedMarkerLine locks the filter that drops icon-glyph marker
// rows (e.g. an ORCID "iD" repeated once per author) while never touching
// real prose or short headings.
func TestIsRepeatedMarkerLine(t *testing.T) {
drop := []string{
"id id id id", // 4/4 = 100%
"iD iD iD iD", // case-insensitive caller lower-cases first; test raw too
"id id id", // 3/3
"id id id id author", // 4/5 = 80% ≥ 60%
}
keep := []string{
"the right to erasure is established in article 17",
"multi-head attention", // 2 tokens, below the 3-token floor
"introduction", // single token
"id id id penny whiting17, david moher 22", // 3 of 9 ≈ 33% → real author line, kept
"id id elie a. akl 8, sue e. brennan", // 2 of 8 → kept
}
for _, s := range drop {
if !isRepeatedMarkerLine(s) {
t.Errorf("expected DROP for %q", s)
}
}
for _, s := range keep {
if isRepeatedMarkerLine(s) {
t.Errorf("expected KEEP for %q", s)
}
}
}
Loading