Skip to content

feat(adapter/copilot): read session-state events.jsonl as default data source - #1209

Open
jay-tau wants to merge 4 commits into
ccusage:mainfrom
jay-tau:feat/copilot-session-state-reader
Open

feat(adapter/copilot): read session-state events.jsonl as default data source#1209
jay-tau wants to merge 4 commits into
ccusage:mainfrom
jay-tau:feat/copilot-session-state-reader

Conversation

@jay-tau

@jay-tau jay-tau commented Jun 5, 2026

Copy link
Copy Markdown

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 daily therefore 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 daily works without any extra setup on every install since the session-state schema was introduced.

Cost-mode surface

--mode Behavior for Copilot
auto (default) True bill, billing-field-aware: AI Credits × $0.01 (when totalNanoAiu is present, typically post-cutover CLI ≥ 1.0.40) → requests.cost × $0.04 (when only requests.cost is 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.
display Source-precomputed only: AIU → premium-requests → $0. No token-priced fallback.
calculate / api Always LiteLLM token pricing — what the same usage would have cost via the underlying provider's API. api is a global discoverability alias for calculate.

LoadedEntry.credits is surfaced in every JSON row whenever the source carries totalNanoAiu, so callers can read the raw credit count regardless of mode.

Behavior changes outside the Copilot adapter

This PR's --all cross-source aggregator improvements affect every adapter, not just Copilot. Flagging the user-observable changes explicitly:

  1. summary_rows no longer drops zero-token summaries with positive cost or credits. Pre-fix any summary where total_tokens == 0 was silently dropped from ccusage daily --all totals. Post-fix the predicate is total_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-message costUSD set on events whose token counts don't aggregate) and Hermes (actual_cost / estimated_cost channel). Pi / OpenCode parsers drop zero-token entries upstream so they don't reach this filter, but Claude / Hermes do. The exact == 0.0 compare is deliberate — total_cost only reaches 0.0 via upstream assignment, never float arithmetic — and the cross-agent 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 --all totals.

  2. AllRow.credits is now first-class. Aggregation sums credits across agents and days; JSON emits metadata.credits per row and totals.credits (gated on a positive sum, matching the direct per-agent renderer's if credits > 0.0 gate). Backward-compatible: agents that never report credits (Claude, Codex, Amp, …) see the same totals JSON shape they always did.

  3. --mode api is a global alias. Available across every agent's subcommand and as a global --mode api flag, not Copilot-scoped. The schema enum was extended; documented in cost-modes.md.

OTel removal scope

The production loader no longer reads ~/.copilot/otel/*.jsonl or honors COPILOT_OTEL_FILE_EXPORTER_PATH / COPILOT_OTEL_DEDUP / COPILOT_PREFER_OTEL. Two pinning tests in paths.rs assert OTel directories and env vars are now inert.

The legacy parse_otel_file parser 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 the otel/ 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 test count: +50+ tests in adapter::copilot::* (post-OTel-test-removal: net +46 vs base, since the 4 OTel parser tests in loader.rs were dropped with the parser itself).
  • Cross-source aggregator: +8 tests in adapter::all::* covering credits aggregation, --all byte-parity with direct renderer, gating semantics, and the cross-agent summary_rows relaxation.
  • Workspace total: 418 tests pass, 0 failures.
  • New regression coverage includes: fractional requests.cost, absent-vs-explicit-zero requests.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_DIR no-fallback semantics, date-filter-before-credit-only-dedup ordering, cross-agent zero-token + positive-cost survival, and totals.credits JSON byte-parity ("credits":5.0 not "credits":5).

CI gates verified locally

  • cargo clippy --workspace --all-targets -- -D warnings: clean
  • cargo fmt --check: clean
  • cargo test --workspace: 418 passed, 0 failed

pnpm run format and pnpm typecheck require Nix dev-shell tools (oxfmt, tsgo) that aren't available outside the dev shell — CI will run them.

Note on apps/ccusage/config-schema.json diff size

The schema diff is ~894 lines but the only semantic change is a single config-schema enum addition (ConfigCostMode::Api variant in config_schema.rs, mapped to CostMode::Calculate in config.rs) — the schema regenerator (pnpm run generate:schema) ripples that into every mode enum block across all agents/report kinds (~69 added "api" lines), and oxfmt also reflows the file because main predates the project's .oxfmtrc.json enum-multiline rule (e849fac chore(oxlint,oxfmt): add json schema). Reviewers should use git diff -w; the schemars .snap snapshot 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 u64 deserialization of requests.cost, plus fixes for date-filter-before-dedup ordering, cross-agent credits aggregation, --all JSON 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

    • Added api cost mode as an alias for calculate, including CLI/config/schema support.
    • Enhanced Copilot ingestion to prefer session-state events.jsonl from the configured base directory.
  • Bug Fixes

    • Improved Copilot detection when date filters exclude data, while still keeping the Copilot section available when session-state exists.
    • Refined credit handling in JSON output, including correct absence-vs-zero behavior and omission of credits when not applicable.
  • Documentation

    • Updated CLI, cost-mode, environment variable, and Copilot guides to match the new mode semantics and session-state data location.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds the api cost-mode alias, migrates Copilot ingestion to COPILOT_CONFIG_DIR session-state files, propagates credits through aggregation and JSON output, updates CLI/schema/documentation, and expands related tests.

Changes

Copilot ingestion, credits, and api mode

Layer / File(s) Summary
Mode alias contract across configuration and CLI
apps/ccusage/config-schema.json, rust/crates/ccusage/src/config_schema.rs, rust/crates/ccusage/src/config.rs, rust/crates/ccusage-cli/src/*
Adds api as an alias for calculate across configuration, schema, parsing, help output, command options, and cost-mode documentation.
Environment override test support
rust/crates/ccusage-test-support/src/lib.rs
Extends EnvVarGuard to snapshot and restore multiple environment variables.
Copilot session-state discovery and parsing
rust/crates/ccusage/src/adapter/copilot/{paths,parser,mod}.rs
Replaces OTel discovery with session-state loading, tolerant parsing, deduplication, model normalization, legacy-variable detection, and one-time warnings.
Copilot loading and billing
rust/crates/ccusage/src/adapter/copilot/loader.rs, rust/crates/ccusage/src/adapter/all/loader.rs
Applies date filtering before deduplication, isolates file errors, computes costs and credits, preserves detection state, and routes Copilot through dedicated loading logic.
All-report credit aggregation and JSON output
rust/crates/ccusage/src/adapter/all/{types,report,tests}.rs
Carries optional credits through aggregation and breakdowns, controls session metadata, emits row and totals credits, and adds regression coverage.
Copilot and CLI documentation
docs/guide/*, rust/crates/ccusage/src/adapter/copilot/README.md
Documents session-state sources, COPILOT_CONFIG_DIR, billing modes, credits, troubleshooting, and ignored legacy OTel variables.

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
Loading

Possibly related issues

Possibly related PRs

  • ccusage/ccusage#1058: Related to configuration schema generation affected by the new ConfigCostMode::Api variant.
  • ccusage/ccusage#1148: Overlaps the AllAccumulator, AllRow, and aggregation test paths updated for credits.
  • ccusage/ccusage#1332: Overlaps Copilot loader file-reading and entry-loading behavior.

Suggested labels: enhancement

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: Copilot now reads session-state events.jsonl as the default data source.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

This PR was auto-closed. Only contributors approved with lgtm can open PRs. Open an issue first.

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 lgtmi, your future issues will stay open. If a maintainer replies lgtm, your future issues and PRs will stay open.

See CONTRIBUTING.md.

@github-actions github-actions Bot closed this Jun 5, 2026
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 5, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 5, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 5, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
@ryoppippi ryoppippi reopened this Jun 6, 2026
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
… 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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 6, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
… + 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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
… 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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
… 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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 7, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 8, 2026
… 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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 8, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 9, 2026
…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]>
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 9, 2026
…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]>
@jay-tau
jay-tau force-pushed the feat/copilot-session-state-reader branch from 225d38e to f9fcb0d Compare June 9, 2026 13:12
jay-tau added a commit to jay-tau/ccusage that referenced this pull request Jun 9, 2026
…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]>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread rust/crates/ccusage-test-support/src/lib.rs Outdated
Comment thread rust/crates/ccusage/src/adapter/copilot/mod.rs Outdated
Comment thread rust/crates/ccusage-test-support/src/lib.rs Outdated
Comment thread rust/crates/ccusage/src/adapter/all/tests.rs Outdated
Comment thread rust/crates/ccusage/src/adapter/copilot/paths.rs
Comment thread rust/crates/ccusage/src/adapter/copilot/loader.rs Outdated
Comment thread rust/crates/ccusage/src/adapter/copilot/loader.rs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Pullfrog  | View workflow run | Using Big Pickle (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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_cost and premium_cost now use expect with descriptive messages matching their has_aiu / has_premium_data guards, resolving the prior Pullfrog nitpick
  • has_data() short-circuit — new has_any_session_state_event_file() replaces full directory enumeration with a first-hit probe, reducing filesystem work in --all detection
  • Poison-recovering test lockacquire_env_test_lock() wraps ENV_TEST_LOCK with into_inner() poison recovery so a panic in one env-mutating test does not cascade-fail every subsequent env test
  • trim_os_string UTF-8 fast path — Unix path trimming now uses str::trim() for the UTF-8 common case before falling back to ASCII byte trimming
  • Test cleanup — removed redundant .clone() in all/tests.rs

Pullfrog  | View workflow run | Using Big Pickle (free via Pullfrog for OSS) | 𝕏

@jay-tau

jay-tau commented Jun 24, 2026

Copy link
Copy Markdown
Author

@ryoppippi would appreciate your review on this PR

@appleflyerv3

Copy link
Copy Markdown

please push, this is useful

@jay-tau

jay-tau commented Jul 13, 2026

Copy link
Copy Markdown
Author

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.

@ryoppippi

Copy link
Copy Markdown
Member

ok i'll take a look

@pullfrog fix conflicts

@pullfrog

pullfrog Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Resolved all merge conflicts for #1209 against main and verified the Rust workspace is clean (cargo check, clippy -D warnings, and cargo test --workspace all pass). I could not push the resolved branch because the available GitHub token is rejected with 403 Permission denied to github-actions[bot] from both the fork and this repo. I’ve posted a ready-to-apply patch on the PR for the author/maintainer to push.

Task list (5/5 completed)
  • Checkout PR branch and fetch base branch
  • Attempt merge of base branch into PR branch
  • Resolve merge conflicts
  • Verify resolution builds/tests
  • Commit and push resolved branch

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | via Pullfrog | Using Kimi K2 (free via Pullfrog for OSS) (DeepSeek Pro not used — the program covers this model; add its provider key to run your pick) | 𝕏

@pullfrog

pullfrog Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

I resolved the merge conflicts against main locally.

Conflicts resolved

  • docs/guide/index.md — kept the session-state Copilot path and the ~/.kimi-code note.
  • rust/crates/ccusage/src/adapter/all/loader.rs — kept the --sections / named pi-store refactor and reapplied the Copilot credit-only filtering, load_copilot_rows, and credits-aware summary_rows.
  • rust/crates/ccusage/src/adapter/all/mod.rs — kept the new sections wiring.
  • rust/crates/ccusage/src/adapter/all/report.rs — kept the section report helpers and reapplied the credits JSON injection in build_row_metadata / totals_json.
  • rust/crates/ccusage/src/adapter/all/types.rs — added the first-class credits field to AllRow and AllAccumulator.
  • rust/crates/ccusage/src/adapter/copilot/loader.rs — kept the PR’s session-state test for model normalization and dropped the OTel read_otel_file test that no longer applies.

Verification

  • cargo check --workspace --all-targets — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo test --workspace — all passing

Push blocker

I cannot push the resolved branch: both the fork (jay-tau/ccusage) and this repo reject pushes from the available GitHub token with 403 Permission denied to github-actions[bot].

I’ve attached a ready-to-apply patch (diff from main to the resolved branch). You can apply it with:

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

resolved_pr.patch

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | via Pullfrog | Using Kimi K2 (free via Pullfrog for OSS) (DeepSeek Pro not used — the program covers this model; add its provider key to run your pick) | 𝕏

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review
@cubic-dev-ai review

Rebased onto current main and resolved the conflicts in 78130a5. Preserved the current sections and named Pi-store changes alongside the Copilot session-state and credits behavior.

...

@jay-tau I have started the AI code review. It will take a few minutes to complete.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 api alias (config + CLI + schema + docs) that maps to calculate (LiteLLM token pricing).
  • Improve --all aggregation 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.

Comment thread rust/crates/ccusage/src/adapter/copilot/loader.rs
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

@jay-tau Thanks for the rebase and validation details. I’ll review the updated PR, including the resolved-conflict changes.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review
@cubic-dev-ai review

Follow-up ccb71aa restores ordered parallel Copilot session-state reads while preserving --single-thread and deterministic warnings. Formatting, checks, clippy, and all 523 Rust tests pass; a 676-file benchmark improved from an 8.35s median to 2.65s with byte-identical JSON output.

@jay-tau I have started the AI code review. It will take a few minutes to complete.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

@jay-tau Thanks for the follow-up and validation details. I’ll review the ordered parallel-read changes, including --single-thread behavior, deterministic warnings, and output-order preservation.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 791b377 and 78130a5.

⛔ Files ignored due to path filters (2)
  • rust/crates/ccusage-cli/src/snapshots/ccusage_cli__tests__root_help.snap is excluded by !**/*.snap
  • rust/crates/ccusage/src/snapshots/ccusage__config_schema__tests__snapshots_schema_agent_specific_option_edges.snap is excluded by !**/*.snap
📒 Files selected for processing (25)
  • apps/ccusage/config-schema.json
  • docs/guide/cli-options.md
  • docs/guide/configuration.md
  • docs/guide/copilot/index.md
  • docs/guide/cost-modes.md
  • docs/guide/environment-variables.md
  • docs/guide/getting-started.md
  • docs/guide/index.md
  • rust/crates/ccusage-cli/src/cli-commands.json
  • rust/crates/ccusage-cli/src/cli-help.json
  • rust/crates/ccusage-cli/src/help_codegen.rs
  • rust/crates/ccusage-cli/src/parser.rs
  • rust/crates/ccusage-test-support/src/lib.rs
  • rust/crates/ccusage/src/adapter/all/loader.rs
  • rust/crates/ccusage/src/adapter/all/mod.rs
  • rust/crates/ccusage/src/adapter/all/report.rs
  • rust/crates/ccusage/src/adapter/all/tests.rs
  • rust/crates/ccusage/src/adapter/all/types.rs
  • rust/crates/ccusage/src/adapter/copilot/README.md
  • rust/crates/ccusage/src/adapter/copilot/loader.rs
  • rust/crates/ccusage/src/adapter/copilot/mod.rs
  • rust/crates/ccusage/src/adapter/copilot/parser.rs
  • rust/crates/ccusage/src/adapter/copilot/paths.rs
  • rust/crates/ccusage/src/config.rs
  • rust/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

Comment thread docs/guide/environment-variables.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread docs/guide/index.md Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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 in load_entries_inner with read_files_parallel(&paths, shared.single_thread, parse_session_state_file). The read_files_parallel utility 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 — the zip + match loop emits the same eprintln! warning for unreadable files.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread rust/crates/ccusage/src/adapter/all/tests.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread rust/crates/ccusage/src/adapter/copilot/mod.rs Outdated
@jay-tau

jay-tau commented Jul 23, 2026

Copy link
Copy Markdown
Author

@ryoppippi I incorporated Pullfrog’s conflict-resolution changes and pushed the resolved branch. The PR is now up to date with main and conflict-free.
Thank you for reviewing

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread rust/crates/ccusage/src/adapter/copilot/loader.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_entries will 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());

Comment thread rust/crates/ccusage/src/adapter/all/loader.rs Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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_credits with an integer-division + remainder split for values exceeding f64's exact integer range (2^53 ≈ 9 quadrillion nano-AIU), and a direct cast for the common small-value path. Pinned by nano_aiu_conversion_preserves_fractional_precision.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread rust/crates/ccusage/src/adapter/copilot/loader.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

jay-tau and others added 4 commits July 23, 2026 16:25
…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]>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ 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 api alias, and OTel removal.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(adapter/copilot): read usage from ~/.copilot/session-state events.jsonl (no OTel required)

4 participants