Skip to content

test(control-plane): read canonical todo list result - #6068

Merged
huangruiteng merged 2 commits into
loopx-project:mainfrom
Duang777:codex/fix-todo-projection-canonical-readback
Oct 10, 2026
Merged

huangruiteng merged 2 commits into
loopx-project:mainfrom
Duang777:codex/fix-todo-projection-canonical-readback

Conversation

@Duang777

Copy link
Copy Markdown
Collaborator

Summary

  • read the canonical Todo inventory from the public todo list result
  • keep the unreadable-display recovery assertion aligned with the current CLI contract

Why

The test read agent_todos.items, which is a compact display projection. Those items do not carry todo_id. The canonical CLI result exposes full records in todos, so both parameterized cases failed after the read contract changed.

Validation

  • 25 projection recovery and Todo reference tests
  • premerge: 4 direct checks and 3 selected smoke checks
  • exact failing case on Python 3.11

Ruff still reports four pre-existing import-order findings in the touched file; the same findings reproduce on upstream/main.

Signed-off-by: Duang777 <duangjl007@gmail.com>
…ection-canonical-readback

Signed-off-by: Duang777 <duangjl007@gmail.com>

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@huangruiteng
huangruiteng merged commit 8dff6b6 into loopx-project:main Oct 10, 2026
21 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants