fix: keep custom cloud provider as default#2142
Conversation
📝 WalkthroughWalkthroughThis PR updates cloud provider slug migration and runtime resolution to preserve custom provider choices across configuration migrations. When a legacy ChangesCloud Provider Resolution and Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/openhuman/config/schema/load.rs`:
- Around line 613-634: The current logic in legacy_custom_slug maps any
non-OpenHuman inference_url to the first non-OpenHuman provider via the or_else
fallback, which can incorrectly rewrite primary_cloud; modify the closure so it
only returns a slug when a provider's normalized endpoint actually matches the
normalized inference_url (i.e., remove the or_else fallback), keeping the use of
normalize_provider_endpoint, cloud_providers, is_openhuman_provider_entry,
inference_url and looks_like_openhuman_provider_endpoint to locate an exact
match before mapping to entry.slug.
In `@src/openhuman/providers/factory.rs`:
- Around line 278-282: is_openhuman_cloud_entry currently only checks slug and
AuthStyle but must also detect known hosted endpoints to match migration/load
heuristics; update the function (is_openhuman_cloud_entry) to return true if the
entry.slug == PROVIDER_OPENHUMAN or AuthStyle::OpenhumanJwt OR if the
entry.endpoint host matches hosted domains (e.g., equals or contains
"api.openhuman.ai" or ends_with ".tinyhumans.ai"); use the CloudProviderCreds
endpoint field (e.g., entry.endpoint.host or entry.endpoint.as_ref()) to check
host patterns alongside the existing slug/auth checks so hosted backend URLs are
routed through the backend/JWT flow.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26243dcb-b110-4c0c-965c-edb6b96796ab
📒 Files selected for processing (6)
src/openhuman/config/schema/load.rssrc/openhuman/config/schema/load_tests.rssrc/openhuman/migrations/unify_ai_provider_settings.rssrc/openhuman/migrations/unify_ai_provider_settings_tests.rssrc/openhuman/providers/factory.rssrc/openhuman/providers/factory_test.rs
| let legacy_custom_slug = config | ||
| .inference_url | ||
| .as_deref() | ||
| .map(str::trim) | ||
| .filter(|url| !url.is_empty() && !looks_like_openhuman_provider_endpoint(url)) | ||
| .and_then(|url| { | ||
| let normalized = normalize_provider_endpoint(url); | ||
| config | ||
| .cloud_providers | ||
| .iter() | ||
| .find(|entry| { | ||
| !is_openhuman_provider_entry(entry) | ||
| && normalize_provider_endpoint(&entry.endpoint) == normalized | ||
| }) | ||
| .or_else(|| { | ||
| config | ||
| .cloud_providers | ||
| .iter() | ||
| .find(|entry| !is_openhuman_provider_entry(entry)) | ||
| }) | ||
| .map(|entry| entry.slug.clone()) | ||
| }); |
There was a problem hiding this comment.
Only preserve legacy custom routing on an actual endpoint match.
The fallback at Lines 627-632 turns any non-OpenHuman inference_url into the first non-OpenHuman slug, even when that URL does not match a configured provider. If primary_cloud is still OpenHuman, Line 663 will then rewrite "cloud" to that unrelated provider and can persist the wrong route.
Suggested fix
let legacy_custom_slug = config
.inference_url
.as_deref()
.map(str::trim)
.filter(|url| !url.is_empty() && !looks_like_openhuman_provider_endpoint(url))
.and_then(|url| {
let normalized = normalize_provider_endpoint(url);
config
.cloud_providers
.iter()
.find(|entry| {
!is_openhuman_provider_entry(entry)
&& normalize_provider_endpoint(&entry.endpoint) == normalized
})
- .or_else(|| {
- config
- .cloud_providers
- .iter()
- .find(|entry| !is_openhuman_provider_entry(entry))
- })
.map(|entry| entry.slug.clone())
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let legacy_custom_slug = config | |
| .inference_url | |
| .as_deref() | |
| .map(str::trim) | |
| .filter(|url| !url.is_empty() && !looks_like_openhuman_provider_endpoint(url)) | |
| .and_then(|url| { | |
| let normalized = normalize_provider_endpoint(url); | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| { | |
| !is_openhuman_provider_entry(entry) | |
| && normalize_provider_endpoint(&entry.endpoint) == normalized | |
| }) | |
| .or_else(|| { | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| !is_openhuman_provider_entry(entry)) | |
| }) | |
| .map(|entry| entry.slug.clone()) | |
| }); | |
| let legacy_custom_slug = config | |
| .inference_url | |
| .as_deref() | |
| .map(str::trim) | |
| .filter(|url| !url.is_empty() && !looks_like_openhuman_provider_endpoint(url)) | |
| .and_then(|url| { | |
| let normalized = normalize_provider_endpoint(url); | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| { | |
| !is_openhuman_provider_entry(entry) | |
| && normalize_provider_endpoint(&entry.endpoint) == normalized | |
| }) | |
| .map(|entry| entry.slug.clone()) | |
| }); |
🤖 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/openhuman/config/schema/load.rs` around lines 613 - 634, The current
logic in legacy_custom_slug maps any non-OpenHuman inference_url to the first
non-OpenHuman provider via the or_else fallback, which can incorrectly rewrite
primary_cloud; modify the closure so it only returns a slug when a provider's
normalized endpoint actually matches the normalized inference_url (i.e., remove
the or_else fallback), keeping the use of normalize_provider_endpoint,
cloud_providers, is_openhuman_provider_entry, inference_url and
looks_like_openhuman_provider_endpoint to locate an exact match before mapping
to entry.slug.
| fn legacy_custom_inference_provider_string(config: &Config) -> Option<String> { | ||
| let inference_url = config | ||
| .inference_url | ||
| .as_deref() | ||
| .map(str::trim) | ||
| .filter(|url| !url.is_empty())?; | ||
|
|
||
| if looks_like_openhuman_backend(inference_url) { | ||
| return None; | ||
| } | ||
|
|
||
| let normalized_inference = normalize_endpoint_for_compare(inference_url); | ||
| config | ||
| .cloud_providers | ||
| .iter() | ||
| .find(|entry| { | ||
| !is_openhuman_cloud_entry(entry) | ||
| && normalize_endpoint_for_compare(&entry.endpoint) == normalized_inference | ||
| }) | ||
| .or_else(|| { | ||
| config | ||
| .cloud_providers | ||
| .iter() | ||
| .find(|entry| !is_openhuman_cloud_entry(entry)) | ||
| }) | ||
| .map(|entry| cloud_entry_provider_string(entry, config)) |
There was a problem hiding this comment.
Don't route unmatched legacy URLs to an arbitrary provider.
Lines 244-249 fall back to the first non-OpenHuman entry when inference_url doesn't match any configured endpoint. That makes empty/"cloud" roles send traffic to whichever provider happens to be first, instead of honoring primary_cloud or falling back to OpenHuman.
Suggested fix
config
.cloud_providers
.iter()
.find(|entry| {
!is_openhuman_cloud_entry(entry)
&& normalize_endpoint_for_compare(&entry.endpoint) == normalized_inference
})
- .or_else(|| {
- config
- .cloud_providers
- .iter()
- .find(|entry| !is_openhuman_cloud_entry(entry))
- })
.map(|entry| cloud_entry_provider_string(entry, config))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn legacy_custom_inference_provider_string(config: &Config) -> Option<String> { | |
| let inference_url = config | |
| .inference_url | |
| .as_deref() | |
| .map(str::trim) | |
| .filter(|url| !url.is_empty())?; | |
| if looks_like_openhuman_backend(inference_url) { | |
| return None; | |
| } | |
| let normalized_inference = normalize_endpoint_for_compare(inference_url); | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| { | |
| !is_openhuman_cloud_entry(entry) | |
| && normalize_endpoint_for_compare(&entry.endpoint) == normalized_inference | |
| }) | |
| .or_else(|| { | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| !is_openhuman_cloud_entry(entry)) | |
| }) | |
| .map(|entry| cloud_entry_provider_string(entry, config)) | |
| fn legacy_custom_inference_provider_string(config: &Config) -> Option<String> { | |
| let inference_url = config | |
| .inference_url | |
| .as_deref() | |
| .map(str::trim) | |
| .filter(|url| !url.is_empty())?; | |
| if looks_like_openhuman_backend(inference_url) { | |
| return None; | |
| } | |
| let normalized_inference = normalize_endpoint_for_compare(inference_url); | |
| config | |
| .cloud_providers | |
| .iter() | |
| .find(|entry| { | |
| !is_openhuman_cloud_entry(entry) | |
| && normalize_endpoint_for_compare(&entry.endpoint) == normalized_inference | |
| }) | |
| .map(|entry| cloud_entry_provider_string(entry, config)) | |
| } |
| fn is_openhuman_cloud_entry( | ||
| entry: &crate::openhuman::config::schema::cloud_providers::CloudProviderCreds, | ||
| ) -> bool { | ||
| entry.slug == PROVIDER_OPENHUMAN || matches!(entry.auth_style, AuthStyle::OpenhumanJwt) | ||
| } |
There was a problem hiding this comment.
Keep OpenHuman entry detection aligned with the migration/load heuristics.
is_openhuman_cloud_entry no longer considers the endpoint host, so an entry pointing at api.openhuman.ai or *.tinyhumans.ai but carrying stale slug/auth metadata is treated as custom here. That can push hosted-backend URLs through the OpenAI-compatible path instead of the backend/JWT flow this PR is trying to preserve.
Suggested fix
fn is_openhuman_cloud_entry(
entry: &crate::openhuman::config::schema::cloud_providers::CloudProviderCreds,
) -> bool {
- entry.slug == PROVIDER_OPENHUMAN || matches!(entry.auth_style, AuthStyle::OpenhumanJwt)
+ entry.slug == PROVIDER_OPENHUMAN
+ || matches!(entry.auth_style, AuthStyle::OpenhumanJwt)
+ || looks_like_openhuman_backend(&entry.endpoint)
}🤖 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/openhuman/providers/factory.rs` around lines 278 - 282,
is_openhuman_cloud_entry currently only checks slug and AuthStyle but must also
detect known hosted endpoints to match migration/load heuristics; update the
function (is_openhuman_cloud_entry) to return true if the entry.slug ==
PROVIDER_OPENHUMAN or AuthStyle::OpenhumanJwt OR if the entry.endpoint host
matches hosted domains (e.g., equals or contains "api.openhuman.ai" or ends_with
".tinyhumans.ai"); use the CloudProviderCreds endpoint field (e.g.,
entry.endpoint.host or entry.endpoint.as_ref()) to check host patterns alongside
the existing slug/auth checks so hosted backend URLs are routed through the
backend/JWT flow.
vaddisrinivas
force-pushed
the
fix/custom-provider-budget-exhausted
branch
from
f633b9a to
4d7eb35
Compare
vaddisrinivas
force-pushed
the
fix/custom-provider-budget-exhausted
branch
from
4d7eb35 to
fbc2178
Compare
|
Addressed CodeRabbit feedback in latest push fbc2178: removed unmatched custom-provider fallback in load/runtime resolution, added hosted OpenHuman/TinyHumans endpoint detection in the factory, and added regressions for unmatched legacy URLs plus hosted endpoint entries. Focused Rust tests and cargo check pass locally. |
|
CI note after latest check: the only red check is |
# Conflicts: # src/openhuman/config/schema/load_tests.rs # src/openhuman/inference/provider/factory.rs
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
2 participants
Summary
"cloud"/ unset workload routing so it resolves throughprimary_cloudinstead of hard-falling back to OpenHuman.inference_urlrouting for users whose config already contains a custom endpoint but still hasprimary_cloudpointing at OpenHuman."cloud"workload fields point at the legacy custom provider instead of freezing onto OpenHuman.Problem
"cloud"workloads as OpenHuman even when the config had a custom cloud provider seeded frominference_url.primary_cloudto the OpenHuman entry.Solution
"cloud"workload routes throughprimary_cloud.primary_cloudstill points at OpenHuman but a legacy externalinference_urlmatches a non-OpenHuman cloud provider, route through that custom provider for backward compatibility."openhuman"routes explicit so users can still force OpenHuman.Submission Checklist
## RelatedCloses #NNNin the## RelatedsectionImpact
inference_urlnow keep routing on the custom provider instead of consuming OpenHuman hosted budget.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— not run; Rust formatter command below covers touched files.pnpm typecheck— not run; no TypeScript changed.cargo test --manifest-path Cargo.toml openhuman::inference::provider::factory::factory_test --libcargo test --manifest-path Cargo.toml openhuman::migrations::unify_ai_provider_settings::tests --libcargo test --manifest-path Cargo.toml migrate_cloud_provider_slugs --libcargo fmt --manifest-path Cargo.toml --all --checkcargo check --manifest-path Cargo.toml -p openhumangit diff --checkValidation Blocked
command:node scripts/codex-pr-preflight.mjs --strict-path --lightweighterror:local worktree path is/Users/srinivasvaddi/Projects/openhuman-custom-provider-budget-fixinstead of/workspace/openhuman; branch prefixfix/custom-provider-budget-exhausteddoes not match the Codex branch convention.impact:required-file and remote checks passed; failures are local harness/path + branch naming policy only.Behavior Changes
"cloud"workloads resolve to the configured primary/custom provider instead of always OpenHuman.Parity Contract
"openhuman"still forces OpenHuman."cloud", role-based unset routing, legacyinference_url, load-time rewrite, and explicit OpenHuman override.Duplicate / Superseded PR Handling