From 0eed90df3a175138b93e506047292305dee53cb9 Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Sun, 4 Oct 2026 09:09:15 +0800 Subject: [PATCH] refactor(config): converge the boolean guards into one shared helper (#6226) * refactor(config): converge the boolean guards into one shared helper Willem-bd's #6171 review follow-up: a tiny shared helper lets the next numeric field get the guard in one line and unifies the messages. Consolidates 14 validator copies across 9 config files into deerflow/config/_boolean_guards.reject_boolean(value, info, kind=...); the bare per-file messages gain their field name prefix, command_timeout keeps its distinct wording. Signed-off-by: yetuge <2219677952@qq.com> * refactor(config): converge stream_bridge_config and pin the guard wording Review follow-ups on #6226: convert the last surviving per-file copy (stream_bridge_config) so deerflow/config is fully converged, make kind required keyword-only so a float field cannot silently inherit integer wording, and pin the unified field-prefixed messages with direct tests. Signed-off-by: yetuge <2219677952@qq.com> * test(config): cast the boolean-guard test probe to ValidationInfo The _info probe was annotated -> object, so a type checker flags every call site that passes its result to reject_boolean's info: ValidationInfo parameter (runtime was fine since the helper only reads .field_name). Return cast(ValidationInfo, _Probe(field_name)) to keep the probe honest for both. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> --- .../deerflow/config/_boolean_guards.py | 37 +++++++++++++++++++ .../harness/deerflow/config/app_config.py | 18 +++------ .../deerflow/config/database_config.py | 15 +++----- .../deerflow/config/extensions_config.py | 15 +++----- .../deerflow/config/loop_detection_config.py | 10 ++--- .../harness/deerflow/config/model_config.py | 10 ++--- .../deerflow/config/run_events_config.py | 10 ++--- .../harness/deerflow/config/sandbox_config.py | 22 +++++------ .../deerflow/config/stream_bridge_config.py | 6 +-- .../deerflow/config/summarization_config.py | 10 ++--- .../harness/deerflow/config/title_config.py | 8 ++-- backend/tests/test_boolean_guards.py | 34 +++++++++++++++++ 12 files changed, 121 insertions(+), 74 deletions(-) create mode 100644 backend/packages/harness/deerflow/config/_boolean_guards.py create mode 100644 backend/tests/test_boolean_guards.py diff --git a/backend/packages/harness/deerflow/config/_boolean_guards.py b/backend/packages/harness/deerflow/config/_boolean_guards.py new file mode 100644 index 0000000000..d03097a61c --- /dev/null +++ b/backend/packages/harness/deerflow/config/_boolean_guards.py @@ -0,0 +1,37 @@ +"""Shared ``mode="before"`` field validators that reject booleans for numeric config fields. + +Pydantic coerces ``true``/``false`` into ``1``/``0`` for int and float fields, so a boolean in +config silently flips behavior — capping a loop at one step, disabling retries, serializing +concurrent calls, or skipping a health check (#6017, #6171). Register one guard per numeric +field instead of copying the check into another config file: + + @field_validator("max_concurrent_calls", mode="before") + @classmethod + def _reject_boolean_llm_call_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") +""" + +from __future__ import annotations + +from pydantic import ValidationInfo + + +def reject_boolean(value: object, info: ValidationInfo, *, kind: str) -> object: + """Reject a boolean for the numeric config field named by ``info.field_name``. + + Args: + value: the raw field value, before Pydantic's numeric coercion. + info: the validation context carrying the field name. + kind: the wording after "must be" — ``"an integer"`` for int fields, + ``"a number"`` for float fields, or a custom phrase such as + ``"a number of seconds or null"``. Required, so wiring a float + field cannot silently inherit integer wording. + + Raises: + ValueError: when ``value`` is a bool. ``bool`` is an ``int`` subclass, + so without this guard Pydantic coerces ``true``/``false`` into + ``1``/``0`` before ``ge``/``le`` constraints ever run. + """ + if isinstance(value, bool): + raise ValueError(f"{info.field_name} must be {kind}, not a boolean") + return value diff --git a/backend/packages/harness/deerflow/config/app_config.py b/backend/packages/harness/deerflow/config/app_config.py index 1c7d136c43..a4c0121d13 100644 --- a/backend/packages/harness/deerflow/config/app_config.py +++ b/backend/packages/harness/deerflow/config/app_config.py @@ -9,6 +9,7 @@ import yaml from dotenv import load_dotenv from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, ValidationInfo, field_validator, model_validator +from deerflow.config._boolean_guards import reject_boolean from deerflow.config.acp_config import ACPAgentConfig, load_acp_config_from_dict from deerflow.config.agent_storage_config import AgentStorageConfig from deerflow.config.agents_api_config import AgentsApiConfig, load_agents_api_config_from_dict @@ -76,13 +77,6 @@ CONFIG_FILE_DATABASE_DEFAULTS = { } -def _reject_boolean_int(value: object, info: ValidationInfo) -> object: - """Reject YAML `true`/`false` before Pydantic coerces them to `1`/`0`.""" - if isinstance(value, bool): - raise ValueError(f"{info.field_name} must be an integer, not a boolean") - return value - - class CircuitBreakerConfig(BaseModel): """Configuration for the LLM Circuit Breaker.""" @@ -99,10 +93,8 @@ class CircuitBreakerConfig(BaseModel): @field_validator("failure_threshold", "recovery_timeout_sec", mode="before") @classmethod - def _reject_boolean_circuit_settings(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_circuit_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") class LlmCallConfig(BaseModel): @@ -171,7 +163,7 @@ class LlmCallConfig(BaseModel): ) @classmethod def _reject_boolean_llm_call_settings(cls, value: object, info: ValidationInfo) -> object: - return _reject_boolean_int(value, info) + return reject_boolean(value, info, kind="an integer") class LoggingEnhanceConfig(BaseModel): @@ -275,7 +267,7 @@ class AppConfig(BaseModel): @field_validator("recursion_limit", "max_recursion_limit", mode="before") @classmethod def _reject_boolean_recursion_limits(cls, value: object, info: ValidationInfo) -> object: - return _reject_boolean_int(value, info) + return reject_boolean(value, info, kind="an integer") models: list[ModelConfig] = Field(default_factory=list, description="Available models") sandbox: SandboxConfig = Field( diff --git a/backend/packages/harness/deerflow/config/database_config.py b/backend/packages/harness/deerflow/config/database_config.py index 87eb7d83f7..4e409fbbf5 100644 --- a/backend/packages/harness/deerflow/config/database_config.py +++ b/backend/packages/harness/deerflow/config/database_config.py @@ -35,8 +35,9 @@ import logging import os from typing import Any, Literal -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator +from deerflow.config._boolean_guards import reject_boolean from deerflow.config.postgres_schema import POSTGRES_SCHEMA_PATTERN, validate_postgres_schema logger = logging.getLogger(__name__) @@ -217,17 +218,13 @@ class DatabaseConfig(BaseModel): @field_validator("pool_size", "pool_recycle", mode="before") @classmethod - def _reject_boolean_pool_settings(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_pool_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") @field_validator("command_timeout", mode="before") @classmethod - def _reject_boolean_command_timeout(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("command_timeout must be a number of seconds or null, not a boolean") - return value + def _reject_boolean_command_timeout(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="a number of seconds or null") # -- Legacy key migration (not user-configured) -- diff --git a/backend/packages/harness/deerflow/config/extensions_config.py b/backend/packages/harness/deerflow/config/extensions_config.py index 1cd63d8ab9..0a5781b456 100644 --- a/backend/packages/harness/deerflow/config/extensions_config.py +++ b/backend/packages/harness/deerflow/config/extensions_config.py @@ -14,8 +14,9 @@ from datetime import date, datetime, time from pathlib import Path from typing import Any, Literal -from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from pydantic import BaseModel, ConfigDict, Field, ValidationInfo, field_validator, model_validator +from deerflow.config._boolean_guards import reject_boolean from deerflow.config.runtime_paths import existing_project_file from deerflow.constants import ( DEFAULT_MCP_SESSION_INIT_TIMEOUT, @@ -206,10 +207,8 @@ class McpOAuthConfig(BaseModel): @field_validator("refresh_skew_seconds", mode="before") @classmethod - def _reject_boolean_refresh_skew(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_refresh_skew(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") extra_token_params: dict[str, str] = Field(default_factory=dict, description="Additional form params sent to token endpoint") model_config = ConfigDict(extra="allow") @@ -262,10 +261,8 @@ class McpServerConfig(BaseModel): @field_validator("tool_call_timeout", "session_init_timeout", mode="before") @classmethod - def _reject_boolean_mcp_timeouts(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be a number, not a boolean") - return value + def _reject_boolean_mcp_timeouts(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="a number") task_toolsets: list[McpTaskToolsetConfig] = Field( default_factory=list, diff --git a/backend/packages/harness/deerflow/config/loop_detection_config.py b/backend/packages/harness/deerflow/config/loop_detection_config.py index 6ecb4a21fe..e7ed3d0acb 100644 --- a/backend/packages/harness/deerflow/config/loop_detection_config.py +++ b/backend/packages/harness/deerflow/config/loop_detection_config.py @@ -2,11 +2,7 @@ from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator - -def _reject_boolean_threshold(value: object, info: ValidationInfo) -> object: - if isinstance(value, bool): - raise ValueError(f"{info.field_name} must be an integer, not a boolean") - return value +from deerflow.config._boolean_guards import reject_boolean class ToolFreqOverride(BaseModel): @@ -23,7 +19,7 @@ class ToolFreqOverride(BaseModel): @field_validator("warn", "hard_limit", mode="before") @classmethod def reject_boolean_thresholds(cls, value: object, info: ValidationInfo) -> object: - return _reject_boolean_threshold(value, info) + return reject_boolean(value, info, kind="an integer") @model_validator(mode="after") def _validate(self) -> "ToolFreqOverride": @@ -85,7 +81,7 @@ class LoopDetectionConfig(BaseModel): ) @classmethod def reject_boolean_thresholds(cls, value: object, info: ValidationInfo) -> object: - return _reject_boolean_threshold(value, info) + return reject_boolean(value, info, kind="an integer") @model_validator(mode="after") def validate_thresholds(self) -> "LoopDetectionConfig": diff --git a/backend/packages/harness/deerflow/config/model_config.py b/backend/packages/harness/deerflow/config/model_config.py index 0e18682ccb..b9a700a48d 100644 --- a/backend/packages/harness/deerflow/config/model_config.py +++ b/backend/packages/harness/deerflow/config/model_config.py @@ -1,7 +1,9 @@ import re from typing import Annotated, Literal -from pydantic import BaseModel, BeforeValidator, ConfigDict, Field, field_validator, model_validator +from pydantic import BaseModel, BeforeValidator, ConfigDict, Field, ValidationInfo, field_validator, model_validator + +from deerflow.config._boolean_guards import reject_boolean _INTEGER_LITERAL_PATTERN = re.compile(r"[0-9]+") @@ -197,10 +199,8 @@ class ModelConfig(BaseModel): @field_validator("stream_chunk_timeout", "context_window", mode="before") @classmethod - def _reject_boolean_numeric_settings(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be a number, not a boolean") - return value + def _reject_boolean_numeric_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="a number") thinking: dict | None = Field( default_factory=lambda: None, diff --git a/backend/packages/harness/deerflow/config/run_events_config.py b/backend/packages/harness/deerflow/config/run_events_config.py index b71f6e310e..ea640375c2 100644 --- a/backend/packages/harness/deerflow/config/run_events_config.py +++ b/backend/packages/harness/deerflow/config/run_events_config.py @@ -15,7 +15,9 @@ from __future__ import annotations from typing import Literal -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, ValidationInfo, field_validator + +from deerflow.config._boolean_guards import reject_boolean class RunEventsConfig(BaseModel): @@ -31,10 +33,8 @@ class RunEventsConfig(BaseModel): @field_validator("max_trace_content", mode="before") @classmethod - def _reject_boolean_max_trace_content(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_max_trace_content(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") track_token_usage: bool = Field( default=True, diff --git a/backend/packages/harness/deerflow/config/sandbox_config.py b/backend/packages/harness/deerflow/config/sandbox_config.py index d012ce727f..0531eb275a 100644 --- a/backend/packages/harness/deerflow/config/sandbox_config.py +++ b/backend/packages/harness/deerflow/config/sandbox_config.py @@ -2,7 +2,9 @@ import ipaddress import math from typing import Literal -from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from pydantic import BaseModel, ConfigDict, Field, ValidationInfo, field_validator, model_validator + +from deerflow.config._boolean_guards import reject_boolean SandboxOwnershipType = Literal["memory", "redis"] SandboxOverflowPolicy = Literal["wait", "reject", "burst"] @@ -39,10 +41,8 @@ class SandboxNetworkConfig(BaseModel): @field_validator("temporary_grant_ttl", mode="before") @classmethod - def _reject_boolean_temporary_grant_ttl(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_temporary_grant_ttl(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") proxy_image: str = Field( default="ghcr.io/bytedance/deer-flow-sandbox-network-proxy:latest", @@ -123,10 +123,8 @@ class SandboxOwnershipConfig(BaseModel): @field_validator("renewal_interval_seconds", "ttl_multiplier", mode="before") @classmethod - def _reject_boolean_ownership_settings(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be a number, not a boolean") - return value + def _reject_boolean_ownership_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="a number") key_prefix: str = Field( default="deerflow:sandbox:owner", @@ -267,10 +265,8 @@ class SandboxConfig(BaseModel): mode="before", ) @classmethod - def _reject_boolean_numeric_settings(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be a number, not a boolean") - return value + def _reject_boolean_numeric_settings(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="a number") health_check_skip_seconds: float | None = Field( default=None, diff --git a/backend/packages/harness/deerflow/config/stream_bridge_config.py b/backend/packages/harness/deerflow/config/stream_bridge_config.py index fb2124dc3d..d051663e95 100644 --- a/backend/packages/harness/deerflow/config/stream_bridge_config.py +++ b/backend/packages/harness/deerflow/config/stream_bridge_config.py @@ -4,6 +4,8 @@ from typing import Any, Literal from pydantic import BaseModel, Field, ValidationInfo, field_validator +from deerflow.config._boolean_guards import reject_boolean + StreamBridgeType = Literal["memory", "redis"] DEFAULT_HEARTBEAT_INTERVAL_SECONDS = 15.0 MAX_HEARTBEAT_INTERVAL_SECONDS = 86_400.0 @@ -66,9 +68,7 @@ class StreamBridgeConfig(BaseModel): @classmethod def reject_boolean_seconds(cls, value: Any, info: ValidationInfo) -> Any: """Reject booleans before Pydantic coerces them to floats.""" - if isinstance(value, bool): - raise ValueError(f"{info.field_name} must be a number, not a boolean") - return value + return reject_boolean(value, info, kind="a number") # Global configuration instance — None means no stream bridge is configured diff --git a/backend/packages/harness/deerflow/config/summarization_config.py b/backend/packages/harness/deerflow/config/summarization_config.py index 043e26cbca..08d751b671 100644 --- a/backend/packages/harness/deerflow/config/summarization_config.py +++ b/backend/packages/harness/deerflow/config/summarization_config.py @@ -3,7 +3,9 @@ import math from typing import Literal -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import BaseModel, Field, ValidationInfo, field_validator, model_validator + +from deerflow.config._boolean_guards import reject_boolean ContextSizeType = Literal["fraction", "tokens", "messages"] DEFAULT_SKILL_FILE_READ_TOOL_NAMES: tuple[str, ...] = ("read_file", "read", "view", "cat") @@ -88,10 +90,8 @@ class SummarizationConfig(BaseModel): @field_validator("trim_tokens_to_summarize", mode="before") @classmethod - def _reject_boolean_trim_tokens(cls, value: object) -> object: - if isinstance(value, bool): - raise ValueError("must be an integer, not a boolean") - return value + def _reject_boolean_trim_tokens(cls, value: object, info: ValidationInfo) -> object: + return reject_boolean(value, info, kind="an integer") summary_prompt: str | None = Field( default=None, diff --git a/backend/packages/harness/deerflow/config/title_config.py b/backend/packages/harness/deerflow/config/title_config.py index 679183e6ce..7a21dc6994 100644 --- a/backend/packages/harness/deerflow/config/title_config.py +++ b/backend/packages/harness/deerflow/config/title_config.py @@ -2,6 +2,8 @@ from pydantic import BaseModel, Field, ValidationInfo, field_validator +from deerflow.config._boolean_guards import reject_boolean + class TitleConfig(BaseModel): """Configuration for automatic thread title generation.""" @@ -34,11 +36,7 @@ class TitleConfig(BaseModel): @field_validator("max_words", "max_chars", mode="before") @classmethod def _reject_boolean_limits(cls, value: object, info: ValidationInfo) -> object: - # YAML `true`/`false` next to neighbouring boolean knobs must not coerce - # into integers (Pydantic would turn `true` into 1 before ge/le runs). - if isinstance(value, bool): - raise ValueError(f"{info.field_name} must be an integer, not a boolean") - return value + return reject_boolean(value, info, kind="an integer") # Global configuration instance diff --git a/backend/tests/test_boolean_guards.py b/backend/tests/test_boolean_guards.py new file mode 100644 index 0000000000..afbf82fc79 --- /dev/null +++ b/backend/tests/test_boolean_guards.py @@ -0,0 +1,34 @@ +"""Direct tests for the shared boolean-rejection helper.""" + +from typing import cast + +import pytest +from pydantic import ValidationInfo + +from deerflow.config._boolean_guards import reject_boolean + + +class _Probe: + """Minimal harness so ``info.field_name`` resolves like a real field.""" + + def __init__(self, field_name: str) -> None: + self.field_name = field_name + + +def _info(field_name: str) -> ValidationInfo: + return cast(ValidationInfo, _Probe(field_name)) + + +def test_reject_boolean_names_the_field() -> None: + with pytest.raises(ValueError, match="recursion_limit must be an integer, not a boolean"): + reject_boolean(True, _info("recursion_limit"), kind="an integer") + + +def test_reject_boolean_passes_numbers_through() -> None: + assert reject_boolean(0, _info("hard_limit"), kind="an integer") == 0 + assert reject_boolean(2.5, _info("interval"), kind="a number") == 2.5 + + +def test_reject_boolean_custom_kind_wording() -> None: + with pytest.raises(ValueError, match="command_timeout must be a number of seconds or null, not a boolean"): + reject_boolean(True, _info("command_timeout"), kind="a number of seconds or null")