simplify AnnotationsResult #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now, providers just return `Annotation[]` from their `annotations` method. Items no longer have an `id`. It turned out to not be useful to give items an `id` because items were not often reused for multiple ranges in a file.
sqs
force-pushed
the
simplify-annresult
branch
from
123cb25 to
caea992
Compare
thenamankumar
pushed a commit
that referenced
this pull request
May 17, 2024
The CLI is great for quickly testing of a provider. This setups up the CLI to be able to do more than just `items` as a subcommand. Note: This commit was extracted from a much larger change I have going on for testing on top of #41. I'll send out the other changes once that lands. ### Test Plan Ran it with my sourcegraph-search provider. See the shell session below. <details> ```shellsession $ cd provider/sourcegraph-search/ $ pnpm bundle > @openctx/[email protected] bundle /Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search > esbuild --bundle --format=esm --outfile=dist/bundle.js index.ts dist/bundle.js 5.4kb ⚡ Done in 2ms $ cd ../../bin $ cat config.json { "enable": true, "debug": true, "providers": { "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js": true } } $ OPENCTX_CONFIG=$(cat config.json) pnpm openctx items 'TODO(keegancsmith)' > @openctx/cli@ openctx /Users/keegan/src/github.com/sourcegraph/openctx/bin > esbuild --log-level=error --platform=node --bundle --outdir=dist --format=esm --out-extension:.js=.mjs cli.mts && node --no-warnings=ExperimentalWarning --es-module-specifier-resolution=node --loader ts-node/esm/transpile-only dist/cli.mjs "items" "TODO(keegancsmith)" #1 github.com/sourcegraph/sourcegraph internal/httpcli/external.go:115-117 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (138 characters) #2 github.com/sourcegraph/sourcegraph internal/ratelimit/monitor.go:88-90 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (105 characters) #3 github.com/sourcegraph/cody vscode/src/services/GuardrailsProvider.ts:3-5 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (182 characters) #4 github.com/sourcegraph/sourcegraph internal/updatecheck/client.go:461-463 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (92 characters) #5 github.com/sourcegraph/sourcegraph internal/search/result/symbol.go:121-123 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (181 characters) #6 github.com/sourcegraph/sourcegraph internal/search/symbol/symbol.go:45-47 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (335 characters) #7 github.com/sourcegraph/sourcegraph cmd/frontend/internal/guardrails/dotcom/dotcom.go:31-33 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (200 characters) #8 github.com/sourcegraph/sourcegraph internal/search/job/jobutil/filter_file_contains.go:224-226 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (181 characters) #9 github.com/sourcegraph/sourcegraph cmd/frontend/internal/guardrails/attribution/attribution.go:137-139 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (149 characters) #10 github.com/sourcegraph/zoekt api.go:814-816 - hover.text: From Sourcegraph query TODO(keegancsmith) - ai.content: (153 characters) $ OUTPUT_JSON=1 OPENCTX_CONFIG=$(cat config.json) pnpm openctx items 'TODO(keegancsmith)' > @openctx/cli@ openctx /Users/keegan/src/github.com/sourcegraph/openctx/bin > esbuild --log-level=error --platform=node --bundle --outdir=dist --format=esm --out-extension:.js=.mjs cli.mts && node --no-warnings=ExperimentalWarning --es-module-specifier-resolution=node --loader ts-node/esm/transpile-only dist/cli.mjs "items" "TODO(keegancsmith)" [ { "title": "github.com/sourcegraph/sourcegraph internal/httpcli/external.go:115-117", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/httpcli/external.go?L115-117", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\t\t}\n\t\t// TODO(keegancsmith) ensure we validate these certs somewhere\n\t\teffective.TLSClientConfig.RootCAs.AppendCertsFromPEM([]byte(cert))\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph internal/ratelimit/monitor.go:88-90", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/ratelimit/monitor.go?L88-90", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\n// TODO(keegancsmith) Update RecommendedWaitForBackgroundOp to work with other\n// rate limits. Such as:\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/cody vscode/src/services/GuardrailsProvider.ts:3-5", "uri": "https://sourcegraph.com/github.com/sourcegraph/cody/-/blob/vscode/src/services/GuardrailsProvider.ts?L3-5", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "export class GuardrailsProvider {\n // TODO(keegancsmith) this provider should create the client since the guardrails client requires a dotcom graphql connection.\n constructor(\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph internal/updatecheck/client.go:461-463", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/updatecheck/client.go?L461-463", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\n\t// TODO(keegancsmith) should be using pool.Get and closing conn?\n\tconn, err := dialFunc()\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph internal/search/result/symbol.go:121-123", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/search/result/symbol.go?L121-123", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "func (s Symbol) Range() lsp.Range {\n\t// TODO(keegancsmith) For results from zoekt s.Character is not the start\n\t// of the symbol, but the start of the match. So doing s.Character +\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph internal/search/symbol/symbol.go:45-47", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/search/symbol/symbol.go?L45-47", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "func (s *ZoektSymbolsClient) Compute(ctx context.Context, repoName types.MinimalRepo, commitID api.CommitID, inputRev *string, query *string, first *int32, includePatterns *[]string) (res []*result.SymbolMatch, err error) {\n\t// TODO(keegancsmith) we should be able to use indexedSearchRequest here\n\t// and remove indexedSymbolsBranch.\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph cmd/frontend/internal/guardrails/dotcom/dotcom.go:31-33", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/cmd/frontend/internal/guardrails/dotcom/dotcom.go?L31-33", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "func NewClient(externalHTTPClient httpcli.Doer, endpoint, token string) Client {\n\t// TODO(keegancsmith) we allow unauthed requests for now but should\n\t// require it when promoting guardrails for use.\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph internal/search/job/jobutil/filter_file_contains.go:224-226", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/search/job/jobutil/filter_file_contains.go?L224-226", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\tslices.SortFunc(cm.DiffPreview.MatchedRanges, func(a, b result.Range) int {\n\t\t// TODO(keegancsmith) I changed this from b.End to b.Start since that\n\t\t// matches the comment above.\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/sourcegraph cmd/frontend/internal/guardrails/attribution/attribution.go:137-139", "uri": "https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/cmd/frontend/internal/guardrails/attribution/attribution.go?L137-139", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\n\t// TODO(keegancsmith) Reading the SearchClient code it seems to miss out\n\t// on some of the observability that we instead add in at a later stage.\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" }, { "title": "github.com/sourcegraph/zoekt api.go:814-816", "uri": "https://sourcegraph.com/github.com/sourcegraph/zoekt/-/blob/api.go?L814-816", "ui": { "hover": { "text": "From Sourcegraph query TODO(keegancsmith)" } }, "ai": { "content": "\t//\n\t// TODO(keegancsmith) audit updates to IndexTime and document how and when\n\t// it changes. Concerned about things like metadata updates or compound\n" }, "providerUri": "file:///Users/keegan/src/github.com/sourcegraph/openctx/provider/sourcegraph-search/dist/bundle.js" } ] ``` </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now, providers just return
Annotation[]from theirannotationsmethod. Items no longer have anid. It turned out to not be useful to give items anidbecause items were not often reused for multiple ranges in a file.