Add Pi token usage provider - #3246
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3732c325e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let piDaily: CostUsageDailyReport = try await CostUsageScanExecutor.run { checkCancellation in | ||
| try PiSessionCostScanner.loadDailyReportCancellable( | ||
| // Provider-specific by design: this call reads Pi's local aggregate session ledger. | ||
| provider: .pi, |
There was a problem hiding this comment.
Deduplicate Pi spend from Claude totals
When Pi and Claude are both enabled with cost usage, this branch publishes Claude-priced Pi rows under Pi, while the existing loadLocalTokenScanResult path still merges those same rows into Claude at CostUsageFetcher.swift:726-740; SpendDashboardModel then sums provider totals at line 522, so Claude-backed Pi tokens and spend are counted twice. Assign these rows to only one dashboard source or explicitly deduplicate the overlap.
Useful? React with 👍 / 👎.
|
|
||
| // Provider-specific by design: Pi has an independent aggregate token-cost history over its local JSONL logs. | ||
| if provider == .pi { | ||
| var piOptionsOnly = overridePiScannerOptions ?? PiSessionCostScanner.Options() |
There was a problem hiding this comment.
Resolve configured Pi session roots
For users who relocate sessions through PI_CODING_AGENT_SESSION_DIR, PI_CODING_AGENT_DIR, Pi's sessionDir setting, or OMP profiles, constructing default options here ignores the supplied environment and scans only ~/.pi/agent/sessions and ~/.omp/agent/sessions. The existing root logic in PiFamilySessionScanner.swift:631-698 already recognizes these supported layouts, but the new token provider will report no sessions for them unless equivalent roots are passed into PiSessionCostScanner.Options.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Maintainer update
I have repaired items 1–3 directly on this PR, preserving @Yuxin-Qiao's contributor credit. The integration is in fa1ab1d9fe9427df40977ab1e6c872773468bd92, with Linux portability and cache-persistence follow-ups through 6f5e173d9020e6f49f73e34de615d5f4468a386a. Regression coverage now exercises disjoint combined totals, native account isolation, selected filesystem scope, cache identity, and incomplete history. Independent review is clean, and both native Linux build/test/smoke jobs pass.
Final verification and the remaining installed-app/widget proof are maintainer-owned. No further contributor changes are requested. The current commands, results, CI links, and proof limitations are recorded in the PR description; this review remains blocking until the remaining verification is complete.
Original source review — historical findings, now repaired
Thanks @Yuxin-Qiao. Reusing the existing scanner and provider descriptors is a sensible starting point, but this needs changes before merge. I checked c3732c3 against main b4694bd and followed the actual shared-spend callers.
-
Prevent overlapping Claude/Pi totals. The new Pi report merges the Anthropic and OpenAI Codex Pi ledgers, while
CostUsageFetcherstill merges Anthropic Pi rows into Claude by default.UsageStore.loadTokenUsageSnapshotleaves that default enabled, and the shared spend model sums the distinct Claude and Pi source inputs. With both enabled and visible, the same Anthropic Pi event contributes twice. Native account-scoped Codex dashboard scans explicitly exclude Pi, so this is not a claim that every Codex view doubles usage. Define whether Pi is a non-additive view of the existing ledger or a disjoint additive source, and prove exactly-once combined totals with Pi plus Claude, including non-Pi Claude usage and source visibility changes. Hiding all Claude usage or subtracting aggregate totals would lose the required attribution. -
Honor the selected filesystem scope. The new fetcher branch does not pass its supplied environment into root resolution; default scanner options read the ambient user's
.pi/.ompdirectories. Agent Sessions already recognizes configured Pi roots and supplied HOME, so relocated history can appear there but be absent from the new provider—or the provider can inspect the wrong default root. Resolve an explicit supported root context and tie cache identity to it. Cover configured versus ambient roots, supplied HOME and OMP profile boundaries using isolated fixtures; do not broaden scanning to every profile as a fallback. -
Do not claim established history without coverage evidence. The branch unconditionally sets
historyCoverageIsEstablished: true, but the reused scanner returns empty discovery for some missing/unreadable roots and skips malformed/truncated records/read failures. Its report currently covers only the two recognized backends. Carry scan-completeness evidence into the snapshot and distinguish confirmed empty from unavailable/partial history. Add negative fixtures, not just an assertion that the flag is true. Keep this at the scanner/report boundary; it should not become another provider-specific publication workaround.
The submitted tests exercise the isolated aggregate and registration, not these combined-source and failure cases. This review is source-based; I have not run this PR's tests or independently verified its reported CI/runtime results. Once the accounting contract is settled and those regressions are covered, the cost-only menu/widget behavior still needs packaged runtime proof. Keeping the provider opt-in does not resolve the overlap or scope issues.
|
Codex review: needs real behavior proof before merge. Reviewed September 9, 2026, 11:50 AM ET / 15:50 UTC (Revision 24). ClawSweeper reviewWhat this changesAdds an opt-in Pi provider that reads local Pi/OMP session logs for token and estimated-cost history, with menu, dashboard, CLI, widget, icon, and cache integration. Merge readiness⛔ Blocked before merge - 8 items remain Keep open: the independent Pi provider remains useful and absent from main. Earlier accounting and OMP fixes are present, but two completeness defects remain, alongside an unresolved accounting contract and the owner-held runtime proof requirement. Priority: P2 Review scores
Verification
How this fits togetherCodexBar converts local coding-agent session logs into daily token and estimated-cost snapshots. Those snapshots feed provider menus, combined spend views, the CLI, and widgets. flowchart LR
A[Pi and OMP logs] --> B[Resolve selected history roots]
B --> C[Parse usage and assess completeness]
D[Saved history cache] --> C
C --> E[Price and group daily usage]
E --> F[Select provider accounting scope]
F --> G[Menus dashboard CLI and widgets]
Decision needed
Why: The latest implementation and contributor reply deliberately choose global ownership, while the retained maintainer-verification text describes combined-view-only selection. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve inclusive standalone reports, deduplicate combined views at their shared owner, and retain prior history whenever selected roots or usage records cannot be interpreted reliably. Do we have a high-confidence way to reproduce the issue? Yes, source establishes focused failure paths: corrupt previously valid custom-root settings, or scan recognized assistant usage with unusable timestamps, and the new completeness contract can still report success. These paths were not executed in this read-only review. Is this the best way to solve the issue? Partly: reusing the existing scanner and provider framework is appropriate, but completeness must include settings and semantic record validation, and accounting selection must match the owner-approved contract. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 4fe002072ede. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (23 earlier review cycles; latest 8 shown)
|
Integrate the Pi provider and preserve standalone Claude/Codex reports while counting selected Pi history exactly once in combined spend surfaces. Bind scans, caches and publications to validated roots and coverage, share Pi settings resolution, and retain native Codex account isolation. Add isolated regression and production-view proof, repair template icon and local-source labels, and document the estimator's accounting limits. Refs steipete#3246 Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
6f5e173 to
9723a64
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9723a6497c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1 @@ | |||
| <svg width="100" height="100" viewBox="0 0 100 100" fill="none" xmlns="http://www.w3.org/2000/svg"><circle cx="50" cy="50" r="48" fill="#7C3AED"/><text x="50" y="62" text-anchor="middle" font-family="Helvetica, Arial, sans-serif" font-size="42" font-weight="700" fill="white">π</text></svg> No newline at end of file | |||
There was a problem hiding this comment.
Preserve the Pi glyph in the template icon
ProviderBrandIcon.image(for:) marks every provider SVG as a template image, but this asset composites opaque white text over an opaque purple circle. Template rendering uses the resulting alpha mask, which is only a solid circle, so the π disappears anywhere the provider brand icon is displayed. Encode the glyph and surrounding shape using transparency, preferably with paths rather than font-dependent SVG text.
Useful? React with 👍 / 👎.
Use a final-path atomic write and cover repeated cache saves with a portable roundtrip regression. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
365a2a7 to
2df0447
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2df04477d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d500468879
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Pushed follow-up commit Evidence: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0113b14009
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c3001efb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1c51293e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let resolved = PiFamilySessionScanner.costSessionRoots( | ||
| environment: options.environment, | ||
| baseDirectory: options.workingDirectory) |
There was a problem hiding this comment.
Invalidate the Pi cache when resolved roots change
When two non-forced CLI cost reads use different PI_CODING_AGENT_SESSION_DIR, profile, or working-directory settings within the 60-second debounce window, shouldRefresh is false before these roots are resolved, so the second read returns the first root's cached report as complete. Persist and compare a fingerprint of the resolved roots before accepting a cache hit so usage from one configured store is not presented for another.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b1a4f9. Pi cache hits now compare a deterministic fingerprint of the resolved session roots before the debounce decision, so changing PI_CODING_AGENT_SESSION_DIR, OMP profile, or working directory cannot reuse another root’s report. Added a regression covering two roots within the debounce window; the full suite is 1040/1040 selections across 87/87 groups.
| for dialect in dialects { | ||
| let roots = Self.sessionRoots( | ||
| for: process, | ||
| dialect: dialect, | ||
| cwd: cwdURL.path, | ||
| environment: environment) |
There was a problem hiding this comment.
Propagate failed OMP root resolution
When an explicit OMP selector is rejected, such as an invalid OMP_PROFILE or an absolute PI_CONFIG_DIR, sessionRoots returns an empty array and this loop emits no CostSessionRoot for that dialect. The cost scanner therefore treats the scan as complete, can remove previously cached OMP usage, and may publish confirmed-empty history; return resolution completeness or an incomplete sentinel when an explicitly selected root cannot be resolved.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b1a4f9. An explicitly selected OMP root that cannot be resolved now produces an unresolved sentinel with resolutionIsComplete=false; the scanner skips it for I/O and keeps the overall scan incomplete, preserving cached usage instead of publishing confirmed empty history. Added invalid OMP_PROFILE and absolute PI_CONFIG_DIR regressions in PiProviderTests; the full suite is 1040/1040 selections across 87/87 groups.
| let parsedBytes: Int64 | ||
| var isComplete = true |
There was a problem hiding this comment.
Mark malformed JSONL records incomplete
When a structurally terminated JSONL line contains invalid JSON, the callback silently skips it while this flag remains true; the scanner then advances parsedBytes, persists the cache, and reports established history even if the dropped line contained usage. Set completeness to false on JSON deserialization or non-object failures, as is already done for truncated lines, so the record is not permanently omitted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9b1a4f9. Structurally terminated but malformed JSONL lines, and non-object JSON records, now set isComplete=false before returning; the cache does not advance as a complete scan, so prior cached usage is retained and the bad record can be retried. Added a malformed-record regression in PiSessionCostScannerTests; the full suite is 1040/1040 selections across 87/87 groups.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b1a4f9b98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| costProvenance: .listPriceEstimate, | ||
| projects: [], | ||
| sessions: [], | ||
| updatedAt: now) |
There was a problem hiding this comment.
Preserve the cache timestamp for incomplete Pi scans
When a previously successful Pi cache exists and a session root later becomes unreadable or malformed, PiSessionCostScanner intentionally returns the cached report with isComplete == false, but this path stamps those retained rows with the current time. Because UsageStore.regularTokenSnapshotIsConfirmedEmpty accepts nonempty incomplete snapshots and the UI renders snapshot.updatedAt as the update time, stale spend replaces the established publication and appears freshly scanned. Preserve the cache's lastScanUnixMs, or retain/reject the prior publication, when piScanResult.isComplete is false.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 95195b9. Pi scan results now carry the cache lastScanAt; when an incomplete scan serves retained rows, CostUsageFetcher preserves that timestamp instead of stamping the snapshot with the current time. The regression asserts the unavailable-root refresh keeps the original updatedAt. Focused Pi and architecture suites, make check, and the full 1040-selection/87-group suite all pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95195b9d70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if resolvedPiOptions.piSessionsRoot == nil, resolvedPiOptions.ompSessionsRoot == nil { | ||
| resolvedPiOptions.environment = environment | ||
| } |
There was a problem hiding this comment.
Propagate Pi completeness into inclusive provider snapshots
When includePiSessions is enabled for Claude or unscoped Codex and the resolved Pi root is unreadable, missing, malformed, or truncated, the regular path later calls the report-only loadDailyReportCancellable and discards its isComplete/lastScanAt metadata; loadLocalTokenScanResult consequently marks Claude history established unconditionally and Codex history according to only its native scan, potentially presenting retained Pi rows as fresh, complete history. Fresh evidence beyond the Pi-provider completeness fix is that only the dedicated .pi branch consumes DailyReportResult; the inclusive provider path still does not.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 19c365d. Inclusive Claude/Codex Pi merges now consume DailyReportResult: Pi completeness gates historyCoverageIsEstablished, and an incomplete retained Pi cache contributes its lastScanAt to staleSnapshotUpdatedAt. Added a regression covering a retained Pi report after the configured root becomes unavailable; focused Pi and architecture suites, make check, and the full 1040-selection/87-group suite pass.
| let preservedFiles = cache.files.keys | ||
| .filter { !visiblePaths.contains($0) } | ||
| .map { SessionFileCandidate(url: URL(fileURLWithPath: $0), rootIndex: Int.max) } |
There was a problem hiding this comment.
Avoid combining caches from different Pi roots
When the configured root changes from A to B and B yields both valid files and any incomplete evidence, the valid B files are inserted into cache.files, while this branch preserves every non-visible cached path from A and rebuilds the report from both sets. The resulting incomplete snapshot can therefore temporarily sum unrelated stores and inflate tokens/spend. Fresh evidence after the root-fingerprint fix is that the incomplete branch does not scope preserved entries to the current fingerprint; retain the previous report wholesale or exclude cache entries owned by the old root set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 19c365d. When the resolved root fingerprint changes and scanning the new scope is incomplete, the scanner restores the pre-scan cache wholesale before rebuilding the report; provisional new-root files cannot be combined with old-root entries. Added a root-switch-with-malformed-record regression; focused Pi and architecture suites, make check, and the full 1040-selection/87-group suite pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19c365dd0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "PI_CODING_AGENT_DIR", | ||
| "OMP_PROFILE", | ||
| "PI_PROFILE", | ||
| "XDG_DATA_HOME", |
There was a problem hiding this comment.
Do not treat XDG_DATA_HOME as an explicit OMP store
When a Linux environment defines XDG_DATA_HOME but $XDG_DATA_HOME/omp/sessions does not exist, OMPSessionRootResolver.defaultProfileRoots deliberately falls back to $HOME/.omp/agent/sessions; this entry nevertheless makes hasExplicitSelection true, so a missing fallback directory is classified as required rather than a known-empty default. Consequently otherwise valid Pi-only scans remain incomplete indefinitely, preventing empty history from being established and forcing stale-cache behavior. Only treat XDG as explicit when its OMP sessions root was actually selected, or classify the resolved home fallback as a default root.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ef9fac2. OMP root selection no longer treats XDG_DATA_HOME alone as explicit. If XDG’s OMP sessions directory is absent, the resolver’s $HOME/.omp/agent/sessions fallback remains a known-empty default; an actually selected XDG sessions root is still used when present. Added the fallback regression; focused Pi/OMP suites, make check, and the full 1040-selection/87-group suite pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef9fac2fb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let defaultRoot = Self.defaultCostSessionRoot(for: dialect, environment: environment) | ||
| output.append(CostSessionRoot( | ||
| url: canonical, | ||
| missingIsKnownEmpty: !hasExplicitSelection && defaultRoot.map { $0 == canonical } == true, | ||
| resolutionIsComplete: true)) |
There was a problem hiding this comment.
Treat missing auto-discovered profile stores as empty
When no OMP profile is selected, ompSessionRoots auto-discovers every directory under ~/.omp/profiles, and profileSessionRoots appends <profile>/agent/sessions even when that directory does not exist. This expression classifies those non-default roots as required, so a profile that has merely been created but has no sessions makes every Pi scan incomplete; on the initial scope change, PiSessionCostScanner restores the empty pre-scan cache and suppresses otherwise valid Pi usage. Auto-discovered profile roots should either be emitted only when their sessions directory exists or treat a missing sessions directory as known-empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e495ceb. Auto-discovered OMP profile roots are now emitted only when their sessions directory exists: XDG-style /sessions remains supported, and legacy /agent/sessions is included when present. A profile directory with no sessions store no longer makes the cost scan incomplete. Added the regression; focused Pi/OMP suites, make check, and the full 1040-selection/87-group suite pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e495ceb3a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let roots = Self.sessionRoots( | ||
| for: process, | ||
| dialect: dialect, | ||
| cwd: cwdURL.path, | ||
| environment: environment) |
There was a problem hiding this comment.
Honor PI_PROFILE when resolving cost roots
When PI_PROFILE=work is set without OMP_PROFILE, this new path calls sessionRoots, whose activeProfile correctly selects work, but ompSessionRoots still auto-discovers every profile because it checks only whether OMP_PROFILE is nil. Sessions from unrelated OMP profiles are therefore added to the Pi snapshot and inclusive Claude/Codex reports, inflating tokens and estimated spend despite the explicit profile selection; suppress auto-discovery when either profile selector is active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ccafbee. OMP auto-discovery now runs only when neither OMP_PROFILE nor PI_PROFILE is selected, so PI_PROFILE=work cannot pull sessions from unrelated OMP profiles. Added a regression covering selected and unrelated roots; focused Pi/OMP suites, make check, and the full 1040-selection/87-group suite pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccafbee299
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| piDaily = piScanResult.report | ||
| } | ||
| daily = CostUsageDailyReport.merged([daily, piReport]) | ||
| daily = CostUsageDailyReport.merged([daily, piScanResult.report]) |
There was a problem hiding this comment.
Exclude Pi rows from Claude when Pi is visible
When both Claude and Pi are enabled, this still merges Claude-priced Pi rows into the Claude snapshot, while SpendDashboardSource.makeRequest independently captures both enabled non-Codex providers and sums their snapshots. Fresh evidence in the final tree is that this merge remains at line 764 and the Pi snapshot itself aggregates both Claude and Codex Pi rows, so Claude-backed Pi tokens and spend appear once under Claude and again under Pi; select a native-only Claude snapshot whenever the visible Pi source owns those rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a881c63. UsageStore now applies one Pi cost-source ownership rule: when Pi is enabled and cost usage is effective, regular and Spend Dashboard Claude loads pass includePiSessions=false; Pi remains the owning aggregate source. The Claude token scope includes the fallback/owned state so changing Pi visibility invalidates stale publications. Added the ownership regression. Validation: make check passed (SwiftFormat 0/2149, SwiftLint 0 violations) and make test passed with 1040/1040 selections across 87/87 groups.
| let roots = Self.sessionRoots( | ||
| for: process, | ||
| dialect: dialect, | ||
| cwd: cwdURL.path, | ||
| environment: environment) |
There was a problem hiding this comment.
Mark failed OMP profile discovery incomplete
When no profile is selected and ~/.omp/profiles or the XDG profiles directory cannot be enumerated, profileSessionRoots silently returns no roots, but this historical resolver still treats the returned root set as fully resolved. The cost scanner can consequently complete the scan, delete previously cached contributions from those profiles, and publish undercounted history; propagate enumeration/resource failures from profile discovery into resolutionIsComplete instead of interpreting them as an empty profile set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a881c63. OMP root resolution now carries profile-discovery completeness; unreadable/non-directory profile parents, directory enumeration failures, unreadable profile/session directories, and the discovery cap produce an unresolved sentinel with resolutionIsComplete=false. The cost scanner skips that sentinel for I/O, keeps the scan incomplete, and preserves cached contributions; missing default profile directories remain known-empty. Added a failed-profile-discovery regression. Validation: make check passed (SwiftFormat 0/2149, SwiftLint 0 violations) and make test passed with 1040/1040 selections across 87/87 groups.
There was a problem hiding this comment.
💡 Codex Review
CodexBar/Sources/CodexBarCore/PiSessionCostScanner.swift
Lines 331 to 332 in a881c63
When a CLI/process populates the shared cache from root A and a later process starts with root B, this cached-only reader validates the window and pricing key but never compares cache.sessionRootsFingerprint with the currently resolved roots. loadCachedCodexTokenSnapshotResult can therefore hydrate Codex with Pi usage from A until the next scan replaces it. Fresh evidence beyond the debounce-cache fix is that this path bypasses loadDailyReportResultCancellable, where the fingerprint comparison now lives; pass the expected root scope into cached reads and reject mismatches.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| roots = Self.sessionRoots( | ||
| for: process, | ||
| dialect: dialect, | ||
| cwd: cwdURL.path, | ||
| environment: environment) |
There was a problem hiding this comment.
Mark failed Pi settings resolution incomplete
When a project or global Pi settings.json previously selected a custom sessionDir but later becomes unreadable or malformed, sessionRoots silently falls back to ~/.pi/agent/sessions; this new historical resolver then treats that fallback as fully resolved and known-empty. A complete scan can consequently replace the custom-root cache with empty/default usage. Fresh evidence distinct from the OMP resolution fix is that only OMP propagates a resolver completeness result here; Pi settings reads need the same fail-closed signal.
Useful? React with 👍 / 👎.
| let parsedBytes: Int64 | ||
| var isComplete = true |
There was a problem hiding this comment.
Reject malformed assistant rows from complete scans
For a syntactically valid assistant record with a recognized provider/model but a missing or malformed timestamp, the later guards simply skip the row while this flag remains true; a JSON boolean timestamp is also accepted as NSNumber(1) and falls outside the requested range. The cache then advances parsedBytes, persists the scan as complete, and permanently omits that usage. Fresh evidence beyond the JSON-deserialization fix is that semantic assistant validation still does not affect completeness; mark recognized-but-unbucketable usage rows incomplete.
Useful? React with 👍 / 👎.
Summary
Add Pi as an opt-in local-history provider with its own menu, dashboard source, icon, widget choice, and documentation. Thanks @Yuxin-Qiao for the original implementation; the maintainer follow-up repairs the shared accounting and filesystem boundaries on this same PR.
Pi combines supported Anthropic and OpenAI Codex Pi/OMP session usage. Standalone Claude and unscoped Codex keep their existing inclusive reports. Combined Usage & Spend and Overview views instead select retained native-only values when a visible Pi source owns the same history, so every contribution is counted once. Native account-scoped Codex remains Pi-free. Pi is a local source, not an extra subscription.
The live-session and historical readers now share Pi root selection. Historical caches and publications carry root identity and completeness; explicit missing or invalid roots fail closed, absent default history is unavailable, and malformed/truncated/unsupported records cannot become falsely complete history. Project/global settings, custom agent directories, OMP profiles and eligible XDG roots have isolated regression coverage. A valid custom Pi settings directory does not require a companion OMP installation.
The icon uses a font-free transparent glyph so macOS template rendering retains the Pi mark. Documentation explains the assistant-message list-price estimator, unsupported billing components, standalone CLI behavior, and widget coverage limitations.
Maintainer verification
6f5e173d9020e6f49f73e34de615d5f4468a386a. The integration commitfa1ab1d9fe9427df40977ab1e6c872773468bd92preserves the original contributor commit as a merge parent. Two small follow-ups fix Linux hashing imports and repeated cache persistence.fa1ab1d9fe9with the repository's canonical--no-parallelsetting: 224 tests in 20 suites, plus four portable tests executed on macOS, passed. The production-view render test also passed. Coverage includes accounting/visibility, native accounts, root/cache contracts, backend/day-specific partial evidence, malformed records and boolean counters/timestamps, publication ownership, cached startup, and provider architecture.make check: passed, zero violations across 2,045 Swift files.make teston6f5e173d902: passed all 963 selections across 81 groups in 1,976.1 seconds. Eighty groups passed initially; one cache-test group reached the normal 180-second limit and recovered when all twelve selections passed individually through the existing runner. There were no full-group retries, source/assertion changes, or deadline increases; non-timeout retries stayed disabled. Earlier interrupted candidate runs are not full-pass claims.make check, and a clean independent review of the four-line diff.CryptoKitimport on Linux. The existing Swift Crypto dependency is now selected there, matching the repository's established pattern. Replacement CI run 33123394523 builds on Linux, including a successful musl job, but its x64/ARM test jobs exposed a saved-cache reload failure. The x64 run executed 455 tests in 63 suites with four failed cache-coverage assertions; the corresponding in-memory parser assertions passed. Remaining obsolete jobs were canceled.make check, explicit portable format/lint, and a clean independent review. The new regression reads the actual JSON after both first and replacement saves, checks changed values, and rejects stray temporary files. These macOS results are not a native Linux pass.6f5e173d9020e6f49f73e34de615d5f4468a386a, including native Linux x64/ARM64 builds, tests and CLI smoke checks, musl build, both macOS test shards, lint, and the aggregate gate. The cache repair is now verified on native Linux.Tests use synthetic history, isolated stores, and Keychain/file-access safeguards. The rendered light/dark menus, dashboard, Overview and widget use production views and the same synthetic accounting fixture: standalone Claude $12/120K tokens, Pi $5/50K, combined $15/150K with native Claude $10/100K. Widget publication is also tested through the in-memory save boundary.
One earlier broad focused run used parallel execution and hit the dashboard test's 30-second wait while a main-actor architecture audit occupied roughly 32 seconds. With the repository's existing serial test setting, that same publication test passed in 1.8 seconds and the entire focused set passed. No deadline, assertion, or source change was made to hide the timeout.
These are offscreen render and publication proofs, not installed-app or installed WidgetKit proofs. All automated gates are green, but packaged runtime validation remains a maintainer-owned hold and this PR is not yet merged. No contributor change request is needed. No live account or credential probe, release, or installed-widget claim is included.
Focused verification command
The render test additionally used an owned temporary output directory through
CODEXBAR_PI_PROOF_DIR.Visual proof
All images contain synthetic data rendered through production views on integration commit
fa1ab1d9fe9. Its final rerender is byte-identical to the inspected attachments. The subsequent Linux import and cache-write corrections change no rendering code or synthetic accounting values.Dashboard before and after icon/source-label cleanup
Before: the first integrated render already includes the accounting repair, but incorrectly calls Pi a subscription and loses the template glyph. This is not the original contributor commit or an installed-app screenshot.
After: the same synthetic amounts, with correct source labels and a visible Pi template glyph.
Combined Overview and Pi cost-only widget in dark appearance