Skip to content

fix(provider): honor bare DeepSeek OpenAI base URLs - #2045

Closed
jatmn wants to merge 1 commit into
Twigpine:mainfrom
jatmn:fix-2036-custom-openai-base-url
Closed

jatmn wants to merge 1 commit into
Twigpine:mainfrom
jatmn:fix-2036-custom-openai-base-url

Conversation

@jatmn

@jatmn jatmn commented Jul 26, 2026 •

Copy link
Copy Markdown
Collaborator

cancled.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The changes add DeepSeek detection for OpenAI-compatible environment configuration, normalize bare provider URLs, select DeepSeek defaults, route requests through the OpenAI shim, and expand startup and routing test coverage.

Changes

DeepSeek routing and defaults

Layer / File(s) Summary
Route resolution and environment normalization
src/integrations/routeMetadata.ts, src/integrations/routeMetadata.test.ts
Base URL selection now trims values, ignores literal "undefined", falls back to OPENAI_API_BASE, recognizes bare DeepSeek origins, and preserves Cloudflare/LongCat boundary checks.
DeepSeek request normalization
src/services/api/providerConfig.ts, src/services/api/providerConfig.test.ts
Bare DeepSeek URLs are normalized to the API base and receive the route default model, while explicit ports and native Anthropic routes retain their expected handling.
Runtime shim and model defaults
src/services/api/client.ts, src/utils/model/model.ts, src/utils/model/model.openai-shim-providers.test.ts
DeepSeek routes use the OpenAI-compatible Anthropic shim and resolve route-specific default models when OPENAI_MODEL is unset.
Startup provider detection
src/components/StartupScreen.ts, src/components/StartupScreen.test.ts
Startup detection uses normalized OpenAI configuration, recognizes DeepSeek routing, resolves route-aware provider details, and tests legacy variable precedence and negative cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested labels: enhancement

Suggested reviewers: kevincodex1, 0xfandom, chioarub

🚥 Pre-merge checks | ✅ 4 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
No Hidden Policy Change ⚠️ Warning FAIL: model.ts now applies route-default fallbacks to all OpenAI-compatible routes, not just DeepSeek, so this PR changes third-party provider policy without explicit alignment. Scope the new fallback to routeId === 'deepseek' and keep existing gpt-4o/gpt-4o-mini defaults for other routes, or get maintainer signoff for broader provider-default policy.
Description check ⚠️ Warning The description is off-topic and does not follow the required summary, impact, testing, or notes template. Replace it with the repository template and fill in Summary, Impact, Testing, and Notes sections with the PR details.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes appear to address #2036 by honoring bare DeepSeek-compatible base URLs, selecting compatible defaults, and routing through the OpenAI shim.
Out of Scope Changes check ✅ Passed The diff stays focused on DeepSeek/OpenAI provider routing and regression tests, with no obvious unrelated changes.
Risk Surface Disclosed ✅ Passed Review comments flagged provider-routing risk and blocked on missing DeepSeek regression coverage; another warned against breaking third-party providers.
Title check ✅ Passed The title is concise, scoped, and accurately reflects the DeepSeek/OpenAI base URL change set.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@jatmn
jatmn force-pushed the fix-2036-custom-openai-base-url branch from 23d5f35 to 93f6a2e Compare July 26, 2026 20:57
@jatmn jatmn closed this Jul 26, 2026

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/services/api/client.ts`:
- Line 642: Update the DeepSeek shim-selection coverage in client.test.ts to
verify that env-only configuration reaches createOpenAIShimClient for a
bare-origin endpoint and legacy OPENAI_API_BASE, and add a negative case
confirming non-DeepSeek providers do not select the shim. Keep the tests focused
on the provider/model path governed by useDeepSeekOpenAIShim.

In `@src/utils/model/model.ts`:
- Around line 62-65: Restrict route-default substitution in model resolution to
the DeepSeek case: in src/utils/model/model.ts lines 62-65, call
getRouteDefaultModel only when routeId is 'deepseek' and otherwise preserve the
existing fallback. In src/components/StartupScreen.ts lines 130-134, apply a
route default only for DeepSeek and retain gpt-4o for other OpenAI-compatible
routes unless a model is explicitly configured.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3f1b5865-58a9-4c75-98bf-814f34af2401

📥 Commits

Reviewing files that changed from the base of the PR and between a3dc345 and 93f6a2e.

📒 Files selected for processing (9)
  • src/components/StartupScreen.test.ts
  • src/components/StartupScreen.ts
  • src/integrations/routeMetadata.test.ts
  • src/integrations/routeMetadata.ts
  • src/services/api/client.ts
  • src/services/api/providerConfig.test.ts
  • src/services/api/providerConfig.ts
  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/model/model.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: smoke-and-tests (24.11.x)
  • GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, including bun run typecheck and, when applicable, bun run typecheck:type-tests.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/integrations/routeMetadata.test.ts
  • src/services/api/providerConfig.test.ts
  • src/components/StartupScreen.test.ts
  • src/services/api/client.ts
  • src/integrations/routeMetadata.ts
  • src/components/StartupScreen.ts
  • src/utils/model/model.ts
  • src/services/api/providerConfig.ts
**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns in docs/integrations/overview.md and the focused guides under docs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Use bun install to install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/integrations/routeMetadata.test.ts
  • src/services/api/providerConfig.test.ts
  • src/components/StartupScreen.test.ts
  • src/services/api/client.ts
  • src/integrations/routeMetadata.ts
  • src/components/StartupScreen.ts
  • src/utils/model/model.ts
  • src/services/api/providerConfig.ts

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/integrations/routeMetadata.test.ts
  • src/services/api/providerConfig.test.ts
  • src/components/StartupScreen.test.ts
  • src/services/api/client.ts
  • src/integrations/routeMetadata.ts
  • src/components/StartupScreen.ts
  • src/utils/model/model.ts
  • src/services/api/providerConfig.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/integrations/routeMetadata.test.ts
  • src/services/api/providerConfig.test.ts
  • src/services/api/client.ts
  • src/integrations/routeMetadata.ts
  • src/utils/model/model.ts
  • src/services/api/providerConfig.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/integrations/routeMetadata.test.ts
  • src/services/api/providerConfig.test.ts
  • src/components/StartupScreen.test.ts
🔇 Additional comments (8)
src/integrations/routeMetadata.ts (1)

241-253: LGTM!

Also applies to: 1012-1055, 1153-1153, 1218-1228

src/integrations/routeMetadata.test.ts (1)

602-628: LGTM!

src/services/api/providerConfig.ts (1)

27-30: LGTM!

Also applies to: 76-110, 1017-1019, 1031-1031, 1075-1077

src/services/api/providerConfig.test.ts (1)

5-36: LGTM!

src/services/api/client.ts (1)

50-50: LGTM!

src/utils/model/model.openai-shim-providers.test.ts (1)

352-373: LGTM!

src/components/StartupScreen.ts (1)

10-13: LGTM!

Also applies to: 33-43, 95-105, 135-162

src/components/StartupScreen.test.ts (1)

46-48: LGTM!

Also applies to: 192-242

useNearaiEnvOnlyProvider ||
useFireworksEnvOnlyProvider ||
useAimlapiEnvOnlyProvider ||
useDeepSeekOpenAIShim ||

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add a focused DeepSeek shim-selection regression test.

This new branch controls the request transport, but this diff adds no src/services/api/client.test.ts coverage proving the documented env-only DeepSeek configuration reaches createOpenAIShimClient. Cover the bare origin, legacy OPENAI_API_BASE, and a non-DeepSeek negative case.

As per coding guidelines, “When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.” As per path instructions, “Block when risky runtime changes lack focused regression coverage.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/api/client.ts` at line 642, Update the DeepSeek shim-selection
coverage in client.test.ts to verify that env-only configuration reaches
createOpenAIShimClient for a bare-origin endpoint and legacy OPENAI_API_BASE,
and add a negative case confirming non-DeepSeek providers do not select the
shim. Keep the tests focused on the provider/model path governed by
useDeepSeekOpenAIShim.

Sources: Coding guidelines, Path instructions

Comment thread src/utils/model/model.ts
Comment on lines +62 to +65
const routeId = resolveActiveRouteIdFromEnv(process.env)
if (routeId && routeId !== 'openai' && routeId !== 'custom') {
return getRouteDefaultModel(routeId) ?? fallback
}

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restrict route-default substitution to the DeepSeek case.

These changes replace the legacy OpenAI fallbacks for every recognized route. For example, an explicit OpenAI-compatible MiniMax or OpenRouter endpoint with no OPENAI_MODEL now receives that route’s catalog default rather than the prior gpt-4o/gpt-4o-mini fallback. The PR scope only establishes DeepSeek’s env-only default and requires preserving legacy precedence.

  • src/utils/model/model.ts#L62-L65: use getRouteDefaultModel only when routeId === 'deepseek'; otherwise return the existing fallback.
  • src/components/StartupScreen.ts#L130-L134: select a route default only for DeepSeek; retain gpt-4o for other OpenAI-compatible routes unless a model is explicitly configured.

As per coding guidelines, “When changing provider behavior, avoid breaking third-party providers.” As per path instructions, “keep changes tightly scoped to the provider-detection/model-routing issue.”

📍 Affects 2 files
  • src/utils/model/model.ts#L62-L65 (this comment)
  • src/components/StartupScreen.ts#L130-L134
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/model/model.ts` around lines 62 - 65, Restrict route-default
substitution in model resolution to the DeepSeek case: in
src/utils/model/model.ts lines 62-65, call getRouteDefaultModel only when
routeId is 'deepseek' and otherwise preserve the existing fallback. In
src/components/StartupScreen.ts lines 130-134, apply a route default only for
DeepSeek and retain gpt-4o for other OpenAI-compatible routes unless a model is
explicitly configured.

Sources: Coding guidelines, Path instructions

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.

1 participant