fix(codebuddy): count function_call usage, fix ~60% token undercount - #402
Conversation
…ng dropped Verified on real ~/.codebuddy data (109 projects, 2245 jsonl, 33k deduped LLM round-trips): the parser undercounts net input by ~60% because it only aggregates type=="message" && role=="assistant" records. Every LLM round-trip that ends in a tool call is recorded as type=="function_call" with full providerData.rawUsage — those records are ~93% of round-trips and carry ~14x the assistant-message token volume. The WorkBuddy reader already aggregates every usage-carrying record; CodeBuddy uses the same transcript format but the fix was never ported back. - Aggregate ANY record with providerData.rawUsage, regardless of type - Dedup per round-trip via providerData.messageId (the response-level id shared by the function_call/message pair) before entry.uuid/entry.id — per-record ids would count a round-trip once per record type - reasoning_tokens from completion_tokens_details (prompt-side is always 0 on real data), subtracted from completion_tokens to avoid double-count - Cache split also reads prompt_cache_hit_tokens / prompt_cache_write_tokens (DeepSeek-style mirrors present in real rawUsage); input subtracts both cache reads and writes Real-data verification: in 172.2M / out 9.5M / cached 2.37B, matching a jsonl-only ground-truth scan within 0.1% (was: 66.5M / 5.3M / 806M).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCodeBuddy parsing now includes function-call records with raw usage. It uses response-level message IDs for deduplication and normalizes cache read, cache write, reasoning, prompt, and completion tokens. ChangesCodeBuddy usage parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/rollout.js (1)
8203-8218: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse a shared dedup path for CodeBuddy round-trips.
parseCodebuddyIncremental()builds its own provider-message/fallback dedup key here. Route the stable response-levelprovider.messageIdfallback sequence through the parser dedup helper, or extend that helper to accept this CodeBuddy key shape before creating the local key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/rollout.js` around lines 8203 - 8218, The local dedup-key construction in parseCodebuddyIncremental must use the shared parser dedup path instead of independently selecting provider.messageId, entry.uuid, entry.id, and the timestamp fallback. Update the relevant dedup helper or extend it to accept CodeBuddy’s key shape, then have parseCodebuddyIncremental reuse that helper while preserving provider.messageId as the preferred stable round-trip key.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/rollout.js`:
- Around line 8250-8251: Update the completion-token filtering logic around
reasoningTokens and completionTokens so records with reasoning-only usage are
retained: the zero-token discard guard must also consider reasoningTokens before
marking a record as empty. Preserve discarding records where both
completionTokens and reasoningTokens are zero.
---
Nitpick comments:
In `@src/lib/rollout.js`:
- Around line 8203-8218: The local dedup-key construction in
parseCodebuddyIncremental must use the shared parser dedup path instead of
independently selecting provider.messageId, entry.uuid, entry.id, and the
timestamp fallback. Update the relevant dedup helper or extend it to accept
CodeBuddy’s key shape, then have parseCodebuddyIncremental reuse that helper
while preserving provider.messageId as the preferred stable round-trip key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 34c32ce9-1002-4f60-8219-f6015fc14f5a
📒 Files selected for processing (1)
src/lib/rollout.js
The zero-token discard guard did not test reasoningTokens, so a record whose completion_tokens is entirely reasoning (completion == reasoning > 0, input/cache all zero) would be marked seen and discarded, permanently losing those tokens. Aligns with the workbuddy branch guard which already includes the reasoningTokens check. Addresses CodeRabbit review on PR xiufengsun#402.
Summary
The CodeBuddy jsonl parser only aggregates
type=="message" && role=="assistant"records, but every LLM round-trip that ends in a tool call is recorded astype=="function_call"with fullproviderData.rawUsage. On a real install those records are ~93% of all round-trips and carry ~14x the assistant-message token volume — the parser silently drops them.Verified on real
~/.codebuddydata (109 projects, 2,245 jsonl files, 33k deduped LLM round-trips):(Residual <0.1% is incremental-scan timing — new sessions written between the two scans.)
The WorkBuddy reader already aggregates every usage-carrying record for exactly this reason (
rollout.jsWorkBuddy header comment: "Usage lives on function_call records too"). CodeBuddy uses the identical transcript format — the fix was simply never ported back.Root cause
A single user "turn" in CodeBuddy is many LLM round-trips: model → tool call → tool result → model → … → final text. Only the final text reply is a
messagerecord; every intermediate round-trip is afunction_callrecord with its ownproviderData.rawUsage.Fix
providerData.rawUsage, regardless of record type (mirrors the WorkBuddy reader).providerData.messageId(the response-level id shared by the function_call/message pair of the same round-trip) before falling back toentry.uuid/entry.id. Per-record ids would count a round-trip once per record type.completion_tokens_details(prompt-side is always 0 on real data), subtracted fromcompletion_tokensto avoid double-counting output.prompt_cache_hit_tokens/prompt_cache_write_tokens(DeepSeek-style mirrors present in real rawUsage); net input subtracts both cache reads and cache writes.Note on the trace-based approach (#399)
Closed #399 in favor of this. Real-data verification showed
trace.modelInfocovers only ~64% of a session's jsonl usage (17%–100% per session — one trace per agent workflow, while sessions accumulate many workflows + sub-agent round-trips), so traces cannot be the authoritative source. The "90% of messages lack rawUsage" observation from #399 was correct, but the usage isn't missing — it lives onfunction_callrecords, which this PR reads directly. No trace files needed.Tests
test/rollout-parser.test.js: 229 pass / 2 fail — both pre-existing on main (#141codebuddy resolver fixture,#154kiro), unrelated to this changeScope
src/lib/rollout.js—parseCodebuddyIncrementalonly; WorkBuddy untouched)Summary by CodeRabbit