fix(core): keep agent usage rounds cumulative - #11200
Conversation
|
Thanks for the PR! Template looks good ✓ — all sections present, including the Chinese translation. Problem: the behaviour itself is real and reproducible. What I could not substantiate is the harm the description claims: "consumers using the run and round as an idempotency key could silently discard the second segment's token usage." I checked all eight Direction: aligned in principle — making the usage event agree with the cumulative round count the stats already report is defensible, and Size: core path ( Approach: minimal and focused — one line plus one regression test, no drive-by edits or scope creep. The open design question is the one that line raises: Risk: no elevated risk signals — neither changed file matches the revert-correlated path list. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 各小节齐全,含中文翻译。 问题: 行为本身真实且可复现。 我无法证实的是描述里声称的危害:"以 run 和 round 作为幂等键的消费者会静默丢弃第二段的 token 用量"。我检查了全部八个 方向: 原则上对齐——让 usage 事件与 stats 已经上报的累计轮次一致是站得住脚的,且 规模: 核心路径( 方案: 精简且聚焦——一行改动加一个回归测试,无顺手改动或范围蔓延。待定的设计问题正是这一行引出的: 风险: 无升级风险信号——两个改动文件都不匹配与 revert 相关的路径列表。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewMy independent proposal (written from the title and "Why it's needed", before reading the diff): Comparing that to the diff, the PR takes the third path, and that is my one substantive concern.
The sharpest instance is the transcript. To be clear about severity: nothing breaks today, precisely because no consumer reads the field. I checked all eight The stated rationale doesn't match the code. "Consumers using the run and round as an idempotency key could silently discard the second segment's token usage" — there is no such consumer, and Minor: What's good: the diff is genuinely minimal — one production line, no drive-by edits. The test is real and non-vacuous: I confirmed the mock's Test evidenceThis is an unattended CI run, so per the gate rules I did not build or execute any PR-derived code — the evidence below is the PR's own CI, read through the API for the reviewed commit. Nothing is red. The three checks that would actually exercise this change — unit tests, lint/typecheck, and integration — were still running when I fetched, so the new test's pass/fail result is not verified yet; I confirmed its logic statically instead. No failing job, so there is no log excerpt to quote.
Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 58 further checks are Sandboxed verification would settle the part the unit test can't: 中文说明我的独立方案(只看标题和"为什么需要"、未读 diff 时写下): 与 diff 对比,本 PR 走了第三条路,这是我唯一的实质性顾虑。
最尖锐的例子是 transcript。 关于严重程度需要说清楚:今天不会出问题,恰恰因为没有消费者读这个字段。我检查了全部八个 描述中的理由与代码不符。"以 run 和 round 作为幂等键的消费者会静默丢弃第二段的 token 用量"——不存在这样的消费者,且 次要: 做得好的地方: diff 确实精简——一行生产代码,无顺手改动。测试真实且非空转:我确认了 mock 的 测试证据: 本次为无人值守 CI 运行,按门禁规则我没有构建或执行任何 PR 派生代码——以上证据来自 API 读取的该 PR 自身 CI(针对被审 commit)。 没有红灯。真正能覆盖此改动的三项检查——单元测试、lint/类型检查、集成测试——在我拉取时仍在运行,因此新测试的通过/失败结果尚未验证;我改为静态确认了它的逻辑。无失败作业,故无日志摘录可引。 表中 沙箱验证可以解决单元测试覆盖不到的部分: — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the change itself is safe, minimal and properly tested, but it makes Stepping back: my independent proposal was to decide what I want to be precise about why this is a defer and not a request-changes. I traced the blast radius and it is genuinely tiny: all eight But the same fact cuts the other way: if no consumer reads the field, the change buys nothing observable today, and what it does leave behind is a divergence a future reader will hit first. The concrete case is the transcript — On the justification: the description rests on a consumer that keys on Worth saying plainly: the author has 40 open PRs, six of them today, several in this same corner of CI hasn't settled either: unit tests, lint/typecheck and integration were all still running when I fetched, so the new test's actual result is unverified. I confirmed its logic statically — it's non-vacuous and would have failed before the change — but I'm not approving against a result that doesn't exist yet, and this comment deliberately carries no deferred-approval instruction, because the verdict is defer rather than approve. ⏸️ Deferring — not approving, not requesting changes. The question for a maintainer is narrow: should round numbering on the agent event bus be cumulative across continuation segments, or segment-local? If cumulative, this PR should extend to the five sibling events (and the transcript's I could not resolve an accountable owner to hand this to: no maintainer handle is configured for this run, the PR carries no labels so no area owner matched, and there are no human reviews yet. Posting without an @mention rather than guessing a login — a maintainer passing through should pick this up. 中文说明Confidence: 3/5 —— 改动本身安全、精简、测试得当,但它让 退一步看:我的独立方案是先确定 我想说清楚为什么这是 defer 而不是 request-changes。我追踪了影响面,它确实极小:全部八个 但同一事实也指向反面:如果没有消费者读这个字段,该改动今天就买不到任何可观测收益,而它留下的、未来读者最先撞上的,是一处分歧。具体例子是 transcript—— 关于理由:描述依赖一个以 需要坦白说明:作者有 40 个开放 PR,其中六个是今天的,且有几个就在 CI 也尚未落定:我拉取时单元测试、lint/类型检查、集成测试都还在运行,因此新测试的实际结果未经验证。我静态确认了它的逻辑——非空转,且改动前会失败——但我不会针对一个尚不存在的结果 approve;本评论也刻意不携带任何延迟审批指令,因为结论是 defer 而非 approve。 ⏸️ 暂缓——不 approve,也不 request changes。 留给维护者的问题很窄:Agent 事件总线上的轮次编号,跨续跑分段应该是累计的,还是分段局部的?如果是累计,本 PR 应扩展到另外五个兄弟事件(以及 transcript 的 我无法解析出可交接的负责人:本次运行未配置维护者 handle,PR 没有标签因而没有匹配到领域负责人,也还没有人类 review。我选择不 @ 任何人,而不是猜一个 login——路过的维护者请接手。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Converted to draft, not closed. The mesh work is being delivered as one PR (#11206), so this change is folded into Tracking: #11206 (umbrella), acceptance criteria in |
Records the runtime seams steps 2 and 3 reuse (executionAllowedTools, the AST read-only shell classifier, tsx for two-process tests), the storage protocol decisions, the named tests each step needs, the dependencies on #11200/#11202/#11204, and two design conflicts owed to the owner: MCP under the read-only ceiling, and batching the v1 schema.
|
Retargeted to base |
a251498
into
codex/multi-agent-mesh-foundation
What this PR does
Keeps background-agent usage event round numbers cumulative when one logical run enters a second reasoning-loop segment to consume late external input.
Why it's needed
The runtime already preserves cumulative execution statistics across the final-drain segment, but usage events still restarted at round 1. Consumers using the run and round as an idempotency key could silently discard the second segment's token usage.
Reviewer Test Plan
How to verify
Run the named headless-agent test and confirm that two segments in one logical execution emit usage rounds 1 and 2, while the existing continuation and statistics tests remain green.
Evidence (Before & After)
Before: the regression test observed
[1, 1]. After: it observes[1, 2].Tested on
Environment (optional)
Named Vitest file with the existing workspace dependencies.
Risk & Scope
Linked Issues
Follow-up for the multi-agent shared-thread design.
中文说明
本 PR 做什么
当同一个后台 Agent 逻辑 run 为消费迟到的外部输入而进入第二个推理循环段时,保持 usage 事件的轮次编号连续递增。
为什么需要
运行时已经在最终 drain 段保留累计执行统计,但 usage 事件仍从第 1 轮重新编号。以 run 和 round 作为幂等键的消费者会静默丢弃第二段的 token 用量。
Reviewer Test Plan
如何验证
运行命名的 headless-agent 测试,确认同一逻辑执行的两个分段依次发出第 1、2 轮 usage,同时已有续跑与统计测试仍通过。
证据(修复前后)
修复前:回归测试观测为
[1, 1]。修复后:观测为[1, 2]。测试平台
环境(可选)
使用现有 workspace 依赖运行命名 Vitest 文件。
风险与范围
关联 Issue
multi-agent shared-thread 设计的后续修正。