fix(quota): clamp window slot spend against the voids that target it - #4384
huangruiteng merged 2 commits into
Conversation
build_usage_summary summed a signed per-run slot value, so a quota_slot_voided run subtracted its slots from every window that contained the void, whether or not the spend it targets was still in that window. A void whose spend had aged out drove quota_spend_slots_24h negative, and a void cancelled an unrelated spend it never referenced. The canonical spend ledger clamps the other way: it keys voids by the run they target and reduces each in-window spend by max(0, spent - voided), so an out-of-window void contributes nothing. Key spends and voids the same way the ledger does, via the shared load_quota_event_from_run loader, and clamp per spend key before summing. This also lets a run whose quota event is only reachable through json_path contribute, as it already does in the ledger. Signed-off-by: rootkiller6788 <17553215+rootkiller6788@user.noreply.gitee.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
build_usage_summary 过去把每个 run 的槽位值当成有符号数直接累加:quota_slot_voided 会在它所在的所有窗口里减掉自己的槽位,与它是否还指向窗口内的那笔花费无关。结果是两处可见错误——当被作废的花费已经超出 24h 窗口时,quota_spend_slots_24h 会被推成负数;同时作废会取消一笔它从未引用的花费。而真正承载配额口径的 canonical spend ledger(loopx/quota.py goal_quota_with_spend_ledger)是按"花费记在被记录的 run 上、作废记在它指向的 run 上"来分桶,再对每个 key 取 max(0, spent - voided) 的。也就是说,报道面与执行面的口径在这一刻并不一致,操作者看到的用量可能比实际执行的配额口径更悲观或更混乱。修复方向正确:让 usage summary 复用同一套分桶与 clamp。
改动思路
入口仍是 build_usage_summary(消费方是 loopx/control_plane/status/runtime_summaries.py:143 与 status/dashboard 渲染),权威输入是 run history 的 classification / generated_at / quota_event。原实现是"边遍历边把一个有符号整数加进 totals",这种形状无法表达"只对某个目标的 key 做 clamp",因此改成两步:遍历时用 quota_slot_contribution 把每个 run 归类成 ("spent"|"voided", key, slots) 的贡献项并按窗口收集,循环结束后用 _net_quota_spend_slots 按 (goal_id, key) 分桶并 clamp。key 的取法与 ledger 一致(花费取 event.run_generated_at or run.generated_at,作废取 voided_run_generated_at),并且改用共享的 load_quota_event_from_run,于是只能通过 json_path 读到 quota event 的 run 也能贡献真实槽位。整体是"把同一条业务规则对齐到既定 owner"的改动,而不是新增状态或协议。
具体改动
loopx/control_plane/quota/usage_summary.py(+64/-11):新增quota_slot_contribution、_net_quota_spend_slots,删除有符号的quota_spend_slots;build_usage_summary收集 24h/7d 两份贡献列表,循环后统一写入totals与各 goal 的quota_spend_slots_24h/7d。tests/control_plane/test_usage_summary.py(+72):新增test_aged_out_void_does_not_reduce_a_later_window与test_void_only_cancels_the_spend_it_targets,覆盖两条目标不变量。
关键代码讲解
quota_slot_contribution(loopx/control_plane/quota/usage_summary.py:27):把 run 归类成贡献项;load_quota_event_from_run返回空时直接返回None——这一条正是下面的阻塞点,因为 base 在这种情况下会回落到默认值 1。_net_quota_spend_slots(同文件 :69):按(goal_id, key)分桶后max(0, spent - voided),与loopx/quota.py:417的 ledger 归一口径一致;它只对窗口内存在花费记录的 key 做扣除,因此"窗口外被作废的花费"不会把窗口拉负。load_quota_event_from_run(loopx/control_plane/quota/slot_accounting.py:889):既有共享 loader,本次由 summary 复用,替代了原先只读run["quota_event"]的写法。
对主干的风险
阻塞项(P1):本次改动把"分类为 quota_slot_spent、但既没有内联 quota_event、其 json_path 指向的记录里也没有 event"的 run 从"贡献 1 个槽位"改成"贡献 0 个槽位",并且没有更新仓库里已经固定该默认值的公开 smoke。
- 证据(HEAD
c8f824a11):PYTHONPATH=<pr-worktree> python examples/usage-summary-smoke.py→AssertionError: {'runs_24h': 6, 'runs_7d': 7, 'quota_spend_slots_24h': 2, 'quota_spend_slots_7d': 2, ...}(该 fixture 期望 3/3),退出码 1。 - 对照(base
6bb413105,同一命令):usage-summary smoke ok,退出码 0。 - 该文件是
examples/**/*-smoke.py中被full-public-smokes.yml全量收录的公开 smoke(该 workflow 会在 push 到main且路径命中loopx/**、examples/**时触发,另有每夜扫描;但按设计不是 PR required check,所以本 PR 的 25 项检查全绿并不代表它通过了)。 - 我也跑了仓库自带的
loopx canary premerge --from-git-diff,gate 为passed:它只选取了受限的 core-control-plane / canary-runner 样本,未覆盖该 smoke,因此这道闸门同样看不到这个回归。
最小修复有两条,需要作者选择并显式披露:(a) 若采纳 ledger 口径(ledger 对无可解析 event 的 run 计 0,因此新行为才是口径对齐),则把 examples/usage-summary-smoke.py 的期望值改到 2/2,并在 PR 说明与发布说明里写明"无可解析 quota event 的花费不再计 1 个槽位";(b) 若这个保守默认值是有意的,则在 quota_slot_contribution 中恢复它。无论选哪条,都建议补一个"无可解析 event"的用例,避免默认值再次静默漂移。
次要项(P2):quota_slot_contribution 对缺失/非整数 slots 仍保留默认 1,而 ledger 用 _int_number(..., default=0) 并跳过 slots <= 0;另外 classification 为 spent、event_type 为 voided 的非法组合在两边判定不同。既然本次目标明确是"与 ledger 同口径",这两处残留差异会让口径再次分叉。
我的整体评价
修复的方向、切分与落点都合理:把有符号累加换成按 key 分桶 + clamp,复用共享 loader,规模与问题相称,两个新测试也确实覆盖了两条目标不变量。但本次 HEAD 会让仓库内一个既有公开 smoke 直接失败(base 通过、HEAD 失败),且默认值变化未在 PR 说明中披露;同时 netting 规则现在在 loopx/quota.py 与 usage_summary.py 两处各自维护,需要明确谁是 owner 以免下次再次漂移。请按上面的 (a)/(b) 任一条收口并更新/说明该 smoke 后,我再做一次 exact head 复审(届时请重新运行包含 examples/usage-summary-smoke.py 的检查,而不只是 PR 上的 required checks)。当前结论:REQUEST_CHANGES。
English verdict: REQUEST_CHANGES — 4384@c8f824a11f0dd6460176c59e742740478897c449. The per-key clamp and the reuse of load_quota_event_from_run correctly remove the negative/cross-cancelling window spend, but the head silently changes the default for a quota_slot_spent run with no resolvable quota event from 1 slot to 0, which makes the tracked public smoke examples/usage-summary-smoke.py fail at the head (24h/7d 2 vs expected 3, exit 1) while the same command passes at origin/main 6bb413105; gh pr checks and loopx canary premerge both stay green because neither selects that smoke. Repair: either update the smoke and disclose the default change, or keep the legacy default, and add a no-resolvable-event test. P2: the slots default (1 here vs 0 in the ledger) and the spent/voided event_type mismatch keep the two accounting rules from being fully aligned.
Review follow-up on the previous commit. It left two things open. The usage summary kept its own reading of the spend rule, which had drifted from the enforcement-side ledger: it decided spent versus voided from the run classification rather than the quota event's event_type, and it defaulted a missing or unreadable slot count to one where the ledger defaults to zero. The rule now lives in one place beside the shared quota-event loader -- slot_accounting.quota_slot_contribution and net_quota_slot_spend -- and both goal_quota_with_spend_ledger and build_usage_summary call it. The ledger keeps its behaviour and stays the enforcement owner; the summary is a read-model consumer of the same two functions, so the reported number cannot drift from the enforced one again. A parity test asserts equality per scenario, covering an aged-out void, a void of another run, an unreadable event, non-positive slots, and a classification that disagrees with event_type; two of those are red against the pre-fix implementation. Two fixtures in tracked public smokes were not producer-faithful, and correcting them changes reported numbers, so it is disclosed here: - examples/usage-summary-smoke.py built a quota_slot_spent run with no quota_event. The spend commit always stamps event_type and slots, so the ledger records no slot for such a run. The summary now agrees instead of inventing one: expected totals move from 3 to 2 in both windows, and the per-goal split is asserted explicitly. - status-runtime-summaries-readmodel-smoke.py omitted event_type from its quota_event. The fixture now carries what the producer writes, so its expectation of one slot is unchanged. Net effect for callers: a spend whose quota_event cannot be read contributes 0 slots instead of 1, a non-positive slots value contributes 0 instead of itself, and dispatch follows the quota event's event_type. The run still appears in the event ledger, so it is unquantified rather than hidden. Verified: tests/control_plane/test_usage_summary.py and test_quota_rolling_window_projection.py pass, with the four slot tests red at the parent commit and green here. examples/usage-summary-smoke.py and every other public smoke that consumes usage_summary pass in a clean worktree. A same-suite run over the quota-adjacent CLI tests shows the same failures before and after, all pre-existing Windows environment. Signed-off-by: rootkiller6788 <17553215+rootkiller6788@user.noreply.gitee.com>
|
Both blockers are addressed in a follow-up commit — 718dfbd sits on top of the reviewed c8f824a, so nothing you P1 — I took option (a), and found a second instance you did not flag. Your diagnosis reproduces exactly. The spend The same class of divergence existed in a second tracked smoke: P2 — the rule now has one owner. Rather than bringing a second copy closer to the ledger, I moved the rule next to the Guard against re-drift. test_window_slot_spend_matches_the_enforcement_ledger asserts Verification (clean worktree at the new head, same command you ran):
|
huangruiteng
left a comment
There was a problem hiding this comment.
动机
build_usage_summary 过去把每个 run 的槽位值当成有符号整数直接累加:quota_slot_voided 会在它出现的每个窗口里减掉自己的槽位,与它指向的那笔花费是否还在窗口内无关。上一轮 review 已经把这个问题点出来了,head c8f824a11 也承认方向正确,但留下了两条待收口:一是"分类为 quota_slot_spent 但读不到 quota event"的 run 由 1 槽变成 0 槽,examples/usage-summary-smoke.py 在 head 上直接失败(base 通过),而 PR 说明没有披露这个默认值变化;二是 netting 规则仍然在 loopx/quota.py 与 usage_summary.py 各存一份,两边的 slots 默认值(1 vs 0)和 spent/voided 优先级已经不一致。
当前 head 718dfbd1bf33107af332cb6cb3a4347c79f3ad05 把这两条都收了:规则收敛到一个 owner,并把默认值变化写进文档、smoke 与用例。
改动思路
权威口径只有一个:loopx/quota.py::goal_quota_with_spend_ledger 按"花费记在它自己被记录的 run 上、作废记在它指向的 run 上",再对每个 key 取 max(0, spent - voided)。本次把这条规则从两处复制品提取成 slot_accounting.py 里的两个纯函数——quota_slot_contribution(把一条 run 归类成 ("spent"|"voided", bucket, slots),不可用时返回 None)与 net_quota_slot_spend(按 bucket 做 clamp)——然后让执行面和报道面都只做调用方。loopx/quota.py 的内联循环逐字搬进共享函数,因此执行面语义不变;usage_summary.py 先遍历窗口收集贡献项、循环结束后统一分桶 clamp,报道面因此和执行面同口径。这不是新增状态或协议,而是把同一条业务规则对齐到既定 owner。
具体改动
loopx/control_plane/quota/slot_accounting.py(+54):新增quota_slot_contribution与net_quota_slot_spend,承载原先散落两处的分类与 clamp 规则。loopx/control_plane/quota/usage_summary.py(+52/-?):删除有符号的quota_spend_slots,改由_goal_quota_spend_slots按(goal_id, run key)聚合共享规则的结果,并在遍历结束后写入totals与各 goal 的quota_spend_slots_24h/7d。loopx/quota.py(-39):执行面改为调用共享函数,spent_slots仍由同一组事件推导。tests/control_plane/test_usage_summary.py(+176):新增 aged-out void、无可解析 event、void 只取消它指向的花费等用例,并用test_window_slot_spend_matches_the_enforcement_ledger直接对执行面做 7 组 parity 断言。docs/status-data-contract.md、examples/usage-summary-smoke.py、examples/control_plane/status-runtime-summaries-readmodel-smoke.py:披露新默认值并把 smoke 期望改到 2/2。
关键代码讲解
quota_slot_contribution(slot_accounting.py:916):内容就是 base 上goal_quota_with_spend_ledger的内联体——load_quota_event_from_run把关、slots = max(0, _int_number(..., default=0))、slots <= 0跳过、event_type决定 spent/voided、key 分别是run_generated_at or generated_at与voided_run_generated_at。执行面因此逐字等价,报道面同时获得"读不到 event 就不计槽"的诚实行为。net_quota_slot_spend(同文件 :944):按 bucket clamp,voided只作用于同名 key,且空的 spent bucket 根本不出现在结果里,所以窗口不会被推负,作废也无法取消无关花费。_goal_quota_spend_slots(usage_summary.py:28):bucket 里带goal_id,避免一个 goal 的作废抵消另一个 goal 的花费;这正是原先有符号累加做不到的事。
对主干的风险
结论:本 head 没有阻塞项,可以进合并队列。已独立验证的四点:
- 执行面未变(关键反事实):我写了 19 个场景的探针(无 event、无 event_type、aged-out void、作废指向未知 key、classification 与 event_type 冲突、slots 缺失/0/负数/浮点/布尔、
json_path解析出的 event、重复 key、窗口外、naive 时间戳),分别在 based9a86fb58与 head 上运行goal_quota_with_spend_ledger,输出逐字节相同。也就是说这次"去重"没有动配额执行口径。 - 报道面与执行面对齐:
pytest tests/control_plane/test_usage_summary.py -q→ 39 passed(head718dfbd1b),含 7 组与执行面直接比对的 parity 用例;test_usage_summary.py + test_quota_slot_accounting.py→ 77 passed。 - 默认值变化已披露:
examples/usage-summary-smoke.py在 head 断言 2/2(注释说明"读不到 quota event 的花费不再计 1 槽"),base 上同一命令仍以旧期望 3/3 通过;docs/status-data-contract.md明确写出规则、无 event 情形与"窗口外被作废"情形。这与执行面本来就计 0 的行为一致,属于口径拉齐而非收紧。 - 红线检查:
gh pr checks 4384只有build(Frontstage Pages / Validate public-safe frontstage bundle)为红,报Missing expected file: .../long-horizon-control/position-en.svg/index.html;同一条 workflow 在 main 上 run 34962489756、34960575404 同样失败,属继承性红。本地tests/control_plane里的[sqlite]用例在 head 与 base 上以同样的 Node 25.5.0 / SQLite 未资格化信息失败,与本次改动无关。
次要项(P3,非阻塞):文档写的是"与 spend ledger 同一规则",这作为规则陈述是对的,但两个面的输入集并不相同——loopx/history.py:376 给报道面的是受 limit 截断的 recent_runs,而 loopx/history.py:330 给执行面的是该 goal 的完整 run 列表。建议在该句补一句"基于采样到的 run 历史",避免把采样差异误读成口径不一致。
我的整体评价
APPROVE。上一轮的两条阻塞都已按建议收口:规则现在只有 slot_accounting.py 一个 owner,两边的 slots 默认值与 spent/voided 优先级都统一到同一段代码,默认值变化与 smoke 期望同步披露,并补上了"无可解析 event"的正反用例。更关键的是,我独立验证了执行面在 base 与 head 上对 19 个边界场景输出完全一致,因此这次抽取是行为保持的,报道面的变化是唯一的有意 delta,且已被文档和 smoke 固定。规模与问题相称:生产代码净增很小,因为删掉了重复实现。剩余只有一条 P3 文档精度建议,不影响合并。
English verdict: APPROVE — reviewed exact head 718dfbd. The signed per-run accumulator is replaced by the ledger's bucket-and-clamp rule with a single owner in slot_accounting.py; the enforcement face is byte-identical across a 19-scenario base-vs-head probe, the summary now equals goal_quota_with_spend_ledger(...)['spent_slots'] (39 focused tests plus 77 across the two files), and the reported default change (a spend whose quota_event cannot be read counts 0 instead of 1) is disclosed in docs/status-data-contract.md and pinned by the updated public smoke, which passes at the head and carried the old expectation at base. The only red check is the Frontstage Pages bundle smoke, which fails identically on origin/main (runs 34962489756, 34960575404) and is therefore inherited. One P3: the documentation should note that the summary is computed over the sampled run history while enforcement reads the goal's full run list.
build_usage_summary summed a signed per-run slot value, so a quota_slot_voided run subtracted its slots from every window that contained the void, whether or not the spend it targets was still in that window. A void whose spend had aged out drove quota_spend_slots_24h negative, and a void cancelled an unrelated spend it never referenced. The canonical spend ledger clamps the other way: it keys voids by the run they target and reduces each in-window spend by max(0, spent - voided), so an out-of-window void contributes nothing.
Key spends and voids the same way the ledger does, via the shared load_quota_event_from_run loader, and clamp per spend key before summing. This also lets a run whose quota event is only reachable through json_path contribute, as it already does in the ledger.
Summary
Issue Or Task
Validation
unitnot_runSee validation disclosure guidance.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Core control-plane hardening
Long-horizon benchmark evidence
Operator surface and IM integration
Shared Goal Authority and cross-host coordination
Architecture and research incubator
Target base branch:
Direction tracker or promotion unit:
Shared-authority RFC fixture impact
Boundary Checklist
.loopx/,.codex/goals/, and liveACTIVE_GOAL_STATE.md).none.Signed-off-bytrailer (git commit -s).