Repository navigation
Conversation
Signed-off-by: shangzh0 <97216392+shangzh0@users.noreply.github.com>
songoow
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | GPT-6 family | OpenAI | self_reported
动机 / Motivation
当前结论为 REQUEST_CHANGES,原因是现有失败 CI 的归因证据尚未闭合;本次代码审查没有发现可复现的阻塞缺陷。 执行多项已声明 Todo 的 Agent 在下一轮重规划时,需要同时看到各项工作的后续结果。同一 Todo 连续留下 12 条结果,另一 Todo 只有一条较早的反例时,旧窗口的六个位置全部被前者占据;本次改动让反例占一个位置,其余五个位置仍按来源顺序展示。已验证的改善是下一轮输入能保留这条反例、维持完整计数,并用原有引用读回完整结果。此 PR 的范围是既有历史的有界展示;不负责生成缺失结果、改变 scheduler 或证明模型采纳以及最终性能收益。
这是一处有实际消费者的展示缺陷:执行重规划的 Agent 可能只能看见重复进展,错过能否定原先方案的对照结果。仅要求 Agent 自行翻阅全部历史并不能修复它收到的默认输入;继续扩大固定窗口也无法避免下一轮重复记录再次占满。因此在现有窗口内为已展示的工作保留结果,是可独立验证的修复边界。
改动思路 / Approach
历史来源、Goal/Agent 范围和可信追加顺序仍由现有代码负责。新函数先为最多六个已展示 Todo 各选一条最新可用记录,再以相关记录及其余记录补齐六个位置,最后按原始来源顺序输出。它不把缺失记录补成成功,也不把 Todo 完成解释为目标收益。保留一个位于既有 TypeScript 投影 owner 内的共享选择函数,能让 typed 与 ordinary 两条路径遵守同一窗口规则,同时保持历史读取和执行权限原有归属。当前 PR 的边界是六行证据窗口与每个已展示 Todo 的引用;不增加状态、协议版本、CLI 或续跑义务。
我在固定 base 与 head 上调用真实的 projectReplanContext 做了同输入对照,覆盖 typed/ordinary 与 stride off/on 四种组合。四组均从“六条 busy、零条反例”变成“五条 busy、一条反例”;总计数仍为 13。未发生拥挤的输入在两版本完整输出相同;反转传入数组后,由可信来源位置决定的输出仍相同。这里只证明证据交付改善,没有执行模型任务或把保留记录当成实际采纳。
具体改动 / Changes
审阅 head 为 a32592d11f598150ccc004a6c4013e7ea9ced4e1;规范依据是 Decision followthrough in replan context,固定于 base 04c5799b88f6c4cf12a224b7fa679f272e1e794e。下列标识是本次评审对该节现有条款的映射:source-order 对应可信来源顺序;bounded-counts 对应六行细节与完整计数;ordinary-links 对应普通回写只链接自身 Todo;strategy-off 对应关闭 stride 后仍有只读 followthrough;unknown-gain 对应不认证执行或收益。以上条款在当前 head 均有实现与相应验证,不扩大为必须主动创建后继或必须提分的义务。
关键代码讲解
boundedWorkEvidence先为todoIds.slice(0, 6)中各项保留第一条匹配记录。上游已经按追加位置从新到旧排序、去重;最后过滤原数组维持来源顺序。超过六个目标沿用planned_work的六项边界,缺少观察的目标不会获得虚构引用。decisionFollowthrough将同一选择函数用于 typed 与 ordinary 记录,再从实际展示记录构造各 Todo 的引用。计数仍来自完整集合,普通回写不会冒充 typed progress。原有resolve/review_evidence继续读取完整范围内历史,展示截断不会取消引用资格。- 新增回归测试 验证反例保留、总数/省略数、引用解析、review replay、off/on 和数组顺序。对应的 Python source codec 测试 通过真实 Python→TypeScript 通道覆盖有/无 typed 附件,证明修复不只存在于单个 helper 的隔离测试中。
对主干的风险 / Risks
PASS: 实际执行两个 TypeScript 文件共 65 个测试,两个 Python context/order/evidence 文件共 46 个测试;另有 1,508 个 helper 边界检查及上述四组 base/head 对照。TypeScript 全项目配置的 no-emit 检查也通过,使用已有依赖的显式类型解析路径。初次 Python 执行被本地 sandbox 拒绝 Unix socket 启动;保持临时 fixture 隔离后重跑通过,未改动断言。初次 TypeScript 默认进程隔离只汇报两个文件,因此另用 --test-isolation=none 确认实际 65 条用例执行。
关闭 stride 时会获得同样的共享窗口修复,这是公开且有测试的默认展示变化;不会新增 stride_strategy、Explore basis、写入要求或调度权限。超过六项、身份冲突、缺观察、重复记录和完整历史引用是重点边界;现有 suite 与独立 helper 检查未发现错误归属或假成功。没有做 native model 采纳与长期效果评测,也没有把这些未测目标作为本 PR 的缺陷。
语义与 CI 对齐
diff 语义 advisory 已执行:检查 1 个生产源文件,发现 0 个受支持的新词汇载体;此工具只覆盖规定的语法形态,不代表完整语义证明。人工核对后,变化复用现有 Todo、progress 和 evidence 身份,没有新增公共词汇。两端内容差分的 whitespace 检查通过。RFC bilingual gate 的三点 diff 因本地 shallow history 缺 merge base 未完成;此次没有 RFC 改动,但不把命令失败写成通过。
FAIL,归因仍 UNVERIFIED: dashboard-browser (3) 在 attached-host-follow-up 场景中,于 codex-attached.mjs 的新页读取 等待带 after 参数的响应超时 30 秒。日志运行的是将本 head 合入上述 base 的 67552bba726741fa3fc5153c8b62f9ace39b12ec。该聊天分页链路不在本次三文件 diff 内,但尚无同条件 base/head 复现或独立基础设施证据,不能仅凭文件未改动就断言无关。最小补证是针对这一场景做同条件对照、保留相同失败指纹并排除因果交集,或提供可核实的基础设施归因。当前 review policy revision 25 要求先闭合此类失败归因,因此暂不发布 APPROVE;这不是要求在本 PR 中随意修改聊天 UI。
另一项当前 FAIL,归因仍 UNVERIFIED: dashboard-browser (2) 在同一 merge commit 上执行 goal-workspace-m2.mjs:206 时,五次已加载视图切换中一次耗时 1138.61ms,违反每次小于 1000ms 的固定预算。其余四次约为 817.32、684.27、881.39、553.64ms。尚未得到同条件 base/head 对照,不能将此失败归为外部噪声,也不能通过放宽预算使其通过。该项与前述聊天分页超时均需独立归因后再作批准判断。
我的整体评价 / Verdict
局部修复目标已达到:后续重规划能看到原本被重复工作挤出的已声明工作结果;只读证据、完整计数、旧引用资格和策略关闭边界仍保留。长程维度的已验证收益限于证据可见性,用户调用与恢复流程没有新增操作轮次;模型采纳与最终任务收益仍未知。检查的改动仅在 replan_context.ts 及两个测试文件,未改变 UI 渲染或布局,因此没有适用的视觉评审。本次没有视觉验证缺口;上文两项 CI 失败的归因另行保留。
结构与代码规模相称,新增共享函数消除了 typed/ordinary 窗口策略分散,未发现需要再引入抽象或协议迁移的理由。当前 REQUEST_CHANGES 是验证证据 hold,findings 为空;请补齐上述失败归因或能覆盖该风险的当前证据后再作批准判断。发布前还必须确认远端仍为本 head。审查本身不授予 merge 权限,也不能把缺失结果事件、无后继或整体性能问题视为已经被这个窗口修复解决。
English verdict: REQUEST_CHANGES for head a32592d: no actionable code defect found; the bounded-evidence fix passes targeted verification, but the two current dashboard CI failures remain unattributed under review policy revision 25.
Review follow-up — unchanged head a32592dThe requested CI investigation has been performed against the fixed base Loaded lens budget: alternating base/head executions of the original
An earlier head execution also failed this same assertion (1389 and 1125 ms switches). Both that failure and the failed base execution are retained; this is not a claim of a performance gain or universal browser stability. The failure is reproducible without this PR. All 272 dashboard source files and 131 scenario/fixture source files are byte-identical between base and head. The 35 generated Chat delivery files outside the release manifest are also identical; that manifest correctly records differing source revisions. Attached history pagination: the complete original The first baseline setup attempt lacked generated Chat assets and had shared dependency paths rejected by Vite. Those setup failures were corrected by building the original bundle and using checkout-local dependencies before the comparison above; they are not counted as baseline regressions or passing scenarios. All inputs were synthetic/loopback and no model calls were made. The original CI failures and review remain recorded. This follow-up supplies the requested baseline reproduction and current exact-head scenario evidence; it does not change the replan patch, relax browser budgets, dismiss the review, authorize merge, or claim a solver gain. Remote CI/re-review remains required. Remote CI rerun note: the failed-jobs rerun API returned HTTP 403 (repository administrator permission required). The existing upstream run remains pending. A maintainer must rerun it; no permissions, workflow gates or assertions were changed to work around that restriction. |
Goal And Delivered Outcome
When several Todos are declared by one path decision, the six-row followthrough window can fill entirely with repeated observations of one Todo. A later review then sees no result for another declared experiment even though that result is in scoped history.
Reserve one newest observation for each displayed planned Todo, fill remaining slots with linked work before unrelated work, and retain source order. Apply the same bounded rule to ordinary writebacks and typed observations. Full counts, omitted counts, immutable evidence references and full-history citation replay retain their existing owners. This is a display correction, not a claim of execution or utility.
Author Declaration
Written by: model_agent — Codex, OpenAI GPT-6 family.
Implemented against
Specification: the bounded repair requested in this PR; existing replan followthrough and stride review contracts at base
04c5799b88f6c4cf12a224b7fa679f272e1e794e, inloopx/control_plane/work_items/replan_context.tsandstride_policy.ts.Scope And Continuation
Complete for the bounded evidence-window defect. The shared followthrough projection changes with stride both off and on; this does not activate stride. No policy forces a pivot, successor, minimum runtime or positive result. Better model choice and benchmark gains remain untested. The nearby duplicated typed/ordinary window logic now shares one helper; broader planner redesign is outside this fix.
Validation
Tested revision: a32592d. State: finished. Inputs: synthetic.
replan_context.test.tsandstride_policy.test.ts: 65 tests; control-plane typecheckNo model run or native score improvement is claimed. This makes existing evidence available to a review; it cannot prove the model adopts it.
Frontend / Visual Evidence
UI impact: none. These changes use existing internal CLI/runtime projections and introduce no screen, setting or capability. Source data: synthetic. Screenshots: N/A.
Type of Change / LoopX Area
Control plane and host integration; focused bug/performance fix with regression tests. No benchmark adapter, scoring, task prompt, budget or submission-policy changes.
Boundary Checklist
Shared-authority RFC fixture impact: N/A; no storage provider, routing or authority schema is changed.
Review follow-up — unchanged head a32592d
The requested CI investigation has been performed against the fixed base
04c5799b88f6c4cf12a224b7fa679f272e1e794eand this PR head. No production or test assertion was changed for these checks.Loaded lens budget: alternating base/head executions of the original
goal-workspace-m2scenario, with coverage enabled, identical dependency versions, the same browser/runtime and the original 1000 ms limit, produced:An earlier head execution also failed this same assertion (1389 and 1125 ms switches). Both that failure and the failed base execution are retained; this is not a claim of a performance gain or universal browser stability. The failure is reproducible without this PR. All 272 dashboard source files and 131 scenario/fixture source files are byte-identical between base and head. The 35 generated Chat delivery files outside the release manifest are also identical; that manifest correctly records differing source revisions.
Attached history pagination: the complete original
attached-host-follow-upscenario, including both confirmed and uncertain native attachment flows, passed on base (70.3 seconds) and this head (68.4 seconds). This covers the original newer-page assertion without raising the 30-second wait. The CIafter-response timeout was not reproduced locally, so its infrastructure cause remains unverified; passing local coverage is not a retroactive pass for the failed CI job.The first baseline setup attempt lacked generated Chat assets and had shared dependency paths rejected by Vite. Those setup failures were corrected by building the original bundle and using checkout-local dependencies before the comparison above; they are not counted as baseline regressions or passing scenarios. All inputs were synthetic/loopback and no model calls were made.
The original CI failures and review remain recorded. This follow-up supplies the requested baseline reproduction and current exact-head scenario evidence; it does not change the replan patch, relax browser budgets, dismiss the review, authorize merge, or claim a solver gain. Remote CI/re-review remains required.
Remote CI rerun note: the failed-jobs rerun API returned HTTP 403 (repository administrator permission required). The existing upstream run remains pending. A maintainer must rerun it; no permissions, workflow gates or assertions were changed to work around that restriction.