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

Cody context: fix an issue with anchoring repo names#63585

Merged
camdencheek merged 2 commits into
mainfrom
cc/fix-cody-context-anchors
Jul 2, 2024
Merged

Cody context: fix an issue with anchoring repo names#63585
camdencheek merged 2 commits into
mainfrom
cc/fix-cody-context-anchors

Conversation

@camdencheek

Copy link
Copy Markdown
Member

Fixes SRCH-658

This fixes an issue with anchoring repos in the keyword search query used for cody context.

Previously, we would use a repo: pattern like repo:^(?:a)|(?:b)$. However, the union operator binds loosely, so this is interpreted as matching (^a)|(b$) rather than ^(a|b)$.

This fixes the pattern so we add the anchors to every repo name. Alternatively, we could modify the behavior of UnionRegExp, but that seemed riskier.

Test plan

Quick manual test that context for github.com/sourcegraph/docs does not return any files from github.com/sourcegraph/docsite

Changelog

  • Fixes a bug that caused context to be fetched for Cody from irrelevant repos

@cla-bot cla-bot Bot added the cla-signed label Jul 1, 2024
@camdencheek camdencheek marked this pull request as ready for review July 1, 2024 23:25
@camdencheek camdencheek requested a review from a team July 1, 2024 23:25

@jtibshirani jtibshirani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I'm surprised we didn't catch this until now. I guess it really shows the power of Cody Web for dogfooding. And that we haven't worked on multi-repo context enough.

regexEscapedRepoNames[i] = fmt.Sprintf("^%s$", regexp.QuoteMeta(string(repo.Name)))
}

keywordQuery := fmt.Sprintf(`repo:^%s$ %s %s`, query.UnionRegExps(regexEscapedRepoNames), getKeywordContextExcludeFilePathsQuery(), args.Query)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's a simple fix, it'd be great to pull out a method like createKeywordQuery and add a couple cases in context_test.go!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@camdencheek camdencheek enabled auto-merge (squash) July 2, 2024 17:13
@camdencheek camdencheek merged commit f164cce into main Jul 2, 2024
@camdencheek camdencheek deleted the cc/fix-cody-context-anchors branch July 2, 2024 17:23
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.

2 participants