perf(status): reuse run history scan for contract checks - #4632
Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
82af821 to
bb8c20d
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Reviewed exact head: bb8c20d63913a580997f8755465025f3a0ef1216 (codex/optimize-core-hotpath-6).
动机
一次 loopx status 需要同一份 run history 服务两个消费者:状态投影(只关心当前 registry 的成员)和 contract 检查(允许看到 runtime-only 的 legacy goal)。改动前两者各自跑一次索引扫描,也就是每次 status 调用都把每个 goal 的 index.jsonl 读两遍,成本随 goal 数量和索引大小增长,而 status 是心跳与 dashboard 都会频繁触发的高频路径。
我在同一个合成 fixture(2 个 registry goal + 1 个 runtime-only goal)上,用真实 collect_status 入口对比了 main 与 head:main 触发 5 次 load_index(g-a、g-b、g-a、g-b、runtime-only),head 只触发 3 次(每个 goal 一次)。同时两次运行的 run_history 载荷在归一化临时路径后逐字节相同,contract 的 run-history goals=3 runs=4 行也相同——即「少读一遍」这个目标达成,且两个作用域都没有被改变。
改动思路
入口是 loopx status(loopx/status.py → loopx/control_plane/status/collection.py:collect_status)。权威输入是 registry(成员关系与激活状态)与每个 goal 的运行时 run index;决策边界仍由 loopx/history.py 拥有(扫描与审计),loopx/contract.py 只消费一个带类型的 RunHistoryAudit,不再自己去扫一遍。
具体路径:collect_status_history 只做一次 collect_history(include_runtime_goals=True),据此构造 RunHistoryAudit(registry/runtime 路径、goal/activation 过滤、goal_count/run_count,以及每个 goal 的 index_path/raw_index_records/unique_runs/legacy_runtime_goal),然后:状态投影在「非全局 registry 且无 goal/activation 过滤」时用 history_for_registry_members 收窄回 registry 成员;contract 检查通过 matches() 校验作用域后直接复用这份审计。
复用上,我没有把「再扫一遍」当成不可替代:审计里的每个字段都是 collect_history 已经产出的既有事实(registry_member、unique_runs、raw_index_records),新代码没有引入第二个 reader,也没有重新从 registry 推导成员关系。被否掉的更小方案是「把状态历史对象直接传给 contract」——不可行,因为两者合法地需要不同作用域(状态必须只含 registry 成员,contract 必须看到 runtime-only goal),且 contract 需要的是每个索引的审计事实而不是展示窗口;因此抽出一个与 limit/lane 无关的类型化审计才是最小形态。
具体改动
7 个文件、+508/-30:4 个生产文件(loopx/history.py +160、loopx/contract.py +55/-13、status/collection.py +5/-3、loopx/status.py ±2)与 3 个测试侧文件(新增 tests/control_plane/test_status_history_reuse.py +261、readmodel smoke、wiring 测试)。StatusCollectionContext 的字段由 collect_history 改名为 collect_status_history,构造点只有 loopx/status.py、一个测试和一个 smoke,改动是收敛的。
关键代码讲解
RunHistoryAudit.matches(loopx/history.py:436附近):把 registry/runtime 路径resolve()后比较,并归一化 activation 过滤,所以「别的 runtime 的审计」不会被静默复用,而是直接报错——这是本次改动里唯一新增的强约束。history_for_registry_members(loopx/history.py:475):按同一次扫描产出的registry_member过滤,并用与collect_history相同的_chronology_key/merge重算展示窗口;因为它是先取每个 goal 的前 limit 条再 merge 再截断,结果与「直接扫 registry 成员」等价(我用整份载荷对比验证过)。collect_status_history(loopx/history.py:515):无论状态侧作用域如何都用include_runtime_goals=True扫一次,再决定是否收窄投影;这让「contract 能看到 runtime-only goal」这一既有语义在改为复用后仍然成立。check_contract(loopx/contract.py:882):无审计时保留自扫回退(独立 CLI/调用方行为不变),有审计时校验作用域后只用审计里的计数与索引路径产生原有的 check/warning 文本。
对主干的风险
我跑了:新增/接线测试 15 passed;tests/test_contract_*.py、status-server 与 tests/control_plane/test_contract_health.py 等 75 + 32 passed;examples/control_plane/status-collection-readmodel-smoke.py ok。头部 CI 中 build、kernel-static-checks、dashboard-acceptance、node 兼容、stage2c(e2e/mutants/installed)、windows-powershell、postgresql-authority、dependency-review 均 SUCCESS,review 时仍有 3 个 test-shard 在跑(无失败),merge_state=BEHIND 属于待更新分支而非冲突。
最强回归场景不是性能而是「作用域静默变化」:如果新投影与旧扫描出现分歧,runtime-only goal 可能漏进 status,或 goal_count/run_count/展示窗口与旧行为不一致,而调用方不会收到任何错误。这一点由三层证据挡住:作者自己的 parity 测试直接把新投影与「直接调用 collect_history(include_runtime_goals=False)」逐字段比较(limit 0/1/2 且带 agent lane),第二个测试覆盖 global registry / goal 过滤 / activation 过滤三种非项目作用域,我的探针则对比了整份 run_history 载荷。我也确认了审计字段与 limit、agent lane 无关(limit 只影响展示窗口,agent_lane_id 只影响 latest_runs),所以状态侧用 20、contract 侧用 2 的不同 limit 不会让复用出错。
一条非阻塞 P2:「registry 成员」这条作用域规则现在有两个表达式——discover_goal_ids(include_runtime_goals=False) 仍按发现期过滤,history_for_registry_members 又在投影期过滤一次。今天两者被 parity 测试钉住且都在同一个模块内,但未来若新增同类作用域开关,或 run_count 的定义变化,两处可能各自漂移。建议把 include_runtime_goals=False 这条路径也走 history_for_registry_members,让规则只有一个归属;否则至少在该 helper 的 docstring 里声明它就是这条规则的唯一表达,并同步删掉发现期的变体。
另一处我确认可以接受的行为:审计作用域不匹配时 check_contract 抛错而不是回退自扫。这是内部边界上的 fail-fast,且当前唯一的生产调用点(collect_status)传的就是同一次扫描的审计,所以不会误伤;独立调用方本来就不传审计,仍走自扫。
我的整体评价
结论 APPROVE。这是一个把「同一份 run history 读两次」收敛为「读一次 + 类型化审计复用」的真实热点改进:我用计数探针复现了 5→3 的读取下降,用整份载荷对比证明了状态投影与 contract 输出都没变,用作者的 parity 测试与额外作用域测试确认了两个作用域规则仍各自成立,并且新增的强约束(审计作用域不匹配即报错)方向正确。它没有引入新的持久化状态、CLI 表面或权威,回退成本是四个文件。
唯一的 P2 是这条作用域规则出现了两处表达,属于维护性问题而非当前行为缺陷;不影响本次判断。按仓库规则,这是运行时代码改动,因此本评审只给出 exact-head 结论,合并就绪度(CI 收敛、BEHIND 更新)属于另一道关卡。
English verdict: APPROVE - exact head bb8c20d; the status path now scans each run index once and shares a typed audit with the contract check, verified independently by a main-vs-head probe (load_index calls 5 -> 3, identical full run_history payload, identical contract run-history line) plus 15 new/wiring tests, 75 + 32 adjacent tests, the readmodel smoke, and green CI. One non-blocking P2: the registry-members scope now has two expressions (discover_goal_ids and history_for_registry_members) and should be collapsed to one owner.
Summary
Performance
The same local profile used 30 Goals, 500 runs per Goal, and 3 status calls:
load_indexcalls: 180 -> 90 (50% reduction)The read-count assertion is deterministic; elapsed time is supporting evidence.
Validation
9765 passed, 37 skipped, 106 subtests passedloopx canary premerge --from-git-diff: 19/19 passedloopx statusandloopx quota should-runreadsRefactor Scope
The bounded future-facing pass introduced an immutable history-audit contract at the existing history owner. No cache, compatibility wrapper, or speculative provider abstraction was added.