Skip to content

Conversation

Ajay-Satish-01
Copy link
Contributor

@Ajay-Satish-01 Ajay-Satish-01 commented Jul 8, 2025

Description

Migration to Cohere V2 format for chat completion and streaming.

Motivation

To support the latest version of Cohere endpoints.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

#1028

Copy link
Contributor

matter-code-review bot commented Jul 8, 2025

Code Quality bug fix new feature

Description

🔄 What Changed

This Pull Request introduces the integration of the Krutrim AI provider, enhancing the platform's capabilities with a new chat completion service. It also refines the handling of streaming responses by adding robust try...catch...finally blocks to ensure proper stream closure even during errors. Furthermore, the PR updates Portkey plugins (gibberish.ts, language.ts, moderateContent.ts) to use getCurrentContentPart for more reliable text extraction, including null checks for content. Bedrock API response transformations for chat completion have been improved to accurately report cache usage tokens, and embedding configurations for Bedrock and Cohere now handle encoding_format more flexibly.

🔍 Impact of the Change

The integration of Krutrim expands the range of available AI models, offering more choices for users. The enhanced error handling in streaming ensures greater stability and prevents resource leaks, improving the reliability of long-running operations. The refined text extraction in Portkey plugins makes them more robust against malformed or empty input, reducing potential runtime errors. Accurate cache usage reporting for Bedrock provides clearer insights into token consumption.

📁 Total Files Changed

  • plugins/portkey/gibberish.ts: Updated text extraction logic with null checks.
  • plugins/portkey/language.ts: Updated text extraction logic with null checks.
  • plugins/portkey/moderateContent.ts: Updated text extraction logic with null checks.
  • src/globals.ts: Added KRUTRIM provider constant.
  • src/handlers/streamHandler.ts: Added try...catch...finally for stream error handling and resource cleanup.
  • src/providers/bedrock/chatComplete.ts: Improved cache token reporting in usage statistics.
  • src/providers/bedrock/embed.ts: Enhanced encoding_format transformation for embedding.
  • src/providers/cohere/embed.ts: Enhanced encoding_format transformation for embedding.
  • src/providers/index.ts: Registered the new KrutrimConfig.
  • src/providers/jina/embed.ts: Added dimensions parameter for Jina embeddings.
  • src/providers/krutrim/api.ts: New file for Krutrim API base URL and headers.
  • src/providers/krutrim/chatComplete.ts: New file for Krutrim chat completion response transformation.
  • src/providers/krutrim/index.ts: New file for Krutrim provider configuration.
  • src/providers/open-ai-base/createModelResponse.ts: Added modalities and parallel_tool_calls parameters.
  • src/providers/types.ts: Extended CResponse interface with detailed token usage.

🧪 Test Added

N/A (No explicit test files or descriptions provided in the PR JSON. However, given the nature of changes, unit tests for new provider integration and integration tests for streaming error handling would be crucial.)

🔒Security Vulnerabilities

No new security vulnerabilities detected. The changes to streamHandler.ts improve reliability, which can indirectly contribute to security by preventing denial-of-service scenarios due to unclosed streams. The null checks in Portkey plugins enhance defensive programming.

Motivation

Migration to Cohere V2 format for chat completion and streaming, and integration of the Krutrim AI provider to expand model offerings.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

.

Tip

Quality Recommendations

  1. Consider implementing a more robust and structured logging mechanism in streamHandler.ts instead of console.error for better observability and debugging in production environments.

  2. Evaluate if the textArray.filter((text) => text).join('\n') logic in Portkey plugins could be encapsulated within getCurrentContentPart or optimized if textArray is guaranteed to contain only strings after initial processing.

  3. Ensure comprehensive unit and integration tests are developed for the new Krutrim provider integration to validate its functionality, error handling, and compliance with expected API responses.

  4. Verify that the html-message error parsing for Krutrim is resilient to variations in the error response structure, potentially adding more specific checks or a fallback for unexpected formats.

Sequence Diagram

