mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-22 14:08:28 +00:00
fix(config): coerce null object config sections to their defaults (#3573)
* fix(config): coerce null object config sections to their defaults #3434 made commented-out list sections (models/tools/tool_groups) parse as [] instead of crashing, but the same scenario still crashes for object sections: commenting out every key under e.g. memory: / summarization: / guardrails: makes PyYAML parse the value as None, and AppConfig then raises "Input should be a valid dictionary" for that section — breaking the documented `cp config.example.yaml config.yaml` first-run flow. Generalize the handling: a model_validator(mode="before") drops None-valued sections so each field falls back to its default (list sections -> [], object sections -> their default config). This subsumes the previous list-only field_validator and the database special-case. Required sections without a default (sandbox) still error when null. Adds test_app_config_coerces_commented_out_object_sections; the existing list-section regression test still passes. * docs+test: address review on null-section coercion - Correct the _drop_null_config_sections docstring: it does not subsume the database special-case; _apply_database_defaults still owns `database` and applies concrete defaults beyond null-coercion. - Strengthen test_app_config_coerces_commented_out_object_sections to assert each null section falls back to its expected default config type, not just that it is non-None. - Add test_app_config_null_required_section_still_errors covering the required-section (sandbox) "still errors when null" claim. --------- Co-authored-by: ly-wang19 <ly-wang19@users.noreply.github.com>
This commit is contained in:
parent
0d732b65dd
commit
c177d0e542
@ -8,7 +8,7 @@ from typing import Any, Self
|
||||
|
||||
import yaml
|
||||
from dotenv import load_dotenv
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator
|
||||
from pydantic import BaseModel, ConfigDict, Field, model_validator
|
||||
|
||||
from deerflow.config.acp_config import ACPAgentConfig, load_acp_config_from_dict
|
||||
from deerflow.config.agents_api_config import AgentsApiConfig, load_agents_api_config_from_dict
|
||||
@ -161,20 +161,30 @@ class AppConfig(BaseModel):
|
||||
),
|
||||
)
|
||||
|
||||
@field_validator("models", "tools", "tool_groups", mode="before")
|
||||
@model_validator(mode="before")
|
||||
@classmethod
|
||||
def _coerce_null_list_sections(cls, value: Any) -> Any:
|
||||
"""Treat a present-but-empty config section as an empty list.
|
||||
def _drop_null_config_sections(cls, data: Any) -> Any:
|
||||
"""Treat a present-but-null config section as absent so its default applies.
|
||||
|
||||
Commenting out every entry under a top-level YAML key — e.g. ``models:``
|
||||
with only comments beneath it, exactly as shipped in
|
||||
``config.example.yaml`` — makes PyYAML parse the value as ``None``.
|
||||
Without this, the documented ``cp config.example.yaml config.yaml``
|
||||
first-run flow crashes with an opaque ``Input should be a valid list``
|
||||
pydantic error. Coercing ``None`` to ``[]`` keeps that flow working and
|
||||
matches the field's own ``default_factory=list``.
|
||||
(a list) or ``memory:`` (an object), with only comments beneath it as
|
||||
shipped throughout ``config.example.yaml`` — makes PyYAML parse the value
|
||||
as ``None``. Without this, the documented ``cp config.example.yaml
|
||||
config.yaml`` first-run flow crashes with an opaque ``Input should be a
|
||||
valid list`` / ``valid dictionary`` pydantic error for that section.
|
||||
|
||||
Dropping the ``None`` lets each field fall back to its default: list
|
||||
sections become ``[]`` via ``default_factory=list`` and object sections
|
||||
get their default config. This generalizes the earlier list-only
|
||||
handling to every section that defines a default. The ``database``
|
||||
section is independent and still owned by ``_apply_database_defaults``
|
||||
(in ``from_file``), which applies concrete defaults beyond null-coercion.
|
||||
Required sections without a default (``sandbox``) intentionally still
|
||||
error when null — there is nothing to fall back to.
|
||||
"""
|
||||
return [] if value is None else value
|
||||
if isinstance(data, dict):
|
||||
return {key: value for key, value in data.items() if value is not None}
|
||||
return data
|
||||
|
||||
@classmethod
|
||||
def resolve_config_path(cls, config_path: str | None = None) -> Path:
|
||||
|
||||
@ -170,6 +170,59 @@ def test_app_config_coerces_commented_out_list_sections(tmp_path, monkeypatch):
|
||||
assert config.tool_groups == []
|
||||
|
||||
|
||||
def test_app_config_coerces_commented_out_object_sections(tmp_path, monkeypatch):
|
||||
"""Commenting out every entry under an object key makes PyYAML parse it as None.
|
||||
|
||||
Same documented ``cp config.example.yaml config.yaml`` flow as the list
|
||||
sections: object sections (memory, summarization, ...) must fall back to
|
||||
their defaults instead of raising ``Input should be a valid dictionary``.
|
||||
"""
|
||||
config_path = tmp_path / "config.yaml"
|
||||
extensions_path = tmp_path / "extensions_config.json"
|
||||
_write_extensions_config(extensions_path)
|
||||
config_path.write_text(
|
||||
yaml.safe_dump(
|
||||
{
|
||||
"sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"},
|
||||
"memory": None,
|
||||
"summarization": None,
|
||||
"guardrails": None,
|
||||
"tool_output": None,
|
||||
"run_events": None,
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path))
|
||||
|
||||
config = AppConfig.from_file(str(config_path))
|
||||
|
||||
# Each present-but-null object section falls back to a real default config
|
||||
# object of the expected type (not merely non-None).
|
||||
assert type(config.memory).__name__ == "MemoryConfig"
|
||||
assert type(config.summarization).__name__ == "SummarizationConfig"
|
||||
assert type(config.guardrails).__name__ == "GuardrailsConfig"
|
||||
assert type(config.tool_output).__name__ == "ToolOutputConfig"
|
||||
assert type(config.run_events).__name__ == "RunEventsConfig"
|
||||
|
||||
|
||||
def test_app_config_null_required_section_still_errors(tmp_path, monkeypatch):
|
||||
"""A present-but-null *required* section still errors.
|
||||
|
||||
``sandbox`` has no default, so dropping a ``sandbox: null`` key leaves the
|
||||
required field absent — there is nothing to fall back to (per
|
||||
``_drop_null_config_sections``), unlike the optional object sections above.
|
||||
"""
|
||||
config_path = tmp_path / "config.yaml"
|
||||
extensions_path = tmp_path / "extensions_config.json"
|
||||
_write_extensions_config(extensions_path)
|
||||
config_path.write_text(yaml.safe_dump({"sandbox": None}), encoding="utf-8")
|
||||
monkeypatch.setenv("DEER_FLOW_EXTENSIONS_CONFIG_PATH", str(extensions_path))
|
||||
|
||||
with pytest.raises(ValidationError):
|
||||
AppConfig.from_file(str(config_path))
|
||||
|
||||
|
||||
def test_app_config_warns_when_no_models_configured(tmp_path, monkeypatch, caplog):
|
||||
config_path = tmp_path / "config.yaml"
|
||||
extensions_path = tmp_path / "extensions_config.json"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user