Autocomplete: Ship jaccard similarity changes (#3135)

This PR ships the new jaccard similarity fork. A/B tests show no
regressions and changes over the baseline, so there's no need to have
two implementations.

## Test plan

- This PR just removes a feature flag and dead code.

Author: philipp-spiess

lib/shared/src/experimentation/FeatureFlagProvider.ts

@@ -19,9 +19,6 @@ export enum FeatureFlag {
     // Enables the bfg-mixed context retriever that will combine BFG with the default local editor
     // context.
     CodyAutocompleteContextBfgMixed = 'cody-autocomplete-context-bfg-mixed',
-    // Enables the new-jaccard-similarity context strategy that can find more than one match per
-    // open file and includes matches from the same file.
-    CodyAutocompleteContextNewJaccardSimilarity = 'cody-autocomplete-new-jaccard-similarity',
     // Enable latency adjustments based on accept/reject streaks
     CodyAutocompleteUserLatency = 'cody-autocomplete-user-latency',
     // Dynamically decide wether to show a single line or multiple lines for completions.

vscode/CHANGELOG.md

@@ -12,6 +12,8 @@ This is a log of all notable changes to Cody for VS Code. [Unreleased] changes a
 
 ### Changed
 
+- Autocomplete: Enable the recent jaccard similarity improvements by default. [pull/3135](https://github.com/sourcegraph/cody/pull/3135)
+
 ## [1.4.3]
 
 ### Changed

vscode/src/completions/completion-provider-config.ts

@@ -11,7 +11,6 @@ class CompletionProviderConfig {
 
     private flagsToResolve = [
         FeatureFlag.CodyAutocompleteContextBfgMixed,
-        FeatureFlag.CodyAutocompleteContextNewJaccardSimilarity,
         FeatureFlag.CodyAutocompleteDynamicMultilineCompletions,
         FeatureFlag.CodyAutocompleteHotStreak,
         FeatureFlag.CodyAutocompleteSingleMultilineRequest,
@@ -77,9 +76,6 @@ class CompletionProviderConfig {
         const { config } = this
 
         const bfgMixedContextFlag = this.getPrefetchedFlag(FeatureFlag.CodyAutocompleteContextBfgMixed)
-        const newJaccardSimilarityContextFlag = this.getPrefetchedFlag(
-            FeatureFlag.CodyAutocompleteContextNewJaccardSimilarity
-        )
 
         const contextStrategy: ContextStrategy =
             config.autocompleteExperimentalGraphContext === 'bfg'
@@ -94,9 +90,7 @@ class CompletionProviderConfig {
                               ? 'new-jaccard-similarity'
                               : bfgMixedContextFlag
                                   ? 'bfg-mixed'
-                                  : newJaccardSimilarityContextFlag
-                                      ? 'new-jaccard-similarity'
-                                      : 'jaccard-similarity'
+                                  : 'jaccard-similarity'
 
         return contextStrategy
     }

vscode/src/completions/context/context-strategy.ts

@@ -4,7 +4,6 @@ import type { ContextRetriever } from '../types'
 
 import type { BfgRetriever } from './retrievers/bfg/bfg-retriever'
 import { JaccardSimilarityRetriever } from './retrievers/jaccard-similarity/jaccard-similarity-retriever'
-import { JaccardSimilarityRetriever as NewJaccardSimilarityRetriever } from './retrievers/new-jaccard-similarity/jaccard-similarity-retriever'
 import { SectionHistoryRetriever } from './retrievers/section-history/section-history-retriever'
 
 export type ContextStrategy =
@@ -47,10 +46,6 @@ export class DefaultContextStrategyFactory implements ContextStrategyFactory {
                 this.localRetriever = new JaccardSimilarityRetriever()
                 this.disposables.push(this.localRetriever)
                 break
-            case 'new-jaccard-similarity':
-                this.localRetriever = new NewJaccardSimilarityRetriever()
-                this.disposables.push(this.localRetriever)
-                break
             case 'local-mixed':
                 this.localRetriever = new JaccardSimilarityRetriever()
                 // Filling the graphRetriever field with another local retriever but that's alright
@@ -101,8 +96,7 @@ export class DefaultContextStrategyFactory implements ContextStrategyFactory {
                 break
 
             // The jaccard similarity strategies only uses the local retriever
-            case 'jaccard-similarity':
-            case 'new-jaccard-similarity': {
+            case 'jaccard-similarity': {
                 if (this.localRetriever) {
                     retrievers.push(this.localRetriever)
                 }

vscode/src/completions/context/retrievers/jaccard-similarity/bestJaccardMatch.test.ts

@@ -1,6 +1,7 @@
+import dedent from 'dedent'
 import { describe, expect, it } from 'vitest'
 
-import { bestJaccardMatch, getWords } from './bestJaccardMatch'
+import { bestJaccardMatches, getWordOccurrences } from './bestJaccardMatch'
 
 const targetSnippet = `
 import { bestJaccardMatch, getWords } from './context'
@@ -25,30 +26,18 @@ describe('getWords', () => {
 })
 `
 
-const matchSnippet = `
-describe('bestJaccardMatch', () => {
-    it('should return the best match', () => {
-        const matchText = [
-            'foo',
-            'bar',
-            'baz',
-            'qux',
-            'quux',
-        ].join('\n')
-    })
-})
-`
+const MAX_MATCHES = 50
 
 describe('getWords', () => {
     it('works with regular text', () => {
-        expect(getWords('foo bar baz')).toEqual(
+        expect(getWordOccurrences('foo bar baz')).toEqual(
             new Map<string, number>([
                 ['foo', 1],
                 ['bar', 1],
                 ['baz', 1],
             ])
         )
-        expect(getWords('running rocks slipped over')).toEqual(
+        expect(getWordOccurrences('running rocks slipped over')).toEqual(
             new Map<string, number>([
                 ['run', 1],
                 ['rock', 1],
@@ -58,7 +47,7 @@ describe('getWords', () => {
     })
 
     it('works with code snippets', () => {
-        expect(getWords(targetSnippet)).toEqual(
+        expect(getWordOccurrences(targetSnippet)).toEqual(
             new Map<string, number>([
                 ['import', 1],
                 ['bestjaccardmatch', 1],
@@ -87,61 +76,158 @@ describe('getWords', () => {
 })
 
 describe('bestJaccardMatch', () => {
-    it('should return the best match in 5 line windows', () => {
-        const matchText = [
-            'foo',
-            'bar',
-            'baz',
-            'qux',
-            'quux',
-            'quuz',
-            'corge',
-            'grault',
-            'garply',
-            'waldo',
-            'fred',
-            'plugh',
-            'xyzzy',
-            'thud',
-        ].join('\n')
-        expect(bestJaccardMatch('foo\nbar\nbaz', matchText, 3)).toEqual({
+    it('should return the best match', () => {
+        const matchText = dedent`
+            foo
+            bar
+            baz
+            qux
+            quux
+            quuz
+            corge
+            grault
+            garply
+            waldo
+            fred
+            plugh
+            xyzzy
+            thud
+        `
+        expect(bestJaccardMatches('foo\nbar\nbaz', matchText, 3, MAX_MATCHES)[0]).toEqual({
             score: 1,
             content: 'foo\nbar\nbaz',
-            startLine: 0,
             endLine: 2,
+            startLine: 0,
         })
-        expect(bestJaccardMatch('bar\nquux', matchText, 4)).toEqual({
+        expect(bestJaccardMatches('bar\nquux', matchText, 4, MAX_MATCHES)[0]).toEqual({
             score: 0.5,
             content: 'bar\nbaz\nqux\nquux',
-            startLine: 1,
             endLine: 4,
+            startLine: 1,
         })
         expect(
-            bestJaccardMatch(
+            bestJaccardMatches(
                 ['grault', 'notexist', 'garply', 'notexist', 'waldo', 'notexist', 'notexist'].join('\n'),
                 matchText,
-                6
-            )
+                6,
+                MAX_MATCHES
+            )[0]
         ).toEqual({
             score: 0.3,
-            content: ['corge', 'grault', 'garply', 'waldo', 'fred', 'plugh'].join('\n'),
-            startLine: 6,
-            endLine: 11,
+            startLine: 4,
+            endLine: 9,
+            content: ['quux', 'quuz', 'corge', 'grault', 'garply', 'waldo'].join('\n'),
         })
     })
 
+    it('returns more than one match', () => {
+        const matchText = dedent`
+            foo
+            bar
+            baz
+            qux
+            foo
+            quuz
+            corge
+            grault
+            garply
+            waldo
+            fred
+            plugh
+            xyzzy
+            thud`
+
+        const matches = bestJaccardMatches('foo\nbar\nbaz', matchText, 3, MAX_MATCHES)
+
+        expect(matches).toHaveLength(4)
+        expect(matches.map(match => match.content.split('\n'))).toEqual([
+            ['foo', 'bar', 'baz'],
+            ['qux', 'foo', 'quuz'],
+            ['corge', 'grault', 'garply'],
+            ['waldo', 'fred', 'plugh'],
+        ])
+    })
+
     it('works with code snippets', () => {
-        expect(bestJaccardMatch(targetSnippet, matchSnippet, 5)).toMatchInlineSnapshot(`
+        expect(
+            bestJaccardMatches(
+                targetSnippet,
+                dedent`
+                    describe('bestJaccardMatch', () => {
+                        it('should return the best match', () => {
+                            const matchText = [
+                                'foo',
+                                'bar',
+                                'baz',
+                                'qux',
+                                'quux',
+                            ].join('\n')
+                        })
+                    })
+                `,
+                5,
+                MAX_MATCHES
+            )[0]
+        ).toMatchInlineSnapshot(`
           {
             "content": "describe('bestJaccardMatch', () => {
               it('should return the best match', () => {
                   const matchText = [
                       'foo',
                       'bar',",
-            "endLine": 5,
+            "endLine": 4,
             "score": 0.08695652173913043,
-            "startLine": 1,
+            "startLine": 0,
           }
         `)
     })
+
+    it('works for input texts that are shorter than the window size', () => {
+        expect(bestJaccardMatches('foo', 'foo', 10, MAX_MATCHES)[0]).toEqual({
+            content: 'foo',
+            endLine: 0,
+            score: 1,
+            startLine: 0,
+        })
+    })
+
+    it('skips over windows with empty start lines', () => {
+        const matches = bestJaccardMatches(
+            'foo',
+            dedent`
+                // foo
+                // unrelated 1
+                // unrelated 2
+
+
+                // foo
+                // unrelated 3
+                // unrelated 4
+            `,
+            3,
+            MAX_MATCHES
+        )
+
+        expect(matches[0].content).toBe('// foo\n// unrelated 1\n// unrelated 2')
+        expect(matches[1].content).toBe('// foo\n// unrelated 3\n// unrelated 4')
+    })
+
+    it("does not skips over windows with empty start lines if we're at the en", () => {
+        const matches = bestJaccardMatches(
+            targetSnippet,
+            dedent`
+                // foo
+                // unrelated
+                // unrelated
+
+
+                // foo
+            `,
+            3,
+            MAX_MATCHES
+        )
+
+        expect(matches[0].content).toBe('\n\n// foo')
+        expect(matches[1].content).toBe('// foo\n// unrelated\n// unrelated')
+    })
 })