fix(langchain): forward provider-reported cost to costDetails - #880
fix(langchain): forward provider-reported cost to costDetails#880mittalpk wants to merge 3 commits into
Conversation
Some OpenAI-compatible providers (e.g. OpenRouter) report a total cost directly on the response instead of leaving it to be derived from Langfuse's own model price table. LangChain surfaces this as `usage.cost` inside `response_metadata`, but CallbackHandler.handleLLMEnd never read it, so generations were ingested with empty costDetails and Langfuse fell back to a price-table lookup that fails for any model it doesn't recognize (most OpenRouter model slugs). Adds extractCostDetails(), mirroring the existing extractModelNameFromMetadata() pattern, and wires it into handleLLMEnd's costDetails attribute. Also fixes a pnpm workspace issue this surfaced: packages/langchain had no devDependency pin on @langchain/core, so it resolved to a different, older copy (1.1.8) than the rest of the workspace (1.1.24) - two physically distinct AIMessage classes, so `instanceof` checks in CallbackHandler silently failed whenever the package was exercised through its own local dependency resolution rather than a consuming app's single hoisted install. This is likely why no integration test previously exercised handleLLMEnd's AIMessage-dependent extraction logic at all (only a tool-observation test existed). Pinning the devDependency plus adding `resolve.dedupe: ["@langchain/core"]` to the integration/e2e vitest projects fixes it for testing; real consumer apps only ever install one copy of @langchain/core so aren't affected. Adds two integration tests: cost forwarding via a minimal fake chat model reporting response_metadata.usage.cost, and confirms costDetails is omitted (not a wrong value) when no provider cost is present. Fixes langfuse#828
`command -v pnpm &> /dev/null` uses bash-only `&>` redirection inside a `#!/bin/sh` script. Under dash (the default /bin/sh on Debian/Ubuntu), `&>` is misparsed as "background this command, then redirect a separate empty command", so the PATH check always reported pnpm as missing regardless of whether it was actually installed - blocking every commit on a dash-based system even with a correctly configured PATH.
|
@mittalpk is attempting to deploy a commit to the langfuse Team on Vercel. A member of the Team first needs to authorize it. |
| !( | ||
| generation["message"] instanceof AIMessage || | ||
| generation["message"] instanceof AIMessageChunk | ||
| ) |
There was a problem hiding this comment.
Module Identity Drops Valid Costs
When an application resolves the handler and provider against separate @langchain/core module instances, a valid provider-created message fails these instanceof checks. Its reported cost is then silently omitted, and the local Vitest dedupe cannot prevent this in consumer installations. Check the message shape instead of constructor identity.
| !( | |
| generation["message"] instanceof AIMessage || | |
| generation["message"] instanceof AIMessageChunk | |
| ) | |
| typeof generation["message"]?.response_metadata !== "object" || | |
| generation["message"].response_metadata === null |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/langchain/src/CallbackHandler.ts
Line: 940-943
Comment:
**Module Identity Drops Valid Costs**
When an application resolves the handler and provider against separate `@langchain/core` module instances, a valid provider-created message fails these `instanceof` checks. Its reported cost is then silently omitted, and the local Vitest dedupe cannot prevent this in consumer installations. Check the message shape instead of constructor identity.
```suggestion
typeof generation["message"]?.response_metadata !== "object" ||
generation["message"].response_metadata === null
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Good catch — fixed in e1138c8. Switched to checking that response_metadata is a non-null object rather than checking instanceof AIMessage/AIMessageChunk, so it no longer depends on message identity across module instances.
…entity Per review feedback on langfuse#880: instanceof AIMessage/AIMessageChunk is an identity check, not a capability check. An app with two separately- resolved copies of @langchain/core (a realistic scenario in a large dependency tree, not just this monorepo's own test setup) would have a genuinely valid, provider-created message silently fail that check and drop its cost - exactly the module-duplication bug this PR's other commit works around for the test suite, but that workaround only covers this repo's own tests, not real consumer installations. extractCostDetails now checks that response_metadata is a non-null object instead of checking message identity, so it no longer depends on which @langchain/core instance constructed the message.
Summary
Fixes #828.
Some OpenAI-compatible providers (e.g. OpenRouter) report a total cost directly on the response instead of leaving it to be derived from Langfuse's own model price table. LangChain surfaces this as
usage.costinsideresponse_metadata, butCallbackHandler.handleLLMEndnever read it, so generations were ingested with emptycostDetailsand Langfuse fell back to a price-table lookup that fails for any model it doesn't recognize (most OpenRouter model slugs).extractCostDetails(), mirroring the existingextractModelNameFromMetadata()pattern, wired intohandleLLMEnd'scostDetailsattribute.response_metadata.usage.cost, and confirmscostDetailsis omitted (not a wrong value) when no provider cost is present.A workspace issue this surfaced
packages/langchainhad nodevDependencypin on@langchain/core, so it resolved to a different, older copy (1.1.8) than the rest of the workspace (1.1.24) — two physically distinctAIMessageclasses, so the existinginstanceof AIMessagechecks inCallbackHandlersilently failed whenever the package was exercised through its own local dependency resolution rather than a consuming app's single hoisted install. This is likely why no integration test previously exercisedhandleLLMEnd'sAIMessage-dependent extraction logic at all (only a tool-observation test existed).Fixed with a
devDependencypin (^1.1.24, matching the rest of the workspace) plusresolve.dedupe: ["@langchain/core"]on theintegration/e2evitest projects. This doesn't affect real consumer apps, which only ever install one copy of@langchain/core.Separate one-line fix
.husky/pre-commituses bash-only&>redirection inside a#!/bin/shscript. Under dash (default/bin/shon Debian/Ubuntu),&>is misparsed as "background this command, then redirect a separate empty command," so the pnpm-on-PATH check always reported it missing regardless of reality, blockingpnpm precommit:checkfrom ever running on a dash-based system. Split into its own commit since it's unrelated to the cost-forwarding fix.Verification
AI-assisted: yes (Claude Code), disclosed per CONTRIBUTING.md. I reviewed and understand every line of the change, and did the debugging (including tracking down the dual-module-instance root cause) interactively rather than accepting a first-pass diff.
Greptile Summary
This PR forwards provider-reported LangChain costs into Langfuse generation data. The main changes are:
Confidence Score: 4/5
Provider costs can still be dropped when consumers load separate LangChain module instances.
packages/langchain/src/CallbackHandler.ts and tests/integration/langchain.integration.test.ts
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: use POSIX-compatible redirect in pr..." | Re-trigger Greptile
Context used:
Learned From
langfuse/langfuse-python#1387