fix(hermes): price subscription-included sessions - #1113

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.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

@coderabbitai review it

#!/bin/bash
# Get the PR diff
gh pr diff 1113 --repo ryoppippi/ccusage

# Also get the full file content
cat rust/crates/ccusage/src/adapter/hermes.rs

@ryoppippi: Here's the review for #1113.

Review Summary

Overall: ✅ Looks good — the fix is correct, minimal, and well-motivated.

Core logic change

// Before — short-circuits on any recorded cost, including 0.0
if let Some(cost) = entry.cost_usd {
    return cost;
}

// After — only short-circuits on a strictly positive recorded cost
if let Some(cost) = entry.cost_usd.filter(|cost| *cost > 0.0) {
    return cost;
}

The one-liner .filter(|cost| *cost > 0.0) is idiomatic Rust and exactly the right fix. A few edge-case notes that all check out:

• Negative stored costs: read_non_negative_f64 already clamps them to 0.0 via .max(0.0), so they arrive as Some(0.0) and correctly fall through with the new filter.
• NaN: NaN > 0.0 is false, so NaN falls through to pricing — correct.
• Infinity: read_f64 guards with .is_finite(), so infinity cannot reach cost_usd at all.

The secondary cleanup removing the dead cost >= 0.0 guard in the pricing loop is valid — cost > 0.0 already implies it.

Tests

Both new tests cover the right invariants:

Minor nit — float equality in recorded_positive_cost_is_trusted:

assert_eq!(calculate_hermes_cost(&entry, &pricing), 0.42);

This works correctly here because the value is stored and returned with no arithmetic, so the bits are preserved exactly. It's not wrong, but if this pattern is extended to cases where arithmetic is applied, it could silently break. A small defensive alternative for future readers:

let cost = calculate_hermes_cost(&entry, &pricing);
assert!((cost - 0.42).abs() < f64::EPSILON, "expected ~0.42, got {cost}");

Not a blocker — just worth keeping in mind if the test is ever expanded.

Coverage gap (non-blocking)

There's no explicit test for cost_usd = None in calculate_hermes_cost, but the existing calculates_cost_for_hermes_frontier_models_from_embedded_pricing test already covers that path via cost_usd: None. No action needed.

The fix is correct and the PR is in good shape. 🐇

#!/bin/bash
# Read the hermes adapter file
cat rust/crates/ccusage/src/adapter/hermes.rs

echo "---CODEX ADAPTER---"
# Read the codex adapter for comparison
cat rust/crates/ccusage/src/adapter/codex.rs 2>/dev/null || echo "Not found"

echo "---DIFF---"
git diff main...HEAD -- rust/crates/ccusage/src/adapter/hermes.rs

@ryoppippi The fix is clean and correct. Here's the full review:

Review of #1113 — fix(hermes): price subscription-included sessions

Core logic change (calculate_hermes_cost)

The change from:

if let Some(cost) = entry.cost_usd {

to:

if let Some(cost) = entry.cost_usd.filter(|cost| *cost > 0.0) {

is the minimal, correct fix. It exactly mirrors the intent: only short-circuit when Hermes has a meaningful recorded cost. Since read_non_negative_f64 already clamps any negative DB value to 0.0 via .max(0.0), NaN and negative values can never reach this function as Some(...), so the > 0.0 guard is both sufficient and correct.

The removal of the redundant cost >= 0.0 in the candidate loop is a clean tidy-up — cost > 0.0 already implies >= 0.0.

Tests

Both new tests are well-written:

• recorded_zero_cost_falls_back_to_pricing_calculation uses real-world token numbers from the linked issue — good regression anchor.
• recorded_positive_cost_is_trusted uses an exact equality assertion (assert_eq!(..., 0.42)) which is correct since the early return path has no floating-point arithmetic.

Note: both tests depend on gpt-5.5 being present in the embedded pricing table, which is already guarded by calculates_cost_for_hermes_frontier_models_from_embedded_pricing — acceptable coupling.

Minor observation: data.cost_usd vs LoadedEntry.cost divergence

For subscription sessions, to_loaded_entry now produces a LoadedEntry where:

• cost (used for aggregation/display) = pricing-computed, non-zero ✅
• data.cost_usd (raw Hermes value passed into UsageEntry) = Some(0.0)

If any JSON output path surfaces UsageEntry.cost_usd directly (e.g., a hermes sessions --json query), it would still show 0.0 for subscription sessions. This is a pre-existing design and not introduced by this PR, but it may be worth a follow-up to either update data.cost_usd to the computed cost or document the intentional split.

Summary

The PR is good to merge. The data.cost_usd inconsistency is a separate concern worth a follow-up issue if it affects any output path.

@cubic-dev-ai review this

@cubic-dev-ai review this

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

Historical audit: this pull request was auto-closed by the legacy contributor gate. That closure did not assess technical importance.

Audit result: resolved. A later merged change or the current main implementation covers this request. This PR is kept for history and does not need to be revived.