feat(subagents): allow model override per subagent in config.yaml (#2064)

* feat(subagents): allow model override per subagent in config.yaml

Wire the existing SubagentConfig.model field to config.yaml so users
can assign different models to different subagent types.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(subagents): cover model override in SubagentsAppConfig + registry

Addresses review feedback on #2064:

- registry.py: update stale inline comment — the block now applies
  timeout, max_turns AND model overrides, not just timeout.
- test_subagent_timeout_config.py: add coverage for model override
  resolution across SubagentOverrideConfig, SubagentsAppConfig
  (get_model_for + load), and registry.get_subagent_config:
  - per-agent model override is applied to registry-returned config
  - omitted `model` keeps the builtin value
  - explicit `model: null` in config.yaml is equivalent to omission
  - model override on one agent does not affect other agents
  - model override preserves all other fields (name, description,
    timeout_seconds, max_turns)
  - model override does not mutate BUILTIN_SUBAGENTS

Copilot's suggestion (3) "setting model to 'inherit' forces inheritance"
is skipped intentionally: there is no 'inherit' sentinel in the current
implementation — model is `str | None`, and None already means
"inherit from parent". Adding a sentinel would be a new feature, not
test coverage for this PR.

Tests run locally: 51 passed (37 existing + 14 new / expanded).

* test(subagents): reject empty-string model at config load time

Addresses WillemJiang's review comment on #2064 (empty-string edge case):

- subagents_config.py: add `min_length=1` to the `model` field on
  SubagentOverrideConfig. `model: ""` in config.yaml would otherwise
  bypass the `is not None` check and reach create_chat_model(name="")
  as a confusing runtime error. This is symmetric with the existing
  `ge=1` guards on timeout_seconds / max_turns, so the validation style
  stays consistent across all three override fields.
- test_subagent_timeout_config.py: add test_rejects_empty_model
  mirroring the existing test_rejects_zero / test_rejects_negative
  cases; update the docstring on test_model_accepts_any_string (now
  test_model_accepts_any_non_empty_string) to reflect the new guard.

Not addressing the first comment (validating `model` against the
`models:` section at load time) in this PR. `SubagentsAppConfig` is
scoped to the `subagents:` block and cannot see the sibling `models:`
section, so proper cross-section validation needs a second pass or a
structural change that is out of scope here — and the current behavior
is consistent with how timeout_seconds / max_turns work today. Happy to
track this as a follow-up issue covering cross-section validation
uniformly for all three fields.

Tests run locally: 52 passed in this file; 1847 passed, 18 skipped
across the full backend suite. Ruff check + format clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Javen Fang 2026-04-12 16:40:21 +08:00 committed by GitHub
parent c4d273a68a
commit ac04f2704f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 196 additions and 2 deletions

View File

@ -20,6 +20,11 @@ class SubagentOverrideConfig(BaseModel):
ge=1,
description="Maximum turns for this subagent (None = use global or builtin default)",
)
model: str | None = Field(
default=None,
min_length=1,
description="Model name for this subagent (None = inherit from parent agent)",
)
class SubagentsAppConfig(BaseModel):
@ -54,6 +59,20 @@ class SubagentsAppConfig(BaseModel):
return override.timeout_seconds
return self.timeout_seconds
def get_model_for(self, agent_name: str) -> str | None:
"""Get the model override for a specific agent.
Args:
agent_name: The name of the subagent.
Returns:
Model name if overridden, None otherwise (subagent will inherit parent model).
"""
override = self.agents.get(agent_name)
if override is not None and override.model is not None:
return override.model
return None
def get_max_turns_for(self, agent_name: str, builtin_default: int) -> int:
"""Get the effective max_turns for a specific agent."""
override = self.agents.get(agent_name)
@ -84,6 +103,8 @@ def load_subagents_config_from_dict(config_dict: dict) -> None:
parts.append(f"timeout={override.timeout_seconds}s")
if override.max_turns is not None:
parts.append(f"max_turns={override.max_turns}")
if override.model is not None:
parts.append(f"model={override.model}")
if parts:
overrides_summary[name] = ", ".join(parts)

View File

@ -23,7 +23,8 @@ def get_subagent_config(name: str) -> SubagentConfig | None:
if config is None:
return None
# Apply timeout override from config.yaml (lazy import to avoid circular deps)
# Apply runtime overrides (timeout, max_turns, model) from config.yaml
# Lazy import to avoid circular deps.
from deerflow.config.subagents_config import get_subagents_app_config
app_config = get_subagents_app_config()
@ -47,6 +48,15 @@ def get_subagent_config(name: str) -> SubagentConfig | None:
effective_max_turns,
)
overrides["max_turns"] = effective_max_turns
effective_model = app_config.get_model_for(name)
if effective_model is not None and effective_model != config.model:
logger.debug(
"Subagent '%s': model overridden by config.yaml (%s -> %s)",
name,
config.model,
effective_model,
)
overrides["model"] = effective_model
if overrides:
config = replace(config, **overrides)

View File

@ -50,11 +50,19 @@ class TestSubagentOverrideConfig:
override = SubagentOverrideConfig()
assert override.timeout_seconds is None
assert override.max_turns is None
assert override.model is None
def test_explicit_value(self):
override = SubagentOverrideConfig(timeout_seconds=300, max_turns=42)
override = SubagentOverrideConfig(timeout_seconds=300, max_turns=42, model="gpt-5.4")
assert override.timeout_seconds == 300
assert override.max_turns == 42
assert override.model == "gpt-5.4"
def test_model_accepts_any_non_empty_string(self):
"""Model name is a free-form non-empty string; cross-reference validation
against the `models:` section happens at registry lookup time."""
override = SubagentOverrideConfig(model="any-arbitrary-model-name")
assert override.model == "any-arbitrary-model-name"
def test_rejects_zero(self):
with pytest.raises(ValueError):
@ -68,6 +76,13 @@ class TestSubagentOverrideConfig:
with pytest.raises(ValueError):
SubagentOverrideConfig(max_turns=-1)
def test_rejects_empty_model(self):
"""Empty-string model would silently bypass the `is not None` check and
reach `create_chat_model(name="")` as a runtime error. Reject at load time
instead, symmetric with the `ge=1` guard on timeout_seconds / max_turns."""
with pytest.raises(ValueError):
SubagentOverrideConfig(model="")
def test_minimum_valid_value(self):
override = SubagentOverrideConfig(timeout_seconds=1, max_turns=1)
assert override.timeout_seconds == 1
@ -165,6 +180,42 @@ class TestRuntimeResolution:
assert config.get_max_turns_for("general-purpose", 100) == 200
assert config.get_max_turns_for("bash", 60) == 80
def test_get_model_for_returns_none_when_no_override(self):
"""No per-agent model override -> returns None so callers fall back to builtin/parent."""
config = SubagentsAppConfig(timeout_seconds=900)
assert config.get_model_for("general-purpose") is None
assert config.get_model_for("bash") is None
assert config.get_model_for("unknown-agent") is None
def test_get_model_for_returns_override_when_set(self):
config = SubagentsAppConfig(
timeout_seconds=900,
agents={
"general-purpose": SubagentOverrideConfig(model="qwen3.5-35b-a3b"),
"bash": SubagentOverrideConfig(model="gpt-5.4"),
},
)
assert config.get_model_for("general-purpose") == "qwen3.5-35b-a3b"
assert config.get_model_for("bash") == "gpt-5.4"
def test_get_model_for_returns_none_for_omitted_agent(self):
"""An agent not listed in overrides returns None even when other agents have model overrides."""
config = SubagentsAppConfig(
timeout_seconds=900,
agents={"bash": SubagentOverrideConfig(model="gpt-5.4")},
)
assert config.get_model_for("general-purpose") is None
def test_get_model_for_handles_explicit_none(self):
"""Explicit model=None in the override is equivalent to no override."""
config = SubagentsAppConfig(
timeout_seconds=900,
agents={"bash": SubagentOverrideConfig(timeout_seconds=300, model=None)},
)
assert config.get_model_for("bash") is None
# Timeout override is still applied even when model is None.
assert config.get_timeout_for("bash") == 300
# ---------------------------------------------------------------------------
# load_subagents_config_from_dict / get_subagents_app_config singleton
@ -211,6 +262,22 @@ class TestLoadSubagentsConfig:
assert cfg.get_max_turns_for("general-purpose", 100) == 100
assert cfg.get_max_turns_for("bash", 60) == 70
def test_load_with_model_overrides(self):
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {
"general-purpose": {"model": "qwen3.5-35b-a3b"},
"bash": {"model": "gpt-5.4", "timeout_seconds": 300},
},
}
)
cfg = get_subagents_app_config()
assert cfg.get_model_for("general-purpose") == "qwen3.5-35b-a3b"
assert cfg.get_model_for("bash") == "gpt-5.4"
# Other override fields on the same agent must still load correctly.
assert cfg.get_timeout_for("bash") == 300
def test_load_empty_dict_uses_defaults(self):
load_subagents_config_from_dict({})
cfg = get_subagents_app_config()
@ -296,6 +363,97 @@ class TestRegistryGetSubagentConfig:
assert gp_config.timeout_seconds == 900
assert gp_config.max_turns == 120
def test_per_agent_model_override_applied(self):
from deerflow.subagents.registry import get_subagent_config
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"model": "gpt-5.4-mini"}},
}
)
bash_config = get_subagent_config("bash")
assert bash_config.model == "gpt-5.4-mini"
def test_omitted_model_keeps_builtin_value(self):
"""When config.yaml has no `model` field for an agent, the builtin default must be preserved."""
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS
from deerflow.subagents.registry import get_subagent_config
builtin_bash_model = BUILTIN_SUBAGENTS["bash"].model
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"timeout_seconds": 300}},
}
)
bash_config = get_subagent_config("bash")
assert bash_config.model == builtin_bash_model
def test_explicit_null_model_keeps_builtin_value(self):
"""An explicit `model: null` in config.yaml is equivalent to omission — builtin wins."""
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS
from deerflow.subagents.registry import get_subagent_config
builtin_bash_model = BUILTIN_SUBAGENTS["bash"].model
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"model": None}},
}
)
bash_config = get_subagent_config("bash")
assert bash_config.model == builtin_bash_model
def test_model_override_does_not_affect_other_agents(self):
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS
from deerflow.subagents.registry import get_subagent_config
builtin_gp_model = BUILTIN_SUBAGENTS["general-purpose"].model
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"model": "gpt-5.4"}},
}
)
gp_config = get_subagent_config("general-purpose")
assert gp_config.model == builtin_gp_model
def test_model_override_preserves_other_fields(self):
"""Applying a model override must leave timeout_seconds / max_turns / name intact."""
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS
from deerflow.subagents.registry import get_subagent_config
original = BUILTIN_SUBAGENTS["bash"]
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"model": "gpt-5.4-mini"}},
}
)
overridden = get_subagent_config("bash")
assert overridden.model == "gpt-5.4-mini"
assert overridden.name == original.name
assert overridden.description == original.description
# No timeout / max_turns override was set, so they use global default / builtin.
assert overridden.timeout_seconds == 900
assert overridden.max_turns == original.max_turns
def test_model_override_does_not_mutate_builtin(self):
"""Registry must return a new object, leaving the builtin default intact."""
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS
from deerflow.subagents.registry import get_subagent_config
original_bash_model = BUILTIN_SUBAGENTS["bash"].model
load_subagents_config_from_dict(
{
"timeout_seconds": 900,
"agents": {"bash": {"model": "gpt-5.4-mini"}},
}
)
_ = get_subagent_config("bash")
assert BUILTIN_SUBAGENTS["bash"].model == original_bash_model
def test_builtin_config_object_is_not_mutated(self):
"""Registry must return a new object, leaving the builtin default intact."""
from deerflow.subagents.builtins import BUILTIN_SUBAGENTS

View File

@ -575,9 +575,14 @@ sandbox:
# general-purpose:
# timeout_seconds: 1800 # 30 minutes for complex multi-step tasks
# max_turns: 160
# # model: qwen3:32b # Use a specific model (default: inherit from lead agent)
# bash:
# timeout_seconds: 300 # 5 minutes for quick command execution
# max_turns: 80
#
# # Model override: by default, subagents inherit the lead agent's model.
# # Set `model` to use a different model (e.g., a local Ollama model for cost savings).
# # The model name must match a name defined in the `models:` section above.
# ============================================================================
# ACP Agents Configuration