fix(review): W1 ServerConfig from_dict wiring, W3 internal kwargs filter, N3 status docstring
Test / backend-test (pull_request) Has been cancelled Details
Test / frontend-unit (pull_request) Has been cancelled Details
Test / api-e2e (pull_request) Has been cancelled Details
Test / frontend-e2e (pull_request) Has been cancelled Details

Code review fixes for Wave 1:
- W1: ServerConfig.from_dict now wires prompt_cache/streaming/verification sections
  from YAML to constructor (previously these params existed but were never read)
- W3: Tool._validate_input filters _-prefixed kwargs (e.g. _skip_dangerous_check)
  before jsonschema.validate, preventing additionalProperties:false schemas from
  rejecting internal control parameters
- N3: ReActResult.status docstring now lists "empty_fallback" and "verify_failed"

Added test test_internal_kwargs_underscore_prefixed_skipped_by_validation for W3.
This commit is contained in:
chiguyong 2026-06-29 21:58:40 +08:00
parent cd211c6cd9
commit d7ca6e8065
4 changed files with 31 additions and 2 deletions

View File

@ -119,7 +119,9 @@ class ReActResult:
trajectory: list[ReActStep] trajectory: list[ReActStep]
total_steps: int total_steps: int
total_tokens: int total_tokens: int
status: str = "success" # "success" | "timeout" | "cancelled" | "partial" status: str = (
"success" # "success"|"timeout"|"cancelled"|"partial"|"empty_fallback"|"verify_failed"
)
fallback_strategy: str | None = None # e.g. "simplified_rewoo", "react", "direct" fallback_strategy: str | None = None # e.g. "simplified_rewoo", "react", "direct"

View File

@ -236,6 +236,11 @@ class ServerConfig:
# Board meeting config (max_rounds, default_template, etc.) # Board meeting config (max_rounds, default_template, etc.)
board_data = data.get("board", {}) board_data = data.get("board", {})
# U2/U3/U4: prompt_cache / streaming / verification 配置(从 YAML 读取)
prompt_cache_data = data.get("prompt_cache", {})
streaming_data = data.get("streaming", {})
verification_data = data.get("verification", {})
return cls( return cls(
host=server.get("host", "0.0.0.0"), host=server.get("host", "0.0.0.0"),
port=server.get("port", 8001), port=server.get("port", 8001),
@ -263,6 +268,9 @@ class ServerConfig:
evolution=evolution_data, evolution=evolution_data,
expert_paths=expert_paths, expert_paths=expert_paths,
board=board_data, board=board_data,
prompt_cache=prompt_cache_data,
streaming=streaming_data,
verification=verification_data,
) )
@staticmethod @staticmethod

View File

@ -108,8 +108,11 @@ class Tool(ABC):
""" """
if self.input_schema is None: if self.input_schema is None:
return return
# 过滤内部 kwargs(下划线开头,如 _skip_dangerous_check),不参与 schema 校验
# W3 fix: 防止 additionalProperties:false 的 schema 拒绝内部控制参数
user_kwargs = {k: v for k, v in kwargs.items() if not k.startswith("_")}
try: try:
jsonschema.validate(instance=kwargs, schema=self.input_schema) jsonschema.validate(instance=user_kwargs, schema=self.input_schema)
except jsonschema.ValidationError as e: except jsonschema.ValidationError as e:
field_path = ".".join(str(p) for p in e.absolute_path) or "<root>" field_path = ".".join(str(p) for p in e.absolute_path) or "<root>"
# required 缺失走 schema_mismatch;类型不符走 tool_call_invalid # required 缺失走 schema_mismatch;类型不符走 tool_call_invalid

View File

@ -122,3 +122,19 @@ async def test_structured_error_dict_str_includes_error_code_for_llm_self_correc
assert msg["role"] == "tool" assert msg["role"] == "tool"
assert msg["tool_call_id"] == "t1" assert msg["tool_call_id"] == "t1"
assert "tool_call_invalid" in msg["content"] assert "tool_call_invalid" in msg["content"]
# ---- W3 fix: 内部 kwargs(_前缀)不参与 schema 校验 ----
async def test_internal_kwargs_underscore_prefixed_skipped_by_validation():
"""_skip_dangerous_check 等内部参数不参与 schema 校验。
防止 additionalProperties:false schema 拒绝内部控制参数(W3)
"""
tool = _StubTool(schema=_SCHEMA) # additionalProperties: False
# _skip_dangerous_check 是内部参数,不应被 schema 拒绝
result = await tool.safe_execute(count=5, _skip_dangerous_check=True)
assert result == {"ok": True}
# execute 仍然收到完整 kwargs(内部参数透传给工具)
assert tool.calls == [{"count": 5, "_skip_dangerous_check": True}]