feat(config): collapse ProviderKind/ApiProvider dual enums behind Provider trait

[sximelon]

Summary

Introduce a Provider trait and 18 concrete provider structs in a new crates/config/src/provider.rs module, replacing the scattered match-arm duplication that every provider addition currently requires.

What's in this PR

• Provider trait — id, display_name, default_base_url, env_vars, default_model, wire, provider_config_key, thinking_supported, cache_telemetry_supported, apply_reasoning_effort.
• 18 zero-sized provider structs: Deepseek, NvidiaNim, Openai, Atlascloud, WanjieArk, Volcengine, Openrouter, XiaomiMimo, Novita, Fireworks, Siliconflow, Arcee, SiliconflowCn, Moonshot, Sglang, Vllm, Ollama, Huggingface.
• Global PROVIDER_REGISTRY — lookup_provider(), resolve_provider() (with alias normalization), default_provider(), all_providers().
• ProviderKind::provider() — bridge method that resolves a ProviderKind enum variant to its &'static dyn Provider trait object via the registry.

What's NOT in this PR (intentionally deferred)

This is the foundation. The existing ProviderKind enum, ApiProvider enum, and all their match arms in lib.rs, tui/src/config.rs, cli/src/lib.rs, and tui/src/client.rs are not yet migrated.
They remain the source of truth for the runtime.
The trait and registry are additive — callers can start migrating piecemeal in follow-up PRs.

Closes #2075

Testing

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features
• cargo test --workspace --all-features

Checklist

• Updated docs or comments as needed
• Added or updated tests where relevant
• Verified TUI behavior manually if UI changes

Greptile Summary

This PR centralizes provider metadata into a new Provider trait with 18 concrete zero-sized struct implementations in crates/config/src/provider.rs, and adds a ProviderKind::provider() bridge method so the existing enums can delegate to trait objects without a full enum migration.

• New provider.rs module: defines WireFormat, the Provider trait (id, display name, base URL, env vars, default model, wire format, config key, reasoning-effort application), 18 concrete structs, a OnceLock-backed PROVIDER_REGISTRY, and public helpers resolve_provider, lookup_provider, all_providers, and default_provider.
• ProviderKind::provider() bridge (lib.rs): resolves the enum variant to a &'static dyn Provider by calling resolve_provider(self.as_str()), falling back to default_provider() (DeepSeek) if the ID is unrecognised.

Confidence Score: 4/5

Mostly safe to merge; the new trait layer is correct for all current providers, but the SiliconflowCN config-routing disconnect from prior rounds remains unresolved.

The new trait and registry are internally consistent for all 18 providers. The unresolved SiliconflowCN mismatch means callers using provider_config_key() for TOML lookups will read the wrong config section for that provider. New findings here are minor cleanup items.

crates/config/src/lib.rs and crates/config/src/provider.rs — the SiliconflowCN provider_config_key vs for_provider mismatch needs resolution before the new trait API is used for config lookups.

Important Files Changed

Filename	Overview
crates/config/src/provider.rs	New module introducing the Provider trait and 18 concrete zero-sized struct implementations, plus a global OnceLock-backed registry with resolve_provider and all_providers helpers. One dead match arm ("siliconflow-CN" after lowercase normalization) is the only new finding here.
crates/config/src/lib.rs	Adds pub mod provider and a ProviderKind::provider() bridge method. The unwrap_or_else(default_provider) fallback creates a silent-failure risk for future variants not registered in resolve_provider.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ProviderKind variant] -->|".as_str()"| B["&'static str ID"]
    B -->|"resolve_provider(id)"| C{ID in registry?}
    C -->|Yes| D["&'static dyn Provider"]
    C -->|No| E["default_provider() → &Deepseek"]
    D --> F["provider_config_key()"]
    D --> G["default_base_url()"]
    D --> H["env_vars()"]
    D --> I["apply_reasoning_effort()"]
    F -->|"used by TUI"| J["[providers.key] TOML lookup"]
    J -.->|"SiliconflowCN: key=siliconflow_cn but reads self.siliconflow"| K["⚠ Config mismatch (prior rounds)"]

Loading

Comments Outside Diff (6)

1. crates/config/src/provider.rs, line 898-921 (link)

   Alias mapping maintained in three separate places

   The canonical-alias table now lives in ProviderKind::parse, the serde #[serde(alias = …)] attributes on each enum variant, and resolve_provider here — three independent copies of the same information. If a new alias is added in one place it must be remembered in the other two. A single source of truth (e.g., a const ALIASES: &[(&str, &str)] array consumed by all three sites) would prevent the lists from drifting.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

   

2. crates/cli/src/lib.rs, line 1473-1493 (link)

   ProviderKind::DeepseekCN missing from TUI provider whitelist

   Before this PR, "deepseek-cn" parsed to ProviderKind::Deepseek, which passes the matches! guard. After this PR, it parses to the new ProviderKind::DeepseekCN variant, which is absent from the guard. Any user whose config file contains provider = "deepseek-cn" — or who passes --provider deepseek-cn — will hit the bail and be completely unable to launch the interactive TUI after upgrading.

   

3. crates/cli/src/lib.rs, line 788-790 (link)

   auth login deepseek-cn silently fails to refresh the root api_key after this PR. Before the PR "deepseek-cn" parsed to ProviderKind::Deepseek, so this branch ran and updated store.config.api_key. Now it parses to ProviderKind::DeepseekCN and misses the branch entirely — the new credential is written only to providers.deepseek_cn.api_key. Because the TUI's deepseek_api_key checks self.api_key (root) before the provider-scoped entry, any pre-existing root key silently shadows the newly saved one, making the login effectively a no-op for those users.

   

4. crates/cli/src/lib.rs, line 804-809 (link)

   auth logout deepseek-cn no longer clears the root api_key after this PR's enum split. If a user previously ran auth login deepseek (which sets api_key at the root), switching to deepseek-cn and later logging out leaves the root key intact. The TUI checks the root api_key first for DeepseekCN, so the credential persists after logout.

   

5. crates/tui/src/config.rs, line 4863-4869 (link)

   provider_config_key returns wrong TOML section for SiliconflowCn after delegation

   ApiProvider::SiliconflowCn.provider().provider_config_key() now resolves through ProviderKind::SiliconflowCN → &provider::Siliconflow → "siliconflow". The old explicit arm returned "siliconflow-CN". Because save_provider_auth_mode_for calls this function to determine the TOML key it writes to, any auth-mode entry it creates for SiliconflowCn will now land in [providers.siliconflow] instead of [providers.siliconflow-CN], diverging from any existing config that users saved under the old key name.

   

6. crates/config/src/lib.rs, line 302-305 (link)

   siliconflow_cn config field is never read

   ProvidersToml gains a new pub siliconflow_cn: ProviderConfigToml field that serde will happily populate from [providers.siliconflow_cn], but for_provider(ProviderKind::SiliconflowCN) still returns &self.siliconflow. Any user who follows the new docs row — which lists [providers.siliconflow_cn] as the config table for siliconflow-cn — and sets api_key, base_url, or model there will have those values silently discarded at runtime; the CLI reads from [providers.siliconflow] instead. Either change for_provider(SiliconflowCN) to return &self.siliconflow_cn (making the field live and the docs accurate) or remove the dead field and update the docs to show [providers.siliconflow].

   

_{Reviews (13): Last reviewed commit: "feat(config): collapse ProviderKind/ApiP..." | Re-trigger Greptile}

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[Hmbown]

Reviewed for the v0.8.50 triage branch. I’m keeping this open rather than harvesting it today.

The provider-trait direction fits CodeWhale, but the current branch still has release-risk blockers from the code review: deepseek-cn splits into ProviderKind::DeepseekCN but is missing from the CLI/TUI provider whitelist and logout provider list, so an upgraded user with provider = "deepseek-cn" could fail to launch, and auth logout can leave providers.deepseek_cn.api_key behind. Windows CI is also red.

Once those DeepSeek-CN compatibility paths are fixed with focused tests, this becomes a much better candidate for a feature branch or the next minor slice. DeepSeek support itself remains first-class; the blocker here is the migration/regression risk, not the direction.

[sximelon]

Thanks for the review. I've addressed the blockers:

1. auth logout leaving deepseek_cn.api_key behind: restored DeepseekCN to PROVIDER_LIST so auth logout iterates over all 16 providers and clears [providers.deepseek_cn] via for_provider_mut(DeepseekCN).

2. CLI whitelist: provider_slot(DeepseekCN) returns "deepseek" — keyring credentials from auth login deepseek-cn share the same slot, no migration break. PROVIDER_LIST includes DeepseekCN so auth status/auth logout cover it.

3. TUI whitelist: ApiProvider::parse("deepseek-cn") and ProviderKind::parse("deepseek-cn") both accept the legacy alias. The provider picker intentionally excludes it (it's an alias, not a separate provider), but config file and --provider flag work.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[Hmbown]

Thanks @sximelon. I reviewed this for the v0.9.0 stewardship branch. The provider trait/registry shape is a useful direction, so if we harvest that architecture we should credit #2479.

I am not going to merge this as-is into the release train. The branch is green, but it is still additive rather than collapsing the duplicated provider surfaces required by #2075: ProviderKind, the TUI ApiProvider, runtime lookup/request shaping, config keys, and docs still remain separate sources of truth. There are also release-risk details to fix first: registry misses should not silently fall back to DeepSeek, siliconflow_cn needs to align with the current shared [providers.siliconflow] config surface, and deepseek-cn / DeepseekCN parity needs to be explicit across config and TUI.

A safer harvest would take the trait shape, WireFormat, zero-sized provider structs, registry helpers, and provider-specific reasoning-effort centralization, then add parity checks before it becomes load-bearing. Please leave #2075 open until the migration actually removes the duplicated provider tables.

[Hmbown]

Thanks again @sximelon. I harvested the release-safe v0.9 provider-foundation slice into #2820, now merged into codex/v0.9.0-stewardship.

What landed is intentionally metadata-only: codewhale-config now has a built-in provider registry with canonical lookup, alias-aware resolution, provider defaults, config-table keys, and API-key env candidates. It preserves the current runtime behavior: no request-body mutation moved into config, no fallback-provider activation, no silent fallback to DeepSeek on registry misses, deepseek-cn remains a legacy alias to DeepSeek, and siliconflow-CN still uses the shared [providers.siliconflow] table.

Your credit is preserved in the commit co-author trailer, #2820 body, changelog, and README v0.9 track credits. I am leaving this PR open rather than closing it because the broader ProviderKind/ApiProvider collapse from #2075 is still a larger design problem; this landed the safe foundation without pretending the whole duplicated provider surface is solved.

Verification on the harvest included:

• cargo fmt --all -- --check
• git diff --check
• ./scripts/release/check-versions.sh
• cmp -s CHANGELOG.md crates/tui/CHANGELOG.md && echo changelogs-match
• python3 scripts/check-provider-registry.py
• cargo test -p codewhale-config provider_ -- --nocapture
• cargo test -p codewhale-config -- --nocapture
• cargo check -p codewhale-config --locked

And #2762 is green again after the merge. Appreciate you pushing the provider unification direction forward.