Sign up
View stargazersSign up

fix(usage): exclude Pi faux runs without losing real history#21517

open1
fix/usage-parser-cache-upgrade
fix/pi-faux-usage
opened17d agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 40m ago

Pi scripted faux sessions appeared as spend, including synthetic compaction and branch summaries. Exclude faux assistant and standalone usage, track the selected model through model changes and assistant responses, and skip summaries while faux. Keep real tool-result usage even during faux runs; reset at session boundaries and resume counting after a real model is selected.

Upper layer rebuilding #19971, with thanks to @coffee100percnt for tracing the faux provider and persisted-cache issue. It uses the generic cache-upgrade hook in #21516. Compared the complete original diff: retained the scanner marker, projected parsing, faux state, and migration intent; replaced the new parser-only test file with service-level coverage and corrected the destructive migration. Every legacy Pi state lacking the marker cold-reparses its source, even when it currently contains no faux rows. Deleted-source history loses only explicitly attributed faux/ rows.

Unattributed Tools/summaries rows from deleted transcripts are preserved: they can contain real tool calls or summaries, and the lost source cannot establish which were synthetic. Some synthetic summary usage can therefore remain until retention expiry when its source is gone. This is the deliberate non-destructive limit.

Verification on 39aa816:

  • Initial main: 55 tests passed. Test-only revision: 54 passed and 4 failed across 58. Implementation: 58 passed. After rebasing, pinned main passes 56 and this stack passes 59; one existing test was added on main. Four selected service/cache/scanner suites pass 85 tests.
  • The submitted five added cases became three new cases plus one extended existing fixture. Service tests cover line gating, over-8-MiB projected parsing, model switching, real tool usage, session reset, append/resume, restart, fork deduplication, pricing, and live/growing/deleted cache migration. The original destructive summary-row assertion was rejected.
  • Scoped server/provider-core/Pi typechecks, targeted lint, formatting, and whitespace checks passed. One fresh parallel whole-stack and per-layer review round by Claude Opus 5.5 and GPT-6 Sol: no actionable findings. Hypothetical tree navigation without model-change events was not treated as a confirmed defect.
  • Actual Linux server and web client ran main 07f8aee and this head at 1280×800. Same local Pi transcript fixtures; base warmed the cache, then one source was deleted before head startup. Displayed and persisted usage changed from $2.28 / 1,200 tokens to $0.48 / 720 tokens. The deleted transcript retained $0.20 of real assistant usage plus $0.06 tool and $0.07 summary usage; the live transcript retained $0.15 including a real tool call while faux. No faux bucket remains.
  • Current-head CI jobs passed, including all eight server shards, web tests, build/typecheck, lint, and the aggregate check. CodeRabbit reported no actionable code comments, but its description check requests explicit maintainer direction/scope approval. Macroscope’s approvability check is neutral and requires human review of usage accounting. Those human approval gates remain outstanding. The fixture does not run a real Pi account. macOS, Windows, iOS, Android, Electron desktop, and remote/relay/tunnel behavior were not verified live.

Before — actual main, same isolated Pi fixtures:

Before: faux usage inflates Pi spend to $2.28

After — head migrates the warmed cache, with one source transcript deleted:

After: Pi shows $0.48 and retains real deleted-transcript usage

Implementation: GPT-6.1 Sol / Codex. Tests: GPT-6 Astra / Codex. Maintainer integration and live verification: GPT-6.1 Sol / Codex. Independent reviews: Claude Opus 5.5 / Claude Code and GPT-6 Sol / Codex.

Checks

22 successful · 9 skipped · 1 neutral
All checks passed
39aa816

Added 1 commit

  • fix(usage): exclude Pi faux runs without dropping real history39aa816
MmacroscopeappBot17d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This changes production Pi usage and spend accounting, including legacy-cache migration and which records contribute to totals. The scope is focused and thoroughly tested, but metering changes warrant human review.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot17d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—5.0 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.9 KiB—29.3 KiB✅
CodexLive turn messages—2—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: 39aa816 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot17d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 050eb605-f275-4f20-8b62-a754fadb4f69
📥 Commits

Reviewing files that changed from the base of the PR and between 6e8e526ebffd70f57611226765c84cda87d7793f and 39aa81602c2a435fb13c21cabb7ed58f749dd9f3.

📒 Files selected for processing: 2
  • apps/server/src/usage/UsageService.test.ts
  • packages/provider-pi/src/server/usage.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

Pi usage parsing now tracks faux-provider state across transcript records. It filters selected faux-provider records and upgrades older scan-cache entries that lack this state.

Changes: Pi usage parsing

Layer / File(s)Summary
Track provider state and filter transcript records
packages/provider-pi/src/server/usage.ts, apps/server/src/usage/UsageService.test.ts		The parser accepts model_change entries and tracks whether the selected provider is faux. It filters records attributed to faux, plus compaction and branch-summary records when the current provider is faux. Tests cover provider changes, session resets, tool results, and updated usage totals.
Upgrade legacy scan-cache entries
packages/provider-pi/src/server/usage.ts, apps/server/src/usage/UsageService.test.ts		When cached state lacks a boolean faux value, cache upgrade filters records whose model starts with faux/. Tests check summaries, resume offsets, file metadata, deleted transcripts, and results after restart.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: stienswout

Merge Risk: ⚪ Minimal · up to 39aa8

This change stops synthetic Pi faux usage from being counted as spend while keeping real usage history. No merge-blocking risk was identified in the supplied change.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description thoroughly covers the problem, implementation, verification, limitations, and test results. However, the required Scope and approval section does not provide explicit maintainer direct…Add a link to the triaged issue or approval discussion and include the maintainer's explicit approval of the direction and scope. If no approval is required, explain why this focused fix qualifies for the exemption.
Passed Passed checks: 3 passed
Check nameStatusExplanation
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Title checkPassedThe title clearly and concisely describes the primary change: excluding Pi faux usage while preserving real history.
Full details: Description check

Explanation

The description thoroughly covers the problem, implementation, verification, limitations, and test results. However, the required Scope and approval section does not provide explicit maintainer direction or scope approval, and it states that this approval remains outstanding.

Resolution

Add a link to the triaged issue or approval discussion and include the maintainer's explicit approval of the direction and scope. If no approval is required, explain why this focused fix qualifies for the exemption.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.