First draft Tech stack file provider#12
Conversation
|
Awesome! I'll take a look this weekend. Any screenshots to share, or need help getting it actually running end-to-end? |
Yeah, I'm trying to get the annotations show up on the playground. It seems alright in a unit test. |
The issue seems to be with loading server side modules(viz. failed to get annotations: Error: Module "path" has been externalized for browser compatibility.
Cannot access "path.resolve" in client code.
See https://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.I read up about polyfilling node libraries in vite.config as a workaround but I'm not too familiar with the frontend stack (vite etc), any suggestions on a workaround @sqs ? |
sqs
left a comment
There was a problem hiding this comment.
Thank you! Some comments and questions. This is super helpful for designing the OpenCodeGraph provider API (which is super early).
(BTW, I have a big refactor of the OpenCodeGraph API in the provider-docs branch. It should be a very small modification to your provider code here, so that need not block merging this.)
provider/techstack/index.test.ts
Outdated
| const result = techstack.capabilities({}, SETTINGS) | ||
|
|
||
| expect(result).toBeDefined() | ||
| expect(result).toStrictEqual<CapabilitiesResult>({ |
There was a problem hiding this comment.
I think we'd want the techstack file to show up not in code files but in config and build files, so maybe something like:
.*(dotfiles)**/package.json**/*.bazel**/README.mdtechstack.y?(a)mlof course- others like that?
There was a problem hiding this comment.
Oh, or is the intent to show it alongside the require or import statements in JS/TS files? Can you add a README describing the expected behavior briefly?
There was a problem hiding this comment.
Yes, that was the intention for the first draft implementation(only imports), it would be too complex and noisy to annotate rest of the code.
Annotating build and config files also makes sense, but I'd like to introduce that in a future iteration. I've intentionally kept this first draft minimal. Does this approach sound okay?
provider/techstack/index.ts
Outdated
| * @param filename - techstack yml filename | ||
| * @returns parsed yaml object | ||
| */ | ||
| async function load(filename: string): Promise<TSF> { |
There was a problem hiding this comment.
Being able to automatically infer the techstack.yml file seems quite important here. Unfortunately, OpenCodeGraph does not yet have a way to help you do this. I was thinking that providers could say in their capabilities response something like "please also send me the contents of the following files with each annotations request: techstack.yml, ...". What do you think about that approach?
There was a problem hiding this comment.
I'm not sure I entirely follow you here but here are my thoughts.
I didn't intend to automatically infer the techstack.yml file - it's set explicitly in the provider settings and the load() method simply references that in L59 and I find the explicitness in this design better.
const report = await load(settings.yaml)That said, it's maybe not impossible to automatically infer a techstack.yml file for a respository and supplement it with an appropriate override in provider settings. You could also introduce an optional member in the AnnotationsParams interface that sends any techstack.yml data for each annotation request.
However, I don't see any real gain in these approaches vs the current settings driven design. Just my 2 cents.
| // "https://sourcegraph.test:3443/.api/opencodegraph": true, | ||
| // "../../../../../../provider/hello-world/dist/index.js": true, | ||
| "https://opencodegraph.org/npm/@opencodegraph/provider-hello-world": true, | ||
| "https://opencodegraph.org/npm/@opencodegraph/provider-techstack": { |
There was a problem hiding this comment.
@sqs can you help me host this provider at opencodegraph.org? how can I test the provider on VSCode locally?
There was a problem hiding this comment.
opencodegraph.org/npm/... proxies any npm module. So it should already work!
|
Thank you @tinvaan. I will follow up on this when I finish a big migration in the API. You are the first to contribute something, which is awesome, but it means you will definitely feel the rapid change during this experimental period, sorry! |
That's fine @sqs , thanks for the heads up re the migration. Before your changes land, could you tag & release the current main branch? I'm planning to implement another provider and would be easier if I have a locked down release version that I can work with. |
e11983d to
4b7a1c6
Compare
Lines not matching import regex are needed to preserve line numbers.
s/OpenCodeGraph/OpenCtx
4b7a1c6 to
4d90f8f
Compare
4d90f8f to
590a4ea
Compare
I think this is from an old version of the |

A first draft implementation of a tech stack file opencodegraph provider. Currently only supports npm packages and
.js&.tssource files.