feat(pi): add pi / oh-my-pi usage adapter with omp auto-detection - #50
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds built-in pi and oh-my-pi JSONL session support, including source discovery, token and cost normalization, integration into daily/session/block reports, tests, configuration documentation, and a new guide page. ChangesPi usage integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SessionFiles as pi/omp JSONL sessions
participant PiAdapter as processPiSessions
participant DataLoader as data-loader
participant Reports as usage reports
SessionFiles->>PiAdapter: provide session records
PiAdapter->>DataLoader: return normalized UsageData
DataLoader->>Reports: aggregate pi usage
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
commit: |
📁 Files Reviewed (10)
🕓 Previous Review Summaries (3 snapshots · latest 10345b2)Snapshot — 10345b2Code Review Roast 🔥 — ISSUES_FOUND | Recommendation: Address before merge
Previous Findings Status
Body🏆 Best PartThe 💀 Worst Part — Still Present Finding #2In 🤏 Nitpick — Finding #3/#4
📊 Overall AssessmentThe pi adapter code itself is solid — well-tested, faithful to upstream, handles edge cases. The dedup gap in 📁 Files Reviewed (9)
Snapshot — 478e9e6Code Review Roast 🔥 — ISSUES_FOUND | Recommendation: Address before merge
Changes Since Last ReviewThis is the initial review of the PR. The diff adds a new Previous Findings Status
🏆 Best PartThe 💀 Worst PartThe pi loops in 📊 Overall AssessmentThe pi adapter itself is excellent — well-tested, follows the established patterns, and the daily loader dedup is correct. But the session and block loaders have a critical gap: they skip the deduplication step that prevents double-counting when the same session exists in both 📁 Files Reviewed (7)
Snapshot — 62de234Code Review Roast 🔥 — CLEAN | Recommendation: Merge
Overall: This is a beautifully implemented pi/oh-my-pi adapter that slots perfectly into the existing architecture. The code is clean, well-tested, and follows all the established patterns. 🏆 Best parts:
💀 Worst parts:
📊 Overall assessment: 📁 Files Reviewed (7)
Review automated by ChatLLM Code Bot using remote/Qwen3.6-35B-A3B with 0 tokens (0 in / 0 out) |
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Incremental pass over commit The dedup itself is textbook: The doc link "fix" is actually a removal: One observation, not a finding: the file-level @fileoverview ( 📊 Overall: Like finally fixing the leaky faucet by replacing the whole pipe — more effort upfront, but you'll never think about it again. Files Reviewed (3 files, incremental)
Previous Review Summaries (3 snapshots, latest commit 10345b2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 10345b2)Verdict: No Issues Found | Recommendation: Merge Incremental pass over commit The README's "Claude/Droid/ZCode" tunnel vision finally got corrected to acknowledge all seven aggregated sources, complete with a tidy data-sources table mapping each tool to its on-disk location. The "Why the fork?" rewrite repositions better-ccusage honestly — a diverged TypeScript monorepo vs upstream's Rust workspace, with feature ports going both ways. Refreshingly candid for a README; those usually read like a press release drafted by someone who's never run the binary. One observation, not a new finding: the new README data-sources table row for 📊 Overall: Like proofreading a résumé you actually believe — accurate, concise, and mercifully free of "synergy." Files Reviewed (3 files, incremental)
Previous review (commit 478e9e6)Verdict: No Issues Found | Recommendation: Merge Incremental pass over the two new commits since the last review ( The The typo fix ( 📊 Overall: Like a sequel that doesn't ruin the original — the sourceFilter integration does exactly what it says on the tin, no surprises, no drama. Files Reviewed (2 files, incremental)
Previous review (commit de7bec2)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. I came in hot expecting the usual "ported from upstream" special — mismatched token buckets, a dedup key held together with hope and string interpolation, an empty-path sentinel that breaks production. Instead I got an adapter that mirrors the devin template down to the log messages, an additive token model that actually matches upstream The 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (7 files)
Reviewed by glm-5.2 · Input: 80.7K · Output: 18K · Cached: 472.8K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@apps/better-ccusage/src/data-loader.ts`:
- Around line 2178-2208: Deduplicate pi session entries before merging them into
combinedEntries, matching the hash-based deduplication used by loadSessionData.
Apply deduplication to the rawPiEntries or mapped piEntries while preserving
distinct sessions, then merge the resulting piEntries without double-counting
sessions present in both .pi and .omp sources.
- Around line 1507-1526: Add the same createUniqueHash, isDuplicateEntry, and
markAsProcessed filtering used by loadDailyUsageData to the piEntries loop
before pushing into allEntries. Reuse the existing deduplication state and
preserve the current cost and entry construction for unique pi sessions; apply
the equivalent fix to the pi loop in loadSessionBlockData as noted by the
consolidated comment.
In `@apps/better-ccusage/src/pi-adapter.ts`:
- Line 321: Update the comment near the timestamp resolution logic to replace
“unparseable” with the spell-checker-approved spelling “unparsable,” without
changing the surrounding behavior.
In `@docs/guide/pi.md`:
- Line 3: Update the pi and oh-my-pi hyperlinks in the introductory paragraph of
docs/guide/pi.md to reference earendil-works/pi and can1357/oh-my-pi,
respectively, while leaving the surrounding description unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d49d9eb-b35d-4f4c-a052-f85bd67c83e3
📒 Files selected for processing (7)
apps/better-ccusage/README.mdapps/better-ccusage/src/_consts.tsapps/better-ccusage/src/data-loader.tsapps/better-ccusage/src/pi-adapter.tsdocs/.vitepress/config.tsdocs/guide/environment-variables.mddocs/guide/pi.md
| // Add pi entries to the collection | ||
| for (const piData of piEntries) { | ||
| piData.source ??= createSource('pi'); | ||
|
|
||
| const sessionKey = path.join('pi', piData.sessionId ?? 'unknown-session'); | ||
| const cost = fetcher == null | ||
| ? piData.costUSD ?? 0 | ||
| : await calculateCostForEntry(piData, mode, fetcher); | ||
|
|
||
| allEntries.push({ | ||
| data: piData, | ||
| sessionKey, | ||
| sessionId: piData.sessionId ?? 'unknown-session', | ||
| projectPath: piData.cwd ?? path.join('pi', 'unknown'), | ||
| cost, | ||
| timestamp: piData.timestamp, | ||
| model: piData.message.model, | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Missing dedup for pi entries — see consolidated comment.
This loop skips the createUniqueHash/isDuplicateEntry/markAsProcessed dedup step that loadDailyUsageData applies to pi entries, which is needed here because pi scans two directories that can contain the same session. Details and fix in the consolidated comment below (shared root cause with the loadSessionBlockData pi loop).
🤖 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 `@apps/better-ccusage/src/data-loader.ts` around lines 1507 - 1526, Add the
same createUniqueHash, isDuplicateEntry, and markAsProcessed filtering used by
loadDailyUsageData to the piEntries loop before pushing into allEntries. Reuse
the existing deduplication state and preserve the current cost and entry
construction for unique pi sessions; apply the equivalent fix to the pi loop in
loadSessionBlockData as noted by the consolidated comment.
Read the JSONL session files that pi and its widely used fork oh-my-pi (omp) write under ~/.pi/agent/sessions and ~/.omp/agent/sessions, and normalize each billable assistant message into the shared UsageData shape, aggregated alongside the other 6 sources. Both directories are auto-detected when neither PI_AGENT_DIR nor a custom path is set (matches upstream ccusage PR ccusage/ccusage#1338). Entries are deduplicated by the loader's createUniqueHash, so a session file present in both directories is counted once. Token accounting is additive (the Claude model): the four buckets (input/output/cacheRead/cacheWrite) are independent and summed, with NO subtraction of cached tokens from input (unlike Codex). The totalTokens fallback folds the surplus into output_tokens when output is 0 (matches upstream's apply_total_token_fallback). Cost: emits message.usage.cost.total (USD) so the default 'auto' cost mode uses it directly; 'calculate' mode recomputes from tokens. Per upstream omp PR #1338, models are NOT prefixed ([pi]/[omp]): both default directories share the 'pi' source label and pricing lookup. 17 new inline tests (record accept/reject, cache buckets, totalTokens fallback, non-object cost, zero-token/no-model skip, omp vs pi dedup, nested path extraction, path resolution, totalTokens fallback unit). Docs: new guide pi.md, README + env var (PI_AGENT_DIR) + sidebar. Backward compat verified: monthly --compact runs without regression. Ported from upstream ccusage (pi adapter default branch + PR #1338).
ci(spell): unparseable -> unparsable
Rebased onto main (which merged #51 adding source filtering via `better-ccusage <source> <report>`). The pi load blocks in the 3 loaders (loadDailyUsageData, loadSessionData, loadSessionBlockData) were added before #51 and lacked the sourceFilter gate, so `better-ccusage pi daily` would not isolate pi. Wrap each pi block in `if (sourceFilter == null || sourceFilter === 'pi')` to match the devin/codex/opencode pattern. Verified e2e: `better-ccusage pi daily` now isolates pi; global `daily` still aggregates all sources (backward compat).
de7bec2 to
478e9e6
Compare
![chatllm-code-bot[bot]](https://avatars.githubusercontent.com/u/1060585?s=60&v=4){kind=small}
The README 'About'/'Why the Fork' and the docs index/getting-started sections were outdated: they listed only Claude/Droid/ZCode and framed the fork as 'ccusage only supports Claude Code'. Both are now inaccurate — better-ccusage supports 7 sources and upstream ccusage has itself diverged into a Rust workspace with its own source set. - README: new 'Supported data sources' table (7 tools + data locations), rewritten 'Why the fork' covering multi-source AND multi-provider, explicit divergence note (TS monorepo vs Rust workspace), pi added to tagline + source-filter examples. - docs/index.md: intro mentions all 7 tools; 'Why better-ccusage' rewritten with the same multi-source + multi-provider framing and the upstream-divergence callout. - getting-started.md: prerequisites list all 7 tools and the SQLite runtime requirement (Node 22.13+ for zcode/opencode/devin).
Address CodeRabbit major review (2x): loadSessionData and loadSessionBlockData don't run the createUniqueHash gate on non-Claude source loops, so pi (which scans BOTH ~/.pi and ~/.omp) could double-count sessions present in both directories. Fix at the adapter boundary: processPiSessions now deduplicates by message.id (which encodes sessionId + timestamp + model + token buckets) before returning, so no entry set is counted twice regardless of the loader. Test 'deduplicates omp vs pi' updated: same content in both dirs now yields 1 entry (was 2, relying on downstream dedup that doesn't exist for these loaders). New test 'keeps distinct sessions that share a timestamp' verifies genuinely different sessions are preserved. Also drop speculative pi/omp GitHub links from docs/guide/pi.md (the repos are not verified; pi is a product, not an open CLI repo) and add pi to the devin.md source list.
1 participant
Summary
Adds a new
pisource adapter that reads JSONL session files from pi and its fork oh-my-pi (omp), aggregated alongside the existing 6 sources (claude/droid/zcode/codex/opencode/devin).Ported from upstream ccusage (pi adapter default branch + PR ccusage/ccusage#1338 for omp auto-detection).
What it reads
~/.pi/agent/sessions/**/*.jsonlAND~/.omp/agent/sessions/**/*.jsonlPI_AGENT_DIRnor a custom path is set, both directories are scanned (existing ones only), order.pithen.omp. Matches upstream PR #1338.createUniqueHash, so a session present in both dirs is counted once.Token & cost semantics (verified against upstream Rust)
input/output/cacheRead/cacheWriteare independent buckets, summed. NO cache subtraction from input \u2014 opposite of Codex. Matches upstreampi/parser.rs.totalTokensfallback: whentotalTokens > known sum, surplus folds intooutput_tokens(when output is 0), else dropped (no cost impact). Matches upstreamapply_total_token_fallback.message.usage.cost.total(USD) emitted ascostUSD. Defaultautomode uses it directly.[pi]/[omp]): both dirs share thepisource label and pricing lookup (matches PR #1338 \u2014 theDefaultscope upstream).typeabsent or"message"+role === "assistant"+usagepresent.Changes
_consts.ts:SOURCE_ORDER+='pi'(now 7 sources \u2192 127 auto-generated subsets); pi/omp path constants +PI_SESSION_GLOB.pi-adapter.ts(new, ~600 lines incl. tests):getPiPaths(): string[](multi-dirs!) +processPiSessions(dirs, options)+ valibot schemas +applyTotalTokenFallback+extractProject(withpath.normalizefor Windows glob paths).data-loader.ts: pi wired into all 3 loaders following the devin template, with multi-dir guards (piPaths.length === 0 || (piPaths.length === 1 && piPaths[0] === '')).docs/guide/pi.md, README updated,PI_AGENT_DIRin environment-variables.md, Vitepress sidebar.Verification
pnpm --filter='better-ccusage' typecheck: cleanpnpm --filter='better-ccusage' test: 336 passed (+17 new: 11 processPiSessions + 3 getPiPaths + 3 applyTotalTokenFallback)pnpm --filter='better-ccusage' lint: cleanpnpm --filter='better-ccusage' build: cleanmonthly --compactruns without regression on existing data.Code review (pre-push subagent)
The critical question \u2014 does pi's
inputincludecacheRead(double-counting)? \u2014 verified against upstream Rustpi/parser.rs: no, additive model is correct (matches upstream). No hidden-session leak analog (pi has no DB). One review finding fixed before push: theuniqueIddedup key now includesmodel+cacheRead:cacheCreation(matching upstreamentry_id) so same-instant messages with different buckets/models stay distinct.Follow-up (not blocking): loader-level dedup integration test;
extra_total_tokenssurplus tracking (currently dropped, no cost impact).Refs
Upstream: ccusage/ccusage#1338
Builds on #48 (sourceSchema) + #49 (devin adapter).
Summary by CodeRabbit
New Features
PI_AGENT_DIR.Documentation