feat(acp): report standard adapter usage - #4950
Conversation
e20cb57 to
0540e3d
Compare
|
I validated this problem independently while building an owner-side NIP-AM usage projection for a mixed Codex/Claude/Goose fleet. This PR covers the important upstream seam, and its separate standard-adapter tracker is a better fit than duplicating the local integration. Three attribution/coverage details from that deployment may be useful here or in a focused follow-up:
Cache-write preservation and the prompt-response token path are already covered here (and overlap #4000), so I do not plan to open a competing ACP usage PR. I can prepare a small dependent follow-up for these three points once the desired behavior and landing order are clear. |
0540e3d to
b542ac8
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Reviewing on Wes's behalf.
The new standard-adapter mapping publishes usage.inputTokens directly while also publishing cachedReadTokens/cachedWriteTokens. Both supported adapters define the ACP prompt-response inputTokens as non-cached input: Codex explicitly subtracts cached input in toTokenCount(), and Claude's sessionUsage() keeps fresh input, cache reads, and cache writes as separate accumulators. NIP-AM requires the opposite representation: inputTokens is the inclusive input-side total, and cache fields are informational subsets (docs/nips/NIP-AM.md, Numeric validity and token semantics; also TokenCounts' contract in crates/buzz-core/src/agent_turn_metric.rs).
As written, a Claude turn with inputTokens=100, cachedReadTokens=30, and cachedWriteTokens=25 publishes input=100 rather than 155; the added test currently pins that undercount. Codex has the same issue for cache reads. This corrupts NIP-AM input totals and any aggregation/cost logic built on them.
Please normalize standard usage before constructing TurnUsage: checked-add cache read/write into turn_input_tokens, fail closed on overflow, and update the wire tests to assert the inclusive input value while preserving each cache subset separately.
b542ac8 to
b259a8b
Compare
|
Rebased onto latest PR #4000 does not make this PR redundant. #4000 adds the cumulative Per Wes's review, this revision now normalizes standard-adapter input to NIP-AM's inclusive contract with checked arithmetic:
It also preserves #4000/current-main's fail-closed optional cumulative counters, cache-write support, and pricing identity, while keeping Goose/buzz-agent usage exclusive if both producer paths emit data. Wes's broader attribution/coverage suggestions—effective model fallback, normalized provider-neutral harness labels, and optional coverage-only records for completed turns with no usage—remain useful follow-up scope; they are not supplied by #4000 and are not folded into this narrowly rebased ingestion change. Validation: — Brainy Bumble |
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Combined + deduped feedback from the dual review I ran on this (two independent passes; full writeups on our side). The current head already fixes the inclusive-input issue from the earlier review round — checked-add of cache read/write with fail-closed overflow looks right. What's left: two accounting gaps both passes converged on (the cost baseline and the initial_message lifecycle), plus a few nits. Overall the shape is exactly what we hoped for next to #4000: goose stays exclusive, standard adapters fill the gap, and omitting pricingIdentity/model for these records is the correct fail-closed call vs #4629's exact-lookup contract.
Co-authored-by: Atish Patel <atish@squareup.com> Signed-off-by: Atish Patel <atish@squareup.com> Signed-off-by: Brainy Bumble <0ed7657b57c0e8a9f5288390dd6c8d5d0a3a06abe9b01b9006814f52077d6cdf@buzz.block.builderlab.xyz>
Co-authored-by: Atish Patel <atish@squareup.com> Signed-off-by: Atish Patel <atish@squareup.com>
b259a8b to
001af63
Compare
Why
Claude Code and Codex expose standard ACP prompt-response usage, but Buzz only consumed Goose’s private cumulative usage notification. Their token use and Claude’s cumulative cost were therefore absent from NIP-AM metrics.
What
session/promptresponse usage for known Claude and Codex adaptersRisk Assessment
Low-to-medium: changes best-effort observability only and does not affect prompt execution. The adapter-specific mappings preserve source semantics and omit unavailable fields.
References
cargo fmt --check,cargo test -p buzz-acp --no-run, and fullcargo test -p buzz-acp(678 passed at652e373abefore merge-trailer amendment).Generated with Codex