refactor(todos): consolidate summary decisions in TypeScript - #4964
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Reviewed exact head: 4c327833f9b88203ba5f3d97f197a3aa89bfd365
动机
Todo summary 原先把一次全量来源的读决策拆在 TS lane、Python 计数/展示/claimant 分配和第二次 TS closure 调用之间。字符串时间排序会让时区偏移或后续编辑误改“最近完成”;partial source 也不能仅因可见行匹配而获得 whole-source closure 证明。
改动思路
新 todo.summary.project 在 TS 合并现有 lane 与 closure owner,Python 只适配 legacy 输入、隐私/公开字段和渲染。全量先决策、展示后限额,方向与 canonical source 一致;但迁移不能绕过旧读入口对“全量 resume evaluation 已完成”的 fail-closed 前置检查。
具体改动
关键代码讲解
projectTodoSummary从原始 source ordinals 计算 counts、claimant 可见项、真实完成瞬间和 closure;partial selection 不重新取得 full-selection 权限。_project_summary适配 Python Todo 行并调用新的 TS summary,但现在先调用project_succession(items, reuse=True),再走原来的全量 resume 条件验证路径。compact_evaluated_todo_group使用 TS 返回的 lane ordinals/fields,再做公开字段 materialization、handoff 和 warning。
25 个聚焦 Python、8 个 TS 测试及 diff 检查通过;这不能覆盖以下失败。
对主干的风险
- 必修回归:
tests/control_plane/test_long_history_todo_reads.py::test_display_cannot_bypass_full_source_resume_evaluation的 5 个变体本地全部失败,CI 四个 shard 也复现。缺失/无效 resume evaluation 现在从project_succession抛出通用EffectRuntimeRejected: succession evaluation must be an object,而非预期ValueError且含full-source resume evaluation诊断。请在 succession RPC 前恢复该完整来源前置校验,并验证无写入/隐藏 work;CI 还有一个 completed-Todo HTTP history read 返回 400,需确认是否同源。 kernel-static-checks在todo_summary.py的 874、910、911、1009 行报告 4 个 mypy 错误(可选 RPC 字段直接传给 set/迭代及返回 Any)。请收窄并校验 typed result 后重跑检查。minimum-Node/Frontstage 也红,需在新 head 的必需检查里区分主干问题。
我的整体评价
单一 TS summary owner 是可取的相关重构,但当前 head 改变了重要失败语义且必需静态/集成检查失败,因此请求修改。最小修复是把旧 full-source guard 放回 RPC 之前并补齐 result narrowing,不是添加一层兼容 fallback。修复后重跑上述负例、HTTP history、mypy 与整个 required check 集。
English verdict: REQUEST_CHANGES - At 4c32783 five full-source resume negative cases regress and four mypy errors fail CI despite 25 Python/8 TS focused passes; restore pre-RPC validation and typed result narrowing.
…nside budget Display may never invent the source's resume decision, so the evaluator that the row facts already compute is now asserted before the succession RPC: an unevaluated source fails with the source's own "full-source resume evaluation" diagnostic instead of a later owner's generic "succession evaluation must be an object". Five negative variants and the completed-history HTTP read show it. That HTTP read also exposed the second defect: one whole-source batch repeated every key name per Todo, so 4087 completed Todos produced a 2.05 MiB request and failed closed with a 400. The adapter now sends the declared columnar facts and the typed owner decodes them back into the same row objects before validating, which brings the same source to about 0.73 MiB. Nothing is defaulted from absence, and the column order is validated as this request version's schema. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
…readmodel-0924 Two conflicts. The effect-runtime handler registry conflicted on imports only, so both sides keep their handlers. The shared-authority RFC conflicted on the delivery-count checkpoint: main carries the 2026-09-23 seven-boundary decomposition while this branch recorded its own dependency reconciliation. Resolution keeps main's decomposition, updates the reconciliation with the now-merged #4922/#4960/#4961 and the still-in-review #4931, and states that the estimate is updated after the combined head is accepted instead of restoring the superseded five-to-eight package range. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head 313bf4a715962fd838555404da3cbd4868c13d23 against base 25b9f7366 (current origin/main at review time).
动机
上一轮评审在 4c327833f 上提出三项:必修的 display 失败语义回归(test_display_cannot_bypass_full_source_resume_evaluation 五个变体把旧的 full-source resume evaluation 诊断换成了下游泛化的 succession evaluation must be an object)、todo_summary.py 的 4 个 mypy 错误,以及一个待确认来源的 completed-Todo HTTP history 读 400;同时要求同步 main 后重跑必需检查。本 head 逐项闭合。
改动思路
两处都按「不新增机制、只把已有规则放回正确位置」处理:
- 显示路径永远不能自己编造 resume 决策。adapter 本来就要为每行计算「是否匹配全量 resume evaluation」,现在把同一个 predicate 提前到 succession RPC 之前断言,于是未评估来源以其自身诊断失败;同一 predicate 继续用于行事实,没有第二份判定。
- completed-Todo HTTP 400 经定位同源但不同因:读取本身没问题,是一次 whole-source batch 重复了每行的键名——4087 条已完成 Todo 打出 2.05 MiB 请求,超过 effect-runtime 的 2 MiB 请求预算而 fail closed。修复方式是把同一批事实改成列式(
columns+ 行数组,列顺序作为该请求版本的模式被校验),typed owner 解码回原来的行对象再校验;不放宽预算、不退回「两次调用」的旧形态,也不靠「缺字段即 false」的隐式默认。
具体改动
todo_summary.py:新增_resume_condition_evaluated(单一 predicate),在project_succession之前对含resume_when的行断言,缺失/不匹配即ValueError: Todo display requires a matching full-source resume evaluation;请求改为todo_summary_projection_request_v1的列式columns/rows形状。summary_projection.ts:导出并校验 typed 列顺序(顺序不符即EffectRuntimeRequestError),decodeRows把单元格还原成与之前完全相同的行对象,行宽不符即拒绝;schema 断言改为 v1。- 测试:
tests/control_plane/test_todo_summary_projection.py新增 long-history 请求预算回归(4096 条完成 Todo 的 batch 必须小于MAX_REQUEST_BYTES);tests/control_plane/test_todo_consumer_scope.py断言列式形状与 v1;TStodo_summary_projection.test.ts的 request helper 走真实列式编码,原有的语义与负例断言不变。 - 合入当前 main(87 个提交)。两处冲突:effect-runtime handler 注册表是纯 import 冲突(双方 handler 都保留);双语共享权威 RFC 是交付数量检查点冲突(保留 main 的七边界分解,把本分支的依赖核对更新为 #4922/#4960/#4961 已合并、#4931 仍在评审,并声明数量在组合 head 验收后更新,不恢复被取代的 5–8 包表述)。
对主干的风险
反向证据:修复前 pytest tests/control_plane/test_long_history_todo_reads.py 的 5 个变体全部以 EffectRuntimeRejected: succession evaluation must be an object 失败,tests/test_chat_completed_todos.py::...[4087] 以 HTTP 400 失败;修复后两者全部通过(含 39 例聚焦运行)。列式编码把同一来源的 whole-source 请求从 2.05 MiB 降到约 0.73 MiB,仍保留全部字段语义(无隐式默认),且列顺序被校验。
验证矩阵(本 head):134 个 Python 用例(摘要投影、consumer scope、long-history、completed-history HTTP、投影恢复等)通过;mypy(23 模块)与配置范围 Ruff 通过;TS 摘要投影单测与 typecheck:control-plane 通过;真实隔离 PostgreSQL 16.15 provider conformance 292/292;pinned Node 22.22.3 的 minimum-Node lane 14/14(同步 main 后恢复);Frontstage showcase catalog smoke 与 RFC docs governance 通过;loopx canary premerge 19 项全通过。完整 TS 套件在本机为 3279/3291,12 项失败均已归因且不指向本 diff(11 项来自 todo_continuation.test.ts 写死 python3 而本机是 Python 3.9,1 项 sqlite_capacity rehearsal 在未修改的 origin/main 上以相同方式失败)。按本 Goal 的 wait_for_ci=false 未等待远端 CI。change-quality receipt:cqr_e7b2bc441013434ec3f8(decision=pass)。
残留边界(已记入 receipt 的 advisory 风险):v1 列式请求是同一部署内的 adapter/runtime 契约,混版本部署会 fail closed 于 schema 检查而不是误读;显示决策仍是 whole-source 单批(4087 条约 0.73 MiB),更大的历史需要 bounded read model 而不是更大的预算。
我的整体评价
APPROVE:上一轮的两个必修项都已按最小修复闭合(旧 full-source guard 回到 RPC 之前、result narrowing 完成),并顺带确认与修掉了 HTTP history 400 的真实成因——不是同一条 guard,而是同一次 batch 因键名重复超预算,现已用显式列式编码夹紧。产品语义、持久格式与公开字段未变,相邻重构仍复用既有 outbox/owner,无新增兼容 fallback。
English verdict: APPROVE - exact head 313bf4a715962fd838555404da3cbd4868c13d23 restores the full-source resume precondition ahead of the succession RPC with one shared predicate, narrows the typed summary result (mypy clean) and fixes the completed-history HTTP 400 by sending the same whole-source facts columnar (2.05 MiB to about 0.73 MiB) with a validated column order instead of loosening the request budget or reverting to two calls; the refreshed main closes the previously red minimum-Node and Frontstage lanes. 134 focused Python cases, both negative regressions, real isolated PostgreSQL 16.15 conformance 292/292, pinned Node 22.22.3 lane 14/14, docs governance and the 19-check canary pass; the 12 local full-suite failures are attributed to the host Python 3.9 and one pre-existing origin/main capacity rehearsal. Receipt cqr_e7b2bc441013434ec3f8.
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head c8ef17cd91be2597758afeaccc7a0d7eb01f659b against base e07ee86a4 (current origin/main at merge time).
动机
上一轮评审在 4c327833f 上提出三项:必修的 display 失败语义回归(test_display_cannot_bypass_full_source_resume_evaluation 五个变体把旧的 full-source resume evaluation 诊断换成了下游泛化的 succession evaluation must be an object)、todo_summary.py 的 4 个 mypy 错误,以及一个待确认来源的 completed-Todo HTTP history 读 400;同时要求同步 main 后重跑必需检查。本 head 逐项闭合。
改动思路
两处都按「不新增机制、只把已有规则放回正确位置」处理:
- 显示路径永远不能自己编造 resume 决策。adapter 本来就要为每行计算「是否匹配全量 resume evaluation」,现在把同一个 predicate 提前到 succession RPC 之前断言,于是未评估来源以其自身诊断失败;同一 predicate 继续用于行事实,没有第二份判定。
- completed-Todo HTTP 400 经定位同源但不同因:读取本身没问题,是一次 whole-source batch 重复了每行的键名——4087 条已完成 Todo 打出 2.05 MiB 请求,超过 effect-runtime 的 2 MiB 请求预算而 fail closed。修复方式是把同一批事实改成列式(
columns+ 行数组,列顺序作为该请求版本的模式被校验),typed owner 解码回原来的行对象再校验;不放宽预算、不退回「两次调用」的旧形态,也不靠「缺字段即 false」的隐式默认。
具体改动
todo_summary.py:新增_resume_condition_evaluated(单一 predicate),在project_succession之前对含resume_when的行断言,缺失/不匹配即ValueError: Todo display requires a matching full-source resume evaluation;请求改为todo_summary_projection_request_v1的列式columns/rows形状。summary_projection.ts:导出并校验 typed 列顺序(顺序不符即EffectRuntimeRequestError),decodeRows把单元格还原成与之前完全相同的行对象,行宽不符即拒绝;schema 断言改为 v1。- 测试:
tests/control_plane/test_todo_summary_projection.py新增 long-history 请求预算回归(4096 条完成 Todo 的 batch 必须小于MAX_REQUEST_BYTES);tests/control_plane/test_todo_consumer_scope.py断言列式形状与 v1;TStodo_summary_projection.test.ts的 request helper 走真实列式编码,原有的语义与负例断言不变。 - 合入当前 main(两次:87 个提交到
25b9f7366,再e07ee86a4,第二次为无冲突自动合并)。两处冲突:effect-runtime handler 注册表是纯 import 冲突(双方 handler 都保留);双语共享权威 RFC 是交付数量检查点冲突(保留 main 的七边界分解,把本分支的依赖核对更新为 #4922/#4960/#4961 已合并、#4931 仍在评审,并声明数量在组合 head 验收后更新,不恢复被取代的 5–8 包表述)。
对主干的风险
反向证据:修复前 pytest tests/control_plane/test_long_history_todo_reads.py 的 5 个变体全部以 EffectRuntimeRejected: succession evaluation must be an object 失败,tests/test_chat_completed_todos.py::...[4087] 以 HTTP 400 失败;修复后两者全部通过(含 39 例聚焦运行)。列式编码把同一来源的 whole-source 请求从 2.05 MiB 降到约 0.73 MiB,仍保留全部字段语义(无隐式默认),且列顺序被校验。
验证矩阵(本 head):134+57 个 Python 用例(摘要投影、consumer scope、long-history、completed-history HTTP、投影恢复等,merge 前后各一轮)通过;mypy(23 模块)与配置范围 Ruff 通过;TS 摘要投影单测与 typecheck:control-plane 通过;真实隔离 PostgreSQL 16.15 provider conformance 292/292;pinned Node 22.22.3 的 minimum-Node lane 14/14(同步 main 后恢复);Frontstage showcase catalog smoke 与 RFC docs governance 通过;loopx canary premerge 19 项全通过。完整 TS 套件在本机为 3279/3292,13 项失败均已归因且不指向本 diff(11 项来自 todo_continuation.test.ts 写死 python3 而本机是 Python 3.9,1 项 sqlite_capacity rehearsal 在未修改的 origin/main 上以相同方式失败,1 项 PostgreSQL service 用例在复用旧数据库时失败、新建数据库后 10/10 通过)。按本 Goal 的 wait_for_ci=false 未等待远端 CI。change-quality receipt:cqr_6f7f6d7366d6676474e9(decision=pass)。
残留边界(已记入 receipt 的 advisory 风险):v1 列式请求是同一部署内的 adapter/runtime 契约,混版本部署会 fail closed 于 schema 检查而不是误读;显示决策仍是 whole-source 单批(4087 条约 0.73 MiB),更大的历史需要 bounded read model 而不是更大的预算。
我的整体评价
APPROVE:上一轮的两个必修项都已按最小修复闭合(旧 full-source guard 回到 RPC 之前、result narrowing 完成),并顺带确认与修掉了 HTTP history 400 的真实成因——不是同一条 guard,而是同一次 batch 因键名重复超预算,现已用显式列式编码夹紧。产品语义、持久格式与公开字段未变,相邻重构仍复用既有 outbox/owner,无新增兼容 fallback。
English verdict: APPROVE - exact head c8ef17cd91be2597758afeaccc7a0d7eb01f659b restores the full-source resume precondition ahead of the succession RPC with one shared predicate, narrows the typed summary result (mypy clean) and fixes the completed-history HTTP 400 by sending the same whole-source facts columnar (2.05 MiB to about 0.73 MiB) with a validated column order instead of loosening the request budget or reverting to two calls; the refreshed main closes the previously red minimum-Node and Frontstage lanes. 134 focused Python cases, both negative regressions, real isolated PostgreSQL 16.15 conformance 292/292, pinned Node 22.22.3 lane 14/14, docs governance and the 19-check canary pass; the 12 local full-suite failures are attributed to the host Python 3.9 and one pre-existing origin/main capacity rehearsal. Receipt cqr_6f7f6d7366d6676474e9.
|
Self-repair and merged decision record (admin-bypass merge, maintainer-authorized).
Repair content:
Validation at the merged head:
|
Problem and result
Todo summaries split one read decision across TS lane selection, Python count/display/claimant allocation, and a second TS closure call. Recent completions used timestamp strings and preferred last-edit time, so offset timestamps or later edits could reorder completed work incorrectly. A later selection could also overwrite an explicitly partial source flag and issue a whole-source closure proof.
For #4574 R5 / shared-authority L5 / TypeScript T3, this PR moves the complete summary reduction into one
todo.summary.projectbatch. It reuses the existing scope, lane, succession and timestamp owners. Python keeps legacy decoding, public field allowlists, privacy handling and rendering. The replaced Python aggregate branches, claimant selector and unused lane/closure RPC registrations are removed.Intentional behavior changes:
Counts, visibility caps, claimant distribution and orchestration positions share the selected source. Resume/succession evaluation still precedes filtering, and original source ordinals are validated before materialization. Public
todo_summary_v0, persisted records and display budgets remain unchanged. This is read policy, not new execution or settlement authority.Validation
A same-snapshot seven-sample measurement reduced summary RPC calls from 11 to 9, while aggregate request bytes grew 4.6% and response bytes were essentially unchanged. Observed medians were approximately 212/206 ms; this is diagnostic evidence, not a statistically established speedup. No budget was raised.
Delivery boundary
Status, Todo-list and quota consumers change for legacy and promoted Goals. Existing frontend/Lark views consume Core projections; no settings, frontend assets or external-sink permission changes are needed. Packaged browser/Lark transport E2E was not rerun. Rollback requires the matching Python/TS package, not a persisted-data migration.
The adjacent refactor replaces shared decisions rather than adding another provider or capability. Updated English/Chinese RFCs reconcile the in-review pagination/runtime/SQLite/display dependencies and retain the conditional 5–8 delivery-package plan. Full consumer/projection qualification, SQLite D2, whole-Goal migration and default/writer retirement remain open. This PR does not flip defaults or claim PostgreSQL service qualification.