Skip to content

Conversation

@narengogi
Copy link
Collaborator

No description provided.

@matter-code-review
Copy link
Contributor

Code Quality new feature bug fix maintainability security

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This pull request refactors and enhances the Azure OpenAI implementation. Key changes include adding support for Azure AD token authentication, improving batch output retrieval to handle error files more robustly, and standardizing response transformations across various operations using a generic OpenAIResponseTransform. Parameter definitions for chatComplete and embed have also been adjusted for consistency, and a redundant uploadFile.ts module has been removed.

🔍 Impact of the Change

The changes significantly improve the security posture and flexibility of Azure OpenAI integrations by enabling Azure AD authentication. The enhanced batch output handling provides more reliable error reporting and a clearer response format for batch processing failures. Code maintainability is boosted through the adoption of generic response transformers, reducing duplication and simplifying future updates. Overall, the system becomes more robust and easier to manage.

📁 Total Files Changed

  • src/providers/azure-openai/api.ts: Added azureAdToken support for Authorization headers, prioritizing it over apiKey.
  • src/providers/azure-openai/chatComplete.ts: Refactored logprobs and top_logprobs parameter placement within the configuration.
  • src/providers/azure-openai/embed.ts: Modified the encoding_format parameter definition, removing an explicit required: false property.
  • src/providers/azure-openai/getBatchOutput.ts: Enhanced batch output retrieval to check for both output_file_id and error_file_id, and improved the error response format with provider_response details.
  • src/providers/azure-openai/index.ts: Replaced specific AzureOpenAIResponseTransform usages with the generic OpenAIResponseTransform for file, finetune, and batch operations.
  • src/providers/azure-openai/uploadFile.ts: This file was removed, indicating a consolidation of request transformation logic.

🧪 Test Added

N/A

🔒 Security Vulnerabilities

No new security vulnerabilities were introduced. The addition of Azure AD token support enhances the security options available for authentication.

Motivation

N/A

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. Consider adding unit tests specifically for the new error handling path in getBatchOutput.ts to ensure robust behavior when output_file_id and error_file_id are both missing or when error_file_id is present.

Tanka Poem ♫

Auth now flows with ease,
Batch errors, clearer seen.
Code refactored, clean,
New features bloom, serene.
Science smiles, a job well done. ✨

Sequence Diagram

sequenceDiagram
    participant Client
    participant BatchOutputHandler as AzureOpenAIGetBatchOutputRequestHandler
    participant AzureOpenAIAPI as Azure OpenAI API
    participant PortkeyAPI as Portkey API

    Client->>BatchOutputHandler: requestBatchOutput(options)
    Note over BatchOutputHandler: Retrieve batch details
    BatchOutputHandler->>AzureOpenAIAPI: GET /batches/{batchId}
    AzureOpenAIAPI-->>BatchOutputHandler: batchDetails: RetrieveBatchResponse

    alt outputFileId or errorFileId exists
        Note over BatchOutputHandler: Determine file to retrieve
        BatchOutputHandler->>PortkeyAPI: GET /v1/files/{fileId}/content
        PortkeyAPI-->>BatchOutputHandler: fileContentStream
        BatchOutputHandler-->>Client: ReadableStream (batch output)
    else No valid file ID
        Note over BatchOutputHandler: Construct detailed error response
        BatchOutputHandler-->>Client: 400 Bad Request (error JSON with provider_response)
    end
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.

Code cleanup improves Azure OpenAI implementation consistency and error handling.

@VisargD VisargD merged commit e212e83 into Portkey-AI:main Oct 22, 2025
1 check 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.

2 participants