feat(media): add utility to resolve media references

[hassiebp]

Important

Adds utility to resolve media references in Langfuse by converting them to base64 data URIs, with new methods, types, and tests.

• Behavior:
  • Adds resolveMediaReferences method in LangfuseMedia to replace media reference strings with base64 data URIs.
  • New test in langfuse-integration-fetch.spec.ts to verify media reference replacement.
• Core Changes:
  • Adds getMediaById method in LangfuseCore to fetch media by ID.
  • Exports LangfuseMedia from index.ts.
• Types:
  • Adds GetMediaResponse type in types.ts for media fetching response.
• Misc:
  • Updates LangfuseCoreTestClient to mock arrayBuffer response.

^{This description was created by for 270fd1e. It will automatically update as commits are pushed.}

[vercel]

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

Name	Status	Preview	Updated (UTC)
langfuse-js	✅ Ready (Inspect)	Visit Preview	Dec 11, 2024 6:38pm

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

This PR adds media reference resolution functionality to the Langfuse SDK, allowing conversion between media reference strings and base64 data URIs for handling binary content like audio files.

• Added resolveMediaReferences static method in LangfuseMedia class to recursively traverse and replace media references with base64 data URIs
• Added arrayBuffer() support to LangfuseFetchResponse type and implemented in test client for binary data handling
• Added _getMediaById protected method in LangfuseCore for fetching media content by ID
• Added comprehensive integration test for audio file encoding/decoding and reference string consistency
• Added proper error handling and depth limits (max 10 levels) for recursive object traversal

_{6 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the provided files and previous review, I'll focus on the most recent significant changes not already covered:

Adds media reference resolution functionality with improved error handling and binary data support.

• Fixed arrayBuffer() implementation in langfuse-node/src/fetch.ts to properly handle binary data conversion
• Added proper error handling for failed media content fetches in LangfuseMedia.resolveMediaReferences
• Added type-safe media reference string parsing with validation in LangfuseMedia.parseReferenceString
• Added concurrent media content fetching with Promise.all for better performance
• Added proper caching of resolved media references to avoid duplicate fetches

_{6 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes, I'll provide a summary focusing on the latest updates not covered in previous reviews:

Added robust media reference handling with improved testing and error recovery.

• Added fallback behavior in LangfuseMedia.resolveMediaReferences to preserve original reference strings when media fetch fails
• Added test in langfuse-integration-fetch.spec.ts to verify reference string consistency across multiple uses
• Added dynamic import wrappers in LangfuseMedia.ts to prevent bundler resolution issues
• Added proper cleanup in integration tests to prevent test interdependencies

The changes focus on reliability and error handling while maintaining backward compatibility.

_{3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes, here's a summary focusing on the latest updates not covered in previous reviews:

Renamed media-related methods and improved media reference handling in the Langfuse SDK.

• Renamed _getMediaById to _fetchMediaById in langfuse-core/src/index.ts for better API consistency
• Added media reference string replacement test in /integration-test/langfuse-integration-fetch.spec.ts to verify base64 data handling
• Reduced debug output in langfuse-integration-vercel.spec.ts by setting debug flag to false
• Added media reference parsing with validation in LangfuseMedia.parseReferenceString for improved type safety

The changes maintain a consistent naming convention for fetch operations while improving test coverage and reducing noise in test output.

_{4 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes, here's a summary focusing on the latest updates not covered in previous reviews:

Improved media reference handling with optimized string replacement and error handling.

• Changed _fetchMediaById to _fetchMedia in langfuse-core/src/index.ts for API consistency
• Added replaceAll in LangfuseMedia.resolveMediaReferences to handle multiple occurrences of media references
• Added proper error handling for non-200 media content responses in LangfuseMedia.resolveMediaReferences
• Added Map for caching resolved media references to avoid duplicate string replacements

The changes focus on robustness and efficiency in media reference resolution while maintaining consistent API naming.

_{3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes shown in the files, here's a focused summary of the latest updates:

Added dynamic import handling and environment detection for media processing in the Langfuse SDK.

• Added dynamic import wrapper in LangfuseMedia.ts to prevent bundlers from incorrectly resolving fs/crypto imports
• Added environment detection logic to properly initialize crypto module for both Node.js and browser contexts
• Added error handling for file system operations when running in non-Node environments
• Added proper error propagation in media content fetch with status code validation

The changes focus on improving cross-environment compatibility while maintaining robust error handling for media operations.

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

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes shown in the files, I'll provide a summary focusing on the latest updates not covered in previous reviews:

Updated CI workflow configuration to improve Docker networking and media handling in integration tests.

• Modified S3 event upload endpoint in CI workflow from localhost:9090 to host.docker.internal:9090 for proper Docker container communication
• Added health check step in CI workflow to verify Langfuse server availability before running integration tests
• Added proper error logging for failed health checks to aid debugging
• Added retry mechanism for server health checks with configurable max attempts

The changes focus on improving reliability of integration tests in CI pipeline, particularly for testing the new media reference functionality.

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

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes shown in the files, here's a focused summary of the latest updates:

Added dynamic import handling and environment-specific media processing in LangfuseMedia.

• Added dynamic import wrapper in langfuse-core/src/media/LangfuseMedia.ts to prevent bundlers from incorrectly resolving Node.js imports
• Added environment detection to initialize fs/crypto modules appropriately for Node.js and browser contexts
• Added proper error handling for file system operations in non-Node environments
• Added media content type validation and parsing for base64 data URIs

The changes focus on improving cross-platform compatibility while ensuring robust media handling across different environments.

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

[greptile-apps]

Disclaimer: Experimental PR review

PR Summary

(updates since last review)

Based on the recent changes shown in the files, here's a focused summary of the latest updates not previously covered:

Improved media handling with dynamic imports and environment-specific optimizations in the Langfuse SDK.

• Added webpackIgnore comment in dynamic imports to prevent incorrect bundler resolution of Node.js modules
• Added referenceStringToMediaContentMap for efficient caching of media content during batch replacements
• Added proper error propagation in resolveMediaReferences with detailed warning messages
• Added Promise.all for concurrent media reference resolution to improve performance
• Added proper cleanup of resolved media references to prevent memory leaks

The changes focus on improving bundling compatibility and performance while maintaining robust error handling.

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