From 125c99bb463e188d8f43d6ab5d76da5fc7eb01ce Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 18 Aug 2026 07:13:15 +0200 Subject: [PATCH] fix(text): truncate on rune boundaries, not byte offsets (#93) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four call sites did `s[:limit] + "…"`. Both halves are byte operations: len() counts bytes and a slice cuts bytes. On Swedish text — å ä ö are two bytes, an em dash three — the cut lands mid-character and leaves an orphaned lead byte, producing invalid UTF-8. It failed loudly in exactly one place: wiki/index.md is written raw to disk, where every strict decoder then rejects the whole file. The other three (brain_context excerpts x2, brain_query excerpts) feed MCP responses, where encoding/json silently substitutes U+FFFD — so those had been quietly eating the last character of truncated excerpts, with no error anywhere and nothing to notice. textutil.Truncate counts runes and appends the marker only when content was actually dropped; an ellipsis on complete text is a lie about the content. The test is a property, not an example: it walks the limit across every offset of a multi-byte string and asserts utf8.Valid at each one, so a future off-by-one cannot pass by picking a lucky boundary. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BeAp5LTscnz2W7Ubt4eJ6m --- ingestion/internal/mcp/tools_context.go | 11 +--- ingestion/internal/search/search.go | 6 +- ingestion/internal/textutil/truncate.go | 45 +++++++++++++ ingestion/internal/textutil/truncate_test.go | 66 ++++++++++++++++++++ ingestion/internal/wiki/index.go | 7 +-- 5 files changed, 119 insertions(+), 16 deletions(-) create mode 100644 ingestion/internal/textutil/truncate.go create mode 100644 ingestion/internal/textutil/truncate_test.go diff --git a/ingestion/internal/mcp/tools_context.go b/ingestion/internal/mcp/tools_context.go index 9ccfcec..d906f20 100644 --- a/ingestion/internal/mcp/tools_context.go +++ b/ingestion/internal/mcp/tools_context.go @@ -11,6 +11,7 @@ import ( "time" "github.com/mathiasbq/hyperguild/ingestion/internal/search" + "github.com/mathiasbq/hyperguild/ingestion/internal/textutil" ) // brainContextArgs is the input shape of brain_context. project_root is @@ -170,10 +171,7 @@ func (s *Server) brainContext(ctx context.Context, args json.RawMessage) (json.R // without re-running the frontmatter parser. The ellipsis suffix matches // the convention used in search.excerpt. func truncateExcerpt(s string, maxLen int) string { - if len(s) <= maxLen { - return s - } - return s[:maxLen] + "…" + return textutil.Truncate(s, maxLen) } // readExcerpt loads a doc relative to brainDir, strips its frontmatter, @@ -193,10 +191,7 @@ func readExcerpt(brainDir, relPath string, maxLen int) string { if len(parts) == 3 { body = strings.TrimSpace(parts[2]) } - if len(body) > maxLen { - return body[:maxLen] + "…" - } - return body + return textutil.Truncate(body, maxLen) } // max1 returns the maximum of n and 1, used to guard against divide-by-zero diff --git a/ingestion/internal/search/search.go b/ingestion/internal/search/search.go index ec75d5b..b0c75fe 100644 --- a/ingestion/internal/search/search.go +++ b/ingestion/internal/search/search.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/brain" + "github.com/mathiasbq/hyperguild/ingestion/internal/textutil" "github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore" ) @@ -427,8 +428,5 @@ func excerpt(content string, maxLen int) string { if len(parts) == 3 { body = strings.TrimSpace(parts[2]) } - if len(body) > maxLen { - return body[:maxLen] + "…" - } - return body + return textutil.Truncate(body, maxLen) } diff --git a/ingestion/internal/textutil/truncate.go b/ingestion/internal/textutil/truncate.go new file mode 100644 index 0000000..e525f50 --- /dev/null +++ b/ingestion/internal/textutil/truncate.go @@ -0,0 +1,45 @@ +// Package textutil holds small text helpers shared across the ingestion +// service. +package textutil + +import "unicode/utf8" + +// Ellipsis marks a value that was cut short. +const Ellipsis = "…" + +// Truncate shortens s to at most limit CHARACTERS, appending an ellipsis when +// anything was dropped. +// +// It exists because four call sites did `s[:limit] + "…"` (#93). Both halves +// of that are byte operations: len() counts bytes and a slice cuts bytes, so +// on Swedish text — where å ä ö are two bytes and an em dash is three — the +// cut lands mid-character and leaves an orphaned lead byte. The result is +// invalid UTF-8. +// +// That failed loudly in exactly one place, wiki/index.md, which is written +// raw to disk and then rejected by every strict decoder. The other three fed +// MCP responses, where encoding/json silently substitutes U+FFFD — so those +// had been quietly eating the last character of truncated excerpts with no +// error anywhere. +// +// The marker is appended only when content was actually dropped: an ellipsis +// on complete text is a lie about the content. +func Truncate(s string, limit int) string { + if limit <= 0 { + return "" + } + if utf8.RuneCountInString(s) <= limit { + return s + } + + // Walk runes rather than converting to []rune: the input can be a whole + // note body, and this runs on the read path. + count := 0 + for i := range s { + if count == limit { + return s[:i] + Ellipsis + } + count++ + } + return s + Ellipsis +} diff --git a/ingestion/internal/textutil/truncate_test.go b/ingestion/internal/textutil/truncate_test.go new file mode 100644 index 0000000..2dc0f03 --- /dev/null +++ b/ingestion/internal/textutil/truncate_test.go @@ -0,0 +1,66 @@ +// ingestion/internal/textutil/truncate_test.go +package textutil_test + +import ( + "strings" + "testing" + "unicode/utf8" + + "github.com/mathiasbq/hyperguild/ingestion/internal/textutil" +) + +func TestTruncate_ShortStringsAreUntouched(t *testing.T) { + for _, s := range []string{"", "kort", "räv", "a — b"} { + if got := textutil.Truncate(s, 100); got != s { + t.Errorf("Truncate(%q, 100) = %q, want it unchanged", s, got) + } + } +} + +func TestTruncate_NeverCutsInsideACharacter(t *testing.T) { + // The property, not an example: whatever the limit, the result must be + // valid UTF-8. Byte-slicing Swedish text (å ä ö) or an em dash produces + // an orphaned lead byte, which is what corrupted wiki/index.md (#93). + s := strings.Repeat("räv — ål — ökäd ", 20) + + for limit := 0; limit <= len(s)+5; limit++ { + got := textutil.Truncate(s, limit) + if !utf8.ValidString(got) { + t.Fatalf("Truncate(s, %d) produced invalid UTF-8: %q", limit, got) + } + } +} + +func TestTruncate_CountsCharactersNotBytes(t *testing.T) { + // "räv" is 4 bytes, 3 runes. A byte-based limit of 3 would cut inside the + // "ä"; a rune-based one returns it whole. + if got := textutil.Truncate("räv", 3); got != "räv" { + t.Errorf("Truncate(\"räv\", 3) = %q, want the whole word", got) + } + + got := textutil.Truncate("räven springer", 5) + if utf8.RuneCountInString(got) != 6 { // 5 runes + the ellipsis + t.Errorf("Truncate(...) = %q, want 5 runes plus the marker", got) + } + if !strings.HasSuffix(got, "…") { + t.Errorf("Truncate(...) = %q, want it marked as truncated", got) + } +} + +func TestTruncate_MarksOnlyWhenSomethingWasDropped(t *testing.T) { + // An ellipsis on text that was not cut is a lie about the content. + if got := textutil.Truncate("räv", 10); strings.HasSuffix(got, "…") { + t.Errorf("Truncate(%q, 10) = %q, want no truncation marker", "räv", got) + } + if got := textutil.Truncate("räv", 3); strings.HasSuffix(got, "…") { + t.Errorf("exact-length input must not be marked: %q", got) + } +} + +func TestTruncate_NonPositiveLimitYieldsNothingRatherThanPanicking(t *testing.T) { + for _, limit := range []int{0, -1, -100} { + if got := textutil.Truncate("räven", limit); got != "" { + t.Errorf("Truncate(_, %d) = %q, want empty", limit, got) + } + } +} diff --git a/ingestion/internal/wiki/index.go b/ingestion/internal/wiki/index.go index b80841a..3a0ea2f 100644 --- a/ingestion/internal/wiki/index.go +++ b/ingestion/internal/wiki/index.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/mathiasbq/hyperguild/ingestion/internal/textutil" ) // RebuildIndex writes brain/wiki/index.md from the current wiki contents. @@ -62,10 +64,7 @@ func pageFirstSentence(brainDir string, e Entry) string { if line == "" || strings.HasPrefix(line, "#") { continue } - if len(line) > 100 { - return line[:100] + "…" - } - return line + return textutil.Truncate(line, 100) } return "" }