Skip to content

fix: keep conversation titles valid UTF-8 in promptToTitle truncation - #9837

Open
eminemead wants to merge 1 commit into
rilldata:mainfrom
eminemead:xiaofei.yin/fix-conversation-title-utf8
Open

eminemead wants to merge 1 commit into
rilldata:mainfrom
eminemead:xiaofei.yin/fix-conversation-title-utf8

Conversation

@eminemead

@eminemead eminemead commented Aug 27, 2026

Copy link
Copy Markdown
  • ListConversations failed to marshal when a stored conversation title contained invalid UTF-8.
  • promptToTitle truncated with a byte slice (title[:47]), which split multi-byte runes.
  • Truncation is now rune-safe, and titles are sanitized with strings.ToValidUTF8 at the marshal boundary (invalid bytes become U+FFFD) so previously stored rows still list.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

Developed in collaboration with Claude Code

Byte-wise title[:47] split multi-byte Chinese runes, so ListConversations could not marshal existing sessions.
@nishantmonu51 nishantmonu51 added Type:Bug Something isn't working Size:M Medium change: 100-499 lines labels Sep 1, 2026

@nishantmonu51 nishantmonu51 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The rune-safe cut in promptToTitle and the strings.ToValidUTF8 sanitization in sessionToPB cover every path that serves a stored title: sessionToPB is the only constructor of runtimev1.Conversation, so ListConversations, GetConversation, and the forked title built in runtime/ai/ai.go:226 all go through it. The four new tests pin both the truncation boundary and the marshal boundary.

Worth stating explicitly: rows already stored with invalid UTF-8 are repaired only at the marshal boundary, not in the catalog, so the U+FFFD stays in those titles until they are renamed. That reads as the right trade-off given the blast radius.


// truncateUTF8 shortens s to at most maxBytes, appending "..." when truncated.
// The cut is always on a UTF-8 rune boundary so the result stays valid UTF-8.
func truncateUTF8(s string, maxBytes int) string {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

truncateUTF8 is called exactly once, with a constant, which also makes the maxBytes <= len(ellipsis) branch unreachable. Per the repo's convention against single-use utility functions, the back-off could be inlined in promptToTitle as a walk from index 47 while !utf8.RuneStart(title[i]), which avoids re-validating the whole prefix on every iteration. Either form relies on the prompt being valid UTF-8 on entry, which holds for all current entry points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size:M Medium change: 100-499 lines Type:Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants