Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cost): allow all usage types #486

Merged
merged 8 commits into from
Dec 16, 2024
Merged

feat(cost): allow all usage types #486

merged 8 commits into from
Dec 16, 2024

Conversation

hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Dec 13, 2024

Important

Deprecates old usage and cost fields, introduces usageDetails and costDetails, and updates related logic across multiple files for improved tracking.

  • Behavior:
    • Deprecates usage, calculatedInputCost, calculatedOutputCost, and calculatedTotalCost fields in favor of usageDetails and costDetails in openapi-server.yaml and server.ts.
    • Increases comment content limit from 500 to 3000 characters in openapi-server.yaml and server.ts.
  • Models:
    • Adds UsageDetails and OpenAIUsageSchema to openapi-server.yaml.
    • Updates Usage model to be deprecated in openapi-server.yaml.
  • Functions:
    • Updates parseUsageDetails and parseUsageDetailsFromResponse in parseOpenAI.ts to handle new usage details format.
    • Modifies handleLLMEnd in callback.ts to use usageDetails.
    • Adjusts processSpanAsLangfuseGeneration in LangfuseExporter.ts to include usageDetails.
  • Misc:
    • Refactors parseUsageDetails logic in LangfuseExporter.ts and traceMethod.ts for consistency.
    • Updates CallbackHandler in callback.ts to handle new usage details format.
    • Fixes test expectations in langfuse-integration-error-handling.spec.ts and langfuse-integration-langchain.spec.ts.

This description was created by Ellipsis for ec41356. It will automatically update as commits are pushed.

Copy link

vercel bot commented Dec 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
langfuse-js ✅ Ready (Inspect) Visit Preview Dec 16, 2024 5:03pm

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

This PR enhances cost and usage tracking capabilities across the Langfuse JS SDK, focusing on more granular metrics and improved flexibility.

  • Added new usageDetails and costDetails fields in observations for detailed token tracking, deprecating older usage fields while maintaining backward compatibility
  • Increased comment character limit from 500 to 3000 in /api/public/comments endpoint
  • Added OpenAI-specific usage schema with detailed token tracking in parseOpenAI.ts and LangfuseExporter.ts
  • Implemented flexible usage reporting in CallbackHandler class to preserve granular token details
  • Added support for expanded media content types including additional image, audio, video formats

7 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse-langchain/src/callback.ts Outdated Show resolved Hide resolved
langfuse-langchain/src/callback.ts Outdated Show resolved Hide resolved
langfuse/src/openai/parseOpenAI.ts Show resolved Hide resolved
langfuse/src/openai/parseOpenAI.ts Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the changes in the integration test file and the context provided, here's a concise summary of the recent updates:

Integration test updates for new usage and cost tracking implementation in LangChain integration:

  • Modified test assertions to validate new usageDetails field structure in generation observations
  • Updated test cases to handle OpenAI-specific usage schema with detailed token tracking
  • Added validation for backward compatibility with deprecated usage fields
  • Enhanced test coverage for streaming LLM calls to verify proper usage tracking
  • Expanded test cases to verify proper handling of tool calls and prompt management with new usage format

Note: These changes specifically focus on the testing aspects of the new usage tracking system while maintaining compatibility with existing integrations.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes shown in the callback.ts file, here's a concise summary of the recent updates:

Improved usage tracking implementation in the Langfuse callback handler with focus on token usage details:

  • Enhanced extractUsageMetadata method to better handle AIMessage.usage_metadata and llmOutput.tokenUsage
  • Added support for new token usage format (input_tokens/output_tokens) while maintaining compatibility with legacy format
  • Improved handling of streaming token counts in handleLLMNewToken for more accurate usage tracking
  • Unified usage tracking across different LLM response types (chat, completion, streaming) in handleLLMEnd

The changes focus on improving the accuracy and consistency of token usage tracking while ensuring backward compatibility with existing integrations.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes to the integration test files, here's a concise summary of the recent updates:

Modified test model configuration and assertions in Langchain integration tests to align with new usage tracking:

  • Changed test model from gpt-4-1106-preview to gpt-3.5-turbo-instruct in LLM call tests
  • Updated model name assertion from OpenAIChat to OpenAI to match new model configuration
  • Adjusted test expectations to validate proper usage tracking with the new model type

These changes appear to be focused on ensuring test consistency with the new cost and usage tracking features while using a more appropriate model for testing purposes.

Note: The changes are minimal but important for maintaining test accuracy with the new usage tracking implementation.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the latest changes and previous reviews, here's a concise summary of the recent updates:

Refactored usage tracking implementation in OpenAI-related files for improved consistency:

  • Added parseUsageDetails and parseUsageDetailsFromResponse in parseOpenAI.ts to standardize usage data extraction
  • Updated traceMethod.ts to include both legacy usage and new usageDetails fields in generation tracking
  • Refactored LangfuseExporter.ts to extract usage parsing logic into a dedicated method for better maintainability
  • Enhanced error handling and type safety across OpenAI integration components

The changes focus on consolidating the usage tracking logic while ensuring proper error handling and maintaining backward compatibility.

8 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

langfuse-core/src/openapi/server.ts Show resolved Hide resolved
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the changes in the integration test file and previous reviews, I'll provide a concise summary of the latest updates focusing on error handling and callback behavior:

Improved error handling and callback behavior in integration tests:

  • Removed console.error expectations in langchain tests for incorrect host/keys scenarios, indicating a more graceful error handling approach
  • Reduced expected flush callback count from 51 to 8 in shutdown async test, suggesting optimized batch processing
  • Updated test assertions to align with new usage tracking implementation while maintaining proper error handling

Key points:

  • The changes reflect a more streamlined approach to error handling in the LangChain integration
  • Callback count reduction indicates improved efficiency in batch processing during shutdown
  • Error handling modifications support the broader updates for cost and usage tracking while ensuring stability

These changes are part of the larger effort to improve cost tracking while maintaining robust error handling and efficient callback management.

Note: This summary focuses on the most recent changes in error handling and callback behavior, avoiding repetition of points from previous reviews.

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@hassiebp hassiebp merged commit 8ddc9aa into main Dec 16, 2024
7 of 9 checks passed
@hassiebp hassiebp deleted the v3-cost-tracking branch December 16, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant