feat: add config model pins for agents#1905
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds optional orchestrator and per-team model pinning in config, implements deterministic precedence for model resolution (inline override → config pin → existing spec fallback), applies pins in session/provider flows, and adds tests validating precedence and routing. ChangesModel Pinning for Agent Routing
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Resolver as resolve_subagent_provider
participant Config as configured_agent_model
Caller->>Resolver: call(agent_id, is_team_lead, model_override)
alt model_override present
Resolver->>Caller: return (parent_provider, model_override)
else no override
Resolver->>Config: configured_agent_model(agent_id, is_team_lead)
alt config pin found
Config->>Resolver: pinned_model
Resolver->>Caller: return (parent_provider, pinned_model)
else no pin
Resolver->>Resolver: ModelSpec fallback logic
Resolver->>Caller: return (resolved_provider, resolved_model)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/agent/harness/subagent_runner/ops.rs (1)
119-133: ⚡ Quick winUse
debug/tracefor pin-resolution branch diagnostics instead ofinfo.These logs are per-spawn branch diagnostics and can get very noisy at
infolevel in normal runs. Switching them to structuredtracing::debug!(ortrace!) keeps observability useful without inflating baseline logs.As per coding guidelines: “Rust core code must use
log/tracingatdebug/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 119 - 133, Replace the noisy info-level logging in the subagent model-pin branches with a debug/trace call: change the two log::info! invocations that report the inline model override (the branch that returns (parent_provider, model.to_string()) after checking the inline override) and the config-level model pin (the branch using cfg.configured_agent_model(agent_id, is_team_lead)) to tracing::debug! (or tracing::trace! if you want even finer granularity) so these per-spawn diagnostics are emitted at a lower verbosity; keep the same message structure and argument values (agent_id and model) when swapping the macro names.
🤖 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/agent.rs`:
- Around line 30-37: The current model_for_role prefers
self.lead_model/self.agent_model as-is so an option containing only whitespace
(e.g., " ") prevents fallback; change the selection so lead_model and
agent_model are first normalized to None if they are empty or whitespace before
applying or(...) logic. Specifically, in model_for_role trim and filter each
Option from self.lead_model.as_deref() and self.agent_model.as_deref() (e.g.,
map(str::trim).filter(|s| !s.is_empty())) and then use that cleaned Option in
the existing preferred logic (referencing model_for_role, lead_model,
agent_model, preferred) so blank strings no longer block cross-tier fallback.
---
Nitpick comments:
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 119-133: Replace the noisy info-level logging in the subagent
model-pin branches with a debug/trace call: change the two log::info!
invocations that report the inline model override (the branch that returns
(parent_provider, model.to_string()) after checking the inline override) and the
config-level model pin (the branch using cfg.configured_agent_model(agent_id,
is_team_lead)) to tracing::debug! (or tracing::trace! if you want even finer
granularity) so these per-spawn diagnostics are emitted at a lower verbosity;
keep the same message structure and argument values (agent_id and model) when
swapping the macro names.
🪄 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: 04f9a014-78e5-44ef-8c2f-feb8a2401a3c
📒 Files selected for processing (14)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/harness/subagent_runner/ops_tests.rssrc/openhuman/agent/harness/subagent_runner/types.rssrc/openhuman/config/mod.rssrc/openhuman/config/schema/agent.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/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
|
@coderabbitai can u explain to me what's the benefit of pinning the config to agents? or explain this PR in more detail? |
|
Tip For best results, initiate chat on the files or code changes.
What Problem Does This Solve?In a multi-agent system like OpenHuman, different agents (an orchestrator, team leads, sub-agents) can run on different AI models. Previously, the only way to control which model an agent used was through workload routing or inline model specs — meaning you had to change code or per-call options. This PR lets you set model preferences once in a config file and have them automatically apply every time an agent is created. What Does the PR Add?1. New Config OptionsYou can now add entries like this to your config: [orchestrator]
model = "gpt-4o"
[teams.research_team]
lead_model = "gpt-4o"
agent_model = "gpt-4o-mini"
2. Deterministic Model Selection PrecedenceThe PR establishes a clear, three-tier priority chain: flowchart TD
A["1. Inline model override\n(per-call, highest priority)"] --> B["2. Config-level pin\n(orchestrator / team)"]
B --> C["3. Existing ModelSpec / workload routing\n(fallback)"]
style A fill:rgba(255,100,100,0.5)
style B fill:rgba(255,180,50,0.5)
style C fill:rgba(100,200,100,0.5)
This means you always know exactly which model will be used and why. 3. New Structs & Config Resolution LogicTwo new structs are introduced in
A new method 4. Tools That Benefit
Why Is Config-Level Pinning Useful?
This PR is stacked on |
# Conflicts: # src/openhuman/agent/harness/session/builder.rs # src/openhuman/agent/harness/subagent_runner/ops.rs # src/openhuman/agent/harness/subagent_runner/ops_tests.rs # src/openhuman/config/mod.rs # src/openhuman/tools/impl/agent/archetype_delegation.rs
…role before fallback A lead_model/agent_model value containing only whitespace (e.g. " ") was treated as Some(_) by or(...), blocking cross-tier fallback to the other field. Normalize each field individually via map(str::trim).filter before applying the or logic so blank strings fall through correctly. Addresses @coderabbitai on src/openhuman/config/schema/agent.rs:37.
…to debug Per-spawn branch diagnostics (inline model override, config-level pin) were logged at info level, making them noisy on normal runs. Switch to debug so they stay visible when needed without inflating baseline logs. Addresses @coderabbitai nitpick on ops.rs:119-133.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/agent/harness/session/builder.rs`:
- Around line 786-790: Change the diagnostic log in the session builder from
info to debug so model-pin selection doesn't flood logs: replace the log::info!
call that prints "[session-builder] agent_id={} using config-level model pin
model={}" (which references target_agent_id and pinned_model) with log::debug!
so this branch-level model-selection message is emitted at debug level instead
of info.
🪄 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: 7447224b-5c0f-418a-965a-5c7c1d743b5c
📒 Files selected for processing (5)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/config/mod.rssrc/openhuman/config/schema/agent.rssrc/openhuman/config/schema/mod.rssrc/openhuman/config/schema/types.rs
✅ Files skipped from review due to trivial changes (1)
- src/openhuman/config/schema/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/config/mod.rs
- src/openhuman/config/schema/agent.rs
- src/openhuman/config/schema/types.rs
Model-pin selection is a per-session branch diagnostic; info level floods logs on normal runs. Switch to debug to match the ops.rs change. Addresses @coderabbitai on builder.rs:786-790.
Co-authored-by: honor2030 <19909783+honor2030@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
2 participants
Summary
[orchestrator] model = ...and[teams.<team>] lead_model/agent_modelconfig tables for agent model defaultsmodeloverride > config-level pin > existing model spec / workload routingNotes
modelparameter plumbing used by the highest-precedence override path.Refs #1837
Test Plan
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 model_pin_tests -- --nocaptureRUSTC=/Users/lee/.rustup/toolchains/1.93.0-aarch64-apple-darwin/bin/rustc GGML_NATIVE=OFF cargo check -p openhumangit diff --checkSummary by CodeRabbit
New Features
Tests