From c6a52a36a8186d42fe20a0692586de842dddd0f8 Mon Sep 17 00:00:00 2001 From: qyqy Date: Fri, 11 Sep 2026 19:15:50 +0800 Subject: [PATCH] =?UTF-8?q?docs:=20=E6=96=B0=E5=A2=9E=E3=80=8A=E8=AF=84?= =?UTF-8?q?=E5=AE=A1=E6=84=8F=E8=A7=81=E5=9B=9E=E5=A4=8D=E3=80=8B=EF=BC=8C?= =?UTF-8?q?=E9=80=90=E6=9D=A1=E5=AF=B9=E7=85=A7=E6=9E=B6=E6=9E=84=E5=B8=88?= =?UTF-8?q?=E7=9A=84=E8=AF=84=E5=AE=A1=E6=84=8F=E8=A7=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 按评审意见结构逐条回复,每条都给实测证据与落地位置: - §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 列出需要架构师确认的三件事(配置补发归属、接线归属、两处让号) --- docs/评审意见回复-NL_develop.md | 251 ++++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 docs/评审意见回复-NL_develop.md diff --git a/docs/评审意见回复-NL_develop.md b/docs/评审意见回复-NL_develop.md new file mode 100644 index 0000000..4fe9f1c --- /dev/null +++ b/docs/评审意见回复-NL_develop.md @@ -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 参数化测试把它锁住了。