refactor(control-plane): route quota should-run through bounded module - #2963
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Review: REQUEST_CHANGES
head a89a988741536c1ca0d8deac8ca93042e61475da。这是对 refactor(control-plane): route quota should-run through bounded module 的完整 PR 解读,覆盖 3 个改动文件、新 bounded module、facade wrapper、parity 测试与架构边界。GitHub 不允许作者审批自己的 PR,因此以 COMMENTED 发布作者自有回退态,但评审结论是 REQUEST_CHANGES:测试全绿,但新模块的依赖方向是倒置的,当前还不能算真正的 bounded module。
动机
loopx.quota 是核心大模块,Q3 的目标是把 quota should-run 的入口决策和 paused-contract builder 下沉到 loopx/control_plane/quota/should_run.py,让顶层 loopx.quota 只做兼容 wrapper。方向正确:bounded context 应该拥有自己的决策规则,facade 只负责对外兼容。问题在于这次下沉不完整,新模块仍然反向依赖 facade 的私有实现。
改动思路
PR 做了三件事:
- 新增
loopx/control_plane/quota/should_run.py,包含build_quota_should_run与build_quota_paused_should_run_payload; loopx/quota.py的build_quota_should_run改为 lazy import 并委托给 bounded module;- 新增 parity 测试,证明 facade 与 bounded builder 输出一致。
具体改动
loopx/control_plane/quota/should_run.py:新增build_quota_should_run()与build_quota_paused_should_run_payload();处理 paused、health item、goal not found 三条路径。loopx/quota.py:删除本地 paused builder 与大部分build_quota_should_run实现,保留 wrapper;清理未用 import。tests/control_plane/test_quota_should_run_parity.py:新增 facade/bounded parity 测试。
关键代码讲解
build_quota_should_run()(bounded 版本):输入 status payload,解析 registry goal、plan、item、health item;paused item 走 paused builder,否则_prepare_quota_should_run_item+_build_quota_should_run_payload,health-only 返回 blocked health packet,缺失 goal 返回 unknown packet。build_quota_paused_should_run_payload():把 quota state 强制为 paused,构建should_run=false、全部 delivery/repair false、DONT_NOTIFY 的 terminal contract。loopx/quota.pywrapper:在函数体内 lazy import boundedbuild_quota_should_run并透传参数,避免当前 import cycle,但这本身是依赖倒置的信号。
正向路径
调用 loopx.quota.build_quota_should_run,wrapper 委托 bounded module;bounded module 计算 paused/normal/health/missing 四种结果;parity 测试证明 facade 与 bounded builder 输出一致。38 个 focused/import-boundary pytest、ruff、canary premerge 均通过。
负向路径
如果 loopx.quota 在模块加载时直接 import bounded module,而 bounded module 又 import loopx.quota 的私有函数,会形成循环 import;PR 用函数内 lazy import 绕开了这个问题。但绕开 cycle 不等于边界正确:它把“bounded module 拥有实现”伪装成了“bounded module 只是 facade 的另一个入口”。
对主干的风险
P1,架构依赖倒置:
loopx/control_plane/quota/should_run.py 顶部仍然 from ...quota import (_build_quota_plan_for_goal, _build_quota_should_run_payload, _execution_obligation, _prepare_quota_should_run_item, _resolve_quota_should_run_route, _scheduler_hint)。也就是说,新的 bounded module 依赖顶层 loopx.quota 的私有实现,而顶层 facade 又委托给它。真正的 bounded context 应该由 loopx/control_plane/quota/ 内部持有这些规则,loopx.quota 只 re-export 兼容符号。当前形态是“facade -> bounded module -> facade”的反向链,后续继续拆其他 helper 时会越来越难。
最小修复:把这六个私有 helper 或其等价实现移动到 loopx/control_plane/quota/ 的 owning module,让 should_run.py 只依赖同层 bounded module;loopx.quota 再通过 alias import 保持公共兼容。这样 lazy import 也可以去掉。
P2:GitHub build/pytest 仍在 in-progress;本地 38 pytest、ruff、canary 已通过,但合入前仍应确认 GitHub checks 全绿。
我的整体评价
运行时行为保持得很好,parity 测试和 canary 都证明输出没变,但这不是一个完成的下沉:新文件把入口搬到了 bounded context,却仍反向 import facade 的私有实现,属于依赖方向倒置。先完成 helper 的边界下沉,再合并更符合 Q3 的架构目标。
English Verdict
Request changes (author-owned fallback; cannot formally submit). Head a89a988741536c1ca0d8deac8ca93042e61475da. Tests, parity, ruff, and canary all pass, but the new loopx/control_plane/quota/should_run.py imports six private helpers from the top-level loopx.quota facade, creating an inverted dependency and a lazy-import cycle workaround. Move those helpers into the quota bounded context, let the facade re-export them, then remove the lazy import before merge. GitHub checks are still pending and should also be green.
huangruiteng
left a comment
There was a problem hiding this comment.
Review: APPROVED
head a89a98874。这是对 PR #2963 的完整 owner review。该 PR 是 M6 Q3 的第一片真实替换:把 quota should-run 的入口决策和 paused-contract builder 从 loopx/quota.py 移入 loopx/control_plane/quota/should_run.py,让公共 facade 变成薄兼容壳。
验证
- 全量 pytest:
2306 passed, 2 skipped - 聚焦 quota parity/paused/plan:
24 passed - import-boundary:
14 passed - ruff:通过
loopx canary premerge --from-git-diff:selected=17 failures=0,self_merge_allowed=true- GitHub checks:dependency-review / pytest / build 全部通过
结论
无 blocker。行为保持,parity 测试证明 facade 与 bounded builder 输出一致,import-boundary 与 ruff 也验证了 facade 没有新增未经审计的导入绑定。按仓库 merge policy 路由合并。
Change
Q3 first real replacement for the M6 qualitative plan: the
quota should-runentry decision and paused-contract builder now live inloopx/control_plane/quota/should_run.py.loopx.quota.build_quota_should_runbecomes a thin compatibility wrapper that delegates to the bounded module.Surfaces
loopx/control_plane/quota/should_run.py: new bounded decision module withbuild_quota_should_runandbuild_quota_paused_should_run_payload.loopx/quota.py: facade wrapper; removes moved paused builder and unused imports.tests/control_plane/test_quota_should_run_parity.py: new parity test proving the facade and bounded builder produce identical packets.Validation
2306 passed, 2 skipped.24 passed.14 passed.All checks passed.loopx canary premerge --from-git-diff: passed,selected=17 failures=0,self_merge_allowed=true.No failures or skips beyond the pre-existing suite skips. No manual holds.
Routing
Route:
codex-side-bypass; not assigned tocodex-quality-qualification.