Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewed exact head: cea53083a9201cfbf7e37d987d25000a0f48df02; immutable base: 420782f03725bf9b7603be481f2b0525beff5807.
动机
#4336 的维护者已确认旧 prose matcher 在当前版本移除,但 issue 的验收明确要求公开 quota should-run 回归,而非仅检查底层 helper。健康任务不应因为 installed / uninstalled / installation completed 的文字被强制 replan;同时真正的 typed no-progress 仍必须触发。这个测试方向有持续价值,但当前 diff 尚未完成 PR 所宣称的那项 CLI exit。
改动思路
现有链路是历史 run 经 Python codec 过滤、归一化,再由 TypeScript 的 history policy 判定重复。新增测试走 typed_progress_repeat_trigger,使用的是 project_replan_history(operation="progress");真实 quota/status 汇总则经 autonomous_replan_obligation_from_runs 调用另一条 all-history 入口。因此 helper 边界测试可以防止一类回退,不能代表公开入口已被回归保护。最小补齐是在现有 quota CLI fixture/harness 上增加一组参数化断言,无需新 matcher、生产修复或测试框架。
具体改动
唯一修改是 tests/control_plane/test_progress_observation.py 新增 54 行,一个测试函数;生产源码、schema、默认值及调度行为均未改变。
关键内容讲解
新增测试包含三条有意义的断言:四条纯 prose run 不触发;历史 observation 的 surface_id 非 opaque 时不被升级为 typed truth;两条有效且等价的 typed unchanged observation 仍触发。现有文件已有归一化拒绝、typed 正例、retry 去重等检查,这次纯 prose / 非法历史行窗口的负例并非重复的演示。正向对照少量重复用于避免“永不触发”的空洞实现,是合理成本。
底层测试独立运行通过;我把 progress_observation_from_run 临时变成 install / stall prose fallback,新测试在第一个断言失败,原有 22 项仍通过;恢复后新增测试再次通过。这证明其 helper 层价值,但不是完整 CLI 保护。
对主干的风险
[P1] 补齐公开 quota 入口的回归
位置:新增测试第 493 行及其三次 typed_progress_repeat_trigger 调用。问题不是当前产品仍有旧 bug,而是测试不覆盖 PR 与 issue 承诺的公开决策边界。
我用隔离的 synthetic registry、有效 run artifacts 和真实 TypeScript runtime 直接执行公开 quota should-run:base/head 上逐一测试四种 prose、非法历史 observation、有效 typed repeat,前五类不产生 replan,typed 对照产生 typed_progress_repeat,两版结果一致。随后仅在 quota 的 autonomous_replan_obligation 所使用的 all-history 调用边界恢复 prose→unchanged fallback,保留 compatibility helper 不变:新增单测仍是 1 passed,但同一公开 CLI 的 installed / uninstalled / installation completed 等输入变成 autonomous_replan_required。这是“测试绿但用户旅程仍可能回归”的可执行反例。
请在仓库测试中复用现有 CLI harness,写入两次成功 prose run 并逐词断言公开 JSON 的 obligation / trigger 和 decision,再加入有效 typed repeat 对照;保留现有 helper 测试。重新运行这组 CLI 回归及相关 pytest,并用上述 downstream fallback 确认公开入口断言会失败。评审者临时探针证明当前行为,不能替代交付的持久回归测试。
其他验证:head 的四个相关 pytest 文件 86 passed,相同 base 85 passed;Ruff、mypy、diff whitespace 及实际改动文件的公开边界扫描通过。本轮未查询 CI,REQUEST_CHANGES 不是因为非自身红 CI。PR 提到的 loopx/quota 目录实际不存在,其 scan 的 errors=0 不能算覆盖证明。最初私有 CLI fixture 缺少 adapter.kind 的诊断已修正后重跑,不是产品或 PR 失败。
我的整体评价
REQUEST_CHANGES,需补一个小而完整的 CLI 回归边界,不要求扩大生产改动。长期有用工作和用户体验在当前产品上保持正确;新增 helper 防回归也有价值,但对已接受的公开入口承诺仍有一个真实、局部可修的缺口。已检查同作者近期批次及现有覆盖:本项是现有单测中的薄增量,不以提交频率或作者身份判为重复贡献。未来向 refactor 检查考虑了 Python codec / TypeScript policy 的单一规则所有权,当前无需额外抽象;补测试应继续复用该 owner。升级指引仍属于 #4336 的明确剩余项,当前不能据此关闭整个 issue。
English verdict: REQUEST_CHANGES - head cea5308; useful helper regression, but #4336's promised public quota CLI regression is missing. A downstream prose-fallback mutation leaves the new test green while the real quota CLI wrongly requires replanning. Base/head normal CLI behavior and 85/86 related tests pass; CI was not consulted.
c0a6b4c to
bbcc30a
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — reviewed exact head bbcc30a111e9c419ac03420f121cf0ebcd4a8ad5 against 69adeba1d63b02a764048a30ecc4bc2b3c2cdb02.
动机
#4336 记录的是旧版把 installed 的自由文本误当作 stalled,进而在健康 Goal 上要求自主重规划。当前产品已移除那条文本推断,仍需要一项持久回归,保证公开 quota should-run 的决策不会因兼容路径重新读取文案而退化,同时明确的 typed no-progress 仍会触发。这个目标对持续执行和用户看到的行动要求都有价值;本 head 只锁住底层 helper,尚未交付 issue 与 PR 自己声明的公开 CLI 验收。
改动思路
新增测试调用 typed_progress_repeat_trigger,它通过 project_replan_history(operation="progress") 读取 typed observation。真实 quota/status 的 autonomous_replan_obligation_from_runs 则在 all-history 入口调用同一投影,不指定该 helper 的 operation;随后才构造对外的 replan obligation。这两个边界共用核心词表,但不是同一个入口。保留 helper 的负例和 typed 正例是合理的薄测试;最小补齐是在现有隔离 registry/runtime 的 quota CLI harness 中断言最终 JSON 决策,而不是新增生产 matcher 或另起测试框架。
具体改动
本次完整 diff 仅修改 tests/control_plane/test_progress_observation.py,新增 54 行,一个测试函数;没有改运行时、持久 schema、权限、默认调度或前端。提交从旧基线重新落到当前 main 后,也没有增加此前评审要求的 CLI 测试。
关键代码讲解
test_prose_progress_summaries_never_trigger_replan依次放入installed、uninstalled、installation completed和含真实 stalled 文案的 run,仅使用summary/note;对typed_progress_repeat_trigger断言没有触发,能保护 helper 不读取文案。- 同一测试把自由文本塞进 observation 的
surface_id,要求progress_observation_from_run拒绝不合法的历史 typed 行,避免偷偷升级为控制事实。 - 最后一组有效且重复的 typed observation 必须得到
typed_progress_repeat,避免“所有输入都不触发”造成空洞绿灯。真实公开 CLI 的decision、autonomous_replan_obligation和 trigger 没有出现在新增断言里。
对主干的风险
[P1] 公开 quota 入口的回归仍缺席
风险不是当前产品已复现旧 bug,也不是无关红 CI,而是本 PR 交付的测试无法守住接受标准:未来若只有 quota 的 all-history 调用边界恢复 prose→no-progress 兼容分支,新增 helper 测试仍可通过,用户却再次收到 autonomous_replan_required。这条路径与前一 exact-head 评审所指出的缺口相同;本轮检查了新 base 与完整 diff,未发现另一个覆盖 installed/uninstalled/installation completed 的公开 quota 测试。当前 23 项定向 pytest 全通过,git diff --check 无问题。扩展 quota CLI 测试时本机临时卷耗尽,所见夹具创建错误不归因于此 PR,也不据此阻断;通过项仍不能替代缺失的公共边界。未查询远端 CI。
请复用仓库现有的 _write_fixture/_run_cli 或等价隔离 harness,写入连续成功且只有上述 prose 的 run,经真实 quota should-run 断言没有 no-progress obligation;再写入有效 typed repeat,断言仍产生正确 obligation。最好把这组断言对 all-history 边界的临时 prose fallback 做 mutation,证明它会失败。保留本次 helper 测试。旧版用户升级指引是 #4336 的另一未完成验收项,可由 issue 后续处理,不需要塞进这个测试 PR 才能合并。
语义与 CI 对齐
本 PR 没有改变产品行为;它复用现有 typed observation 词表,没有新状态分类、默认开关或权限。核心语义是“自由文本仅供展示,不能成为机器重规划义务”;新增 helper 负例与此一致,但公开 CLI 的机器义务尚无持久断言。对无关 CI 红灯不作 request-changes 判断;这里的阻断只来自明确验收边界与实际测试入口不一致。
我的整体评价
REQUEST_CHANGES。新增测试有真实、非重复的防回归价值,代码量和维护成本也合适;但作为本 PR 声称交付的 GH-A03 CLI exit,它仍是一个可在同一小批次补齐的半成品。long_horizon 风险在于未来兼容分支绕过 helper 后重复误报;user_experience 风险是健康安装再次被迫重规划。补齐公开入口测试后再审,不要求无关全仓正则审计或 TypeScript 重写。未合并,也不以本地通过替代最终 head 的评审。
English verdict: REQUEST_CHANGES - exact head bbcc30a111e9c419ac03420f121cf0ebcd4a8ad5 adds useful helper coverage, but #4336 and this PR require a public quota should-run regression. The new test only invokes typed_progress_repeat_trigger; the all-history CLI decision can regress independently. 23 focused tests and diff hygiene passed; remote CI was not polled.
The existing test calls `typed_progress_repeat_trigger` directly, but the path a user exercises is `quota should-run` through `autonomous_replan_obligation_from_runs`, which projects the "all" history window. A prose compatibility matcher restored only at that boundary keeps the helper test green while a healthy goal receives `autonomous_replan_required` again (loopx-project#4336). Add an isolated active goal with an open agent todo and four consecutive successful free-text runs whose only content is "installed" / "uninstalled" / "installation completed", then assert through the real public CLI that the packet carries no replan obligation and no `typed_progress_repeat` marker. The typed repeat control runs the same fixture, so the guard is not vacuous. Verified by mutation: restoring a prose fallback for `operation == "all"` only makes these three cases fail while the helper-level test still passes. Signed-off-by: kokokoXUY <13682395396@163.com>
bbcc30a to
b8f17e8
Compare
|
Thanks — the gap was real and is now closed at the public boundary. What changed (new head
Mutation proof (exactly the counterexample you described): temporarily restoring a prose fallback that only applies where Local results: No production-code change is included, as you asked. |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
Issue #4336 中,成功安装的进度文案包含 installed,曾被误当作停滞并触发自主 replan。现有 typed 规则已修复运行时,但此前回归只测内部 helper;若另一个历史记录分支重新按文字分类,用户仍可能被错误地中断。这个 PR 交付的是公开 quota should-run 入口的持久回归守卫;旧安装升级指引仍是独立的后续工作。
改动思路
测试在隔离目录建立 active Goal、注册表与真实 run index,再通过 CLI 子进程取 quota JSON。负例用成功运行的 installed、uninstalled、installation completed 文案但不提供 typed observation;正例在同样的文案上加入两条规范化 typed 观察。这样同时证明“文案不触发”和“typed 重复仍触发”,而不新增分类器或修改生产规则。
具体改动
原 helper 级测试保留;新增一个可复用的 CLI fixture、run-index 写入工具,以及三个参数化的公开入口负例和一个 typed 正例。负例检查 decision、replan_action_packet、requires_user_action 和序列化结果,正例核对 autonomous_replan_required 及义务 ID。完整 diff 仅修改一个测试文件、增加 193 行;没有运行时代码、状态格式或默认行为变化。
对主干的风险
未发现阻塞项。最强的反例是 helper 仍正确,但 quota 的 all-history 路径又加入文字 fallback;新测试正好经过这个真实入口,旧测试不能覆盖。本人在当前 head 运行该文件 27 个测试、连同相邻 replan policy/transport/provider 共 44 个测试,Ruff 与 diff 检查也通过;未查询远端 CI。测试使用合成 Goal,而非历史 0.4.3 安装,因此不把它当作旧安装升级流程的验收。
我的整体评价
APPROVE。它用测试而非新机制保护既有 typed 决策 owner,能减少长期无谓 replan,并让实际 CLI 用户获得可信的继续执行判断;当前运行时体验不变,未来回归会更早暴露。未来若增加更多 quota-history 用例,可复用此 fixture,不必扩大这次 PR。建议 maintainer 按自身合并门禁处理;此评审不宣称远端 CI 通过。
English verdict: APPROVE - At exact head b8f17e8e7098d54e5c964c37ce4d97d0d94df386, the real quota CLI is guarded against prose-only false replans while typed repeated progress still triggers the obligation. Focused and adjacent local tests passed; remote CI was not consulted.
Goal And Delivered Outcome
S3· #4336.A 0.4.3 install saw
installedin a progress summary matched asstalledand receivedautonomous_replan_requiredon a healthy goal. Typed progress observations replaced that prosematcher after feat(control-plane): close replan through semantic progress #3161, but no public regression pins the contract, so a compatibility matcher
could restore the prose path without failing anything.
window;
tests/control_plane/test_progress_observation.pynow hastest_prose_progress_summaries_never_trigger_replan.quota should-runregression withinstalled/uninstalled/installation completedsummaries emits no no-progress trigger, a typed no-progress observationstill does".
What The Test Pins
summary/notetext —installed,uninstalled,installation completed, andnot installed; stalled on the same route again— produce no trigger fromtyped_progress_repeat_trigger.progress_observationcarries prose insurface_idstill produce no trigger, matchingprogress_observation_from_run's documentedbehaviour that "invalid historical rows are not silently upgraded into typed truth".
kind == "typed_progress_repeat", so assertions 1 and 2 are not vacuous.Scope And Continuation
That is a comment on Legacy replan matcher treats 'installed' as a stall signal #4336 rather than a repository change, and I am not the reporter.
examples/control_plane/autonomous-replan-no-change-smoke.py. That script is not in the tree(the column does hedge with "if present"), so the focused pytest carries the validation; the other
named command,
loopx check --scan-path loopx/quota, was run and is reported below.Validation
unitpassedpython -m pytest -q tests/control_plane/test_progress_observation.py→ 23 passed (22 before this change).unitpassedpython -m pytest -q tests/control_plane/test_progress_observation.py tests/control_plane/test_autonomous_replan_ack.py tests/control_plane/test_goal_amendment_proposal.py→ 72 passed, so the contract the new test touches is otherwise unchanged.staticpassedloopx check --scan-path loopx/quota→errors=0.manualpassedprogress_observation_from_run(synthesise anunchangedtyped observation when the summary or note containsinstall/stall) makes exactly this test fail at its first assertion (1 failed); reverting the mutation returns1 passed. The product file was restored byte-identical afterwards.rule — at the same function the window reads. It does not reproduce the original 0.4.3 install
end-to-end; the defect's product path was removed in feat(control-plane): close replan through semantic progress #3161, which is why the regression pins the
boundary instead of the removed matcher.
Frontend / Visual Evidence
Not applicable: one Python test file.