sequenceDiagram
    participant U as User/Client
    participant Plugin as Portkey Plugin (Gibberish/Language/ModerateContent)
    participant Utils as Plugin Utils
    participant PortkeyAPI as Portkey API
    participant StreamHandler as Stream Handler
    participant Reader as Stream Reader
    participant Writer as Stream Writer
    participant BedrockAPI as Bedrock API
    participant KrutrimAPI as Krutrim API
    participant ProviderConfig as Provider Configuration

    U->>Plugin: callPlugin(context, eventType, parameters)
    Plugin->>+Utils: getCurrentContentPart(context, eventType)
    Note over Utils: Extracts content and text array from request/response JSON
    Utils-->>-Plugin: {content, textArray}
    alt Content is empty
        Plugin-->>U: {error: "request or response json is empty"}
    else Content exists
        Plugin->>Plugin: text = textArray.filter(...).join('
')
        Plugin->>+PortkeyAPI: fetchPortkey(text, parameters)
        Note over PortkeyAPI: POST /v1/complete
        PortkeyAPI-->>-Plugin: responseData
        Plugin-->>U: {verdict, data}
    end

    U->>StreamHandler: handleStreamingMode(proxyProvider, ...)
    alt proxyProvider is BEDROCK
        StreamHandler->>+Reader: readAWSStream(reader, transformer, ...)
        loop For each chunk
            Reader-->>StreamHandler: chunk
            StreamHandler->>+Writer: write(encoder.encode(chunk))
            Writer-->>-StreamHandler: chunkWritten
        end
        alt Error during stream
            Reader--xStreamHandler: streamError
            StreamHandler->>StreamHandler: console.error(error)
        end
        StreamHandler->>Writer: close()
    else proxyProvider is other
        StreamHandler->>+Reader: readStream(reader, transformer, ...)
        loop For each chunk
            Reader-->>StreamHandler: chunk
            StreamHandler->>+Writer: write(encoder.encode(chunk))
            Writer-->>-StreamHandler: chunkWritten
        end
        alt Error during stream
            Reader--xStreamHandler: streamError
            StreamHandler->>StreamHandler: console.error(error)
        end
        StreamHandler->>Writer: close()
    end

    BedrockAPI->>StreamHandler: BedrockChatCompleteResponseTransform(response, status)
    Note over StreamHandler: Transforms Bedrock API response to standard format
    StreamHandler->>StreamHandler: Calculate cacheReadInputTokens, cacheWriteInputTokens
    StreamHandler->>StreamHandler: Update prompt_tokens to include cache tokens
    StreamHandler->>StreamHandler: Conditionally add cache_read/creation_input_tokens
    StreamHandler-->>U: Transformed ChatCompletionResponse

    U->>KrutrimAPI: chatComplete(model, messages, ...)
    Note over KrutrimAPI: POST /v1/chat/completions
    KrutrimAPI-->>U: KrutrimChatCompleteResponse | KrutrimChatCompleteErrorResponse
    alt Krutrim API Error (status != 200)
        U->>ProviderConfig: KrutrimChatCompleteResponseTransform(errorResponse, status)
        ProviderConfig->>ProviderConfig: generateErrorResponse(errorDetails, KRUTRIM)
        ProviderConfig-->>U: ErrorResponse
    else Krutrim API Success
        U->>ProviderConfig: KrutrimChatCompleteResponseTransform(successResponse, status)
        ProviderConfig->>ProviderConfig: Add 'provider: KRUTRIM' to response
        ProviderConfig-->>U: ChatCompletionResponse
    end

    U->>ProviderConfig: embed(params)
    Note over ProviderConfig: BedrockCohereEmbedConfig, BedrockTitanEmbedConfig, CohereEmbedConfig
    ProviderConfig->>ProviderConfig: transform encoding_format (string[] | undefined)
    ProviderConfig-->>U: Transformed Embed Request
Loading

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR implements Cohere V2 API integration for chat completions with streaming support. The code looks good overall, but I've identified a few improvements that could enhance error handling and type safety.

Co-authored-by: matter-code-review[bot] <150888575+matter-code-review[bot]@users.noreply.github.com>
Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@narengogi narengogi linked an issue Jul 9, 2025 that may be closed by this pull request
Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the transforms look incomplete, please reference https://docs.cohere.com/reference/chat for cohere documentation
and https://platform.openai.com/docs/api-reference/chat/create for openai documentation and implement teh full spec

plus you've not updated the api.ts file to call teh /v2 route instead of v1, so I'm unsure on how you've tested your changes, please make the neceessary changes and add testing done

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@Ajay-Satish-01
Copy link
Contributor Author

the transforms look incomplete, please reference https://docs.cohere.com/reference/chat for cohere documentation and https://platform.openai.com/docs/api-reference/chat/create for openai documentation and implement teh full spec

plus you've not updated the api.ts file to call teh /v2 route instead of v1, so I'm unsure on how you've tested your changes, please make the neceessary changes and add testing done

Yep. Sorry, my bad. The V2 change was not pushed and was staged instead.

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requesting changes

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

1 similar comment
Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Migrate Cohere Chat Completions to v2
2 participants