feat(stats): apply nbSortedHits

[eunjae-lee]

Summary

This PR updates stats widget to reflect nbSortedHits when 0 < appliedRelevancyStrictness <= 100.

Result

• When smart sorted: 150 relevant results sorted out of 1,234 found in 42ms
• Normally: 1 result found in 42ms

storybook: https://deploy-preview-4649--instantsearchjs.netlify.app/stories/?path=/story/metadata-stats--with-sorted-hits

related: #4648

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 72c9d48:

Sandbox	Source
InstantSearch.js	Configuration

[Haroenv]

looks good, but let's release this once the feature is ready to be released stable (together with doc etc.) wdyt?

also update bundlesize

[eunjae-lee]

looks good, but let's release this once the feature is ready to be released stable (together with doc etc.) wdyt?

also update bundlesize

Yeah I forgot to mention it. I will merge this once smartSort is merged onto master.
And the bundlesize will be updated from that commit.

[shortcuts]

I don't really know how the templates work, but I think we should handle the case where nbHits === nbSortedHits, to avoid this kind of sentences 10,000 relevant results sorted out of 10,000 found in 1ms

[Haroenv]

yes, that's an important note! isSmartSorted should be false in that case IMO

[shortcuts]

I just handled it like that in RIS, should I fix it before merging smartSort?

[Haroenv]

I think so, but @eunjae-lee and you have been closer to the implementation of this feature, wdyt?

[shortcuts]

If we think isSmartSorted reflect what the engine says, it should stay like that. But it make sense to consider that if the number of results are the same we consider it has not smart sorted. 🤔

[Haroenv]

I disagree, empty query for example looks very odd now on a smart sort, and should be handled as a non-smart sort.

Similarly for example if you search for an item with "paginationLimitedTo" hits, today this will give "paginationLimited to out of nbHits", but that's an engine bug, and should be "nbHits out of nbHits", which will act as if it's not smart sorted (no need to unsort it if the results will be the same).

cc @Ant-hem, what do you think?

[eunjae-lee]

@Haroenv not sure if I understand you correctly.
My suggestion was

With appliedRelevancyStrictness > 0, when nbSortedHits === nbHits:

1. Display the stat as 1000 hits in 10ms (without smart sort mentioned)
2. Display the button as See all results (still as if it's a smart sort)

Which part do you disagree? Do you think we should display the button as See relevant results or not display the button at all?

[Ant-hem]

Chiming there! I don't have a strong opinion on that topic but I can help by giving you some context and pointers on the feature.

The idea behind the banner was to be "transparent" with the end-users as the algorithm can be perceived as a "black-box". That's why I am not really disturbed by the following:

10,000 relevant results sorted out of 10,000 found in 1ms

That's just means all the available hits were selected and then sorted by the Algorithm. I don't see anything weird with it, just the "transparency" we initially wanted.

On top on that, relying on nbHits can be dangerous as this value can sometimes, be interpolated by the engine and thus being volatile/an approximation. See exhaustiveNbHits field in the response.

To finish, one rule of thumb is relevancyStrictness=0 means that the algorithm is disabled and that no results selection/deletion is being done. It's almost (not exactly) equivalent to a hard sort. When relevancyStrictness> 0 the algorithm is enabled and it does select/remove results.

Hope that helps!

[Haroenv]

I think the button should be displayed, so for the context of SmartSort, isSmartSorted is true as long as appliedRelevancyStrictness is not 0, but in the stats widget I think it doesn't make sense to show "x relevant results shown out of x in yms", but it should be "x results found in yms", which correlates with isSmartSorted: false

[eunjae-lee]

@Haroenv agree with what you said, except that two isSmartSorted variables in connectSmartSort and connectStats will have different definitions (but the same name), so confusing.
So I renamed isSmartSorted to areHitsSorted in the context of stats, and added the condition to check nbSortedHits !== nbHits.
3c24187

[Haroenv]

would like a consistent value of isSmartSorted between the two :)