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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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.
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.
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.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed September 10, 2026, 5:31 PM ET / 21:31 UTC (Revision 42). ClawSweeper reviewWhat this changesAdds an opt-in Pi local-history provider across the app, CLI, dashboard, and widget, with shared session discovery, cost accounting, cache validation, and regression tests. Merge readiness⛔ Blocked before merge - 8 items remain This remains a distinct, maintainer-engaged addition absent from main and v0.58.0. The latest follow-up resolves several prior findings, but three concrete defects and incomplete runtime proof prevent a clean review. Priority: P2 Review scores
Verification
How this fits togetherCodexBar reads local Pi and OMP session logs to estimate token usage and spend. Shared accounting feeds standalone provider reports and combined dashboards, where overlapping contributions must be counted once. flowchart TD
A[Settings and live processes] --> B[Resolve session directories]
B --> C[Read Pi and OMP logs]
C --> D[Validate and cache usage]
D --> E[Separate native and Pi totals]
E --> F[Standalone provider reports]
E --> G[Combined dashboard and widget]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve inclusive standalone reports while making Pi a correctly classified local source, with selector changes replacing obsolete roots and current-candidate upgrade proof confirming stable totals. Do we have a high-confidence way to reproduce the issue? Yes, source establishes focused triggers for the findings: remove a retained sessionDir setting, lose CWD lookup for an OMP profile process, or enable Pi in Overview. These paths were inspected, not executed. Is this the best way to solve the issue? The shared resolver and retained native projections are appropriate, but the implementation still mishandles selector removal and local-source classification; those repairs belong in the existing shared owners. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against c9f2eca8da03. 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 (41 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>
clawsweeper
Bot
added
P2
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.
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 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
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: 6653f8333d
ℹ️ 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 provider == .claude, selectedProviders.contains(.pi) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
When a multi-provider cost selection contains both Codex and Pi, this ownership check applies only to Claude. For the default ungrouped text or JSON paths, CostUsageFetcher.swift:756-764 therefore merges Codex-priced Pi/OMP rows into the unscoped Codex snapshot, while the Pi snapshot returns those rows again, duplicating tokens and spend in cost, dashboard, and serve output. Fresh evidence after the Claude-specific fix is that the final helper still returns includePiSessions for Codex unless it is session-grouped text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d8cee6f. CLI cost, dashboard, and serve now suppress Pi/OMP rows in the Codex snapshot whenever Pi is also selected, for both text and JSON output; Pi is the sole owner of those rows. Added regression coverage for both formats.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8cee6f9af
ℹ️ 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 cwdURLs = (baseDirectories?.isEmpty == false ? baseDirectories! : [URL( | ||
| fileURLWithPath: FileManager.default.currentDirectoryPath, | ||
| isDirectory: true)]) |
There was a problem hiding this comment.
Fresh evidence after the live-CWD fix is that a nonempty baseDirectories list replaces, rather than augments, the fallback/default resolution. If a live project uses a project-level custom sessionDir, the Pi default store containing sessions from other projects is omitted; when that process exits, the custom root is omitted instead. Each transition changes the root fingerprint and a complete scan replaces the cache, so valid historical usage appears and disappears with the currently running processes. Keep the default and previously discovered historical roots alongside live project roots.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in db96555. Cost-root resolution now preserves the default and project roots, and appends valid roots from the previous cache fingerprint so historical usage remains available after a live process exits. Added a regression covering the first scan and the post-exit scan.
| guard !result.contains(where: { $0.path == url.path }) else { return } | ||
| result.append(url) | ||
| } | ||
| let process = AgentProcessRecord(pid: 0, ppid: 0, startedAt: nil, command: "") |
There was a problem hiding this comment.
When Pi or OMP is launched with --session-dir (or OMP with --profile) and no equivalent environment override, this synthetic process always has an empty command, so the command-line branches in piSessionRootResolution and ompSessionRootResolution can never select the live process's store. piWorkingDirectories forwards only CWDs, causing the historical token scan to use settings/default roots and omit sessions that the live scanner already discovers. Preserve the relevant live process selectors or pass the resolved roots directly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in db96555. Live Pi and OMP process contexts now carry both the command line and working directory into cost scans, so --session-dir and --profile selectors are resolved through the same root logic as live session discovery. Added regression coverage for Pi, OMP, and the default Pi root.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48baa380b8
ℹ️ 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".
| switch Self.profileDirectoryInspection(canonicalProfile) { | ||
| case .missing, .unavailable: | ||
| isComplete = false |
There was a problem hiding this comment.
When ~/.omp/profiles or the XDG profiles directory contains an ordinary non-hidden file, profileDirectoryInspection returns .unavailable and this branch marks the entire root resolution incomplete. On an initial/root-changed scan, PiSessionCostScanner then restores the pre-scan cache and suppresses otherwise valid Pi/OMP usage. Regular files are not profile candidates and should be skipped; only failures to inspect actual profile directories should make discovery incomplete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c79f0dc. OMP profile discovery now distinguishes a non-directory entry from an unreadable directory: ordinary files in the profiles parent, and non-directory session-layout entries, are ignored; actual unreadable directories still make discovery incomplete. Added a regression covering a regular file beside the default profile root.
| let piWorkingDirectories: [URL] = if provider == .pi || effectiveIncludePiSessions { | ||
| await LocalAgentSessionScanner().piWorkingDirectories(environment: environment) |
There was a problem hiding this comment.
For every non-Claude provider, shouldIncludePiSessionsInTokenSnapshot returns true, so this condition runs a full process/CWD scan before refreshing Cursor, Bedrock, Antigravity, and other providers that never consume Pi roots. The discovery also occurs before the timeout task group starts, so its latency is not covered by tokenFetchTimeout (the Linux path alone permits separate five-second ps and lsof waits). Gate this work to Pi and the Claude/Codex paths that actually merge Pi history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db965559e8
ℹ️ 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, |
There was a problem hiding this comment.
When a live Pi/OMP process supplies --session-dir or --profile, hasExplicitSelection becomes true for the whole dialect, including the default root added separately through uniqueCWDs. If that default directory does not exist—as is common when the user stores all sessions at the explicit location—this marks the otherwise optional default as required, so listPiSessionFiles makes the scan incomplete and the root-change path restores the old cache instead of publishing usage from the valid explicit root. Track explicit/default provenance per resolved root rather than applying the dialect-wide selector to every root.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3124109. Root optionality is now tracked per resolved root: the standard Pi/OMP default root is marked missingIsKnownEmpty only when it is truly default and no explicit process/config selection targets that dialect; explicit roots remain required. Added regression coverage. make check passes, and the full suite passes with 1060/1060 selections across 89/89 groups.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c79f0dcf99
ℹ️ 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 process = AgentProcessRecord( | ||
| pid: 0, | ||
| ppid: 0, | ||
| startedAt: nil, | ||
| command: context.command) |
There was a problem hiding this comment.
When a live Pi/OMP process uses --session-dir with whitespace in its path, this passes the flattened command back through commandLineValue, which splits on whitespace at PiFamilySessionScanner.swift:1148. On macOS, DarwinProcessEnumerator.parseProcArgs2 already joins the original argv with spaces, so a path such as /Users/me/Pi Sessions is resolved as /Users/me/Pi; the actual sessions are omitted and the nonexistent truncated root can keep the scan incomplete. Carry the original argument array or preserve argument boundaries before re-parsing the command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3124109. Darwin KERN_PROCARGS2 now retains the original argv; live PiSessionProcessContext carries those arguments, and flag resolution uses exact argv with the command string only as a portable fallback. Paths containing spaces therefore remain one selector value. Added a regression for that path boundary. make check passes, and the full suite passes with 1060/1060 selections across 89/89 groups.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3124109936
ℹ️ 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".
| return self.appendingPreviousSessionRoots( | ||
| resolvedRoots, | ||
| fingerprint: previousSessionRootsFingerprint) |
There was a problem hiding this comment.
When PI_CODING_AGENT_SESSION_DIR, PI_CODING_AGENT_DIR, or an explicit OMP profile changes from A to B while the same cache is reused, this appends every path from A's fingerprint to B's resolved roots. A complete B scan then persists totals from both stores instead of switching scope; if A later disappears, its retained required flag also makes every B scan incomplete. Fresh evidence after the earlier incomplete-root fix is that this preservation occurs before the completeness branch and is not limited to roots discovered from exited live processes. Preserve only live-discovered historical roots, not superseded explicit configuration roots.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f4e6bc and pushed to feat/pi-provider-tokscale. Root provenance now distinguishes configured/default roots from roots discovered through live Pi/OMP process contexts. Cache fingerprints only carry forward entries explicitly marked live, so switching a configured selector from A to B drops A while an exited live process root remains available. Added the A-to-B regression: the second scan reports only B (15 -> 30 tokens) and its fingerprint excludes A. make check and the full suite pass (1060/1060 selections, 89/89 groups).
| piWorkingDirectories: [URL] = [], | ||
| piSessionProcessContexts: [PiSessionProcessContext] = []) async throws -> CostUsageTokenSnapshot |
There was a problem hiding this comment.
When codexbar cost --provider pi runs outside the Pi project and a running Pi/OMP process selected its store through project .pi/settings.json, --session-dir, or --profile, the CLI leaves these new context parameters empty, so resolution uses the CLI's cwd/default roots and omits the actual logs. CLIDashboardCommand.swift:119 and CLIServeCommand.swift:1480 make the same direct call. Fresh evidence after the live-CWD/root-selector fix is that only UsageStore invokes LocalAgentSessionScanner.piSessionProcessContexts; all newly exposed CLI Pi cost paths bypass it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f4e6bc and pushed to feat/pi-provider-tokscale. CLI cost now collects LocalAgentSessionScanner.piSessionProcessContexts once when Pi history is selected, then passes those CWDs and exact argv through cost, dashboard, and serve loads. This makes project .pi/settings.json, --session-dir, and OMP --profile roots resolve the same way as the live scanner even when the command runs elsewhere. Focused CLI/dashboard tests and the full suite pass (1060/1060 selections, 89/89 groups).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f4e6bc1af
ℹ️ 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".
| /// Provider-specific by design: visible Pi owns its Claude-priced rows in the spend dashboard; | ||
| /// Claude only falls back to those rows while Pi is not an active cost source. | ||
| func shouldIncludePiSessionsInTokenSnapshot(for provider: UsageProvider) -> Bool { | ||
| guard provider == .claude else { return true } |
There was a problem hiding this comment.
When both Codex and Pi are enabled and Codex uses its ambient scope, this guard returns true for Codex, so CostUsageFetcher.loadLocalTokenScanResult merges Codex-priced Pi rows into the Codex snapshot while the separate Pi snapshot aggregates the same rows. The provider cards therefore display the same tokens and spend under both providers. Fresh evidence beyond the CLI ownership fix is that the regular UsageStore policy still special-cases only Claude; apply the same Pi-ownership check and scope invalidation to Codex.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb57e8b and pushed to feat/pi-provider-tokscale. UsageStore now treats Codex and Claude consistently: when Pi is enabled and cost usage is effective, the regular Codex/Claude snapshots suppress Pi rows so the Pi snapshot remains the sole owner. The piRows=fallback/owned marker is included in both provider scope signatures, invalidating stale publications when Pi visibility changes. Added regression coverage for both providers; make check and the full suite pass (1060/1060 selections, 89/89 groups).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb57e8b4f1
ℹ️ 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".
|
|
||
| /// Provider-specific by design: visible Pi owns its Claude- and Codex-priced rows in the spend dashboard; | ||
| /// Claude and Codex only fall back to those rows while Pi is not an active cost source. | ||
| func shouldIncludePiSessionsInTokenSnapshot(for provider: UsageProvider) -> Bool { |
There was a problem hiding this comment.
When ambient Codex cost history and Pi are both enabled at app startup, hydrateCachedTokenSnapshots still calls loadCachedCodexTokenSnapshotResult with its default includePiSessions: true, bypassing this new ownership policy. It installs a Codex snapshot containing Pi rows and may mark it current for the token-fetch TTL, while the separately refreshed Pi snapshot publishes those rows again. Fresh evidence beyond the regular-refresh fix is the unchanged hydration call at UsageStore+TokenCost.swift:305-309; pass this policy into that cached load as well.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0ffa963 and pushed to feat/pi-provider-tokscale. Cached Codex token hydration now captures and passes the same Pi ownership policy as regular refreshes, so when Pi is an active cost source it loads native Codex rows with includePiSessions=false and cannot duplicate Pi rows. Added a synthetic startup hydration regression (42 native + 7 Pi tokens remains 42 in the Codex snapshot). make check and make test pass (1060/1060 selections, 89/89 groups, 0 first-pass failures, 0 retries, 0 timeouts).
| let parsedBytes: Int64 | ||
| var isComplete = true | ||
| do { | ||
| parsedBytes = try CostUsageJsonl.scan( |
There was a problem hiding this comment.
If a scan catches a session file while its final JSON object is only partially written, CostUsageJsonl.scan leaves that tail uncommitted and never invokes onLine, so isComplete remains true. The scanner then stores the file's current size/mtime and publishes established history; if the writer stalls or crashes, later scans take the unchanged-metadata cache hit and never retry the omitted tail, potentially confirming an empty or undercounted Pi snapshot. Compare the returned committed offset with the observed file size and mark the parse incomplete when bytes remain.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0ffa963 and pushed to feat/pi-provider-tokscale. Pi JSONL scans now compare the committed offset with the file size after reading; an unfinished EOF tail marks the scan incomplete, so the cache is not advanced or published as established until the writer completes the record. The regression covers the malformed/unfinished tail and retains the previous usage. make check and make test pass (1060/1060 selections, 89/89 groups, 0 first-pass failures, 0 retries, 0 timeouts).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ffa96308e
ℹ️ 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".
| tokenCost: ProviderTokenCostConfig( | ||
| supportsTokenCost: true, | ||
| noDataMessage: self.noDataMessage, | ||
| menuHintLines: [.estimate], | ||
| supportsTokenSnapshot: true, |
There was a problem hiding this comment.
On a Pi-only installation where tokenCostUsageEnabled has never been set, this provider remains unusable in the app by default: SettingsStore.applyTokenCostDefaultIfNeeded enables cost tracking only when hasAnyTokenCostUsageSources finds Codex or Claude logs (SettingsStore+TokenCost.swift:18-108), so existing ~/.pi or ~/.omp sessions are ignored and UsageStore.refreshTokenUsage clears Pi's token state while cost tracking is off. Extend source detection to the Pi-family roots when adding this cost-only provider.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0daeef7. hasAnyTokenCostUsageSources now resolves the shared Pi-family roots, including ~/.pi, ~/.omp, configured roots, and discovered OMP profiles, before checking Claude sources. This makes a Pi/OMP-only install auto-enable token cost when history exists. Added SettingsStoreTokenCostSourceTests covering both default stores; make check and make test passed (1062/1062 selections).
| for url in result.files { | ||
| files.append(SessionFileCandidate(url: url, rootIndex: rootIndex)) |
There was a problem hiding this comment.
When a live or retained custom session root is nested under the newly preserved default root, such as ~/.pi/agent/sessions/project-a, the recursive enumeration of both roots appends the same JSONL path twice here. rebuildDailyUsage then applies unkeyedContributions once per candidate, so supported records without entry IDs are counted twice in Pi token and spend totals; canonicalize and deduplicate file paths before scanning and rebuilding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0daeef7. The scanner now canonicalizes each discovered JSONL path with standardizedFileURL plus symlink resolution and removes duplicates before parsing or rebuilding daily usage, so overlapping retained/live roots cannot double-count ID-less records. Added a nested-root regression that asserts 7 tokens and one cached file; make check and make test passed (1062/1062 selections).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f772243fc8
ℹ️ 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 processRootIsRetained = Self.hasExplicitProcessRootSelection( | ||
| dialect: dialect, | ||
| processContexts: [context]) |
There was a problem hiding this comment.
When a live Pi process relies on a project-level .pi/settings.json to redirect sessionDir and has no --session-dir flag, this marks the resolved root as non-retainable. After the process exits, its working directory is no longer supplied, and PiSessionCostScanner.appendingPreviousSessionRoots keeps only fingerprint entries marked live, so the custom root is dropped and the cache is replaced with default-root usage, making valid historical spend disappear. Fresh evidence after the earlier preservation fix is that the final processRootIsRetained check now preserves only command-line selectors rather than project-settings roots.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75a48933f9
ℹ️ 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".
| self.sessionRootsFingerprint(self.defaultSessionRoots( | ||
| options: options, | ||
| previousSessionRootsFingerprint: nil)) |
There was a problem hiding this comment.
When a live --session-dir or project-settings root has been cached with the live marker and the process exits, the scanner calls defaultSessionRoots with the cached fingerprint and continues returning that retained root, but this helper passes nil and advertises a scope without it. refreshPiHistoryScope then clears the old publication, while tokenAccountingScopeIsCurrent rejects every new result because the scanner's scopeFingerprint still contains the retained root; valid Pi history therefore disappears and cannot be republished after the process exits. Compute the advertised fingerprint using the same retained-root scope as the scan, or compare scopes with equivalent retention semantics.
Useful? React with 👍 / 👎.
| return PiSessionRootResolution( | ||
| // Project settings are durable configuration. Keep this root in the cache | ||
| // scope after the process exits so its history remains attributable. | ||
| roots: [SessionRoot(url: url, layout: .direct, preserveAfterProcessExit: true)], |
There was a problem hiding this comment.
When a project or global settings.json changes sessionDir from A to B, this marks A as retainable, so appendingPreviousSessionRoots adds A from the prior cache fingerprint alongside newly resolved B. A complete scan then rebuilds totals from both stores—especially visible in direct CLI cost loads, which have no publication-scope rejection—and if A disappears its required-root status can instead keep the scan incomplete. Retain roots only to bridge a process exit, while replacing roots superseded by current settings.
Useful? React with 👍 / 👎.
| return processes.compactMap { process in | ||
| guard let cwd = cwdByPID[process.pid], !cwd.isEmpty else { return nil } |
There was a problem hiding this comment.
When CWD discovery fails for a live Pi/OMP process, this guard drops the entire process context even if its argv contains an absolute --session-dir, which requires no working directory to resolve; the historical scan consequently falls back to ambient/default roots and omits the selected store. Fresh evidence beyond the earlier live-selector fix is that the final compaction still conditions preservation of argv on a successful CWD lookup; retain contexts with absolute selectors using a neutral fallback directory or make the working directory optional.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2e85476a1
ℹ️ 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 piBaseline = providerBaselines.first { $0.provider == .pi } | ||
| let piCurrent = self.capturedTokenPublication(store: store, provider: .pi) | ||
| let piOwnsVisibleSource = providers.contains(.pi) | ||
| && !configuration.hiddenSourceIDs.contains(UsageProvider.pi.rawValue) |
There was a problem hiding this comment.
When both Pi and Claude are enabled and Pi has recorded usage, hiding the Pi source makes this predicate false, so dashboardTokenSnapshot switches Claude from its native projection back to the inclusive snapshot containing Pi rows. The model then filters out the Pi input, but those costs remain visible under Claude, meaning the source filter does not actually remove Pi spend and misattributes it to another provider. Pi ownership should remain active when the row is hidden; visibility should only control whether the separate Pi input contributes to the dashboard.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d04206180
ℹ️ 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".
| guard workingDirectory != nil || | ||
| PiFamilySessionScanner.hasAbsoluteSessionDirectorySelector( | ||
| in: process, | ||
| environment: environment) |
There was a problem hiding this comment.
When an OMP process uses --profile work and its CWD cannot be read, this guard drops the process context because only an absolute --session-dir is accepted. OMP profile resolution is CWD-independent when HOME is available, so the later cost scan instead auto-discovers every profile and can include usage from unrelated profiles. Retain --profile contexts without a CWD and resolve them using the existing OMP profile resolver.
Useful? React with 👍 / 👎.
| tokenCost: ProviderTokenCostConfig( | ||
| supportsTokenCost: true, |
There was a problem hiding this comment.
When Pi is enabled in Overview, this token-cost source enters the spend dashboard as a normal native input, and SpendDashboardController.publishCurrentState assigns every such input the .subscription role; SpendDashboardPublication.subscriptionCount therefore includes Pi in the text “%d of %d subscriptions have spend.” A Claude-plus-Pi setup is consequently reported as two subscriptions even though Pi is only a local-history estimator. Add a local-source classification or exclude Pi from subscription coverage counts.
Useful? React with 👍 / 👎.
3d04206 to
c8fb54e
Compare
2 participants
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