fix(tools): 修掉两个假阳性与一个改得动的内部状态,按两轮独立评审
代码审查(新鲜上下文,只给 diff 与验收标准)报了五条影响正确性的,逐条核实全部成立:
1. spec_for() 交出去的 parameters 就是注册表内部那份真字典。docstring 承诺的快照只挡住了
「调用方改自己那份」,没挡住「从注册表取出来往里伸一层改」——而后者一下同时改掉模型
看见的 schema 和校验用的 schema。改成逐层冻成只读视图,schema_for_model 出口再化回
普通字典与列表。
2. {type: integer} 拒掉 3.0。JSON Schema draft-06 起小数部分为零的浮点数是合法整数,
模型写 1e2 时 json.loads 给的就是 float。这是我自己在注释里点名最怕的那种假阳性。
3. additionalProperties: false 撞上 patternProperties 时拒掉一切匹配 pattern 的键。
那些正是这份 schema 专门要收的键,模型改名也绕不过去。patternProperties 在场就跳过。
4. 工具名不校验类型、纯空白名放行。名字要落进发给模型的 schema,不是字符串会让整个请求
被网关拒掉,报错指向请求体不指向注册表。
5. _by_name 是可变 dict,两条查询路径能被就地改到分岔。换成只读视图。
不可哈希那条不修,改在 docstring 里写明(参数 schema 是映射,注册表放不进 set)。
scope.md 的行号引用换成条目名——行号是最容易漂的一种参数,插一行就静默指错。
0008 按一轮硕士生冷读重写:字段位置那段原来自相矛盾(一边说位置是公共承诺、插在中间会
静默改掉后面字段的含义,一边就插在中间,且没讨论追加在末尾这个同一判据下的显然选项),
改成追加在末尾并说明规则;补上四个名字的就地解释(重放策略、两条完成通路、动作结果五个
字段、restrict_to);决策四那张表原来只有三列却被正文说成填五个字段,恒定的两个单列出来
并各自给了理由;补上 validate 不通过为什么算未执行、executor() 为什么全查、为什么必须是
具体类而不是闭包、异常栈去哪了。
This commit is contained in:
@@ -3,9 +3,10 @@
|
||||
读者是给库注册工具的人,以及循环里要问「这个工具声明了什么」的三个纯逻辑模块。
|
||||
|
||||
**注册、模型可见 schema 的生成、存在性与参数校验、分发——四者由同一个注册表实例驱动**
|
||||
(`research-wiki/explanation/scope.md` 第 68 行那条要求)。不同源就会漂移:模型看见一个已经
|
||||
删掉的工具,或者校验放行了一个分发时找不到的名字。重放策略与完成标记同住这里,理由一样
|
||||
——它们都是「关于某个工具的一条事实」,分开存就会跟工具清单漂移。
|
||||
(`research-wiki/explanation/scope.md` 界内清单里「工具的注册、模型可见 schema 生成、存在性
|
||||
与参数校验、分发」那一条)。不同源就会漂移:模型看见一个已经删掉的工具,或者校验放行了
|
||||
一个分发时找不到的名字。重放策略与完成标记同住这里,理由一样——它们都是「关于某个工具的
|
||||
一条事实」,分开存就会跟工具清单漂移。
|
||||
|
||||
注册表是**不可变值对象**,取子集返回新实例,不是进程级单例:同一进程里可能同时持有多份
|
||||
不同的窄集合(`research-wiki/design/0003-public-api-shape.md` 决策二)。
|
||||
@@ -17,9 +18,9 @@
|
||||
已经能用了。
|
||||
"""
|
||||
|
||||
import copy
|
||||
from collections.abc import Collection, Iterable, Mapping, Sequence
|
||||
from dataclasses import dataclass
|
||||
from types import MappingProxyType
|
||||
|
||||
from polyloop.ports import ToolCall
|
||||
from polyloop.types import ReplayPolicy
|
||||
@@ -39,6 +40,33 @@ class ToolValidationError(ValueError):
|
||||
"""
|
||||
|
||||
|
||||
def _frozen(value: object) -> object:
|
||||
"""把一份 JSON Schema 逐层变成改不动的形状:映射变只读视图,列表变元组。
|
||||
|
||||
只冻最外面一层不够。真正会发生的改法是从注册表里把规格取出来、往里伸一层去改
|
||||
(`spec_for("read").parameters["properties"]["path"]["type"] = ...`),那一下同时改掉了
|
||||
模型看见的 schema 和校验用的 schema,而这次修改没有任何地方记录得到——事后翻轨迹,
|
||||
模型当时到底看见的是哪一份,查不出来。
|
||||
"""
|
||||
if isinstance(value, Mapping):
|
||||
return MappingProxyType({key: _frozen(item) for key, item in value.items()})
|
||||
if isinstance(value, list | tuple):
|
||||
return tuple(_frozen(item) for item in value)
|
||||
return value
|
||||
|
||||
|
||||
def _plain(value: object) -> object:
|
||||
"""把冻过的形状变回普通字典与列表。
|
||||
|
||||
交给模型的那份 schema 要能直接 `json.dumps`,而只读视图与元组里只有元组能被序列化。
|
||||
"""
|
||||
if isinstance(value, Mapping):
|
||||
return {key: _plain(item) for key, item in value.items()}
|
||||
if isinstance(value, tuple):
|
||||
return [_plain(item) for item in value]
|
||||
return value
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class ToolSpec:
|
||||
"""一个工具的全部声明。
|
||||
@@ -46,9 +74,8 @@ class ToolSpec:
|
||||
`parameters` 是一份普通的 JSON Schema 字典,不是任何第三方库的模型对象——签名上一旦出现
|
||||
第三方类型,那个包的 major 就是我们的 major。
|
||||
|
||||
构造时 `parameters` 会被深拷贝一份存下来。调用方传进来的那个字典之后再被改,注册表看见
|
||||
的仍是注册那一刻的形状;不拷贝的话,「模型看见的 schema」和「校验用的 schema」会随调用方
|
||||
在别处的一次修改一起变,而那次修改没有任何地方记录得到。
|
||||
构造时 `parameters` 会被逐层冻成只读的形状存下来,两个方向都堵上:调用方传进来的那个
|
||||
字典之后再被改,注册表看见的仍是注册那一刻的形状;从注册表里把规格取出来往里改,改不动。
|
||||
"""
|
||||
|
||||
name: str
|
||||
@@ -70,11 +97,15 @@ class ToolSpec:
|
||||
|
||||
用显式异常而不是 `assert`:`python -O` 会把断言整条移除(`CLAUDE.md` §6)。
|
||||
"""
|
||||
if not self.name:
|
||||
raise ValueError("工具名不能为空串:空名字在提示词里不可见,模型永远调不到它")
|
||||
if not isinstance(self.name, str):
|
||||
raise TypeError(f"工具名必须是字符串,收到 {type(self.name).__name__}")
|
||||
if not self.name.strip():
|
||||
raise ValueError("工具名不能是空串或纯空白:这种名字在提示词里不可见,模型永远调不到它")
|
||||
if not isinstance(self.description, str):
|
||||
raise TypeError(f"工具说明必须是字符串,收到 {type(self.description).__name__}")
|
||||
if not isinstance(self.parameters, Mapping):
|
||||
raise TypeError(f"parameters 必须是一份映射,收到 {type(self.parameters).__name__}")
|
||||
object.__setattr__(self, "parameters", copy.deepcopy(dict(self.parameters)))
|
||||
object.__setattr__(self, "parameters", _frozen(dict(self.parameters)))
|
||||
|
||||
|
||||
def _matches_one_json_type(value: object, type_name: str) -> bool:
|
||||
@@ -90,7 +121,14 @@ def _matches_one_json_type(value: object, type_name: str) -> bool:
|
||||
return isinstance(value, bool)
|
||||
if type_name == "integer":
|
||||
# 布尔在 Python 里是整数的子类,而 JSON 里不是。不排掉的话 `True` 会被判成合法的整数。
|
||||
return isinstance(value, int) and not isinstance(value, bool)
|
||||
if isinstance(value, bool):
|
||||
return False
|
||||
if isinstance(value, int):
|
||||
return True
|
||||
# JSON Schema draft-06 起,小数部分为零的浮点数是合法的整数。模型写出 `1e2` 或者
|
||||
# `3.0`,`json.loads` 给的就是 float——照「必须是 int」判会拒掉一次合法调用,而模型
|
||||
# 怎么改都过不去。
|
||||
return isinstance(value, float) and value.is_integer()
|
||||
if type_name == "number":
|
||||
return isinstance(value, int | float) and not isinstance(value, bool)
|
||||
if type_name == "string":
|
||||
@@ -124,6 +162,9 @@ class ToolRegistry:
|
||||
**相等按「注册了哪些规格、什么顺序」判,不按对象身份判。** 构造 `RunRequest` 时要比对
|
||||
「执行器持有的注册表」和「本次可见的注册表」是不是同一份,两份内容相同的注册表在模型
|
||||
看见的 schema 与实际分发上完全一致,没有可失败的地方,按身份判会把它们错判成冲突。
|
||||
|
||||
**不可哈希**,因为参数 schema 是映射。放进 `set` 或者拿它当字典键会抛 `TypeError`,
|
||||
要按注册表分组的话用 `names()` 那份元组当键。
|
||||
"""
|
||||
|
||||
__slots__ = ("_by_name", "_specs")
|
||||
@@ -142,7 +183,9 @@ class ToolRegistry:
|
||||
by_name[spec.name] = spec
|
||||
ordered.append(spec)
|
||||
self._specs: tuple[ToolSpec, ...] = tuple(ordered)
|
||||
self._by_name: dict[str, ToolSpec] = by_name
|
||||
# 只读视图而不是那个 dict 本身:两条查询路径(`names`/`schema_for_model` 走 `_specs`,
|
||||
# `spec_for`/`validate` 走这里)一旦有一条被就地改过,四者同源当场破掉。
|
||||
self._by_name: Mapping[str, ToolSpec] = MappingProxyType(by_name)
|
||||
|
||||
def __eq__(self, other: object) -> bool:
|
||||
if not isinstance(other, ToolRegistry):
|
||||
@@ -206,7 +249,7 @@ class ToolRegistry:
|
||||
{
|
||||
"name": spec.name,
|
||||
"description": spec.description,
|
||||
"parameters": copy.deepcopy(dict(spec.parameters)),
|
||||
"parameters": _plain(spec.parameters),
|
||||
}
|
||||
for spec in self._specs
|
||||
]
|
||||
@@ -245,7 +288,11 @@ class ToolRegistry:
|
||||
if missing:
|
||||
raise ToolValidationError(f"{call.name!r} 缺少必填参数:{missing}")
|
||||
|
||||
if schema.get("additionalProperties") is False:
|
||||
# `patternProperties` 在场时,「哪些键是被声明过的」要靠正则匹配才答得出,而这里
|
||||
# 不实现正则匹配那一档。跳过这项检查,不拿一份答不出的问题去拒调用——匹配到 pattern
|
||||
# 的键必然不在 `properties` 里,照下面那行判会把每一次合法调用都拒掉,而模型改名字
|
||||
# 也绕不过去。
|
||||
if schema.get("additionalProperties") is False and "patternProperties" not in schema:
|
||||
unknown = sorted(key for key in call.arguments if key not in properties)
|
||||
if unknown:
|
||||
raise ToolValidationError(
|
||||
|
||||
Reference in New Issue
Block a user