Files
group_fqcd_jr/docs/评审意见回复-NL_develop.md
qyqy c6a52a36a8 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 列出需要架构师确认的三件事(配置补发归属、接线归属、两处让号)
2026-09-11 19:15:50 +08:00

14 KiB
Raw Permalink Blame History

评审意见回复 · 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:

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 列,因此不改表结构直接加键:

{"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 参数化测试把它锁住了。