Skip to content

feat: add DeepSeek + GLM Coding Plan providers, fix DeepSeek reasonin…#604

Open
ngduyanhece wants to merge 1 commit intomainfrom
proj/llm-providers-expansion
Open

feat: add DeepSeek + GLM Coding Plan providers, fix DeepSeek reasonin…#604
ngduyanhece wants to merge 1 commit intomainfrom
proj/llm-providers-expansion

Conversation

@ngduyanhece
Copy link
Copy Markdown
Contributor

Summary

  • Problem: byterover-cli is missing DeepSeek (one of four user-requested providers) and lacks a way to route through Z.AI's Coding Plan subscription endpoint. Separately, DeepSeek reasoning models were mapped to a think-tags parser that never matched the API's actual reasoning_content field — reasoning output was silently dropped.
  • Why it matters: the four user-requested providers (DeepSeek, MiniMax, Moonshot/Kimi, Z.AI); correct billing for Z.AI Coding Plan subscribers (standard endpoint hits pay-per-token quota); DeepSeek users can finally see reasoning output instead of empty thinking blocks.
  • What changed:
    • New deepseek provider via @ai-sdk/openai-compatible against https://api.deepseek.com/v1 (env DEEPSEEK_API_KEY).
    • New glm-coding-plan provider as a separate ID alongside the existing glm, pointing at https://api.z.ai/api/coding/paas/v4, reusing ZHIPU_API_KEY.
    • Capability fix: deepseek-r1 / deepseek-reasoner now correctly map to reasoningFormat: 'native-field' with reasoningField: 'reasoning_content'.
    • DeepSeek SVG icon (Wikimedia Commons, MIT-licensed, whale-only path with cropped viewBox); GLM Coding Plan reuses the existing zai icon.
    • README provider table bumped 18 → 20.
  • What did NOT change (scope boundary): MiniMax, Moonshot/Kimi, and standard glm were already wired and untouched. No kimi-coding-plan provider, and Moonshot's coding-plan tier uses the same api.moonshot.ai/v1 endpoint as the standard provider. zhipuai-coding-plan (mainland-CN), minimax-coding-plan (Anthropic-compatible), and full models.dev catalog adoption are deferred per plan/llm-providers/DESIGN.md D3/D4.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Test
  • Chore (build, dependencies, CI)

Scope (select all touched areas)

  • TUI / REPL
  • Agent / Tools
  • LLM Providers
  • Server / Daemon
  • Shared (constants, types, transport events)
  • CLI Commands (oclif)
  • Hub / Connectors
  • Cloud Sync
  • CI/CD / Infra

Linked issues

  • Closes #N/A
  • Related #N/A

Root cause (bug fixes only)

  • Root cause: getModelCapabilities() in src/agent/infra/llm/model-capabilities.ts mapped deepseek-r1 / deepseek-reasoner to reasoningFormat: 'think-tags'. DeepSeek's OpenAI-compatible API does not emit <think>...</think> markers — it streams reasoning in a native reasoning_content field. The think-tag scanner never matched, so reasoning content was effectively dropped on the floor.
  • Why this was not caught earlier: DeepSeek had no provider-registry entry, so it was never reachable through brv providers connect and never exercised end-to-end. The capability rule sat in a code path that no integration test could trigger. The fix is forced by adding the provider; this PR closes the loop in one go.

Test plan

  • Coverage added:
    • Unit test
    • Integration test
    • Manual verification only
  • Test file(s):
    • test/unit/agent/llm/model-capabilities.test.ts (new) — 3 assertions for the capability fix
    • test/unit/agent/llm/providers/deepseek.test.ts (new) — 6 assertions for provider-module shape
    • test/unit/agent/llm/providers/glm-coding-plan.test.ts (new) — 7 assertions for provider-module shape
    • test/unit/core/domain/entities/provider-registry.test.ts (extended) — 13 new assertions (6 DeepSeek + 7 GLM Coding Plan)
  • Key scenario(s) covered:
    • Both new providers registered with correct base URL, env var, default model
    • deepseek-reasoner and deepseek-r1 resolve to native-field / reasoning_content; deepseek-chat reports no reasoning
    • glm-coding-plan coexists with glm (regression check that getProviderById('glm') still resolves and the two have distinct base URLs)
    • providerRequiresApiKey('deepseek') and providerRequiresApiKey('glm-coding-plan') both return true

User-visible changes

  • New entries in brv providers connect picker: DeepSeek and GLM Coding Plan (Z.AI).
  • DeepSeek reasoning models (deepseek-reasoner, deepseek-r1) now stream reasoning into the dedicated thinking block instead of silently dropping it.
  • Z.AI Coding Plan subscribers can now route through the dedicated endpoint to consume subscription quota (previously only the standard pay-per-token endpoint was reachable).
  • README provider table updated from 18 to 20 providers.

Evidence

  • Failing test/log before + passing after

TDD sequence (full output in commit history):

# Before implementation — 13 assertions red for the right reason
$ npx mocha "test/unit/core/domain/entities/provider-registry.test.ts" \
            "test/unit/agent/llm/model-capabilities.test.ts" \
            "test/unit/agent/llm/providers/deepseek.test.ts"
  6) Provider Registry > DeepSeek provider > should be registered:
     AssertionError: expected undefined not to be undefined
  ...
  22 passing, 13 failing

# After implementation — full suite green
$ npm test
  7316 passing (19s)
  16 pending

Targeted run after Phase 2 (DeepSeek + GLM Coding Plan):

$ npx mocha "test/unit/core/domain/entities/provider-registry.test.ts" \
            "test/unit/agent/llm/providers/glm-coding-plan.test.ts" \
            "test/unit/agent/llm/providers/deepseek.test.ts" \
            "test/unit/agent/llm/model-capabilities.test.ts"
  51 passing (9ms)

Checklist

  • Tests added or updated and passing (npm test — 7316 passing)
  • Lint passes (npm run lint — 0 errors)
  • Type check passes (npm run typecheck — root + webui clean)
  • Build succeeds (npm run build)
  • Commits follow Conventional Commits format (feat: prefix)
  • Documentation updated (README provider table)
  • No breaking changes (capability fix is a behavior change, but the old code path never matched any real response — see Root cause above)
  • Branch is up to date with main

Risks and mitigations

  • Risk: the hardcoded model list for glm-coding-plan includes glm-5-turbo.
    • Mitigation: if the model 404s, drop it from the array in provider-model-fetcher-registry.ts. Smoke test with a subscription holder is documented as a follow-up in plan/llm-providers/IMPLEMENTATION.md §2.3.
  • Risk: the DeepSeek capability fix changes parsed output for any user who already had deepseek-reasoner accessible via OpenRouter (or any other route that flows through getModelCapabilities).
    • Mitigation: the prior think-tags mapping never matched a real DeepSeek response, so users were not seeing reasoning anyway — this is a fix, not a regression. OpenRouter has its own capability detection path that is unaffected.
  • Risk: standard glm users on the Z.AI Coding Plan subscription might not realize they need to switch to the new glm-coding-plan provider to consume their subscription quota.
    • Mitigation: distinct names in the picker (GLM (Z.AI) vs GLM Coding Plan (Z.AI)) and the description string call out the subscription. README table makes both explicit.

…g detection

Adds two LLM providers to bring byterover-cli to opencode parity for the
four user-requested integrations (DeepSeek, MiniMax, Moonshot/Kimi, Z.AI):

- DeepSeek (deepseek-chat, deepseek-reasoner) via @ai-sdk/openai-compatible
  against https://api.deepseek.com/v1, env DEEPSEEK_API_KEY.
- GLM Coding Plan as a separate provider ID (glm-coding-plan) pointing at
  the Z.AI Coding Plan subscription endpoint (api.z.ai/api/coding/paas/v4),
  reusing ZHIPU_API_KEY. Coexists with the existing 'glm' provider so
  standard pay-per-token users aren't silently switched onto a subscription.

Bug fix: deepseek-r1/deepseek-reasoner were mapped to reasoningFormat
'think-tags' in model-capabilities.ts, but the DeepSeek OpenAI-compatible
API streams reasoning through the native 'reasoning_content' field — the
old path never matched, so reasoning was effectively broken. Now correctly
mapped to native-field with reasoningField: 'reasoning_content'.

MiniMax, Moonshot/Kimi, and standard GLM were already wired and work
unchanged. The Kimi Coding Plan subscription tier uses the same Moonshot
endpoint, so no new provider entry is needed for it (matches opencode's
behavior — opencode also has no kimi-coding-plan provider).

Plan: plan/llm-providers/{DESIGN,IMPLEMENTATION}.md
TDD-authored; 21 new test assertions across registry, capability, and
provider-module shape. Full suite: 7316 passing, 16 pending. 0 lint errors.

Icon asset for DeepSeek sourced from Wikimedia Commons (MIT-licensed),
whale-only path with cropped viewBox. GLM Coding Plan reuses the existing
zai icon.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Claude finished @ngduyanhece's task in 3m 16s —— View job


PR Review: feat: DeepSeek + GLM Coding Plan providers, fix DeepSeek reasoning

  • Gather context and read changed files
  • Review new provider implementations (DeepSeek, GLM Coding Plan)
  • Review model-capabilities fix
  • Review provider registry changes
  • Review tests
  • Post final review with inline comments and overall score

Summary

Overall score: 4 / 5

This is a well-executed, well-scoped PR. The TDD sequence is evidenced clearly, the bug fix rationale is sound, and the new providers follow the established module pattern precisely. No security issues found. The issues below are all minor maintainability and documentation concerns.


What's Good

  • Capability fix is correct. deepseek-r1/deepseek-reasonernative-field/reasoning_content instead of think-tags is the right call. The old mapping never matched a real API response, so this is a pure fix with no regression risk.
  • Provider module shape is consistent. Both deepseek.ts and glm-coding-plan.ts match the pattern of existing providers (e.g. glm.ts, moonshot.ts) exactly: same field order, same createOpenAICompatible + AiSdkContentGenerator wiring.
  • glm-coding-plan regression guard is solid. The test that asserts getProviderById('glm') still resolves and that the two providers have distinct base URLs (both in provider-registry.test.ts and glm-coding-plan.test.ts) is a good regression check.
  • Icon handling is clean. Reusing the zai icon for glm-coding-plan in provider-icons.ts rather than adding a near-identical duplicate is the right call.
  • Scope boundary is respected. MiniMax, Moonshot, and standard glm are untouched. Deferred work is documented in DESIGN.md/IMPLEMENTATION.md rather than left as code comments.
  • TDD followed. Failing-tests-first evidence is in the PR description; 7316 passing after.

Issues (inline comments posted)

File Line Type Summary
deepseek.ts 24 nitpick Base URL hardcoded twice — divergence risk if one is updated without the other
glm-coding-plan.ts 23 nitpick Same URL duplication
glm-coding-plan.ts 36 nitpick priority: 17.5 — only float priority in the registry; consider shifting integers
model-capabilities.ts 139 question id.includes('r1') matches distilled variants (deepseek-r1-distill-*); worth a test to pin intent
provider-model-fetcher-registry.ts 91 nitpick Coding-plan URL hardcoded independently of PROVIDER_REGISTRY['glm-coding-plan'].baseUrl
provider-model-fetcher-registry.ts 93 suggestion glm-5-turbo is acknowledged as potentially invalid but has no inline comment at the call site
test/unit/.../model-capabilities.test.ts 25 suggestion Add a test for deepseek-r1-distill-llama-70b to document that the r1 substring match is intentional for the whole distilled family

None of these block merging — the most actionable before merge are the glm-5-turbo comment and the distilled-model test.

createGenerator(config: GeneratorFactoryConfig) {
const provider = createOpenAICompatible({
apiKey: config.apiKey!,
baseURL: 'https://api.deepseek.com/v1',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: The base URL is hardcoded here a second time, duplicating line 19. If the URL ever changes, both spots must be updated in sync. Other providers (e.g. deepinfra, togetherai) use the same pattern, so this is consistent with the codebase — but it's worth extracting to a module-level constant so there's a single source of truth:

Suggested change
baseURL: 'https://api.deepseek.com/v1',
baseURL: 'https://api.deepseek.com/v1',

(No change needed to the suggestion text itself, but consider a shared constant like DEEPSEEK_BASE_URL at the top of the file.)

createGenerator(config: GeneratorFactoryConfig) {
const provider = createOpenAICompatible({
apiKey: config.apiKey!,
baseURL: 'https://api.z.ai/api/coding/paas/v4',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Same URL duplication as in deepseek.tsbaseURL in createGenerator (here) repeats the value declared in the baseUrl module property (line 18). A module-level constant would prevent a silent drift if one is updated without the other.

// (OpenAI-compatible field), not <think> tags.
if (id.includes('deepseek')) {
// DeepSeek-R1 and reasoning models
if (id.includes('r1') || id.includes('reasoner')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: id.includes('r1') will also match distilled variants like deepseek-r1-distill-llama-70b and deepseek-r1-distill-qwen-32b. These models do emit reasoning_content when accessed directly, so the capability mapping is technically correct for direct DeepSeek API use. But users accessing them via OpenRouter may get a different response shape (OpenRouter normalises reasoning differently).

Is the intent that the model ID here always refers to the direct DeepSeek endpoint, so this is fine as-is? If OpenRouter-routed DeepSeek models reach this code path it could cause empty thinking blocks for distilled variants. A clarifying comment, or a test for deepseek-r1-distill-llama-70b, would make the intent explicit.

fetcher = new ChatBasedModelFetcher(
'https://api.z.ai/api/coding/paas/v4',
'GLM Coding Plan (Z.AI)',
['glm-4.7', 'glm-4.7-flash', 'glm-4.7-flashx', 'glm-5-turbo', 'glm-4.5', 'glm-4.5-flash'],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: The PR description calls out glm-5-turbo as potentially returning 404, but there's no inline comment here to capture that. Since this is a static list that a subscriber would scroll through and select, a // unverified — may 404 on non-eligible subscriptions note would surface the known risk at the call site rather than requiring readers to check the PR description or IMPLEMENTATION.md:

Suggested change
['glm-4.7', 'glm-4.7-flash', 'glm-4.7-flashx', 'glm-5-turbo', 'glm-4.5', 'glm-4.5-flash'],
['glm-4.7', 'glm-4.7-flash', 'glm-4.7-flashx', 'glm-5-turbo' /* unverified, may 404 on non-eligible subscriptions */, 'glm-4.5', 'glm-4.5-flash'],


case 'glm-coding-plan': {
fetcher = new ChatBasedModelFetcher(
'https://api.z.ai/api/coding/paas/v4',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: For providers handled via OpenAICompatibleModelFetcher (e.g. deepseek at line 65), the base URL is sourced from PROVIDER_REGISTRY[providerId].baseUrl, giving a single source of truth. Here, the coding-plan URL is hardcoded again independently of PROVIDER_REGISTRY['glm-coding-plan'].baseUrl. The glm case above has the same pre-existing pattern, but it's worth noting so that a future URL change doesn't have to be applied in three places.

envVars: ['ZHIPU_API_KEY'],
id: 'glm-coding-plan',
name: 'GLM Coding Plan (Z.AI)',
priority: 17.5,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: priority: 17.5 works because priority is typed as number, but every other provider uses an integer. Inserting a float between 17 and 18 makes the priority table harder to audit at a glance. If glm-coding-plan must appear after glm, consider shifting existing priorities (e.g. glm: 17, glm-coding-plan: 18, moonshot: 19, deepseek: 20) to keep the list integer-only.

expect(caps.reasoning).to.equal(false)
expect(caps.reasoningFormat).to.equal('none')
})
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider adding a test for a distilled-R1 model ID to make the matching intent explicit:

  it('reports native reasoning_content for deepseek-r1-distill variants', () => {
    const caps = getModelCapabilities('deepseek-r1-distill-llama-70b')
    expect(caps.reasoning).to.equal(true)
    expect(caps.reasoningField).to.equal('reasoning_content')
    expect(caps.reasoningFormat).to.equal('native-field')
  })

This pins the expected behaviour for distilled models and makes it obvious the r1 substring match is intentional for this whole family, not just deepseek-r1 proper.

@cuongdo-byterover
Copy link
Copy Markdown
Collaborator

image (UI): We should take a black-and-white version for consistent with the other providers

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