Skip to content

refactor(types): use canonical identifiers for default models#991

Open
WebMad wants to merge 4 commits into
Zoo-Code-Org:mainfrom
WebMad:feat/954-provider-default-model-identifiers
Open

refactor(types): use canonical identifiers for default models#991
WebMad wants to merge 4 commits into
Zoo-Code-Org:mainfrom
WebMad:feat/954-provider-default-model-identifiers

Conversation

@WebMad

@WebMad WebMad commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace raw provider-name switch cases in getProviderDefaultModelId() with the canonical providerIdentifiers registry
  • preserve all existing static, regional, custom/local, and fallback default-model behavior
  • add focused tests using TDD triangulation across representative provider categories

Closes #954

TDD

The new test first overrides the canonical OpenRouter identifier. Before the implementation change, the literal-based switch fails that case and returns the Anthropic fallback. Migrating the switch to providerIdentifiers makes it pass. Additional cases triangulate the behavior across static/internal, region-dependent, custom/local, and fallback providers.

Verification

  • cd packages/types && npx vitest run src/__tests__/provider-default-model.test.ts src/__tests__/provider-identifiers.test.ts — 17 passed
  • cd packages/types && npm test — 253 passed
  • cd packages/types && npm run check-types — passed
  • cd packages/types && npm run lint — passed
  • repository lint pre-commit hook — passed

Note

The repository-wide pre-push type check currently reports unrelated pre-existing errors involving MiniMax M3, Moonshot router typing, LiteLLM exports, and abandonSubtaskWithId. The affected @roo-code/types package type check passes.

Summary by CodeRabbit

  • Bug Fixes
    • Improved provider default-model selection to consistently recognize supported provider identifiers.
    • Preserved region-specific defaults (including China-dependent behavior) and maintained Anthropic fallback behavior.
    • Ensured custom/local selections (such as OpenAI/Ollama/LM Studio) no longer return an unintended automatic default.
  • Tests
    • Added new Vitest coverage for provider matching, regional defaults, custom/local cases, and Anthropic fallback scenarios.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3104196f-b757-4090-9bd4-8acf72a1e3e2

📥 Commits

Reviewing files that changed from the base of the PR and between b999600 and 64a0508.

📒 Files selected for processing (2)
  • packages/types/src/__tests__/provider-default-model.test.ts
  • packages/types/src/providers/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/types/src/providers/index.ts
  • packages/types/src/tests/provider-default-model.test.ts

📝 Walkthrough

Walkthrough

getProviderDefaultModelId now matches providers through canonical providerIdentifiers constants. Existing defaults, regional Zai behavior, custom/local empty results, and Anthropic fallback behavior are covered by focused Vitest tests.

Changes

Provider default resolution

Layer / File(s) Summary
Canonical provider switch
packages/types/src/providers/index.ts
The default-model switch uses canonical provider identifiers while preserving existing model defaults, regional Zai selection, shared empty defaults, and fallback behavior.
Default resolution tests
packages/types/src/__tests__/provider-default-model.test.ts
Tests cover static defaults, related provider categories, regional defaults, custom/local selections, and Anthropic fallbacks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Suggested labels: awaiting-review

Suggested reviewers: taltas

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: switching default-model logic to canonical provider identifiers.
Description check ✅ Passed The description covers the change, rationale, testing, and linked issue, with only some optional template sections omitted.
Linked Issues check ✅ Passed The code matches #954 by using canonical provider identifiers and preserving existing default-model and fallback behavior.
Out of Scope Changes check ✅ Passed The changes stay focused on default-model selection and its tests, with no obvious unrelated scope added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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.

🧹 Nitpick comments (1)
packages/types/src/__tests__/provider-default-model.test.ts (1)

33-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the international Zai branch.

The test only exercises { isChina: true }; add an assertion for { isChina: false } or omitted options against internationalZAiDefaultModelId so regressions in the other branch are detected.

Proposed test addition
 	import {
 		anthropicDefaultModelId,
 		getProviderDefaultModelId,
+		internationalZAiDefaultModelId,
 		mainlandZAiDefaultModelId,
 		openRouterDefaultModelId,
 		vscodeLlmDefaultModelId,
 	} from "../providers/index.js"

 	it("preserves region-dependent defaults", () => {
 		expect(getProviderDefaultModelId(providerIdentifiers.zai, { isChina: true })).toBe(mainlandZAiDefaultModelId)
+		expect(getProviderDefaultModelId(providerIdentifiers.zai)).toBe(internationalZAiDefaultModelId)
 	})
🤖 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 `@packages/types/src/__tests__/provider-default-model.test.ts` around lines 33
- 35, Extend the “preserves region-dependent defaults” test for
providerIdentifiers.zai with an assertion using { isChina: false } or omitted
options, and verify it returns internationalZAiDefaultModelId while retaining
the existing mainland assertion.
🤖 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.

Nitpick comments:
In `@packages/types/src/__tests__/provider-default-model.test.ts`:
- Around line 33-35: Extend the “preserves region-dependent defaults” test for
providerIdentifiers.zai with an assertion using { isChina: false } or omitted
options, and verify it returns internationalZAiDefaultModelId while retaining
the existing mainland assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bd59281-3da6-4484-abbb-6352934a72e6

📥 Commits

Reviewing files that changed from the base of the PR and between 7af1a8a and 87350ed.

📒 Files selected for processing (2)
  • packages/types/src/__tests__/provider-default-model.test.ts
  • packages/types/src/providers/index.ts

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Jul 23, 2026
@github-actions github-actions Bot added has-conflicts PR has merge conflicts with the base branch and removed awaiting-review PR changes are ready and waiting for maintainer re-review labels Jul 24, 2026
@WebMad
WebMad force-pushed the feat/954-provider-default-model-identifiers branch from b999600 to 64a0508 Compare July 24, 2026 21:25
@github-actions github-actions Bot added awaiting-review PR changes are ready and waiting for maintainer re-review and removed has-conflicts PR has merge conflicts with the base branch labels Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR changes are ready and waiting for maintainer re-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Migrate provider default-model selection to canonical identifiers

1 participant