feat: allow inline model pinning for subagents#1896
Merged
senamakel merged 1 commit intoMay 17, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds per-spawn model pinning to subagent execution. A new ChangesSubagent Model Override
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/openhuman/agent/harness/subagent_runner/ops.rs (2)
115-120: ⚡ Quick winUse debug-level structured tracing for override-branch diagnostics.
This is verbose branch telemetry; keep it at debug/trace and structured fields for consistent grepability.
Suggested patch
- log::info!( - "[subagent_runner] agent_id={} using inline model override model={}", - agent_id, - model - ); + tracing::debug!( + agent_id = %agent_id, + model = %model, + "[subagent_runner] using inline model override" + );As per coding guidelines "
src/**/*.rs: Rust core code must use log / tracing atdebug/tracelevels for verbose diagnostics on new/changed flows...".🤖 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/agent/harness/subagent_runner/ops.rs` around lines 115 - 120, Current info-level log in the override branch is too verbose; change the log to a debug/trace structured call and keep the rest of the logic unchanged: replace the log::info! in the override branch (the site that logs agent_id and model before returning (parent_provider, model.to_string())) with a debug or trace-level structured log (e.g., tracing::debug! or log::debug!) emitting agent_id and model as named fields (agent_id=? or agent_id=%agent_id, model=%model) so diagnostics are lower-volume and grepable.
294-302: ⚡ Quick winSkip config load when a non-empty
model_overrideis already present.The override branch bypasses
ModelSpecresolution, butConfig::load_or_init()still runs unconditionally first. That adds avoidable I/O/latency for pinned spawns.Suggested patch
- let config_loaded = crate::openhuman::config::Config::load_or_init().await; + let model_override = options.model_override.as_deref(); + let config_loaded = if model_override + .map(str::trim) + .filter(|model| !model.is_empty()) + .is_some() + { + None + } else { + crate::openhuman::config::Config::load_or_init().await.ok() + }; let (subagent_provider, model) = resolve_subagent_provider( &definition.model, &definition.id, - config_loaded.as_ref().ok(), + config_loaded.as_ref(), parent.provider.clone(), parent.model_name.clone(), - options.model_override.as_deref(), + model_override, );🤖 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/agent/harness/subagent_runner/ops.rs` around lines 294 - 302, If options.model_override is present and non-empty, avoid the unnecessary Config::load_or_init() call by short-circuiting and passing None for the config parameter to resolve_subagent_provider; i.e., check options.model_override.as_deref().map(|s| !s.is_empty()).unwrap_or(false) and only call crate::openhuman::config::Config::load_or_init().await when that check is false, then pass the resulting config as before (config_loaded.as_ref().ok()) or None when skipped so resolve_subagent_provider(&definition.model, &definition.id, /*config*/ , parent.provider.clone(), parent.model_name.clone(), options.model_override.as_deref()) receives no config when a model_override is provided.
🤖 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/agent/harness/subagent_runner/ops.rs`:
- Around line 115-120: Current info-level log in the override branch is too
verbose; change the log to a debug/trace structured call and keep the rest of
the logic unchanged: replace the log::info! in the override branch (the site
that logs agent_id and model before returning (parent_provider,
model.to_string())) with a debug or trace-level structured log (e.g.,
tracing::debug! or log::debug!) emitting agent_id and model as named fields
(agent_id=? or agent_id=%agent_id, model=%model) so diagnostics are lower-volume
and grepable.
- Around line 294-302: If options.model_override is present and non-empty, avoid
the unnecessary Config::load_or_init() call by short-circuiting and passing None
for the config parameter to resolve_subagent_provider; i.e., check
options.model_override.as_deref().map(|s| !s.is_empty()).unwrap_or(false) and
only call crate::openhuman::config::Config::load_or_init().await when that check
is false, then pass the resulting config as before (config_loaded.as_ref().ok())
or None when skipped so resolve_subagent_provider(&definition.model,
&definition.id, /*config*/ , parent.provider.clone(), parent.model_name.clone(),
options.model_override.as_deref()) receives no config when a model_override is
provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 282fb8cb-13bc-49ee-9c78-d05fa4f6662a
📒 Files selected for processing (9)
src/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/subagent_runner/ops_tests.rssrc/openhuman/agent/harness/subagent_runner/types.rssrc/openhuman/tools/impl/agent/archetype_delegation.rssrc/openhuman/tools/impl/agent/dispatch.rssrc/openhuman/tools/impl/agent/skill_delegation.rssrc/openhuman/tools/impl/agent/spawn_parallel_agents.rssrc/openhuman/tools/impl/agent/spawn_subagent.rssrc/openhuman/tools/impl/agent/spawn_worker_thread.rs
senamakel
approved these changes
May 17, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
modelparameter tospawn_subagentand archetype delegation toolsSubagentRunOptionsanddispatch_subagentmodelis omittedRefs #1837
Tests
cargo fmt --allRUSTC=/Users/lee/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc GGML_NATIVE=OFF cargo test -p openhuman --lib subagent_runner -- --nocaptureRUSTC=/Users/lee/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc GGML_NATIVE=OFF cargo test -p openhuman --lib spawn_subagent -- --nocaptureRUSTC=/Users/lee/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc GGML_NATIVE=OFF cargo test -p openhuman --lib archetype_delegation -- --nocaptureRUSTC=/Users/lee/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc GGML_NATIVE=OFF cargo check -p openhumangit diff --checkSummary by CodeRabbit
Release Notes