From c429851c97af4305a0b418c40a8ee0a04c2151e3 Mon Sep 17 00:00:00 2001 From: Chiguyong Date: Mon, 6 Jul 2026 23:51:20 +0800 Subject: [PATCH] =?UTF-8?q?docs:=20=E6=B2=89=E6=B7=80=20PEP=20562=20re-exp?= =?UTF-8?q?ort=20=E6=9E=B6=E6=9E=84=E6=A8=A1=E5=BC=8F=20+=20=E6=96=B9?= =?UTF-8?q?=E6=A1=88=E6=96=87=E6=A1=A3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 经验 --- ...x-architecture-smell-and-test-gaps-plan.md | 246 +++++++++++++++ .../circular-import-pep562-lazy-reexport.md | 298 ++++++++++++++++++ ...ithout-caller-silently-disables-feature.md | 98 ++++++ 3 files changed, 642 insertions(+) create mode 100644 docs/plans/2026-07-06-003-fix-architecture-smell-and-test-gaps-plan.md create mode 100644 docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md create mode 100644 docs/solutions/logic-errors/gate-without-caller-silently-disables-feature.md diff --git a/docs/plans/2026-07-06-003-fix-architecture-smell-and-test-gaps-plan.md b/docs/plans/2026-07-06-003-fix-architecture-smell-and-test-gaps-plan.md new file mode 100644 index 0000000..83c72ca --- /dev/null +++ b/docs/plans/2026-07-06-003-fix-architecture-smell-and-test-gaps-plan.md @@ -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/） diff --git a/docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md b/docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md new file mode 100644 index 0000000..cd5706e --- /dev/null +++ b/docs/solutions/architecture-patterns/circular-import-pep562-lazy-reexport.md @@ -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）。 diff --git a/docs/solutions/logic-errors/gate-without-caller-silently-disables-feature.md b/docs/solutions/logic-errors/gate-without-caller-silently-disables-feature.md new file mode 100644 index 0000000..d3cc261 --- /dev/null +++ b/docs/solutions/logic-errors/gate-without-caller-silently-disables-feature.md @@ -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 — 本次修复