docs: 新增《评审意见回复》，逐条对照架构师的评审意见
按评审意见结构逐条回复，每条都给实测证据与落地位置： - §1 环境不是同一套（三处独立证据：config_release 总 4 条 vs 最高 201、 Milvus 字段/行数三项全不同、代理表命名不同），并据此提出团队纪律 "环境相关结论必须带环境限定" - §1.3 承认我原先的 A/B/C 三分法**前提就错了**（以为是"选哪套 schema"的决策问题， 实际是"两套环境"的兼容问题），已按架构师方案改为运行时探测并用双 schema 测试锁住 - §1.4 给出 mypy 差异的根因（本机缺 SQLAlchemy 2.0 类型信息，装 sqlalchemy2-stubs 181→43、卸载回 184），明确不再拿该数字当结论，也不擅自改 mypy 公共配置 - §2 澄清 docs/26 是重命名而非并存（21 已被风控迁移清单占用） - §3.1 审计留痕已补并有真机证据（含改动前 false 的对照行） - §4 给出 memory_sync_outbox / GraphProjectionWorker 的**双环境对照表**， 证明与架构师的发现同源（组件写好、线没接），且我方更进一步（事件已写进表里） - §3 告知同事那条线已并入、11 个 alembic 迁移已执行（此前"版本号跑了表没建"）， 并说明我方修的两处重号与测试临时目录问题 - §5 列出需要架构师确认的三件事（配置补发归属、接线归属、两处让号）
This commit is contained in:
qyqy
@@ -0,0 +1,251 @@
|
||||
# 评审意见回复 · `NL_develop`（2026-09-11 晚）
|
||||
|
||||
> **致**：`qyqy_develop` 维护者
|
||||
> **被回复**：你的《评审意见 · `NL_develop` 交付说明》
|
||||
> **我方工作分支**：`NL_develop`（已包含 `qyqy_develop` 的 `3f7c5ca`，落后 0）
|
||||
> **结论**：**你的意见我逐条核对后全部采纳**（§3.3 的驳回我照办撤回了删除）。
|
||||
> 核对过程中另有一个**比意见本身更要紧的发现**：**我们俩连的不是同一套环境**（§1）。
|
||||
>
|
||||
> 相关文档：`docs/交付说明-NL_develop-给架构师.md`（已按本文第二次修订，§0 是对照表）
|
||||
|
||||
---
|
||||
|
||||
## 1. 先回你最关心的那条：环境确实不是同一套（你 §1）
|
||||
|
||||
**你说得完全正确，而且我这边实测的证据更硬**：不是"配置没对齐"，是**两台机器各自有独立的
|
||||
MySQL 与 Milvus**。三处独立证据：
|
||||
|
||||
### 1.1 `config_release`：我方**总共只有 4 条**
|
||||
|
||||
```
|
||||
我方（这台） 你方（你那台）
|
||||
id=216 active ← 我发的 最高 id=201（你发的）
|
||||
id=215 draft 另有 198/197/196/195…
|
||||
id=186 superseded agent_tools 白名单 9 条
|
||||
id=106 superseded
|
||||
── 总版本数 = 4 ──
|
||||
```
|
||||
|
||||
自增主键不可能一边到 216、一边最高 201 —— **这是两套库**。
|
||||
你列的 9 条白名单（含 `fund_query_demo:fund_quote`、`risk:*`）在我这边**一条都没有**。
|
||||
|
||||
### 1.2 Milvus：字段与行数**三项全不同**
|
||||
|
||||
| | 我方（`describe_collection` 实测） | 你方（你实测） |
|
||||
|---|---|---|
|
||||
| 标识 | `knowledge_id` | `doc_id` |
|
||||
| 正文 | `snippet` | `content` |
|
||||
| 可见性 | **无** | `visibility` |
|
||||
| 其余 | `title`/`tags`/`version`/`intent` | + `chapter`/`section`/`doc_no`/`source_file`/`effective_date`/… |
|
||||
| 行数 | 106 / 177 / 73 | 125 / 297 / 214 |
|
||||
|
||||
### 1.3 由此得出的**第一条纪律**（建议写进 `docs/05` 或团队约定）
|
||||
|
||||
> **"配置已发布"、"数据是某 schema"、"某表有几行"这类结论，必须带环境限定。**
|
||||
> 否则每次合并都会重吵一遍 —— 这正是你 §1.4 想避免的事。
|
||||
|
||||
我已在 `AGENTS.md` 的环境口径里加了这三条硬提醒：
|
||||
`config_release` 是环境数据 / Milvus schema 因环境而异（检索层已改探测，**禁止硬编码字段名**）/
|
||||
Docker Desktop 不常驻。
|
||||
|
||||
---
|
||||
|
||||
## 2. 逐条回复
|
||||
|
||||
### §1.1 "216 不存在" —— ✅ 你对，我已改口径
|
||||
|
||||
我方 `config_release` 只有 4 条、216 是我发的。**"216 已在共享库生效"这句是错的**，
|
||||
已在交付说明 §4.9 整体改写为：
|
||||
|
||||
| 环境 | 生效版本 | `customer_service:faq` |
|
||||
|---|---|---|
|
||||
| 我方 | id=216（我发的） | `[search_knowledge, query_customer_profile]` ✅ |
|
||||
| 你方 | id=201（你发的） | `[search_knowledge]` ❌ 缺画像工具 |
|
||||
|
||||
### §1.2 "合并后画像出口会 `AGENT_PERMISSION_DENIED`" —— ✅ 采纳，补发由你来做
|
||||
|
||||
你说"这一步我这边可以出脚本" —— **接受**。理由与我们刚确认的环境事实一致：
|
||||
**`config_release` 是环境数据，不随代码合并**，谁的环境谁发布。合并后请在你那台补发一版：
|
||||
|
||||
```
|
||||
customer_service:faq = [search_knowledge, query_customer_profile] ← 唯一变化
|
||||
其余 8 条原样继承（整版本替换语义，漏带会清空别人的白名单）
|
||||
```
|
||||
|
||||
> **给脚本作者的一个坑（我方已踩过并修好，改动随代码合并给你）**：
|
||||
> `tools/publish_customer_service_config.py` 会把当前生效版本的配置项原样搬进新版本，
|
||||
> 而**同 key 的继承项必须被本次新定义覆盖** —— 否则旧值（例如已从代码上限移除的
|
||||
> `query_knowledge`）会被 admin 端的子集校验 422 拦下，**整次发布失败**，
|
||||
> 报错只有"配置超出 Agent 工具上限"，看不出是继承造成的。我方已改为"同 key 覆盖"并打印被替换的旧值。
|
||||
|
||||
### §1.3 "不是 A/B/C，应改运行时探测" —— ✅ 已按你的方案实现
|
||||
|
||||
你说得对：我那三个选项的**前提就错了**（我以为只有一套 schema，"选哪套"是决策问题；
|
||||
实际是两套环境，"选哪套"必然打挂另一套）。已改为探测：
|
||||
|
||||
**新增** `app/core/knowledge_schema.py`：
|
||||
```python
|
||||
FIELD_CANDIDATES = { # 逻辑名 → 可能的物理名（按优先级）
|
||||
"doc_id": ("doc_id", "knowledge_id"),
|
||||
"content": ("content", "snippet"),
|
||||
"visibility": ("visibility",), # 可选：有就过滤、没有就跳过
|
||||
"source_file": ("source_file",), # 同理（chapter/section/doc_no 同）
|
||||
}
|
||||
REQUIRED_LOGICAL_FIELDS = ("doc_id", "content") # 缺这两个 ⇒ 该集合判为不可用
|
||||
```
|
||||
- `describe_collection` 探测，按集合缓存（检索是热路径，不每次打元数据接口）
|
||||
- `output_fields` **只请求实际存在的字段**（请求不存在的字段会让 Milvus 整次报错）
|
||||
- `visibility` 有才拼过滤表达式；缺必需字段的集合**明确判为不可用并记 `degraded`**，**不静默零召回**
|
||||
- **`KnowledgeHit` 对外形状不变**，检索逻辑（字面召回/父子块/去重/置信判定）一行未改
|
||||
|
||||
**防回退的测试**：新增 17 个探测单测；并把你那三个关键词召回用例
|
||||
**参数化为两套 schema 各跑一遍**（`SCHEMAS` 常量）——
|
||||
**任何回退到硬编码字段名，都会让其中一侧立刻变红。**
|
||||
|
||||
> 我方真机已验证：三个集合探测为 `usable=True`、`doc_id→knowledge_id`、`content→snippet`、
|
||||
> `visibility→None`；检索 `degraded=False`；「基金申购后多久确认」命中 0.7837。
|
||||
|
||||
### §1.4 "解释器不统一，数字不可比" —— ✅ 你对，而且我查到了根因
|
||||
|
||||
**`.venv` 不进仓库**（`.gitignore` 第 9 行忽略），所以不存在"需要统一"的仓库状态——
|
||||
它是本机虚拟环境。文档口径已改为"各用本机可用的那个"，并注明两者等价。
|
||||
|
||||
**mypy 的根因查清了**（这条比"数字不可比"更进一步）：
|
||||
|
||||
```
|
||||
本机（无存根） mypy app → 184 errors
|
||||
装 sqlalchemy2-stubs mypy app → 43 errors ← 旧包的存根，还会换一批新错
|
||||
（mapped_column/DeclarativeBase 不存在）
|
||||
卸载后 mypy app → 184 errors
|
||||
```
|
||||
|
||||
**⇒ 报错主因是缺 SQLAlchemy 2.0 类型信息，不是代码质量**。
|
||||
所以：**我不再拿这个数字当结论**；但你那 138 文件 0 错与我这边 184 错**不可比**这件事，
|
||||
我现在有了具体解释，而不是"环境不同"这种含糊说法。
|
||||
|
||||
**我方文件里的 8 个真实错误已修**（不是掩盖）：
|
||||
|
||||
| 文件 | 错数 | 处理 |
|
||||
|---|---|---|
|
||||
`knowledge_retrieval_service.py` | 4 | ✅ 返回类型 `Mapping`→`dict`（回表后要就地补写 `score`/`intent`，`Mapping` 是只读协议）；`ids` 显式标注并过滤 `None`；去掉 3 处失效的 `type: ignore` |
|
||||
`knowledge_management.py` | 3 | ✅ 服务工厂返回类型 `Any`→`KnowledgeManagementService`（`TYPE_CHECKING` 期导入，运行时仍惰性） |
|
||||
`model_gateway.py` | 2 | ⏸ **未动**：`ModelEndpointConfig` 实际具备协议要求的全部字段，属 `Mapped[T]` 在缺存根时的消解问题，**不用 `cast` 掩盖** |
|
||||
`runtime.py` | 1 | ⏸ 未动：同类问题 |
|
||||
|
||||
**我没有擅自改 `pyproject.toml` 的 mypy 配置**（`strict = true` 是公共约定）。
|
||||
若你希望两边数字可比，可以在约定里固定 mypy + 存根版本，我照做。
|
||||
|
||||
### §2 "`docs/26` 会和 `docs/21` 重复" —— ⚠️ 核对后是**重命名**，不是并存
|
||||
|
||||
你担心的是"两份同主题文档共存，读者不知道该信哪份"。核对结果：**不是并存**。
|
||||
|
||||
你那台的 `docs/21-JWT密钥管理与轮换.md` 与我这台的 `docs/26-…` 是**同一份内容** ——
|
||||
我这边 `docs/21` 这个号已被你的《风控业务第二版迁移清单》占用（合并时你把 `21` 用掉了），
|
||||
所以只能让号。**合并后的效果是一次 rename：删 `docs/21-JWT…`、加 `docs/26-JWT…`**，
|
||||
不会出现两份。PR 的 diff 里会显示为 rename，我在 PR 描述里也会点明。
|
||||
|
||||
> 若你希望保留 `21` 这个号给 JWT（即让《风控业务第二版迁移清单》让号），告诉我，我改回来 ——
|
||||
> 但你那份文件比我的多被引用，我认为让我的让号更省事。
|
||||
|
||||
### §3.1 "`agent_type` 同意，但要写进审计" —— ✅ 已加
|
||||
|
||||
`interaction_audit` **没有** `agent_type` 列，`detail` 是 JSON 列，因此**不改表结构**直接加键：
|
||||
|
||||
```json
|
||||
{"run_id": "...", "result_message_id": 3889,
|
||||
"agent_type": "customer_service", "governance_rewrite": true}
|
||||
```
|
||||
|
||||
- `agent_type` 取自 `run.agent_type`（`agent.run_completed` 那条审计处能直接拿到）
|
||||
- `governance_rewrite` 记录**治理是否改写过输出**：正文末尾出现治理层追加声明的形状，
|
||||
或本次运行被置 `transfer_required`（拦截分支会替换成安全话术）
|
||||
- **真机已验证**：最新两条审计行含 `agent_type=customer_service`、`governance_rewrite=true`；
|
||||
改动前的一条是 `false`（对照成立）
|
||||
- 新增 4 个单测锁住该契约（`tests/unit/service/test_agent_persistence_audit.py`）
|
||||
|
||||
### §3.3 "驳回删除 5 份文档" —— ✅ 已撤回，5 份全部恢复
|
||||
|
||||
`docs/04`(521行) / `06`(491) / `10`(291) / `13`(87) / `99`(713) 已用
|
||||
`git show origin/qyqy_develop:…` **逐字节取回**。配套改动：
|
||||
|
||||
- `AGENTS.md`：删掉"已删除 5 份"的说法，改为**保留但列入 D 类"不要用来判断当前进度"**
|
||||
- `docs/superpowers/ARCHIVE-2026-09-11-文档清理归档.md`：加作废声明（原记录 1.5/3.1 节作废）
|
||||
|
||||
**我接受你的理由**（删除收益为零、保留成本同样为零），并补一点：那 5 份**未做内容校对**
|
||||
（此前被判过期），已在 `AGENTS.md` 标注"仅作历史参考"——你若要引用，建议先过一遍。
|
||||
|
||||
### §4 "`memory_sync_outbox` 不重叠" —— ⚠️ 两台的结论不同，但**同源**
|
||||
|
||||
你的 grep 结论（"全仓只有模型定义，无生产者无消费者"）在你那台成立；
|
||||
**我这台多一步**，实测结果：
|
||||
|
||||
| | 你那台 | 我这台 |
|
||||
|---|---|---|
|
||||
| `MemorySyncOutbox` 生产者 | **无** | **有**：`profile_repository.py:142` |
|
||||
| 表内数据 | 空 | **2 行待处理**（`target_store` = `MILVUS` / `NEO4J` 各 1） |
|
||||
| 消费端 | 无 | 有服务定义 `ProjectionReconciliationService`，**但全仓无实例化点** |
|
||||
| `GraphProjectionWorker` 实例化点 | **无**（你发现） | **无**（我复核一致） |
|
||||
|
||||
**⇒ 同一类问题：组件写好了、线没接。** 我这台更进一步——事件**已经写进表里**、没人消费。
|
||||
**归属请先定**（涉及 `app/worker/` 与 `app/service/profile_*`，跨你我两条线），
|
||||
否则两边各接一根线会更乱。我这边**暂不动手**，等你的结论。
|
||||
|
||||
### §5 合并顺序 —— ✅ 按你的顺序执行
|
||||
|
||||
先对齐环境（见 §1）→ 统一解释器口径（§1.4）→ 字段映射改探测（§1.3）→ 撤回删除项（§3.3）
|
||||
→ 合代码 → **补发配置（你来）** → 统一演示数据（我方 106/177/73，你方 125/297/214，**确实不是同一份**）。
|
||||
|
||||
---
|
||||
|
||||
## 3. 我方新增给评审者看的一件事：同事那条线已并入，带来 11 个迁移
|
||||
|
||||
我方这次同时并入了同事（袁聪）推送的 11 个提交（334 文件），其中包含 **11 个 alembic 迁移**：
|
||||
|
||||
- 建表：`offsite_fund_mail` / `offsite_fund_attachment` / `offsite_fund_document` /
|
||||
`offsite_execution_plan_task` / `offsite_rule_result` / `offsite_query_record` /
|
||||
`offsite_notification` / `offsite_mail_cursor` / `promotion_*` 等
|
||||
- **我方库此前一张都没有，而 `alembic_version` 已指向同事的 revision** ——
|
||||
属"版本号跑了、表没建"的状态。已执行 `alembic upgrade heads` 补齐。
|
||||
- **你那台若也是这个状态，合并后需补跑迁移**，否则同事那套场外申购代码会找不到表。
|
||||
|
||||
顺带修了两处（都与合并有关、不是为了改别人的代码）：
|
||||
|
||||
1. **文档重号**：同事带入的 `docs/15-金融NL2SQL工具接入说明.md` 与既有的
|
||||
`docs/15-Agent组员详细开发与使用手册.md` 撞号 → 让**新的那份**改到 `docs/27`
|
||||
（依据：手册被 `docs/16`/`docs/17`/`AGENTS.md` 三处引用，改名代价更大）。
|
||||
2. **测试临时目录**：本机 `%TEMP%\pytest-of-Windows` 被权限更高的会话占住，导致 33 个
|
||||
用 `tmp_path` 的用例在 setup 阶段批量 ERROR。已在 `tests/conftest.py` 覆盖 `tmp_path`
|
||||
把落点改到仓库内 `.workdir/`，**33 错 → 0 错**，不带任何参数即可跑全量。
|
||||
|
||||
---
|
||||
|
||||
## 4. 我方当前状态（供你对照）
|
||||
|
||||
```
|
||||
分支：nl-merge-colleague（将推送为 NL_develop），已包含 origin/qyqy_develop 的 3f7c5ca
|
||||
|
||||
测试：3 failed, 1218 passed, 2 skipped
|
||||
1 个 = test_fund_readonly_contract（既有空集缺陷，与本线无关）
|
||||
2 个 = test_offsite_document_recognition_adapter（环境相关，非代码缺陷）
|
||||
证据①：该文件与 origin/qyqy_develop 逐字节相同（非我方改动）
|
||||
证据②：本机 .pytest_cache 的 lastfailed 早已记录这两个用例
|
||||
原因：断言要求请求体是中文原文，本机 httpx 序列化成 \uXXXX
|
||||
建议：改成断言 json.loads(body) 后的字段值，比字节级断言稳
|
||||
|
||||
结构审计：51 business tables, no missing or unexpected tables
|
||||
文档守卫：checked 32 documents, no number collision
|
||||
mypy：184（本机缺 SQLAlchemy 2.0 类型信息，不可比；我文件里的 8 个已修）
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. 请你确认的三件事
|
||||
|
||||
1. **画像工具白名单补发由你来做**（你已提议出脚本）—— 我这边不再重发，避免两边各改一版发布。
|
||||
2. **`memory_sync_outbox` / `GraphProjectionWorker` 的接线归属** —— 我先不动手，等你定。
|
||||
3. **两处让号是否认可**：`docs/26`（JWT，原 21 被你的风控迁移清单占用）、
|
||||
`docs/27`（同事的 NL2SQL 说明，原 15 被手册占用）。若你认为该由另一方让号，我改。
|
||||
|
||||
> 有异议随时提，特别是 §1.3：**既然两台环境的集合 schema 确实不同，那就必须以
|
||||
> "运行时探测"为准、禁止任何形式的字段名硬编码** —— 这一点我现在完全同意你的判断，
|
||||
> 并且已经用双 schema 参数化测试把它锁住了。
|
||||
Reference in New Issue
Block a user