feat(llm): 交付开关拆分为中间轮/最终轮两域 - #237
Conversation
- 配置键 agent_delivery_intermediate_enabled / agent_delivery_final_enabled 独立控制两域，旧键 agent_delivery_enabled 读取时按两域同值映射并告警 - group_settings 增 agent_delivery_intermediate / agent_delivery_final 两列，加列时一次性回填旧值（与 reset 语义解耦），旧列退役只读 - Recorder 按域路由逐 Turn 交付计划；中间轮关闭的 suppressed 项显式排除出 sink 发送清单 - /llm delivery 扩为 interim|final|all 三域子命令，无域 status 输出两域概览；Web Admin 群设置双三态开关 - 恢复此前不可达的组合：中间轮照常发送 + 最终正文整条单发；补迁移回填、四组合行为矩阵、抑制不外发与配置映射测试
- set_chat_agent_delivery_enabled 未知 domain 改抛 ValueError，不再静默兜底双域写 - 补组合 B（仅中间轮开）首段失败静默终止与组合 C（仅最终轮开）suppressed 不外发 + 最终首段失败的端到端断言 - 组合 B 断言 agent_turn_row_id 回填钩子齐全（record_final_receipt 消费路径）
![khpilot[bot]](https://avatars.githubusercontent.com/u/178400596?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bot Review — 💬 Comment
本 PR 将 agent_delivery_enabled 单开关拆分为中间轮（intermediate）和最终轮（final）两域独立开关，覆盖配置层、存储层、Recorder、service 层、命令层和 Web Admin。general reviewer 覆盖率仅 12/24（1 个失败批次），无法对未覆盖的 12 个 patch 单元作出完整的阻塞性判断，style reviewer 完成了全量 24/24 审查并产出 6 个 should-fix 和 4 个 nit 发现。按 coverage 元数据规则，incomplete 的一般审查不能支撑 approve 或 request_changes 裁定，verdict 降级为 comment。
General reviewer 唯一保留发现（nit 级别）：回填 SQL 用单一 OR 标志同时触发两列 UPDATE，可能在历史半迁移状态（仅缺一列）下跨列覆写已有用户值；该路径实际触发面小。Style reviewer 最重要的 should-fix 是 delivery 命令分支膨胀为 god function、跨层近义词（interim/intermediate/final）需维护映射表、status 分支控制流隐含依赖 finish 终止语义、域参数缺乏枚举建模导致静默默认值行为等问题，均值得在合入前处理。
Findings
🔴 BLOCKING: General reviewer coverage: 12/24 units completed, 1 failed batch. 未完成审查的 12 个 patch 单元中可能存在阻塞性缺陷，无法通过现有证据排除。
File: coverage
General reviewer 需完成全部 24 个 patch 单元的审查并解决失败批次问题后，再提交最终裁定。
🟡 SHOULD-FIX: delivery 子命令 handler 在已膨胀的 llm_cmd.handle god function 中又扩了约 40 行（tokens 解析、域映射表、闭包、中文文案），多类职责进一步叠加。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
抽成模块级 async def _handle_delivery_command(llm_cmd, svc, *, chat_id, chat_type, scope_label, tokens) -> None，handler 只留 if 分支调用。
🟡 SHOULD-FIX: 同一语义在本 PR 中用了三套近义词：命令 token interim、配置属性 agent_delivery_intermediate_enabled、存储字段 agent_delivery_intermediate（与旧键相比还丢了 _enabled 后缀）。需长期维护翻译表 service_domain = {'interim': 'intermediate', ...}。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
统一域词汇：命令 token 也用 intermediate（/llm delivery intermediate|final|all …），或若坚持短词 interim 则在配置/存储命名上也同步为同一词根；删除映射表。
🟡 SHOULD-FIX: status 分支控制流隐含依赖 finish 必然终止：''all'' 分支里 finish 后还继续执行单域逻辑，domain=='' 时 KeyError、domain=='all' 时重复发送。当前写法脆弱。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
拆成显式互斥分支：if domain in ('', 'all'): await llm_cmd.finish(overview) / else: await llm_cmd.finish(single_domain_line)。
🟡 SHOULD-FIX: 新增的 domain: str = 'all' 是裸字符串参数，手工 if/elif 分派，与本仓库用 enum.StrEnum 建模有限取值集的既有做法相悖；默认值 'all' 使漏传域时静默同时写两列无任何提示。
File: src/quickquip/llm/service_parts/state.py
定义 class DeliveryDomain(enum.StrEnum): INTERMEDIATE = 'intermediate'; FINAL = 'final'; ALL = 'all'，签名改为 domain: DeliveryDomain = DeliveryDomain.ALL 并用 match 分派；命令层只在解析 token 的一处做 interim -> DeliveryDomain.INTERMEDIATE 映射。
🟡 SHOULD-FIX: test_delivery_override_false_beats_global_on_and_reset_follows 的 docstring 说「reset 单域只影响该域，另一域覆盖保留」，但测试只写过 intermediate 一域（domain='intermediate'），assert override.agent_delivery_final is None 是空断言；行为复跑用的 group_id=1003 从未设过任何覆盖，与被测对象无关。测试 oracle 与契约不一致。
File: tests/integration/test_agent_delivery_optin.py
先显式写入两域不同覆盖，reset 中间轮后断言 final 域保留，复跑改回 group_id=1001 用同一 scope 验证 reset 后回到全局默认。
⚪ NIT: 回填用的 added_delivery_split_columns 标志由两次独立 ALTER 任一命中置 True，随后 UPDATE 同时覆盖两列。若某库只缺其中一列（历史半迁移状态），另一列上用户已有的取值（包括 reset 后刻意置的 NULL）会被整体覆写。
File: src/quickquip/llm/store_parts/_base.py
按列分别门控回填：if 'agent_delivery_intermediate' not in existing_columns: 后紧接只写 intermediate 列的 UPDATE，final 列同理各自一段，避免跨列覆写。
⚪ NIT: _delivery_views() 返回无名四元组，调用侧只能靠位置丢弃前两项，domain_default 形参与外层变量同名遮蔽。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
返回具名结构（如 NamedTuple 或 dict）；_domain_default 形参改名（如 scope_domain）避免遮蔽。
Review provenance
- General reviewer: incomplete — 12/24 units completed, 1 failed batches
- Style reviewer: complete — 24/24 units completed
- Coverage: incomplete — manifest=complete; general=incomplete(1 failed batches); style=complete
- Reviewed head:
36bd0bc(base5a963fa)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
| default = "开" if svc.config.runtime.agent_delivery_enabled else "关" | ||
| await llm_cmd.finish(f"{scope_label}分段发送已跟随全局默认（当前：{default}）") | ||
| if sub == "status": | ||
| if tokens[:1] == ["delivery"]: |
There was a problem hiding this comment.
SHOULD-FIX: delivery 子命令 handler 在已膨胀的 llm_cmd.handle god function 中又扩了约 40 行（tokens 解析、域映射表、闭包、中文文案），多类职责进一步叠加。
抽成模块级 async def _handle_delivery_command(llm_cmd, svc, *, chat_id, chat_type, scope_label, tokens) -> None，handler 只留 if 分支调用。
| domain = rest[0] if rest and rest[0] in domain_labels else "" | ||
| action = rest[1] if len(rest) >= 2 else "" | ||
| # 命令域 token（interim）到 service 写入域（intermediate）的映射 | ||
| service_domain = {"interim": "intermediate", "final": "final", "all": "all"}.get(domain, "") |
There was a problem hiding this comment.
SHOULD-FIX: 同一语义在本 PR 中用了三套近义词：命令 token interim、配置属性 agent_delivery_intermediate_enabled、存储字段 agent_delivery_intermediate（与旧键相比还丢了 _enabled 后缀）。需长期维护翻译表 service_domain = {'interim': 'intermediate', ...}。
统一域词汇：命令 token 也用 intermediate（/llm delivery intermediate|final|all …），或若坚持短词 interim 则在配置/存储命名上也同步为同一词根；删除映射表。
| f"{scope_label}分段交付：中间轮 {current_intermediate}（默认 {default_intermediate}）" | ||
| f" / 最终轮 {current_final}（默认 {default_final}）" | ||
| ) | ||
| current = current_intermediate if domain == "interim" else current_final |
There was a problem hiding this comment.
SHOULD-FIX: status 分支控制流隐含依赖 finish 必然终止：''all'' 分支里 finish 后还继续执行单域逻辑，domain=='' 时 KeyError、domain=='all' 时重复发送。当前写法脆弱。
拆成显式互斥分支：if domain in ('', 'all'): await llm_cmd.finish(overview) / else: await llm_cmd.finish(single_domain_line)。
| chat_id: int | str, | ||
| enabled: bool | None, | ||
| chat_type: str = "group", | ||
| domain: str = "all", |
There was a problem hiding this comment.
SHOULD-FIX: 新增的 domain: str = 'all' 是裸字符串参数，手工 if/elif 分派，与本仓库用 enum.StrEnum 建模有限取值集的既有做法相悖；默认值 'all' 使漏传域时静默同时写两列无任何提示。
定义 class DeliveryDomain(enum.StrEnum): INTERMEDIATE = 'intermediate'; FINAL = 'final'; ALL = 'all'，签名改为 domain: DeliveryDomain = DeliveryDomain.ALL 并用 match 分派；命令层只在解析 token 的一处做 interim -> DeliveryDomain.INTERMEDIATE 映射。
| service.set_chat_agent_delivery_enabled(1001, None, chat_type="group", domain="intermediate") | ||
| override = service.store.get_group_settings("1001") | ||
| assert override.agent_delivery_intermediate is None | ||
| assert override.agent_delivery_final is None |
There was a problem hiding this comment.
SHOULD-FIX: test_delivery_override_false_beats_global_on_and_reset_follows 的 docstring 说「reset 单域只影响该域，另一域覆盖保留」，但测试只写过 intermediate 一域（domain='intermediate'），assert override.agent_delivery_final is None 是空断言；行为复跑用的 group_id=1003 从未设过任何覆盖，与被测对象无关。测试 oracle 与契约不一致。
先显式写入两域不同覆盖，reset 中间轮后断言 final 域保留，复跑改回 group_id=1001 用同一 scope 验证 reset 后回到全局默认。
| if "agent_delivery_final" not in existing_columns: | ||
| conn.execute("ALTER TABLE group_settings ADD COLUMN agent_delivery_final INTEGER") | ||
| added_delivery_split_columns = True | ||
| if added_delivery_split_columns: |
There was a problem hiding this comment.
NIT: 回填用的 added_delivery_split_columns 标志由两次独立 ALTER 任一命中置 True，随后 UPDATE 同时覆盖两列。若某库只缺其中一列（历史半迁移状态），另一列上用户已有的取值（包括 reset 后刻意置的 NULL）会被整体覆写。
按列分别门控回填：if 'agent_delivery_intermediate' not in existing_columns: 后紧接只写 intermediate 列的 UPDATE，final 列同理各自一段，避免跨列覆写。
| settings = svc.get_chat_settings(chat_id, chat_type=chat_type) | ||
| default = "开" if svc.config.runtime.agent_delivery_enabled else "关" | ||
| current = "开" if settings.agent_delivery_enabled else "关" | ||
| return ( |
There was a problem hiding this comment.
NIT: _delivery_views() 返回无名四元组，调用侧只能靠位置丢弃前两项，domain_default 形参与外层变量同名遮蔽。
返回具名结构（如 NamedTuple 或 dict）；_domain_default 形参改名（如 scope_domain）避免遮蔽。
- 域建模：DeliveryDomain StrEnum 枚举 + state.py match 分派，非法域在解析期拒绝 - 命令层抽模块级 _handle_delivery_command；域词汇统一为 intermediate/final/all（删除 interim→intermediate 翻译表）；status 概览与单域分支显式互斥；DeliveryViews 具名结构 + 形参遮蔽改名 - 迁移回填按列独立门控，半迁移状态下不跨列覆写既有值；补半迁移回归测试 - optin 测试 oracle 修正：两域异值覆盖 + reset 单域保留断言 + 同群 resolve 断言（同群行为复跑受五 Turn 剧本耗尽约束，以 resolve 钉住 reset 生效路径）
There was a problem hiding this comment.
Bot Review — ⛔ Request Changes
本 PR 将单域交付开关拆分为中间轮/最终轮两域独立开关，变更范围涵盖配置、存储、Recorder、service 层、命令与 Web Admin。coverage 完整，但 general reviewer 的两处 should-fix 发现真实的逻辑缺陷：① aborted_silently 用「任一域开启」判断，导致「中间轮抑制 + 最终轮未及交付时中止」场景下用户完全收不到消息也无提示（原注释中「零交付时静默尚可解释」的理由已不成立）；② state.py 的 match domain: 缺兜底分支，非法域值会静默返回而不写库也不报错。另有一处 style should-fix（_plan_deliveries 与 on_turn 对抑制判定各写一遍且条件写法不一致，多项计划项时会错漏）同样有隐蔽的运行时风险。其余 nit 均为命名重复或测试重复，可后续改进，不阻止合并。
Findings
🔴 BLOCKING: aborted_silently 判定条件从「总开关开启」改为「任一域开启」后，在「中间轮域关闭 + 最终轮域开启 + Loop 在最终正文交付前中止（request_budget_exceeded / delivery_limit / split_limit 等）」场景下，零条消息实际送达用户，但 aborted_silently 仍为 True，reply 被置空，用户既无正文也无中止提示，完全静默。原代码中「开关开启即静默」尚可解释为「已交付分段即为全部」，新条件将其扩展到零交付场景，注释中的解释已不成立。
File: src/quickquip/llm/service.py
改为基于「确实有过成功交付」判断：aborted_silently = recorder is not None and recorder.summary().sent > 0（sent 来自 DeliverySummary，表示可信成功交付数）；或至少收紧为 settings.agent_delivery_intermediate_enabled or (deliveries_happened)，避免中间轮抑制且最终轮未交付时的全静默。
🔴 BLOCKING: match domain: 只有 INTERMEDIATE/FINAL/ALL 三个 case，没有兜底分支。docstring 声称「非法域由 DeliveryDomain 枚举在解析期拒绝」，但该方法本身不做校验——传入大写 "ALL"、错误拼写或未来新 token 时所有 case 都不匹配，函数静默返回、既不写库也不抛错，调用方误以为覆盖已落库。测试中已在用裸字符串 "intermediate"/"final"/"all" 而非枚举，这条路径并非不会被走到。
File: src/quickquip/llm/service_parts/state.py
在 match 前加显式校验：domain = DeliveryDomain(domain)（非法值抛 ValueError）；或在末尾补 case _: raise ValueError(f"未知交付域：{domain!r}")；同时修正 docstring 中「解析期拒绝」的误导性表述。
🟡 SHOULD-FIX: 「非最终轮 + 中间轮域关闭 ⇒ 抑制」这一判定在 _plan_deliveries（追加 suppressed 计划项）和 on_turn（据 record.delivery_ids 构造排除清单）两处各写一次，条件写法还不一致。一旦 plan 的抑制分支改为产出多个/零个项，on_turn 会把 record.delivery_ids 全量当成抑制集合，deliver_turn 就可能漏发或多抑制，且无任何断言保护。
File: src/quickquip/llm/service_parts/agent_runtime.py
抑制决策只由一个归属者产出：_plan_deliveries 返回 (items, suppressed_ids)（或给 DeliveryPlanItem 加显式抑制标记），on_turn 直接消费该结果写库并缓存，不再自行复算 not intermediate and not is_final。同时把 _pending_record/_pending_text/_pending_plan/_pending_suppressed_ids 在 __init__ 中显式声明，避免继续用 getattr 默认值隐式扩展实例状态。
🟡 SHOULD-FIX: 旧键迁移告警只看旧键是否存在，不判断新键是否已显式配置。当部署已配置两枚新键时，日志仍打出「未显式配置的新键按旧键值映射」，误导运维以为旧值覆盖了新配置；且每次 load_llm_config（含 /llm reload）都重复打印。
File: src/quickquip/llm/config.py
把告警限定在真正发生了回退的情况：if legacy_agent_delivery is not None and ("agent_delivery_intermediate_enabled" not in runtime_raw or "agent_delivery_final_enabled" not in runtime_raw): logger.warning("[runtime] agent_delivery_enabled 已废弃：未显式配置的域正按旧键值映射，请迁移到新键")；两枚新键都已显式配置时改为 info/debug 的「旧键已失效、忽略」提示。
🟡 SHOULD-FIX: 两个新列的迁移块逐字重复（只有列名不同），回填规则在两处各写一份，后续再加域就是第三份拷贝；且 _ensure_schema（DDL 初始化函数）混入了一次性 DML 迁移，职责不再单一。
File: src/quickquip/llm/store_parts/_base.py
用列名元组循环表达同一规则：
for column in ("agent_delivery_intermediate", "agent_delivery_final"):
if column in existing_columns:
continue
conn.execute(f"ALTER TABLE group_settings ADD COLUMN {column} INTEGER")
conn.execute(f"UPDATE group_settings SET {column} = agent_delivery_enabled WHERE agent_delivery_enabled IS NOT NULL")可抽成 _backfill_delivery_domains(conn, existing_columns) 命名帮手，让 _ensure_schema 只保留列清单与调用。
🟡 SHOULD-FIX: 同一布尔量出现两种命名词序：agent_delivery_intermediate_enabled（配置键、ResolvedGroupSettings、DB 列/前端字段）与 intermediate_delivery_enabled（RecorderConfig）。跨层比对与全局搜索时容易漏改。
File: src/quickquip/llm/service_parts/agent_runtime.py
统一为 agent_delivery_intermediate_enabled / agent_delivery_final_enabled（与配置键、设置解析、Web 字段同词根），或在 RecorderConfig docstring 中明确说明并全仓统一命名。
Review provenance
- General reviewer: complete — 24/24 units completed
- Style reviewer: complete — 24/24 units completed
- Coverage: complete
- Reviewed head:
171fa7d(base5a963fa)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
| # 同样可能在这里终止（逐轮预算门禁不依赖 recorder），但它没有任何 | ||
| # sink 交付，必须给出可见的中止提示而不是空串。 | ||
| aborted_silently = recorder is not None and settings.agent_delivery_enabled | ||
| aborted_silently = recorder is not None and ( |
There was a problem hiding this comment.
BLOCKING: aborted_silently 判定条件从「总开关开启」改为「任一域开启」后，在「中间轮域关闭 + 最终轮域开启 + Loop 在最终正文交付前中止（request_budget_exceeded / delivery_limit / split_limit 等）」场景下，零条消息实际送达用户，但 aborted_silently 仍为 True，reply 被置空，用户既无正文也无中止提示，完全静默。原代码中「开关开启即静默」尚可解释为「已交付分段即为全部」，新条件将其扩展到零交付场景，注释中的解释已不成立。
改为基于「确实有过成功交付」判断：aborted_silently = recorder is not None and recorder.summary().sent > 0（sent 来自 DeliverySummary，表示可信成功交付数）；或至少收紧为 settings.agent_delivery_intermediate_enabled or (deliveries_happened)，避免中间轮抑制且最终轮未交付时的全静默。
| """按域写交付开关覆盖；非法域由 DeliveryDomain 枚举在解析期拒绝。""" | ||
| value = None if enabled is None else int(enabled) | ||
| self._update_chat_settings(chat_id, chat_type, agent_delivery_enabled=value) | ||
| match domain: |
There was a problem hiding this comment.
BLOCKING: match domain: 只有 INTERMEDIATE/FINAL/ALL 三个 case，没有兜底分支。docstring 声称「非法域由 DeliveryDomain 枚举在解析期拒绝」，但该方法本身不做校验——传入大写 "ALL"、错误拼写或未来新 token 时所有 case 都不匹配，函数静默返回、既不写库也不抛错，调用方误以为覆盖已落库。测试中已在用裸字符串 "intermediate"/"final"/"all" 而非枚举，这条路径并非不会被走到。
在 match 前加显式校验：domain = DeliveryDomain(domain)（非法值抛 ValueError）；或在末尾补 case _: raise ValueError(f"未知交付域：{domain!r}")；同时修正 docstring 中「解析期拒绝」的误导性表述。
| if not self._config.agent_delivery_enabled and not is_final: | ||
| # 中间轮关闭的非最终 suppressed 交付落 planned 即收敛为 suppressed； | ||
| # 同批 ID 记入待发排除清单，deliver_turn 不得经 sink 发出抑制正文。 | ||
| if not self._config.intermediate_delivery_enabled and not is_final: |
There was a problem hiding this comment.
SHOULD-FIX: 「非最终轮 + 中间轮域关闭 ⇒ 抑制」这一判定在 _plan_deliveries（追加 suppressed 计划项）和 on_turn（据 record.delivery_ids 构造排除清单）两处各写一次，条件写法还不一致。一旦 plan 的抑制分支改为产出多个/零个项，on_turn 会把 record.delivery_ids 全量当成抑制集合，deliver_turn 就可能漏发或多抑制，且无任何断言保护。
抑制决策只由一个归属者产出：_plan_deliveries 返回 (items, suppressed_ids)（或给 DeliveryPlanItem 加显式抑制标记），on_turn 直接消费该结果写库并缓存，不再自行复算 not intermediate and not is_final。同时把 _pending_record/_pending_text/_pending_plan/_pending_suppressed_ids 在 __init__ 中显式声明，避免继续用 getattr 默认值隐式扩展实例状态。
| source_path=config_path, | ||
| ) | ||
| legacy_agent_delivery = runtime_raw.get("agent_delivery_enabled") | ||
| if legacy_agent_delivery is not None: |
There was a problem hiding this comment.
SHOULD-FIX: 旧键迁移告警只看旧键是否存在，不判断新键是否已显式配置。当部署已配置两枚新键时，日志仍打出「未显式配置的新键按旧键值映射」，误导运维以为旧值覆盖了新配置；且每次 load_llm_config（含 /llm reload）都重复打印。
把告警限定在真正发生了回退的情况：if legacy_agent_delivery is not None and ("agent_delivery_intermediate_enabled" not in runtime_raw or "agent_delivery_final_enabled" not in runtime_raw): logger.warning("[runtime] agent_delivery_enabled 已废弃：未显式配置的域正按旧键值映射，请迁移到新键")；两枚新键都已显式配置时改为 info/debug 的「旧键已失效、忽略」提示。
| # 两列。回填只与各自加列绑定执行——若挂在启动路径无条件重跑，会与 | ||
| # 后续 reset（列置 NULL 跟随默认）的语义冲突，重启后覆盖用户选择； | ||
| # 按列独立门控，避免半迁移状态下跨列覆写另一列的已有取值。 | ||
| if "agent_delivery_intermediate" not in existing_columns: |
There was a problem hiding this comment.
SHOULD-FIX: 两个新列的迁移块逐字重复（只有列名不同），回填规则在两处各写一份，后续再加域就是第三份拷贝；且 _ensure_schema（DDL 初始化函数）混入了一次性 DML 迁移，职责不再单一。
用列名元组循环表达同一规则：
for column in ("agent_delivery_intermediate", "agent_delivery_final"):
if column in existing_columns:
continue
conn.execute(f"ALTER TABLE group_settings ADD COLUMN {column} INTEGER")
conn.execute(f"UPDATE group_settings SET {column} = agent_delivery_enabled WHERE agent_delivery_enabled IS NOT NULL")可抽成 _backfill_delivery_domains(conn, existing_columns) 命名帮手，让 _ensure_schema 只保留列清单与调用。
| @dataclass(slots=True) | ||
| class RecorderConfig: | ||
| agent_delivery_enabled: bool = False | ||
| intermediate_delivery_enabled: bool = False |
There was a problem hiding this comment.
SHOULD-FIX: 同一布尔量出现两种命名词序：agent_delivery_intermediate_enabled（配置键、ResolvedGroupSettings、DB 列/前端字段）与 intermediate_delivery_enabled（RecorderConfig）。跨层比对与全局搜索时容易漏改。
统一为 agent_delivery_intermediate_enabled / agent_delivery_final_enabled（与配置键、设置解析、Web 字段同词根），或在 RecorderConfig docstring 中明确说明并全仓统一命名。
- aborted_silently 改以「实际成功交付数」为据（summary().sent > 0）：中间轮抑制 + 最终轮未及交付即中止时不再全静默，给出可见中止提示；补零交付可见/部分交付静默对照测试 - set_chat_agent_delivery_enabled 域入参入口归一 DeliveryDomain(domain)，非法值显式 ValueError，修正 docstring 误导表述 - 抑制判定单一归属：_plan_deliveries 返回 (items, suppressed_ids)，on_turn 直接消费不再复算；_pending_* 四项状态在 __init__ 显式声明，deliver_turn 去除 getattr 隐式默认 - 旧键告警限定真实回退场景（存在未显式配置的域才 warning），两域齐全时降为 debug - 迁移加列+回填抽 _backfill_delivery_split_columns 命名帮手（列名循环），_ensure_schema 回归单一职责 - RecorderConfig 字段统一为 agent_delivery_intermediate/final_enabled，与配置/存储/Web 同词根
There was a problem hiding this comment.
Bot Review — ✅ Approve
本 PR 将 agent_delivery_enabled 单开关拆分为中间轮（intermediate）与最终轮（final）两域独立开关，配套更新配置读取、数据库迁移、Recorder 交付逻辑、命令接口与 Web Admin UI，同时保留旧配置键的向后兼容映射。两个 reviewer 覆盖均完整，0 个 blocking 发现；所有保留发现均为 nit 或 should-fix（测试文档描述与实际行为略有不符、控制流写法不一致、重复代码片段、命名后缀不统一、测试用例位置或结构偏弱），均属本次合并后可改进的代码质量项，不阻碍功能交付。
Findings
🟡 SHOULD-FIX: on/off/reset 三个互斥动作在 _handle_delivery_command 里用并列 if 写成，依赖 finish() 抛异常终止控制流；同一函数下方的 status 分支已写明「不依赖 finish 的终止副作用」注释，两套控制流惯用法并存，与同批 service_parts/state.py 中 set_chat_agent_delivery_enabled 用 match 表达互斥的风格不统一。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
在 _handle_delivery_command 内改用显式互斥分支，例如 if domain is not None and action in {"on", "off", "reset"}: 内先算出 value/label 再单点 finish，或对动作 match 分发，使「一次命令只走一个动作」在函数内部自证。
⚪ NIT: 两域×{当前值, 默认值} 的扇出在 _views()、_domain_default()（内部又调一次 _views()）和 status 分支里各写了一遍，导致单条 status 命令重复调用 svc.get_chat_settings 读库两次。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
把 _views() 的结果按域组织成一次取值的映射（如 {DeliveryDomain.INTERMEDIATE: (current, default), DeliveryDomain.FINAL: (current, default)}），改为 _domain_default(views, domain) 复用已取快照，避免重复读与三处重复的域分支。
⚪ NIT: 配置键/RecorderConfig 用 agent_delivery_intermediate_enabled/agent_delivery_final_enabled，而 Web 字段与存储列用 agent_delivery_intermediate/agent_delivery_final，两套命名规则在投影处交汇；旧键时期两边同名，本 PR 起读者须记住「API/存储少 _enabled、配置多 _enabled」这一例外规则。
File: src/quickquip/app/web/routes/group_settings.py
统一后缀：Web/存储字段补上 _enabled（新列改名窗口最小），并更新 agent_runtime.py 的注释以陈述实际规则。
⚪ NIT: 调用方已用 if tokens[:1] == ["delivery"]: 剥掉动词，却把整串 token 传入函数，函数内再自行偏移；子命令解析依赖「tokens[0] 一定是 delivery」这一未写明的调用约定，接口比需要的宽。
File: src/quickquip/adapters/nonebot/command_parts/llm.py
调用处传子 token 列表（如 tokens=tokens[1:]），函数签名与 docstring 相应改为接收子命令 token，去掉内部的隐式偏移假设。
⚪ NIT: 测试 docstring 写「旧键名被 Pydantic 拒绝」，但断言 legacy_body.model_dump(exclude_unset=True) == {} 证明旧键是被静默丢弃而非拒绝；真实行为是：旧前端只带 agent_delivery_enabled 提交 → payload 为空 → 400；与其他字段一起提交 → 其余字段落库、交付开关静默不生效，接口仍返回 200。docstring 与行为相反会误导维护者判断兼容边界。
File: tests/unit/web/test_group_settings_delivery_routes.py
把 docstring 改为「旧键名被 Pydantic 静默忽略（不落库、不报错）」；或补 warning 日志/422 响应并添相应断言，使描述与行为一致。
⚪ NIT: docstring 声称覆盖「读 llm.toml 失败时空对象兜底不回归」，但用例把 load_llm_config 替换成必定成功的 fake_cfg，get_options 的 except 分支完全没有被执行，声明的覆盖并不存在。
File: tests/unit/web/test_group_settings_delivery_routes.py
补一条 monkeypatch.setattr(routes, "load_llm_config", lambda path: (_ for _ in ()).throw(RuntimeError("boom"))) 之类的用例，断言 payload["defaults"] == {} 且 load_error 非空；或删掉 docstring 中该括注以免虚报覆盖。
⚪ NIT: 文件 docstring 声明覆盖「写入路由、默认投影与列表形状」三块，但没有任何用例调用 list_group_settings；本 PR 恰在列表 SELECT 上新增了两列，列名写错或投影漏字段在现有测试下不会被发现。
File: tests/unit/web/test_group_settings_delivery_routes.py
在 _DB 临时库中预置一行含两域取值的 group_settings，调用 routes.list_group_settings() 并断言返回条目里两域字段被正确投影；或从 docstring 中移除「列表形状」。
⚪ NIT: 函数签名声明 domain: DeliveryDomain = DeliveryDomain.ALL，函数体却做 domain = DeliveryDomain(domain) 归一任意可解析值，docstring 也写明「非法值抛 ValueError」；同批单测正是传字符串 domain="intermediate"。注解与实现/文档三者不一致，调用方无法从签名判断边界。
File: src/quickquip/llm/service_parts/state.py
注释放宽为 domain: DeliveryDomain | str = DeliveryDomain.ALL（保留归一），或在入口处显式拒绝非枚举入参并更新 docstring 去掉「归一」表述，使签名、实作、文档三处一致。
Review provenance
- General reviewer: complete — 24/24 units completed
- Style reviewer: complete — 24/24 units completed
- Coverage: complete
- Reviewed head:
833c542(base5a963fa)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
| return views.default_final | ||
| return f"中间轮 {views.default_intermediate} / 最终轮 {views.default_final}" | ||
|
|
||
| if domain is not None and action == "on": |
There was a problem hiding this comment.
SHOULD-FIX: on/off/reset 三个互斥动作在 _handle_delivery_command 里用并列 if 写成，依赖 finish() 抛异常终止控制流；同一函数下方的 status 分支已写明「不依赖 finish 的终止副作用」注释，两套控制流惯用法并存，与同批 service_parts/state.py 中 set_chat_agent_delivery_enabled 用 match 表达互斥的风格不统一。
在 _handle_delivery_command 内改用显式互斥分支，例如 if domain is not None and action in {"on", "off", "reset"}: 内先算出 value/label 再单点 finish，或对动作 match 分发，使「一次命令只走一个动作」在函数内部自证。
| ) | ||
|
|
||
| def _domain_default(scope_domain: DeliveryDomain) -> str: | ||
| views = _views() |
There was a problem hiding this comment.
NIT: 两域×{当前值, 默认值} 的扇出在 _views()、_domain_default()（内部又调一次 _views()）和 status 分支里各写了一遍，导致单条 status 命令重复调用 svc.get_chat_settings 读库两次。
把 _views() 的结果按域组织成一次取值的映射（如 {DeliveryDomain.INTERMEDIATE: (current, default), DeliveryDomain.FINAL: (current, default)}），改为 _domain_default(views, domain) 复用已取快照，避免重复读与三处重复的域分支。
| "memory_enabled": cfg.runtime.memory_enabled, | ||
| "auto_memory_enabled": cfg.runtime.auto_memory_enabled, | ||
| "agent_delivery_enabled": cfg.runtime.agent_delivery_enabled, | ||
| "agent_delivery_intermediate": cfg.runtime.agent_delivery_intermediate_enabled, |
There was a problem hiding this comment.
NIT: 配置键/RecorderConfig 用 agent_delivery_intermediate_enabled/agent_delivery_final_enabled，而 Web 字段与存储列用 agent_delivery_intermediate/agent_delivery_final，两套命名规则在投影处交汇；旧键时期两边同名，本 PR 起读者须记住「API/存储少 _enabled、配置多 _enabled」这一例外规则。
统一后缀：Web/存储字段补上 _enabled（新列改名窗口最小），并更新 agent_runtime.py 的注释以陈述实际规则。
| f"{scope_label}分段发送：{current}（全局默认 {default}）" | ||
| ) | ||
| if tokens[:1] == ["delivery"]: | ||
| await _handle_delivery_command( |
There was a problem hiding this comment.
NIT: 调用方已用 if tokens[:1] == ["delivery"]: 剥掉动词，却把整串 token 传入函数，函数内再自行偏移；子命令解析依赖「tokens[0] 一定是 delivery」这一未写明的调用约定，接口比需要的宽。
调用处传子 token 列表（如 tokens=tokens[1:]），函数签名与 docstring 相应改为接收子命令 token，去掉内部的隐式偏移假设。
|
|
||
|
|
||
| def test_put_body_fields_route_to_store(monkeypatch, tmp_path): | ||
| """PUT body 的两域字段进入 store 写入；旧键名被 Pydantic 拒绝。""" |
There was a problem hiding this comment.
NIT: 测试 docstring 写「旧键名被 Pydantic 拒绝」，但断言 legacy_body.model_dump(exclude_unset=True) == {} 证明旧键是被静默丢弃而非拒绝；真实行为是：旧前端只带 agent_delivery_enabled 提交 → payload 为空 → 400；与其他字段一起提交 → 其余字段落库、交付开关静默不生效，接口仍返回 200。docstring 与行为相反会误导维护者判断兼容边界。
把 docstring 改为「旧键名被 Pydantic 静默忽略（不落库、不报错）」；或补 warning 日志/422 响应并添相应断言，使描述与行为一致。
|
|
||
|
|
||
| def test_options_defaults_carry_both_delivery_domains(monkeypatch, tmp_path): | ||
| """options 投影携带两域全局默认（读 llm.toml 失败时空对象兜底不回归）。""" |
There was a problem hiding this comment.
NIT: docstring 声称覆盖「读 llm.toml 失败时空对象兜底不回归」，但用例把 load_llm_config 替换成必定成功的 fake_cfg，get_options 的 except 分支完全没有被执行，声明的覆盖并不存在。
补一条 monkeypatch.setattr(routes, "load_llm_config", lambda path: (_ for _ in ()).throw(RuntimeError("boom"))) 之类的用例，断言 payload["defaults"] == {} 且 load_error 非空；或删掉 docstring 中该括注以免虚报覆盖。
| @@ -0,0 +1,60 @@ | |||
| """group-settings 路由的交付两域字段：写入路由、默认投影与列表形状。""" | |||
There was a problem hiding this comment.
NIT: 文件 docstring 声明覆盖「写入路由、默认投影与列表形状」三块，但没有任何用例调用 list_group_settings；本 PR 恰在列表 SELECT 上新增了两列，列名写错或投影漏字段在现有测试下不会被发现。
在 _DB 临时库中预置一行含两域取值的 group_settings，调用 routes.list_group_settings() 并断言返回条目里两域字段被正确投影；或从 docstring 中移除「列表形状」。
| chat_id: int | str, | ||
| enabled: bool | None, | ||
| chat_type: str = "group", | ||
| domain: DeliveryDomain = DeliveryDomain.ALL, |
There was a problem hiding this comment.
NIT: 函数签名声明 domain: DeliveryDomain = DeliveryDomain.ALL，函数体却做 domain = DeliveryDomain(domain) 归一任意可解析值，docstring 也写明「非法值抛 ValueError」；同批单测正是传字符串 domain="intermediate"。注解与实现/文档三者不一致，调用方无法从签名判断边界。
注释放宽为 domain: DeliveryDomain | str = DeliveryDomain.ALL（保留归一），或在入口处显式拒绝非枚举入参并更新 docstring 去掉「归一」表述，使签名、实作、文档三处一致。
- on/off/reset 改互斥分发（动作表 + 单点 finish），与 status 分支控制流惯用法一致 - status 命令单次取值快照复用，消除重复读库；_handle_delivery_command 改收子 token，去除隐式偏移约定 - 存储列/Web 字段补 _enabled 后缀（agent_delivery_intermediate/final_enabled），趁未进生产库零迁移窗口与配置键全链同名 - set_chat_agent_delivery_enabled 域注解放宽 DeliveryDomain | str，签名/实作/文档一致 - web 测试三条收口：旧键静默忽略 docstring 修正、llm.toml 加载失败兜底分支用例、列表投影两域字段用例
1 participant
背景
现行 `agent_delivery_enabled` 单开关同时控制「非最终轮正文是否发送」与「最终正文是否走 sink 自然分段」，2×2 行为矩阵只剩对角线两格可达，群友需要的「中间轮照常说话 + 最终正文整条单发」无法表达。本 PR 按热修复节奏拆为两域独立开关，恢复矩阵全覆盖。实施计划见内部稿（dev/plans/2026-09-11-agent-delivery-switch-split.md）。
同分支后续将追加第二步：MCP resource 正文有界交付（另出简计划）。
变更摘要
测试
CHANGELOG 草稿