mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-04-25 11:18:22 +00:00
refactor(config): tighten Phase 2 explicit-passing, isolate current() fallback
Review feedback: the previous sitting added AppConfig.current() fallback inside every new explicit-config helper, which perpetuated the implicit lookup we are trying to eliminate. This commit: - requires app_config as a non-None parameter in internal functions that have clean callers (build_lead_runtime_middlewares, _build_runtime_middlewares) - isolates the AppConfig.current() fallback to the single boundary where LangGraph Server's registration API genuinely cannot pass config (make_lead_agent), and to the two factories still reachable from not-yet-migrated community tool paths (create_chat_model, get_available_tools), each marked with a TODO(P2-10) grep anchor - types RunContext.app_config as AppConfig | None instead of Any - drops the narrative # Phase 2: comments from production source and test bodies; they belong in commit messages, not the code - drops the AppConfig.resolve() helper introduced last commit — it was just another name for the implicit-lookup pattern Make _build_middlewares's kw-only separator explicit so the app_config / config distinction is clear at call sites. 196 targeted tests pass.
This commit is contained in:
parent
cbe8b12744
commit
0e5ff6f431
@ -145,12 +145,10 @@ async def _migrate_orphaned_threads(store, admin_user_id: str) -> int:
|
||||
async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
|
||||
"""Application lifespan handler."""
|
||||
|
||||
# Load config and check necessary environment variables at startup.
|
||||
# Phase 2: explicit-passing primitive. app.state.config is the single source
|
||||
# of truth for FastAPI request handlers via Depends(get_config). AppConfig.init()
|
||||
# is still invoked for backward compatibility with legacy AppConfig.current()
|
||||
# callers that haven't been migrated yet.
|
||||
try:
|
||||
# app.state.config is the source of truth for Depends(get_config).
|
||||
# AppConfig.init() mirrors it to the process-global for not-yet-migrated
|
||||
# callers; both go away in P2-10 once AppConfig.current() is removed.
|
||||
app.state.config = AppConfig.from_file()
|
||||
AppConfig.init(app.state.config)
|
||||
logger.info("Configuration loaded successfully")
|
||||
|
||||
@ -210,6 +210,7 @@ Being proactive with task management demonstrates thoroughness and ensures all r
|
||||
def _build_middlewares(
|
||||
app_config: AppConfig,
|
||||
config: RunnableConfig,
|
||||
*,
|
||||
model_name: str | None,
|
||||
agent_name: str | None = None,
|
||||
custom_middlewares: list[AgentMiddleware] | None = None,
|
||||
@ -297,6 +298,11 @@ def make_lead_agent(
|
||||
from deerflow.tools.builtins import setup_agent
|
||||
|
||||
if app_config is None:
|
||||
# LangGraph Server registers make_lead_agent via langgraph.json; its
|
||||
# invocation path only hands us RunnableConfig. Until that registration
|
||||
# layer owns its own AppConfig, we tolerate the process-global fallback
|
||||
# here. All other entry points (DeerFlowClient, Gateway Worker) pass
|
||||
# app_config explicitly. Remove alongside AppConfig.current() in P2-10.
|
||||
app_config = AppConfig.current()
|
||||
|
||||
cfg = config.get("configurable", {})
|
||||
|
||||
@ -72,7 +72,7 @@ class ToolErrorHandlingMiddleware(AgentMiddleware[AgentState]):
|
||||
|
||||
def _build_runtime_middlewares(
|
||||
*,
|
||||
app_config: "AppConfig | None" = None,
|
||||
app_config: "AppConfig",
|
||||
include_uploads: bool,
|
||||
include_dangling_tool_call_patch: bool,
|
||||
lazy_init: bool = True,
|
||||
@ -81,10 +81,6 @@ def _build_runtime_middlewares(
|
||||
from deerflow.agents.middlewares.llm_error_handling_middleware import LLMErrorHandlingMiddleware
|
||||
from deerflow.agents.middlewares.thread_data_middleware import ThreadDataMiddleware
|
||||
from deerflow.sandbox.middleware import SandboxMiddleware
|
||||
from deerflow.config.app_config import AppConfig
|
||||
|
||||
if app_config is None:
|
||||
app_config = AppConfig.current()
|
||||
|
||||
middlewares: list[AgentMiddleware] = [
|
||||
ThreadDataMiddleware(lazy_init=lazy_init),
|
||||
@ -133,7 +129,7 @@ def _build_runtime_middlewares(
|
||||
return middlewares
|
||||
|
||||
|
||||
def build_lead_runtime_middlewares(*, app_config: "AppConfig | None" = None, lazy_init: bool = True) -> list[AgentMiddleware]:
|
||||
def build_lead_runtime_middlewares(*, app_config: "AppConfig", lazy_init: bool = True) -> list[AgentMiddleware]:
|
||||
"""Middlewares shared by lead agent runtime before lead-only middlewares."""
|
||||
return _build_runtime_middlewares(
|
||||
app_config=app_config,
|
||||
|
||||
@ -47,7 +47,12 @@ def create_chat_model(
|
||||
Returns:
|
||||
A chat model instance.
|
||||
"""
|
||||
config = app_config if app_config is not None else AppConfig.current()
|
||||
if app_config is None:
|
||||
# TODO(P2-10): fold into a required parameter once all callers
|
||||
# (memory updater, summarization middleware's implicit model) thread
|
||||
# config explicitly.
|
||||
app_config = AppConfig.current()
|
||||
config = app_config
|
||||
if name is None:
|
||||
name = config.models[0].name
|
||||
model_config = config.get_model_config(name)
|
||||
|
||||
@ -54,9 +54,7 @@ class RunContext:
|
||||
run_events_config: Any | None = field(default=None)
|
||||
thread_store: Any | None = field(default=None)
|
||||
follow_up_to_run_id: str | None = field(default=None)
|
||||
# Phase 2: app-level config flows through RunContext so Worker can build
|
||||
# DeerFlowContext without consulting the process-global.
|
||||
app_config: Any | None = field(default=None)
|
||||
app_config: AppConfig | None = field(default=None)
|
||||
|
||||
|
||||
async def run_agent(
|
||||
|
||||
@ -56,7 +56,11 @@ def get_available_tools(
|
||||
Returns:
|
||||
List of available tools.
|
||||
"""
|
||||
config = app_config if app_config is not None else AppConfig.current()
|
||||
if app_config is None:
|
||||
# TODO(P2-10): fold into a required parameter once all callers thread
|
||||
# config explicitly (community tool factories, subagent registry, etc.).
|
||||
app_config = AppConfig.current()
|
||||
config = app_config
|
||||
tool_configs = [tool for tool in config.tools if groups is None or tool.group in groups]
|
||||
|
||||
# Do not expose host bash by default when LocalSandboxProvider is active.
|
||||
|
||||
@ -85,7 +85,6 @@ class TestClientInit:
|
||||
DeerFlowClient(agent_name="../path/traversal")
|
||||
|
||||
def test_custom_config_path(self, mock_app_config):
|
||||
# Phase 2: DeerFlowClient stores config locally in self._app_config
|
||||
# rather than touching AppConfig.init() / process-global state.
|
||||
with patch.object(AppConfig, "from_file", return_value=mock_app_config) as mock_from_file:
|
||||
client = DeerFlowClient(config_path="/tmp/custom.yaml")
|
||||
@ -1090,7 +1089,6 @@ class TestMcpConfig:
|
||||
ext_config = MagicMock()
|
||||
ext_config.mcp_servers = {"github": server}
|
||||
|
||||
# Phase 2: client reads from self._app_config, not AppConfig.current()
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
result = client.get_mcp_config()
|
||||
|
||||
@ -1116,7 +1114,6 @@ class TestMcpConfig:
|
||||
# Pre-set agent to verify it gets invalidated
|
||||
client._agent = MagicMock()
|
||||
|
||||
# Phase 2: initial config is stored on the client, not process-global
|
||||
client._app_config = MagicMock(extensions=current_config)
|
||||
|
||||
with (
|
||||
@ -1330,7 +1327,6 @@ class TestMemoryManagement:
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = mem_config
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
result = client.get_memory_config()
|
||||
|
||||
@ -1351,7 +1347,6 @@ class TestMemoryManagement:
|
||||
app_cfg.memory = mem_config
|
||||
data = {"version": "1.0", "facts": []}
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=data):
|
||||
result = client.get_memory_status()
|
||||
@ -2030,7 +2025,6 @@ class TestScenarioMemoryWorkflow:
|
||||
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = config
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=updated_data):
|
||||
status = client.get_memory_status()
|
||||
@ -2315,7 +2309,6 @@ class TestGatewayConformance:
|
||||
ext_config = MagicMock()
|
||||
ext_config.mcp_servers = {"test": server}
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
result = client.get_mcp_config()
|
||||
|
||||
@ -2341,7 +2334,6 @@ class TestGatewayConformance:
|
||||
config_file = tmp_path / "extensions_config.json"
|
||||
config_file.write_text("{}")
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = MagicMock(extensions=ext_config)
|
||||
with (
|
||||
patch("deerflow.client.ExtensionsConfig.resolve_config_path", return_value=config_file),
|
||||
@ -2379,7 +2371,6 @@ class TestGatewayConformance:
|
||||
app_cfg = MagicMock()
|
||||
app_cfg.memory = mem_cfg
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
result = client.get_memory_config()
|
||||
|
||||
@ -2415,7 +2406,6 @@ class TestGatewayConformance:
|
||||
"facts": [],
|
||||
}
|
||||
|
||||
# Phase 2: client reads from self._app_config
|
||||
client._app_config = app_cfg
|
||||
with patch("deerflow.agents.memory.updater.get_memory_data", return_value=memory_data):
|
||||
result = client.get_memory_status()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user