Skip to content

FIX: truncation leading to invalid strings when on UTF boundaries - #7

Merged
SamSaffron merged 4 commits into
mainfrom
fix-trunc
Dec 10, 2025
Merged

FIX: truncation leading to invalid strings when on UTF boundaries #7
SamSaffron merged 4 commits into
mainfrom
fix-trunc

Conversation

@SamSaffron

Copy link
Copy Markdown
Member
  • Generally gems should not check in gemfile lock, dependencies should be handled via gemspec
  • FIX: tokenizer not handling UTF-8 truncation gracefully

@SamSaffron SamSaffron changed the title fix trunc FIX: truncation leading to invalid strings when on UTF boundaries Dec 10, 2025
Comment on lines 20 to +41
def decode(token_ids)
tokenizer.decode(token_ids)
rescue Tiktoken::UnicodeError => e
rescue Tiktoken::UnicodeError
# Handle invalid token IDs gracefully by returning empty string
""
end

def safe_decode(token_ids)
tokenizer.decode(token_ids)
rescue Tiktoken::UnicodeError
# Token slice created incomplete UTF-8 sequence
# Decode individual tokens and join, skipping any that fail
token_ids
.map do |id|
begin
tokenizer.decode([id])
rescue StandardError
""
end
end
.join
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I could be mistaken.

I am not sure who might be calling open_ai_tokenizer#decode. would it make more sense to just move this safe_decode implementation to the decode function itself?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

you know what you are probably right, it will probably give us more consistent behavior, will go ahead and do so, at least it will part decode stuff that is messy

Comment on lines +36 to +38
rescue StandardError
""
end

@nattsw nattsw Dec 10, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It could be worth it to do a rails warn here, to get some signal when our tokenizers are erroring due to some unknown reason.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

warnings here will kill us, some truncation is just not possible cause the token is on a utf boundary

Comment on lines +36 to +42
rescue StandardError
rescue Tiktoken::UnicodeError

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think StandardError here is still good? Your choice

@SamSaffron
SamSaffron merged commit f45b1a7 into main Dec 10, 2025
5 checks passed
@SamSaffron
SamSaffron deleted the fix-trunc branch December 10, 2025 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants