mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 00:17:53 +00:00
fix(subagents): clamp subagent limit consistently with MIN_SUBAGENT_LIMIT (#4081)
* fix(subagents): align prompt and middleware subagent limit; allow min of 1 SubagentLimitMiddleware clamped max_concurrent to [2, 4] internally, but agent.py and client.py fed the raw config value into the system prompt, so a user-configured 1 (or 5) produced a prompt that disagreed with the enforced middleware limit. Lower MIN_SUBAGENT_LIMIT to 1 and clamp the raw config value with _clamp_subagent_limit() at both the agent factory and the embedded client so the prompt and middleware see the same value. * fix: remove unused imports MAX_CONCURRENT_SUBAGENT_CALLS, MIN_CONCURRENT_SUBAGENT_CALLS, clamp_subagent_concurrency * fix: harmonize clamp range [1,4] across middleware, config, and prompt path; fix lint - Changed MIN_CONCURRENT_SUBAGENT_CALLS from 2 to 1 so prompt.py's clamp_subagent_concurrency and the middleware's _clamp_subagent_limit both clamp to [1,4] — eliminating the divergence where the prompt told the model 'max 2 task calls' but the middleware enforced 1. - Applied _clamp_subagent_limit at build_middlewares (agent.py:360) so all 3 construction sites (agent.py:360, agent.py:450, client.py:259) consistently clamp the config-resolved limit. - Derived MIN_SUBAGENT_LIMIT / MAX_SUBAGENT_LIMIT from MIN_CONCURRENT_SUBAGENT_CALLS / MAX_CONCURRENT_SUBAGENT_CALLS so the two module-level definitions stay in sync. - Added TestConfigParity.test_prompt_path_and_middleware_clamp_agree regression test. - Fixed lint. * fix(lint): add missing imports for MIN_CONCURRENT_SUBAGENT_CALLS and MAX_CONCURRENT_SUBAGENT_CALLS * docs+test: update AGENTS.md clamp range to 1-4; add prompt/middleware parity regression test - backend/AGENTS.md still documented the old [2,4] clamp in two places; updated to [1,4] to match MIN_CONCURRENT_SUBAGENT_CALLS = 1. - Added test_apply_prompt_template_single_subagent_limit_matches_middleware: renders the real system prompt with max_concurrent_subagents=1 and asserts the advertised HARD LIMITS value equals SubagentLimitMiddleware's enforced max_concurrent — the end-to-end check that would have caught the [1,4] vs [2,4] prompt-path divergence flagged in review. * refactor: simplify per review — restore clamp delegation, drop redundant call-site clamps Per willem-bd's review, reduce the PR to the one behavioral change plus docs/tests: - _clamp_subagent_limit delegates to clamp_subagent_concurrency again instead of inlining a byte-identical copy; with a single source of truth the TestConfigParity sync-check class is unnecessary — dropped. - Revert the call-site clamps in agent.py (build_middlewares, _make_lead_agent) and client.py (_ensure_agent) to main: both downstream consumers (SubagentLimitMiddleware.__init__ and the prompt path) already clamp internally, and the cross-module private import of _clamp_subagent_limit goes away with them. - Keep MIN_CONCURRENT_SUBAGENT_CALLS = 1 (the fix), the [1, 4] docstring updates, the AGENTS.md range corrections, and the end-to-end prompt/middleware parity test for single-subagent mode (docstring reworded: on main a configured 1 was bumped to 2 by both paths — there was no divergence to fix, just a silently raised floor). * test: fix stale comment referencing reverted agent.py/client.py call-site clamps --------- Co-authored-by: nankingjing <nankingjing@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
ca3e510b7d
commit
126fc9ea81
@ -342,7 +342,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc
|
||||
24. **McpRoutingMiddleware** - *(optional, if `tool_search.enabled` and PR1 MCP routing metadata produce a routing index)* Auto-promotes matching deferred MCP tool schemas before the model call by writing a minimal `promoted` state update. It matches only the latest real `HumanMessage`, uses the global `tool_search.auto_promote_top_k` limit (default 3, clamped to 1..5), never executes tools, and must be installed before `DeferredToolFilterMiddleware`
|
||||
25. **DeferredToolFilterMiddleware** - *(optional, if `tool_search.enabled`)* Hides deferred (MCP) tool schemas from the bound model until `tool_search` or `McpRoutingMiddleware` promotes them (reads per-thread promotions from `ThreadState.promoted`, hash-scoped)
|
||||
26. **SystemMessageCoalescingMiddleware** - Merges every SystemMessage into a single leading SystemMessage per request; provider-agnostic fix for strict backends (vLLM/SGLang/Qwen/Anthropic) that reject non-leading system messages. Touches the per-request payload only (checkpoint state unchanged); on midnight crossings only the latest `dynamic_context_reminder` SystemMessage survives
|
||||
27. **SubagentLimitMiddleware** - *(optional, if `subagent_enabled`)* Truncates excess `task` tool calls to enforce both the per-response concurrency limit (`max_concurrent_subagents`, clamped to 2-4) and the per-run total delegation cap (`max_total_subagents` runtime override or `subagents.max_total_per_run`, default 6, clamped to 1-50). The total cap counts current-run entries in the durable delegation ledger (entries are tagged with `run_id` when captured), so repeated planning checkpoints in one run cannot keep launching legal-sized batches indefinitely, while later user turns in the same thread get a fresh run budget. If the cap is exhausted, the middleware strips remaining `task` calls, forces `finish_reason="stop"`, and appends a visible limit note so the run can synthesize existing results instead of ending with an empty tool-call response.
|
||||
27. **SubagentLimitMiddleware** - *(optional, if `subagent_enabled`)* Truncates excess `task` tool calls to enforce both the per-response concurrency limit (`max_concurrent_subagents`, clamped to 1-4) and the per-run total delegation cap (`max_total_subagents` runtime override or `subagents.max_total_per_run`, default 6, clamped to 1-50). The total cap counts current-run entries in the durable delegation ledger (entries are tagged with `run_id` when captured), so repeated planning checkpoints in one run cannot keep launching legal-sized batches indefinitely, while later user turns in the same thread get a fresh run budget. If the cap is exhausted, the middleware strips remaining `task` calls, forces `finish_reason="stop"`, and appends a visible limit note so the run can synthesize existing results instead of ending with an empty tool-call response.
|
||||
28. **LoopDetectionMiddleware** - *(optional, if `loop_detection.enabled`)* Detects repeated tool-call loops; hard-stop clears both structured `tool_calls` and raw provider tool-call metadata before forcing a final text answer; stamps `loop_capped` via `consume_stop_reason` (#3875 Phase 2), symmetric to `TokenBudgetMiddleware`
|
||||
29. **TokenBudgetMiddleware** - *(optional, if `token_budget.enabled`)* Enforces per-run token limits
|
||||
30. **Custom middlewares** - *(optional)* Any `custom_middlewares` passed to `build_middlewares` are injected here, before config-declared extensions and the terminal-response/safety/clarification tail
|
||||
@ -518,7 +518,7 @@ copying a raw checkpoint because delta state is not self-contained in one tuple.
|
||||
**Built-in Agents**: `general-purpose` (all tools except `task`) and `bash` (command specialist)
|
||||
**User-scoped Skills**: Subagents resolve their configured skills through `get_or_new_user_skill_storage(user_id)` using the parent runtime identity, with `DEFAULT_USER_ID` only when no identity is available. This keeps custom-skill shadowing and visibility aligned with the lead agent instead of reading the global-only catalog.
|
||||
**Execution**: Dual thread pool - `_scheduler_pool` (3 workers) + `_execution_pool` (3 workers)
|
||||
**Concurrency and total delegation cap**: `MAX_CONCURRENT_SUBAGENTS = 3` is enforced by `SubagentLimitMiddleware` (truncates excess tool calls in `after_model`; runtime `max_concurrent_subagents` is clamped to 2-4). The same middleware also enforces `subagents.max_total_per_run` (default 6, config schema 1-50, runtime override `max_total_subagents` clamped to the same range) against current-run entries in the durable delegation ledger, so a long lead-agent run cannot bypass concurrency limits by launching repeated legal-sized batches at each planning checkpoint, but historical delegations from previous runs in the same thread do not consume the new run's budget. The lead-agent prompt uses the same clamped values, so model-visible limits match enforcement. Gateway `run_agent()` and embedded `DeerFlowClient.stream()` both provide a per-invocation `run_id` in runtime context; `DeerFlowClient.stream()` also tags its input `HumanMessage` with that same id so durable-context capture can identify the current request boundary. Gateway resume paths may not append a new `HumanMessage`, so the worker also exposes the pre-run checkpoint's message ids in runtime context; durable-context capture uses that as the current-run boundary and never re-tags older task calls as the resumed run. When no delegation slots remain, task calls are stripped, provider raw tool-call metadata is synced, `finish_reason` is forced to `stop`, and a visible "subagent delegation limit" note is appended so the agent can synthesize already-collected results. Default subagent timeout `subagents.timeout_seconds=1800` (30 min) and built-in `general-purpose` `max_turns=150` (raised from 100/15-min so deep-research subtasks stop hitting `GraphRecursionError` out of the box)
|
||||
**Concurrency and total delegation cap**: `MAX_CONCURRENT_SUBAGENTS = 3` is enforced by `SubagentLimitMiddleware` (truncates excess tool calls in `after_model`; runtime `max_concurrent_subagents` is clamped to 1-4). The same middleware also enforces `subagents.max_total_per_run` (default 6, config schema 1-50, runtime override `max_total_subagents` clamped to the same range) against current-run entries in the durable delegation ledger, so a long lead-agent run cannot bypass concurrency limits by launching repeated legal-sized batches at each planning checkpoint, but historical delegations from previous runs in the same thread do not consume the new run's budget. The lead-agent prompt uses the same clamped values, so model-visible limits match enforcement. Gateway `run_agent()` and embedded `DeerFlowClient.stream()` both provide a per-invocation `run_id` in runtime context; `DeerFlowClient.stream()` also tags its input `HumanMessage` with that same id so durable-context capture can identify the current request boundary. Gateway resume paths may not append a new `HumanMessage`, so the worker also exposes the pre-run checkpoint's message ids in runtime context; durable-context capture uses that as the current-run boundary and never re-tags older task calls as the resumed run. When no delegation slots remain, task calls are stripped, provider raw tool-call metadata is synced, `finish_reason` is forced to `stop`, and a visible "subagent delegation limit" note is appended so the agent can synthesize already-collected results. Default subagent timeout `subagents.timeout_seconds=1800` (30 min) and built-in `general-purpose` `max_turns=150` (raised from 100/15-min so deep-research subtasks stop hitting `GraphRecursionError` out of the box)
|
||||
**Flow**: `task()` tool → `SubagentExecutor` → background thread → poll 5s → SSE events → result. `task_started` carries the resolved effective model name. The per-subagent `SubagentTokenCollector` publishes a cumulative usage snapshot to the shared `SubagentResult` after every completed LLM response; the next `task_running` event carries that snapshot, so collapsed workspace cards can update without re-accounting parent-run totals. Terminal ToolMessage metadata (`subagent_model_name`, `subagent_token_usage`) and the persisted `subagent.end` event retain the model/usage after reload; absent provider usage stays absent rather than being estimated as zero.
|
||||
**Events**: `task_started`, `task_running`, `task_completed`/`task_failed`/`task_timed_out`
|
||||
**Handled LLM failures**: `LLMErrorHandlingMiddleware` deliberately converts provider/model exceptions into an `AIMessage` so the graph can end cleanly, stamping `additional_kwargs.deerflow_error_fallback=true` plus error metadata. Clean graph termination does not imply subagent success: `SubagentExecutor` inspects the last assistant message at terminalization and maps a marked fallback to `SubagentStatus.FAILED`, which then emits `task_failed` and the existing structured `subagent_error`. Only the marker is authoritative — error-looking assistant prose without it remains a normal completed result, so neither the executor nor frontend parses display text as a status protocol.
|
||||
|
||||
@ -36,7 +36,7 @@ _TOTAL_LIMIT_STOP_MSG = (
|
||||
|
||||
|
||||
def _clamp_subagent_limit(value: int) -> int:
|
||||
"""Clamp subagent limit to valid range [2, 4]."""
|
||||
"""Clamp subagent limit to valid range [1, 4]."""
|
||||
return clamp_subagent_concurrency(value)
|
||||
|
||||
|
||||
@ -104,7 +104,7 @@ class SubagentLimitMiddleware(AgentMiddleware[AgentState]):
|
||||
|
||||
Args:
|
||||
max_concurrent: Maximum number of concurrent subagent calls allowed.
|
||||
Defaults to MAX_CONCURRENT_SUBAGENTS (3). Clamped to [2, 4].
|
||||
Defaults to MAX_CONCURRENT_SUBAGENTS (3). Clamped to [1, 4].
|
||||
max_total: Maximum number of subagent calls allowed across the run.
|
||||
Defaults to 6. Clamped to [1, 50].
|
||||
"""
|
||||
|
||||
@ -11,7 +11,7 @@ logger = logging.getLogger(__name__)
|
||||
DEFAULT_MAX_TOTAL_SUBAGENTS_PER_RUN = 6
|
||||
MIN_TOTAL_SUBAGENTS_PER_RUN = 1
|
||||
MAX_TOTAL_SUBAGENTS_PER_RUN = 50
|
||||
MIN_CONCURRENT_SUBAGENT_CALLS = 2
|
||||
MIN_CONCURRENT_SUBAGENT_CALLS = 1
|
||||
MAX_CONCURRENT_SUBAGENT_CALLS = 4
|
||||
|
||||
|
||||
|
||||
@ -262,6 +262,48 @@ def test_apply_prompt_template_clamps_subagent_limits_to_enforced_bounds(monkeyp
|
||||
assert "MAXIMUM 50 `task` CALLS PER RUN" in prompt
|
||||
|
||||
|
||||
def test_apply_prompt_template_single_subagent_limit_matches_middleware(monkeypatch):
|
||||
"""Regression test for single-subagent mode (MIN_CONCURRENT_SUBAGENT_CALLS = 1).
|
||||
|
||||
Before the floor was lowered to 1, a user-configured limit of 1 was silently
|
||||
bumped to 2 by both the prompt path and the middleware. This renders the real
|
||||
system prompt with max_concurrent_subagents=1 and asserts the advertised
|
||||
HARD LIMITS value equals the middleware-enforced max_concurrent, so the two
|
||||
paths cannot drift apart on the newly-allowed value.
|
||||
"""
|
||||
from deerflow.agents.middlewares.subagent_limit_middleware import SubagentLimitMiddleware
|
||||
|
||||
explicit_config = SimpleNamespace(
|
||||
sandbox=SimpleNamespace(
|
||||
use="deerflow.sandbox.local:LocalSandboxProvider",
|
||||
allow_host_bash=False,
|
||||
mounts=[],
|
||||
),
|
||||
subagents=SubagentsAppConfig(),
|
||||
skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")),
|
||||
skill_evolution=SimpleNamespace(enabled=False),
|
||||
tool_search=SimpleNamespace(enabled=False),
|
||||
memory=SimpleNamespace(enabled=False, injection_enabled=True, max_injection_tokens=2000),
|
||||
acp_agents={},
|
||||
)
|
||||
|
||||
monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None: SimpleNamespace(load_skills=lambda enabled_only=True: []))
|
||||
monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "")
|
||||
|
||||
enforced = SubagentLimitMiddleware(max_concurrent=1).max_concurrent
|
||||
assert enforced == 1 # 1 must pass through, not be bumped to 2
|
||||
|
||||
prompt = prompt_module.apply_prompt_template(
|
||||
subagent_enabled=True,
|
||||
max_concurrent_subagents=1,
|
||||
max_total_subagents=6,
|
||||
app_config=explicit_config,
|
||||
)
|
||||
|
||||
assert f"MAXIMUM {enforced} `task` CALLS PER RESPONSE" in prompt
|
||||
assert f"HARD LIMITS: max {enforced} `task` calls per response" in prompt
|
||||
|
||||
|
||||
def test_build_acp_section_uses_explicit_app_config_without_global_config(monkeypatch):
|
||||
explicit_config = SimpleNamespace(acp_agents={"codex": object()})
|
||||
|
||||
|
||||
@ -52,11 +52,23 @@ def _raw_tool_call(call_id: str, name: str = "task") -> dict:
|
||||
|
||||
|
||||
class TestClampSubagentLimit:
|
||||
def test_below_min_clamped_to_min(self):
|
||||
assert _clamp_subagent_limit(0) == MIN_SUBAGENT_LIMIT
|
||||
assert _clamp_subagent_limit(1) == MIN_SUBAGENT_LIMIT
|
||||
def test_min_limit_is_one(self):
|
||||
# MIN lowered from 2 to 1 so a user asking for a single subagent gets 1.
|
||||
# Both consumers (SubagentLimitMiddleware.__init__ and the prompt path)
|
||||
# share this floor via clamp_subagent_concurrency in subagents_config.py.
|
||||
assert MIN_SUBAGENT_LIMIT == 1
|
||||
assert MAX_SUBAGENT_LIMIT == 4
|
||||
|
||||
def test_above_max_clamped_to_max(self):
|
||||
def test_below_min_clamped_to_one(self):
|
||||
assert _clamp_subagent_limit(0) == 1
|
||||
assert _clamp_subagent_limit(-5) == 1
|
||||
|
||||
def test_one_is_allowed_not_bumped_to_two(self):
|
||||
# Previously 1 clamped up to 2; it must now pass through as 1.
|
||||
assert _clamp_subagent_limit(1) == 1
|
||||
|
||||
def test_above_max_clamped_to_four(self):
|
||||
assert _clamp_subagent_limit(5) == 4
|
||||
assert _clamp_subagent_limit(10) == MAX_SUBAGENT_LIMIT
|
||||
assert _clamp_subagent_limit(100) == MAX_SUBAGENT_LIMIT
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user