feat(metrics): per-agent toolCalls in agents[], from the run's own walk - #333
Conversation
Every `agents[]` entry in a `stage: "final"` row carried what a worker COST and nothing about how much it DID, so "did workers make fewer calls after change X" could only be answered by grouping a 20 MB trace by `parent_tool_use_id` — the artifact that rotates, against a row kept forever. `StartupProbe` now counts each tool call to its actor as well as to the run, keyed by `owner_key` (the same key `token_attribution` groups spend by), and `final_record` reads that partition onto the rows it emits. One writer (`record_call`) moves both counters, so the parts account for the whole by construction rather than by assertion; one key derivation serves both walks, so per-agent dollars and per-agent calls cannot describe different populations. `backfill-metrics` recounts too — it rebuilds `agents` wholesale and would otherwise strip the field. No per-agent `startupToolCalls`: at run level it counts calls before the run's first ORG mutation, and a worker that reworks a diff and hands it back never issues one — the field would mean orientation overhead on some rows and "never got productive" on others. Closes #330 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The same job on the pull_request run passed at the same sha; the push-event run failed downloading rainlanguage/rainix's tarball, not on a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # pr-review-report-rs/src/main.rs
|
Merged Resolved by taking both parameters and keeping main’s reroute, which is what Kept this PR’s comment on why there is deliberately no per-agent One test then failed to compile: it called clippy |
Every
agents[]entry in astage: "final"metrics row now carriestoolCallsbeside its
usd/messages/cacheRead/cacheWrite, so "did workers makefewer calls after change X" is answerable from
metrics/runs.jsonlalone.Before this, the run-level
toolCallswas a single total and the only way tosplit it per worker was
jqoverparent_tool_use_idin the raw trace — theartifact that rotates, against a row that is kept forever. Getting the "23
poll-shaped calls" figure in #330 needed a 20 MB trace that only still existed
because the run was 21 hours old.
What it does
StartupProbe— the walk that already produces the run-leveltoolCalls— nowcounts each call to its actor as well as to the run, keyed by
parent_tool_use_id(
__main__for the main loop), which is exactly howtoken_attributionalreadygroups spend.
final_recordreads that map onto theagents[]rows it emits.Three things make it hard to drift:
StartupProbe::record_callis the only place either countermoves, so the partition IS the total by construction rather than by two
+= 1sthat a future guard could be added to one of.
owner_keyis the single derivation of "whose event is this",used by both the spend walk and the call walk. If they ever disagreed, a row's
per-agent
usdand per-agenttoolCallswould describe different populationswhile both looked fine.
backfill-metricsrebuildsagentswholesale, soa backfill that did not recount would strip the field off rows that had it.
startupToolCallsper agent: left out, deliberatelyThe issue asks for a decision on the record. It is not well-defined per
worker, on two counts the code settles:
firstMutationIndex, andis_mutation_toolrecognises the run's org mutations —gh pr create,git push,git commit, or the vetter'srecord_verdict. A rework workerthat edits a diff and hands it back to the main loop never issues one, so its
per-worker analogue would read "every call was startup" for a worker that did
nothing but work.
rows. So the field would mean "orientation overhead" on some rows and "this
worker never got productive" on others, decided by which row you are looking
at — the number whose meaning differs by row that agents[] carries no toolCalls, so per-worker call counts need the trace the sweep deletes #330 says to leave out.
One honest field, per the issue.
Measured on real traces
The two post-change producer runs from the issue's table, run through the new
binary, against an independent
jqgrouping of the same traces:toolCallsagents[].toolCallsPer-worker, per-owner counts match the
jqgrouping exactly, not just the total.The question the issue could not answer is now a field read: mean tool calls per
rework worker was 72.2 on 110404Z and 42.7 on 050103Z.
On the base binary the same command emits
"toolCalls": nullfor every one ofthose six agent rows.
Closes #330
QA
usage_probe_tests::per_agent_tool_calls_partition_the_runs_own_total(per-label counts AND the sum identity),usage_probe_tests::spend_and_calls_are_keyed_to_the_same_actor(the two walks agree on the owner for null / absent /""/ a real id),usage_probe_tests::the_backfill_recounts_per_agent_tool_calls— each fails on base by construction:RunMetrics::tool_calls_by_owner,owner_keyandagent_row's second parameter do not exist there, and the observable base behaviour isagents[].toolCalls == nullon every row of a real trace (pr-review-report run-metrics runs/20260817T110404Z.jsonlat 307a598, output in the body above). Baseline suite green at 307a598: 1412 passed, 0 failed; with this change 1415 passed, 0 failed.StartupProbe::record_call→ drop the per-owner increment, keep the run total → kills all three;final_recordagents map → look every agent up onMAIN_LOOP_OWNER(total preserved, partition destroyed) → killsper_agent_tool_calls_partition_the_runs_own_total;token_attribution→ re-inline its own owner derivation with a different default instead ofowner_key→ killsspend_and_calls_are_keyed_to_the_same_actor(+ 3 pre-existing label tests);agent_row→ drop thetoolCallsfield → killsthe_backfill_recounts_per_agent_tool_calls;backfill_row→ pass0instead of the recount → killsthe_backfill_recounts_per_agent_tool_calls. 5 applied, 5 killed, 0 survivors.jqoverparent_tool_use_idon two real traces (~/issue-pr-cron/runs/20260817T110404Z.jsonl,20260817T050103Z.jsonl) — the same hand method agents[] carries no toolCalls, so per-worker call counts need the trace the sweep deletes #330 says is the only way to get the number today, run independently of this code; its totals also match therun toolCallscolumn of the issue's own table (376, 237). The unit-test fixtures' counts are hand-derived from the events written into them (2 dispatches + 1 main + 5 A + 2 B = 10), not read back from the implementation.toolCallson everyagents[]entry of astage: "final"row, (b) a test pinning that the per-agent counts account for the run-leveltoolCalls, (c) a decision on per-agentstartupToolCalls. Covered a, b, and c (decided against, with the reason on the record above).Closesbecause the field, the identity test and the ruling are the whole ask.