Repository navigation
test(control-plane): read canonical todo list result - #6068
huangruiteng merged 2 commits into
Conversation
Signed-off-by: Duang777 <duangjl007@gmail.com>
…ection-canonical-readback Signed-off-by: Duang777 <duangjl007@gmail.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=xhigh
动机
维护恢复测试的贡献者需要确认,Markdown 展示损坏时仍能读取原有规范任务。 原测试从简略展示项索取不存在的 todo_id,两个恢复案例因此报 KeyError;本次改读 todo list 返回的 todos,保留原任务身份与存储不变断言。 同一不可变基础版本的两个案例均复现 KeyError;候选版本的 25 项恢复与引用测试通过。 只修正回归测试的读取位置,不修改生产代码、默认设置、权限、任务内容或界面,也不替代整个存储迁移验收。
改动思路
不修改会保留已复现的假失败;给简略展示补字段会不必要地改动生产接口。这里复用既有规范读取结果,保留完整存储前后相等检查,修复范围足够且最小。
具体改动
精确 head b5d422da43475d2274f08f492942d05c020e7082,base ed79bfd63c0dae3dd0111ae3854790cce47ca014;仅 tests/control_plane/test_todo_projection_recovery.py:449 的一行断言:agent_todos.items[0] 改为 todos[0]。code == 0、todo_active 身份和最后 _read(runtime) == before 均保留。既有 list_readback 返回规范列表;摘要展示不能代替该列表。
独立规范为 docs/project-agent-todo-contract.md,spec_revision ed79bfd63c0dae3dd0111ae3854790cce47ca014,criterion_id Write Contract:Markdown 丢失或陈旧时,规范 Todo 仍是任务来源;provider 失败不能回退展示。该要求 implemented,由真实 CLI 恢复及规范快照相等断言覆盖。
对主干的风险
未发现阻塞项。相同解释器、相同不可变 base 的两个 Objective 案例均在旧断言复现 KeyError: todo_id;候选的恢复和引用 suite 25 passed,包括损坏展示、缺失文件、陈旧版本拒绝、完整内容保留。默认 Ruff 与 diff-check 通过。显式 import-order 检查在 base/head 均有相同四项 I001(2/314/328/433 行,同消息与列号),与第 449 行修正无因果关系,原有清理债务保留。最初审查命令使用不存在的引用测试路径,未执行测试;改用仓库实际文件后完成上述验证,未改测试或阈值。未查询、轮询或等待 CI。
最强反例是把错误隐藏而没有验证规范任务:本行仍断言原 ID,后续仍比较整个规范记录,没有删负例或只检查返回码。没有生产、权限、默认、自动加载指令、状态词汇或 UI 变化;相关 lenses 未发现改变。此源码 CLI 结果不声称安装态 UI、真实模型或长期运行合格。
我的整体评价
APPROVE。这是修复已有回归测试的完整有界结果,长期执行与用户旅程保持原行为。已扫描相邻恢复/引用覆盖和同作者当前批次(#5966、#6040):其他 PR 处理不同实际缺陷,本 PR 不增加重复 smoke。未来相关重构检查考虑了测试读取 helper,但一行更正不值得增加抽象;保留既有 fixture 和不变性断言即可。公共提交没有私有证据或生成文件。
English review
Motivation
Contributors maintaining display-recovery tests need to verify that damaged Markdown cannot replace canonical Todo authority. The old assertion requested an absent todo_id from the compact agent_todos display, causing two false KeyError failures. The patch reads the existing todos result instead, while retaining the original task identity and complete before/after authority equality. It changes no production behavior, default, permission, task or UI.
Approach and changes
Doing nothing keeps the demonstrated false failure; adding an identity field to the display would unnecessarily change production. One assertion in the existing parameterized test is the smallest complete repair. The independent Write Contract at the immutable specification revision above keeps canonical Todos authoritative when Markdown is missing or stale. Existing list_readback already returns the canonical list. No new smoke, fixture, state owner, decoder or compatibility path is introduced.
Main risks and validation
The identical two objective-display cases on the immutable base reproduce KeyError(todo_id) at the old assertion. All 25 head recovery/reference tests pass through real CLI subprocesses and disposable authority, preserving missing/damaged-display recovery, stale-revision rejection and non-mutating readback. Default Ruff and diff-check pass. Explicit import-order checking produces the same four I001 rule/location/message signatures on base and head; they are unrelated unchanged maintenance debt, not hidden by the verdict. An initial reviewer command named a nonexistent test file and executed no tests; the corrected repository-native suite supplies the actual evidence. No CI was queried or awaited. Installed UI, live-model, cross-host and sustained-store qualification were not attempted by this test-only change.
Overall judgment
No blocking finding. The full identity and authority-equality assertions prevent a superficial green test from hiding lost tasks or mutations. Adjacent coverage and the author's current batch were inspected; this fixes existing coverage rather than farming duplicate tests. Future-facing review found that retaining the current fixture and direct assertion is clearer than introducing a helper. Maintainer merge and any unrelated lint cleanup remain separate.
English verdict: APPROVE - exact head b5d422d; the demonstrated test consumer error is repaired with the original canonical recovery invariant preserved.
Summary
todo listresultWhy
The test read
agent_todos.items, which is a compact display projection. Those items do not carrytodo_id. The canonical CLI result exposes full records intodos, so both parameterized cases failed after the read contract changed.Validation
Ruff still reports four pre-existing import-order findings in the touched file; the same findings reproduce on
upstream/main.