Skip to content

feat: transform raw image responses to json #1067

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.

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

unsync
Copy link
Contributor

@unsync unsync commented Apr 26, 2025

Code Quality new feature

Summary By MatterAI

🔄 What Changed

This PR adds functionality to transform image content type responses from Segmind to JSON format. It introduces a new handler for converting image responses to base64-encoded JSON, specifically targeting the imageGenerate route. The implementation includes error handling to fall back to the original response if transformation fails.

🔍 Impact of the Change

This change improves the handling of image responses by standardizing them to JSON format, making it easier for clients to process the responses. It's designed as a non-breaking change that enhances the existing functionality without disrupting current behavior.

📁 Total Files Changed

5 files changed with 67 additions and 1 deletion:

  • Added new file: src/handlers/imageToJsonHandler.ts
  • Modified: src/handlers/responseHandlers.ts
  • Modified: src/handlers/retryHandler.ts
  • Modified: src/providers/segmind/imageGenerate.ts
  • Modified: src/providers/segmind/index.ts

🧪 Test Added

No tests have been added in this PR.

🔒 Security Vulnerabilities

No security vulnerabilities detected.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • 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

Quality Recommendations

  1. Add unit tests for the new image transformation functionality

  2. Add more detailed error logging in imageToJsonHandler.ts with specific error types

  3. Consider adding a content-type validation function to avoid repetitive checks

  4. Add documentation for the new image transformation feature

Sequence Diagram

sequenceDiagram
    participant Client
    participant ResponseHandler
    participant RetryHandler
    participant ImageToJsonHandler
    participant SegmindProvider

    Client->>ResponseHandler: Request image generation
    ResponseHandler->>RetryHandler: Process request with retry logic
    RetryHandler->>SegmindProvider: Call Segmind API
    SegmindProvider-->>RetryHandler: Return image response
    RetryHandler-->>ResponseHandler: Return response
    
    Note over ResponseHandler: Check if response needs transformation
    alt responseTransformer === 'imageGenerate' && has imageToJson transformer
        ResponseHandler->>ImageToJsonHandler: getTransformedResponse(response, transformer)
        Note over ImageToJsonHandler: Convert image to base64
        ImageToJsonHandler-->>ResponseHandler: Return JSON response
    end
    
    ResponseHandler-->>Client: Return final response
Loading

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.

@unsync I have a few items in mind wrt this PR

  1. we should be handling transforms only for specific routes and not based on response, so if segmind has a /v1/images/generations route, we can do these changes for that route
  2. I'm not entirely keen we should load the entire image file into memory and convert it to json, but that's okay

overall I'm saying this PR could only include changes to provider/segmind/imageGenerate.ts and provider/segmind/api.ts and provider/segmind/index.ts

@unsync
Copy link
Contributor Author

unsync commented Apr 30, 2025

@narengogi if i get this correctly, you would prefer to scope the changes to the providers handlers.

Unless i'm missing something, that would not work without a change in the reponseHandler because of this:

if (responseContentType?.startsWith(CONTENT_TYPES.GENERIC_IMAGE_PATTERN)) {
return { response: handleImageResponse(response), responseJson: null };
}

This prevents to reach the code that handles the transformations:

const nonStreamingResponse = await handleNonStreamingMode(
response,
responseTransformerFunction,
strictOpenAiCompliance,
gatewayRequestUrl,
gatewayRequest,
areSyncHooksAvailable
);

And before you hit handleNonStreamingMode, you need to have a JSON body, otherwise this fails:

const isJsonParsingRequired = responseTransformer || areSyncHooksAvailable;
const originalResponseBodyJson: Record<string, any> | null =
isJsonParsingRequired ? await response.json() : null;

Since the provider returns a content type image/ then we don't enter the transformer function of the providers (if we did, i definitely would have gone this road, that was my first idea ^^).

Let me know if i missed something here 🙏

And if that doesn't makes it's way to main no hard feelings, i'm using this on a fork, i just wanted for others to benefit from it 👍

@narengogi
Copy link
Collaborator

I spent some time on this, I was initially misreading what the PR is doing, this fix makes sense for the problem,

one change we could do is to convert to json only if routes are in the unified routes like v1/images/generations

I'm copying @VisargD on this for a review of the approach

@unsync
Copy link
Contributor Author

unsync commented Apr 30, 2025

I spent some time on this, I was initially misreading what the PR is doing, this fix makes sense for the problem,

one change we could do is to convert to json only if routes are in the unified routes like v1/images/generations

I'm copying @VisargD on this for a review of the approach

@narengogi @VisargD i made a last commit to the PR to, imho, limit the scope of the behaviour in a more elegant way:

  // if the original response is an image, convert it to JSON if the provider has a transformer
  if (
    responseTransformer === 'imageGenerate' && // check that we are on the imageGenerate route
    responseTransformerFunction && // check that we have a transformer for this provider
    providerTransformers?.[`imageToJson`] && // check that we have a 'imageToJson" transformer for this provider
    response.headers
      ?.get('content-type')
      ?.startsWith(CONTENT_TYPES.GENERIC_IMAGE_PATTERN) // check that the original response content type is an image
  ) {
    // transformers are async, because we read the body as an array buffer
    response = await providerTransformers?.[`imageToJson`](response);
  }

This ensures that :

  • we only apply it when the response is initially a image/***
  • we are looking for a responseTransformer of value imageGenerate (ensuring to do this only on image generation)
  • the provider has the responseTransformerFunction (no point transforming the response if the provider doesn't provide a transformer)
  • obviously, checks if the provider has a imageToJson transformer

I also added a try/catch the the transformer wrapper, so that if the transformation fails we return the original response to avoid any failures due to this logic

@unsync unsync changed the title poc: transform segmind image content type responses to json feat: transform raw image responses to json May 5, 2025
@unsync unsync marked this pull request as ready for review May 5, 2025 11:56
Copy link

Code Quality new feature

Summary By MatterAI MatterAI logo

🔄 What Changed

This PR adds functionality to transform raw image responses from providers (specifically Segmind) into JSON format. It introduces a new handler for converting image responses to base64-encoded JSON, enhancing the API's response consistency. The implementation includes fallback mechanisms to return the original response if transformation fails.

🔍 Impact of the Change

This change improves client experience by providing consistent JSON responses instead of raw binary image data. It makes it easier for clients to handle image generation responses without needing special handling for binary data. The transformation is specifically targeted at the imageGenerate route and only applies when appropriate conditions are met.

📁 Total Files Changed

  • Added: src/handlers/imageToJsonHandler.ts (35 lines)
  • Modified: src/handlers/responseHandlers.ts (16 additions, 1 deletion)
  • Modified: src/handlers/retryHandler.ts (2 additions)
  • Modified: src/providers/segmind/imageGenerate.ts (12 additions)
  • Modified: src/providers/segmind/index.ts (2 additions)

🧪 Test Added

N/A - No tests were added in this PR.

🔒Security Vulnerabilities

N/A - No security vulnerabilities were identified.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • 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
  • Manual Testing
  • Integration Tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Sequence Diagram

sequenceDiagram
    participant Client
    participant Gateway
    participant ResponseHandler
    participant ImageToJsonHandler
    participant SegmindProvider
    participant ExternalAPI

    Client->>Gateway: Request image generation
    Gateway->>ExternalAPI: Forward request to provider API
    ExternalAPI-->>Gateway: Return raw image response
    Gateway->>ResponseHandler: Process response
    Note over ResponseHandler: Check if response is for imageGenerate
    Note over ResponseHandler: Check if content-type is image
    Note over ResponseHandler: Check if provider has imageToJson transformer
    ResponseHandler->>SegmindProvider: Call imageToJson transformer
    SegmindProvider->>ImageToJsonHandler: Call getTransformedResponse()
    Note over ImageToJsonHandler: Convert image to base64
    Note over ImageToJsonHandler: Transform to JSON format
    ImageToJsonHandler-->>SegmindProvider: Return JSON Response
    SegmindProvider-->>ResponseHandler: Return transformed response
    ResponseHandler-->>Gateway: Return final response
    Gateway-->>Client: Return JSON response instead of raw image
    Note over Gateway: If transformation fails, original response is returned
Loading

unsync added 6 commits June 18, 2025 14:14
Implements provider-specific response formatting for image data.

Adds helper functions to identify image providers and format responses correctly based on the provider's requirements, ensuring proper handling of base64 image responses.

Improves the robustness of the image handling logic to adapt to different provider responses.
Streamlines the image response transformation process by consolidating functions for image transformation into `getImageTransformer`.

Removes unnecessary constants and simplifies the logic to parse image data, resulting in improved readability and maintainability.

Enhances logging for better traceability during retry attempts by providing detailed response status information.

No functionality changes are introduced; the focus is on code clarity and structure.
@unsync unsync force-pushed the feature/transform-images-to-json branch from 7777f37 to 14b014f Compare June 18, 2025 12:16
Copy link

Code Quality new feature

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This pull request introduces a new feature to transform raw image responses from providers into a JSON format. A new handler imageToJsonHandler.ts was added to convert image ArrayBuffer to a base64 string and embed it into a JSON object. The responseHandlers.ts file was modified to conditionally apply this transformation when an imageGenerate route is detected, a transformer is available, and the response content type is an image. Specifically, the Segmind provider was updated to include an imageToJson response transformer that utilizes the new handler.

🔍 Impact of the Change

This change allows clients to receive image generation responses in a standardized JSON format, where the image data is base64 encoded. This is particularly useful for environments that prefer or require JSON payloads, simplifying client-side parsing and handling of image responses. It enhances the flexibility of the API by providing an alternative response format for image generation.

📁 Total Files Changed

4 files were changed:

  • src/handlers/imageToJsonHandler.ts (added)
  • src/handlers/responseHandlers.ts (modified)
  • src/providers/segmind/imageGenerate.ts (modified)
  • src/providers/segmind/index.ts (modified)

🧪 Test Added

No explicit tests were added or mentioned in the pull request description. The PR is marked as "base for discussion/comments", suggesting it might be a work in progress or requires further testing.

🔒Security Vulnerabilities

No security vulnerabilities were detected in the provided code changes.

Motivation

To transform raw image responses to JSON format, embedding the image as a base64 string, for better client-side handling and standardization.

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
    Not explicitly mentioned in PR description, PR is marked as 'base for discussion/comments'.

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. In getTransformedResponse, consider returning a specific error response (e.g., 500 status with an error message) instead of the original response when an error occurs during transformation. This would make it clearer to the consumer that the transformation failed, rather than potentially receiving an unexpected content type.

  2. The if condition for image transformation in responseHandlers.ts is quite verbose. While functional, consider extracting this logic into a separate, well-named helper function to improve readability and maintainability, especially if more such content-type specific transformations are anticipated.

  3. For production environments, consider replacing console.info and console.error with a more robust logging solution that supports different log levels, structured logging, and integration with monitoring systems.

Sequence Diagram

sequenceDiagram
    participant User
    participant API_Endpoint
    participant ResponseHandler
    participant ProviderTransformers
    participant SegmindImageToJsonResponseTransform
    participant GetTransformedResponse

    User->>API_Endpoint: Request Image Generation (e.g., POST /v1/image/generate)
    API_Endpoint->>ResponseHandler: Call responseHandler(response: Response, provider: Object, ...)
    ResponseHandler->>ResponseHandler: Check if responseTransformer is 'imageGenerate'
    ResponseHandler->>ResponseHandler: Check if providerTransformers.imageToJson exists
    ResponseHandler->>ResponseHandler: Check if response.headers['content-type'] starts with 'image/'
    alt Image Transformation Conditions Met
        ResponseHandler->>ProviderTransformers: Call providerTransformers.imageToJson(originalResponse: Response)
        ProviderTransformers->>SegmindImageToJsonResponseTransform: Call SegmindImageToJsonResponseTransform(originalResponse: Response)
        SegmindImageToJsonResponseTransform->>GetTransformedResponse: Call getTransformedResponse({response: originalResponse, transformer: (base64Image: string) => ({ image: base64Image })})
        GetTransformedResponse->>GetTransformedResponse: Read originalResponse.arrayBuffer()
        GetTransformedResponse->>GetTransformedResponse: Convert ArrayBuffer to base64 string (via Uint8Array, String.fromCharCode, btoa)
        GetTransformedResponse->>GetTransformedResponse: Apply transformer function to base64 string
        GetTransformedResponse-->>SegmindImageToJsonResponseTransform: Return new Response (JSON, status, headers)
        SegmindImageToJsonResponseTransform-->>ResponseHandler: Return transformed JSON Response
    else No Image Transformation
        ResponseHandler->>ResponseHandler: Continue with original Response
    end
    ResponseHandler->>ResponseHandler: Read final response.headers['content-type']
    ResponseHandler-->>API_Endpoint: Return {response: finalResponse, ...}
    API_Endpoint-->>User: Return Final Response (JSON or original format)
Loading

Copy link

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

@unsync unsync requested a review from narengogi June 18, 2025 12:17
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