Prefix highlight should handle unicode

[ManyTheFish]

Describe the bug
It is not possible for the actual Highlighter to count precisely how many bytes should be highlighted when a word contains deunicoded characters.

To Reproduce

1. Add a document containing words like Go💼od or Vývoj
2. Search word by prefix Go💼 or vývo
3. result is <em>Go💼od</em> or <em>Vývoj</em>

Expected behavior
<em>Go💼</em>od or <em>Vývo</em>j

Version
meilisearch v0.21

Additional context
related to #1368 and #1173

Fix Proposals

Token as an array of characters
make tokenizer return [str] corresponding to each character of the current Token, the highlighter would be able to return a character count based on the array.

Spaned Normalization
make the normalizer return bounds in bytes of each character in the token via a new function.

Forbid to the normalizer to return more than 1 char per normalized char
forbid at least:

• emoji deunicoding
• non-Latin scripts deunicoding

this last proposal is the easiest to implement but would be the ugliest

[curquiza]

Checked with the team, this is not easy to fix for v0.21.0 and not that an emergency, I remove this issue from the v0.21.0 milestones.
However, it should not crash, if you have a crash behavior, please report it 🙂

[Samyak2]

Now that meilisearch/charabia#54 is closed, this should be possible to fix. Can I take a shot at this?

[ManyTheFish]

Hey @Samyak2!
Yes, you can, I would be glad to help you if you have any questions, we will soon make a release of the tokenizer allowing you to work on this. 🚀

[Samyak2]

Great! Thank you

[curquiza]

@Samyak2 tokenizer v0.2.6 is out

[Samyak2]

I have been working on this, but I have run into some issues.

I changed the PrimitiveQueryPart struct to store Token itself instead of just the String (This is needed because the num_graphemes_from_bytes of Token will be used to count number of characters to highlight). Since Token has a reference internally, I had to add lifetime specifiers to everything that uses PrimitiveQuery.

This leads to issues with the borrow checker as the reference internal to Token depends on the analyzer and the stop words.

Cargo build errors

error[E0597]: `stop_words.0` does not live long enough
   --> milli/src/search/mod.rs:118:29
    |
108 |         let (query_tree, primitive_query) = match self.query.as_ref() {
    |              ---------- borrow later used here
...
118 |                 if let Some(ref stop_words) = stop_words {
    |                             ^^^^^^^^^^^^^^ borrowed value does not live long enough
...
126 |             }
    |             - `stop_words.0` dropped here while still borrowed

error[E0597]: `analyzer` does not live long enough
   --> milli/src/search/mod.rs:122:30
    |
108 |         let (query_tree, primitive_query) = match self.query.as_ref() {
    |              ---------- borrow later used here
...
122 |                 let result = analyzer.analyze(query);
    |                              ^^^^^^^^ borrowed value does not live long enough
...
126 |             }
    |             - `analyzer` dropped here while still borrowed

error[E0597]: `result` does not live long enough
   --> milli/src/search/mod.rs:123:30
    |
108 |         let (query_tree, primitive_query) = match self.query.as_ref() {
    |              ---------- borrow later used here
...
123 |                 let tokens = result.tokens();
    |                              ^^^^^^ borrowed value does not live long enough
...
126 |             }
    |             - `result` dropped here while still borrowed

error[E0597]: `builder` does not live long enough
   --> milli/src/search/mod.rs:124:53
    |
108 |         let (query_tree, primitive_query) = match self.query.as_ref() {
    |              ---------- borrow later used here
...
124 |                 let (query_tree, primitive_query) = builder.build(tokens)?.map_or((None, None), |(qt, pq)| (Some(qt), Some(pq)));
    |                                                     ^^^^^^^ borrowed value does not live long enough
125 |                 (query_tree, primitive_query.clone())
126 |             }
    |             - `builder` dropped here while still borrowed

For more information about this error, try `rustc --explain E0597`.
error: could not compile `milli` due to 4 previous errors

I'm quite new to Rust, so I couldn't figure out how to fix these issues. Any pointers would be appreciated :)

[ManyTheFish]

Hey @Samyak2! I think you try to modify an unrelated code part.
I invite you to look at matching_bytes function in matching_words.rs, this function is used to compute highlights and matches in meilisearch.
For now, this function takes a &str but I know that the "caller" of the function can provide a &Token instead. I think that changing this function would do the job. 🙂

[adellinocasas]

Hi there,
has this issue - Prefix highlight should handle unicode - already been fixed?
(using version Scout / Meilisearch "pkgVersion":"0.20.0")
thanks

[Samyak2]

Hi @ManyTheFish. Sorry I was too busy with college exams in the last month :(

I can continue working on this now and will have a PR in the next 1-2 days (if it hasn't been fixed already).

[curquiza]

No worry @Samyak2, no pressure for the contributors here! 🙂 Hope your exams went well!

Thanks a lot for your PRs and your involvement!

[Samyak2]

Thank you!

[curquiza]

Reopen since it's only fixed milli's side but not on MeiliSearch side

To close this issue, we need to

• Release milli
• Update the milli dependency in this repo

[curquiza]

Fixed by #2005 that bump the milli dependency to milli v0.22.0 containing the fix of this issue 🙂

The bug fix will be released in Meilisearch v0.26.0.

[yngc0der]

Still have this problem with meilisearch v0.27.2 and cyrillic text in json field.
Query: "Компания"
Response formatted part: "Компания дарит бесплатные билеты"

[ManyTheFish]

Hey @yngc0der,
we completely changed the tokenizer for the version 0.28, and it should fix your issue.
This version will be pre-released next week (beta release) and will be stabilized in 4 weeks.
Don't hesitate to create a new issue if your bug persists.

Thanks for your interest!