Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

use chunk matches over line matches for insights#44969

Merged
coury-clark merged 3 commits into
mainfrom
cclark/use-chunk-matches-for-insights
Nov 30, 2022
Merged

use chunk matches over line matches for insights#44969
coury-clark merged 3 commits into
mainfrom
cclark/use-chunk-matches-for-insights

Conversation

@coury-clark

@coury-clark coury-clark commented Nov 30, 2022

Copy link
Copy Markdown
Contributor

Closes https://github.com/sourcegraph/sourcegraph/issues/44965
This solves a bug where multiple whitespaces in a sequence would produce too many line matches. It seems we just missed the boat at some point when this became exposed in the API, so we would want to migrate anyway. The next step will be to retire this for the internal search client, but this solves the bug for now.

Test plan

Here is an example insight that demonstrated the overcounting bug before and after.

CleanShot 2022-11-30 at 15 41 16@2x

@cla-bot cla-bot Bot added the cla-signed label Nov 30, 2022
@coury-clark coury-clark changed the title use chunk matches over line matches to resolve overcounting bugs in t… use chunk matches over line matches for insights Nov 30, 2022
@coury-clark coury-clark requested a review from a team November 30, 2022 22:41
@coury-clark coury-clark marked this pull request as ready for review November 30, 2022 23:10
@sourcegraph-bot

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff fb2b15f...c9a33c8.

Notify File(s)
@sourcegraph/code-insights-backend enterprise/internal/insights/query/streaming/decoder.go
enterprise/internal/insights/query/streaming/search.go

@coury-clark coury-clark merged commit a85b45c into main Nov 30, 2022
@coury-clark coury-clark deleted the cclark/use-chunk-matches-for-insights branch November 30, 2022 23:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

insights: correctness issue with multiple whitespace characters

3 participants