chore(canary): record chat_runtime's reviewed module ceiling - #4567
Conversation
`examples/control_plane/control-plane-maintainability-ratchet-smoke.py` fails on main: `loopx/chat_runtime.py` is 1560 lines against its checked-in ceiling of 1502, and 35 `Any` against 33. The growth is exactly #4533's team-preview admission facts - one 51-line rule plus its 7-line wiring - and nothing else in the module moved, so this is reviewed growth rather than drift. The ceiling is the remedy the ratchet itself provides for that case. A module metric budget is settled in the checked-in ledger, not through `REVIEWED_MAINTAINABILITY_EXCEPTIONS`: an evaluated finding still counts in `category_counts` even when an exception covers it, so the ledger edit is the reviewer-visible act. The repository already works this way - `loopx/todos.py` was refreshed 2165 -> 2190 -> 2229 -> 2249 -> 2285, `#2896` is a baseline refresh, and `#2953` grandfathers a module. Relocating the rule was considered first and is not enough on its own: moving the whole method and its call site out of the module leaves 1504 lines, still above the frozen 1502, so the ledger would have to be edited either way and the extra churn would not restore the ceiling. The failure output also now names the ledger to refresh next to the finding. Main sat red here because nothing in the CI text pointed at `loopx/canary/module_metric_baseline.json`, and both the new line and the negative case are locked by focused tests. Verified: pytest tests/canary/test_maintainability_ratchet.py tests/control_plane/test_m6_quality_gates.py -q -> 12 passed; examples/control_plane/control-plane-maintainability-ratchet-smoke.py -> ok, unreviewed: 0. Follow-up (deferred, recorded rather than bundled): `loopx/chat_runtime.py` stays a hot module with no headroom, so the next change that touches the manager turn prologue - the turn context assembly and the `ManagerInspection` wiring around it - should extract that bounded context builder instead of growing the controller again. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Pre-merge gate
First gate run in this worktree reported two failures that were worktree environment gaps, not diff regressions: the TypeScript-backed smokes need Coverage sufficiency: the diff changes a canary policy ledger (JSON) plus a report string, so the meaningful checks are the ratchet's own smoke and focused unit tests, both run here; the ledger value was verified against |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head 0370a13a1b059939640c40392ca99039f149030c.
动机
main 上的 control-plane-maintainability-ratchet 检查当前是红的:module_metric_budget:loopx/chat_runtime.py 被判为 unreviewed finding,tests/canary/test_maintainability_ratchet.py 因此失败。作者在 PR 里把原因钉在 #4533 —— 那一次改动只动了这一个文件(git show 9714c9606 --stat -- loopx/chat_runtime.py = 58 insertions / 0 deletions),把行数从 1502 推到 1560、Any 计数从 33 推到 35。我在 head 上独立复算 module_metrics('loopx/chat_runtime.py') 得到 {'lines': 1560, 'any_count': 35, 'dict_any_count': 0},与账本记录逐字一致。
受影响的不是某个运行时调用方,而是仓库 CI 本身:这条 check 是 required,红着就等于每个开着的 PR 都要走 admin bypass 才能合。压缩成本很直接——reviewer 拿不到"该去哪里补账本"的指针,只能自己去读 ratchet 实现。
更小的修法被作者否决过两次,我认为否决成立:走 REVIEWED_MAINTAINABILITY_EXCEPTIONS 仍然计入 category_counts,现有断言依旧失败;把整条规则搬出 chat_runtime.py 也只能到 1504,仍高于 1502,账本无论如何都得改。
改动思路
入口是 examples/control_plane/control-plane-maintainability-ratchet-smoke.py 与 tests/canary/test_maintainability_ratchet.py;权威输入是 checked-in 的 loopx/canary/module_metric_baseline.json,决策与渲染归 loopx/canary/maintainability_ratchet.py。
改动做成两件互不干扰的事:把账本里 loopx/chat_runtime.py 的 ceiling 从 1502/33 记到 1560/35,以及在 report 渲染处补一行"要刷哪个账本"。这里的取舍是对的——它复用了 ratchet 自己的既有权衡机制(reviewed ledger,本来就是"审查过的增长就记账本"),没有新加 policy 字段、没有新增第二个 metric 来源、没有把 chat_runtime 的重构捆绑进来。作者在 repository_reuse 里逐一比过三条候选路径,结论与我一致:账本路径常量 MODULE_METRIC_BASELINE_PATH 与 _module_metric_baseline_name 已经存在,新增的 helper 只是把 policy 里声明的路径解析出来显示,不是新的权威。
正路很清楚:collect_module_metric_findings 拿真实 metrics 对比账本 → evaluate_maintainability_findings 不再产出 unreviewed → smoke 打 unreviewed: 0。状态迁移是"红 → 绿,且失败文案指向修复动作",没有引入任何新状态。
具体改动
3 个文件、+71/-2。按 changed_line_classification 拆开:约 20 行生产代码(一行 report 文案 + 一个从 policy 解析账本路径的 helper)、4 行账本记录、49 行测试(一正一负)。
关键代码讲解
loopx/canary/maintainability_ratchet.py 的 render_control_plane_maintainability_report(约 791 行):改动前只报债务计数,改动后在 findings 里出现 module_metric_budget 类别时追加一行,点名该去刷 loopx/canary/module_metric_baseline.json。关键不变量是这条线只在存在该类债务时出现——负例测试 test_review_without_module_metric_debt_keeps_the_report_unchanged 直接钉住"无该债务时输出不变"。新增的 _module_metric_baseline_name(payload) 在 policy 未声明路径时退回 MODULE_METRIC_BASELINE_PATH.name,无 I/O、无副作用,消费者是 smoke 输出与 pytest 失败文案。
loopx/canary/module_metric_baseline.json 里 loopx/chat_runtime.py 的 ceiling(约 38 行):1502/33 → 1560/35。这里的有意约束是"零余量"——记录值等于 head 实测值,不留 slack,所以下一次该模块再长就会立刻触发检查,这是设计意图而不是遗漏。消费者是 collect_module_metric_findings。
tests/canary/test_maintainability_ratchet.py 的两个新用例:一个断言债务报告确实点名账本,一个断言无模块指标债务时报告逐字不变。它们把"渲染分支"的两个方向都覆盖了,属于薄而持久的边界断言,没有去断言临时 builder 的偶然字段。
对主干的风险
最强的回归场景是"账本刷错模块、或刷出余量,从而把后续增长藏起来"。检查链路是 collect_module_metric_findings 拿实时 metrics 比账本,而记录值就是 head 实测值,没有引入 slack;爆炸半径只在 canary policy,不触及任何运行时行为或交付路径。回滚成本是恢复那 4 行账本,检查回到红。观测面是 smoke 输出的 unreviewed/stale_exceptions/magnitude_regressions,加上新增的 report 文案。
真实边界我验过:跑的是真 smoke 和真 module_metrics,不是只跑单测;git show 9714c9606^:loopx/chat_runtime.py | wc -l 的 1502 → 1560 与 PR 叙事吻合。本机未跑全量测试套件,但 canary/相邻 surfaces 的 12 条 focused 用例全过。
语义与 CI 对齐
受影响契约是 canary maintainability policy(模块指标 ceiling 与 ratchet 报告)。结论是 reuse_existing:这次改动是用 ratchet 自己文档化的机制(reviewed ledger + 失败文案指向账本)恢复 required check,没有新增或改写词汇。核对范围是账本取值、文案的守卫条件、以及拥有这条红检查的测试;三处都对齐。剩余一处语义脆弱点见 residual_risk:文案按字面类别名 module_metric_budget 匹配,类别改名会让这条指路提示静默消失(检查仍会失败并暴露问题,因此只降级为 P3 非阻塞)。
P3(非阻塞):账本对 loopx/chat_runtime.py 零余量,意味着后续任何触及 manager turn prologue 的改动都会立刻撞上这条 check。作者把"抽取 prologue"记在了 commit message 和 PR body 里,但这不进入任何可跟踪的 todo/monitor。建议把它落成一条 tracked follow-up,让下一个改这个文件的人能看到义务本身,而不是只看到一句历史说明。这不阻塞合并——增长有出处、记录值与 head 一致、修法是 ratchet 的既定机制。
100% 校验:pytest tests/canary/test_maintainability_ratchet.py tests/control_plane/test_m6_quality_gates.py -q = 12 passed;examples/control_plane/control-plane-maintainability-ratchet-smoke.py = ok,unreviewed: 0。
我的整体评价
baseline(origin/main ccbc53c)与 head(0370a13a1)的对比:finding 计数 unreviewed: 1 → unreviewed: 0(debt compatibility_facade=2 保持不变);失败文案从"无账本指针"变成点名账本;无该债务时的报告输出逐字不变。三行对比里没有隐藏的语义漂移。
改动体量必要(code_volume: necessary):4 行账本 + 20 行渲染/解析 + 49 行测试,没有新增 CLI 或状态契约。比例也合适(change_proportionality: proportionate):问题是确定性的、全仓范围的 required check 变红,最小可行修复就是这 4 行账本,文案与测试是让修法可被发现的小型配套。default_off_isolation 属 not_applicable(这是 CI required check,不是 opt-in 功能),authority_semantics: aligned(未引入 actor/lease/生命周期,也没有豁免该模块未来的 ratcheting)。
结论 APPROVE,不阻塞。P3 建议把 deferred extraction 记为可跟踪工作,以便下一次改动前可见。复评只需在 head 变化时重跑上面两条命令加上 module_metrics 复核。
English verdict: APPROVE - exact head 0370a13; the reviewed ledger refresh plus a ledger-naming report line restores the red required ratchet check without slack or new policy surface; validated by 12 passed (tests/canary + m6 quality gates), smoke ok with unreviewed 0, and module_metrics matching the recorded 1560/35. One non-blocking P3: record the deferred chat_runtime extraction as tracked work since the ceiling now has zero headroom.
Replaces the unsigned web-UI merge ea69b88 with a byte-identical tree so the DCO gate passes. Brings in the two baseline fixes this PR's checks were failing on: loopx-project#4565 (refresh-state test patch targets) and loopx-project#4567 (chat_runtime reviewed module ceiling). The M2 Turn contract, its 29 ordered controller rules, and the generated Python/TypeScript bindings are unchanged. Signed-off-by: song <22676124+songoow@users.noreply.github.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ree steward repairs main has been red since loopx-project#4587: `tests/canary/test_maintainability_ratchet.py` reports `unreviewed finding: module_metric_budget:loopx/chat_actions.py` because the module is 1604 lines against a reviewed ceiling of 1590. The failure reproduces on a clean `origin/main` worktree and on every open PR, so it blocks all merges including the pending steward stack. The growth is deliberate and already merged, not new debt invented here: the file was 1449 lines when the ceiling was set for loopx-project#4567, then 1520 (loopx-project#4582), 1575 (loopx-project#4585) and 1604 (loopx-project#4587). Those three repairs extended the single Chat action settlement owner with typed lane-level results (`lane_failure`, `lane_settlements`, bounded `details` on `mark_failed`) instead of a second settlement path, so the reviewer-visible decision this ledger records is to accept the module as the owner of that behaviour. It stays a bounded debt rather than a limit change: the default ceiling is 1500 lines, this module keeps its own 1604 entry, and `any_count` keeps its existing 52 headroom (currently 45). Extracting the lane-level settlement code now would rewrite work that three open PRs (loopx-project#4590, loopx-project#4600, loopx-project#4602) are already changing in this module. Validation: `tests/canary -q` reports 21 passed; on `origin/main` before this change the same group fails with the unreviewed finding above. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
What
Restore the red
control-plane-maintainability-ratchetcheck onmainand make its remedy visible at the failure.Why it was red
loopx/chat_runtime.pyis 1560 lines (any_count35) against its checked-in ceiling of 1502 (any_count33), so the ratchet reportsunreviewed finding: module_metric_budget:loopx/chat_runtime.py,unreviewed: 1, andtests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pinsfails with it.The growth is exactly one feature: commit
9714c9606(#4533) added_team_plan_admission_context— a 51-line rule plus its 7-line call site — and nothing else in the module moved.git diffagainst the previous baseline confirms the module was at 1502 before that commit.Why the ledger, and not an exception or a relocation
REVIEWED_MAINTAINABILITY_EXCEPTIONS: an evaluated finding still counts incategory_countseven when an exception covers it, so the test'sset(report["category_counts"]) == {"compatibility_facade"}would still fail. The checked-in ledger is the mechanism this ratchet provides.loopx/todos.pywas refreshed 2165 -> 2190 -> 2229 -> 2249 -> 2285;#2896ischore(canary): refresh module metric baseline;#2953isfix(canary): grandfather auto research demo module size. Values are recorded at the reviewed actual, with no headroom by design.The recorded entry becomes
lines: 1560,any_count: 35.Companion change
The failure text now names the ledger to refresh next to the finding. Main sat red here because neither the smoke output nor the pytest failure pointed at
loopx/canary/module_metric_baseline.json— the remedy was only discoverable from the JSON policy blob. Both the new line and the negative case (a review with no module metric debt keeps its output unchanged) are locked by focused tests.Validation
python -m pytest tests/canary/test_maintainability_ratchet.py tests/control_plane/test_m6_quality_gates.py -q-> 12 passedpython -m pytest tests/cli_commands tests/test_project_lifecycle_refresh_state_ownership.py tests/canary -q-> 123 passedpython examples/control_plane/control-plane-maintainability-ratchet-smoke.py->ok,reviewed_exceptions: 2,unreviewed: 0python -m ruff check tests loopx/canary-> All checks passedpython -m mypy-> Success, no issuesRisk and boundary
No runtime product behaviour changes: the ledger is canary policy input and the added line is a report string. No credential, private state, local path, or raw evidence is included. Deferred follow-up, recorded in the commit message:
loopx/chat_runtime.pynow has no headroom, so the next change touching the manager turn prologue (turn-context assembly and theManagerInspectionwiring) should extract that bounded context builder rather than grow the controller again.