Search: boost phrases in hybrid search (#64367)

This PR passes the 'boost' annotation down to searcher, so that it can
apply phrase boosting. For now, we just pass the boost to the Zoekt
query in hybrid search, which already gives a nice benefit since the
Zoekt results are streamed back first.

Note: this doesn't completely implement boosting in searcher, but it was
really simple and seemed worth it. We're purposefully not investing in
big searcher ranking improvements, since we think a better investment is
to unify logic across Zoekt + searcher.

Author: jtibshirani

cmd/searcher/internal/search/matchtree.go

@@ -8,10 +8,10 @@ import (
 	"strings"
 
 	"github.com/grafana/regexp"
-	"github.com/sourcegraph/zoekt/query"
 	zoektquery "github.com/sourcegraph/zoekt/query"
 
 	"github.com/sourcegraph/sourcegraph/internal/search/casetransform"
+	"github.com/sourcegraph/sourcegraph/internal/search/query"
 	"github.com/sourcegraph/sourcegraph/internal/searcher/protocol"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
@@ -98,7 +98,7 @@ func toRegexpTree(node *protocol.PatternNode, isCaseSensitive bool) (matchTree,
 
 		// OptimizeRegexp currently only converts capture groups into
 		// non-capture groups (faster for stdlib regexp to execute).
-		re = query.OptimizeRegexp(re, syntax.Perl)
+		re = zoektquery.OptimizeRegexp(re, syntax.Perl)
 
 		pattern = re.String()
 	}
@@ -124,6 +124,7 @@ func toRegexpTree(node *protocol.PatternNode, isCaseSensitive bool) (matchTree,
 		re:               re,
 		ignoreCase:       !isCaseSensitive,
 		isNegated:        node.IsNegated,
+		boost:            node.Boost,
 		literalSubstring: literalSubstring,
 	}, nil
 }
@@ -185,6 +186,9 @@ type regexMatchTree struct {
 	// isNegated indicates whether matches on the pattern should be negated (representing a 'NOT' in the query)
 	isNegated bool
 
+	// boost indicates whether the regexp should have its score boosted in Zoekt ranking
+	boost bool
+
 	// literalSubstring is used to test if a file is worth considering for
 	// matches. literalSubstring is guaranteed to appear in any match found by
 	// re. It is the output of the longestLiteral function. It is only set if
@@ -236,8 +240,9 @@ func (rm *regexMatchTree) ToZoektQuery(matchContent bool, matchPath bool) (zoekt
 	}
 	re = zoektquery.OptimizeRegexp(re, syntax.Perl)
 
+	var result zoektquery.Q
 	if matchContent && matchPath {
-		return zoektquery.NewOr(
+		result = zoektquery.NewOr(
 			rm.negateIfNeeded(
 				&zoektquery.Regexp{
 					Regexp:        re,
@@ -250,16 +255,21 @@ func (rm *regexMatchTree) ToZoektQuery(matchContent bool, matchPath bool) (zoekt
 					FileName:      true,
 					CaseSensitive: !rm.ignoreCase,
 				}),
-		), nil
+		)
+	} else {
+		result = rm.negateIfNeeded(
+			&zoektquery.Regexp{
+				Regexp:        re,
+				Content:       matchContent,
+				FileName:      matchPath,
+				CaseSensitive: !rm.ignoreCase,
+			})
 	}
 
-	return rm.negateIfNeeded(
-		&zoektquery.Regexp{
-			Regexp:        re,
-			Content:       matchContent,
-			FileName:      matchPath,
-			CaseSensitive: !rm.ignoreCase,
-		}), nil
+	if rm.boost {
+		result = &zoektquery.Boost{Child: result, Boost: query.ZoektScoreBoost}
+	}
+	return result, nil
 }
 
 func (rm *regexMatchTree) negateIfNeeded(q zoektquery.Q) zoektquery.Q {

cmd/searcher/internal/search/matchtree_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/grafana/regexp"
+	"github.com/hexops/autogold/v2"
 	"github.com/sourcegraph/zoekt/query"
 	"github.com/stretchr/testify/require"
 )
@@ -59,7 +60,8 @@ func TestToZoektQuery(t *testing.T) {
 						isNegated: true,
 					},
 					&regexMatchTree{
-						re: regexp.MustCompile("bbbb*"),
+						re:    regexp.MustCompile("bbbb*"),
+						boost: true,
 					},
 				},
 			},
@@ -74,24 +76,23 @@ func TestToZoektQuery(t *testing.T) {
 		name         string
 		matchContent bool
 		matchPath    bool
-		want         string
+		want         autogold.Value
 	}{{
 		name:         "matches content only",
 		matchContent: true,
 		matchPath:    false,
-		want:         `(and (or (not case_regex:"aaaaa") case_regex:"bbbb*") regex:"cccc?")`,
+		want:         autogold.Expect(`(and (or (not case_regex:"aaaaa") (boost 20.00 case_regex:"bbbb*")) regex:"cccc?")`),
 	}, {
 		name:         "matches path only",
 		matchContent: false,
 		matchPath:    true,
-		want:         `(and (or (not case_file_regex:"aaaaa") case_file_regex:"bbbb*") file_regex:"cccc?")`,
+		want:         autogold.Expect(`(and (or (not case_file_regex:"aaaaa") (boost 20.00 case_file_regex:"bbbb*")) file_regex:"cccc?")`),
+	}, {
+		name:         "matches content and path",
+		matchContent: true,
+		matchPath:    true,
+		want:         autogold.Expect(`(and (or (not case_regex:"aaaaa") (not case_file_regex:"aaaaa") (boost 20.00 (or case_regex:"bbbb*" case_file_regex:"bbbb*"))) (or regex:"cccc?" file_regex:"cccc?"))`),
 	},
-		{
-			name:         "matches content and path",
-			matchContent: true,
-			matchPath:    true,
-			want:         `(and (or (not case_regex:"aaaaa") (not case_file_regex:"aaaaa") case_regex:"bbbb*" case_file_regex:"bbbb*") (or regex:"cccc?" file_regex:"cccc?"))`,
-		},
 	}
 
 	for _, c := range cases {
@@ -100,7 +101,7 @@ func TestToZoektQuery(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			require.Equal(t, c.want, query.Simplify(got).String())
+			c.want.Equal(t, query.Simplify(got).String())
 		})
 	}
 }