nvms / wingman

Your pair programming wingman. Supports OpenAI, Anthropic, or any LLM on your local inference server.
https://marketplace.visualstudio.com/items?itemName=nvms.ai-wingman
ISC License
61 stars 10 forks source link

Anthropic Claude support #3

Closed capdevc closed 1 year ago

capdevc commented 1 year ago

Resolves: https://github.com/nvms/wingman/issues/1

PR for adding Anthropic Claude support

capdevc commented 1 year ago

@nvms This is an early draft PR. I'm running into a problem that I don't think is in my code.

When I try to use the new provider for anything I get

rejected promise not handled within 1 second: ReferenceError: fetch is not defined
extensionHostProcess.js:105
stack trace: ReferenceError: fetch is not defined
    at /Users/cc/src/wingman/out/extension.js:6206:85
    at new Promise (<anonymous>)
    at fetchEventSource (/Users/cc/src/wingman/out/extension.js:6178:14)
    at /Users/cc/src/wingman/out/extension.js:12931:53
    at new Promise (<anonymous>)
    at Client2.completeStream (/Users/cc/src/wingman/out/extension.js:12923:16)
    at AnthropicProvider.send (/Users/cc/src/wingman/out/extension.js:32001:44)
    at send (/Users/cc/src/wingman/out/extension.js:18389:32)
    at commandHandler (/Users/cc/src/wingman/out/extension.js:18446:11)

This is thrown at the point the call is being made to the Claude API Client.completeStream method.

This happens even if all I try to do is run the example code for streamed completions provided in the API repo from within the extension and skip all the vs code webview stuff etc.

I'm not really sure what's going on. I did find is a lot of talk about fetch not existing in some node versions and there being multiple packages that provide it. The Claude API package uses one, and wingman apparently uses another. Any idea if this could be caused by some kind of conflict? I'm not familiar enough with how all this works so I thought maybe this was something simple.

nvms commented 1 year ago

I ran into this with the chatgpt-api module as well. This is exactly why the ChatGPTAPI constructor accepts a fetch parameter:

https://github.com/transitive-bullshit/chatgpt-api/blob/main/src/chatgpt-api.ts#L64

I opted to use node-fetch for this as it "just worked". It looks like the SDK you linked is using cross-fetch, which I am not familiar with but appears to be solving the same problem: https://github.com/anthropics/anthropic-sdk-typescript/blob/main/src/index.ts#L2 Unfortunately it doesn't look like they give us a way to define our own (working) fetch.

My suggestion: since the entire SDK is a single 150ish line file, let's just bring it over into wingman entirely (e.g. src/providers/sdks/anthropic.ts) and replace the cross-fetch import with node-fetch, and of course sticking their MIT license at the top of the file.

edit: maintaining our own SDK for this might have some other benefits that aren't immediately clear to me yet, e.g. opportunity to broadcast additional events, only need to support streaming responses, etc.

nvms commented 1 year ago

also thank you for tackling this!

capdevc commented 1 year ago

@nvms

So I have a working version and I've learned a bit.

I took your advice and vendored the anthropic SDK. It uses a package called fetch-event-source that wraps fetch for events. Apparently web fetch and node-fetch return different types of streams for response.body, and that package assumes web fetch. So I replaced it with a fork that works with node-fetch. Neither package seems very popular. It seems that there must be a better way of dealing with streams, but I don't know the ecosystem well enough to figure it out.

I've tested it and it everything seems to work, including aborting, and followups.