search_excerpt() doesn't work well with stemming

I've worked out a fix for this. In D6 search module on line 1212 add in .'.*?' so the line becomes:

if (preg_match('/'. $boundary . $key . '.*?' . $boundary .'/iu', $text, $match, PREG_OFFSET_CAPTURE, $included[$key]))

And the same for line 1270 so that becomes

$text = preg_replace('/'. $boundary .'('. implode('|', $keys) .')'. '.*?' . $boundary .'/iu', '<strong>\0</strong>', $text);

The first says match the key plus any more characters up to a boundary. So this find the right extract with the stemmed words. The second is the fix so the whole word is made bold.

Good point. You are right it isn't that simple. Taking your example like, liking likely the porter/stemmer says these all have the stem of like. So all nodes with any of those 3 words get a search key of like when cron runs. Type in any of those 3 words and the search key changes to like, finds the node, but then fails at finding the extract. Stemming rules that reduce say 10 possible words to one is fair enough but to go the other is likely to make one word into 50, some of which will be nonsense i.e. nationality stems to nation, but station doesn't unstem to stationality.

Perhaps the solution is to have multiple passes at the extract shrinking the keys by the end letter each time until there is match. Or I guess the stemmer could have a function that reduces the key down to the common root, so like, liking, likely would reduce to just lik. Or rather lik.*? which would be the right search key. That's probably better.

OK, new mod. Add the '.*?' . to lines 1212 and 1270 as in post #1. And in addition before line 1186

$workkeys = $keys;

insert this code:

  foreach($keys as $k => $key){
    search_invoke_preprocess($key);
    $keys[$k] = substr($key,0,-1);
  }

what that does is take the search keys as typed. Stem them and remove the last character. Of course it would be better if there was a proper hook and stem root function but this probably works most of the time. The stem will be the smallest word but of course like -> liking loses the e, y becomes ies etc. so just removing the last stem character probably gives the correct result most of time, perhaps all, at least in english.

You could be right, but at least the above returns more extracts that are sensible that the current method which returns next none (only if you try in a word that is also a stem, if that is possible).

Feel free to look at. I agree my solution is quick and dirty and not suitable as a patch. It just improves a bad situation, for english, to a state where the error is not so noticeable.

BTW it would be useful if your patch had a D6 as well as D7 version as I'm more interested in D6.

OK, I'm giving this a try. Seems to work well so far.

Related: #103548: Partial Search in Drupal Core. The reason the test fail to pass now, is the search_admin_validate() function is now gone. It was replaced by a proper submit() handler.

I think my solution in that issue was considerably smaller. I would consider testing that patch and see if it achieves the issue with less code changes. It was just a lil bit of regex.

sigh u missed the point...

In the lasted patch, I had to accomplish what you are trying here, meaning highlight a full word when a part of the word was in the $keys

like so

@@ -1252,7 +1280,7 @@ function search_excerpt($keys, $text) {
       }
       // Locate a keyword (position $p), then locate a space in front (position
       // $q) and behind it (position $s)
-      if (preg_match('/' . $boundary . $key . $boundary . '/iu', $text, $match, PREG_OFFSET_CAPTURE, $included[$key])) {
+      if (preg_match('/' . $boundary .'[^' . PREG_CLASS_SEARCH_EXCLUDE . PREG_CLASS_CJK . ']*' . $key . '[^' . PREG_CLASS_SEARCH_EXCLUDE . PREG_CLASS_CJK . ']*' . $boundary . '/iu', $text, $match, PREG_OFFSET_CAPTURE, $included[$key])) {
         $p = $match[0][1];
         if (($q = strpos($text, ' ', max(0, $p - 60))) !== FALSE) {
           $end = substr($text, $p, 80);
@@ -1310,7 +1338,7 @@ function search_excerpt($keys, $text) {
   $text = (isset($newranges[0]) ? '' : '... ') . implode(' ... ', $out) . ' ...';
 
   // Highlight keywords. Must be done at once to prevent conflicts ('strong' and '<strong>').
-  $text = preg_replace('/' . $boundary . '(' . implode('|', $keys) . ')' . $boundary . '/iu', '<strong>\0</strong>', $text);
+  $text = preg_replace('/' . $boundary . '[^' . PREG_CLASS_SEARCH_EXCLUDE . PREG_CLASS_CJK . ']*' . '(' . implode('|', $keys) . ')' . '[^' . PREG_CLASS_SEARCH_EXCLUDE . PREG_CLASS_CJK . ']*' . $boundary . '/iu', '<strong>\0</strong>', $text);
   return $text;
 }

N-grams are NOT the same as stemming algorithms at all. Stemming algorithms are language-specific ways to linguistically reduce a word to its basic root, which is done to both the search terms and the text, and may not result in an actual sub-string of the original words.

Sorry for not being clearer, but u should assume people are this dumb :-D. I was trying to point out I had to accomplish exactly what you have here and it did it by that Regex ^^^ which is considerably smaller then what you have here, and is utf-8 safe.

If this really does involve an API change, we may need to push it back to D8 now, sadly...

This is rather cool.

Have sort of got it working on Drupal 6.x after a bit of hacking. I'm assuming the latest code in porterstemmer_sbp_excerpt_match() from porterstemmer 6.x-2.5 is what should be used rather than what's in porterstemmer_search_excerpt_match() from #16 above?

I hit a problem with the line
if ($foundstem == $key) {
since this test will fail if there are differences in capitalisation.

Also am I right in thinking that if an exact match for a $key is found -- in the new search_excerpt() -- then any potential matches of the stemmed $key prior to this will be missed?

[Currently I'm experimenting with a custom module which implements mymodule_preprocess_search_result() to override the default snippet - this seems to provide a practical way of getting this working in 6.x]

Thanks!

>Porter Stemmer lower cases everything before it does any stemming
It looks as though the lowercasing happens in porterstemmer_search_preprocess http://drupalcode.org/viewvc/drupal/contributions/modules/porterstemmer/..., rather than in porterstemmer_stem. So $foundstem (http://drupalcode.org/viewvc/drupal/contributions/modules/porterstemmer/...) can have caps in it, as can $key. Using

if (drupal_strtolower($foundstem) == drupal_strtolower($key)) {

at line 105 fixed this problem for me. I've opened an issue against porterstemmer for this.

#850950: capitalization can cause porterstemmer_sbp_excerpt_match() to miss matches

re. the other question, yes I need to do some proper tests for this. Probably another day!!!!

Subscribe

Thanks jhodgdon, #850950-7: capitalization can cause porterstemmer_sbp_excerpt_match() to miss matches takes care of the capitalization problem. While I was testing that I also investigated the problem I alluded to at #27 and #29 above.

A node has content "qqQqqeateat qqQqqeating qqQqqeat hello world"

When I search for "qqqqqeat" I get the following snippet:

qqQqqeateat qqQqqeating qqQqqeat hello world

(I'm using the latest code from Search by Page instead of #15/#16.)

What seems to be happening is that the exact match is taking priority, and then, provided our excerpt length is under 256 characters, and having tried any other keys, the code only looks for any *subsequent* matches against that particular key. If the node has the words in a different order "qqQqqeat qqQqqeateat qqQqqeating hello world" then I do get qqQqqeating highlighted as well i.e. the snippet is qqQqqeat qqQqqeateat qqQqqeating hello world, because having found the exact match (the "bare keyword") there is a *subsequent* valid stem match.

Well, then, I reattach here the patch I originally wrote for #916086: search_excerpt() doesn't highlight words that are matched via search_simplify() as it addresses search_excerpt not supporting matches made via search_simplify().

I'm not convinced that we should focus on it respecting stemmed matches, since that could be handled by the contributed modules themselves. Handling stemmed excerpts is a feature to me -- not handling search_simplify and diacritics excerpting is a bug.

Lastly, I'll reiterate my point from #731298: Searches for words with diacritics/accents: word not highlighted in results that the diacritics issue has nothing to do with search_excerpt, and everything to do with mysql collation, and so perhaps should be handled separately by stripping diacritics entirely in the index. jhodgdon, you seem to disagree since you closed it as a dupe -- can you explain? Or do you think we should do fix that problem in this thread anyway?

Yep, you're right: this doesn't handle quoted keywords correctly. I'll take a stab at that in the near future, and add a test for it as well. I'm not sure other tests are needed, because it's not as if I'm just testing numbers here -- I'm testing the use of search_simplify in general (which is tested in all of its variations elsewhere). Thus just needs to make sure that changes made by search_simplify are still excerpted.

I see your point re: contributed modules using the search API, and I think it would likely involve adding a new API call in this patch to allow other modules to have a say about excerpting. But I'm still not convinced that this should hold up the rest of this thread, which is focused on a core bug caused by search_simplify (and nothing else in core as far as I know, if you accept my diacritics argument). I don't see why a contributed module couldn't just add a new preprocess variable to search_result.tpl.php to solve this itself. Again, I'm not against adding this functionality, but I don't see it as important as the search_simplify bug fix. But when I get to the above fix, I'll take a stab at this in case it's fairly easy to do.

I re-opened #731298: Searches for words with diacritics/accents: word not highlighted in results and removed diacritics from the subject line here.

Well, in any case, it does turn out that quoted keywords are a problem when search_simplify gets involved. That is, if you have "one: two" in the text and search for "one two," my above patch wasn't highlighting the phrase appropriately. I've now fixed this issue, and from some tests it looks like the "abc,def" and "abc--def" situation isn't a problem. I'll add these to the testing suite.

I've looked at your patch and I agree that ideally I should combine my patch with yours. That is, search should implement search_excerpt_match to find search_simplify() related matches. However, I do think this might involve slightly tweaking your patch to allow an array of keywords to be returned, as opposed to just one, to catch the "one: two" situation mentioned above.

Assuming the merge works well, hopefully webchick/dries will be cool with adding a new API call (since it fixes a bug and it won't break any contrib functionality). If not, it wouldn't be too hard to post a version w/o it, but it would only work with search_simplify and not contributed stemmers, etc until D8.

No, it doesn't currently do that when search_simplify is modifying one of the words, at least not in my current sandbox at HEAD.

To reproduce:

1) Create a node with body: "Word follows: this"

2) Run cron

3) Search for "follows this" and the node will be returned, but with nothing highlighted. If you search for "follows: this", however, it gets highlighted.

But as I said, I believe I've already solved this, because w/o search_simplify getting involved this wouldn't be an issue.

Sorry, that wasn't well put. I was just stating the obvious, which is just that search_simplify() strips out punctuation and hence searching "follows" will find "follows:" -- so core is responsible for making sure that "follows this" will highlight "follows: this".

I think I've solved it in the patch I'm working on, but I still need to add more tests and integrate it with your patch by putting it inside of search_search_excerpt_match. But I've attached the latest version if you want to check it out.

OK, I've added an implementation of the search_excerpt_match hook to find matches made via search_simplify(). I ended up having to rewrite the code I originally wrote, and I think the version I have now is much more robust. Hit me on IRC if you want to discuss the algorithm.

I've also added more tests -- five fail without the hook implementation.

Sorry. I really do think it's too late for this. :( API freeze was over a year ago.

I see in #34 where this was changed back to 7.x, that this fixes some problems, but doesn't explain why this needs to happen via a new hook.

#916086: search_excerpt() doesn't highlight words that are matched via search_simplify() seems to be addressing the same issue without an API change. Is it worth looking at that again?

This eventually needs to happen via a new hook in order to allow proper excerpting for 3rd party stemmers, but if we can't have that in 7.x, then I think we should just stick with solving the search_simplify issue. The patch here had some algorithmic improvements over the patch over there, so I'll work on a new version that solves the search_simplify issue without the API change, and then for 8.x we can add the new hook.

This should get a new title then so as not to be confused with the newly reopened issue.

#17: 493270.patch queued for re-testing.

Just wondering what the status of this issue is. I guess the most recent patch is at #44 but now that #916086: search_excerpt() doesn't highlight words that are matched via search_simplify() has gone in to 8.x (and 7.x) I guess #44 needs updating? Also is it best to leave #731298: Searches for words with diacritics/accents: word not highlighted in results in its own issue?

Suppose we should introduce a kind of hook to alter data before indexing and make the same for excerpt generation

did it fix it, or there is still a bug here?