Skip to content

feat(providers): add siliconflow-cn as a built-in model provider - #775

Open
wu21-web wants to merge 13 commits into
alibaba:mainfrom
wu21-web:feat/siliconflow-cn
Open

feat(providers): add siliconflow-cn as a built-in model provider#775
wu21-web wants to merge 13 commits into
alibaba:mainfrom
wu21-web:feat/siliconflow-cn

Conversation

@wu21-web

@wu21-web wu21-web commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Description

This pull request is a follow-up of #772 , adds a siliconflow-cn for chinese users at the sametime.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual testing (describe below)

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (if applicable)
  • I have signed the CLA

Related Issues

none yet

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

@wu21-web
wu21-web marked this pull request as ready for review August 7, 2026 14:54
@wu21-web
wu21-web marked this pull request as draft August 8, 2026 05:25
@wu21-web
wu21-web marked this pull request as ready for review August 8, 2026 05:29
@wu21-web wu21-web changed the title feat: add siliconflow-cn as a built-in model provider feat(providers): add siliconflow-cn as a built-in model provider Aug 10, 2026
@lizhengfeng101

Copy link
Copy Markdown
Collaborator

Same feedback as on #772 — the Models list here is identical, so the same concern applies to this preset.

Several entries look like previous-generation models rather than what a new user would want as a first choice:

  • deepseek-ai/DeepSeek-V3 and deepseek-ai/DeepSeek-R1 are superseded by the V3.2/V4 line already in the same list.
  • Qwen/Qwen3-32B and Qwen/Qwen3-14B are both older and quite small for this workload — review runs depend heavily on reliable tool calling and long-context handling, and small models tend to degrade there.
  • baidu/ERNIE-4.5-300B-A47B also looks like an older generation.

These presets are exactly what people pick from the provider TUI without thinking twice, so the defaults carry a lot of weight. Could you re-verify the list and confirm:

  1. Every model id still resolves on the CN endpoint (https://api.siliconflow.cn/v1) — the region split with feat(providers): add siliconflow (GLOBAL) to built-in providers #772 means the two lists should not be assumed identical unless you have checked both.
  2. Each listed model supports the streaming + tool-calling behaviour the review loop needs.
  3. The list reflects the current generation, and older/smaller entries are either dropped or intentionally kept (a one-line rationale in the PR description would help if intentional).

One process note: this PR and #772 both edit the same expected slice in TestListProviders_Order, so whichever lands first will leave the other with a conflict. It may be simpler to fold both presets into a single PR, or to confirm the model list once and apply the same result to both.

A shorter list of models you have actually run a review with is more valuable than a longer list that is partly stale. Thanks!

@wu21-web

Copy link
Copy Markdown
Contributor Author

I will notify you when a merged conflict is resolved.

@wu21-web

Copy link
Copy Markdown
Contributor Author

Model changes have been synced to this pull request.

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