Repository navigation
fix(canary): lower the stale collaboration_mcp module ceiling - #7646
huangruiteng merged 1 commit into
Conversation
c2c8620 to
56fc7fa
Compare
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · reasoning_effort=max
未发现阻塞 finding,建议 APPROVE。对象 56fc7fa3b1bf7c2b581a726c63f84f1e1f84e30c,基线 9dcdc0bacd455a8cf6f6146f052ffc4df0f5d04a;本次未查询 GitHub CI。
动机 / Motivation
维护仓库质量门的开发者会遇到一个模块已经降到默认规模以内、账本却仍宣称它需要更高上限的过期记录。collaboration_mcp.py 实测为1924行,旧账本还保留2058行例外,导致例外生命周期测试失败;删除这条记录后,它回到既有2000行默认上限,当前源码通过,未来2001行增长则会被报告。本次实测恢复了账本一致性,并验证只收紧这个已退役的例外,邻近模块和新模块的既有规则保持不变。只移除过期模块上限,不改运行时代码、默认预算、语义注册表或实验资格门。
改动思路 / Approach
模块已经回到现有默认上限以内,删除失效例外即可复用原检查器;把1924写成新上限会无故冻结当前代码量。只退役 collaboration_mcp.py 的2058行例外,默认2000和其它模块上限均保持。这里的账本保存经过评审的特殊上限,实际源码大小由现有度量器读取。当前模块已不需要特殊上限,保留旧记录会允许后续2001至2058行的增长继续不报错;只把它改成1924,又会制造一个与当前粗粒度政策不符的精确行数钉子。
具体改动 / Changes
规范依据为 AGENTS.md,spec_revision 9dcdc0bacd455a8cf6f6146f052ffc4df0f5d04a,相关条件 Evidence-Based Budget Decisions:同输入测量、保留旧失败,不靠放松门槛把失败说成通过。本次只退役既有例外,当前源码与默认预算均未改变。
关键代码讲解
module_metric_baseline(maintainability_ratchet.py:519)校验 schema 和 default_limits 与运行时一致,再读取各模块上限。本次只删 collaboration_mcp.py 的 lines=2058,没有改 reader 或其它字段。
collect_module_metric_findings(同文件:584)先建立默认2000/300/300,再叠加对应路径的覆盖值。删掉该路径记录后,模块自然回到2000行,不需要新判断。实际源码1924行、Any=0、dict[Any]=0;现有生命周期测试会拒绝“源码已经低于默认、账本却还保留超默认额度”的记录。
| 同一文件输入 | 基线2058上限 | head默认2000 |
|---|---|---|
| 1924或2000行 | 通过 | 通过 |
| 2001或2058行 | 通过 | 报告超限 |
| 2059行 | 报告超限 | 报告超限 |
对主干的风险 / Risks
主要风险是误伤其它模块或只验证当前绿色、没有验证未来阈值。我在两版源码下运行同一脚本,通过真实临时源文件、相同 AST/行数度量器和各自真实账本读取8组输入;fixture 指纹和度量器源码均一致。除了表中目标阈值,还核对保留2020额度的邻近模块、新增2001行模块,以及把目标还原1924后的恢复。邻近模块和新模块规则一致,恢复后不再报错,检查不写入源文件或账本。
基线生命周期测试实际复现 1924 > 2000 失败;head 的 canary 目录89 passed。两版真实仓库的公开 ratchet smoke 均正常、仍只有2项既有兼容 facade 债务;因此这个 smoke 的绿色本身不能替代退役记录的生命周期测试。PASS:标准 premerge 的4项直接检查、5项 catalog 与8项选定风险检查。
语义与 CI 对齐
这是 reuse_existing,JSON schema、默认预算和词汇不变;目标未来2001至2058行增长恢复受默认预算约束,是明确的有意变化。原失败证据保留,没有放松冻结资格或修改测试期待。GitHub CI 按 wait_for_ci=false 未查询。
我的整体评价 / Verdict
long_horizon 与 user_experience 均 preserved:静态维护政策恢复一致,不改变 Agent 运行、用户配置或确认流程。只修改静态仓库检查的预算账本,没有产品布局或渲染变化;视觉评审不适用。没有重新运行全量发布资格,也不据此宣称 Agent 效果、效率或产品运行时有改善。删除3行配置与这个问题相称。未来若确有合理增长需求,应走已有评审流程重新说明额度,而不保留已失去依据的历史例外。
English verdict: APPROVE 56fc7fa;89 canary tests and local premerge pass, and matched real-file checks confirm the intended2058→2000 limit with unrelated scopes preserved.
`loopx/canary/module_metric_baseline.json` records a reviewed ceiling for every module that exceeds the coarse 2000-line default. `loopx/collaboration_mcp.py` was recorded at 2058 lines, but two later changes moved that decision into the typed owner and the module now measures 1924 lines. The entry still claims an exception the module no longer needs, and `tests/canary/test_maintainability_ratchet.py:: test_current_repository_debt_is_reviewed_without_line_count_pins` fails on main: ``` assert 1924 > 2000 ``` The entry is removed rather than lowered: with the module back under the default limit, the default limits are the honest bound and no reviewed exception is required. The module's other metrics are 0 for both the `Any` and the `dict[str, Any]` counts, so the default limits hold on every metric. Validation: tests/canary/ 89 passed on main 2a23237, and examples/control_plane/control-plane-maintainability-ratchet-smoke.py is ok with unreviewed 0 and stale_exceptions 0. Signed-off-by: xiaoan17 <anbc1224@gmail.com>
56fc7fa to
6913de9
Compare
Scope
One stale entry in the module metric baseline, and the main-branch breakage it
was hiding.
loopx/canary/module_metric_baseline.jsonrecords a reviewed ceiling forevery module that exceeds the coarse 2000-line default.
loopx/collaboration_mcp.pywas recorded at 2058 lines, but two later changes moved that decision into the
typed owner, and the module now measures 1924 lines.
The entry still claims an exception the module no longer needs, and
tests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pinsfails on
main:The baseline is the repo's own pin: it keeps an exception while the exception
applies and lowers it in the same change that removes it. A stale entry that
claims a ceiling the module no longer crosses is the pin refusing to be honest
about its own debt.
Fix
The entry is removed rather than lowered. With the module back under the
default limit, the default limits are the honest bound and no reviewed
exception is required. The module's other metrics are
0for both theAnyand the
dict[str, Any]counts, so the default limits hold on every metric.Validation
On
main2a2323756:tests/canary/— 89 passedexamples/control_plane/control-plane-maintainability-ratchet-smoke.py— ok(
unreviewed: 0,stale_exceptions: 0)No production code, schema, wire or default change.
Signed-off-by: xiaoan17 anbc1224@gmail.com
Rebase and re-validation (2026-10-29)
mainatb201da8bab305d053866aa0d4516b97ba4a4499e6913de97de339f7a88ef66eba20e365d637c1100This is one of six pull requests that together restore
mainto green. Onmainatb201da8bab305d053866aa0d4516b97ba4a4499ethe repository is red in exactly six places, and this one owns the maintainability-ratchet red:tests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pinsThe other five are owned by #7631, #7632, #7640 and #7642. None of the six is caused by a pull request in this set.
All nine pull requests in this set were rebased onto the then-current
main(this one ontob201da8bab305d053866aa0d4516b97ba4a4499e) and merged together in a local worktree. The merged state passestests/architecture/,tests/canary/,tests/test_manager_context_roundtrip.py,tests/test_lark_native_reply_context.py,tests/control_plane/test_host_adapter_settlement_step_vocabulary.pyandtests/control_plane/test_host_adapter_settlement_partial_steps.py— 1805 passed, 0 failed.python scripts/generate_rfc_status_index.py --checkis clean.Merge sequence (2026-10-29)
mainto green. Each one fixes only its own share of the six places wheremainis currently red, so until all five land every one of them still shows the others' red tests in itspytest/test-shardlanes. test(semantics): point the stale turn_route fixtures at their current owners #7621 was merged the same way, with a known base-red. Merge them in any order; each merge removes one share.