Skip to content

fix(inference): forward raw BYOK model ids to provider construction instead of collapsing to a managed tier#4629

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/byok-model-passthrough-4598
Jul 7, 2026
Merged

fix(inference): forward raw BYOK model ids to provider construction instead of collapsing to a managed tier#4629
senamakel merged 2 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/byok-model-passthrough-4598

Conversation

@YellowSnnowmann

@YellowSnnowmann YellowSnnowmann commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Agent-node raw BYOK model overrides silently collapse to the chat model #4598: an agent/node pinning a raw BYOK model id (e.g. claude-opus-4) silently ran on reasoning-v1 because the provider factory collapsed every model string that wasn't a known managed tier.
  • make_openhuman_backend now forwards a non-empty, non-hint:, non-managed-tier model id verbatim to the OpenHuman backend (which is authoritative over its validity) instead of substituting the default.
  • The tinyflows simple-LLM path forwards the node's pinned raw model to provider construction via a new resolve_completion_model helper, rather than only the role's default model.
  • Managed-tier and hint:* resolution are unchanged. Added Rust unit coverage.

Problem

inference/provider/factory.rs::make_openhuman_backend collapsed every unrecognized model string to MODEL_REASONING_V1. A raw BYOK id set on an agent node's custom-model field therefore never reached provider construction — the node silently ran on the configured default rather than the model the user selected. Both the simple-LLM path (tinyflows/caps.rs, which mapped the node model to a role and discarded the raw id) and the harness path went through this helper. Flagged P2 by chatgpt-codex-connector on #4578.

Solution

  • Added is_raw_passthrough_model() — a non-empty string that is neither a hint:* alias nor a known managed tier.
  • In make_openhuman_backend, such ids are forwarded verbatim (with a debug log) instead of collapsing to reasoning-v1. The empty-string fallback is retained.
  • In caps.rs, resolve_completion_model(node_model, resolved_model) returns the pinned raw id when the node pinned one, else the role-resolved default; wired into OpenHumanLlm::complete after create_chat_provider.
  • The harness path already overrides the model via configured_agent_model and now benefits from the factory passthrough — no harness code change was required (verified against the code, not assumed).
  • The backend's resolve_model preserves non-empty ids verbatim, so the raw id reaches the wire and the backend validates it.

Intended semantic tradeoff (call out in review): a genuinely stale/garbage pinned id (e.g. a mistyped tier) now reaches the backend and returns a clear error instead of silently running on reasoning-v1. This is the correct #4598 behaviour — honour the user's explicit selection and fail loud rather than silently downgrade.

Submission Checklist

  • Tests added or updated (happy path + failure/edge case) — raw id forwarded verbatim; managed-tier + hint:* invariance; resolve_completion_model over raw / managed / hint / none / whitespace inputs.
  • Diff coverage ≥ 80% — changed lines covered by the new factory + caps unit tests.
  • Coverage matrix updated — N/A: behaviour-only bugfix, no feature row change.
  • All affected feature IDs listed under ## RelatedN/A: behaviour-only bugfix.
  • No new external network dependencies introduced.
  • Manual smoke checklist updated — N/A: does not touch release-cut surfaces.
  • Linked issue closed via Closes #4598.

Impact

CLI/desktop inference. Per-node BYOK model overrides are now honoured on agent nodes. A stale/invalid pinned id now surfaces a backend error rather than silently downgrading to a managed tier. No migration implications.

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/byok-model-passthrough-4598

Summary by CodeRabbit

  • New Features

    • Raw/BYOK model IDs are now preserved and forwarded as entered instead of being remapped to a default managed model.
    • Pinned model settings in agent completions now respect raw model IDs, while managed tiers and hint:* values keep their existing behavior.
  • Bug Fixes

    • Prevented silent fallback that could send users’ custom model IDs to the wrong backend model.

…nstead of collapsing to a managed tier

An agent/node pinning a raw model id (e.g. `claude-opus-4`) silently ran on
`reasoning-v1` because `make_openhuman_backend` collapsed every model string
that wasn't a known managed tier. Non-empty, non-`hint:` ids that aren't a
managed tier are now forwarded verbatim to the OpenHuman backend, which is
authoritative over their validity; the tinyflows simple-LLM path forwards the
node's pinned model rather than the role default via a new
`resolve_completion_model` helper. Managed-tier and `hint:*` resolution are
unchanged.

Closes tinyhumansai#4598
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80253b76-d656-443c-97e3-e612703bf891

📥 Commits

Reviewing files that changed from the base of the PR and between 2371f6a and aab56b6.

📒 Files selected for processing (2)
  • src/openhuman/tinyflows/caps.rs
  • src/openhuman/tinyflows/tests.rs
📝 Walkthrough

Walkthrough

Adds an is_raw_passthrough_model predicate to detect non-empty, non-hint, non-tier model ids as raw BYOK identifiers. Updates the managed OpenHuman backend to forward unrecognized model ids verbatim instead of collapsing them to reasoning-v1, re-exports the helper, and applies it in the tinyflows completion path to honor pinned BYOK model ids over tier-resolved defaults, with accompanying tests.

Changes

BYOK model passthrough

Layer / File(s) Summary
Passthrough predicate and backend model resolution
src/openhuman/inference/provider/factory.rs, src/openhuman/inference/provider/factory_tests.rs
Adds is_raw_passthrough_model and changes make_openhuman_backend to forward unrecognized non-hint model ids verbatim (debug log) instead of collapsing to reasoning-v1; tests updated to assert verbatim forwarding for BYOK ids like claude-opus-4 and deepseek-v4-pro.
Crate re-export of passthrough helper
src/openhuman/inference/provider/mod.rs
Re-exports factory::is_raw_passthrough_model as pub(crate).
Tinyflows completion model passthrough
src/openhuman/tinyflows/caps.rs
Adds resolve_completion_model to forward pinned raw/BYOK node model ids over tier-resolved defaults, applies it in OpenHumanLlm::complete, adjusts imports, and adds unit tests for passthrough and non-passthrough cases.

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

Possibly related issues

Possibly related PRs

  • tinyhumansai/openhuman#4433: Both PRs touch the OpenHumanLlm::complete path in caps.rs; this PR extends that adapter with BYOK passthrough handling.
  • tinyhumansai/openhuman#4446: Both PRs modify provider construction logic in factory.rs's create_chat_provider_from_string chokepoint.

Suggested labels: bug, rust-core

Suggested reviewers: M3gA-Mind

Poem

A rabbit hops through model strings so free,
"Pass it through raw, let BYOK be!"
No more collapsing to tier-v1's embrace,
Your chosen id now keeps its place.
Hop, hop, hooray — verbatim delight! 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: raw BYOK model IDs are now forwarded instead of being collapsed to a managed tier.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@YellowSnnowmann YellowSnnowmann marked this pull request as ready for review July 7, 2026 12:10
@YellowSnnowmann YellowSnnowmann requested a review from a team July 7, 2026 12:10
@coderabbitai coderabbitai Bot added bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Jul 7, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2371f6afd9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/openhuman/tinyflows/caps.rs

@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)
src/openhuman/inference/provider/factory.rs (1)

1232-1253: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Untrimmed model bypasses tier matching, causing whitespace-padded tiers to be treated as raw passthrough.

is_known_openhuman_tier(&model) here is checked against the raw (non-trimmed) model string, unlike is_raw_passthrough_model, which trims first. If config.default_model carries incidental whitespace (e.g. " reasoning-v1 "), it won't match is_known_openhuman_tier, so it falls into the else branch and is forwarded verbatim to the backend as an "unrecognized" raw id instead of being resolved to the known tier — a behavior regression versus the previous fallback-to-reasoning-v1 path for at least that edge case.

Consider reusing is_raw_passthrough_model here for consistent trimming semantics:

