docs: 沉淀 PEP 562 re-export 架构模式 + 方案文档
ce-compound (Full mode) 沉淀本次架构修复经验: - docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md knowledge track 文档,覆盖 Context/Guidance/Why This Matters/When to Apply/ Examples,记录 PEP 562 模块级 __getattr__ 延迟 re-export 打破循环依赖的 模式、一次性缓存机制、27 调用方兼容性保护、__module__ 取舍。 同时提交: - docs/plans/2026-07-06-003-fix-architecture-smell-and-test-gaps-plan.md ce-plan 产出的方案文档(4 个 Implementation Units) - docs/solutions/logic-errors/gate-without-caller-silently-disables-feature.md 之前会话沉淀的 gate-without-caller 经验
This commit is contained in:
parent
7449458710
commit
c429851c97
|
|
@ -0,0 +1,246 @@
|
|||
---
|
||||
artifact_contract: ce-unified-plan/v1
|
||||
artifact_readiness: implementation-ready
|
||||
product_contract_source: ce-plan-bootstrap
|
||||
execution: code
|
||||
title: "fix: Resolve Core↔LLM architecture smell and fill high-risk test gaps"
|
||||
created: 2026-07-06
|
||||
plan_type: fix
|
||||
---
|
||||
|
||||
# fix: Resolve Core↔LLM architecture smell and fill high-risk test gaps
|
||||
|
||||
## Summary
|
||||
|
||||
基于知识图谱分析发现两类问题:(1) Core Agent 引擎与 LLM 网关之间存在架构分层异味 — LLM 异常定义在 `core/exceptions.py`,导致 LLM 层反向依赖 Core;(2) 2 个核心源文件完全无测试覆盖,1 个核心模块(WebSocket 聊天路由)缺少 WebSocket 路径测试。本方案通过 Re-export 兼容方式迁移异常定义,并为 3 个测试缺口补充单元测试。
|
||||
|
||||
## Problem Frame
|
||||
|
||||
### 背景
|
||||
|
||||
知识图谱分析(基于 understand-anything 生成的 1579 节点图谱)发现:
|
||||
|
||||
1. **架构分层异味**:`core/exceptions.py` 定义了 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 三个 LLM 相关异常类(L135-154),导致 7 个 LLM 层文件反向依赖 Core 层获取自己的异常定义。虽然 `core/exceptions.py` 是叶子模块(不导入 agentkit.*),不构成 Python 循环导入,但这违反了分层架构原则。
|
||||
|
||||
2. **测试覆盖缺口**(经实际代码搜索验证,纠正了知识图谱 `tested_by` 边不足导致的误判):
|
||||
- `llm/migration.py` — 完全无测试,含高风险函数 `migrate_api_keys_to_secrets`
|
||||
- `memory/models.py` — 完全无测试,含 `EpisodeModel` 等数据模型
|
||||
- `server/routes/chat.py` 的 WebSocket 路径 — `chat_websocket`/`_handle_chat_message` 无直接 WebSocket 测试(现有 `test_chat_routes.py` 只覆盖 REST 端点)
|
||||
|
||||
### 不在范围内
|
||||
|
||||
- 知识图谱 imports/tested_by 边补全(图谱分析局限性,非代码问题)
|
||||
- experts/_*.py mixin 直接单元测试(通过 ExpertTeam 间接测试,用户确认跳过)
|
||||
- 大规模模块结构重组
|
||||
- 前端/桌面端测试补充
|
||||
|
||||
## Requirements
|
||||
|
||||
- **R1**: LLM 异常定义迁移到 `llm/exceptions.py`,消除 LLM → Core 的反向依赖
|
||||
- **R2**: `core/exceptions.py` 保留 Re-export,确保现有代码无需修改(向后兼容)
|
||||
- **R3**: `llm/migration.py` 有单元测试覆盖 `migrate_api_keys_to_secrets` 函数
|
||||
- **R4**: `memory/models.py` 有单元测试覆盖核心数据模型
|
||||
- **R5**: `server/routes/chat.py` WebSocket 路径有测试覆盖关键消息处理流程
|
||||
|
||||
## Key Technical Decisions
|
||||
|
||||
### KTD1: Re-export 兼容迁移策略
|
||||
|
||||
**决策**:将 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 的定义迁移到 `llm/exceptions.py`,在 `core/exceptions.py` 末尾保留 `from llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError`(Re-export)。
|
||||
|
||||
**理由**:
|
||||
- 消除架构异味 — LLM 异常定义在 LLM 层
|
||||
- 零破坏性变更 — 现有 `from core.exceptions import LLMProviderError` 的 7 个文件无需修改
|
||||
- 符合项目"未经明确请求不得修改"的约束 — 只移动定义,不改变 API 表面
|
||||
- 真正的异常定义位置(`llm/exceptions.py`)符合分层架构原则
|
||||
|
||||
**替代方案**:全量替换(更新所有 7 个文件的 import)— 破坏性更大,且 `core.exceptions` 中的 re-export 仍需保留以避免外部消费者破坏,反而增加维护负担。
|
||||
|
||||
### KTD2: 测试范围基于实际代码验证而非知识图谱
|
||||
|
||||
**决策**:测试补充范围基于实际 Grep 搜索验证,而非知识图谱的 `tested_by` 边。
|
||||
|
||||
**理由**:知识图谱中 `tested_by` 边只有 19 条(文件级),因为子代理分析时 `batchImportData` 为空。实际验证发现 `filter_pii`、`generate_cache_key`、`detect_verification_commands` 均已有充分测试,无需补充。只有 3 个真正的测试缺口需要处理。
|
||||
|
||||
### KTD3: WebSocket 测试使用 httpx.AsyncClient + WebSocketTestSession
|
||||
|
||||
**决策**:WebSocket 测试使用 FastAPI 的 `TestClient` WebSocket 支持(`client.websocket_connect`),遵循 `tests/unit/test_websocket.py` 的现有模式。
|
||||
|
||||
**理由**:项目已有 WebSocket 测试模式(`test_websocket.py`、`test_chat_plan_exec_ws.py`),无需引入新依赖。
|
||||
|
||||
## Implementation Units
|
||||
|
||||
### U1. 迁移 LLM 异常到 llm/exceptions.py(Re-export 兼容)
|
||||
|
||||
**Goal**: 将 `LLMError`/`LLMProviderError`/`ModelNotFoundError` 定义迁移到 `llm/exceptions.py`,`core/exceptions.py` 保留 re-export。
|
||||
|
||||
**Requirements**: R1, R2
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `src/agentkit/llm/exceptions.py`(新建)
|
||||
- `src/agentkit/core/exceptions.py`(修改 — 删除 L135-154 的 3 个类定义,添加 re-export)
|
||||
- `src/agentkit/llm/__init__.py`(修改 — 如有 `__all__` 则添加 exceptions 导出)
|
||||
|
||||
**Approach**:
|
||||
1. 创建 `src/agentkit/llm/exceptions.py`,包含从 `core/exceptions.py` 迁移的 3 个异常类定义(`LLMError`、`LLMProviderError`、`ModelNotFoundError`)
|
||||
2. `llm/exceptions.py` 中的 `LLMError` 继承 `Exception`(而非 `AgentFrameworkError`),因为 LLM 层不应依赖 Core 层。但为保持 `isinstance` 兼容性,`LLMError` 需同时继承 `AgentFrameworkError` — 通过在 `llm/exceptions.py` 中导入 `AgentFrameworkError` from `core.exceptions`(`AgentFrameworkError` 是框架基础异常,不是 LLM 异常,保留在 core 中是正确的)
|
||||
3. `core/exceptions.py` 删除 L135-154 的 3 个类定义,在文件末尾添加:
|
||||
```python
|
||||
# Re-export LLM exceptions from llm/exceptions.py for backward compatibility.
|
||||
# New code should import from llm.exceptions directly.
|
||||
from llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError # noqa: F401
|
||||
```
|
||||
4. 验证所有 `from core.exceptions import LLMProviderError` 的导入仍能工作
|
||||
|
||||
**Patterns to follow**: 项目现有的异常定义模式(`class XError(ParentError): def __init__(self, ...): super().__init__(...)`)
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: `from llm.exceptions import LLMProviderError` 能正常导入且 `isinstance(LLMProviderError("test", "reason"), AgentFrameworkError)` 为 True
|
||||
- Backward compat: `from core.exceptions import LLMProviderError` 仍能正常导入
|
||||
- isinstance 兼容: `isinstance(LLMProviderError(...), LLMError)` 为 True(继承链保持)
|
||||
- 构造函数: `LLMProviderError("openai", "timeout").provider == "openai"` 且消息包含 "openai" 和 "timeout"
|
||||
- `ModelNotFoundError("gpt-4").model == "gpt-4"` 且消息包含 "gpt-4"
|
||||
|
||||
**Verification**: `ruff check src/agentkit/llm/exceptions.py src/agentkit/core/exceptions.py && pytest tests/unit/ -x -q -k "exception or llm_provider or remote_provider"` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U2. 新建 llm/migration.py 单元测试
|
||||
|
||||
**Goal**: 为 `llm/migration.py` 的 `migrate_api_keys_to_secrets` 函数创建单元测试。
|
||||
|
||||
**Requirements**: R3
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/llm/test_migration.py`(新建)
|
||||
- `src/agentkit/llm/migration.py`(读取 — 确认函数签名和行为)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `llm/migration.py` 了解 `migrate_api_keys_to_secrets` 的签名、参数、返回值和副作用
|
||||
2. 创建 `tests/unit/llm/test_migration.py`,遵循 `tests/unit/llm/test_cache.py` 的测试模式
|
||||
3. 使用 mock 隔离外部依赖(secrets store、配置文件、环境变量)
|
||||
4. 覆盖正常迁移、幂等性(重复迁移)、错误处理(源不存在/目标写入失败)场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 有 API Key 配置时,`migrate_api_keys_to_secrets` 将配置中的 key 迁移到 secrets store,返回迁移数量
|
||||
- Idempotent: 对已迁移的配置再次调用,不重复迁移,返回 0
|
||||
- Empty config: 无 API Key 配置时调用,返回 0,不修改 secrets store
|
||||
- Source missing: 源配置文件不存在时,函数优雅处理(返回 0 或抛出预期异常,视实现而定)
|
||||
- Target write failure: secrets store 写入失败时,函数抛出异常且不部分迁移(事务性)
|
||||
- Secret overwrite: 已存在同 key 的 secret 时,按实现行为验证(覆盖或跳过)
|
||||
|
||||
**Verification**: `pytest tests/unit/llm/test_migration.py -x -q` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U3. 新建 memory/models.py 单元测试
|
||||
|
||||
**Goal**: 为 `memory/models.py` 的核心数据模型创建单元测试。
|
||||
|
||||
**Requirements**: R4
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/test_memory_models.py`(新建)
|
||||
- `src/agentkit/memory/models.py`(读取 — 确认模型定义)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `memory/models.py` 了解数据模型(`EpisodeModel` 等)的字段、验证器和序列化
|
||||
2. 创建 `tests/unit/test_memory_models.py`,遵循 `tests/unit/test_session_models.py` 的 Pydantic 模型测试模式
|
||||
3. 覆盖模型构造、字段验证、序列化/反序列化场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 用有效字段构造 `EpisodeModel`(或其他模型),字段值正确
|
||||
- Required fields: 缺少必填字段时抛出 `ValidationError`
|
||||
- Field validation: 字段类型/格式不正确时抛出 `ValidationError`(如时间戳格式、向量维度)
|
||||
- Serialization: `model_dump()` / `model_dump_json()` 输出格式正确
|
||||
- Deserialization: `model_validate_json()` / `model_validate()` 能从 dict/JSON 重建模型
|
||||
- Default values: 可选字段使用默认值时行为正确
|
||||
- Edge case: 空向量、超长文本、特殊字符等边界值处理
|
||||
|
||||
**Verification**: `pytest tests/unit/test_memory_models.py -x -q` 全部通过
|
||||
|
||||
---
|
||||
|
||||
### U4. 增强 server/routes/chat.py WebSocket 测试覆盖
|
||||
|
||||
**Goal**: 为 `server/routes/chat.py` 的 `chat_websocket` 和 `_handle_chat_message` 函数补充 WebSocket 路径测试。
|
||||
|
||||
**Requirements**: R5
|
||||
|
||||
**Dependencies**: 无
|
||||
|
||||
**Files**:
|
||||
- `tests/unit/test_chat_ws_routes.py`(新建 — 避免修改现有 `test_chat_routes.py` 的 REST 测试)
|
||||
- `src/agentkit/server/routes/chat.py`(读取 — 确认 WebSocket 处理逻辑)
|
||||
|
||||
**Approach**:
|
||||
1. 阅读 `server/routes/chat.py` 的 `chat_websocket` 函数,了解 WebSocket 消息处理流程(`message`/`reply`/`confirmation_reply`/`cancel`/`ping`)
|
||||
2. 参考 `tests/unit/test_websocket.py` 和 `tests/unit/test_chat_plan_exec_ws.py` 的 WebSocket 测试模式
|
||||
3. 使用 `TestClient.websocket_connect("/api/v1/chat/ws/...")` 发送消息并断言响应事件
|
||||
4. Mock LLM gateway 和 agent 执行,聚焦 WebSocket 协议层测试
|
||||
5. 覆盖连接、消息处理、取消、错误事件场景
|
||||
|
||||
**Test scenarios**:
|
||||
- Happy path: 客户端发送 `{"type": "message", "content": "hello"}`,服务端返回 `connected` → `token`* → `final_answer` 事件序列
|
||||
- Ping/Pong: 客户端发送 `{"type": "ping"}`,服务端返回 `{"type": "pong"}`
|
||||
- Cancel: 客户端发送 `{"type": "cancel"}`,服务端停止处理并返回确认(或静默关闭)
|
||||
- Confirmation flow: 服务端发送 `confirmation_request`,客户端回复 `confirmation_reply`,服务端继续执行
|
||||
- Error handling: 无效消息格式时,服务端返回 `{"type": "error", "message": ...}`
|
||||
- Session isolation: 不同 `session_id` 的 WebSocket 连接互不干扰
|
||||
- Connection cleanup: 客户端断开后,服务端清理资源(无孤立 future)
|
||||
|
||||
**Verification**: `pytest tests/unit/test_chat_ws_routes.py -x -q` 全部通过
|
||||
|
||||
## Verification Contract
|
||||
|
||||
### 自动化验证
|
||||
|
||||
```bash
|
||||
# Lint
|
||||
ruff check src/agentkit/llm/exceptions.py src/agentkit/core/exceptions.py
|
||||
|
||||
# 单元测试(本次新增 + 受影响的现有测试)
|
||||
python3 -m pytest tests/unit/llm/test_migration.py tests/unit/test_memory_models.py tests/unit/test_chat_ws_routes.py -x -q
|
||||
|
||||
# 回归测试(确保异常迁移不破坏现有行为)
|
||||
python3 -m pytest tests/unit/test_llm_provider.py tests/unit/test_remote_provider.py tests/unit/test_llm_retry.py tests/unit/test_llm_gateway.py -x -q
|
||||
|
||||
# 全量单元测试(最终验证)
|
||||
python3 -m pytest tests/unit/ -x -q
|
||||
```
|
||||
|
||||
### Definition of Done
|
||||
|
||||
- [ ] U1: `llm/exceptions.py` 创建,`core/exceptions.py` 保留 re-export,`ruff check` 通过
|
||||
- [ ] U2: `tests/unit/llm/test_migration.py` 创建,所有测试场景通过
|
||||
- [ ] U3: `tests/unit/test_memory_models.py` 创建,所有测试场景通过
|
||||
- [ ] U4: `tests/unit/test_chat_ws_routes.py` 创建,所有测试场景通过
|
||||
- [ ] 全量 `pytest tests/unit/ -x -q` 通过(无回归)
|
||||
- [ ] `ruff check src/ && ruff format src/` 通过
|
||||
|
||||
## Scope Boundaries
|
||||
|
||||
### In scope
|
||||
|
||||
- 迁移 3 个 LLM 异常类到 `llm/exceptions.py`(Re-export 兼容)
|
||||
- 为 3 个测试缺口补充单元测试(llm/migration.py、memory/models.py、chat WebSocket)
|
||||
|
||||
### Out of scope
|
||||
|
||||
- 知识图谱 imports/tested_by 边补全
|
||||
- experts/_*.py mixin 直接单元测试
|
||||
- 大规模模块结构重组
|
||||
- 前端/桌面端测试补充
|
||||
- 已有充分测试的函数(filter_pii、generate_cache_key、detect_verification_commands)的测试增强
|
||||
|
||||
### Deferred to Follow-Up Work
|
||||
|
||||
- 知识图谱增量更新时补充 tests/ 目录扫描,提高 `tested_by` 边覆盖率
|
||||
- experts/_*.py mixin 类的直接单元测试(如需更细粒度覆盖时)
|
||||
- `core/exceptions.py` 中其他异常类的分层审查(如 ToolNotFoundError 是否应迁移到 tools/)
|
||||
|
|
@ -0,0 +1,298 @@
|
|||
---
|
||||
title: "Breaking circular imports with PEP 562 module-level __getattr__ lazy re-export"
|
||||
date: 2026-07-06
|
||||
category: docs/solutions/architecture-patterns/
|
||||
module:
|
||||
- agentkit.core.exceptions
|
||||
- agentkit.llm.exceptions
|
||||
- agentkit.llm.retry
|
||||
problem_type: architecture_pattern
|
||||
component: tooling
|
||||
severity: medium
|
||||
applies_when:
|
||||
- Two modules form an import cycle (A imports B, B imports A) at module-init time that simple reordering cannot resolve
|
||||
- Migrating exception or symbol classes to a new home module while preserving many legacy 'from old.path import X' call sites
|
||||
- Eager re-export at module top would re-introduce the cycle; PEP 562 lazy per-name __getattr__ breaks the cycle while keeping the legacy import path working
|
||||
tags: [circular-import, pep562, lazy-reexport, module-getattr, backward-compat, exception-hierarchy]
|
||||
---
|
||||
|
||||
# 用 PEP 562 模块级 `__getattr__` 延迟 re-export 打破循环依赖
|
||||
|
||||
## Context
|
||||
|
||||
`agentkit` 项目里 `core.exceptions` 与 `llm.exceptions` 之间存在一条隐蔽的循环依赖链路:
|
||||
|
||||
```
|
||||
core.exceptions → llm.exceptions → llm.__init__ → llm.retry → core.exceptions.LLMProviderError
|
||||
```
|
||||
|
||||
这条链路之所以会出现,根因是一个**模块分层异味**:LLM 相关异常(`LLMError` / `LLMProviderError` / `ModelNotFoundError`)原本被定义在 `src/agentkit/core/exceptions.py`,但它们的真正消费者几乎全部位于 `llm/` 子系统内(`llm/retry.py:10` 通过 `from agentkit.core.exceptions import LLMProviderError` 拿到基类)。换句话说,core 层在为 llm 层"代持"异常定义,而 llm 层又反过来要被 core 引用——这就构成了**反向依赖**。
|
||||
|
||||
链路在常规顺序导入时不会暴露问题,但当 Python 解释器在 `core.exceptions` 部分初始化阶段(即 `core.exceptions` 还没把 `LLMProviderError` 绑定到模块命名空间)被拉起 `llm.__init__` 时,`llm.retry` 拿到的就是一个**未绑定符号**,触发 `ImportError`。这种 part-initialized failure 极难复现,往往只在某些导入顺序、测试收集顺序或打包工具的 import graph 重排后才显形,是典型的"代码味道"演化为"运行时炸弹"。
|
||||
|
||||
更棘手的是,`from agentkit.core.exceptions import LLMProviderError` 这一契约已经被 27 个调用方依赖。任何"把异常类直接搬走"的迁移方案,如果不提供 re-export 兼容层,都会触发大面积 `ImportError`,回归测试会瞬间失败。因此我们需要一种**既能搬走定义、又能在原路径透明访问**的手段。
|
||||
|
||||
## Guidance
|
||||
|
||||
推荐模式:**把定义迁移到正确的层级,再用 PEP 562 模块级 `__getattr__` 在原路径做延迟 re-export**。这是 Python 3.7+ 标准库原生能力,无需任何新依赖,也无需引入 lazy-import 框架。
|
||||
|
||||
### 步骤一:把异常定义搬到与消费者同层
|
||||
|
||||
新建 `src/agentkit/llm/exceptions.py`,作为 LLM 异常的新家。**顶层只 import 已经完全加载好的 `AgentFrameworkError`**,不在 `llm.exceptions` 内反向 import `core.exceptions` 的任何 LLM 符号,避免把循环又请回来:
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/exceptions.py
|
||||
"""LLM 子系统的异常定义。
|
||||
|
||||
放在这里而不是 core.exceptions,因为这些异常的唯一消费者
|
||||
是 llm/ 子系统;core 层不应为 llm 层代持定义。
|
||||
"""
|
||||
from agentkit.core.exceptions import AgentFrameworkError
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 调用相关的所有异常基类。"""
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""Provider 配置或不可用导致的错误。"""
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""请求的模型在当前 provider 中不存在。"""
|
||||
```
|
||||
|
||||
### 步骤二:在原路径用 PEP 562 `__getattr__` 延迟 re-export
|
||||
|
||||
修改 `src/agentkit/core/exceptions.py`,删除三个 LLM 异常类的原定义,改用模块级 `__getattr__`:
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(节选)
|
||||
# 已删除 LLMError / LLMProviderError / ModelNotFoundError 的本地定义
|
||||
# 改为延迟 re-export,保持 27 个调用方的 import 契约不变
|
||||
|
||||
_LLM_EXCEPTION_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
def __getattr__(name: str): # PEP 562 模块级 __getattr__
|
||||
if name in _LLM_EXCEPTION_NAMES:
|
||||
# 首次访问时才 import llm.exceptions,避免模块加载期触发循环
|
||||
import agentkit.llm.exceptions as _llm_exc # noqa: PLC0415
|
||||
# 一次性把三个类都缓存进 globals(),后续访问直接命中
|
||||
for n in _LLM_EXCEPTION_NAMES:
|
||||
globals()[n] = getattr(_llm_exc, n)
|
||||
return globals()[name]
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
def __dir__():
|
||||
# 让 dir(core.exceptions) 和 IDE 自动补全仍然看到这三个名字
|
||||
return [*(globals().keys()), *_LLM_EXCEPTION_NAMES]
|
||||
```
|
||||
|
||||
### 步骤三:让 `llm/__init__.py` 直接从新家 re-export
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/__init__.py(追加)
|
||||
from agentkit.llm.exceptions import LLMError, LLMProviderError, ModelNotFoundError
|
||||
|
||||
__all__ = [*__all__, "LLMError", "LLMProviderError", "ModelNotFoundError"]
|
||||
```
|
||||
|
||||
### 步骤四:写守护测试锁定契约
|
||||
|
||||
`tests/unit/test_core_exceptions_reexport.py`(6 个测试)——这一步**不可省略**。re-export 兼容性的失败模式非常安静(`is` identity 漂移、`__module__` 变化、`__dir__` 缺项),没有自动化守护就会在几个月后悄悄回退:
|
||||
|
||||
```python
|
||||
def test_reexport_is_identity():
|
||||
# 跨路径访问必须是同一个类对象,否则 isinstance 双重判断会裂开
|
||||
from agentkit.core.exceptions import LLMProviderError as A
|
||||
from agentkit.llm.exceptions import LLMProviderError as B
|
||||
assert A is B
|
||||
|
||||
|
||||
def test_reexport_isinstance_cross_path():
|
||||
# 实例在任一路径 isinstance 都成立
|
||||
from agentkit.core.exceptions import LLMProviderError
|
||||
from agentkit.llm.exceptions import LLMError
|
||||
assert isinstance(LLMProviderError("x"), LLMError)
|
||||
|
||||
|
||||
def test_unknown_name_raises_attribute_error():
|
||||
import agentkit.core.exceptions as ce
|
||||
with pytest.raises(AttributeError):
|
||||
ce.NotARealException
|
||||
|
||||
|
||||
def test_dir_contains_reexported_names():
|
||||
import agentkit.core.exceptions as ce
|
||||
assert "LLMProviderError" in dir(ce)
|
||||
|
||||
|
||||
def test_cached_after_first_access():
|
||||
# 首次访问后写入 globals(),第二次访问不再触发 __getattr__
|
||||
import agentkit.core.exceptions as ce
|
||||
_ = ce.LLMProviderError # 触发懒加载
|
||||
assert "LLMProviderError" in ce.__dict__ # 已缓存
|
||||
|
||||
|
||||
def test_inheritance_chain_preserved():
|
||||
from agentkit.core.exceptions import LLMProviderError, AgentFrameworkError
|
||||
assert issubclass(LLMProviderError, AgentFrameworkError)
|
||||
```
|
||||
|
||||
## Why This Matters
|
||||
|
||||
### 为什么 PEP 562 `__getattr__` 能打破循环
|
||||
|
||||
Python 模块级 `__getattr__`(PEP 562,3.7+)的关键性质是:**它只在属性访问时触发,不在模块加载时执行 import**。这与顶层 `from ... import ...` 形成对照——后者在模块字节码执行阶段就立即触发依赖解析,正是它把循环依赖变成 `ImportError` 的根源。
|
||||
|
||||
把循环链路拆解来看:
|
||||
|
||||
1. 解释器开始加载 `core.exceptions`,执行到文件底部 PEP 562 定义——此时 `__getattr__` **只是注册了一个函数对象**,不会调用,也不会触发任何 import。
|
||||
2. `core.exceptions` 完全加载完毕,`AgentFrameworkError`(core 顶层定义)已绑定。
|
||||
3. 后续某处 `from agentkit.core.exceptions import LLMProviderError` 触发 `__getattr__("LLMProviderError")`。
|
||||
4. 此时 `agentkit.llm` 通常已经在 `sys.modules` 中(即使部分加载,子模块 `llm.exceptions` 仍可独立 import);`import agentkit.llm.exceptions` 不会重新执行 `llm/__init__.py`,而是直接拿到子模块对象。
|
||||
5. `llm.exceptions` 顶层只需要 `AgentFrameworkError`——它在步骤 2 就已就绪,所以 import 成功,不会回到 `core.exceptions` 找未绑定符号。
|
||||
|
||||
整条链路从"加载期硬解析"退化为"访问期软解析",循环就断了。
|
||||
|
||||
### 一次性缓存机制
|
||||
|
||||
注意 `__getattr__` 内部用的是 `for n in _LLM_EXCEPTION_NAMES: globals()[n] = ...`,**一次性把三个类都写入模块 globals**,而不是只写当前请求的那个。这样做有两个好处:
|
||||
|
||||
- **后续访问零开销**:第二次 `core.exceptions.LLMProviderError` 直接命中 `globals()`,不再进入 `__getattr__`,等价于普通模块属性。
|
||||
- **避免半缓存状态**:如果只缓存当前请求的名字,访问 `LLMError` 后 `LLMProviderError` 仍需走 `__getattr__`,行为不对称,调试时容易误判。批量缓存让"已懒加载"成为一个布尔状态,而不是逐名字的状态机。
|
||||
|
||||
### 向后兼容的价值:27 调用方保护
|
||||
|
||||
`from agentkit.core.exceptions import LLMProviderError` 这个契约被 27 个调用方依赖,散落在 `llm/`、`tools/`、`server/`、`tests/` 等多个子系统。如果直接搬走定义而不做 re-export,这 27 处会同时报 `ImportError`,回归测试雪崩。PEP 562 方案让所有现有 import 语句**一字不改**地继续工作——这是该模式相对于"批量重写 import"的核心收益。
|
||||
|
||||
### `__module__` 变化的取舍
|
||||
|
||||
ce-code-review 的 9 个 reviewer 共识指出一个**已知副作用**:`LLMProviderError.__module__` 会从 `agentkit.core.exceptions` 变为 `agentkit.llm.exceptions`。这会影响 `pickle` 序列化路径和部分 observability 工具的异常归类。在本项目中,异常不跨进程 pickle、内部无 observability 消费者依赖 `__module__`,因此这是一个**可接受的取舍**。如果你的项目有跨进程 pickle 异常或依赖 `__module__` 的遥测管线,应在迁移前显式评估这一项。
|
||||
|
||||
### 传递依赖面扩大
|
||||
|
||||
另一个 reviewer 共识是:迁移后 `import agentkit.core` 的传递依赖面会扩大到整个 LLM 栈(因为 `__getattr__` 触发时会拉起 `llm.exceptions`,进而可能拉起 `llm.__init__`)。在 agentkit 中这是可接受的——core 与 llm 在同一进程、同一打包单元。如果你的项目把 core 做成"轻量可独立分发"的包,应评估这条传递依赖是否破坏 packaging 边界。
|
||||
|
||||
## When to Apply
|
||||
|
||||
满足以下任一条件时,优先考虑 PEP 562 延迟 re-export 模式:
|
||||
|
||||
- **模块分层异味**:某个层(如 core)在为另一个层(如 llm)"代持"定义,而定义的唯一消费者在后者,构成反向依赖。
|
||||
- **循环依赖链路**:`A → B → A` 的 import 链路在部分初始化时触发 `ImportError`,且无法通过简单的"把 import 移到函数内"消除(因为 import 出现在模块顶层类定义的基类位置)。
|
||||
- **需要保持 re-export 兼容性**:原 import 路径已被大量调用方依赖,无法或不希望批量重写。
|
||||
- **不想引入新依赖**:项目希望用标准库能力解决问题,避免 lazy-import 第三方库(如 `lazy-object-proxy`)。
|
||||
- **Python >= 3.7**:PEP 562 的硬性前置条件。agentkit 要求 3.11+,自然满足。
|
||||
|
||||
**不适用**的场景:
|
||||
|
||||
- 循环依赖可以通过**重新切分模块边界**(把共享定义下沉到更底层)干净消除时——优先重新切分,PEP 562 是兼容性约束下的退路。
|
||||
- 循环链路里没有"代持"问题,纯粹是双向业务依赖时——应重构为依赖注入或事件总线,而不是用 re-export 掩盖。
|
||||
- 需要跨进程 pickle 异常且 `__module__` 必须保持原值时——PEP 562 re-export 会改 `__module__`,需要额外用 `__reduce__` 或元类技巧补回,复杂度通常不值。
|
||||
|
||||
## Examples
|
||||
|
||||
### Before:原定义在 core,循环链路存在
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(旧版本,问题代码)
|
||||
class AgentFrameworkError(Exception):
|
||||
...
|
||||
|
||||
|
||||
# 三个 LLM 异常定义在 core 层 —— 分层异味
|
||||
class LLMError(AgentFrameworkError):
|
||||
...
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
...
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
...
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/retry.py(旧版本,问题代码)
|
||||
from agentkit.core.exceptions import LLMProviderError # ← 顶层 import,触发循环
|
||||
|
||||
def with_retry(fn):
|
||||
...
|
||||
raise LLMProviderError(...)
|
||||
```
|
||||
|
||||
导入顺序不吉利时:`core.exceptions` 部分初始化 → 拉起 `llm.__init__` → `llm.retry` 顶层 import `LLMProviderError` → 该名字尚未绑定 → `ImportError`。
|
||||
|
||||
### After:定义迁移到 llm,core 用 PEP 562 re-export
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/exceptions.py(新建)
|
||||
from agentkit.core.exceptions import AgentFrameworkError
|
||||
|
||||
|
||||
class LLMError(AgentFrameworkError):
|
||||
"""LLM 调用相关异常基类。"""
|
||||
|
||||
|
||||
class LLMProviderError(LLMError):
|
||||
"""Provider 配置或不可用错误。"""
|
||||
|
||||
|
||||
class ModelNotFoundError(LLMError):
|
||||
"""模型在 provider 中不存在。"""
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/core/exceptions.py(修改后)
|
||||
class AgentFrameworkError(Exception):
|
||||
...
|
||||
|
||||
|
||||
# 已迁移走的 LLM 异常 —— 用 PEP 562 延迟 re-export 保持兼容
|
||||
_LLM_EXCEPTION_NAMES = ("LLMError", "LLMProviderError", "ModelNotFoundError")
|
||||
|
||||
|
||||
def __getattr__(name: str):
|
||||
if name in _LLM_EXCEPTION_NAMES:
|
||||
import agentkit.llm.exceptions as _llm_exc # noqa: PLC0415
|
||||
for n in _LLM_EXCEPTION_NAMES:
|
||||
globals()[n] = getattr(_llm_exc, n)
|
||||
return globals()[name]
|
||||
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
|
||||
|
||||
|
||||
def __dir__():
|
||||
return [*(globals().keys()), *_LLM_EXCEPTION_NAMES]
|
||||
```
|
||||
|
||||
```python
|
||||
# src/agentkit/llm/retry.py(不变!27 个调用方之一,零修改)
|
||||
from agentkit.core.exceptions import LLMProviderError # 仍然 work
|
||||
```
|
||||
|
||||
### 守护测试片段
|
||||
|
||||
```python
|
||||
# tests/unit/test_core_exceptions_reexport.py
|
||||
def test_reexport_is_identity():
|
||||
from agentkit.core.exceptions import LLMProviderError as A
|
||||
from agentkit.llm.exceptions import LLMProviderError as B
|
||||
assert A is B # 同一个类对象
|
||||
|
||||
|
||||
def test_dir_contains_reexported_names():
|
||||
import agentkit.core.exceptions as ce
|
||||
assert "LLMProviderError" in dir(ce) # IDE / 自动补全友好
|
||||
```
|
||||
|
||||
完整守护测试覆盖 6 个维度:`is` identity、跨路径 `isinstance`、未知名 `AttributeError`、`__dir__` 完整性、首次访问后缓存命中、继承链保持。这 6 个维度对应了 re-export 模式所有已知的安静失败模式——任何一个回退都会被测试抓住。
|
||||
|
||||
## Related
|
||||
|
||||
- ce-code-review 9 reviewer 共识:架构修复正确,关键风险集中在 `__module__` 变化与 `import agentkit.core` 传递依赖面扩大;7 reviewer 共识要求 re-export identity 契约必须有自动化守护测试。
|
||||
- ce-simplify-code 简化:初版用 3 个独立 `import ... as ...; globals()[...] = ...`,简化为 for 循环 + `getattr`;测试 helper 提取 3 个 `_assert_*` 函数。
|
||||
- PEP 562(Module level `__getattr__` and `__dir__`):https://peps.python.org/pep-0562/
|
||||
- 项目约定:所有 Pydantic 模型用 `model_config = ConfigDict(...)`、Python >= 3.11、`ruff check src/ && ruff format src/`(行宽 100)。
|
||||
|
|
@ -0,0 +1,98 @@
|
|||
---
|
||||
title: "Gate without caller silently disables feature"
|
||||
date: 2026-07-06
|
||||
module: experts/orchestrator, core/reflexion
|
||||
tags: [gate, trigger_reason, dead-code, design-pattern, code-review]
|
||||
problem_type: logic_error
|
||||
severity: P1
|
||||
resolution_type: code_fix
|
||||
category: logic-errors
|
||||
---
|
||||
|
||||
# Gate without caller silently disables feature
|
||||
|
||||
## Problem
|
||||
|
||||
`_decompose_task` 新增了 `trigger_reason: str | None = None` 参数和门控逻辑,用于控制 `retrieve_prompt_reflection` 的调用。但两个生产调用方都不传 `trigger_reason`,导致门控条件恒为 False,功能被静默禁用。
|
||||
|
||||
## Symptoms
|
||||
|
||||
- `retrieve_prompt_reflection` 在生产中永远不会被调用(死代码)
|
||||
- 无错误日志,无异常 — 功能只是"消失"了
|
||||
- 单元测试全部通过(因为测试直接传入 `trigger_reason="verify_retry"`,未覆盖生产调用路径)
|
||||
- 修复前是"每次分解都检索"(浪费 token),修复后变成"永不检索"(功能丢失)
|
||||
|
||||
## What Didn't Work
|
||||
|
||||
1. **单元测试无法发现**:测试直接调用 `_decompose_task(..., trigger_reason="verify_retry")`,验证门控逻辑本身正确,但未覆盖生产调用路径(两个调用方都不传 trigger_reason)
|
||||
2. **功能测试无法发现**:`retrieve_prompt_reflection` 返回空时不报错,只是降级到默认 prompt,行为差异不明显
|
||||
3. **类型检查无法发现**:`trigger_reason` 有默认值 `None`,类型检查通过
|
||||
|
||||
## Solution
|
||||
|
||||
在 `_rebalance` 路径传入 `trigger_reason="loop_detection"`,使重新规划时检索历史反思:
|
||||
|
||||
```python
|
||||
# 修复前 — 两个调用方都不传 trigger_reason
|
||||
phases = await self._decompose_task(lead, task) # 正常分解
|
||||
new_phases = await self._decompose_task(lead, augmented_task) # 重分解
|
||||
|
||||
# 修复后 — _rebalance 路径传入 trigger_reason
|
||||
new_phases = await self._decompose_task(
|
||||
lead, augmented_task, trigger_reason="loop_detection"
|
||||
)
|
||||
```
|
||||
|
||||
并在正常分解路径添加注释说明 verify/schema 重试路径待接入。
|
||||
|
||||
## Why This Works
|
||||
|
||||
门控设计的核心假设是"有调用方会传入触发条件"。当这个假设不成立时,门控会静默禁用功能:
|
||||
|
||||
- `trigger_reason: str | None = None` — 默认 None
|
||||
- `None not in frozenset({"verify_retry", ...})` — 门控条件恒为 False
|
||||
- 无错误、无日志、无异常 — 功能只是"消失"了
|
||||
|
||||
修复方式是确保至少有一个调用方传入触发条件。`_rebalance` 是重新规划路径,传入 `loop_detection` 语义合理(检测到规划问题后重新规划)。
|
||||
|
||||
## Prevention
|
||||
|
||||
### 1. 门控设计检查清单
|
||||
|
||||
设计门控/开关时,必须:
|
||||
- 列出所有调用方,确保至少有一个会传入触发条件
|
||||
- 添加断言或日志:如果门控条件从未为 True,发出警告
|
||||
- 在 docstring 中明确说明"调用方必须传入 X 才能触发 Y"
|
||||
|
||||
### 2. 测试覆盖生产调用路径
|
||||
|
||||
```python
|
||||
# 不要只测试门控逻辑本身
|
||||
async def test_gate_with_trigger():
|
||||
await decompose_task(lead, task, trigger_reason="verify_retry")
|
||||
assert retrieve_called # 这只验证门控逻辑
|
||||
|
||||
# 还要测试生产调用路径
|
||||
async def test_production_call_path_does_not_silently_disable():
|
||||
# 模拟生产调用(不传 trigger_reason)
|
||||
await decompose_task(lead, task)
|
||||
# 如果功能应该被禁用,assert 它被禁用
|
||||
# 如果功能不应该被禁用,assert 它仍然工作
|
||||
```
|
||||
|
||||
### 3. 代码审查重点
|
||||
|
||||
审查门控逻辑时,追踪所有调用方:
|
||||
- 门控条件是否可达?
|
||||
- 是否有调用方传入触发条件?
|
||||
- 默认值是否会导致功能被静默禁用?
|
||||
|
||||
### 4. 死代码检测
|
||||
|
||||
定期检查从未执行的代码路径。`retrieve_prompt_reflection` 如果从未被调用,是死代码的信号。
|
||||
|
||||
## Related
|
||||
|
||||
- `docs/solutions/runtime-errors/async-generator-yield-before-await-deadlock.md` — 同一 IQ-boost 工作中的另一个非显而易见 bug
|
||||
- PR #27 (feat/agent-iq-boost) — 原始 trigger_reason 门控引入
|
||||
- PR fix/agent-iq-boost-gaps — 本次修复
|
||||
Loading…
Reference in New Issue