test(steward): point the machine-defaults guard at the current owner - #4520
Conversation
The guard asserts that the modules which resolve the steward's endpoint, model and effort are the ones that pass machine_defaults, but it still named `loopx/chat_server.py`. That module no longer holds such a call -- the manager readability projection moved to `loopx/chat_manager_context.py` -- so the test failed on every commit, including untouched ones, and a permanently red invariant check is exactly what hides a real regression later. The expected set now names the module that actually carries the call. The undocumented-call-site assertion, the call-site floor, and all other assertions are unchanged, so the check keeps its teeth. Verified: tests/test_manager_channel_binding.py 25 passed (was 1 failed), and a combined sweep of the steward surfaces touched today -- Lark topic connections and runtime, manager channel binding, manager context handoff, the Lark API contract, the manager guidance contract, the team-plan preview contract and the SSH evidence suite -- is 212 passed with no remaining failure. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Reviewed exact head on codex/post-merge-sweep (test-only, 1 file).
动机
test_every_production_steward_caller_passes_the_machine_defaults 断言"解析管家端点/模型/档位的模块必须传 machine_defaults",但它点名了 loopx/chat_server.py——该模块已不再持有这类调用(相关可读性投影搬到了 loopx/chat_manager_context.py)。于是这条不变量检查在每一次提交上都是红的(干净 origin/main 同样复现),而长期变红的检查正是下一次真实回归会被掩盖的地方。
改动思路
只把期望集合里的过期路径换成当前承载该调用的模块;"未披露的调用点"断言、调用点数量下限等其它断言一律不动,检查力度不降。
具体改动
tests/test_manager_channel_binding.py:期望文件集合 loopx/chat_server.py → loopx/chat_manager_context.py。
对主干的风险
仅测试。风险是替换后的路径本身不对——已由本轮组合回归(212 passed)与该测试自带的"逐调用点扫描"断言共同覆盖。
我的整体评价
无阻断性问题,建议合并:它把一条长期红的不变量检查恢复成有效护栏,属维护性修复而非放松门禁。验证:该文件 25 passed(此前 1 failed);本轮对今日动过的全部管家表面做组合回归为 212 passed / 0 failed(Lark 话题连接与运行时、manager channel binding、manager context handoff、Lark API 契约、管家指引契约、团队预览契约、SSH 证据)。诚实说明:未跑 pr-review --check-result 机器校验、未等远端 CI,已在此与 PR 中披露。
English verdict: APPROVE - repairs a stale invariant guard by naming the module that now carries the steward resolver call, turning a check that failed on every commit (including untouched ones) back into an effective one. Test-only, no assertion weakened; the file is 25 passed and a combined sweep of today's steward surfaces is 212 passed / 0 failed. Machine --check-result skipped and disclosed.
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
审查对象:4520@b3f3df63bfb344d890a22711457d8060efcf07ce(已合并,合并后审计)。merge base 3acd07697f05d670b7c0f97f2e54a7a22ff71a41,1 个文件 +1/-1。
动机
test_every_production_steward_caller_passes_the_machine_defaults 断言"resolver 的调用方必须传 machine_defaults",但它把 loopx/chat_server.py 列为应有调用方的模块——该模块早已不持有这类调用(可读性投影搬到了 loopx/chat_manager_context.py)。后果不是"少查一处",而是这条不变量在每个 commit 上都红(包括未改动的干净检出),红的检查会掩盖下一次真实回归。我在评审 #4514 时就亲手复现过这 1 failed。
改动思路
只换一个期望路径,不动其它任何断言:未文档化调用点的扫描、调用点数量下限(>=14)、以及"每个生产调用必须传 machine_defaults"的判定全部保留。
具体改动
tests/test_manager_channel_binding.py 的期望模块集合:loopx/chat_server.py → loopx/chat_manager_context.py。
我做的独立核对(不止跑测试):
- 修复有效:
pytest tests/test_manager_channel_binding.py→ 25 passed(我在 merge base 上跑同一测试是 1 failed)。 - 替换路径确实是对的:我另写了一遍 AST 扫描(覆盖七个 resolver 名),生产侧调用点为
chat_manager.py:259/262/343/369、chat_manager_context.py:435、chat_runtime.py:400/425,全部传machine_defaults;chat_server.py里已无 resolver 调用(只有manager_capabilities_projection,不在该名单内)——与测试的发现一致。 - 没有削弱:读 diff 后的测试体,
undocumented == []的全量扫描与>= 14下限原样保留。 - 该 PR 只碰测试文件,无生产代码、无契约变更。
对主干的风险
测试专用、一行改动,风险仅在"替换的路径本身写错",我已用独立扫描交叉验证。一处 P3(非阻塞):这条 guard 的发现是动态的、期望却是手写清单,所以 resolver 调用一旦搬家,检查就会一直红到有人来改测试(正是本次情形)。建议把期望集合改为从"导入 resolver 的模块"推导,或至少在断言失败信息里同时打印发现到的模块清单,让下次搬家是一行修复而不是一次考古。
我的整体评价
修的是"永久红的守卫"这一具体危害:一行让不变量恢复意义,且没有把检查变空。我除了跑测试,还独立复算了它依赖的调用点事实,确认替换路径正确、原有约束未削弱。剩下的只是期望清单的维护形态问题(P3)。作为已合并精确 head 的合并后审计,证据支持通过。
English verdict: APPROVE (exact head b3f3df6)
Motivation
test_every_production_steward_caller_passes_the_machine_defaultsasserts that the modules which resolve the steward's endpoint, model and effort are the ones that passmachine_defaults. It still namedloopx/chat_server.py, which no longer holds such a call — the manager readability projection moved toloopx/chat_manager_context.py. The assertion therefore failed on every commit, including untouched ones (it reproduces on a cleanorigin/maincheckout). A permanently red invariant check is what hides a real regression the next time one appears.Change
One expected path swapped:
loopx/chat_server.py→loopx/chat_manager_context.py. The undocumented-call-site assertion, the call-site floor, and every other assertion are unchanged, so the guard keeps its teeth.Validation
pytest tests/test_manager_channel_binding.py→ 25 passed (was 1 failed).Risk
Test-only. The risk is that the replacement path is itself wrong; it is verified by the sweep above and by the guard's own undocumented-call-site assertion, which still scans every production call.