🔧 Suggested fix
-            if is_known_openhuman_tier(&model) {
-                model
-            } else {
+            if !is_raw_passthrough_model(&model) {
+                model
+            } else {
                 // Unrecognised NON-empty model id — a raw/BYOK model the user
🤖 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/inference/provider/factory.rs` around lines 1232 - 1253, The
tier check in the model-selection branch is using the raw `model` string, so
whitespace-padded known tiers can be misclassified as raw passthrough. Update
the `factory.rs` logic around `is_known_openhuman_tier` to use the same trimmed
semantics as `is_raw_passthrough_model` (or trim before matching) so values like
`default_model` with incidental spaces still resolve to canonical tiers instead
of falling into the raw/BYOK path.
🤖 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 `@src/openhuman/inference/provider/factory.rs`:
- Around line 1232-1253: The tier check in the model-selection branch is using
the raw `model` string, so whitespace-padded known tiers can be misclassified as
raw passthrough. Update the `factory.rs` logic around `is_known_openhuman_tier`
to use the same trimmed semantics as `is_raw_passthrough_model` (or trim before
matching) so values like `default_model` with incidental spaces still resolve to
canonical tiers instead of falling into the raw/BYOK path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e32d45c6-8b48-47c5-b20d-fd60f1051305

📥 Commits

Reviewing files that changed from the base of the PR and between 2ce5e60 and 2371f6a.

📒 Files selected for processing (4)
  • src/openhuman/inference/provider/factory.rs
  • src/openhuman/inference/provider/factory_tests.rs
  • src/openhuman/inference/provider/mod.rs
  • src/openhuman/tinyflows/caps.rs

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 7, 2026
…odes too

Addresses review feedback on tinyhumansai#4598: harness_model_default_override wrapped every
node model in hint:chat, collapsing a raw BYOK id (e.g. claude-opus-4) onto the
managed chat tier for agent nodes routed through run_via_harness. Raw passthrough
ids are now forwarded verbatim (guarded by is_raw_passthrough_model), matching
the simple OpenHumanLlm::complete path: they reach the harness chat role, inherit
config.default_model, and make_openhuman_backend forwards them to the backend.

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

Review

Bug fix is correct and well-tested. Fixes the silent collapse of raw/BYOK model ids onto reasoning-v1 at make_openhuman_backend and threads the same fix through the tinyflows simple-LLM path and the harness override. The semantic tradeoff (invalid pinned ids now surface a backend error rather than silent downgrade) is the right #4598 behaviour and is called out in the PR body.

Verified

  • Traced the backend wire boundary at src/openhuman/inference/provider/openhuman_backend.rs:32-50. resolve_model trims and substitutes for empty at every entry (chat/embed/complete, lines 150/164/176), and existing tests (resolve_model_trims_surrounding_whitespace, resolve_model_preserves_non_empty_value_verbatim) already cover the padded/custom-id shapes. So a padded " claude-opus-4 " forwarded verbatim from the factory is trimmed before the wire — no user-visible breakage.
  • Both call sites (OpenHumanLlm::complete and harness_model_default_override) use the same is_raw_passthrough_model predicate, so the simple-LLM lane and the harness lane stay symmetric.

Non-blocking follow-ups

  1. Stale doc comment in an unrelated filesrc/openhuman/inference/provider/openhuman_backend.rs:27-31 still reads "Mirrors the same fallback make_openhuman_backend already applies when default_model is missing" and points at factory.rs:404. That framing was accurate before this PR — the factory fell back for both empty and unrecognised ids. After this PR the two sites diverge: only resolve_model still substitutes (for empty), the factory now forwards unrecognised non-empty ids verbatim. Worth a one-line tightening so the next reader doesn't think the fallback still mirrors.
  2. Optional style tightening — the 6-line "model is guaranteed non-empty here" block inside make_openhuman_backend goes beyond the CLAUDE.md "one short line max" rule for inline comments. The rationale is genuinely non-obvious, but most of it duplicates the doc comment on is_raw_passthrough_model; a one-liner referencing that would suffice.
  3. Optional cross-reference — see inline comment on resolve_completion_model.

Verdict: approve. None of the above block merge.

/// role — in that case the pinned id, not the role default, is the model the
/// user selected, so it is forwarded verbatim (issue #4598). Managed tiers and
/// every `hint:*` alias fall through to `resolved_model` unchanged.
fn resolve_completion_model(node_model: Option<&str>, resolved_model: String) -> String {

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.

Nit: the sister function harness_model_default_override already cross-references this one ("Mirrors the per-node routing [OpenHumanLlm::complete] applies via [resolve_completion_model]"), but the reference is one-way. Adding a symmetric back-reference here ("mirrored on the harness path by [harness_model_default_override]") would keep future readers from touching only one lane and drifting the two apart.

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

LGTM. Fix is correct at both call sites (factory + tinyflows), backend handles the wire boundary cleanly, tests cover the raw/tier/hint/empty/whitespace matrix. The follow-ups I noted in my earlier review are all cosmetic — none block merge.

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

LGTM. Fix is correct at both call sites (factory + tinyflows), backend resolve_model handles the wire boundary cleanly, tests cover the raw/tier/hint/empty/whitespace matrix. The follow-ups I noted in my earlier review are all cosmetic — none block merge.

@senamakel senamakel merged commit 7f1e121 into tinyhumansai:main Jul 7, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Agent-node raw BYOK model overrides silently collapse to the chat model

3 participants