Skip to content

Conversation

horochx
Copy link
Contributor

@horochx horochx commented Jun 30, 2025

Description

This PR enhances the DashScope provider by adding support for new features documented in the official guide(zh,en).

Key Updates:

  • Support for new parameters, including enable_search and enable_thinking.

  • Integration of the rerank API.

Motivation

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

…d `enable_search`, and can invoke a rerank model.
Copy link

matter-code-review bot commented Jun 30, 2025

Code Quality new feature architecture

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

  • The DashScope API base URL has been updated from https://dashscope.aliyuncs.com to https://dashscope-intl.aliyuncs.com/compatible-mode/v1.
  • The chatComplete and embed API endpoints were simplified by removing the now redundant /compatible-mode/v1 prefix, as it's now part of the base URL.
  • The rerank API functionality, including its configuration in src/providers/dashscope/index.ts and src/providers/dashscope/api.ts, and its dedicated file src/providers/dashscope/rerank.ts, has been entirely removed.

🔍 Impact of the Change

  • Aligns the DashScope provider with a new international base URL, potentially enabling broader access or specific regional features.
  • Streamlines API endpoint definitions for chatComplete and embed by centralizing the version prefix in the base URL.
  • Removes the rerank functionality, which may indicate a deprecation, a shift in feature scope, or a move to a different service.

📁 Total Files Changed

  • src/providers/dashscope/api.ts: Modified base URL and endpoints, removed rerank endpoint (3 additions, 5 deletions).
  • src/providers/dashscope/index.ts: Removed rerank configuration import and usage (0 additions, 2 deletions).
  • src/providers/dashscope/rerank.ts: Entire file removed (0 additions, 15 deletions).

🧪 Test Added

N/A

🔒Security Vulnerabilities

N/A

Motivation

This PR enhances the DashScope provider by updating its API configuration to align with new service endpoints and removing deprecated or unused functionality.

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 unit and/or integration tests for the updated API endpoints (chatComplete, embed) to ensure correct URL construction and successful API interaction.

  2. Update documentation to clearly reflect the new base URL, simplified endpoints, and the removal of the rerank functionality.

  3. Clarify the reason for rerank removal in the PR description or code comments if it's a deprecation or intentional scope change, as the PR body was misleading.

Tanka Poem ♫

New base URL set,
Rerank code now fades away,
Endpoints align.
API's path, a clearer way,
Data flows, a silent hum. 🚀

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant DashScopeAPIConfig as DashScope API Config
    participant DashScopeService as DashScope Service
    participant DashScopeExternal as DashScope External API

    App->>DashScopeService: callChatComplete(params)
    DashScopeService->>DashScopeAPIConfig: getBaseURL()
    DashScopeAPIConfig-->>DashScopeService: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1"
    DashScopeService->>DashScopeAPIConfig: getEndpoint("chatComplete")
    DashScopeAPIConfig-->>DashScopeService: "/chat/completions"
    DashScopeService->>DashScopeExternal: POST /compatible-mode/v1/chat/completions (with new base URL)
    DashScopeExternal-->>DashScopeService: chatResponse

    App->>DashScopeService: callEmbed(params)
    DashScopeService->>DashScopeAPIConfig: getBaseURL()
    DashScopeAPIConfig-->>DashScopeService: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1"
    DashScopeService->>DashScopeAPIConfig: getEndpoint("embed")
    DashScopeAPIConfig-->>DashScopeService: "/embeddings"
    DashScopeService->>DashScopeExternal: POST /compatible-mode/v1/embeddings (with new base URL)
    DashScopeExternal-->>DashScopeService: embedResponse

    Note over DashScopeAPIConfig: Rerank functionality removed
    Note over DashScopeService: No longer supports rerank calls
Loading

Copy link

@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 excellent improvements to the DashScope provider by adding rerank functionality and enhancing the chat completion support. I've identified a few minor improvements that could make the code even better.

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.

You can remove the rerank changes, rest looks good

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.

I'd recommend removing the rerank code because it wouldn't work out of the box, but rest of the PR is good to go!

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

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

@horochx
Copy link
Contributor Author

horochx commented Aug 28, 2025

Hi @narengogi,

Apologies for the long delay on this. I got a bit sidetracked with some other work.

I believe I've already responded to your comment about removing the rerank code here: #1196 (comment). It's been a while, so it's totally understandable if you forgot! (And if you've changed your mind and still want me to remove the rerank code, no problem—just let me know!)

I've now made the requested changes and moved the rerank code into a separate file. Please take another look whenever you have a chance. Thanks so much!

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

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

@VisargD VisargD merged commit 1464cb4 into Portkey-AI:main Aug 28, 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.

3 participants