New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Provider] Added Support for MonsterAPI Integration #280

Merged

VisargD merged 22 commits into Portkey-AI:main from Dheeraj-Bhandari:main

May 2, 2024

Contributor

DheerajQblocks commented Apr 1, 2024

Description:
This PR adds support for the MonsterAPI provider to our project. It includes the necessary configurations, request/response structures, and transformation logic to integrate MonsterAPI into our project.

Changes:

Added MonsterAPI configuration to provider/index.ts.
Defined MonsterAPI request and response structures in types.ts.
Implemented request transformation logic in provider/transform.ts.
Implemented response transformation logic in provider/transform.ts.

Additional Information:

Documentation for MonsterAPI can be found at MonsterAPI Documentation.
The MonsterAPI API endpoint is https://llm.monsterapi.ai/v1/generate.

Testing:

Tested the integration with MonsterAPI using sample requests.
Verified the correctness of response transformation logic.

Screenshots:

Related Issue: #279 (Add MonsterAPI provider)

DheerajQblocks added 4 commits

March 27, 2024 16:50


          Addded Monsterapi Config As Provider

40320b0


          Added Monsterapi to other global requrments

ba57b4e


          Added Monsterapi As Provider


          Added Monsterapi as Proviedr

b595047

Collaborator

VisargD commented Apr 1, 2024

Hey @DheerajQblocks . Thanks for the PR! Can you please update this with latest main branch changes. Its missing a lot of major changes that were pushed last week. And the integration does not seem to follow OpenAI transform convention that we follow for other providers. All the response are still in monster API format which is not correct. Please let me know if I am missing something or if you need anymore details on this.

DheerajQblocks and others added 6 commits

April 1, 2024 20:11


          Taken pull

e2c45d2


          Merge remote-tracking branch 'upstream/main'

af3c756


          Modified Monsterapi Provider as per required convention

c342b61


          Added Default model

4f60035


          npm updted

403092a


          Merge branch 'main' into main

8b67b11

Contributor Author

DheerajQblocks commented Apr 1, 2024

Thank you for reviewing the PR. I appreciate your feedback.

I have updated the PR with the latest changes from the main branch and ensured that it aligns with the OpenAI transform convention as per our standards for other providers. The integration now follows the required format for responses, adhering to the guidelines.

Regarding the response format, I've revised the implementation to conform to the expected structure for MonsterAPI, ensuring that it meets the requirements outlined in the project specifications.

Please review the changes and let me know if there are any further adjustments needed. I'm here to address any additional concerns or questions you may have.

Thank you for your attention to this matter.

Best regards,
@DheerajQblocks


          Fromated code

c51fd91

VisargD reviewed

View reviewed changes

package-lock.json Outdated

Collaborator

VisargD Apr 2, 2024

Can you please remove these changes to package-lock.json? Seems to be unrelated to the PR and are also causing npm install issues.

Contributor Author

DheerajQblocks Apr 2, 2024

Done

DheerajQblocks and others added 4 commits

April 2, 2024 14:55


          Package lock changes reverted

0ffb5ed


          Package lock changes reverted

62874e7


          Ownership for chages changed

554cd7b


          Merge remote-tracking branch 'origin/main'

Contributor Author

DheerajQblocks commented Apr 4, 2024

@VisargD Please review / Approve this PR.

Collaborator

VisargD commented Apr 6, 2024

@Dheeraj-Bhandari - Is this correct doc for the API that you have integrated? https://developer.monsterapi.ai/reference/generate_v1_generate_post

Contributor Author

DheerajQblocks commented Apr 6, 2024

Is

Yes it's correct


          Merge branch 'main' into main

61d7fd0

Contributor Author

DheerajQblocks commented Apr 9, 2024

@VisargD Please Take a Look

Collaborator

VisargD commented Apr 9, 2024

Hey @DheerajQblocks - Sorry for the delay here. I will start the review today in sometime.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/index.ts Outdated

+                api: MonsterAPIApiConfig,
+                generate: MonsterAPIChatCompleteConfig,
+                responseTransforms: {
+                  generate: MonsterAPIChatCompleteResponseTransform,

Collaborator

VisargD Apr 9, 2024

Instead of generate, the name should be chatComplete for this.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/generate.ts Outdated

+                if ('response' in response) {
+                  return {
+                    id: '',
+                    object: '',

Collaborator

VisargD Apr 9, 2024

The object value can be chat.completion as we are following OpenAI specs

VisargD reviewed

View reviewed changes

src/providers/monsterapi/generate.ts Outdated

+                  return {
+                    id: '',
+                    object: '',
+                    created: 0,

Collaborator

VisargD Apr 9, 2024

For created, you can use Math.floor(Date.now() / 1000) as value.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/generate.ts Outdated

+                if ('response' in response) {
+                  return {
+                    id: '',

Collaborator

VisargD Apr 9, 2024

To have the same response structure across, you can generate a random id and send it in response so that none of the existing SDKs break. We currently use Date.now().toString() in some providers as response id.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/generate.ts Outdated

+                generateInvalidProviderResponseError,
+              } from '../utils';
+              export const MonsterAPIChatCompleteConfig: ProviderConfig = {

Collaborator

VisargD Apr 9, 2024

As per the API docs, some of the mappings are missing here. Can you please check this once?
If any param is similar to an existing OpenAI param, you can use the OpenAI param name as input and map it to Monster's param. If there is no OpenAI compatible param, then you can add them as a new mapping as it is.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/generate.ts Outdated

Collaborator

VisargD Apr 9, 2024

As a part of naming convention that we follow in gateway, please use chatComplete instead of generate.

Collaborator

VisargD commented Apr 13, 2024

Hey @Dheeraj-Bhandari - Just checking up on this. Please let me know if there any blockers.

Contributor Author

DheerajQblocks commented Apr 13, 2024

Hey @Dheeraj-Bhandari - Just checking up on this. Please let me know if there any blockers.

Sure I'm on with the required changes I will update you once it done

Dheeraj-Bhandari and others added 2 commits

April 23, 2024 21:39


          Added Required Changed as Per PR reviewer

55810c5


          Merge branch 'main' into main

1b63972

Contributor Author

DheerajQblocks commented Apr 23, 2024

@VisargD Added the Required changes as per the suggestion provided by you.

Please review/merge this PR ASAP.

VisargD reviewed

View reviewed changes

src/providers/monsterapi/api.ts Outdated

+                },
+                getEndpoint: ({ fn }) => {
+                  switch (fn) {
+                    case 'generate':

Collaborator

VisargD Apr 25, 2024

This case should also use chatComplete instead of generate. And the default should return an empty string. Basically this:

switch (fn) {
      case 'chatComplete':
        return '/generate';
      default:
        return '';
    }

Contributor Author

DheerajQblocks Apr 26, 2024

Done

VisargD reviewed

View reviewed changes

src/providers/monsterapi/chatComplete.ts Outdated

+                  min: 0,
+                  max: 1,
+                },
+                temp: {

Collaborator

VisargD Apr 25, 2024

The openAI compatible name is temperature and not temp. So the input will be temperature which can then be mapped to MonsterAPI specific param name

Contributor Author

DheerajQblocks Apr 26, 2024

Done

VisargD reviewed

View reviewed changes

src/providers/monsterapi/index.ts

Collaborator

VisargD Apr 25, 2024

We can remove the comments from the file as they are not required and they are a part of github PR review so they should not be a part of the actual code.

Contributor Author

DheerajQblocks Apr 26, 2024

Done

Dheeraj-Bhandari and others added 2 commits

April 26, 2024 09:41


          Added Required Chamges

944a285


          Merge branch 'main' into main

9e68d49

Contributor Author

DheerajQblocks commented Apr 26, 2024

@VisargD Required changes made. Please review PR


          Merge branch 'main' into main

f30926b

Collaborator

VisargD commented Apr 29, 2024

Looks good to me. There is one problem that I found. I am not sure if its expected. The API error response structure that you have used in the chatComplete transformer is not aligned with monster API docs. The docs shows 2 types of error response:

{
  "detail": "\n    \"Invalid input payload, could be following.1 The specified model was not found, \"\n    1. Invalid Model\n    2. Bad payload\n"
}

{
  "detail": [
    {
      "loc": [
        "string",
        0
      ],
      "msg": "string",
      "type": "string"
    }
  ]
}

But the code that is written in this PR does not handle any of the above.

Collaborator

VisargD commented May 1, 2024

Hey @DheerajQblocks - Can you please check the last comment and the merge conflicts. Everything else looks good to me.

Contributor Author

DheerajQblocks commented May 1, 2024

Hey @DheerajQblocks - Can you please check the last comment and the merge conflicts. Everything else looks good to me.

Sure


          Merge branch 'main' into main

f9a3c19

Contributor Author

DheerajQblocks commented May 2, 2024

Looks good to me. There is one problem that I found. I am not sure if its expected. The API error response structure that you have used in the chatComplete transformer is not aligned with monster API docs. The docs shows 2 types of error response:
{
  "detail": "\n    \"Invalid input payload, could be following.1 The specified model was not found, \"\n    1. Invalid Model\n    2. Bad payload\n"
}
{
  "detail": [
    {
      "loc": [
        "string",
        0
      ],
      "msg": "string",
      "type": "string"
    }
  ]
}
But the code that is written in this PR does not handle any of the above.

Hi @VisargD
I think our docs are not updated so you can ignore this.
Although I am sending the exact same error response which I am getting from API.

Contributor Author

DheerajQblocks commented May 2, 2024

Hey @DheerajQblocks - Can you please check the last comment and the merge conflicts. Everything else looks good to me.

@VisargD I have resolved the conflict. Please Review / Merge it on periority.

VisargD approved these changes

View reviewed changes

VisargD merged commit c1c714b into Portkey-AI:main

1 check passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet