Skip to content

chore(canary): refresh chat_actions' reviewed module ceiling to unblock main - #4618

Merged
huangruiteng merged 1 commit into
mainfrom
codex/canary-chat-actions-ceiling-20260917
Sep 17, 2026
Merged

huangruiteng merged 1 commit into
mainfrom
codex/canary-chat-actions-ceiling-20260917

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

  • Goal/source and gap: main has been red since fix(manager): make a partial team-plan materialization recoverable #4587. tests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pins reports unreviewed finding: module_metric_budget:loopx/chat_actions.py, so every open PR inherits a failing test-shard and no PR can merge.
  • Observable before → after: on origin/main (809f0cf) the ratchet fails with debt: module_metric_budget=1, unreviewed=1; with this one-line ledger refresh the report is ok: True, unreviewed: 0, finding_count: 2 (compatibility_facade), which is exactly what the test asserts.
  • Issue/task and intended base: main (809f0cf). No tracking issue; this is the canary's own prescribed remedy.

Scope And Continuation

Validation

  • Tested revision: bf2b283
  • Run state: finished
  • Input classes: none
Check kind Result Public-safe evidence / limitation
regression_parity passed Failing before: a clean origin/main worktree fails the ratchet with unreviewed finding: module_metric_budget:loopx/chat_actions.py (also failing on main's own Python Tests run 35176730888, job test-shard (2)). Passing after: same test green on this head.
unit passed uv run --extra test python -m pytest tests/canary -q → 21 passed
  • Coverage and gaps: the ratchet, and the ledger it reads, are the whole subject of this change; the group above is the owning suite. No runtime, frontend, permission or scoring behaviour is touched.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • States and viewports shown: N/A
  • Source data: none

Type of Change

  • Bug fix

LoopX Area

  • Build, packaging, installer, or CI

Technical Direction

  • Direction / acceptance reference, when applicable: the canary's reviewed module-metric ledger (loopx/canary/module_metric_baseline.json), whose own report text states that the ledger edit is the reviewer-visible decision whether growth is accepted.

Shared-authority RFC fixture impact

  • Production-scale fixture schema: N/A
  • Semantic dimensions changed, or reviewed no-impact rationale: N/A
  • Provider conformance arms run: N/A
  • Read-only legacy/file/PostgreSQL three-arm rehearsal: N/A

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths
  • I did not duplicate maintainer-owned benchmark work
  • I kept the change scoped to the linked issue/task
  • I completed the visual evidence section for UI changes, or marked UI impact none
  • Every commit includes a DCO Signed-off-by trailer

中文摘要

…ree steward repairs

main has been red since #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 #4567, then 1520
(#4582), 1575 (#4585) and 1604 (#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 (#4590, #4600, #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>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@/tmp/review-4618.md

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

main 从 #4587 合入起就是红的,而且这个红与任何一条在飞 PR 的改动无关。tests/canary/test_maintainability_ratchet.py::test_current_repository_debt_is_reviewed_without_line_count_pins 报 unreviewed finding: module_metric_budget:loopx/chat_actions.py:模块当前 1604 行,超过 ledger 里已评审的 1590。结果是每条 PR 的 test-shard 都挂在同一个用例上、全部合不了,包括本车道正在等的管家栈和 owner 待决的三条。

我先在干净的 origin/main worktree 上复现,再核对 main 自己的 Python Tests run 35176730888 的 test-shard (2),两处失败文本一致,所以这确实是一个仓库级阻塞,而不是本 PR 自己引入的债务。

改动思路

canary 对模块体积债务给出的处置方式是唯一的:债务在签入的 ledger 里结清,而不是走 reviewed exception——其渲染文本原话就是「refresh the reviewed ceiling in module_metric_baseline.json for the growth this review accepts」,也就是说这次编辑本身就是要评审的判断。因此本 PR 只改这一行,把上限刷新成本次评审接受的 1604,并在 PR 正文里交代增长来源与归属判断。

我没有选择「把增长抽出去」这条更漂亮的路,理由是三处已合并的修复恰好都加在同一个结算宿主上,而现在有三个在飞 PR(#4590、#4600、#4602)正在改这个文件;此刻抽取等于在三条未完成改动下面重写同一段代码,冲突成本高于收益,而且会把 owner 待决的那几条 PR 一起拖住。

具体改动

  • loopx/canary/module_metric_baseline.json:loopx/chat_actions.py 的 lines 从 1590 改为 1604。any_count 保持 52 不动(实测 45,仍有余量),模块继续保留自己的条目,不使用 1500 行的默认上限,债务保持可见、有界、可回退。

对主干的风险

风险面很窄:只动一个被 canary 直接读取的 JSON 数字,不改变任何运行时、前端、权限或评分行为。需要明确的是,这次编辑只判断「增长已经被接受」,它不会阻止再次增长——下次再涨仍会红,仍要重新评审。增长的归属我也写进了提交信息:1449(#4567 设定上限时)→ 1520(#4582)→ 1575(#4585)→ 1604(#4587),三次都是把按 lane 的结算结果加在同一个 Chat action 结算宿主上,而不是新开第二条结算路径,因此判断它属于该模块。

可复现的对照:改动前在干净 origin/main 上该用例失败;改动后 uv run --extra test python -m pytest tests/canary -q 为 21 passed,本 head 的全部必过检查转绿(含四个 test-shard、kernel-static-checks、dashboard-acceptance、merge-gate)。

我的整体评价

这是一次把仓库从红拉回绿的窄修复,判断依据可复现、范围可回退,且没有用「放宽默认上限」来换绿。建议在该 head 的必过检查已全绿的前提下合并。

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Head: bf2b283

English verdict: APPROVE

@huangruiteng
huangruiteng merged commit 3937f6b into main Sep 17, 2026
26 checks passed
@huangruiteng
huangruiteng deleted the codex/canary-chat-actions-ceiling-20260917 branch September 17, 2026 03:58
songoow added a commit to songoow/loopx that referenced this pull request Sep 17, 2026
A PR that grew a module past its reviewed line ceiling, without raising
that ceiling in the same diff, previously merged and turned main red until
a separate reconciliation PR refreshed the ledger. That split settlement is
the structural hole behind loopx-project#4587 -> loopx-project#4618/loopx-project#4619.

Add diff_scoped_module_ceiling_violations to the maintainability ratchet
and wire it into 'loopx canary premerge' as a direct gate check. The check
flags exactly the diff that crossed an inherited ceiling while leaving the
head ledger short; growth below the ceiling and in-diff ceiling settlements
both stay silent.

Signed-off-by: song <liusongstep@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant