Repository navigation
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
AI Code Review审查结论不通过 审查范围:base b33f164..head 8b9115c,共 28 个新增文件、3374 行,实现「支持 systemone model 调用」。计划要求:SystemOneClient 提供 typesafe(官方 SDK,必须带 key/model)与 laya(自建 JSON,可省略 key/model、支持批量)两个后端,发请求前校验参数、返回后核对答案、统一 TypeSafeError 异常体系、typesafe 可选依赖 extra。主体实现结构清晰、设计文档与实现基本对齐:URL/API key 校验、4MiB 限长读取、错误正文不进 str(error)、超时/取消、外部 HTTP client 保持打开等均有测试。\n\n主要风险与测试问题(均为本变更引入):\n1. SEVERE:tests/models/systemone/native/ 4 个测试文件约 760 行引用不存在的 trpc_agent_sdk.models.systemone.native 子包,同提交内 typesafe/ 测试又用官方 SDK 实现同一功能,两套测试互相矛盾;CI 全量 pytest 会因 ImportError 直接红,importorskip 无法掩盖。\n2. SEVERE:check_answers 用 isinstance 判断 ChoiceAnswer 类型,SDK 的多重继承答案类型(Choice+Score)会落入 _check_choice 并抛 AttributeError 而非 SystemOneError,违反统一异常契约。\n3. MODERATE:encode_questions 对非映射问题值不校验,坏参数会发出请求后才在服务端报错,违反「发请求前拦截」承诺。\n4. MODERATE:_read_limited 超限后不关闭流,4MiB 限长只保护本地拼接、不保护网络读取,巨型/恶意响应仍可无限读取。\n5. MODERATE:_payload 用 if model: 真值判断省略模型,合法模型名 "0" 被静默丢弃,与「None/'' 才省略」契约不符。\n6. MODERATE:timeout==0 时 _wait 不做 wait_for 兜底、HTTP 层无期限,调用可无限挂起,与文档「无截止时间」的描述冲突。\n7. LOW:validate_base_url 截尾与百分号检测顺序导致 %zz+尾斜杠叠加输入误判。\n8. LOW:pyproject.toml 新增 typesafe extra 未同步进 requirements-test.txt,CI 测试环境缺 typesafe-sdk/httpx2,全量测试收集失败或覆盖率误判。\n\n计划符合性:功能主路径(typesafe/laya 双后端、批量、答案核对、异常映射、可选依赖)与设计文档一致,但测试与实现的分离(native 遗留测试)和上述边界缺陷表明提交未完全满足「发请求前拦截一切坏参数、统一异常」的质量要求。门禁结论:FAILED(存在 2 个 SEVERE)。 发现的问题严重
问题: 本提交新增的 触发条件: 在安装了 实际影响: CI 全量测试直接红( 修正方向: 删除 严重
问题: 触发条件: Laya/官方后端返回的类型联合体答案(如 实际影响: 抛出的不是统一异常体系内的 修正方向: 在 中等
问题: 触发条件: Laya 调用方传入 实际影响: 本应本地校验拒绝的请求被发送,服务端返回 422/400 后 mapped 为 修正方向: 在 中等
问题: 触发条件: Laya 服务器返回超过 4MiB 的成功响应体(或分块传输的巨型响应),客户端在 untrusted 服务器/慢网络下持续读取。 实际影响: 限长保护被绕过:连接保持打开继续消耗带宽和事件循环时间,攻击或异常响应可拖垮客户端任务,直到超时/取消才终止; 修正方向: 在 中等
问题: 触发条件: Laya 调用方传 实际影响: 合法的非空模型名被静默丢弃,服务端按默认 checkpoint 处理,客户预期落空且无报错,属于数据正确性偏差。 修正方向: 中等
问题: 触发条件: 调用方使用默认 实际影响: 对无限等待的服务器,调用永久挂起、连接泄漏,与设计文档「防止外部服务阻塞耗尽资源」的目标相悖; 修正方向: 要么文档明确「0 = 完全无保护、仅 HTTP 层不设期限」,要么在 较低
问题: 触发条件: 调用方传 实际影响: 合法带尾斜杠的 URL(如内部网关前缀 修正方向: 先 较低问题: 本提交新增的 15 个测试文件全部依赖 触发条件: CI 自托管 runner 上未预装 实际影响: 全量测试收集即失败(typesafe 目录无 conftest 跳过逻辑之外的兜底,native 目录更是直接 import 失败),CI 门禁对本次变更失守;即使 runner 恰好预装了依赖,依赖未声明也导致本地开发者无法复现测试环境。 修正方向: 在 |
| _check_choice(name, _choice_labels(name, question), answer) | ||
| elif isinstance(answer, ScoreAnswer): | ||
| _check_score(name, _score_levels(name, question), answer) | ||
|
|
||
|
|
||
| def _validate_question(name: str, question: Question) -> None: | ||
| if isinstance(question, _QUESTION_TYPES): | ||
| _reject_blank_text(name, getattr(question, "instructions", None)) | ||
| if isinstance(question, Choice) and not question.criteria: | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| if isinstance(question, Score) and not question.criteria: | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| return | ||
| if not isinstance(question, dict) or not isinstance(question.get("type"), str) or not question["type"]: | ||
| raise SystemOneError(f"systemone: question {name!r}: expected a question object or a typed dictionary") | ||
| _reject_blank_text(name, question.get("instructions")) | ||
| kind = question["type"] | ||
| if kind == "choice" and not _mapping(question.get("criteria")): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| if kind == "score" and not _sequence(question.get("criteria")): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
|
|
||
|
|
||
| def _reject_blank_text(name: str, instructions: object) -> None: | ||
| if isinstance(instructions, str) and instructions.strip() == "": | ||
| raise SystemOneError(f"systemone: question {name!r}: instructions must not be blank") | ||
|
|
||
|
|
||
| def _question_kind(name: str, question: Question) -> str: | ||
| if isinstance(question, _QUESTION_TYPES): | ||
| return question.type | ||
| if isinstance(question, dict) and isinstance(question.get("type"), str): | ||
| return question["type"] | ||
| raise SystemOneError(f"systemone: question {name!r}: expected a question object or a typed dictionary") | ||
|
|
||
|
|
||
| def _choice_labels(name: str, question: Question) -> set[str]: | ||
| criteria = question.criteria if isinstance( | ||
| question, Choice) else question.get("criteria") if isinstance(question, dict) else None | ||
| if not isinstance(criteria, Mapping): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| return {label for label in criteria if isinstance(label, str)} | ||
|
|
||
|
|
||
| def _score_levels(name: str, question: Question) -> int: | ||
| criteria = question.criteria if isinstance( | ||
| question, Score) else question.get("criteria") if isinstance(question, dict) else None | ||
| if not _sequence(criteria): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| return len(criteria) | ||
|
|
||
|
|
||
| def _check_choice(name: str, labels: set[str], answer: ChoiceAnswer) -> None: | ||
| if answer.choice not in labels: |
There was a problem hiding this comment.
问题: _validate.py 的 check_answers 用 isinstance(answer, ChoiceAnswer) 判断答案类型并进入 _check_choice,但 SDK 的 ChoiceAnswer 可能同时是 ScoreAnswer(同目录 typesafe 测试 test_answers.py 对 score 类型答案调用 choice_answer 时也出现双重类型)。一旦某答案同时满足两个类型,_check_choice 读 answer.choice 会抛 AttributeError(_validate.py:118, 171),而不是契约规定的 SystemOneError。
触发条件: Laya/官方后端返回的类型联合体答案(如 ChoiceAnswer(ScoreAnswer))命中 isinstance(answer, ChoiceAnswer) 分支,但 answer.choice 不存在。
实际影响: 抛出的不是统一异常体系内的 TypeSafeError 子类,调用方 except TypeSafeError 无法接住,得到难以诊断的裸 AttributeError 崩溃。
修正方向: 在 _check_choice 中对 answer.choice/answer.probabilities 先做 hasattr 或类型精确匹配(比较 type(answer) 而非 isinstance),把缺字段转化为 SystemOneError。
| def encode_questions(questions: Mapping[str, Question]) -> dict[str, object]: | ||
| """Serialize question objects to the JSON object Laya expects.""" | ||
| encoded: dict[str, object] = {} | ||
| for name, question in questions.items(): | ||
| if isinstance(question, BaseModel): | ||
| encoded[name] = question.model_dump(mode="json", exclude_none=True) | ||
| elif isinstance(question, dict): | ||
| encoded[name] = question | ||
| else: | ||
| raise SystemOneError(f"systemone: question {name!r}: expected a question object or a typed dictionary") | ||
| return encoded |
There was a problem hiding this comment.
问题: encode_questions(_laya.py:50-59)只校验 BaseModel 和 dict,非映射值(如列表)既不抛错也不处理;validate_questions 也只做 up-front 检查,不验证问题值是映射。只有 _dumps 的 json.dumps 在发送前兜底报错。契约(设计文档)明确「发请求之前校验失败都是 SystemOneError,请求不会发出」——这里仍满足不发送,但列表值被 json.dumps 合法编码后会在服务端产生 4xx,调用方收到的是 HTTP 错误而非本地 SystemOneError,与「坏参数不发请求」的承诺不符。
触发条件: Laya 调用方传入 questions = {"q": ["a"]} 这类非映射值。
实际影响: 本应本地校验拒绝的请求被发送,服务端返回 422/400 后 mapped 为 TypeSafeUnprocessableEntityError,调用方错误分类错误、排查困难,且多消耗一次网络往返。
修正方向: 在 encode_questions 的 else 分支(或 validate_questions)对非 BaseModel/dict 的值抛出 SystemOneError(当前 else 分支已存在但未命中列表值,需补充 isinstance 检查)。
| size = 0 | ||
| try: | ||
| async for chunk in response.aiter_bytes(): | ||
| room = limit - size | ||
| if len(chunk) > room: | ||
| parts.append(chunk[:room]) | ||
| size = limit | ||
| break | ||
| parts.append(chunk) | ||
| size += len(chunk) | ||
| if size >= limit: | ||
| break | ||
| except Exception as exc: | ||
| return b"".join(parts), exc | ||
| return b"".join(parts), None | ||
|
|
||
|
|
||
| def _model(model: Optional[str], default_model: Optional[str]) -> Optional[str]: | ||
| """Choose the model to send. |
There was a problem hiding this comment.
问题: _read_limited(_backends.py:221-239)在 len(chunk) > room 时只把 parts.append(chunk[:room]) 后 break,但流式响应未被真正截断——httpx2 的 aiter_bytes() 循环虽退出,底层连接仍在接收剩余字节,服务器只要持续写入即可无界地输出超过 4MiB 的数据。设计文档声称「成功正文超出 4 MiB 时抛出 SystemOneError」,但限长只保护了内存拼接,没有保护网络读取。
触发条件: Laya 服务器返回超过 4MiB 的成功响应体(或分块传输的巨型响应),客户端在 untrusted 服务器/慢网络下持续读取。
实际影响: 限长保护被绕过:连接保持打开继续消耗带宽和事件循环时间,攻击或异常响应可拖垮客户端任务,直到超时/取消才终止;MAX_RESPONSE_BYTES 的「有界响应」承诺落空。
修正方向: 在 _read_limited 中读取到 limit 后立即调用 response.aclose()(或直接在中止字节处 raise 并关闭流),确保连接不再接收更多字节。
| async def _wait(self, awaitable: Awaitable[object]) -> object: | ||
| if self._timeout <= 0: | ||
| return await awaitable | ||
| try: | ||
| return await asyncio.wait_for(awaitable, self._timeout) | ||
| except TypeSafeAPITimeoutError: | ||
| raise | ||
| except TimeoutError as exc: | ||
| raise TypeSafeAPITimeoutError(self._timeout) from exc |
There was a problem hiding this comment.
问题: SystemOneClient._wait(_client.py:176-184)对 timeout == 0 直接 return await awaitable 且不设截止时间,_http_timeout(0) 返回 httpx2.Timeout(None)(_backends.py:253-260)。设计文档声称「timeout == 0 表示这次调用没有截止时间」,但文档后又写「HTTP 层使用同一个秒数」「没有期限写在无期限的 HTTP timeout 上」,实际语义是「无 HTTP 截止时间、无 wait_for 兜底」,与 timeout > 0 时「wait_for 兜底 + HTTP 同秒数」的契约不一致,且没有任何兜底保护。
触发条件: 调用方使用默认 timeout=30 时正常;一旦显式传 timeout=0,任何永不返回的服务器响应都会让调用无限阻塞。
实际影响: 对无限等待的服务器,调用永久挂起、连接泄漏,与设计文档「防止外部服务阻塞耗尽资源」的目标相悖;timeout=0 与文档描述不符(文档暗示仍有 guard 的 wait_for 语义)。
修正方向: 要么文档明确「0 = 完全无保护、仅 HTTP 层不设期限」,要么在 _wait 对 timeout==0 仍用无限 wait_for(asyncio.wait_for(awaitable, None))并让 HTTP 层无期限。
| if not isinstance(base_url, str) or not base_url or any(char in base_url for char in "?#"): | ||
| raise SystemOneError("systemone: invalid base url") | ||
| if _INVALID_PERCENT.search(base_url): | ||
| raise SystemOneError("systemone: invalid base url") | ||
| try: | ||
| parts = urlsplit(base_url) | ||
| except ValueError as exc: | ||
| raise SystemOneError("systemone: invalid base url") from exc | ||
| if parts.scheme not in {"http", "https"} or not parts.hostname: | ||
| raise SystemOneError("systemone: invalid base url") | ||
| if parts.username is not None or parts.password is not None: | ||
| raise SystemOneError("systemone: invalid base url") | ||
| return parts.geturl().rstrip("/") |
There was a problem hiding this comment.
问题: validate_base_url(_validate.py:49-67)先做字符检测(?#、百分号不完整 %),后 parts.geturl().rstrip("/") 截尾。对 "https://host/%zz/" 这类「百分号不完整且带尾斜杠」的输入,%zz 检测发生在截尾之前,会误判整段 URL 非法而拒绝——反之亦然:先截尾后检测才能正确归类。当前测试 test_invalid_base_url 只覆盖了 "https://host/%zz"(无尾斜杠),未覆盖两种畸形叠加的情况。
触发条件: 调用方传 base_url 同时含不完整百分号编码和尾斜杠。
实际影响: 合法带尾斜杠的 URL(如内部网关前缀 https://host/gateway/)与畸形 URL 混在一个检测链里,行为依赖顺序;若未来把截尾提前,%zz 检测即失效。当前实现下,合法输入 https://host/gateway/%2f/ 等也被错误拒绝(%2f 是合法编码却被 % 正则误判)。
修正方向: 先 rstrip("/") 再去尾斜杠后再做百分号/字符检测,并补充 %2f 这类合法转义与尾斜杠叠加的测试用例。
- typesafe_sdk: official jev model - custom http: laya
8b9115c to
bac1740
Compare
AI Code Review审查结论不通过 本次审查覆盖 git b33f164..bac1740(feat(model): 支持 systemone model 调用),共 23 个文件、2548 行纯新增。核心是新增 trpc_agent_sdk/models/systemone/ 模块(SystemOneClient 双后端:官方 TypeSafe API + 本地 Laya HTTP),配套 4 个测试文件、示例与设计文档。整体实现与设计文档高度一致:参数校验、答案核对、超时/错误契约、批量限制(1-64)、资源生命周期均实现得细致且有较充分的测试(19 个用例覆盖往返、错误映射、字节限长、取消、关闭语义)。主要风险集中在三处:(1) Python 3.10 支持声明(pyproject 发现的问题严重
问题: 触发条件: 在 Python 3.10 上运行,且整次调用(后端请求 + 实际影响: 裸 修正方向: 将 中等
问题: 触发条件: Laya 批量响应中任一条含 实际影响: 裸 修正方向: 在 中等
问题: 触发条件: Laya 返回 2xx 响应头后、正文未读完整即断连或读取超时(响应很大被截断、服务端异常关闭、网络抖动)。 实际影响: 违反设计文档契约(docs/design/02-systemone-design.md §2.5):「HTTP 层超时……都是 修正方向: 在 中等
问题: 触发条件: 用户按文档 §1.1 声称的「未安装时,导入 实际影响: 与设计文档(02-systemone-design.md §1.1、§2.6)明确承诺的行为直接矛盾;也偏离仓库既有惯例—— 修正方向: 在 中等
问题: 字典形式 question 的 触发条件: 调用方传入 实际影响: 违反「发出去之前就拦住坏参数」的设计承诺(docs/design/02-systemone-design.md §2.2/§2.5:「这些检查失败都是 SystemOneError,请求不会发出」):请求被打到服务器(浪费配额并可能暴露请求内容),服务器返回 4xx 时用户看到的是 修正方向: 在 较低
问题: 触发条件: 调用方通过 实际影响: 修正方向: 在 较低
问题: 触发条件: Laya 后端 + 传入含不可序列化字段的 Pydantic 模型作为 实际影响: 裸 pydantic 序列化异常泄漏给调用方,违反「失败都是 TypeSafeError 子类」的契约;而同一 修正方向: 将 较低
问题: 触发条件: 实际影响: 错误信息误导——对 TypeSafe 后端 1..64 条的本地限制检查毫无意义,调用方会误以为调整批量大小即可,实际需要改用 Laya 后端;两处均为 修正方向: 将 较低
问题: README 第 55 行称「安装项目时已包含 触发条件: 按 README 步骤克隆、 实际影响: 示例在 import 阶段因 修正方向: 将第 55 行改为明确「需要执行 |
| async def _wait(self, awaitable: Awaitable[object]) -> object: | ||
| if self._timeout <= 0: | ||
| return await awaitable | ||
| try: | ||
| return await asyncio.wait_for(awaitable, self._timeout) | ||
| except TypeSafeAPITimeoutError: | ||
| raise | ||
| except TimeoutError as exc: | ||
| raise TypeSafeAPITimeoutError(self._timeout) from exc |
There was a problem hiding this comment.
问题: _wait 用 except TimeoutError 捕获 asyncio.wait_for 的超时异常,但项目声明支持 Python 3.10(pyproject.toml requires-python = ">=3.10"、README「Python3.10+」),而 3.10 中 asyncio.wait_for 超时抛的是 asyncio.TimeoutError(即 concurrent.futures.TimeoutError 的别名),它并不继承内置 TimeoutError(该别名关系从 Python 3.11 才建立)。本环境实测:issubclass(asyncio.TimeoutError, TimeoutError) 为 False,except TimeoutError 捕获不到。
触发条件: 在 Python 3.10 上运行,且整次调用(后端请求 + check_answers)超过 timeout 秒,使 asyncio.wait_for 先于 HTTP 层超时触发(HTTP 层与 wait_for 使用同一秒数,但 wait_for 包裹整体调用,响应接近超时时必先触发)。typesafe-sdk 0.7.5 与 httpx2 2.14.0 均支持 3.10,该场景可达。
实际影响: 裸 asyncio.TimeoutError 直接泄漏给调用方,而不是文档承诺的 TypeSafeAPITimeoutError(docs/design/02-systemone-design.md 明确「HTTP 层超时和 wait_for 超时都是 TypeSafeAPITimeoutError」)。调用方按文档写 except TypeSafeError 接不住超时,异常冒泡导致未处理崩溃,且错误类型契约在两个后端间不一致。
修正方向: 将 _client.py:183 的 except TimeoutError 改为 except asyncio.TimeoutError(3.11+ 与内置 TimeoutError 同一对象,向下兼容),并补充一个用 MockTransport 慢 handler 但把 SystemOneClient._wait 超时调小到先于 HTTP 层触发的测试,覆盖 wait_for 超时分支。
| decoded = [] | ||
| for item in results: | ||
| if not isinstance(item, dict): | ||
| raise SystemOneError("systemone: batch response item is not an object") | ||
| decoded.append(decode_response(url, json.dumps(item, ensure_ascii=False).encode("utf-8"), headers)) | ||
| return decoded |
There was a problem hiding this comment.
问题: decode_batch 对每个结果项执行 json.dumps(item, ensure_ascii=False).encode("utf-8") 再交给 decode_response,当项内字符串含孤立代理项(\uD800-\uDFFF)时,.encode("utf-8") 抛 UnicodeEncodeError——它虽是 ValueError 子类,但该调用位于 _exchange 的 try 之外、decode_batch 内也无任何 try/except,异常裸泄漏。已实测确认:json.loads 接受 \ud800 合法转义(\ud800 在 JSON 字符串中是合法转义序列),而 json.dumps(..., ensure_ascii=False) 保持代理项后 .encode("utf-8") 必然抛错。
触发条件: Laya 批量响应中任一条含 \ud800-\udfff 转义(例如用户提交的 state 内容含该字符被服务端回显,或上游数据损坏)。
实际影响: 裸 UnicodeEncodeError 从 system_one_batch 一路泄漏,违反「失败都是 TypeSafeError 子类」的契约(_errors.py docstring),调用方按 except TypeSafeError 写的错误处理接不住;单发路径 _dumps 有 except (TypeError, ValueError) 兜底,批量路径却无,行为不对称。
修正方向: 在 decode_batch 的 item 重编码处用 try/except 包裹并转成 SystemOneError(与第 97 行批量 JSON 解析错误同风格),或直接复用已解析的 Python 对象构造响应而避免二次序列化。
| if read_error is not None: | ||
| raise SystemOneError("systemone: read response") from read_error |
There was a problem hiding this comment.
问题: _complete 在 HTTP 头已收到(2xx)但正文读取中途失败(连接被 reset、代理断流或读取阶段超时)时,把错误一律降级为 SystemOneError("systemone: read response")。而 _read_limited(212-235 行)捕获的是包括 httpx2.TimeoutException 在内的所有异常,读取超时也会走这条降级路径。
触发条件: Laya 返回 2xx 响应头后、正文未读完整即断连或读取超时(响应很大被截断、服务端异常关闭、网络抖动)。
实际影响: 违反设计文档契约(docs/design/02-systemone-design.md §2.5):「HTTP 层超时……都是 TypeSafeAPITimeoutError」「连接失败是 TypeSafeAPIConnectionError」。读取阶段的超时/连接失败被误报为「本地参数错误」SystemOneError,按文档写 except TypeSafeAPITimeoutError(如做重试)的调用方无法区分,错误语义错误且两个错误类型不可互换。
修正方向: 在 _complete(或 _read_limited)里区分 read_error 的最终类型:httpx2.TimeoutException 转 TypeSafeAPITimeoutError、其他连接类异常转 TypeSafeAPIConnectionError,仅把确属本地限制的情形留作 SystemOneError;并添加一个 MockTransport 中途断流/读超时的测试。
| from typesafe_sdk import Choice | ||
| from typesafe_sdk import ChoiceAnswer | ||
| from typesafe_sdk import Noul | ||
| from typesafe_sdk import NoulAnswer | ||
| from typesafe_sdk import NoulCriteria | ||
| from typesafe_sdk import RetryPolicy | ||
| from typesafe_sdk import Score | ||
| from typesafe_sdk import ScoreAnswer | ||
| from typesafe_sdk import SystemOneResponse | ||
| from typesafe_sdk import TypeSafeAPIError | ||
| from typesafe_sdk import TypeSafeError | ||
| from typesafe_sdk import Usage |
There was a problem hiding this comment.
问题: __init__.py 顶层裸 from typesafe_sdk import ...,未安装可选 extra 时抛裸 ModuleNotFoundError: No module named 'typesafe_sdk',没有任何安装提示;设计文档承诺的 _typesafe_sdk.py 再导出模块(「官方 SDK 的再导出;未安装时给出安装提示」)也未实现,ls trpc_agent_sdk/models/systemone/ 下只有 7 个文件。
触发条件: 用户按文档 §1.1 声称的「未安装时,导入 trpc_agent_sdk.models.systemone 会提示安装这个 extra」的预期,在未 pip install 'trpc-agent-py[typesafe]' 的环境下 from trpc_agent_sdk.models.systemone import SystemOneClient(或按 §2.6 从 _typesafe_sdk 导入 Box)。
实际影响: 与设计文档(02-systemone-design.md §1.1、§2.6)明确承诺的行为直接矛盾;也偏离仓库既有惯例——server/_a2a_detect.py、memory/hy_memory_service.py、models/_litellm_model.py、agents/__init__.py 对每个可选依赖都会重抛带 pip install 'trpc-agent-py[...]' 提示的 ImportError。用户面对裸 ModuleNotFoundError 无法知道如何安装,Box 等类型也没有文档所述的可达导入路径。
修正方向: 在 __init__.py 对 typesafe_sdk 的导入包一层 try/except ImportError,重抛带「pip install 'trpc-agent-py[typesafe]'」提示的 ImportError,并补建文档承诺的 _typesafe_sdk.py 再导出模块;同时补充一个未安装 SDK 时导入报错含提示的测试。
| if not isinstance(question, dict) or not isinstance(question.get("type"), str) or not question["type"]: | ||
| raise SystemOneError(f"systemone: question {name!r}: expected a question object or a typed dictionary") | ||
| _reject_blank_text(name, question.get("instructions")) | ||
| kind = question["type"] | ||
| if kind == "choice" and not _mapping(question.get("criteria")): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") | ||
| if kind == "score" and not _sequence(question.get("criteria")): | ||
| raise SystemOneError(f"systemone: question {name!r}: criteria must not be empty") |
There was a problem hiding this comment.
问题: 字典形式 question 的 type 字段不做白名单校验——_validate_question 只对 "choice"/"score" 检查 criteria,对任意其他字符串 type(如拼写错误的 "Choice"、"spellcheck")直接放行并发出请求,直到响应后 check_answers 才报「unknown type」。
触发条件: 调用方传入 {"q": {"type": "spellcheck", "instructions": "x"}} 这类打字错误/未知类型的问题字典。
实际影响: 违反「发出去之前就拦住坏参数」的设计承诺(docs/design/02-systemone-design.md §2.2/§2.5:「这些检查失败都是 SystemOneError,请求不会发出」):请求被打到服务器(浪费配额并可能暴露请求内容),服务器返回 4xx 时用户看到的是 TypeSafeBadRequestError 而非本地校验错,错误语义偏离设计。
修正方向: 在 _validate_question(或 validate_questions)中对 question["type"] 做白名单校验({"noul", "choice", "score", "box"}),未知 type 直接抛 SystemOneError,与 check_answers 的 _ANSWER_TYPES 保持一致;并补充未知 type 不发请求的测试。
| if isinstance(timeout, bool) or not isinstance(timeout, (int, float)) or timeout < 0: | ||
| raise SystemOneError("systemone: timeout must not be negative") | ||
| return float(timeout) |
There was a problem hiding this comment.
问题: validate_timeout 只拒绝负数/bool/非数值,docstring 自称「Reject a negative timeout or a non-numeric value」,但对 float("nan") 和 float("inf") 放行——nan < 0 恒为 False、isinstance(nan, float) 为 True。
触发条件: 调用方通过 float(os.getenv("TIMEOUT", "30")) 之类解析环境变量时传入 "nan"/"inf"。
实际影响: nan 进入 _client._wait 后 nan <= 0 为 False,走 asyncio.wait_for(run(), nan),实测 nan >= deadline 比较恒 False 导致定时器立即到期、调用瞬间抛超时;inf 则永不触发。两者都与「设置了超时」的预期不符,且 _http_timeout 同样把 nan/inf 原样传给 httpx2。错误行为是静默的,调用方无从察觉。
修正方向: 在 validate_timeout 中增加 math.isfinite(timeout) 检查(拒绝 NaN 和 Infinity),并补充 NaN/Infinity 入参的测试。
| def json_ready_state(state: object) -> object: | ||
| """Return ``state`` in a form ``json.dumps`` can encode.""" | ||
| if isinstance(state, BaseModel): | ||
| return state.model_dump(mode="json") | ||
| return state |
There was a problem hiding this comment.
问题: json_ready_state 中的 state.model_dump(mode="json") 在 _laya._dumps 的 try/except 作用域之外求值(_backends.py:127 作为 request_body 参数先求值),BaseModel state 含不可 JSON 序列化字段(如 bytes、Path 类型字段)时,pydantic 抛出的序列化错误(PydanticSerializationError 等)不会转成 SystemOneError。
触发条件: Laya 后端 + 传入含不可序列化字段的 Pydantic 模型作为 state(设计文档 §2.3 明确支持 Pydantic 模型作为 state)。
实际影响: 裸 pydantic 序列化异常泄漏给调用方,违反「失败都是 TypeSafeError 子类」的契约;而同一 _dumps 里对普通 dict state 的同类问题(如 NaN)却能正确转成 SystemOneError,行为不一致。
修正方向: 将 json_ready_state 的 model_dump 调用移入 _dumps 的 try 作用域内(或为其单独包一层 try/except 转 SystemOneError),并补充含不可序列化字段的 BaseModel state 测试。
| validate_questions(questions) | ||
| _validate_states(states) | ||
| if not self._backend.supports_batch: | ||
| raise SystemOneError("systemone: batch is only supported by laya") |
There was a problem hiding this comment.
问题: system_one_batch 中 _validate_states(states) 在 self._backend.supports_batch 检查之前执行。对官方 TypeSafe 后端(supports_batch = False),若调用方传入空列表或超过 64 条 states,会先抛「systemone: batch accepts between 1 and 64 states」,而不是真正根因的「batch is only supported by laya」。
触发条件: SystemOneClient.typesafe(...).system_one_batch([]) 或传入 65 条 states(测试 test_batch_is_rejected 只覆盖了 1 条合法 state)。
实际影响: 错误信息误导——对 TypeSafe 后端 1..64 条的本地限制检查毫无意义,调用方会误以为调整批量大小即可,实际需要改用 Laya 后端;两处均为 SystemOneError 但根因被掩盖。
修正方向: 将 supports_batch 检查移到 _validate_states 之前(后端不支持时优先报「batch is only supported by laya」),或对不支持批量时跳过状态数量校验;补充 typesafe 后端传空/超量 states 的测试。
|
|
||
| - Python3.10+,推荐 Python3.12 | ||
| - TypeSafe 路径需要可访问官方 API 的 key | ||
| - Laya 路径需要已启动的 `laya-serve`。安装项目时已包含 `typesafe-sdk` |
There was a problem hiding this comment.
问题: README 第 55 行称「安装项目时已包含 typesafe-sdk」,但 typesafe-sdk 只在 [typesafe] 和 [all] extra 中(pyproject.toml),普通 pip install trpc-agent-py 并装入 examples/systemone_model/ 运行 python3 run.py 时不会安装它;该表述只在走 ./build.sh(会装 requirements-test.txt)时成立。
触发条件: 按 README 步骤克隆、pip install trpc-agent-py(不带 extra)、配置好环境变量后直接 python3 run.py。
实际影响: 示例在 import 阶段因 ModuleNotFoundError: typesafe_sdk 崩溃且无指引(与 __init__.py 裸导入无提示叠加),用户无法从文档判断需要安装 [typesafe] extra,与 pyproject 把 typesafe-sdk 设计为可选依赖的意图矛盾。
修正方向: 将第 55 行改为明确「需要执行 pip install 'trpc-agent-py[typesafe]' 或通过 ./build.sh 安装测试依赖」,与 pyproject 的 extra 语义保持一致。
支持 systemone 决策模型 , examples 和 设计文档: