refactor(control-plane): move quota should-run preparation into bounded module - #2964
Conversation
7ae05c4 to
943fcaa
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Review: REQUEST_CHANGES
head 943fcaa06115a7704181e0040fcc6078a623eda0。这是对 refactor(control-plane): move quota should-run preparation into bounded module 的完整 PR 解读,覆盖 3 个改动文件、preparation helpers 的下沉、facade 收窄与测试。这比 #2963 前进了一大步,但 #2963 指出的依赖倒置仍未完全消除,当前 head 还不能算真正的 bounded module。
动机
#2963 只是把入口搬进 loopx/control_plane/quota/should_run.py,bounded module 仍反向 import loopx.quota 的私有实现。这个 PR 的目标是把 _prepare_quota_should_run_item 及其 decision-preparation helpers 真正移入 bounded module,让 loopx.quota 只保留 facade 和剩余 resolver/packet 阶段。方向正确,也确实显著缩小了 quota.py。
改动思路
should_run.py新增并拥有_prepare_quota_should_run_item、work-lane preparation、blocked-priority fallback、automation prompt upgrade、todo identity/class helpers、recovery allowance 等实现。quota.py删除这些私有 helper,保留 publicbuild_quota_should_runwrapper 与剩余 route/payload 阶段。- 测试的 monkeypatch seam 移到 bounded module。
具体改动
loopx/control_plane/quota/should_run.py:+606 行,新增 preparation 相关 helper 与_prepare_quota_should_run_item。loopx/quota.py:-550 行,移除已下沉的私有 helper。tests/control_plane/test_quota_paused_precedence.py:monkeypatch seam 改到 canonical bounded module。
关键代码讲解
_prepare_quota_should_run_item:现在由 bounded module 拥有,负责把 status、agent identity、workspace guard、work-lane contract、capability gate、todo summary 等组装成准备后的 decision item。_blocked_priority_fallback():从 agent todo summary 中识别更高优先级 blocked/deferred todo,并生成只读 fallback 说明。_automation_prompt_upgrade():委托build_identity_aware_prompt_upgrade,保持 identity-aware 语义。_QuotaDecisionPreparation/ 剩余 resolver 阶段:仍留在loopx.quota顶层。
正向路径
调用 loopx.quota.build_quota_should_run,wrapper 委托 bounded module;bounded module 拥有 preparation 逻辑,facade 只负责剩余 route/payload 和公共兼容。38 个 focused/import-boundary pytest、ruff、canary premerge 均通过,git merge-tree origin/main HEAD 无冲突。
负向路径
当前 loopx/control_plane/quota/should_run.py 顶部仍有:
from ...quota import (
AUTONOMOUS_REPLAN_ACK_NEUTRAL_CLASSIFICATIONS,
_QuotaDecisionPreparation,
_build_quota_plan_for_goal,
_build_quota_should_run_payload,
_execution_obligation,
_resolve_reward_memory_experiment_from_status,
_resolve_quota_should_run_route,
_scheduler_hint,
)也就是说 bounded module 仍然依赖顶层 loopx.quota 的私有符号。这不是“只有入口在 bounded module”,而是“入口和 preparation 在 bounded module,核心 resolver/packet 仍在 facade 并被反向引用”。
对主干的风险
P1,依赖倒置未完全消除:
should_run.py仍从loopx.quotaimport_build_quota_plan_for_goal、_build_quota_should_run_payload、_execution_obligation、_resolve_quota_should_run_route、_scheduler_hint、_QuotaDecisionPreparation等私有符号。bounded module 继续反向依赖 facade,后续拆分这些 stage 时会越来越难。- 最小修复是把这些剩余 helper 也移入
loopx/control_plane/quota/的 owning module,让should_run.py只依赖同层 bounded context;loopx.quota只负责 alias re-export。
P2:
- GitHub build/dependency-review 已绿,pytest 仍在 in-progress;合入前需确认 rebase/checks 全绿。
- 当前
git merge-tree origin/main HEAD已无冲突,这一点比之前好。
我的整体评价
这是一个很好的增量,preparation 逻辑确实下沉了,quota.py 也明显变小,测试和 canary 都证明行为没变。但 #2963 的核心架构问题仍然存在:bounded module 还反向 import facade 的私有 resolver/packet 符号。建议把剩余 helper 一并下沉后再合,否则这一系列 Q3 refactor 会在边界上留下长期 debt。
English Verdict
Request changes (author-owned fallback; cannot formally approve). Head 943fcaa06115a7704181e0040fcc6078a623eda0. Preparation logic is now correctly moved into the bounded quota module and 38 tests, ruff, and canary pass, but the inverted dependency remains: loopx/control_plane/quota/should_run.py still imports private resolver/packet helpers from the top-level loopx.quota facade. Move those remaining helpers into the bounded context before merge; checks are also still pending.
huangruiteng
left a comment
There was a problem hiding this comment.
Review: APPROVED
head 943fcaa06。这是对 PR #2964 的完整 owner review。该 PR 是 M6 Q3 的第二片真实替换:把 _prepare_quota_should_run_item 及其决策准备私有依赖从 loopx/quota.py 移入 loopx/control_plane/quota/should_run.py,同时保留 loopx.quota 的公共兼容壳。
验证
- 全量 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。reward-memory resolver 通过 loopx.quota 的本地兼容包装接入,避免 control-plane 新增 capability 层依赖;paused-precedence 测试的 monkeypatch seam 已迁到新的消费模块。按仓库 merge policy 路由合并。
Change
Q3 second real replacement slice for M6:
_prepare_quota_should_run_itemand its private decision-preparation helpers move fromloopx/quota.pyintoloopx/control_plane/quota/should_run.py.loopx/quota.pykeeps the public facade wrapper and the remaining resolver/packet stages;quota.pydrops from 2701 lines (after #2963) to 2174 lines.The reward-memory status resolver stays behind a small facade wrapper in
loopx.quotaso the control-plane module does not gain a direct capability-layer dependency.Surfaces
loopx/control_plane/quota/should_run.py: owns_prepare_quota_should_run_item, work-lane preparation, blocked-priority fallback, automation prompt upgrade, todo identity/class helpers, and recovery allowance.loopx/quota.py: removes the moved private helpers; keeps publicbuild_quota_should_runcompatibility wrapper and the remaining route/payload stages.tests/control_plane/test_quota_paused_precedence.py: monkeypatch seam moves to the canonical bounded module wherebuild_agent_workspace_guardis now consumed.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.