Repository navigation
fix(manager): preserve exact Todo details in context - #5949
mikamikasuki wants to merge 2 commits into
Conversation
c5ed479 to
434deb2
Compare
Signed-off-by: mika <211269698+mikamikasuki@users.noreply.github.com>
434deb2 to
f6a7a74
Compare
Signed-off-by: mika <211269698+mikamikasuki@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — 当前证据显示这段运行时兼容是在容纳旧测试stub,尚未证明修复真实读取问题。
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · reasoning_effort=xhigh
Reviewed head: e86d5c9cd57a9ce24c14cb456c82e9821c42077c
Baseline: a1890a37f4f759bc2e39823074bbe78f78e49fdb
动机
请求管家按Todo ID读取完整任务与原始约束的用户。
PR声称旧版精确读取丢失详情;独立基础对照显示真实File和SQLite读取已经返回完整正文与备注,变化只让一个列表形状的测试stub通过。
当前head有46项测试通过,基础版本45项通过且仅priority-context测试失败;真实生产入口的基础/head结果相同,未观察到声称的用户结果改善。
此评审不要求重写整个管家、迁移Python适配器或完成全部协作RFC,只要求证明这段兼容逻辑的真实必要性。
改动思路
当前单条生产契约已在基础版本生效,新增列表回退只修复不匹配的测试stub;尚未证明需要生产兼容分支。
此PR应交付一个可复现的真实精确读取修复,或收敛为纠正测试夹具;不应把未证明的响应形状当成现行生产契约。
接受规格:docs/architecture/rfcs/capable-manager-semantic-handoff-v0.md,固定版本a1890a37f4f759bc2e39823074bbe78f78e49fdb,§5.14的5.14-complete-records:existing scoped reader restores complete records before decisions or action。真实File/SQLite入口在基础与head都满足此要求;该规格未要求列表形状的精确读取兼容。
具体改动
loopx/chat_manager_details.py:46-47在精确读取没有todo时新增todos回退。基础版本已优先读取单条todo;list_goal_todos经过projectTodoDetailPayload后明确移除todos,返回匹配的单条记录或null。
独立执行uv run --extra test python -m pytest -q tests/test_chat_manager_inspection.py tests/test_chat_manager_details.py:基础版本45 passed / 1 failed,失败仅是test_priority_context_keeps_conditions_and_scoped_decision,其stub忽略todo_id并总返回列表;head为46 passed。PR提到的精确读取测试以及真实权威读取测试在基础版本已经通过。
另用真实、一次性File/SQLite权威运行ManagerInspection.read:基础/head均保留长正文、owner备注;外部受众不含备注,缺失ID为空,错误Goal与撤销授权被拒绝,读取前后权威及Todo不变。输出完全相同,没有进入新增回退。4项TS投影测试与本机完整diff canary(包含语义检查)通过,未查询/等待CI。
[P2] 请先证明这条兼容分支的真实producer,或修正测试夹具。
请用真实list_goal_todos→ManagerInspection入口证明当前或受支持独立版本确实返回列表形状的精确读取;若只有测试stub如此,修正stub为当前单条契约,并移除这段运行时兼容分支。同步修正PR的失败测试和前后行为说明。
这是已定位的证据/维护边界缺口,不是在声称新增了可复现的数据丢失或越权。一个mock变绿不能为同包生产响应定义第二种契约。
对主干的风险
没有新状态、权限、调用入口或功能开关;既有TS身份过滤和受众边界保留。问题是会掩盖夹具与现行精确响应契约的偏差,并产生未经证明的兼容承诺。尚未见真实或受支持独立版本输出这种列表响应;此证据缺失阻止approve。无需为本修复扩展至整个管家RFC、PostgreSQL存储重构或TS语言迁移。
我的整体评价
请求最小修复后复核:实际producer回归证明,或纠正stub并删除运行时回退;同时改正PR的前后行为说明。未来改动便利性检查已应用到当前契约边界:维护一个真实精确读取形状、让测试忠实反映它,比增加不明兼容分支更便于后续修改与撤销。
English verdict: REQUEST_CHANGES
The immutable base already preserves native exact Todo details. Independent File/SQLite ManagerInspection replay is identical at base and head; only a list-only test double fails at base. Demonstrate a supported real producer requiring the fallback, or repair the stale fixture and remove the unneeded runtime branch. Correct the reported failing tests and before/after claim. No CI prerequisite or speculative migration is imposed.
| raise ValueError("Todo authority unavailable or conflicting") | ||
| records = ([result["todo"]] if result.get("todo") else []) if todo_id else result.get("todos", []) | ||
| records = ( | ||
| ([result["todo"]] if result.get("todo") else result.get("todos", [])) |
There was a problem hiding this comment.
[P2] Prove the real producer or repair the stale test double. The immutable base already reads result.todo. Native list_goal_todos passes exact results through projectTodoDetailPayload, which explicitly omits todos. Independent real File/SQLite ManagerInspection reads preserve full text/owner notes at both base and head. Of the 46 focused cases, only test_priority_context_keeps_conditions_and_scoped_decision fails at base; its list-only stub ignores todo_id. Please show a supported actual producer requiring this fallback, or correct that stub and remove the runtime compatibility branch. Update the reported failing tests and before/after claim; a green mock cannot define a second production response contract.
|
I rechecked the reported behavior against the current producer contract and could not reproduce a production defect. On current main 7ec2d94, exact list_goal_todos(todo_id=...) reads pass through projectTodoDetailPayload, which returns the singleton todo field and omits the list aliases. I ran the PR's focused command with the same locked test dependencies on the PR base a1890a3, this head e86d5c9, and current main 7ec2d94: all three runs passed 46/46. The cited priority-context test uses the overview path without todo_id, so this patch does not affect it; the actual File/SQLite exact-read regression is included and passes at base and head. The review's 45/1 baseline result was not reproducible in these runs, and the added list fallback is not backed by the current producer contract. I'm withdrawing the unsupported compatibility claim and closing this PR to avoid retaining an unneeded production branch. Sorry for the extra review work. |
Goal And Delivered Outcome
Author Declaration
Implemented against
Scope And Continuation
Validation
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist