Skip to content

Add Pi token usage provider - #3246

Open
Yuxin-Qiao wants to merge 17 commits into
steipete:mainfrom
Yuxin-Qiao:feat/pi-provider-tokscale
Open

Add Pi token usage provider#3246
Yuxin-Qiao wants to merge 17 commits into
steipete:mainfrom
Yuxin-Qiao:feat/pi-provider-tokscale

Conversation

@Yuxin-Qiao

@Yuxin-Qiao Yuxin-Qiao commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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

  • Candidate commit: 6f5e173d9020e6f49f73e34de615d5f4468a386a. The integration commit fa1ab1d9fe9427df40977ab1e6c872773468bd92 preserves the original contributor commit as a merge parent. Two small follow-ups fix Linux hashing imports and repeated cache persistence.
  • Integration-focused run on fa1ab1d9fe9 with the repository's canonical --no-parallel setting: 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.
  • Final make check: passed, zero violations across 2,045 Swift files.
  • Full make test on 6f5e173d902: 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.
  • Independent source review: clean, no actionable P0–P2 findings after repairing the five accepted findings at their shared owners. The generated social-card PNG was visually reviewed separately from the complete source/test/documentation/SVG/HTML review.
  • The portability follow-up passed 69 focused tests and four portable tests on macOS, final make check, and a clean independent review of the four-line diff.
  • The first hosted CI run exposed an unconditional CryptoKit import 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.
  • The cache follow-up replaces the manual file swap with one atomic write to the final cache path. It passed 61 focused tests and five portable tests on macOS, 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.
  • Final CI run 33124888635: all eight jobs passed on 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
env -u CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS \
  CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 \
  CODEXBAR_TEST_CODEX_FILE_ISOLATION=1 \
  CODEXBAR_TEST_RETRY_NON_TIMEOUT_FAILURES=0 LIVE_GEMINI_FETCH=0 \
  swift test --no-parallel --filter 'PiRecordValidationTests|PiPartitionCoverageTests|PiRootSelectionContractTests|PiHistoryCoverageTests|PiHistoryScopeLinuxTests|PiProviderTests|PiSessionCostScannerTests|PiSessionCostCompatibilityTests|PiSpendAccountingTests|PiPublicationTests|PiNativeAccountingTests|CostUsageFetcherTests|CostUsageFetcherCacheSnapshotTests|CostUsageFetcherUnknownModelPricingTests|OverviewSpendSummaryTests|SpendDashboardModelTests|SpendDashboardPublicationTests|ProviderArchitectureGatekeeperTests|ProviderIconResourcesTests|UsageStoreCachedTokenHydrationTests|PiFamilySessionTests.*custom session directories|MenuLayoutScreenshotRenderTests.test_renderPiAccountingProof'

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.

Dashboard before maintainer icon and source-label cleanup

After: the same synthetic amounts, with correct source labels and a visible Pi template glyph.

Dashboard after maintainer icon and source-label cleanup

Combined Overview and Pi cost-only widget in dark appearance

Combined Overview with one subscription and one local source

Pi cost-only production widget view with synthetic data

@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@steipete steipete left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. Prevent overlapping Claude/Pi totals. The new Pi report merges the Anthropic and OpenAI Codex Pi ledgers, while CostUsageFetcher still merges Anthropic Pi rows into Claude by default. UsageStore.loadTokenUsageSnapshot leaves 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.

  2. 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/.omp directories. 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.

  3. 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 clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 27, 2026
@clawsweeper

clawsweeper Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 9, 2026, 11:50 AM ET / 15:50 UTC (Revision 24).

ClawSweeper review

What this changes

Adds 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
Reviewed head: a881c63d08d36e3926aaa7b97a2eef2e438cd53b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) Useful implementation and extensive regression work remain limited by two correctness defects and incomplete candidate-specific runtime evidence.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: Four inspected screenshots demonstrate synthetic production-view rendering on an older integration candidate, but do not exercise this head's Pi scanner, accounting selection, or fresh/existing-cache recovery in a real setup. Preserve the explicitly owner-held packaged-app/widget validation responsibility; the proof gap does not transfer that hold to the contributor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: Four inspected screenshots demonstrate synthetic production-view rendering on an older integration candidate, but do not exercise this head's Pi scanner, accounting selection, or fresh/existing-cache recovery in a real setup. Preserve the explicitly owner-held packaged-app/widget validation responsibility; the proof gap does not transfer that hold to the contributor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 9 items Repository policy: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-note files were found in the inspected changed subtrees. Applied provider isolation, focused validation, and Keychain safeguards. No builds or tests were executed during this read-only review.
Still-needed capability: The pinned branch registers a default-disabled Pi provider with token snapshots and cost CLI support. The fetched main manifest and latest release's provider enum do not register Pi. The repository onboarding profile also disallows automatic cleanup closure.
Pi settings failures lose resolution completeness: The new historical resolver calls the existing Pi root resolver without a completeness result. Settings read/JSON failures return nil at lines 976–985, select the default root, and leave resolutionIsComplete true; the scanner's complete branch can then remove the previous custom-root cache entries.
Findings 2 actionable findings [P2] [P2] Propagate failed Pi settings resolution into completeness
[P2] [P2] Include unusable assistant timestamps in scan completeness
Security None None.

How this fits together

CodexBar 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]
Loading

Decision needed

Question Recommendation
Should enabling Pi preserve inclusive standalone Claude totals, as the PR body states, or make every app Claude snapshot native-only? Preserve standalone accounting: Keep standalone Claude reports inclusive and select native-only contributions when composing combined views.

Why: The latest implementation and contributor reply deliberately choose global ownership, while the retained maintainer-verification text describes combined-view-only selection.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Four inspected screenshots demonstrate synthetic production-view rendering on an older integration candidate, but do not exercise this head's Pi scanner, accounting selection, or fresh/existing-cache recovery in a real setup. Preserve the explicitly owner-held packaged-app/widget validation responsibility; the proof gap does not transfer that hold to the contributor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • [P2] Propagate failed Pi settings resolution into completeness (P2) - If project/global settings previously selected a custom sessionDir and later become malformed or unreadable, sessionRoots falls back to the default directory. This newly introduced historical path leaves resolution complete, so a successful default-root scan deletes the custom-root cached contributions and can publish zero usage. Distinguish absent settings from failed settings resolution and retain the prior report on failure. This concern was visible at the previous reviewed head and is raised late.
  • [P2] Include unusable assistant timestamps in scan completeness (P2) - The new completion result remains true when a recognized assistant usage row has no usable timestamp: the guard at line 741 silently returns. A boolean message timestamp is also accepted as an epoch number before a valid entry timestamp can be considered. The provider consequently publishes established history and persists a consumed offset despite omitted usage. Reject boolean timestamps and mark recognized usage that cannot be bucketed incomplete. This concern was visible at the previous reviewed head and is raised late.
  • Resolve merge risk (P1) - Enabling Pi changes standalone Claude totals in the pinned implementation, while the PR body promises those totals remain inclusive.
  • Resolve merge risk (P1) - The persisted root-fingerprint addition needs fresh-cache and existing-v8-cache compatibility evidence tied to the candidate being landed.
  • Resolve merge risk (P1) - The supplied runtime claims and screenshots reference an older integration candidate; packaged app/widget validation remains explicitly owner-held.
  • Complete next step (P2) - Repair both completeness defects, obtain the owner's accounting-contract decision, and complete the retained runtime and cache-upgrade proof before merge.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P2] [P2] Propagate failed Pi settings resolution into completeness — Sources/CodexBarCore/PiFamilySessionScanner.swift:711-716
  • [P2] [P2] Include unusable assistant timestamps in scan completeness — Sources/CodexBarCore/PiSessionCostScanner.swift:773
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth production +654 net lines; tests +909 net lines The growth supports the new provider and shared root/completeness handling, with substantial focused regression coverage.

Merge-risk options

Maintainer options:

  1. Settle accounting and validate cache upgrades (recommended)
    Confirm standalone versus combined ownership, repair completeness propagation, and verify fresh and existing caches before landing.
  2. Keep the runtime hold
    Pause landing while the owner completes candidate-specific app and widget verification.

Technical review

Best 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:

  • [P2] [P2] Propagate failed Pi settings resolution into completeness — Sources/CodexBarCore/PiFamilySessionScanner.swift:711-716
    If project/global settings previously selected a custom sessionDir and later become malformed or unreadable, sessionRoots falls back to the default directory. This newly introduced historical path leaves resolution complete, so a successful default-root scan deletes the custom-root cached contributions and can publish zero usage. Distinguish absent settings from failed settings resolution and retain the prior report on failure. This concern was visible at the previous reviewed head and is raised late.
    Confidence: 0.98
    Late finding: first raised on code an earlier review cycle already covered.
  • [P2] [P2] Include unusable assistant timestamps in scan completeness — Sources/CodexBarCore/PiSessionCostScanner.swift:773
    The new completion result remains true when a recognized assistant usage row has no usable timestamp: the guard at line 741 silently returns. A boolean message timestamp is also accepted as an epoch number before a valid entry timestamp can be considered. The provider consequently publishes established history and persists a consumed offset despite omitted usage. Reject boolean timestamps and mark recognized usage that cannot be bucketed incomplete. This concern was visible at the previous reviewed head and is raised late.
    Confidence: 0.97
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4fe002072ede.

Labels

Label justifications:

  • P2: This is an opt-in local-history improvement with bounded accounting and cache correctness defects.
  • merge-risk: 🚨 compatibility: The patch changes standalone Claude accounting and persisted history-root identity without candidate-specific upgrade proof.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Four inspected screenshots demonstrate synthetic production-view rendering on an older integration candidate, but do not exercise this head's Pi scanner, accounting selection, or fresh/existing-cache recovery in a real setup. Preserve the explicitly owner-held packaged-app/widget validation responsibility; the proof gap does not transfer that hold to the contributor. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Four inspected screenshots demonstrate synthetic production-view rendering on an older integration candidate, but do not exercise this head's Pi scanner, accounting selection, or fresh/existing-cache recovery in a real setup. Preserve the explicitly owner-held packaged-app/widget validation responsibility; the proof gap does not transfer that hold to the contributor.

Evidence

What I checked:

  • Repository policy: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-note files were found in the inspected changed subtrees. Applied provider isolation, focused validation, and Keychain safeguards. No builds or tests were executed during this read-only review. (AGENTS.md:1, a881c63d08d3)
  • Still-needed capability: The pinned branch registers a default-disabled Pi provider with token snapshots and cost CLI support. The fetched main manifest and latest release's provider enum do not register Pi. The repository onboarding profile also disallows automatic cleanup closure. (Sources/CodexBarCore/Providers/Pi/PiProviderDescriptor.swift:18, a881c63d08d3)
  • Pi settings failures lose resolution completeness: The new historical resolver calls the existing Pi root resolver without a completeness result. Settings read/JSON failures return nil at lines 976–985, select the default root, and leave resolutionIsComplete true; the scanner's complete branch can then remove the previous custom-root cache entries. (Sources/CodexBarCore/PiFamilySessionScanner.swift:712, a881c63d08d3)
  • Semantic timestamp failures remain complete: The parser clears completeness for malformed JSON and truncated lines, but the recognized assistant timestamp guard returns without clearing it. The timestamp parser also accepts NSNumber booleans before checking the entry timestamp. Successful completion then persists parsedBytes and established history. (Sources/CodexBarCore/PiSessionCostScanner.swift:741, a881c63d08d3)
  • Accounting fix and remaining contract discrepancy: Both regular and dashboard Claude loads now exclude Pi rows when Pi is an enabled cost source, and the publication scope records that choice. This resolves the previous duplicate-total finding, but differs from the captured PR body, which promises inclusive standalone Claude reports and native-only selection only in combined views. (Sources/CodexBar/UsageStore+TokenCost.swift:64, a881c63d08d3)
  • Re-review continuity: The two parser/root findings concern code unchanged since the previous completed review at this same SHA and are acknowledged as late findings. The latest OMP discovery completeness repair and Claude ownership regression were inspected; the older resolved findings are not repeated. (a881c63d08d3)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Yuxin-Qiao: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Repair settings-resolution and timestamp completeness with focused regressions covering retained cache values and timestamps.
  • Resolve standalone versus combined accounting and verify fresh-cache and existing-cache behavior.
  • Complete the owner-held real-history/app/widget proof using screenshots, recordings, terminal output, or logs; redact private details and update candidate references in the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (23 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-09T02:29:56.028Z sha 19c365d :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals | [P2] Keep an absent default OMP store optional when XDG is set
  • reviewed 2026-09-09T03:16:53.634Z sha ef9fac2 :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals
  • reviewed 2026-09-09T03:26:31.694Z sha ef9fac2 :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals | [P2] Treat absent auto-discovered profile stores as optional
  • reviewed 2026-09-09T04:08:06.589Z sha e495ceb :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals
  • reviewed 2026-09-09T04:16:18.019Z sha e495ceb :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals | [P2] Honor PI_PROFILE before discovering additional cost roots
  • reviewed 2026-09-09T04:54:16.899Z sha ccafbee :: needs real behavior proof before merge. :: [P1] Deduplicate Pi usage in combined spend totals
  • reviewed 2026-09-09T05:03:18.468Z sha ccafbee :: needs real behavior proof before merge. :: [P1] Deduplicate Pi contributions in combined spend views | [P2] Propagate OMP profile discovery failures into completeness
  • reviewed 2026-09-09T14:57:55.502Z sha a881c63 :: needs real behavior proof before merge. :: none

steipete added a commit to Yuxin-Qiao/CodexBar that referenced this pull request Aug 27, 2026
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 clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. P1 Urgent regression or broken agent/channel workflow affecting real users now. labels Aug 27, 2026
@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/pi-provider-tokscale branch from 6f5e173 to 9723a64 Compare September 2, 2026 10:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Sep 2, 2026
Yuxin-Qiao and others added 5 commits September 3, 2026 02:48
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>
@Yuxin-Qiao
Yuxin-Qiao force-pushed the feat/pi-provider-tokscale branch from 365a2a7 to 2df0447 Compare September 2, 2026 18:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/CodexBarCore/CostUsageFetcher.swift Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/CodexBarCore/PiSessionCostScanner.swift
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Sep 3, 2026
@Yuxin-Qiao

Copy link
Copy Markdown
Contributor Author

Pushed follow-up commit 0113b1400 to refresh the ProviderArchitectureGatekeeper anchors after the current upstream source moved.

Evidence: swift test --filter ProviderArchitectureGatekeeperTests passes 41/41; make check passes (95 Python tests, 1 skipped, SwiftFormat 0 files, SwiftLint 0 violations). The added provider marker documents why Pi refreshes both Codex and Claude pricing catalogs. CI is now running on the new head.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/CodexBarCore/PiSessionCostScanner.swift Outdated
Comment thread Sources/CodexBarCore/Providers/Pi/PiProviderDescriptor.swift Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/CodexBarCore/PiSessionCostScanner.swift
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 3, 2026
@clawsweeper clawsweeper Bot removed the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Sep 3, 2026
@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label Sep 8, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +386 to +388
let resolved = PiFamilySessionScanner.costSessionRoots(
environment: options.environment,
baseDirectory: options.workingDirectory)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +684 to +689
for dialect in dialects {
let roots = Self.sessionRoots(
for: process,
dialect: dialect,
cwd: cwdURL.path,
environment: environment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines 655 to +656
let parsedBytes: Int64
var isComplete = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +598 to +600
if resolvedPiOptions.piSessionsRoot == nil, resolvedPiOptions.ompSessionsRoot == nil {
resolvedPiOptions.environment = environment
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +235 to +237
let preservedFiles = cache.files.keys
.filter { !visiblePaths.contains($0) }
.map { SessionFileCandidate(url: URL(fileURLWithPath: $0), rootIndex: Int.max) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +710 to +714
let defaultRoot = Self.defaultCostSessionRoot(for: dialect, environment: environment)
output.append(CostSessionRoot(
url: canonical,
missingIsKnownEmpty: !hasExplicitSelection && defaultRoot.map { $0 == canonical } == true,
resolutionIsComplete: true))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +692 to +696
let roots = Self.sessionRoots(
for: process,
dialect: dialect,
cwd: cwdURL.path,
environment: environment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +692 to +696
let roots = Self.sessionRoots(
for: process,
dialect: dialect,
cwd: cwdURL.path,
environment: environment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

guard !allowEstablishedEmpty || cache.lastScanUnixMs > 0 else { return nil }
guard allowEstablishedEmpty || !cache.daysByProvider.isEmpty else { return nil }

P2 Badge Validate roots before accepting cached Pi history

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".

Comment on lines +712 to +716
roots = Self.sessionRoots(
for: process,
dialect: dialect,
cwd: cwdURL.path,
environment: environment)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines 695 to +696
let parsedBytes: Int64
var isComplete = true

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants