Skip to content

Conversation

@narengogi
Copy link
Collaborator

@narengogi narengogi commented Jun 19, 2025

this only fixes the finish reason for anthropic and bedrock converse for chat completions, need to add it slowly for every provider

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 adds a nice improvement for OpenAI compatibility by properly mapping Anthropic's finish reasons to OpenAI's format. The implementation is clean and well-documented. I have a few minor suggestions to improve the code.

@matter-code-review
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

@Portkey-AI Portkey-AI deleted a comment from matter-code-review bot Jun 19, 2025
@narengogi narengogi requested review from VisargD and b4s36t4 June 19, 2025 11:40
@matter-code-review
Copy link
Contributor

matter-code-review bot commented Jun 19, 2025

Code Quality new feature

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This pull request introduces a new mechanism to improve strict OpenAI compliance, specifically for the finish_reason field in chat completion responses from Anthropic and Bedrock providers. It centralizes the transformation logic for provider-specific stop reasons into a new transformFinishReason utility function. New enums (ANTHROPIC_STOP_REASON, BEDROCK_STOP_REASON, FINISH_REASON) and a union type (PROVIDER_FINISH_REASON) have been added for enhanced type safety. A dedicated finishReasonMap.ts file now explicitly defines the mapping from provider stop reasons to OpenAI's standard FINISH_REASON.

🔍 Impact of the Change

This change ensures that the finish_reason returned by Anthropic and Bedrock chat completion APIs aligns with OpenAI's API specification when strictOpenAiCompliance is enabled. This improves interoperability and consistency for consumers expecting OpenAI-like responses. It also enhances code maintainability and readability by centralizing the mapping logic and introducing strong typing for stop reasons.

📁 Total Files Changed

7 files were changed in this pull request:

  • src/providers/anthropic/chatComplete.ts (modified)
  • src/providers/anthropic/types.ts (modified)
  • src/providers/bedrock/chatComplete.ts (modified)
  • src/providers/bedrock/types.ts (modified)
  • src/providers/types.ts (modified)
  • src/providers/utils.ts (modified)
  • src/providers/utils/finishReasonMap.ts (added)

🧪 Test Added

No specific tests were explicitly mentioned or added in the provided pull request data. The changes primarily involve type definitions and a mapping function, which would ideally be covered by unit tests to ensure correct transformations for all defined stop reasons and edge cases.

🔒Security Vulnerabilities

No new security vulnerabilities were detected in this pull request. The changes are focused on API response transformation and type safety, which do not introduce direct security risks.

Motivation

To improve strict OpenAI compliance by standardizing the finish_reason field across different AI providers (Anthropic, Bedrock) to match OpenAI's API specification.

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

N/A

Tip

Quality Recommendations

  1. Add comprehensive unit tests for the transformFinishReason function to cover all possible PROVIDER_FINISH_REASON values, including undefined and potentially unmapped future values, ensuring correct mapping and default behavior.

  2. Consider adding JSDoc comments to the transformFinishReason function for better API documentation, detailing its parameters, return value, and behavior based on strictOpenAiCompliance.

Sequence Diagram

sequenceDiagram
    participant PR as Pull Request
    participant AnthropicChatComplete as Anthropic Chat Completion
    participant BedrockChatComplete as Bedrock Chat Completion
    participant Utils as src/providers/utils.ts
    participant FinishReasonMap as src/providers/utils/finishReasonMap.ts

    PR->AnthropicChatComplete: Modify AnthropicChatCompleteResponse/StreamResponse
    PR->BedrockChatComplete: Modify BedrockChatCompletionResponse/StreamChunk
    PR->AnthropicChatComplete: Import transformFinishReason
    PR->BedrockChatComplete: Import transformFinishReason

    AnthropicChatComplete->Utils: Call transformFinishReason(response.stop_reason, strictOpenAiCompliance)
    BedrockChatComplete->Utils: Call transformFinishReason(response.stopReason, strictOpenAiCompliance)

    Utils->FinishReasonMap: Lookup finishReasonMap.get(providerFinishReason)
    alt strictOpenAiCompliance is true
        FinishReasonMap-->Utils: Return mapped FINISH_REASON (e.g., 'stop', 'length', 'tool_calls')
    else strictOpenAiCompliance is false
        FinishReasonMap-->Utils: Return original PROVIDER_FINISH_REASON
    end
    Utils-->AnthropicChatComplete: Return transformed finish_reason
    Utils-->BedrockChatComplete: Return transformed finish_reason

    AnthropicChatComplete->AnthropicChatComplete: Assign transformed finish_reason to OpenAI-compatible response
    BedrockChatComplete->BedrockChatComplete: Assign transformed finish_reason to OpenAI-compatible response

    Note over PR,FinishReasonMap: New enums ANTHROPIC_STOP_REASON, BEDROCK_STOP_REASON, FINISH_REASON, PROVIDER_FINISH_REASON defined in types.ts
    Note over PR,FinishReasonMap: finishReasonMap.ts created for explicit mapping
Loading

@matter-code-review
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

@matter-code-review
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

@matter-code-review
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

@matter-code-review
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

@matter-code-review
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

@matter-code-review
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

@matter-code-review
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

@VisargD VisargD merged commit b52ad3b into Portkey-AI:main Jun 25, 2025
2 checks passed
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.

3 participants