feat(adapter/copilot): read session-state events.jsonl as default data source - #1209
feat(adapter/copilot): read session-state events.jsonl as default data source#1209jay-tau wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the ChangesCopilot ingestion, credits, and api mode
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant session_state_paths
participant copilot_loader
participant usage_to_loaded
participant all_aggregator
participant report_renderer
session_state_paths->>copilot_loader: enumerate events.jsonl files
copilot_loader->>copilot_loader: parse, filter, deduplicate, and sort entries
copilot_loader->>usage_to_loaded: convert entries to cost and credit rows
usage_to_loaded->>all_aggregator: provide LoadedEntry values
all_aggregator->>report_renderer: render AllRow and totals JSON
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This PR was auto-closed. Only contributors approved with Maintainers review auto-closed issues and reopen worthwhile ones. Issues that do not meet the quality bar in CONTRIBUTING.md may not be reopened or receive a reply. If a maintainer replies See CONTRIBUTING.md. |
…ct + name AI-credit rate External code review on PR ccusage#1209 flagged two real polish items. # Drop the second-pass JSON parse for the discriminator `parse_session_state_file` previously deserialized each shutdown line twice: once as the full `ShutdownEvent` and a second time as a `TypeOnly` struct (via the `event_type` helper) to confirm the `type` field is exactly `"session.shutdown"`. The substring filter on `SESSION_SHUTDOWN_MARKER` already skips non-shutdown lines without any serde work, so the only purpose of the second parse was to defend against other event types whose `arguments.command` / `content` strings happen to contain the literal substring "session.shutdown" (e.g. a `tool.execution_start` event invoking a shell command that mentions it). That defense is still required, but folding the discriminator into `ShutdownEvent` itself accomplishes it in one parse: struct ShutdownEvent { #[serde(rename = "type")] event_type: String, ... } let event = serde_json::from_str::<ShutdownEvent>(line)?; if event.event_type != SESSION_SHUTDOWN_TYPE { continue; } The `event_type` function and the `TypeOnly` inner struct are now removed. Result: one allocation/parse per shutdown line instead of two, with identical behavior (verified by 254 unchanged copilot tests and a real-data smoke that still bills $6,480.33 / 74 days under `--mode auto --offline`). # Name the AI-credit USD rate symmetrically with the premium-request rate `PREMIUM_REQUEST_COST_USD = 0.04` was a named constant with a clear doc-comment, but the AI-credit rate was a magic `0.01` floating in `loader.rs:113`. That's asymmetric — two rates for two billing eras, only one of them named, and a future change to either rate would only be discoverable in one place. Added: const AI_CREDIT_COST_USD: f64 = 0.01; …with a doc-comment that mirrors the premium-request constant's shape, and replaced the magic value at the only call site. # Note on the third reviewer finding The third finding — `model_for_data.clone()` allocating per shutdown row — is real but on a path that runs ~660 times per typical user run and produces a small `Option<String>` allocation. The cleanups that would eliminate it (Arc<str>, restructuring the LoadedEntry constructor to consume `model_for_data` exactly once) either change type signatures in shared layers or add control-flow complexity for a marginal win. Left as-is. # Validation - `cargo clippy --workspace --all-targets -- -D warnings` clean. - 254 ccusage tests + 79 across the rest of the workspace, all pass. No tests touched the dropped `event_type` helper directly (the behavior it tested is now structural). - Smoke against `~/.copilot/session-state/`: 74 days, $6,480.33 — unchanged. Refs ccusage#1174 Co-authored-by: Copilot <[email protected]>
…ie-break Another external code review on PR ccusage#1209 raised three actionable items. Applied here. # skip_metrics_row was inconsistent with build_session_state_entry The skip rule's `no_tokens` check only looked at four buckets (input / output / cache_read / cache_write), but `build_session_state_entry` further preserves `reasoningTokens` verbatim for Gemini models (provider convention: thoughtsTokenCount is a separate field from output). A pure-reasoning Gemini row (reasoning > 0, all other tokens 0, no requests, no credits) would therefore be silently dropped by the skip rule before the provider-aware entry construction had a chance to surface it. Real local data does not contain any such row today, but the asymmetry is a latent inconsistency: the skip rule's job is "drop rows that carry no usage," not "drop rows that carry no non-reasoning usage." Fix: add `usage.reasoning_tokens == 0` to `no_tokens`. Updated: - Inline doc-comment on `skip_metrics_row` to spell out the symmetry rationale (mentions Gemini specifically). - Adapter README `### Skip rule` section: "all four token fields" → "all five token fields" plus the Gemini reasoning carve-out. - New regression test `keeps_row_with_only_reasoning_tokens` that pins the new behavior using a Gemini fixture. # Loader sort was non-deterministic for equal-millisecond timestamps `load_entries_inner` deduped via `HashMap` (upstream iteration order is non-deterministic) and then `sort_by_key(|e| e.timestamp)` — stable, but still non-deterministic for rows sharing a millisecond timestamp because the input order was already random. JSON-snapshot stability and `--json` diff-friendliness across runs both suffer. Fix: secondary sort key on `data.message.id` (= the parser's `dedup_key`), which is the most specific per-entry identifier we have. Token-bearing keys embed `shutdown:{session_id}:{event_id}:{model}` so they sort deterministically by event_id within a session and by model within an event. Credit-only keys (`credit-shutdown:{session_id}:{model}:{naiu}`) sort deterministically by model within a session. Added regression test `loader_sort_is_deterministic_for_equal_timestamps` that runs the loader 5 times against a fixture with three rows sharing one millisecond and asserts every run produces identical ordering, plus that the order follows the expected lexicographic dedup-key chain. # Trimmed meta-comment from the COPILOT_CONFIG_DIR no-fallback test The `nonexistent_copilot_config_dir_does_not_fall_back_to_default` regression test carried a multi-line meta-comment about reviewers having misread the original `then_some` code shape. The reviewer correctly pointed out that the test name and assertion already convey the intent, and the meta-narrative is noise. Removed. # Other review findings — deliberately not applied The same review flagged five other items that I evaluated and left as-is, each for a specific reason recorded here for the record: - OTel removal is a breaking change for OTel-only users: covered in the PR description (ccusage#1209). No code change needed. - Hardcoded `PREMIUM_REQUEST_COST_USD` / `AI_CREDIT_COST_USD` constants: maintenance liability if GitHub changes rates, but the constants carry doc-comments explaining their source. Doing anything more involved would require runtime config or a remote pricing fetch — out of scope for this PR. - Auto bills pre-cutover free-tier rows as $0: documented in `docs/guide/cost-modes.md` and the adapter README. Genuine bill, not a bug. - Credit-only content dedup can collapse legitimately-distinct events: explicit, well-documented tradeoff (real-world data has duplicate snapshots; the alternative would inflate credits by ~2x for the observed pairs). - `--mode api` is a global alias rather than Copilot-scoped: intentional. Already framed as a universal alias in `docs/guide/cli-options.md` and `docs/guide/cost-modes.md`. # Validation - `cargo clippy --workspace --all-targets -- -D warnings` clean. - 256 ccusage tests + 79 across the rest of the workspace (`+2` vs the previous baseline: the reasoning-skip and deterministic-sort regression tests). - Smoke against `~/.copilot/session-state/`: 74 days, $6,480.33 — unchanged. None of these fixes affect real-world numbers (the reasoning-skip change is forward-compat; the deterministic-sort change is observability-only). Refs ccusage#1174 Co-authored-by: Copilot <[email protected]>
…4 nano-AIU typing Another external code review on PR ccusage#1209 surfaced two actionable items. # Stream events.jsonl instead of slurping into a Vec<u8> `parse_session_state_file` used `fs::read(path)?` to load the entire file into a Vec<u8>, then `bytes.split(|b| b == b'\n')` to iterate lines as byte slices. For real-world session-state files (the reviewer cites 69MB+ on heavy users) this briefly allocates the whole file into RAM per parsed session — fine on modern machines but unnecessarily generous. Switched to: let reader = BufReader::new(fs::File::open(path)?); for line_result in reader.split(b'\n') { let line_bytes = line_result?; ... } `BufReader::split(b'\n')` returns `io::Result<Vec<u8>>` per line, preserving the byte-level UTF-8 tolerance and trailing-line semantics the previous code relied on (`std::str::from_utf8` skip and empty-line skip both still hold for the new byte slice). Peak RAM per file drops from O(file-size) to O(line-size), bounded by BufReader's 8KB internal buffer plus one line's Vec<u8>. Behavioral parity: - Empty trailing segments after the last `\n` are still skipped by the existing whitespace/empty checks (BufReader::split matches Vec::split's no-trailing-empty-element behavior — the final segment is only returned if it has bytes). - Mid-file read errors now propagate via `?` rather than being impossible (since fs::read was all-or-nothing). Session-state files are append-only in practice, so concurrent writers don't trigger this; if they ever do, the failure mode is "skip this file's remaining lines" rather than "silently truncated parse," which is the correct behavior. # Explain why total_nano_aiu stays u64 (vs requests.cost which is f64) The same review noted that `total_nano_aiu: Option<u64>` on `ShutdownData` and `ModelMetrics` lacks the float-tolerance hardening that `requests.cost` got — if a future Copilot CLI ever emits `totalNanoAiu` as `1.5e11` or `100000000000.0`, the whole `ShutdownEvent` deserialize would fail and every model in that shutdown would be silently dropped (the exact failure class the `requests.cost: f64` change fixed). The risk is genuinely low — "nano" units are integer by definition, nano-AIU has no meaningful fractional component, and the CLI has emitted integer JSON for AIU across every observed version. But the asymmetry with `requests.cost` (which IS legitimately fractional — 2% of real rows show Opus 4.7's 7.5× multiplier as 7.5) deserves an in-source explanation so future maintainers don't repeat the audit. Added a doc-comment on both fields explaining: - why u64 is safe here (integer by definition) - what the failure mode would be if it weren't (same drop-the-event cascade fixed for requests.cost) - the escape hatch (switch to a tolerant deserializer that accepts integer-or-float-with-zero-fract) No code change — the reviewer themselves rated the risk as "genuinely low" and suggested "either a tolerant number helper or a comment explaining why u64 is safe here when cost wasn't." Picked the comment route because the helper would add complexity for a hypothetical schema change. # Other findings deliberately skipped - Credit-only content dedup collapsing legitimately-distinct snapshots: documented as deliberate tradeoff (real-world duplicates outweigh the hypothetical content-collision case). - OTel dead code retained pending ccusage#1208: deliberate scope split. - config-schema.json oxfmt churn: documented in PR description; fixing it would require an upstream-first reformat commit. - Empty-state message on `--since/--until` filtered output: matches existing adapter convention; changing it would be a cross-adapter pattern shift. # Validation - `cargo clippy --workspace --all-targets -- -D warnings` clean. - 257 ccusage tests + 79 across the rest of the workspace, all pass. All test fixtures continue to round-trip through the new BufReader path (the byte-level handling is identical to the prior Vec::split). - Smoke against `~/.copilot/session-state/`: 74 days, $6,480.33 — unchanged. No semantic difference, only a memory-profile improvement. Refs ccusage#1174 Co-authored-by: Copilot <[email protected]>
…e + close calculate-mode coverage gap A fresh external code review (full re-pass, post-ccusage#1209-close) confirmed the branch is high-quality and well-tested, surfacing three small actionable items. # SESSION_SHUTDOWN_MARKER vs SESSION_SHUTDOWN_TYPE — anti-cleanup comment Both constants hold the literal "session.shutdown", which invites a "dedupe these" cleanup that would silently break the parse-time pre-filter optimisation. `SESSION_SHUTDOWN_MARKER` is a substring needle used for the cheap `str::contains` filter BEFORE serde parsing; `SESSION_SHUTDOWN_TYPE` is the authoritative exact-match compared against the parsed `ShutdownEvent.event_type` after serde has converted it to a String. Different roles, same literal today — but a future schema rename (e.g. "session.exit") would only update one. Added a block comment above both constants explaining this. # ConfigCostMode::Api — confirm global scope is intentional The reviewer noted that `--mode api` is global rather than Copilot-scoped: it propagates through `ConfigCostMode` and the shared `parse_cost_mode`, so every agent's `--mode` accepts `"api"` as a calculate alias. This was intentional — the semantics ("compute from tokens via LiteLLM, never from source-precomputed cost") generalise cleanly across all adapters, and exposing the shorter, intent-revealing name globally was a deliberate design choice. Added a block comment on `ConfigCostMode::Api` confirming this so future maintainers don't try to scope it down to Copilot. # Test coverage — credit-only row under --mode calculate The reviewer's only test-gap finding: the existing `calculate_mode_uses_token_pricing_even_with_naiu_present` test uses a token-bearing row, leaving the credit-only-zero-token shape under-`calculate` untested. Added `calculate_mode_bills_zero_for_credit_only_zero_token_row` to pin the documented behavior: zero tokens × any pricing = $0, calculate mode does NOT fall back to surfacing the credit-converted cost (that's auto/display's job), but the credits channel stays surfaced in JSON regardless of mode. # Other reviewer findings — deliberately skipped - Lossy credit-only dedup (omits event_id by design): already documented in PR body and adapter README; the inflation it prevents ($2.65 observed locally) is real, the collision shape it could under-count is hypothetical. - Extract a `compute_cost(mode, has_aiu, ...)` fn from `usage_entry_to_loaded`: would need 6-7 parameters and gain only cosmetic isolation; integration tests already cover every dispatch branch. Skip. - Promote `EnvScope` to `ccusage-test-support`: cross-crate refactor for minor de-duplication. Skip. - Poison-tolerant `COPILOT_ENV_LOCK`: `.lock().unwrap()` is the repo convention; poison propagation is a real signal that a test panicked while holding the lock and should fail subsequent tests. - `mem::take` for the loader's `HashMap` dedup String clone: the cloned value is a short `dedup_key`; refactor adds complexity for no measurable win. # Validation - `cargo clippy --workspace --all-targets -- -D warnings` clean. - 258 ccusage tests + 79 across the rest of the workspace (`+1` vs the previous baseline: the new calculate-mode credit-only regression test). - Schema regeneration is a no-op: the new comment on `ConfigCostMode::Api` is a line comment (not a doc-comment) so schemars does not surface it, and `generate-config-schema` output is byte-identical to the committed `apps/ccusage/config-schema.json`. - Smoke against `~/.copilot/session-state/`: 74 days, $6,480.33 — unchanged. Refs ccusage#1174 Co-authored-by: Copilot <[email protected]>
…regression test External PR ccusage#1209 code review (Low) flagged a subtle edge in the era-aware ladder: `has_aiu = credits.is_some()` means a row with `totalNanoAiu: 0` (genuine "AIU charged, value zero") plus a non-zero `requests.cost` bills $0 via the AIU arm and never consults the premium arm. The behavior is correct — `requests.cost` is an inert legacy field on post-cutover sessions, so treating `Some(0)` AIU as "AIU absent, fall through" would invent cost the user never owed — but it's the one place the "presence, not value" dispatch framing diverges from intuition, and no regression test pinned the behavior. Add a comment at the `has_aiu` definition documenting the presence-vs-value semantics, and add `auto_mode_some_zero_aiu_short_circuits_to_zero_over_nonzero_premium` to pin: row with `totalNanoAiu: 0` + `requests.cost: 4.0` (= $0.16 under the premium arm) bills $0.00 under Auto. Co-authored-by: Copilot <[email protected]>
…regate path External PR ccusage#1209 code review (Low) flagged a self-contradictory framing in the synthetic aggregate-credit comment: it described the aggregate AIU as "the only priced channel" for the tokens-only forward-compat shape, but in `--mode auto` the per-model row IS independently token-priced via LiteLLM. The two stack: AIU cost from the synthetic + token cost on the per-model row. Tighten the wording to "only source-provided priced signal," and explicitly call out: * The stack is accepted because the alternative (dropping the aggregate AIU) silently under-counts a real charge — worse failure mode than visible over-count. * `--mode display` does NOT have this stack (the per-model row bills $0 in Display when neither AIU nor cost is present). * The shape is unobserved in the local 200-file corpus. Mirror the same clarification in the `aggregate_credit_is_emitted_when_per_model_has_only_tokens` test comment so the rationale travels with the regression. Co-authored-by: Copilot <[email protected]>
… when warranted External PR ccusage#1209 code review (Nit) flagged that `warn_about_inert_legacy_env_vars_once` consumed the OnceLock BEFORE checking the `log_level == 0` and `in_use.is_empty()` guards. So the first `load_entries` of the process always burned the one-shot, even with no legacy env vars set. Harmless in practice (env vars are constant within a process), but the function name promises "warn-once," not "called-once," and a future caller pattern where the env vars get set mid-process would silently never warn. Reorder so the guards run first; only set the OnceLock once we've decided to actually emit the warning. Co-authored-by: Copilot <[email protected]>
…oat values + fix line-number rot External PR ccusage#1209 code review (recommended before merge): the `total_nano_aiu` doc-comment acknowledged that a future Copilot CLI emitting the field as a JSON float (e.g. `1.5e11` or `100000000000.0`) would fail the whole `ShutdownEvent` deserialize and drop every model in that shutdown — the exact failure class fixed for `requests.cost`. The previous fix only documented the future hardening; this commit actually performs it. Replace `#[serde(default)]` on both `ShutdownData::total_nano_aiu` and `ModelMetrics::total_nano_aiu` with `#[serde(default, deserialize_with = "deserialize_nano_aiu")]`. The new helper accepts BOTH integer and float JSON forms, truncates floats to `u64`, and rejects negative / non-finite / overflow values via serde errors (the whole event is then skipped via the existing tolerate-malformed-line path — preferable to silently coercing to zero and over-suppressing the aggregate-credit guard). Three regression tests pin the behavior: - `tolerates_float_valued_per_model_total_nano_aiu`: mixes a float-valued per-model AIU with an integer-cost sibling so both rows must survive (would have dropped the whole event under u64-only). - `tolerates_float_valued_aggregate_total_nano_aiu`: the synthetic aggregate-credit entry must still emit when the session-level `totalNanoAiu` is a float. - `rejects_negative_float_total_nano_aiu`: negative values surface as a deserialize error and drop the event (NOT coerced to zero). Also addresses the reviewer's Nit about brittle line-number references rotting: replace `loader.rs:64`, `summary.rs:56-57`, and `data-loader.ts:206-214` cites in parser.rs comments with stable symbol names (`TokenCounts::total`, `UsageAccumulator::add_entry`, `skipMetricsRow`). Co-authored-by: Copilot <[email protected]>
…rt + fix line-number rot External PR ccusage#1209 code review (Nit) flagged the `EnvScope` test scaffolding as a candidate for hoisting: the RAII helper for snapshot-and-restore of process env vars was defined identically (~28 lines each) in both `adapter/copilot/loader.rs` and `adapter/copilot/paths.rs` test modules. Two prior reviewers had also flagged it; with a third hit on the same nit, the duplication is now worth eliminating. Move `EnvScope` to `ccusage-test-support` with: - The same RAII shape (`new(overrides) -> Self`, `Drop` restores prior values via `set_var` / `remove_var`). - A clearer doc comment warning that callers must hold their own test mutex (we use `COPILOT_ENV_LOCK` in this repo) because `std::env::set_var` is not thread-safe. Update both copilot test modules to import the hoisted symbol (`use ccusage_test_support::EnvScope;`) and drop the local definitions. Also drops a now-unused `use std::env;` in loader.rs. Drive-by: replace one remaining brittle `summary.rs:82` line-number reference in a loader.rs assertion comment with the stable symbol name (`UsageAccumulator::add_entry` breakdown branch in `summary.rs`), finishing the line-number-rot cleanup started in cc48cad. Co-authored-by: Copilot <[email protected]>
…en-priced arm External PR ccusage#1209 code review (Nit): the token-priced arm of `usage_entry_to_loaded` (`loader.rs`, `Auto | Calculate` branch) forwarded the surrounding `mode` variable into `calculate_cost_for_usage` / `missing_pricing_model_for_usage`. The two values are equivalent today because `cost_usd: None` is hardcoded at the call site (Auto with `None` falls through to `calculate_cost_from_tokens` anyway), but the implicit coupling relies on a behavior of `calculate_cost_for_usage` that is one refactor away from drifting silently. Pass `CostMode::Calculate` explicitly to make the intent self-evident — this branch IS the token-priced fallback, so it wants token pricing unconditionally, regardless of how Auto handles `cost_usd: None` in the future. Same behavior today; clearer code and decoupled from `calculate_cost_for_usage`'s Auto semantics. Co-authored-by: Copilot <[email protected]>
…e comments External PR ccusage#1209 code review identified the single recommended maintainer gate before merge: strip references to ephemeral, private development artifacts ("plan finding #N", "GPT-5.5 R12/R14", "Piece B/C", "round-1/2 finding", "plan round-3 finding", "seq-7 round-1B") from shipped code. These were meaningful to the author during the iterative development + multi-model review process, but read as noise (or confusion) to a future maintainer six months out. Preserve the substance of every comment — what the code does and why — while dropping the development-internal identifiers: - `plan finding ccusage#10` → "Aggregate-only credits path:" - `plan finding from PR ccusage#957 + GPT-5.5 R12` → "Skip rule for per-model metrics rows:" (PR ccusage#957 citation kept — external + persistent) - `plan finding ccusage#7` → "in real session-state data" - `plan finding #2` → (dropped — comment is self-explanatory) - `plan finding for GPT-5.5 R14` → "Dedup key selection:" - `plan finding ccusage#3 / Piece C carry-over from PR ccusage#957` → "carried over from PR ccusage#957's TypeScript implementation" - `round-1/2 finding` → "documented invariant" - `// ----- Piece B (credits) -----` → "AI Credits tests (totalNanoAiu)" - `plan finding #1` → (dropped) - `plan finding ccusage#8` → (dropped — paraphrased into the comment) - `plan round-3 finding` → (dropped — paraphrased into the comment) - `mirrors the seq-7 round-1B parser fixture fix` → (dropped — the preceding sentences already explain the genuinely-absent-vs-Some(0) reasoning standalone) PR ccusage#957 references stay (external, persistent, useful for context). External symbol/test/file references all resolve. No behavioral change. Co-authored-by: Copilot <[email protected]>
… + UTF-8-safe claude_dot_to_dash External PR ccusage#1209 code review (two Low findings, both in parser.rs): 1. Per-row fs::metadata() in the timestamp-fallback path (`parse_session_state_file`): the unparseable-timestamp branch did a fresh `fs::metadata(path)` + `.modified()` syscall PER EVENT row. Real RFC3339 timestamps from the Copilot CLI always parse, so this is effectively never hit on observed data — but a file with many unparseable-timestamp events would do an fs syscall per row. The legacy OTel parser already solved this with a hoisted `file_modified_timestamp(path)`; the session-state path now reuses it via a `std::cell::OnceCell<TimestampMs>` that: * Computes the fallback ONCE per file on first need (not per row). * Stays free when the fallback never fires (the common case) — no upfront syscall on parse, no allocation. 2. `claude_dot_to_dash` was not UTF-8-safe: iterated `value.as_bytes()` and did `out.push(byte as char)`, which corrupts any multi-byte UTF-8 sequence (non-ASCII bytes get reinterpreted as U+0000..U+00FF chars instead of decoded as proper code points). The `claude-` gate at the only call site keeps real inputs ASCII today, so the bug is latent — but the helper itself must be UTF-8-safe for forward-compat (model identifiers could grow non-ASCII suffixes). Rewrite to iterate `char_indices()` and operate on `char` values directly. Same dot-between-ASCII-digits semantics, no byte manipulation. Regression test `claude_dot_to_dash_preserves_multi_byte_utf8_characters` pins three properties: * ASCII-digit dot still converts (`claude-opus-4.7` → `claude-opus-4-7`) * Multi-byte UTF-8 round-trips unchanged (`café`, é = 2 bytes) * Dot between digit and non-ASCII-digit (e.g. `日` — digit-shaped but not ASCII digit) is NOT converted Also drop stray untracked artifacts from working tree (test/test.rs/drain_test/drain_test.rs/diff.txt/.docs_diff.txt) that the external reviewer noted as housekeeping. Total tests: 352 pass (was 351, +1 UTF-8 regression). Co-authored-by: Copilot <[email protected]>
… into a single literal External PR ccusage#1209 code review (6th pass, Minor) flagged the `SESSION_SHUTDOWN_MARKER` / `SESSION_SHUTDOWN_TYPE` split as the one defensive instinct that added surface rather than removed it: two consts holding the same literal, defended by a ~25-line comment explicitly invoking 'inlining-then-deduping cleanup' as the threat the split guards against — which is exactly the cleanup this commit performs. Multiple prior internal reviewers (Gemini, others) also flagged it as 'over-engineered'; this is the cumulative tipping point that finally outweighs the defensive case. The reviewer's 'cannot desync' argument is decisive: with a single const used in both roles, a future schema rename (e.g. 'session.shutdown' → 'session.exit') updates ONE literal that flips BOTH the pre-filter and the exact-match together. The two-const split bought nothing the single const doesn't — it just added surface area where a refactor could drop one role. Collapse to a single `SESSION_SHUTDOWN_TYPE: &str`. Both the substring `str::contains` pre-filter (line 739) and the exact-match post-parse check (line 745) now reference it; behavior is unchanged. Replace the 25-line defense essay with a short comment documenting the dual role of the single const. Update the inline comment on `ShutdownEvent.event_type` to reference the surviving name. Also clean stray scratch files in working tree the reviewer noted (diff*.txt, full_diff.txt, test, test.rs, test2, test2.rs) so nothing accidentally gets staged. Total tests: 352 pass (unchanged — pure refactor + comment trim). Co-authored-by: Copilot <[email protected]>
… doesn't abort the report External PR ccusage#1209 code review (7th pass, Medium gating) flagged that `load_entries_inner` propagated any `parse_session_state_file` error via `?` — a single unreadable / locked / corrupted session file would abort the WHOLE report. Now that session-state is the always-on default path (per ccusage#1174), this changes the failure mode of `ccusage copilot daily` from 'graceful: show whatever sessions parsed' to 'all-or-nothing'. Real triggers: Windows file lock from antivirus scanning, partial-write JSONL from a still-running Copilot CLI, permission denied on a file owned by another user, EISDIR if a crash recovery left a directory where a file should be. The legacy OTel reader (`load_entries` in the OTel branch this PR deletes) treated parse errors as per-file skips, and every other ccusage adapter does the same. Match that semantic here. Wrap `parse_session_state_file(&path)` in `match` so: - Ok(entries) extends the working Vec (existing behavior) - Err(err) logs a one-line warning to stderr with the path and the underlying io::Error, then continues to the next file. Gated on `crate::log_level() != Some(0)` so LOG_LEVEL=0 silences (same gate as the rest of the adapter's warning surface). Add `one_unreadable_session_file_does_not_abort_whole_report` regression test: fixture has one good session + one bad path (a directory at the events.jsonl position, which makes `fs::File::open` return EISDIR). Before the fix `unwrap()` would panic on the Err propagation; after the fix the good session's entry survives. Co-authored-by: Copilot <[email protected]>
External PR ccusage#1209 code review (7th pass, optional Nit) flagged that the UTF-8-safe `claude_dot_to_dash` (rewritten in 506a612) built a transient `Vec<char>` for index-based lookahead. The collection is discarded after the function returns — a per-call O(n) heap allocation that fires on every Claude pricing lookup (every Auto/ Calculate mode entry for any `claude-*` model). Switch to a streaming impl: walk `value.chars().peekable()` with a single-char lookahead (`Peekable::peek`) and a one-char trailing memory (`prev_ch`). No intermediate Vec. Same semantics — dot between two ASCII digits → dash, everything else preserved verbatim. Multi-byte UTF-8 round-trip (`café`, `日`) still pinned by `claude_dot_to_dash_preserves_multi_byte_utf8_characters`. Total tests: 353 pass (was 353; pure refactor, no test count change). Co-authored-by: Copilot <[email protected]>
…pression guard External PR ccusage#1209 code review (8th pass, Low — likely-unreachable robustness nit) flagged that `any_per_model_priced_billing` in `parse_session_state_file` iterates `event.data.model_metrics.values()` — including rows that the immediately-above loop discards via `skip_metrics_row`. A per-model row with `cost > 0` but `requests.count == 0` AND zero tokens AND no per-model AIU would satisfy the skip predicate (no entry pushed) yet still suppress the synthetic aggregate-credit row — silently losing the session AIU. This shape is unreachable on observed data because `requests.cost = count × multiplier` makes `count == 0 ⇒ cost == 0`. But the guard's correctness shouldn't depend on that schema invariant — if the CLI ever emits this shape, the aggregate must survive. Symmetric belt-and-suspenders fix. Switch the guard from `.values()` to `.iter()` and AND with `!skip_metrics_row(model, m)` so the predicate considers only the rows that were actually pushed to entries. Equivalent today (on observed data the skip filter never excludes a priced row), but removes the 'what was actually emitted vs. what the raw map shows' inconsistency permanently. Regression test `aggregate_credit_guard_ignores_skipped_per_model_rows` pins the unreachable-today case: - Per-model row: 0 tokens, count=0, cost=7.5, no per-model AIU (would be skipped → no entry pushed) - Aggregate: totalNanoAiu=1_000_000_000 (the only priced signal) - Expected: per-model row absent from entries, synthetic aggregate-credit row IS present Verified locally: the test FAILS against the pre-fix `.values()` guard (synthetic dropped silently) and PASSES against the fix. Total tests: 354 pass (was 353, +1 new regression). Comment block updated to call out the EMITTED-vs-raw-map distinction. Co-authored-by: Copilot <[email protected]>
…parity with nano_aiu deserializer External PR ccusage#1209 code review (9th pass, prioritized #1) flagged that `requests.cost` was deserialized via serde's default `Option<f64>` while `totalNanoAiu` had a defensive custom deserializer (`deserialize_nano_aiu`) rejecting NaN / ±Infinity / negative values. The asymmetry meant a future Copilot CLI emitting a negative `cost` would silently propagate `-1.5 × $0.04 = -$0.06` into the billing total, making the summary appear smaller than reality — strictly worse than dropping the single malformed row. Add `deserialize_premium_request_cost`, structurally identical to `deserialize_nano_aiu`: accepts int and float JSON forms, rejects non-finite (NaN, ±Infinity) and negative values via serde errors. On rejection the whole event drops via the existing tolerate-malformed-line path — preferable to coercing through into the billing total. Regression test `rejects_negative_premium_request_cost` verified to FAIL against the pre-fix default deserializer (which accepts -1.5 and propagates) and PASS against the new guard. NOTE on NaN testing: standard JSON doesn't allow `NaN` / `Infinity` literal tokens — `serde_json` rejects them at parse time before our deserializer runs. A NaN regression test would therefore be tautological (line-skip via JSON parse failure, not via our guard). The `is_finite()` check is kept as defense-in-depth against any future caller constructing the struct from a non-JSON source. The comment in the deserializer documents this. Total tests: 355 pass (was 354, +1 negative-cost regression). Co-authored-by: Copilot <[email protected]>
External PR ccusage#1209 code review (9th pass, prioritized #2) noted that the generic `shared_claude_options` --mode help entry lists `api` as a choice but doesn't explain it ('Cost calculation mode'). The copilot-scoped --mode entry already documents it (added in f49a0c3), but the Claude/shared help didn't — users running `ccusage claude daily --help` see the `api` choice with no explanation. Update the shared description to match the parity hint: 'Cost calculation mode (api is an alias for calculate)'. Keeps the description short while making the alias discoverable wherever --mode appears. Co-authored-by: Copilot <[email protected]>
…it-content-dedup designs External PR ccusage#1209 code review (10th pass, only actionable item) flagged that token-bearing rows and credit-only rows encode opposite beliefs about duplicate shutdowns: token rows trust event ids (distinct events = distinct charges, sum them), while credit-only rows distrust event ids (distinct events with same naiu = same snapshot, collapse them). Reviewer asked for a doc reconciliation explaining why the asymmetry is deliberate. Replace the brief 'Resumed sessions' section with a longer 'Resumed sessions and dedup-key design' section that: - Names the two key shapes explicitly (`shutdown:{session_id}:{event_id}:{model}` vs `credit-shutdown:{session_id}:{model}:{naiu}`). - Documents WHY token rows trust event ids: per-process snapshots commonly produce near-identical counts when a user resumes a similar workload; event-id keying prevents silent under-count. - Documents WHY credit-only rows distrust event ids: real-data ships them in paired snapshots with distinct event ids but identical naiu (1135875b-… / 76dc438a-… pattern; ~$2.65 of credits would be double-billed in the local corpus by event-id keying). - Names the matching pinning tests for both paths so a future reader has the contracts at hand. - Calls out the residual risk: if a future Copilot CLI ships GENUINE non-coincidental duplicate event ids on credit-only rows, the over-suppression would surface as a visible under-bill in `credits` JSON output (the soft signal) rather than silently doubling the bill. Also explains the synthetic aggregate-credit key (`credit-aggregate:{session_id}:{nano_aiu}`) mirrors the same content-based 'paired snapshot' semantics for consistency. No code change — pure design-rationale documentation. Co-authored-by: Copilot <[email protected]>
…ly rows with same naiu but distinct event ids External PR ccusage#1209 code review (10th pass) asked for a test 'locking the chosen semantics' for two credit-only rows in one session with identical `totalNanoAiu` across distinct event ids that COULD be intended as distinct charges. The existing dedup test (`auto_mode_dedupes_duplicate_credit_only_rows_within_same_session`) pins the same shape but frames it as 'collapse the duplicate pair'; the reviewer wanted a sibling test that explicitly documents the deliberate trade-off for the genuine-distinct-charges case. Add `credit_only_rows_with_identical_naiu_but_distinct_event_ids_are_treated_as_duplicates`: - Fixture: two shutdowns a full day apart (`evt-genuine-charge-a` at T+0, `evt-genuine-charge-b` at T+24h) — explicitly framed as potentially distinct charges, not paired snapshots. - Same model, same naiu (154_481_000_000 → $1.54481). - Assertion: loader collapses both to ONE entry billed at $1.54481, NOT $3.08962. This is the deliberate dedup choice — the alternative (event-id-based keying) would silently double-bill every real-data paired-snapshot pair, which is the failure mode the design optimises against. The test message cross-references the new README section ('Resumed sessions and dedup-key design', commit f80c972) so a future maintainer who hits the assertion has the rationale at hand. Test named to make the semantic explicit in grep output. Total tests: 357 pass (was 356, +1 design-locking pin). Co-authored-by: Copilot <[email protected]>
… clarify metadata reachability External review of PR ccusage#1209 raised two non-blocking items both worth addressing for clarity. The third item (double iteration in `parse_session_state_file`) was explicitly marked "not worth changing now" by the reviewer and is skipped. The fourth item (process: mark PR ready-for-review) is a user decision. ## Issue 1: lock down cross-agent behavior of `summary_rows` relaxation The r2 fix relaxed `summary_rows` from `total_tokens == 0 → drop` to `total_tokens == 0 && total_cost == 0.0 && credits.unwrap_or(0.0) == 0.0 → drop`. The reviewer (correctly) noted this is a cross-agent behavior change: pre-fix, any non-Copilot adapter producing a zero-token + positive-cost summary (e.g. Claude `costUSD` set on an event whose token counts didn't aggregate, or Pi/OpenCode/ OpenClaw/Hermes with their precomputed `cost_usd: Some(positive)`) had its row silently dropped from `ccusage daily --all` totals. The new behavior surfaces these rows uniformly. The existing test `summary_rows_keeps_zero_token_summaries_when_cost_or_credits_present` DID already cover a `cost_only_no_credits` shape (marked "codebuff-shape forward-compat"), but used `summary_rows("copilot", ...)` as the agent label, making the cross-agent coverage implicit. Added new test `summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents` that drives the SAME shape (zero tokens + positive cost + no credits) through `summary_rows` with three different non-Copilot agent labels — claude, pi, opencode — and asserts all three survive. A companion loop asserts the still-fully-empty row drops uniformly across the same agents (phantom-row guard fires cross-agent). This makes the cross-agent applicability explicit and catches any future regression that re-introduces an agent-discriminating filter. The existing comment in `summary_rows` (loader.rs:578-593) already documented the cross-agent intent ("which is the right behavior for any adapter that bills outside the token channel, not just Copilot") — the new test pins that documented intent at the test boundary. ## Issue 2: clarify reachability of `build_row_metadata`'s metadata-copy branch The reviewer noted that `row.metadata` copy at `report.rs:67-71` "never reaches JSON via this path" because every row reaching `row_json` has been through `AllAccumulator::into_row` which sets `metadata: None`. That diagnosis is true for non-session modes (daily/weekly/monthly), but NOT for session mode — `load_rows` at `loader.rs:240-249` returns session-mode rows directly without calling `aggregate_rows`, so per-session per-agent rows preserve the loader-side `metadata = Some({lastActivity, projectPath, ...})` produced by `summary_metadata` / `codex_group_row`, and the copy branch DOES fire and surface those fields in `session --all --json`. Expanded the `build_row_metadata` doc-comment to make this asymmetry explicit, walking both paths and naming the exact code sites. The branch is correctly kept active for both rendering paths so any future surface that renders nested or non-aggregated AllRows automatically preserves loader-side metadata. ## Validation at this tip - `cd rust && cargo fmt --check` — clean - `cd rust && cargo clippy --workspace --all-targets -- -D warnings` — clean - `cd rust && cargo test --workspace` — 368 tests pass (was 367, +1 new cross-agent regression test) Refs ccusage#1174. Co-authored-by: Copilot <[email protected]>
…omment
Opus 4.7-xhigh (seq-18 r1f) caught a stale doc-comment value: the
header for `accepts_fractional_premium_request_cost`
(`parser.rs:1194`) says the fixture mixes "a fractional row
(Opus 4.7, cost 7.5) with an integer-cost sibling (Sonnet, cost 4)",
but the actual fixture (`parser.rs:1221`) sets the Sonnet sibling
to cost 1 ("1 request × 1× multiplier = cost 1"), the inline
comment at `parser.rs:1219-1220` says "1× multiplier ... so 1
request × 1× = cost 1", and the assertion at `parser.rs:1240`
checks `Some(1.0)`.
Reviewer's git archaeology: the fixture was introduced as `cost: 4`
in `20b20ca` together with the matching "Sonnet, cost 4" header.
A later commit (`cc48cad`) renumbered the fixture to `cost: 1`
and added the "1× multiplier" inline comment, but the older
header line was not updated alongside it. Both commits are in this
PR's history.
## Fix
Reworded the header parenthetical from "(Sonnet, cost 4)" to
"(Sonnet 4.5, cost 1 — 1 request × 1× multiplier)". This:
- Matches the fixture (cost 1).
- Matches the inline comment's multiplier explanation.
- Names the specific Sonnet variant (4.5) so a reader doesn't
have to scroll to the fixture to disambiguate.
## Validation at this tip
- `cd rust && cargo fmt --check` — clean
- `cd rust && cargo clippy --workspace --all-targets -- -D warnings` — clean
- `cd rust && cargo test --workspace` — 368 tests pass (unchanged;
doc-comment-only change)
## Out-of-scope finding from same round (NOT addressed by this commit)
GPT-5.5 (seq-18 r1f) flagged a HIGH about `codex/aggregate.rs:347`
double-counting branch-history events because the aggregate dedupe
key includes `session_id` while the loader dedupe intentionally
removed it. `git blame` confirms that file was last touched by
upstream PRs ccusage#1158 (eef2543) and ccusage#1137 (41c0c6f) and has NOT been
touched by PR ccusage#1209. `git log upstream/main..HEAD -- rust/crates/ccusage/src/adapter/codex/`
returns zero commits from this PR.
That bug is pre-existing in the Codex adapter and unrelated to PR
ccusage#1209's scope (Copilot adapter + cross-source --all credits
handling). Per repo guidelines ("Don't fix pre-existing issues
unrelated to your task ... unless they're tightly coupled to the
code you're changing"), it's out of scope here and should be filed
as a separate issue against the Codex adapter or fixed in a
follow-up PR. The seq-17 / seq-18 review framework happened to
surface it, but addressing it in this branch would expand scope.
Refs ccusage#1174.
Co-authored-by: Copilot <[email protected]>
…ead ref External review ccusage#15 (nit a) flagged three remaining multi-paragraph comment blocks as obvious targets for the comment-trim pattern this branch has been applying (b529041, 0cbfeac). Nit d flagged the test-internal `ccusage#1209-r1q` review-thread reference as meaningless post-squash. Trims (load-bearing rationale retained; narrative prose dropped): * adapter/all/loader.rs `summary_rows` filter rationale: 16->8 lines. Kept the relaxed-predicate invariant + exact-zero safety claim; dropped the `session.shutdown` example narrative and the `unwrap_or(0.0)/total_cost = 0.0` source trace. * adapter/all/types.rs `AllRow.credits` doc: 19->9 lines. Kept the byte-parity claim (Option::is_some per-row, SUM > 0.0 totals) and the `None` vs `Some(0.0)` distinction; dropped the metadata side-channel essay. * adapter/copilot/loader.rs mode-dispatch table narrative: 25->17 lines. Kept the load-bearing table verbatim, the field-presence rationale, the README pointer, the `Some(0.0)` semantics, and the synthetic-row bypass pointer; dropped the duplicated "Auto/Display bill those as a genuine $0" rephrasing and the `parse_session_state_file` provenance prose. Scrubbed: * adapter/all/loader.rs `summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents` test comment dropped the `External review ccusage#1209-r1q` / `r1q-r1 follow-up` references. The behavior they document is identical; only the transient review-thread provenance is gone. Validation: * `cargo fmt --check` clean. * `cargo clippy --workspace --all-targets -- -D warnings` clean. * `cargo test --workspace`: 364 tests pass, 0 failed (unchanged). Diff: 3 files changed, +23 / -50 lines (-27 net). Co-authored-by: Copilot <[email protected]>
…ead ref External review ccusage#15 (nit a) flagged three remaining multi-paragraph comment blocks as obvious targets for the comment-trim pattern this branch has been applying (b529041, 0cbfeac). Nit d flagged the test-internal `ccusage#1209-r1q` review-thread reference as meaningless post-squash. Trims (load-bearing rationale retained; narrative prose dropped): * adapter/all/loader.rs `summary_rows` filter rationale: 16->8 lines. Kept the relaxed-predicate invariant + exact-zero safety claim; dropped the `session.shutdown` example narrative and the `unwrap_or(0.0)/total_cost = 0.0` source trace. * adapter/all/types.rs `AllRow.credits` doc: 19->9 lines. Kept the byte-parity claim (Option::is_some per-row, SUM > 0.0 totals) and the `None` vs `Some(0.0)` distinction; dropped the metadata side-channel essay. * adapter/copilot/loader.rs mode-dispatch table narrative: 25->17 lines. Kept the load-bearing table verbatim, the field-presence rationale, the README pointer, the `Some(0.0)` semantics, and the synthetic-row bypass pointer; dropped the duplicated "Auto/Display bill those as a genuine $0" rephrasing and the `parse_session_state_file` provenance prose. Scrubbed: * adapter/all/loader.rs `summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents` test comment dropped the `External review ccusage#1209-r1q` / `r1q-r1 follow-up` references. The behavior they document is identical; only the transient review-thread provenance is gone. Validation: * `cargo fmt --check` clean. * `cargo clippy --workspace --all-targets -- -D warnings` clean. * `cargo test --workspace`: 364 tests pass, 0 failed (unchanged). Diff: 3 files changed, +23 / -50 lines (-27 net). Co-authored-by: Copilot <[email protected]>
225d38e to
f9fcb0d
Compare
…ead ref External review ccusage#15 (nit a) flagged three remaining multi-paragraph comment blocks as obvious targets for the comment-trim pattern this branch has been applying (b529041, 0cbfeac). Nit d flagged the test-internal `ccusage#1209-r1q` review-thread reference as meaningless post-squash. Trims (load-bearing rationale retained; narrative prose dropped): * adapter/all/loader.rs `summary_rows` filter rationale: 16->8 lines. Kept the relaxed-predicate invariant + exact-zero safety claim; dropped the `session.shutdown` example narrative and the `unwrap_or(0.0)/total_cost = 0.0` source trace. * adapter/all/types.rs `AllRow.credits` doc: 19->9 lines. Kept the byte-parity claim (Option::is_some per-row, SUM > 0.0 totals) and the `None` vs `Some(0.0)` distinction; dropped the metadata side-channel essay. * adapter/copilot/loader.rs mode-dispatch table narrative: 25->17 lines. Kept the load-bearing table verbatim, the field-presence rationale, the README pointer, the `Some(0.0)` semantics, and the synthetic-row bypass pointer; dropped the duplicated "Auto/Display bill those as a genuine $0" rephrasing and the `parse_session_state_file` provenance prose. Scrubbed: * adapter/all/loader.rs `summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents` test comment dropped the `External review ccusage#1209-r1q` / `r1q-r1 follow-up` references. The behavior they document is identical; only the transient review-thread provenance is gone. Validation: * `cargo fmt --check` clean. * `cargo clippy --workspace --all-targets -- -D warnings` clean. * `cargo test --workspace`: 364 tests pass, 0 failed (unchanged). Diff: 3 files changed, +23 / -50 lines (-27 net). Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
7 issues found across 27 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rust/crates/ccusage/src/adapter/copilot/mod.rs">
<violation number="1" location="rust/crates/ccusage/src/adapter/copilot/mod.rs:35">
P3: `has_data()` performs a full file enumeration/sort for a boolean check, adding avoidable filesystem work to Copilot detection.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — This run re-reviews PR #1209 after the author restructured the commit history (78 review-iteration commits force-pushed to 5 atomic conventional commits). The aggregate diff is unchanged from the prior Pullfrog review — the same +6140/-831 across the same 27 files, same behavior, same code. The restructuring introduced no new code changes or behavioral differences.
- Restructured commit history — 78 test-iteration commits consolidated into 5 atomic commits. The code and diff per file are identical to the prior version at 5844c6a.
Big Pickle (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Incremental re-review after the author force-pushed 751203f with code improvements atop the same 5-commit structure. The tree changed from the prior Pullfrog-approved version (791b377) in 6 files, all improvements addressing prior feedback or hardening test infrastructure.
unwrap_or(0.0)→expect(...)—credit_costandpremium_costnow useexpectwith descriptive messages matching theirhas_aiu/has_premium_dataguards, resolving the prior Pullfrog nitpickhas_data()short-circuit — newhas_any_session_state_event_file()replaces full directory enumeration with a first-hit probe, reducing filesystem work in--alldetection- Poison-recovering test lock —
acquire_env_test_lock()wrapsENV_TEST_LOCKwithinto_inner()poison recovery so a panic in one env-mutating test does not cascade-fail every subsequent env test trim_os_stringUTF-8 fast path — Unix path trimming now usesstr::trim()for the UTF-8 common case before falling back to ASCII byte trimming- Test cleanup — removed redundant
.clone()inall/tests.rs
Big Pickle (free via Pullfrog for OSS) | 𝕏
|
@ryoppippi would appreciate your review on this PR |
|
please push, this is useful |
|
PR has been ready for a very long time. It now has developed conflicts as well, the maintainer pushing other changes, before merging/approving this. As soon as @ryoppippi gives me the go-ahead, I can resolve the conflicts and this can be merged. |
|
ok i'll take a look @pullfrog fix conflicts |
|
Resolved all merge conflicts for #1209 against Task list (5/5 completed)
|
|
I resolved the merge conflicts against Conflicts resolved
Verification
Push blockerI cannot push the resolved branch: both the fork ( I’ve attached a ready-to-apply patch (diff from git fetch origin main
git checkout feat/copilot-session-state-reader
git rebase origin/main
# if conflicts remain, or on a clean branch:
git apply --stat resolved_pr.patch
git apply resolved_pr.patch
|
@jay-tau I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust Copilot adapter so ccusage copilot works out-of-the-box by ingesting Copilot CLI’s default per-session session-state/<sessionId>/events.jsonl stream, and extends cross-source aggregation to carry first-class credits while preserving correct cost semantics across Copilot’s June 1, 2026 billing-model cutover.
Changes:
- Switch Copilot ingestion from OTel exports to session-state
events.jsonl, including legacy-env-var deprecation warning behavior and billing-field-aware cost dispatch. - Add a global
--mode apialias (config + CLI + schema + docs) that maps tocalculate(LiteLLM token pricing). - Improve
--allaggregation to retain zero-token rows when cost/credits are present and to aggregate/surface credits consistently in JSON and totals.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/crates/ccusage/src/snapshots/ccusage__config_schema__tests__snapshots_schema_agent_specific_option_edges.snap | Snapshot updates to reflect new "api" cost-mode enum value in schema edges. |
| rust/crates/ccusage/src/config.rs | Map ConfigCostMode::Api to CostMode::Calculate. |
| rust/crates/ccusage/src/config_schema.rs | Add ConfigCostMode::Api schema variant (lowercase) as a discoverability alias. |
| rust/crates/ccusage/src/adapter/copilot/README.md | New adapter-internal documentation for session-state parsing, deduping, and cost-mode semantics. |
| rust/crates/ccusage/src/adapter/copilot/paths.rs | Discover session-state/*/events.jsonl, add COPILOT_CONFIG_DIR, and treat legacy OTel env vars as inert (warning-only). |
| rust/crates/ccusage/src/adapter/copilot/mod.rs | Add has_data() sentinel and one-shot legacy-env-var warning emission; update empty-state guidance. |
| rust/crates/ccusage/src/adapter/copilot/loader.rs | Implement session-state loading, pre-dedup date filtering, AI Credits / premium-requests billing logic, and LoadedEntry.credits. |
| rust/crates/ccusage/src/adapter/all/types.rs | Add first-class credits: Option<f64> to aggregated rows/accumulators and sum them across merges. |
| rust/crates/ccusage/src/adapter/all/tests.rs | Add tests for credits aggregation/rendering and for relaxed zero-token summary filtering behavior. |
| rust/crates/ccusage/src/adapter/all/report.rs | Centralize metadata construction, inject per-row credits, and gate totals.credits on positive sums. |
| rust/crates/ccusage/src/adapter/all/mod.rs | Expose additional test-only helpers needed by the expanded test suite. |
| rust/crates/ccusage/src/adapter/all/loader.rs | Route Copilot through a custom loader to preserve “detected” semantics; relax summary_rows filter; keep credits out of loader metadata. |
| rust/crates/ccusage-test-support/src/lib.rs | Extend EnvVarGuard to support setting/restoring multiple environment variables atomically for tests. |
| rust/crates/ccusage-cli/src/snapshots/ccusage_cli__tests__root_help.snap | Help snapshot updated to show --mode choices including api. |
| rust/crates/ccusage-cli/src/parser.rs | Accept --mode api as an alias for calculate and add parser tests. |
| rust/crates/ccusage-cli/src/help_codegen.rs | Include api in generated help choices. |
| rust/crates/ccusage-cli/src/cli-help.json | Document --mode including api globally and for Copilot-specific options. |
| rust/crates/ccusage-cli/src/cli-commands.json | Ensure Copilot subcommands expose the combined options that include mode docs/choices. |
| docs/guide/index.md | Update Copilot default data location to session-state and clarify COPILOT_CONFIG_DIR non-comma semantics. |
| docs/guide/getting-started.md | Update “no data” troubleshooting and custom dir guidance for Copilot session-state. |
| docs/guide/environment-variables.md | Replace OTel env var with COPILOT_CONFIG_DIR and clarify single-dir semantics. |
| docs/guide/cost-modes.md | Document api alias and clarify auto/display semantics across sources (including Copilot). |
| docs/guide/copilot/index.md | Rewrite Copilot guide for session-state ingest and explain billing-field-aware cost behavior and legacy env var warnings. |
| docs/guide/configuration.md | Update configuration examples to use COPILOT_CONFIG_DIR and clarify comma-splitting exceptions. |
| docs/guide/cli-options.md | Document --mode api and link to Cost Modes reference. |
| apps/ccusage/config-schema.json | Regenerated schema reflecting the new "api" enum choice (plus formatting ripples). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✅ Action performedReview finished.
|
@jay-tau I have started the AI code review. It will take a few minutes to complete. |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/guide/environment-variables.md`:
- Line 7: Update the introductory text to identify both COPILOT_CONFIG_DIR and
GOOSE_PATH_ROOT as exceptions to comma-separated directory handling, while
preserving the explanation that COPILOT_CONFIG_DIR uses single-directory
semantics and reads session-state/ beneath the configured path. Align the
wording with the existing guidance near GOOSE_PATH_ROOT and line 47.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2b9b492-9b5d-4c45-adae-2e8b67000091
⛔ Files ignored due to path filters (2)
rust/crates/ccusage-cli/src/snapshots/ccusage_cli__tests__root_help.snapis excluded by!**/*.snaprust/crates/ccusage/src/snapshots/ccusage__config_schema__tests__snapshots_schema_agent_specific_option_edges.snapis excluded by!**/*.snap
📒 Files selected for processing (25)
apps/ccusage/config-schema.jsondocs/guide/cli-options.mddocs/guide/configuration.mddocs/guide/copilot/index.mddocs/guide/cost-modes.mddocs/guide/environment-variables.mddocs/guide/getting-started.mddocs/guide/index.mdrust/crates/ccusage-cli/src/cli-commands.jsonrust/crates/ccusage-cli/src/cli-help.jsonrust/crates/ccusage-cli/src/help_codegen.rsrust/crates/ccusage-cli/src/parser.rsrust/crates/ccusage-test-support/src/lib.rsrust/crates/ccusage/src/adapter/all/loader.rsrust/crates/ccusage/src/adapter/all/mod.rsrust/crates/ccusage/src/adapter/all/report.rsrust/crates/ccusage/src/adapter/all/tests.rsrust/crates/ccusage/src/adapter/all/types.rsrust/crates/ccusage/src/adapter/copilot/README.mdrust/crates/ccusage/src/adapter/copilot/loader.rsrust/crates/ccusage/src/adapter/copilot/mod.rsrust/crates/ccusage/src/adapter/copilot/parser.rsrust/crates/ccusage/src/adapter/copilot/paths.rsrust/crates/ccusage/src/config.rsrust/crates/ccusage/src/config_schema.rs
🚧 Files skipped from review as they are similar to previous changes (20)
- rust/crates/ccusage/src/adapter/all/mod.rs
- rust/crates/ccusage/src/config.rs
- rust/crates/ccusage-cli/src/cli-commands.json
- rust/crates/ccusage/src/config_schema.rs
- docs/guide/cli-options.md
- docs/guide/index.md
- rust/crates/ccusage-cli/src/parser.rs
- rust/crates/ccusage-cli/src/help_codegen.rs
- docs/guide/getting-started.md
- rust/crates/ccusage/src/adapter/copilot/README.md
- rust/crates/ccusage-cli/src/cli-help.json
- rust/crates/ccusage/src/adapter/all/report.rs
- rust/crates/ccusage/src/adapter/copilot/mod.rs
- rust/crates/ccusage/src/adapter/all/types.rs
- docs/guide/cost-modes.md
- docs/guide/copilot/index.md
- apps/ccusage/config-schema.json
- rust/crates/ccusage/src/adapter/all/tests.rs
- rust/crates/ccusage/src/adapter/copilot/paths.rs
- rust/crates/ccusage/src/adapter/copilot/loader.rs
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Incremental re-review after the author pushed ccb71aa with one performance improvement atop the 6-commit structure.
- Parallelized session-state file I/O — replaced the sequential
for path in session_state_paths()? { parse_session_state_file(&path) }loop inload_entries_innerwithread_files_parallel(&paths, shared.single_thread, parse_session_state_file). Theread_files_parallelutility preserves file order (backed by a dedicated order-preservation test), respects--single-thread, uses size-balanced chunking for even worker distribution, and returns results in the same order as the input path slice. Error handling is unchanged — thezip+matchloop emits the sameeprintln!warning for unreadable files.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
All reported issues were addressed across 27 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
|
@ryoppippi I incorporated Pullfrog’s conflict-resolution changes and pushed the resolved branch. The PR is now up to date with |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
rust/crates/ccusage/src/adapter/all/loader.rs:1600
- Same hermeticity issue as the test above:
copilot::load_entrieswill print a deprecation warning if COPILOT_OTEL_* legacy env vars are set in the developer environment. Clear them in this EnvVarGuard scope to keep tests quiet and deterministic.
let fixture = ccusage_test_support::Fixture::new();
let _env =
ccusage_test_support::EnvVarGuard::set(copilot::COPILOT_CONFIG_DIR_ENV, fixture.root());
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — One commit atop the prior Pullfrog-reviewed state (a6ca738) fixes precision loss when converting large u64 nano-AIU values to f64 credits, covering sessions exceeding ~$90K in AI credit spend.
- Preserve fractional AIU precision for large values — extracted
nano_aiu_to_creditswith an integer-division + remainder split for values exceedingf64's exact integer range (2^53≈ 9 quadrillion nano-AIU), and a direct cast for the common small-value path. Pinned bynano_aiu_conversion_preserves_fractional_precision.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
…onl with billing-field-aware dispatch Closes ccusage#1174. Replaces the opt-in OTel file-export reader (`~/.copilot/otel/*.jsonl`, empty unless the user explicitly enables OpenTelemetry) with the per-session event stream the Copilot CLI now writes by default at `~/.copilot/session-state/<sessionId>/events.jsonl`. After this commit `ccusage copilot daily` works without any extra setup on every Copilot CLI install that ships the session-state schema. Bills correctly across GitHub's June 1, 2026 cutover from premium-request billing to AI Credits billing. The mode dispatch is **billing-field-aware (not date-based)** — the loader keys off `has_aiu` / `has_premium_data` rather than `entry.timestamp`, so backfills and future billing channels route correctly without code changes: | `--mode` | AIU present | AIU absent, premium present | AIU absent, premium absent | | --------------------- | --------------- | --------------------------- | -------------------------- | | `auto` *(default)* | credits × $0.01 | requests.cost × $0.04 | token-priced (LiteLLM) | | `display` | credits × $0.01 | requests.cost × $0.04 | 0 | | `calculate` | token-priced | token-priced | token-priced | Highlights: * **Tolerant deserializers** for `requests.cost` (`Option<f64>`, accept int+float, reject NaN/±Inf/negative). The previous u64 type silently dropped fractional values like Opus 4.7's 7.5×-multiplier rows — recovering ~73M tokens / $6.68 of real local data in this fix alone. * **Content-based dedup keys for credit-only rows** (`credit-shutdown:{session_id}:{model}:{naiu}`, event_id deliberately omitted) so paired AIU snapshots with distinct event_ids collapse rather than double-counting. Token-bearing rows keep the event_id key for the opposite reason — coincidental count-collisions across resumes must not collapse to one row. See adapter README "Resumed sessions and dedup-key design". * **Date filter runs BEFORE the HashMap dedup collapse** to prevent an out-of-range credit-only twin from evicting an in-range row in `HashMap::insert` last-wins. Pinned by `date_filter_runs_before_credit_only_dedup_collapse_to_preserve_in_range_row`. * **Per-file error isolation** so one locked / corrupted / partially-written session-state file does not abort the whole report. Tested with a Unix `chmod 0o000` fixture that genuinely exercises the `File::open` error path. * **Streaming `BufReader::split(b'\n')`** parser with bounded memory on 69MB+ files heavy users accumulate. Substring pre-filter on `SESSION_SHUTDOWN_TYPE` short-circuits non-shutdown lines before serde. * **Non-UTF-8 safe path/env handling** via `var_os` (not `var`), override-is-authoritative semantics (no silent fallback to `~/.copilot` when `COPILOT_CONFIG_DIR` is set). * **Legacy OTel env-var deprecation warning** — users who had `COPILOT_OTEL_FILE_EXPORTER_PATH` / `COPILOT_OTEL_DEDUP` / `COPILOT_PREFER_OTEL` set get a one-shot stderr notice that they're now inert. Process-wide `OnceLock` keeps it to one emission. `LOG_LEVEL=0` silences. The legacy OTel parser is fully removed; the `otel/` directory is intentionally not enumerated even when present on disk (pinned by `ignores_otel_directory_even_when_present`). CLI wiring: adds `copilot_options` block with a Copilot-specific `--mode` description that names the billing fields. Wires `copilot_combined_options` into the `copilot daily|monthly|session` subcommands. Adapter README (`adapter/copilot/README.md`) documents the session-state schema, token mapping, reasoning-token rules, skip rule, dedup-key design, date-filter-before-dedup invariant, cost-mode dispatch table, and env-var deprecation. Co-authored-by: Copilot <[email protected]>
… env-var changes
Updates the user-facing documentation guides to match the rewritten
Copilot adapter. Paired with the preceding code-only feature commit
to keep the source change reviewable in isolation.
* `docs/guide/copilot/index.md` — rewrites the Copilot guide to
describe the session-state source (no opt-in OTel setup required),
the billing-field-aware `--mode auto` dispatch, the `COPILOT_CONFIG_DIR`
override, the empty-state message, and the legacy OTel env-var
deprecation warning.
* `docs/guide/index.md` — Copilot row in the "supported sources"
table updated to point at the session-state path; adds a note that
`COPILOT_CONFIG_DIR` does not split on commas.
* `docs/guide/getting-started.md` — troubleshooting and "Custom Data
Directory" sections updated to the session-state path. Also fixes
the comma-separated-paths example: `GOOSE_PATH_ROOT` was incorrectly
shown as multi-directory but `goose/paths.rs` reads it via plain
`env::var` + `PathBuf::from` (no `.split(',')`), so it actually
mirrors `COPILOT_CONFIG_DIR`'s single-directory semantics. Updated
the exception clause and example accordingly.
* `docs/guide/configuration.md` — replaces the
`COPILOT_OTEL_FILE_EXPORTER_PATH` example with `COPILOT_CONFIG_DIR`.
* `docs/guide/environment-variables.md` — replaces the
`COPILOT_OTEL_*` rows with `COPILOT_CONFIG_DIR`; updates the
multi-directory example and exception clause to also exclude
`GOOSE_PATH_ROOT` (same single-directory fix as getting-started).
* `docs/guide/cost-modes.md` — generalizes Claude-centric prose so
the page covers Copilot's AI Credits / premium-request dispatch
alongside Claude's `costUSD`. Adds a billing-field-aware sub-table
under `auto` describing the AIU/premium/token-priced ladder.
Co-authored-by: Copilot <[email protected]>
Adds `api` as a discoverability alias for `calculate` across every agent subcommand and as a global `--mode api` flag. Both names map to the same `CostMode::Calculate` — token-priced billing via LiteLLM. No new code path; this is purely surface-level nomenclature. Motivation: Copilot users want an obvious way to contrast `auto`'s "what GitHub billed" with the equivalent if they had gone direct to the provider's API. `--mode api` reveals that intent in `--help` without expanding the runtime `CostMode` enum. The alias is **global**, not Copilot-scoped: `--mode api` works for every agent (Claude, Codex, Amp, Copilot, etc.) and means the same thing everywhere — "compute from token counts via LiteLLM, never from any source-precomputed cost." The semantics generalize cleanly so exposing it globally is intentional rather than a leak from the Copilot adapter work. Plumbing: * `parse_cost_mode` (ccusage-cli/parser.rs) accepts `"api"` → maps to `CostMode::Calculate`. Case-sensitive; rejects the old `"api-equiv"` pre-rename spelling. * `ConfigCostMode::Api` (config_schema.rs) added so `mode: "api"` works in YAML/TOML config files. Mapped to `CostMode::Calculate` in the `ConfigCostMode → CostMode` `From` impl (config.rs). * `cli-help.json` advertises `"api"` in the global `all_agent_options` choices and in the Copilot-specific `copilot_options` choices. Both `--mode` descriptions mention the alias explicitly. * `help_codegen.rs` test fixture + assertion updated to include `"api"` in the rendered choices line. * `apps/ccusage/config-schema.json` regenerated via `pnpm run generate:schema` — adds `"api"` to every `mode` enum block across all agents and report kinds (~69 added `"api"` lines). The schemars `.snap` snapshot regenerates similarly (6 added `"api"` lines). The bulk of the JSON diff is oxfmt reflow because `main` predates the project's `.oxfmtrc.json` enum-multiline rule; reviewers should use `git diff -w`. Docs: * `docs/guide/cli-options.md` — adds `--mode api` to the `--mode` example block + cross-links to the cost-modes page. * `docs/guide/cost-modes.md` — labels `calculate` as "also aliased as `api`" in the overview, adds an `api` alias note + example to the `calculate` section. Pinned by `parse_cost_mode_api_aliases_calculate`, `parse_cost_mode_accepts_all_documented_values`, and `parse_cost_mode_rejects_unknown_values`. Co-authored-by: Copilot <[email protected]>
… --all aggregator
Plumbs Copilot's `totalNanoAiu`-derived credits through the
`ccusage daily --all` (and weekly / monthly / session) cross-source
aggregator as a first-class `credits: Option<f64>` field on
`AllRow` and `AllAccumulator`, so post-cutover AI-Credit-only days
no longer get silently dropped from `--all` totals.
Before this commit:
* `summary_rows` filter rejected any summary with `total_tokens == 0`,
which silently omitted Copilot's credit-only sessions
(`session.shutdown` events with zero tokens but positive
`totalNanoAiu` are real charges).
* `AllAccumulator::into_row` reset `metadata: None`, so credits
threaded through per-source loader metadata also got dropped on
aggregation.
After:
* **`summary_rows` filter relaxed** to drop only fully-empty
summaries (zero tokens AND zero cost AND zero/absent credits).
This is an **intentional cross-adapter behavior change** — it
also affects Claude (per-message `costUSD`) and Hermes
(`actual_cost`), which can produce zero-token + positive-cost
rows that were previously silently dropped from `--all`. The
invariant is pinned by
`summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents`.
Worth a release-notes line since it changes existing non-Copilot
totals.
* **`AllRow::credits: Option<f64>`** — first-class field threaded
through `AllAccumulator::add`, `into_row`, and `merge_agent_breakdown`.
`Option` preserves the absent-vs-explicit-zero channel
distinction at the row layer.
* **`build_row_metadata` + `totals_json` credit injection** — the
JSON renderer surfaces `metadata.credits` per row (gated on
`Option::is_some`) and `totals.credits` (gated on SUM > 0.0,
raw `json!` for `5.0`-not-`5` byte-parity with the direct
per-agent renderer at `output.rs::totals_json`).
* **`load_copilot_rows`** — wraps the Copilot loader to run the
date filter inside `copilot::load_entries` before passing rows
on (required because the credit-only dedup key is content-based
and omits event_id, so the standard post-collapse filter could
silently drop in-range twins of out-of-range collisions).
OR-s `copilot::has_data()` into `detected` so the on-disk
signal survives the pre-filter, mirroring `qwen::has_data()`.
* **`finalize_session_mode_rows` helper** extracted from the
inline `kind == Session` clear so the renderer-side invariant
("no `agents` key in session-mode --all --json output") is
pinned by a dedicated test that mutation-fails when the clear
is removed.
Pinned by `summary_rows_keeps_zero_token_summaries_when_cost_or_credits_present`,
`summary_rows_treats_zero_credits_same_as_absent_credits`,
`summary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents`,
`totals_json_credits_uses_float_representation_matching_direct_renderer`,
`totals_json_omits_credits_when_all_credits_sum_to_zero`,
`build_row_metadata_does_not_inject_agents_for_session_mode_rows`,
`build_row_metadata_injects_agents_for_aggregated_all_rows`,
`finalize_session_mode_rows_clears_metadata_agents`,
`load_copilot_rows_keeps_copilot_in_detected_when_date_filter_excludes_all_data`,
and `load_copilot_rows_reports_not_detected_when_no_session_state_files_exist`.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Force-push rebase from f4ff7d5 onto a newer main. The 4 commits (e73dadd through 2b388ae) carry the same code, same behavior, and same 418-passing test suite as the previously-reviewed version. The diff against origin/main shifted slightly (context-line changes from the rebase base) but the substantive code is unchanged.
- Rebase-only — no new code, no behavioral changes, no test changes. Same session-state parser, billing-field-aware dispatch, credits aggregation,
--mode apialias, and OTel removal.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Closes #1174.
Problem
On a fresh Copilot CLI install the adapter only reads
~/.copilot/otel/*.jsonl, which is empty unless the user has explicitly enabled OpenTelemetry file export.ccusage copilot dailytherefore prints "No usage data found" out of the box, and most users never get past that.What this PR does
Reads the per-session events the Copilot CLI now writes by default at
~/.copilot/session-state/<sessionId>/events.jsonl, and bills correctly across GitHub's June 1, 2026 cutover from premium-request billing to AI Credits billing.After this PR,
ccusage copilot dailyworks without any extra setup on every install since the session-state schema was introduced.Cost-mode surface
--modeauto(default)totalNanoAiuis present, typically post-cutover CLI ≥ 1.0.40) →requests.cost × $0.04(when onlyrequests.costis present, typically pre-cutover) → token-priced via LiteLLM (when neither billing field is recorded). Free-tier rows (requests.cost == 0) bill as $0. Dispatch is keyed on field presence (not the cutover date), so backfills and future billing channels route correctly without code changes.displaycalculate/apiapiis a global discoverability alias forcalculate.LoadedEntry.creditsis surfaced in every JSON row whenever the source carriestotalNanoAiu, so callers can read the raw credit count regardless of mode.Behavior changes outside the Copilot adapter
This PR's
--allcross-source aggregator improvements affect every adapter, not just Copilot. Flagging the user-observable changes explicitly:summary_rowsno longer drops zero-token summaries with positive cost or credits. Pre-fix any summary wheretotal_tokens == 0was silently dropped fromccusage daily --alltotals. Post-fix the predicate istotal_tokens == 0 && total_cost == 0.0 && credits.unwrap_or(0.0) == 0.0 → drop. This was a behavior change to make Copilot credit-only days visible in--all, but it also surfaces zero-token + positive-cost rows for any other adapter that can produce them — notably Claude (per-messagecostUSDset on events whose token counts don't aggregate) and Hermes (actual_cost/estimated_costchannel). Pi / OpenCode parsers drop zero-token entries upstream so they don't reach this filter, but Claude / Hermes do. The exact== 0.0compare is deliberate —total_costonly reaches0.0via upstream assignment, never float arithmetic — and the cross-agent invariant is pinned bysummary_rows_relaxed_filter_applies_uniformly_across_non_copilot_agents. Worth a release-notes line since it changes existing non-Copilot--alltotals.AllRow.creditsis now first-class. Aggregation sums credits across agents and days; JSON emitsmetadata.creditsper row andtotals.credits(gated on a positive sum, matching the direct per-agent renderer'sif credits > 0.0gate). Backward-compatible: agents that never report credits (Claude, Codex, Amp, …) see the same totals JSON shape they always did.--mode apiis a global alias. Available across every agent's subcommand and as a global--mode apiflag, not Copilot-scoped. The schema enum was extended; documented incost-modes.md.OTel removal scope
The production loader no longer reads
~/.copilot/otel/*.jsonlor honorsCOPILOT_OTEL_FILE_EXPORTER_PATH/COPILOT_OTEL_DEDUP/COPILOT_PREFER_OTEL. Two pinning tests inpaths.rsassert OTel directories and env vars are now inert.The legacy
parse_otel_fileparser and its ~500 lines of OTel-specific helpers + 4 tests were fully removed (commit dropped into squash). Follow-up issue #1208 was auto-closed by the contribution gate on 2026-06-05; the cleanup was folded in here rather than left orphaned.Retained intentionally (user-facing surface): the OTel env-var deprecation warning (
warn_about_inert_legacy_env_vars_once) and its tests, and theotel/directory ignore tests — so users upgrading with legacy env vars still set get a one-shot stderr notice that they're now inert.Empirical validation
Against my local
~/.copilot/session-state/(74 days of real Copilot CLI usage):--mode auto --offline: $6,480.33--mode api --offline: $7,561.16 (what the same usage would have cost via direct provider APIs)The auto vs api gap is visible: Copilot's subscription is cheaper than equivalent direct API spend for this workload, which is what most users want to see.
Test changes
adapter::copilot::*(post-OTel-test-removal: net +46 vs base, since the 4 OTel parser tests inloader.rswere dropped with the parser itself).adapter::all::*covering credits aggregation,--allbyte-parity with direct renderer, gating semantics, and the cross-agentsummary_rowsrelaxation.requests.cost, absent-vs-explicit-zerorequests.cost(forward-compat), cross-session dedup-key collisions, credit-only row content-based dedup, era dispatch across all(AIU, premium, model)permutations, OTel env vars /otel/dir being inert,COPILOT_CONFIG_DIRno-fallback semantics, date-filter-before-credit-only-dedup ordering, cross-agent zero-token + positive-cost survival, andtotals.creditsJSON byte-parity ("credits":5.0not"credits":5).CI gates verified locally
cargo clippy --workspace --all-targets -- -D warnings: cleancargo fmt --check: cleancargo test --workspace: 418 passed, 0 failedpnpm run formatandpnpm typecheckrequire Nix dev-shell tools (oxfmt,tsgo) that aren't available outside the dev shell — CI will run them.Note on
apps/ccusage/config-schema.jsondiff sizeThe schema diff is ~894 lines but the only semantic change is a single config-schema enum addition (
ConfigCostMode::Apivariant inconfig_schema.rs, mapped toCostMode::Calculateinconfig.rs) — the schema regenerator (pnpm run generate:schema) ripples that into everymodeenum block across all agents/report kinds (~69 added"api"lines), and oxfmt also reflows the file becausemainpredates the project's.oxfmtrc.jsonenum-multiline rule (e849fac chore(oxlint,oxfmt): add json schema). Reviewers should usegit diff -w; the schemars.snapsnapshot diff shows the true scope (6 added"api"lines in the snapshot, all derived from the same enum addition).Commit history
5 atomic conventional commits (restructured from 78 review-iteration commits). The squashed-from history included findings that recovered ~73M tokens / $6.68 of real-world data silently dropped by an over-strict
u64deserialization ofrequests.cost, plus fixes for date-filter-before-dedup ordering, cross-agent credits aggregation,--allJSON byte-parity, the OTel dead-code removal, and multiple documentation-accuracy passes.If the squash-merge is the intended path, the per-commit narrative isn't load-bearing — the aggregate diff stands on its own. If you want a per-commit walkthrough at review time, I can pin a follow-up comment with the curated logical sequence.
Marking as draft for maintainer review pace; happy to mark ready and iterate on any feedback.
Summary by CodeRabbit
New Features
apicost mode as an alias forcalculate, including CLI/config/schema support.events.jsonlfrom the configured base directory.Bug Fixes
Documentation