Boost symbol matches in BM25 #876
Conversation
| termFreqs := map[string]int{} | ||
| for _, m := range cands { | ||
| term := string(m.substrLowered) | ||
| if m.fileName || p.matchesSymbol(m) { |
There was a problem hiding this comment.
Personally, this is still on the right side of the "black magic" line :) I didn't tune any parameters, just threw in this check and it works well across two eval datasets.
| "testing" | ||
| ) | ||
|
|
||
| func TestCalculateTermFrequency(t *testing.T) { |
There was a problem hiding this comment.
Now calculateTermFrequency requires access to indexData, so it's hard to unit test. I checked the e2e test scoring_test.go carefully to confirm the new calculation is correct.
| @@ -588,6 +588,22 @@ func findMaxOverlappingSection(secs []DocumentSection, off, sz uint32) (uint32, | |||
| return uint32(j), ol1 > 0 | |||
| } | |||
|
|
|||
| func (p *contentProvider) matchesSymbol(cm *candidateMatch) bool { | |||
There was a problem hiding this comment.
We are duplicating some checks, since we run both calculateTermFrequency for the overall file score, plus candidateMatchScore for the individual chunk scores. It would be good to unify these, but I didn't want to embark on a big refactor in this PR.
|
NLS eval results (before vs. after) 
|
When digging into our Natural Language Search (NLS) eval results, I found that one of the leading causes for flexible search types like "Fuzzy symbol search" and "Find logic" was noisy matches in top results. Currently, our BM25 ranking rewards any substring match equally. So for queries like 'extract tar', any match on 'tar' (even within unrelated terms like 'start', etc.) counts towards the term frequency.
This PR helps reduce noise by boosting symbol matches the same as we do filename matches. Our NLS evals show positive improvement, and context evals are the tiniest bit better.
Note: I also tried rewarding matches on word boundaries, taking inspiration from
candidateMatchScore. I did not see any improvement in results by "stacking" this on top of this symbol boost. It also felt like I was really venturing into "overfitting" territory, as it requires a new tunable parameter.Closes SPLF-758