Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metadata to context items #5153

Merged
merged 5 commits into from
Aug 9, 2024
Merged

Add metadata to context items #5153

merged 5 commits into from
Aug 9, 2024

Conversation

beyang
Copy link
Member

@beyang beyang commented Aug 8, 2024

Context items now include optional metadata about their source or scoring, which can help developers understand why certain items are appearing in the context. This change adds a metadata field to the ContextItemCommon interface and populates it for context items from Sourcegraph search, the local symbol index, and live search results.

You can view these by setting "cody.internal.debug.context": true in settings. The cody.internal.showContextAlternatives has been removed and replaced by this one.

Test plan

Test locally, doesn't affect prod.

@beyang beyang requested review from rishabhmehrotra, janhartman and a team August 8, 2024 22:35
@@ -135,6 +135,7 @@ export function getConfiguration(
*/

internalUnstable: getHiddenSetting('internal.unstable', isTesting),
internalDebugContext: getHiddenSetting('internal.debugContext', false),
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what we can do to make adding new settings easier. Something which doesn't require updating 3 different files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, tbh we should probably just use the vscode.workspace.getConfiguration api. What we've built around that feels like an unnecessary abstraction.

Copy link
Member

@thenamankumar thenamankumar left a comment

Choose a reason for hiding this comment

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

Will it be better to link this with cody.debug.enable.all "Enable Debug Mode" rather than a separate config flag?

LGTM

@@ -244,6 +244,7 @@ export class ContextRetriever implements vscode.Disposable {
range,
source: ContextItemSource.Search,
content: text,
metadata: ['source:symf-live'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maybe add symf-live into the context source enum?

Context items now include optional metadata about their source or
scoring, which can help developers understand why certain items are
appearing in the context. This change adds a `metadata` field to the
`ContextItemCommon` interface and populates it for context items from
Sourcegraph search, the local symbol index, and live search results.
@beyang
Copy link
Member Author

beyang commented Aug 9, 2024

Will it be better to link this with cody.debug.enable.all "Enable Debug Mode" rather than a separate config flag?

"Enable Debug Mode" is technically user visible and I want to keep this internal for now (so as to not worry about the way it looks at all)

Should we maybe add symf-live into the context source enum?

Not sure, I think that belongs in a separate change, as that alters how we want to convey to the end user what is happening. "This came from symf" feels like a low-level implementation detail, where as "came from search" maps to more of a user-level mental model.

@beyang beyang enabled auto-merge (squash) August 9, 2024 21:13
@beyang beyang disabled auto-merge August 9, 2024 21:53
@beyang beyang enabled auto-merge (squash) August 9, 2024 21:53
@beyang beyang merged commit 869de99 into main Aug 9, 2024
21 checks passed
@beyang beyang deleted the bl/symf-debugging branch August 9, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants