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(media): add multimodal support #451

Merged
merged 12 commits into from
Nov 18, 2024
Merged

feat(media): add multimodal support #451

merged 12 commits into from
Nov 18, 2024

Conversation

hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Nov 17, 2024

Important

Adds multimodal support to Langfuse with new media handling class, API updates, and enhanced OpenAI integration.

  • Media Handling:
    • Introduces LangfuseMedia class for handling media content, supporting base64 data URIs and raw content bytes.
    • Adds methods for processing media in events, including uploading and patching media records.
  • API Changes:
    • Updates server.ts to include new endpoints for media operations: media_get, media_patch, and media_getUploadUrl.
    • Adds new schemas in server.ts for media-related requests and responses.
  • Testing:
    • Adds langfuse.media.spec.ts to test LangfuseMedia functionalities, including constructor, parsing, and JSON serialization.
  • OpenAI Integration:
    • Updates observeOpenAI.ts to include shutdownAsync method for Langfuse client.
    • Enhances OpenAI SDK tracing to support media handling.
  • Miscellaneous:
    • Updates types.ts to include new types for media operations.
    • Minor import and type adjustments in LangfuseExporter.ts and types.ts.

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

Copy link

vercel bot commented Nov 17, 2024

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

Name Status Preview Updated (UTC)
langfuse-js ✅ Ready (Inspect) Visit Preview Nov 18, 2024 2:55pm

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

Added comprehensive multimodal support to the Langfuse SDK, enabling handling of media files (images, audio, PDFs) with secure upload, processing, and storage capabilities.

  • Added new LangfuseMedia class in /langfuse-core/src/media/LangfuseMedia.ts for handling media content through base64 data URIs, raw bytes, and file paths
  • Implemented media processing in /langfuse-core/src/index.ts with recursive traversal, OpenAI format support, and secure upload handling
  • Added new /api/public/media endpoints in OpenAPI spec for media record management with presigned upload URLs
  • Added media-specific test coverage in /langfuse-core/test/langfuse.media.spec.ts and integration tests for OpenAI vision/audio support
  • Enhanced type system in /langfuse-core/src/types.ts with new media-related types while maintaining backward compatibility

13 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile

integration-test/integration-utils.ts Outdated Show resolved Hide resolved
integration-test/integration-utils.ts Outdated Show resolved Hide resolved
integration-test/langfuse-openai.spec.ts Outdated Show resolved Hide resolved
langfuse-core/src/index.ts Outdated Show resolved Hide resolved
langfuse-core/src/media/LangfuseMedia.ts Show resolved Hide resolved
langfuse-core/src/media/LangfuseMedia.ts Show resolved Hide resolved
langfuse-core/src/openapi/server.ts Show resolved Hide resolved
langfuse-core/src/openapi/server.ts Show resolved Hide resolved
langfuse-core/test/langfuse.media.spec.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)

This PR continues to enhance the multimodal support implementation with important test suite updates to handle asynchronous media operations properly. Here's a focused summary of the latest changes:

Improved test suite reliability for multimodal support by:

  • Updated all test files to use jest.advanceTimersByTimeAsync() for proper async timing control
  • Added explicit flushAsync() calls in Node.js tests for more reliable media upload verification
  • Enhanced CI pipeline with health checks and v3preview docker setup for media handling support
  • Improved OpenAI integration tests to properly verify media processing in streaming/non-streaming modes
  • Added robust error handling around media processing to prevent pipeline failures

The changes demonstrate careful attention to test reliability and proper async handling for the new media features, which is critical for production stability.

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

langfuse-core/test/langfuse.debug.spec.ts Show resolved Hide resolved
.github/workflows/ci.yml Outdated 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)

This PR update focuses on test suite improvements for the multimodal support implementation. Here's a concise summary of the latest changes:

  • Modified langfuse-core/src/index.ts to improve media processing with better traceId extraction and OpenAI format handling
  • Updated test files to use jest.runAllTimersAsync() instead of manual flushAsync() calls for more reliable async testing
  • Added jest.advanceTimersByTimeAsync() between operations in web tests to ensure proper timing control
  • Enhanced error handling in media processing with fallback mechanisms and improved logging
  • Improved cycle detection in recursive media processing using WeakMap to prevent infinite loops

The changes demonstrate a focus on test reliability and proper error handling for the new media features, which is essential for production stability.

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

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
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)

This update focuses on the CI workflow improvements and media handling enhancements. Here's a concise summary of the latest changes:

  • Added cleanup step in CI workflow to remove node_modules after database seeding for better resource management
  • Enhanced media handling in LangfuseMedia.ts with graceful fallbacks for environments without fs/crypto support
  • Added robust error handling for media processing with detailed error messages and logging
  • Improved OpenAI integration with proper media format detection for audio input/output
  • Added health check retries in CI workflow to ensure server availability before running integration tests

The changes show careful attention to CI reliability and cross-environment compatibility for the media handling features.

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

.github/workflows/ci.yml 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 latest changes, here's a focused summary of the key updates:

Added graceful error handling and environment compatibility for media processing:

  • Enhanced LangfuseMedia class with graceful fallbacks when fs/crypto modules are unavailable in browser environments
  • Improved media processing error handling in index.ts by emitting warnings instead of throwing errors for missing traceIds
  • Added proper trace context propagation in callback.ts for media-related span updates
  • Implemented comprehensive test coverage in langfuse.media.spec.ts for media handling features

The changes demonstrate careful attention to cross-environment compatibility and error resilience for the multimodal support implementation, particularly in browser contexts where certain Node.js modules may be unavailable.

4 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, here's a focused summary of the key updates:

Enhanced media handling with improved error handling and environment detection:

  • Added graceful module loading in langfuse-core/src/media/LangfuseMedia.ts with try-catch blocks for fs/crypto modules
  • Improved error handling in langfuse-core/src/index.ts by converting JSON serialization errors to emitted events
  • Added cycle detection using WeakMap in recursive media processing to prevent infinite loops
  • Added support for OpenAI audio format detection and conversion in media processing

The changes focus on making the multimodal support more robust across different environments while preventing potential runtime errors.

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

@hassiebp hassiebp merged commit c64b3a7 into main Nov 18, 2024
9 checks passed
@hassiebp hassiebp deleted the add-multimodal-support branch November 18, 2024 15:04
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.

2 participants