[search] Improve search ranking behavior #1661

2021-12-06T04:38:46Z

bnitkin commented

2021-12-06 04:38:46 +00:00

(Migrated from github.com)

This PR fixes some bugs in search rankings, and should make search more usable, especially for partial matches and buildings whose numbers aren't yet in OSM. I think it'll help with some of the issues linked in #1560

Here's a summary of changes:

Added new synonyms

Search can now recognize rt=route, ct=court, and a few other synonyms.

Disabled m_prefix

(Reverted this to prior behavior with better understanding of its role)
The current version of search pops the last word of a query and saves it to a variable called m_prefix. The prefix token was used differently than the rest of the query when ranking results, so a search for "S Valley Way" would be more-or-less truncated to "S Valley". As a result, the search was unable to differentiate between Way/St/Circle/etc.

~~Searching for an address as "1600 Pennsylvania Ave" is very common, and the prefix handling made that very inaccurate.~~

That said... I'm American. I'm not sure if the prefix handling serves an important role for users of other nationalities. Let me know if so and I'll revisit it. (All code to handle prefixes is still there; I just disabled it in search/processor.cpp)

Fixed matchedLength

The current search computes matchLength based on the name of matched feature. That meant a search for "S Xenia St" could match "South Valley Highway" - they share the "South". But the highway is 18 characters long. That produced a matchedFraction of 18/8, which incorrectly increased the relevance of that match.

The new implementation computes and stores matchedLength as part of the GetNameScores routine. It computes match length on a token-by-token basis and uses the query length instead of feature length. In the prior example, that means that only the letter S matches. That yields a 1/8 matchedFraction score, which is much more reasonable.

Migrating matchedLength into nameScores allows GetNameScores to return just a NameScores instead of a pair<NameScores, size_t>. That simplified accessing the data.

The change also allowed for removing a few other places that computed matchLength, such as ranker.cpp:195.

Leave suggestions available as results

The current implementation removes typing suggestions from search results. While that prevents duplication, it also keeps users from accessing the highly-ranked result. They'd have to click the suggestion and wait for the search to rerun.

The new implementation leaves suggestions alone, but also presents the suggestions as normal results.

Rewrote `GetNameScores`

Most of the ranking logic lies in the ranking_utils.hpp implementation of GetNameScores. I was struggling to understand it as-was due to nondescript variable names and missing comments. The new version is heavily commented and behaves very similarly in unit testing.

Unit tests

There are a few new test cases to explicitly test synonyms (s/south, st/street, etc). All tests now check matchedLength in addition to error count. ~~One was removed:~~

   test("San Francisco", "Fran ", TokenRange(0, 1), NAME_SCORE_ZERO, 0);

~~failed since the tokenizing process strips spaces. Once spaces are removed, "Fran" does match "San Francisco".~~
(Reenabling m_prefix tokenization restored this test)

Let me know what you think! Between playing around in unit testing and the desktop app, I'm much happier with the search results I'm seeing.

This PR fixes some bugs in search rankings, and should make search more usable, especially for partial matches and buildings whose numbers aren't yet in OSM. I think it'll help with some of the issues linked in #1560 Here's a summary of changes: # Added new synonyms Search can now recognize `rt`=route, `ct`=court, and a few other synonyms. # Disabled m_prefix (Reverted this to prior behavior with better understanding of its role) ~~The current version of search pops the last word of a query and saves it to a variable called m_prefix. The prefix token was used differently than the rest of the query when ranking results, so a search for "S Valley Way" would be more-or-less truncated to "S Valley". As a result, the search was unable to differentiate between Way/St/Circle/etc.~~ ~~Searching for an address as "1600 Pennsylvania Ave" is very common, and the prefix handling made that very inaccurate.~~ ~~That said... I'm American. I'm not sure if the prefix handling serves an important role for users of other nationalities. Let me know if so and I'll revisit it. (All code to handle prefixes is still there; I just disabled it in `search/processor.cpp`)~~ # Fixed matchedLength The current search computes matchLength based on the name of matched feature. That meant a search for "S Xenia St" could match "South Valley Highway" - they share the "South". But the highway is 18 characters long. That produced a matchedFraction of 18/8, which incorrectly increased the relevance of that match. The new implementation computes and stores matchedLength as part of the GetNameScores routine. It computes match length on a token-by-token basis and uses the query length instead of feature length. In the prior example, that means that only the letter S matches. That yields a 1/8 matchedFraction score, which is much more reasonable. Migrating matchedLength into nameScores allows `GetNameScores` to return just a `NameScores` instead of a `pair<NameScores, size_t>`. That simplified accessing the data. The change also allowed for removing a few other places that computed matchLength, such as `ranker.cpp:195`. # Leave suggestions available as results The current implementation removes typing suggestions from search results. While that prevents duplication, it also keeps users from accessing the highly-ranked result. They'd have to click the suggestion and wait for the search to rerun. The new implementation leaves suggestions alone, but also presents the suggestions as normal results. # Rewrote `GetNameScores` Most of the ranking logic lies in the ranking_utils.hpp implementation of `GetNameScores`. I was struggling to understand it as-was due to nondescript variable names and missing comments. The new version is heavily commented and behaves very similarly in unit testing. # Unit tests There are a few new test cases to explicitly test synonyms (s/south, st/street, etc). All tests now check matchedLength in addition to error count. ~~One was removed:~~ ``` test("San Francisco", "Fran ", TokenRange(0, 1), NAME_SCORE_ZERO, 0); ``` ~~failed since the tokenizing process strips spaces. Once spaces are removed, "Fran" does match "San Francisco".~~ (Reenabling m_prefix tokenization restored this test) Let me know what you think! Between playing around in unit testing and the desktop app, I'm much happier with the search results I'm seeing.

👍 1

bnitkin (Migrated from github.com) reviewed 2021-12-06 05:36:08 +00:00

search/ranking_info.cpp

					
				@ -45,1 +45,4 @@

				};

				// @todo These are worth reevaluating. A few issues (i.e. 1376) say

				// that distant cities outrank nearby buildings & SUBPOIs when searching.

				// Adjusting kDistanceToPivot or the values below would help with that.

bnitkin (Migrated from github.com) commented

2021-12-06 05:36:08 +00:00

It might be worth changing more of these. A few issues in the tracker talk about getting city entries from far away. Reducing the city/state/country bonuses below, or increasing kCategoriesDistanceToPivot would help with that.

It might be worth changing more of these. A few issues in the tracker talk about getting city entries from far away. Reducing the city/state/country bonuses below, or increasing `kCategoriesDistanceToPivot` would help with that.

vng (Migrated from github.com) reviewed 2021-12-06 06:37:53 +00:00

vng (Migrated from github.com) left a comment

Thanks for the PR! Let's wait for the CI tests first. Search ranking is very sensitive part and unit/integration tests are the best/only quality criteria here.

search/processor.cpp

vng (Migrated from github.com) commented

2021-12-06 06:33:36 +00:00

I have doubts here, because m_prefix is not only for ranking, but for fetching results/suggestions first.
m_tokens work like full match, m_prefix works like prefix match.

Try "penselv" search query and check the suggestions.

I have doubts here, because m_prefix is not only for ranking, but for fetching results/suggestions first. m_tokens work like _full match_, m_prefix works like _prefix match_. Try "penselv" search query and check the suggestions.

bnitkin (Migrated from github.com) reviewed 2021-12-06 14:52:49 +00:00

search/processor.cpp

bnitkin (Migrated from github.com) commented

2021-12-06 14:52:49 +00:00

Got it. I'll try turning prefixing back on and looking around a little. The prefix token was definitely being ignored in GetNameScores, but I can probably fix that less invasively.

I'll go back and take a look at the tests. I was focusing on search_tests, but it looks like I broke some of the search_quality_tests.

Got it. I'll try turning prefixing back on and looking around a little. The prefix token was definitely being ignored in `GetNameScores`, but I can probably fix that less invasively. I'll go back and take a look at the tests. I was focusing on `search_tests`, but it looks like I broke some of the `search_quality_tests`.

biodranik (Migrated from github.com) reviewed 2021-12-06 21:21:06 +00:00

search/query_params.cpp

biodranik (Migrated from github.com) commented

2021-12-06 21:21:06 +00:00

Is this log really necessary? It would make sense to log matched synonym.

bnitkin (Migrated from github.com) reviewed 2021-12-06 22:39:44 +00:00

search/query_params.cpp

bnitkin (Migrated from github.com) commented

2021-12-06 22:39:43 +00:00

Nope. I'd meant to take that out when I finished debugging why S/South type matches weren't working. I'll push a new round of changes later today.

bnitkin (Migrated from github.com) reviewed 2021-12-07 15:41:55 +00:00

search/query_params.cpp

bnitkin (Migrated from github.com) commented

2021-12-07 15:41:55 +00:00

Fixed in 1da320 - I just removed the logging statement.

bnitkin (Migrated from github.com) reviewed 2021-12-07 15:42:52 +00:00

search/processor.cpp

bnitkin (Migrated from github.com) commented

2021-12-07 15:42:52 +00:00

Reverted in 1da320 - adding synonym matching to the prefix token fixed the issues I was seeing with st/rd/etc matching without breaking the PREFIX nameScore.

vng (Migrated from github.com) reviewed 2021-12-07 20:46:56 +00:00

search/ranking_utils.hpp

vng (Migrated from github.com) commented

2021-12-07 20:42:25 +00:00

Use "const &" in both functions.

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-07 20:44:42 +00:00

Not sure here, but probably sequenced equal calls check for no side effect .. But, most likely, they are useless.

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-07 20:45:44 +00:00

Add "@todo" marker here

bnitkin (Migrated from github.com) reviewed 2021-12-07 21:26:25 +00:00

search/search_integration_tests/processor_test.cpp

bnitkin (Migrated from github.com) commented

2021-12-07 21:26:24 +00:00

I think it's testing the trailing space. Adding a trailing space forces a normal match instead of an m_prefixToken prefix match.

But abcd doesn't normal-match to abcdstr since there are three errors and length-four tokens only allow one error. It works for prefix matching since those accept partial matches.

I think it's testing the trailing space. Adding a trailing space forces a normal match instead of an `m_prefixToken` prefix match. But `abcd` doesn't normal-match to `abcdstr` since there are three errors and length-four tokens only allow one error. It works for prefix matching since those accept partial matches.

vng (Migrated from github.com) reviewed 2021-12-07 21:32:25 +00:00

search/ranking_utils.hpp

					
				@ -182,0 +208,4 @@

				      if (sliceCount == tokenCount)

				          nameScore = NAME_SCORE_FULL_MATCH;

				      else

				          nameScore = NAME_SCORE_PREFIX;

vng (Migrated from github.com) commented

2021-12-07 21:32:24 +00:00

The correspondent .cpp functions should also be fixed.

biodranik (Migrated from github.com) approved these changes 2021-12-07 21:44:14 +00:00

biodranik (Migrated from github.com) left a comment

nit comments

search/query_params.cpp

					
				@ -29,3 +35,4 @@

				    {"rt",   {"route"}},

				    {"св",   {"святой", "святого", "святая", "святые", "святых", "свято"}},

				    {"б",    {"большая", "большой"}},

				    {"бол",  {"большая", "большой"}},

biodranik (Migrated from github.com) commented

2021-12-07 21:34:12 +00:00

@vng in Germany and Switzerland "str" is used for "strasse" (street).

biodranik (Migrated from github.com) commented

2021-12-07 21:35:38 +00:00

Should these synonyms check the language of the map region and use only corresponding translation? @TODO it in the issue?

search/ranker.cpp

biodranik (Migrated from github.com) commented

2021-12-07 21:38:11 +00:00

Why is it not removed now?

search/ranking_info.cpp

					
				@ -45,1 +45,4 @@

				};

				// @todo These are worth reevaluating. A few issues (i.e. 1376) say

				// that distant cities outrank nearby buildings & SUBPOIs when searching.

				// Adjusting kDistanceToPivot or the values below would help with that.

biodranik (Migrated from github.com) commented

2021-12-07 21:38:52 +00:00

Please add it as a todo comment.

search/ranking_utils.hpp

biodranik (Migrated from github.com) commented

2021-12-07 21:39:29 +00:00

    auto const matchedLengthsAreSimilar = (m_matchedLength - m_matchedLength / 4) <= rhs.m_matchedLength;

```suggestion auto const matchedLengthsAreSimilar = (m_matchedLength - m_matchedLength / 4) <= rhs.m_matchedLength; ```

search/ranking_utils.hpp

biodranik (Migrated from github.com) commented

2021-12-07 21:40:53 +00:00

Space after // here and around.

biodranik (Migrated from github.com) commented

2021-12-07 21:41:59 +00:00

      size_t const tokenIndex = i + (tokenCount - 1) - offset;

```suggestion size_t const tokenIndex = i + (tokenCount - 1) - offset; ```

biodranik (Migrated from github.com) commented

2021-12-07 21:43:25 +00:00

Is enum necessary here?

Is `enum` necessary here?

search/ranking_utils.hpp

biodranik (Migrated from github.com) commented

2021-12-06 21:31:33 +00:00

nit: omit {} for single block bodies.

search/search_integration_tests/processor_test.cpp

biodranik (Migrated from github.com) commented

2021-12-07 21:31:38 +00:00

Please align all comments here and below (add space after //)

biodranik commented

2021-12-07 21:45:23 +00:00

(Migrated from github.com)

Please rebase on top of the master branch, check and fix all GH action check errors.

vng (Migrated from github.com) reviewed 2021-12-07 21:56:49 +00:00

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-07 21:56:48 +00:00

Ah, I see! Well, but it should, because "straße" suffix is generated as a separate token. This test is valid and should work.

vng (Migrated from github.com) reviewed 2021-12-07 21:59:12 +00:00

search/query_params.cpp

					
				@ -29,3 +35,4 @@

				    {"rt",   {"route"}},

				    {"св",   {"святой", "святого", "святая", "святые", "святых", "свято"}},

				    {"б",    {"большая", "большой"}},

				    {"бол",  {"большая", "большой"}},

vng (Migrated from github.com) commented

2021-12-07 21:59:11 +00:00

strasse (straße) is a special case and it's processed separately.
See AddStrasseNames in search_index_builder.cpp

strasse (straße) is a special case and it's processed separately. See AddStrasseNames in search_index_builder.cpp

biodranik (Migrated from github.com) reviewed 2021-12-07 22:20:50 +00:00

search/query_params.cpp

					
				@ -29,3 +35,4 @@

				    {"rt",   {"route"}},

				    {"св",   {"святой", "святого", "святая", "святые", "святых", "свято"}},

				    {"б",    {"большая", "большой"}},

				    {"бол",  {"большая", "большой"}},

biodranik (Migrated from github.com) commented

2021-12-07 22:20:50 +00:00

@bnitkin it is worth being added as a comment here.

bnitkin (Migrated from github.com) reviewed 2021-12-08 15:15:00 +00:00

search/query_params.cpp

					
				@ -29,3 +35,4 @@

				    {"rt",   {"route"}},

				    {"св",   {"святой", "святого", "святая", "святые", "святых", "свято"}},

				    {"б",    {"большая", "большой"}},

				    {"бол",  {"большая", "большой"}},

bnitkin (Migrated from github.com) commented

2021-12-08 15:14:59 +00:00

@todo is a good idea. I was thinking about matching languages, but don't know how to get the map region language from QueryParams::AddSynonyms().

@todo is a good idea. I was thinking about matching languages, but don't know how to get the map region language from `QueryParams::AddSynonyms()`.

bnitkin (Migrated from github.com) reviewed 2021-12-08 15:24:49 +00:00

search/ranker.cpp

bnitkin (Migrated from github.com) commented

2021-12-08 15:24:49 +00:00

This fixes the third bullet in #1376. vec is a shallow copy of the search results, so erasing the element means there's an autocomplete match for the feature but not a result the user can select.

The attached image shows an example. In the PR version (top) "S Quebec Street" is present both as an autocomplete hint and as the second result. In the bottom (current), it's only a suggestion. That forces the user to click the suggestion and rerun the search.

This fixes the third bullet in #1376. `vec` is a shallow copy of the search results, so erasing the element means there's an autocomplete match for the feature but not a result the user can select. The attached image shows an example. In the PR version (top) "S Quebec Street" is present both as an autocomplete hint and as the second result. In the bottom (current), it's only a suggestion. That forces the user to click the suggestion and rerun the search. ![2021-12-08-082314_491x772_scrot](https://user-images.githubusercontent.com/4681424/145234684-d93720fd-1c04-46ce-829e-596f58bf9a5a.png)

bnitkin (Migrated from github.com) reviewed 2021-12-08 15:27:21 +00:00

search/ranking_utils.hpp

bnitkin (Migrated from github.com) commented

2021-12-08 15:27:21 +00:00

Doesn't look it. I'll get rid of it.

vng (Migrated from github.com) reviewed 2021-12-08 15:32:43 +00:00

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-08 15:32:43 +00:00

This test should work, because "strasse" suffix is separately processed in generator. Is this test-case breaks your logic somehow?

vng (Migrated from github.com) reviewed 2021-12-08 15:32:59 +00:00

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-08 15:32:59 +00:00

This test should work, because "strasse" suffix is separately processed in generator. Is this test-case breaks your logic somehow?

bnitkin (Migrated from github.com) reviewed 2021-12-08 16:50:02 +00:00

search/search_integration_tests/processor_test.cpp

bnitkin (Migrated from github.com) commented

2021-12-08 16:50:02 +00:00

Hm. Going back to the master branch, there are two flavors of prefix matches, and I only support one right now. I'll fix that.

One case is where the prefix token partially matches the result token, like below. That's supported in my code.

  test("фотоателье", "фото", TokenRange(0, 1), NAME_SCORE_PREFIX, 0);

The other case is where a non-prefix token matches the start of a query:

  test("San Francisco", "San ", TokenRange(0, 1), NAME_SCORE_PREFIX, 0);

I'm currently parsing that as a SUBSTRING match, but as a human, PREFIX makes more sense.

Let me think on how to write that without adding too many special cases. I'll have something soon.

Hm. Going back to the master branch, there are two flavors of prefix matches, and I only support one right now. I'll fix that. One case is where the prefix token partially matches the result token, like below. That's supported in my code. ``` test("фотоателье", "фото", TokenRange(0, 1), NAME_SCORE_PREFIX, 0); ``` The other case is where a non-prefix token matches the start of a query: ``` test("San Francisco", "San ", TokenRange(0, 1), NAME_SCORE_PREFIX, 0); ``` I'm currently parsing that as a SUBSTRING match, but as a human, PREFIX makes more sense. Let me think on how to write that without adding too many special cases. I'll have something soon.

vng (Migrated from github.com) reviewed 2021-12-08 17:04:03 +00:00

search/search_integration_tests/processor_test.cpp

vng (Migrated from github.com) commented

2021-12-08 17:04:03 +00:00

Well, "фото " should not match "фотоателье". But "abcdstrasse" is an exception because of German language specific.

Generator does index for "abcdstrasse" as 2 separate tokens like "abcd" and "strasse", so "abcd " query will match the first token.

Well, "фото " should not match "фотоателье". But "abcdstrasse" is an exception because of German language specific. Generator does index for "abcdstrasse" as 2 separate tokens like "abcd" and "strasse", so "abcd " query will match the first token.

bnitkin (Migrated from github.com) reviewed 2021-12-08 17:53:29 +00:00

search/search_integration_tests/processor_test.cpp

bnitkin (Migrated from github.com) commented

2021-12-08 17:53:29 +00:00

I think this is the same issue as in the "abcd " test - I'll fix my ranking code and put them both back in.

vng commented

2021-12-08 18:58:19 +00:00

(Migrated from github.com)

Great, waiting for checks. And the last commit is bad. You should make "git submodule update --recursive" after rebase, instead of making a new commit with old boost ref.

vng (Migrated from github.com) approved these changes 2021-12-08 20:32:05 +00:00

vng (Migrated from github.com) left a comment

CI checks are ok. I approve, but remove boost submodule change and merge all changes as 1 commit!

bnitkin commented

2021-12-08 22:09:39 +00:00

(Migrated from github.com)

I force-undid the submodule commit. Sorry about that - I'm not familiar with submodules. It's rebuilding & running tests locally now.

Should I squash the branch down to one commit, or do you make that happen on merge?

Thanks for all the feedback! I'm really excited to see this change in a release.

I force-undid the submodule commit. Sorry about that - I'm not familiar with submodules. It's rebuilding & running tests locally now. Should I squash the branch down to one commit, or do you make that happen on merge? Thanks for all the feedback! I'm really excited to see this change in a release.

vng commented

2021-12-08 22:13:49 +00:00

(Migrated from github.com)

Squash by yourself, please.

bnitkin commented

2021-12-08 22:37:28 +00:00

(Migrated from github.com)

Squished and retested locally. Let me know if you need anything else from me on this!

vng (Migrated from github.com) reviewed 2021-12-09 03:39:47 +00:00

search/ranking_utils.hpp

vng (Migrated from github.com) commented

2021-12-09 03:37:05 +00:00

Comment this log, please.

bnitkin (Migrated from github.com) reviewed 2021-12-09 04:00:24 +00:00

search/ranking_utils.hpp

bnitkin (Migrated from github.com) commented

2021-12-09 04:00:23 +00:00

Can do.

vng commented

2021-12-09 11:25:58 +00:00

(Migrated from github.com)

@bnitkin Congrats with your first merged PR!

biodranik (Migrated from github.com) reviewed 2021-12-09 21:26:28 +00:00

search/ranker.cpp

biodranik (Migrated from github.com) commented

2021-12-09 21:26:28 +00:00

Wait, but then the result (if it is NOT a street, like in #1376) should be removed from the suggestion: existing logic should be inverted. Why user needs the result (not a street) in the suggestion? The case for streets is to type the full address with fewer clicks. That's why they need a suggestion (and you're right, they shouldn't be displayed in results).

biodranik (Migrated from github.com) reviewed 2021-12-09 21:30:30 +00:00

search/search_integration_tests/processor_test.cpp

biodranik (Migrated from github.com) commented

2021-12-09 21:30:30 +00:00

To clarify about german streets: they are often named like "Bahnhofstraße" (ß is normalized to "ss"). But people often type them as "Bahnhof strasse" or "Bahnhofstr" or "Bahnhofstr."

bnitkin (Migrated from github.com) reviewed 2021-12-10 00:05:57 +00:00

search/ranker.cpp

bnitkin (Migrated from github.com) commented

2021-12-10 00:05:57 +00:00

I think it already works that way - suggestions require the RankerResult to be a country, city, or street:

matcher.cpp:879     if (type == ftypes::LocalityType::Country || type == ftypes::LocalityType::City || r.IsStreet())

But I'm not sure how that intersects with #1376. Maybe there's a street that's also called Carrefour that was converted to a suggestion, and the search never found the SUBPOI?

I think it already works that way - suggestions require the `RankerResult` to be a country, city, or street: ``` matcher.cpp:879 if (type == ftypes::LocalityType::Country || type == ftypes::LocalityType::City || r.IsStreet()) ``` But I'm not sure how that intersects with #1376. Maybe there's a street that's also called Carrefour that was converted to a suggestion, and the search never found the SUBPOI?

vng (Migrated from github.com) reviewed 2021-12-10 08:13:56 +00:00

search/ranker.cpp

vng (Migrated from github.com) commented

2021-12-10 08:13:56 +00:00

Well, this change is not about #1376.
The logic of "i = vec.erase(i);" - If some street (country, city) result goes to the suggestion part, it will NOT go to the results list. Actually, I can't say for sure is it good or bad, so I also agree to change this logic and test.
Now we have "South Quebec Street" as a suggestion AND as a result (see highway-secondary).

Well, this change is not about #1376. The logic of "i = vec.erase(i);" - If some street (country, city) result goes to the suggestion part, it will NOT go to the results list. Actually, I can't say for sure is it good or bad, so I also agree to change this logic and test. Now we have "South Quebec Street" as a suggestion AND as a result (see highway-secondary).

biodranik (Migrated from github.com) reviewed 2021-12-10 12:42:16 +00:00

search/ranker.cpp

biodranik (Migrated from github.com) commented

2021-12-10 12:42:16 +00:00

What about this approach?

Query matches the same result and suggestion partially: display both result and suggestion in all cases except one: do not display any street type as a result (people do not usually search for streets alone).
Query fully matches the same result and suggestion and doesn't have a space at the end: the same as (1) above.
Query fully matches the same result and suggestion and have a space at the end: display the result only (even if it's a street type).

What about this approach? 1. Query matches the same result and suggestion partially: display both result and suggestion in all cases except one: do not display any street type as a result (people do not usually search for streets alone). 2. Query fully matches the same result and suggestion and doesn't have a space at the end: the same as (1) above. 3. Query fully matches the same result and suggestion and have a space at the end: display the result only (even if it's a street type).

bnitkin (Migrated from github.com) reviewed 2021-12-11 20:10:13 +00:00

search/ranker.cpp

bnitkin (Migrated from github.com) commented

2021-12-11 20:10:13 +00:00

At least for the US, I think it's important to present streets as results.

As mentioned in #1263, lots of people have trouble when the address they entered doesn't exist. In the US, there are many areas where roads are mapped fully but street numbers aren't. Returning the street as a result is an easy fallback to get a user close to the right place.

That's what I was thinking in including the suggestions as results. (It'd obviously be nicer if all buildings existed and had addresses, but that's a much larger project.)

At least for the US, I think it's important to present streets as results. As mentioned in #1263, lots of people have trouble when the address they entered doesn't exist. In the US, there are many areas where roads are mapped fully but street numbers aren't. Returning the street as a result is an easy fallback to get a user close to the right place. That's what I was thinking in including the suggestions as results. (It'd obviously be nicer if all buildings existed and had addresses, but that's a much larger project.)

biodranik (Migrated from github.com) reviewed 2021-12-11 22:32:12 +00:00

search/ranker.cpp

biodranik (Migrated from github.com) commented

2021-12-11 22:32:12 +00:00

Users will see their street as a suggestion (but not in results, saving potential space for another relevant result or suggestion). And they click on the suggested street. And it will appear in results after the click was done (and disappear from suggestions).

Users will see their street as a suggestion (but not in results, saving potential space for another relevant result or suggestion). And they click on the suggested street. And it will appear in results _after_ the click was done (and disappear from suggestions).

biodranik (Migrated from github.com) reviewed 2021-12-11 22:45:06 +00:00

search/ranker.cpp

biodranik (Migrated from github.com) commented

2021-12-11 22:45:06 +00:00

The main idea with suggestions is to save the number of clicks and avoid potential mistypes. The most popular pattern is to type several first street letters, then click a suggestion, and then type the house number.
In the US, however, people usually type the number first and then type the street name (and here we don't show suggestions).

If there is a house with a number, then it should be in results (and no street alone, of course). If there is no house, then only street should be displayed in results (but not in suggestion in both cases).

The main idea with suggestions is to save the number of clicks and avoid potential mistypes. The most popular pattern is to type several first street letters, then click a suggestion, and then type the house number. In the US, however, people usually type the number first and then type the street name (and here we don't show suggestions). If there is a house with a number, then it should be in results (and no street alone, of course). If there is no house, then only street should be displayed in results (but not in suggestion in both cases).

vng (Migrated from github.com) reviewed 2021-12-12 07:13:24 +00:00

search/ranker.cpp

vng (Migrated from github.com) commented

2021-12-12 07:13:24 +00:00

Turned out that suggestion only logic is confusing a bit. Also suggestions have limits.
If we search for street only with suggestions, we don't have fuzzy (partial) matching in this case.

I'd like to check how it will go with street results. And I like it so far.

- Turned out that suggestion only logic is confusing a bit. Also suggestions have limits. - If we search for street only with suggestions, we don't have fuzzy (partial) matching in this case. I'd like to check how it will go with street results. And I like it so far.

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik

vng