Skip to content

fix(langfuse): export canonical generation total - #64797

Closed
NaMinhyeok wants to merge 1 commit into
NousResearch:mainfrom
NaMinhyeok:codex/fix-langfuse-cost-rollup
Closed

fix(langfuse): export canonical generation total#64797
NaMinhyeok wants to merge 1 commit into
NousResearch:mainfrom
NaMinhyeok:codex/fix-langfuse-cost-rollup

Conversation

@NaMinhyeok

@NaMinhyeok NaMinhyeok commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes Langfuse generation/trace/session cost rollups when Hermes exports cache or other custom cost buckets.

The bug was reproduced on unchanged main at d2c81eb681dea1382fbd1ed403f58320d5aef575; the branch is now rebased onto current main at 9baa7d4673ce89f09378daa3660530f8bf142708. Hermes already computes the right request-level total, but both Langfuse export paths discard it and send only component buckets. Current Langfuse ingestion treats any supplied cost_details as authoritative and skips model-price inference. It derives total only for the exact built-in input + output case; adding cache_read_input_tokens leaves total absent. Downstream generation conversion treats an absent total as zero, and trace/session aggregation consumes that total.

The contract was verified against the current ingestion schema, ingestion implementation, Python SDK serialization, observation conversion, trace rollup, and session rollup.

This patch exports Hermes's canonical estimate_usage_cost(...).amount_usd as cost_details.total in both the response-object and sanitized-summary paths. It deliberately does not sum component buckets: the canonical amount also includes per-request pricing (request_count * request_cost). Unknown or partially priced usage still emits no guessed cost, and subscription-included routes retain their existing component-only semantics without an explicit total.

This cannot be repaired reliably with Langfuse model-price configuration or Hermes environment settings. Once Hermes supplies any cost bucket, current Langfuse ingestion does not run model inference; removing all supplied costs would also discard Hermes-specific cache/custom/request pricing and is not equivalent to completing the payload.

Reproduction on unchanged main

Local Langfuse server 3.178.0 with Python SDK 4.7.1:

Runtime path Case Hermes canonical total Cost payload before Stored generation total Trace rollup
response object no cache $1.60 input=$1.00, output=$0.60 $1.60 $1.60
response object cache $0.88 input=$0.20, output=$0.60, cache=$0.08 $0.00 / absent $0.00
sanitized summary no cache $1.60 input=$1.00, output=$0.60 $1.60 $1.60
sanitized summary cache $0.88 input=$0.20, output=$0.60, cache=$0.08 $0.00 / absent $0.00

After this patch, the same four calls export explicit totals of $1.60, $0.88, $1.60, and $0.88. Each generation and trace stores the matching value, and the session metrics roll up to $4.96.

Related Issue

Related: #49932, #43130

Credit to @rdguidry: draft PR #49932 first identified the missing-explicit-total problem. This is a separate, provider-agnostic plugin-only fix because #49932 also owns Venice pricing/catalog changes and derives total by summing buckets, which omits request-level pricing. No code was copied from #49932; its Venice work remains there.

#43130 is orthogonal subscription-included policy work: it removes all explicit cost details so Langfuse may estimate nominal list price. This patch is behaviorally compatible because it never adds total for an included result. If #43130 lands first, only a mechanical rebase around the shared helper may be needed.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/observability/langfuse/__init__.py: share canonical usage/cost export between both call paths and include the authoritative request-level total only when pricing is complete and not subscription-included.
  • tests/plugins/test_langfuse_plugin.py: cover cache/no-cache, request-priced, request-only, partial/unknown pricing, and included-route parity through both real exporter paths.

How to Test

  1. Run scripts/run_tests.sh tests/plugins/test_langfuse_plugin.py tests/agent/test_usage_pricing.py -q.
  2. Run python -m ruff check plugins/observability/langfuse/__init__.py tests/plugins/test_langfuse_plugin.py and python scripts/check-windows-footguns.py plugins/observability/langfuse/__init__.py tests/plugins/test_langfuse_plugin.py.
  3. Point the plugin at a Langfuse v3 instance and compare otherwise identical calls with and without cached input tokens across the response-object and sanitized-summary hooks. Generation, trace, and session totals should match Hermes's canonical amount.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.5.1 arm64

Full wrapper run: 41,121 passed and 39 failed in unrelated host/environment-sensitive tests (live credential guards, Linux service/systemd assumptions, macOS /tmp path normalization, AWS credentials, and gateway SDK/race tests). The changed Langfuse tests passed in the full run. Focused wrapper: 70 passed.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; no public configuration or API change
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — Windows footgun check passed
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

  • Focused tests: 70 passed.
  • Langfuse plugin tests: 55 passed.
  • Ruff, Python compile, git diff --check, and Windows footgun checks: passed.
  • Before-fix E2E run: codex-langfuse-rollup-1784094329-099614.
  • After-fix E2E run: codex-langfuse-fix-1784095229-0cbc8f.

@NaMinhyeok
NaMinhyeok force-pushed the codex/fix-langfuse-cost-rollup branch from 7a893ac to 320a152 Compare July 15, 2026 06:13
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 15, 2026
@NaMinhyeok
NaMinhyeok marked this pull request as ready for review July 15, 2026 06:25
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Langfuse fix. The premise remains verified on current main: both exporter paths emit only component costs whenever get_pricing_entry() succeeds (plugins/observability/langfuse/__init__.py:576-596 and :1003-1016). The new shared helper applies estimate_usage_cost() to both paths and preserves request_count, which is necessary because the canonical estimator includes request-level pricing (agent/usage_pricing.py:966-967; run_agent.py:2324-2329).

