sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.1k stars 1.27k forks source link

Code intel: consolidate search-based implementations #49199

Open olafurpg opened 1 year ago

olafurpg commented 1 year ago

We currently have 3 different implementations of search-based code intel

This is an umbrella issue to track progress towards unifying all of these under the "new" codeintel API.

Related problems caused by the current situation

philipp-spiess commented 1 year ago

@olafurpg Looking into this a bit, what do you think would be the best way to resolve this? Do you think it's reasonable to use the new hooks-based API everywhere? Why is there even a mismatch in logic between those and what would you call the "reference implementation" at this point? 😅

I think we'd probably also want to extract some logic of the hooks version into plain JS so it can also work outside of React (e.g. for the browser extension).

If we clean this up, I think what would be really helpful too is if we can unify the GraphQL queries for precise and search-based on the backend somehow so that we can move the logic to the server and have a lighter client. This would also reduce water falling in cases like this because we can simply join the data on the backend: https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/web/src/enterprise/codeintel/useCodeIntel.ts?L145-148

Or is the issue for that that we can't dedupe the two different data sources on the backend without a stateful connection?

olafurpg commented 1 year ago

@philipp-spiess My recommendation is to standardize on the CodeIntelAPI interface

https://sourcegraph.com/github.com/sourcegraph/sourcegraph@main/-/blob/client/shared/src/codeintel/api.ts?L34

We should remove the hooks-based API and refactor the code to use that promise-based API instead. I think it's desirable to be able to use the codeintel API outside of React, which disqualifies the hook-based solution.

If we clean this up, I think what would be really helpful too is if we can unify the GraphQL queries for precise and search-based on the backend somehow so that we can move the logic to the server and have a lighter client.

I agree, and I think everybody have touched the codeintel API agrees as well. However, I don't think we'll always need some client-side logic to provide the best possible UX. My vision is that we end up with a reasonably small client that works with scip.Document and does some smart things like caching results based on symbols instead of line/column positions. I recently added the ScipParameters parameter, which is the first step towards this vision even if it's not pretty right now. Down the road, I want the codeintel API to be responsible for fetching the SCIP Document instead of requiring it as a parameter. This would require refactoring the blob view to ask the codeintel API for syntax highlighting.