…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…4 nano-AIU typing

Another external code review on PR ryoppippi#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 ryoppippi#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 ryoppippi#1174

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

… + 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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…ly rows with same naiu but distinct event ids

External PR ryoppippi#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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

… 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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>

…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 <223556219+Copilot@users.noreply.github.com>