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

Add .cody/ignore support to remote context #59836

Merged
chwarwick merged 15 commits into
mainfrom
cw/context-ignore-filter
Jan 31, 2024
Merged

Add .cody/ignore support to remote context #59836
chwarwick merged 15 commits into
mainfrom
cw/context-ignore-filter

Conversation

@chwarwick

@chwarwick chwarwick commented Jan 24, 2024

Copy link
Copy Markdown
Contributor

Updates the getCodyContext resolver to filter out context if a .cody/ignore file is present and it matches an ignore rule. The behavior is behind an experimental feature in the site config.

closes https://github.com/sourcegraph/sourcegraph/issues/59682
closes https://github.com/sourcegraph/sourcegraph/issues/59976

Test plan

updated existing context resolver tests
add new tests for filtering logic

Manual testing

  • Created a test repo with a main.go file and printed hello world
  • Asked Cody what it printed

Screenshot 2024-01-25 at 10 01 29 AM

  • Added a .cody/ignore file with main.go in it
  • waiting for repo sync
  • Asked Cody what the program prints - it did not read the file anymore.

Screenshot 2024-01-25 at 9 56 29 AM

@chwarwick chwarwick requested review from a team and emidoots January 24, 2024 20:33
Comment thread internal/codycontext/filter.go Outdated
@cla-bot cla-bot Bot added the cla-signed label Jan 24, 2024
Comment thread internal/codycontext/filter.go Outdated
@chwarwick chwarwick force-pushed the cw/context-ignore-filter branch from 2f3a254 to 3455612 Compare January 25, 2024 00:57
@chwarwick chwarwick marked this pull request as ready for review January 25, 2024 16:22
"github.com/sourcegraph/sourcegraph/internal/types"
)

const codyIgnoreFile = ".cody/ignore"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@abeatrix this is correct for the ignore file right?

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.

Yea this is the name we use for the .cody/ignore file

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.

@chwarwick would this work with nested directory?

@chwarwick chwarwick Jan 26, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@chwarwick would this work with nested directory?

I don't think that is an issue since I'm checking directly against the repo from gitserver and I thought it had to be at the root of the repo. I shouldn't need to handle any of the complexities that the extensions have were multiple repos in a workspace or a folder that contains subfolders with repos etc.. Is there a scenario are you thinking of?

The api is "get me context from repos [A, B, C]" so it runs a search against those repos and checks for ignore files in each of them to apply to the search results.

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.

thought it had to be at the root of the repo

I was wondering how it'd work with monorepos that contain sub repo but I think we should be good as long as we communicate that with the users?

Comment thread internal/codycontext/context.go Outdated
Comment thread internal/codycontext/filter.go Outdated
Comment thread internal/codycontext/context.go Outdated
Comment thread internal/codycontext/filter.go
Comment thread internal/codycontext/filter.go Outdated

@emidoots emidoots left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally looks good

@abeatrix abeatrix 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.

Lgtm! Can't wait to test it out in the clients!

@arafatkatze arafatkatze 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.

Love the process and the idea of this PR. I wanna approve after the resolution of

  1. Caching
  2. Edge Case of git
  3. Blocking on errors

@chwarwick chwarwick requested a review from emidoots January 31, 2024 00:39
Comment thread internal/codycontext/filter.go
@chwarwick chwarwick merged commit a73d371 into main Jan 31, 2024
@chwarwick chwarwick deleted the cw/context-ignore-filter branch January 31, 2024 19:48
abeatrix referenced this pull request in sourcegraph/cody-public-snapshot Feb 5, 2024
Part of https://github.com/sourcegraph/cody/issues/2920

Summary:
- Adding a new `check/isCodyIgnoredFile` request handler to check if a
file path is being ignored by Cody
- Updating tests to use this new request instead of the
isCodyIgnoredFile utility method
- Adding a test to confirm .cody/ignore is working on startup

This PR adds integration tests for the following features to agent:
- [x] autocomplete https://github.com/sourcegraph/cody/pull/2986
- [x] chat (chat submission)
- [x] chat command (/explain)
- [x] inline edit command (/doc)

Local context source covered in the test:
- [x] editor (used by commands, chat, inline-edit, auto-completes)
- [x] symf (local search) (used by chat when enhanced context is
enabled)
- [x] local context used by autocompletion
https://github.com/sourcegraph/cody/pull/2986

Also updated to allow remote context which should be filtered during
sync time (https://github.com/sourcegraph/sourcegraph/pull/59836) to
allow support for multi-repo context in clients.

## Follow-Up Works

- [x] Add assertions for checking input context during Network Request

## Test plan

<!-- Required. See
https://sourcegraph.com/docs/dev/background-information/testing_principles.
-->

Green bots means 👍
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.

P1 allow disabling codyignore via site config Keyword search respects codyignore

5 participants