The PR is cleanly salvageable: upstream has not changed either modified Langfuse file since the PR base 9baa7d4.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 16, 2026
@jeff-mettel

Copy link
Copy Markdown
Contributor

Link for discoverability: this PR is the fix for #72661, which currently shows no linked PR in its timeline.

Same mechanism, reported from the user side: per-type cost buckets populate while trace total_cost stays 0.00.


Filed by an AI agent (Claude Opus 5) operating autonomously on @jeff-mettel's behalf.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Two PRs address related Langfuse cost-export failures through distinct changes: #43130 omits authoritative zero costs for subscription-included providers, while #64797 exports Hermes' canonical request total when component buckets alone do not produce a Langfuse total.

Related pull requests

Suggested consolidation

Keep both open with separate salvage paths: preserve #43130's focused omission of zero costs for subscription-included routes, and preserve #64797's canonical-total helper and request-count handling for priced routes. Coordinate the overlapping edits in the two Langfuse paths so #64797 does not retain zero component costs for included routes contrary to #43130's recorded best-fix behavior; neither PR should be closed as a duplicate.

Cross-PR triage: Reviewed 2 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 32 kB of PR diffs, 17 kB of issue/PR text, 1 kB of discussion (3 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

erosika added a commit to erosika/hermes-agent that referenced this pull request Aug 10, 2026
Supersedes this branch's earlier sum-of-components total with the
canonical Hermes estimate. Summing the per-type breakdown undercounts
whenever a component can't be priced (cache rates missing) or when
request-level pricing applies; the estimate_usage_cost amount is the
number Hermes itself reports, so both response-object and summary-dict
paths now share one _canonical_usage_and_cost helper that exports it as
the explicit total alongside the per-type breakdown. A partial
breakdown with no valid estimate exports no total at all, so Langfuse
can't mistake a subtotal for the full cost. Subscription-included
routes keep their component-only payload, and a zero estimate is not
exported as an authoritative 0.0.

Adopted from NousResearch#64797 — thanks @NaMinhyeok for the thorough repro and
test matrix.

Co-authored-by: NaMinhyeok <nmh9097@gmail.com>
@erosika

erosika commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

adopted into #83437 with co-author credit — thanks for the thorough repro and the both-paths test matrix. it superseded the branch's earlier sum-of-components total; one integration change adds a zero-guard so a priced model that billed no tokens doesn't export an authoritative 0.0 total. closes #72661 on merge together with the cost-total commit.

@alt-glitch alt-glitch added area/usage-cost Token accounting, usage reporting, billing, cost tracking telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge labels Aug 11, 2026
kshitijk4poor added a commit that referenced this pull request Aug 13, 2026
… fan-out

Salvaged from PR #83437 by @erosika, with adopted fixes from @bgodlin (#81054),
@aldoeliacim (#82332), @nftpoetrist (#42326), @rodboev (#39653), @FnExpress
(#64292, supersedes #32175 by @db-aeon), @Per0-1 (#61166), @NaMinhyeok (#64797),
and @liuhao1024 (#43130).

Widens the bundled Langfuse plugin from 6 to 11 hooks and fixes two
attribution bugs. Also adopts shutdown/atexit lifecycle fixes and composes
8 prior community PRs with interaction-fix follow-ups.

Model attribution: on_pre_llm_request and on_post_llm_call now prefer the
wire value (request body model, response model) over the agent attribute,
which goes stale after /model switch or provider fallback.

Cost total: both cost paths now send a summed total alongside the per-type
breakdown, since Langfuse does not derive calculatedTotalCost from
cost_details keys. Subscription-included routes send no cost keys at all.

New coverage: api_request_error closes failed generations with ERROR level;
on_session_finalize/on_session_end close dangling traces for tool-only and
interrupted turns; subagent_start/subagent_stop trace delegated children as
spans; MoA advisor fan-out emits one generation per advisor priced at the
advisor's own model.

Capture modes: HERMES_LANGFUSE_CAPTURE=metadata|sanitized|full (default
sanitized). Sanitized mode redacts secret patterns before truncation.

Adopted lifecycle fixes: shutdown client at session finalize when
reason=shutdown (not on session rotation); atexit finalizer ends open root
spans for short-lived processes; root context manager exited to prevent
interpreter-teardown TypeError; TOCTOU on _get_langfuse() fixed with lock;
reasoning_content surfaced in traces; system prompt included in generation
input for Anthropic/Codex/Bedrock; SDK v3 update_trace replaces set_trace_io.

Closes #29482, #43129, #72661.
Supersedes #81054, #82332, #42326, #39653, #64292, #32175, #61166, #64797, #43130.
Partially addresses #67544 (capture modes + secret redaction; user_id remains open).
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #85439 — your fix was adopted and composed into the wider Langfuse tracing PR by @erosika. Your contribution is credited in the commit body. Thanks @NaMinhyeok!

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

Labels

area/usage-cost Token accounting, usage reporting, billing, cost tracking comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants