provider: SambaNova FastAPI implementation. #548

ilsubyeega · 2024-08-28T09:55:21Z

This PR implements SambaNova FastAPI implementation. Which is currently private-beta, but the request codes are in sambanova/ai-starter-kit repository. Whole codebases are copied from /src/provider/groq directory.

Related #407. But this does not implement any other apis that are mentioned.

It's working when quick and simple test, but I must verify that it works properly, so I've submitted this as Draft.

- Copied from `/src/providers/groq`.

ilsubyeega · 2024-09-01T06:39:20Z

I think this is okay for now.

VisargD · 2024-09-01T17:49:25Z

Hey! Thanks for the PR. We recently released a openai base provider which can be used for all openai compatible providers. It would be great if you can use it for this provider as well because its OpenAI compliant. You can check the Cerebras integration as a reference on how to use the base provider.

src/providers/sambanova/api.ts

ilsubyeega · 2024-09-02T01:15:08Z

We recently released a openai base provider which can be used for all openai compatible providers.

@VisargD I'm curious that this open-ai-base provides stream-chatComplete. I can only see that transform non-stream repsonse but not stream one (and even it has custom property at sambanova).[1]

Also, i doubt at

gateway/src/providers/open-ai-base/index.ts

Line 32 in 1a4ab70

defaultValues?: Record<string, string>,

chatCompleteParams#defaultValues?: Record<string, string> looks wrongly designed. PrarameterConfig.default? provides any type, but #defaultValues only appliable with string but not boolean and numbers. I'm just putting string at this moment, but not ideal i guess.

[1]: Edit; seems groq got reverted, nevermind.

src/providers/sambanova/chatComplete.ts

b4s36t4 · 2024-09-02T13:58:51Z

src/providers/sambanova/chatComplete.ts

+    };
+    index: number;
+    finish_reason: string | null;
+    logprobs: object | null;


Can we just do Record instead of object here?

src/providers/sambanova/chatComplete.ts

b4s36t4 · 2024-09-02T14:03:55Z

src/providers/sambanova/index.ts

+      'tools',
+      'tool_choice',
+      'response_format',
+      'logprobs',


We're expecting the response to send logprobs but we're excluding it here, is there any issue here?

Sorry, I'm missing the documentation with me, so had to confirm.

My bad, i will review these as soon as possible.

if it meant that there are no logprob params, at this moment, there is no mention of these params. i want to believe the docs that i have dont have complete references.

My understanding is that the request would have that logprobs arg, but it wasn't documented.

Ok, for now we can remove the logprobs i.e don't expect it to be available in the response and don't accept it in the request body as well. Once they are with good documentation/ available for everyone we can make the change again.

cc: @VisargD

src/providers/sambanova/chatComplete.ts

b4s36t4 · 2024-09-06T11:45:12Z

@ilsubyeega I have tried running the code locally but keep on getting 401, were you able to test this locally if so can you share a video recording of the response.

Following is the curl I have tried.

curl --request POST \
  --url http://localhost:8787/v1/chat/completions \
  --header 'authorization: key' \
  --header 'content-type: application/json' \
  --header 'x-portkey-provider: sambanova' \
  --data '{
  "messages": [
    {
      "role": "user",
      "content": "Hello, world"
    }
  ]
}'

ilsubyeega · 2024-09-06T11:56:11Z

were you able to test this locally if so can you share a video recording of the response.

@b4s36t4 Here you go.

Screencast.from.2024-09-06.20-54-31.webm

b4s36t4 · 2024-09-07T08:22:10Z

@ilsubyeega Sorry for the delay. But I just have got merged the typing issue for the defaluValues for the base provider. If possible please update the PR to use stream with boolean value instead of string value. Then we can merge it.

ilsubyeega · 2024-09-07T09:50:39Z

Here are some concerns: Default parameter does not affect isStreamingMode, which requires stream paramter on request even though it should appended as default.

gateway/src/handlers/handlerUtils.ts

Lines 459 to 491 in 7eaf932

    
           const overrideParams = providerOption?.overrideParams || {}; 
        
           const params: Params = { ...inputParams, ...overrideParams }; 
        
           const isStreamingMode = params.stream ? true : false; 
        
           let strictOpenAiCompliance = true; 
        
           if (requestHeaders[HEADER_KEYS.STRICT_OPEN_AI_COMPLIANCE] === 'false') { 
        
             strictOpenAiCompliance = false; 
        
           } else if (providerOption.strictOpenAiCompliance === false) { 
        
             strictOpenAiCompliance = false; 
        
           } 
        
           const provider: string = providerOption.provider ?? ''; 
        
           const hooksManager = c.get('hooksManager'); 
        
           const hookSpan = hooksManager.createSpan( 
        
             params, 
        
             provider, 
        
             isStreamingMode, 
        
             providerOption.beforeRequestHooks || [], 
        
             providerOption.afterRequestHooks || [], 
        
             null, 
        
             fn 
        
           ); 
        
           // Mapping providers to corresponding URLs 
        
           const apiConfig: ProviderAPIConfig = Providers[provider].api; 
        
           // Attach the body of the request 
        
           const transformedRequestBody = transformToProviderRequest( 
        
             provider, 
        
             params, 
        
             inputParams, 
        
             fn 
        
           );

transformToProviderRequest parses with default provider config, if it done correctly, but it does not affect isStreamingMode, make it fails.

Since stream is not required param, so it does not append stream: true on body.

gateway/src/services/transformToProviderRequest.ts

Lines 116 to 128 in 7eaf932

    
           // If the parameter is not present in the incoming request body but is required, set it to the default value 
        
           else if ( 
        
             paramConfig && 
        
             paramConfig.required && 
        
             paramConfig.default !== undefined 
        
           ) { 
        
             // Set the transformed parameter to the default value 
        
             setNestedProperty( 
        
               transformedRequest, 
        
               paramConfig.param, 
        
               paramConfig.default 
        
             ); 
        
           }

Here is possible workarounds:

/src/handlers/handlerUtils.ts#tryPost, it should be reverify after transformToProviderRequest, e.g

if ((transformedRequestBody as { stream?: boolean })['stream'] == true) isStreamingMode = true;

Two options: add require params on /src/provider/open-ai-base/index.ts#chatCompleteParams, or remove paramConfig.required check to /src/services/transformToProviderRequest.ts#transformToProviderRequestJSON

    stream: {
      param: 'stream',
      ...(defaultValues?.stream && { default: defaultValues.stream, required: true }), // <--
    },

      else if (
        paramConfig &&
        // paramConfig.required && <--- remove this
        paramConfig.default !== undefined
      ) {

it seems default option is useless when required=false, so the last option looks ideal for me.

Since the provider only supports streaming-mode at this moment, which is unusual, so this issue being kinda tricky.
Or we can just ignore this since the client(requesting to gateway) should set stream=true explicitly, otherwise it will break parsing responses.

b4s36t4 · 2024-09-07T10:07:41Z

@ilsubyeega Sorry, I didn't understand what you're trying to say. If possible can we take this over discord chat. b4s36t4 is my discord hit me up.

b4s36t4 · 2024-09-07T10:10:21Z

Also sambanova does support non-stream request as well.

{
  "choices": [
    {
      "finish_reason": "stop",
      "index": 0,
      "logprobs": null,
      "message": {
        "content": "It seems like you've mentioned a name, \"Mahesh\". Would you like to know more about someone with this name, or perhaps discuss something related to it?"
      }
    }
  ],
  "created": 1725703777,
  "id": "08f08fe8-889a-4cdb-b406-e4f8077b2537",
  "model": "Meta-Llama-3.1-8B-Instruct",
  "object": "chat.completion",
  "system_fingerprint": "fastcoe",
  "usage": {
    "completion_tokens": 33,
    "completion_tokens_after_first_per_sec": 483.3644057419852,
    "completion_tokens_after_first_per_sec_first_ten": 889.0098453777595,
    "completion_tokens_per_sec": 355.31535860884566,
    "end_time": 1725703777.6294942,
    "is_last_response": true,
    "prompt_tokens": 11,
    "start_time": 1725703777.536619,
    "time_to_first_token": 0.0266726016998291,
    "total_latency": 0.09287524223327637,
    "total_tokens": 44,
    "total_tokens_per_sec": 473.7538114784609
  }
}

ilsubyeega · 2024-09-07T10:22:43Z

Uh... While testing i got nonstream response so I thought that the gateway caching it, but was sambanova side, it works sometimes and sometimes not.

b4s36t4

Hi, @ilsubyeega. Sorry to drag this longer, but while testing I have observed an issue where the provider is sending more than one chunk in the response for stream which is throwing error and not working as expected.

To resolve this you need update the function getStreamModeSplitPattern to support the sambanova. Currently we have different split patterns for different providers, for sambanova we should be adding \n as the separator. Please let me know if there's anything we can discuss over discord.

vrushankportkey · 2024-09-11T09:48:21Z

Hey @ilsubyeega - can you please share the email/name of person from Sambanova you were in touch with? We can reach out to to inform about this integration and discuss what more can be done.

ilsubyeega · 2024-09-11T10:02:49Z

Hey @ilsubyeega - can you please share the email/name of person from Sambanova you were in touch with? We can reach out to to inform about this integration and discuss what more can be done.

I am just a private beta test api user and have no connection with sambanova. I received the mail through salesforce.

ilsubyeega added 3 commits August 28, 2024 18:45

Initial commit for sambanova fastapi implementation.

b999c55

- Copied from `/src/providers/groq`.

Merge remote-tracking branch 'upstream/main' into provider-sambanova

2e7c49b

remove non-stream request at this moment

7e703d3

ilsubyeega marked this pull request as ready for review September 1, 2024 06:39

VisargD reviewed Sep 1, 2024

View reviewed changes

src/providers/sambanova/api.ts Outdated Show resolved Hide resolved

rework: use customHost in getBaseURL & use openai-base common

6874a67

ilsubyeega requested a review from VisargD September 2, 2024 01:29

VisargD previously approved these changes Sep 2, 2024

View reviewed changes

b4s36t4 suggested changes Sep 2, 2024

View reviewed changes

Merge branch 'main' into provider-sambanova

041bf45

ilsubyeega commented Sep 3, 2024

View reviewed changes

src/providers/sambanova/chatComplete.ts Show resolved Hide resolved

append real logprobs value to data

4daa1b1

ilsubyeega dismissed VisargD’s stale review via 4daa1b1 September 3, 2024 05:42

append fake finish_reason to last response stream chunk

b52ee0e

ilsubyeega requested a review from b4s36t4 September 4, 2024 03:24

b4s36t4 mentioned this pull request Sep 6, 2024

fix: type support for default values #583

Merged

Move basic auth into bearer which is now supported

7a08d6a

ilsubyeega added 2 commits September 7, 2024 17:29

Merge branch 'main' into provider-sambanova

9ecbf36

move defaultoption with typed

a7a5fe3

fix name typo

16a92ff

ilsubyeega added 4 commits September 7, 2024 21:06

add chatcomplete transformer

17ddf11

remove stream:true default

1c35cf8

move chatcomplete to index.ts and remove unused

60b0e5f

add provider to nonstream response

17ecd59

b4s36t4 suggested changes Sep 9, 2024

View reviewed changes

split the chunks of sambanova

b307a80

b4s36t4 approved these changes Sep 9, 2024

View reviewed changes

ilsubyeega requested a review from VisargD September 9, 2024 23:18

VisargD merged commit 08048e9 into Portkey-AI:main Sep 10, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider: SambaNova FastAPI implementation. #548

provider: SambaNova FastAPI implementation. #548

ilsubyeega commented Aug 28, 2024

ilsubyeega commented Sep 1, 2024

VisargD commented Sep 1, 2024

ilsubyeega commented Sep 2, 2024 •

edited

Loading

b4s36t4 Sep 2, 2024

b4s36t4 Sep 2, 2024

ilsubyeega Sep 2, 2024

ilsubyeega Sep 3, 2024

b4s36t4 Sep 3, 2024 •

edited

Loading

b4s36t4 commented Sep 6, 2024

ilsubyeega commented Sep 6, 2024

b4s36t4 commented Sep 7, 2024

ilsubyeega commented Sep 7, 2024 •

edited

Loading

b4s36t4 commented Sep 7, 2024

b4s36t4 commented Sep 7, 2024

ilsubyeega commented Sep 7, 2024

b4s36t4 left a comment

vrushankportkey commented Sep 11, 2024

ilsubyeega commented Sep 11, 2024

provider: SambaNova FastAPI implementation. #548

provider: SambaNova FastAPI implementation. #548

Conversation

ilsubyeega commented Aug 28, 2024

ilsubyeega commented Sep 1, 2024

VisargD commented Sep 1, 2024

ilsubyeega commented Sep 2, 2024 • edited Loading

b4s36t4 Sep 2, 2024

Choose a reason for hiding this comment

b4s36t4 Sep 2, 2024

Choose a reason for hiding this comment

ilsubyeega Sep 2, 2024

Choose a reason for hiding this comment

ilsubyeega Sep 3, 2024

Choose a reason for hiding this comment

b4s36t4 Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

b4s36t4 commented Sep 6, 2024

ilsubyeega commented Sep 6, 2024

b4s36t4 commented Sep 7, 2024

ilsubyeega commented Sep 7, 2024 • edited Loading

b4s36t4 commented Sep 7, 2024

b4s36t4 commented Sep 7, 2024

ilsubyeega commented Sep 7, 2024

b4s36t4 left a comment

Choose a reason for hiding this comment

vrushankportkey commented Sep 11, 2024

ilsubyeega commented Sep 11, 2024

ilsubyeega commented Sep 2, 2024 •

edited

Loading

b4s36t4 Sep 3, 2024 •

edited

Loading

ilsubyeega commented Sep 7, 2024 •

edited

Loading