2043430 - [LinkPreview] Unhandled malformed UTF8 sequence on previews containing Chinese characters

Status: RESOLVED FIXED

(Core :: Machine Learning: On Device, defect, P2)

Description

[Valentin Pollet]

Not exactly sure what's happening, but sometimes Link Preview will fail and log:
Error: Error: malformed UTF-8 character sequence at offset 47

On my MacBook Pro, this link triggers it reliably.

I set this as P2 because I suspect this is not specific to Link Preview, rather linked to the way we stream tokens in the llama.cpp backend. It could also very well be the model itself generating bad UTF8 sequences, and us not guarding against it.

Comment 1

[Greg Tatum [:gregtatum]]

It might be breaking on UTF-16 high/low surrogate pairs, as JS encodes strings as UTF-16: https://gregtatum.com/writing/2021/encoding-text-utf-32-utf-16-unicode/#Code-units-code-points-and-high-and-low-surrogates

Comment 2

[Valentin Pollet]

For context, each decoded token is buffered by LlamaRunner. I suspect this breaks UTF8 sequences, as llama_decode is token-wise, not character-wise.

The error we see is defined here. This error message is only ever produced by InflateUTF8ToUTF16, in Javascript. This function is called when a C++ string from LlamaRunner crosses WebIDL boundary, when bound to JS the C++ string (UTF8) is inflated to UTF16 (JS standard as noted by Greg). This is consistent with either the model outputing incomplete UTF8 sequences, or our buffering causing issues.

To understand the issue and make sure we don't regress around this, I started by writing a minimal reproducer. To exercise the failing path end-to-end, I asked Claude to write a minimal gguf that always outputs a 3-byte UTF-8
codepoint. I added a small test to load and run the minimal gguf. The test runs the generation with:

• a large enough buffer size (minOutputBufferSize) so that LlamaRunner buffers the whole model response
• a buffer size of 1

On Nightly, the first test case passes and the second fails with exactly the message I get when running Link Preview on the link I gave above

Comment 3

[Valentin Pollet]

Attachment: UTF8 gguf model generator

This python script generates a gguf file for a model outputing a forced sequence of tokens corresponding to multibyte UTF8 chars. This model is part of the tests introduced with the revision fixing the bug.

Comment 4

[Valentin Pollet]

Attachment: Bug 2043430 - Fixed UTF8 sequences handling in llama.cpp generation r=jbowser

Added a test to validate that llama.cpp native generation handles UTF8 sequences correctly.
The test uses a tiny model outputing a predictable sequence of tokens.
The python script used to produce the test model is attached on Bugzilla.

Comment 5

[Valentin Pollet]

Using the model produced by the attached generator, I extended the test I added and validated that, in current state, LlamaBackend/LlamaRunner do not handle UTF8 properly. Three cases, all failing:

• Buffering of tokens breaks a UTF8 sequence -> Error: malformed UTF-8 character sequence at offset X
• Model outputs a bad UTF8 sequence -> Error: malformed UTF-8 character sequence at offset X
• Desired number of token is reached in the middle of a UTF8 sequence -> Error: malformed UTF-8 character sequence at offset

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 1 week.
:vpollet, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 7

[Valentin Pollet]

Landed

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3f9b0a06e8fc
https://hg.mozilla.org/integration/autoland/rev/925ce9dba7cc
Fixed UTF8 sequences handling in llama.cpp generation r=jbowser,ai-platform-reviewers

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b596b27ac917
https://hg.mozilla.org/integration/autoland/rev/4d96203ad172
Revert "Bug 2043430 - Fixed UTF8 sequences handling in llama.cpp generation r=jbowser,ai-platform-reviewers" for causing mochitests failures @ browser_ml_utf8_codepoint.js

Comment 10

[pstanciu]

Backed out for causing mochitests failures @ browser_ml_utf8_codepoint.js

• https://hg.mozilla.org/integration/autoland/rev/4d96203ad1727e940c9a16983b693fa8b2692c0c
• Push with failures
• Failure Log

Comment 11

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/925ce9dba7cc

Comment 12

[Serban Stanca [:SerbanS]]

Backout merged in central: https://hg-edge.mozilla.org/mozilla-central/rev/4d96203ad1727e940c9a16983b693fa8b2692c0c

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a3b2456bf5bd
https://hg.mozilla.org/integration/autoland/rev/4f6a3500eebf
Fixed UTF8 sequences handling in llama.cpp generation r=jbowser,ai-platform-reviewers

Comment 14

[Valentin Pollet]

Landed again, failures resolved.

Comment 15

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/4f6a3500eebf

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:vpollet, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox153 to wontfix.

For more information, please visit BugBot documentation.

Comment 17

[Valentin Pollet]

The bug is only affecting Link Preview users (<0.1% of the release fleet) in very rare cases and was never reported. Not worth the uplift