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): broaden content type support #1011

Merged
merged 17 commits into from
Nov 28, 2024
Merged

Conversation

hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Nov 26, 2024

Important

Expand MediaContentType support and update import paths in documentation and examples.

  • Behavior:
    • Expanded MediaContentType in media_content_type.py to include more image, audio, video, text, and application types.
    • Updated import paths in README.md, reference.md, and client.py to use langfuse.api.client instead of finto.client.
    • Affects examples in comments/client.py, dataset_items/client.py, datasets/client.py, and 10 other files.
  • Misc:
    • Added Python 3.12 to CI in ci.yml.
    • Updated Python version requirement in pyproject.toml to >=3.9,<4.0.

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

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)

Expanded media content type support and improved code quality by enabling linting for all Python files in the langfuse/api directory.

  • Added extensive media content type support in langfuse/api/resources/media/types/media_content_type.py including common image, audio, video, text and application formats
  • Removed linting exclusion for langfuse/api/**/*.py in ci.ruff.toml to enforce consistent code quality standards
  • Updated import paths across documentation and examples to use langfuse.api.client instead of finto.client for better consistency

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)

The PR continues to improve code formatting across the codebase by breaking long lines into multiple lines and adding proper indentation for better readability.

  • Reformatted dictionary definitions and function parameters in all resource files under langfuse/api/resources/ for improved readability
  • Split long import statements into multiple lines in various __init__.py files
  • Maintained consistent indentation and line breaks in error handling blocks across client files
  • Added trailing commas to function parameters in langfuse/api/core/client_wrapper.py

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

langfuse/api/README.md Outdated Show resolved Hide resolved
langfuse/api/resources/comments/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/dataset_items/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/dataset_run_items/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/dataset_run_items/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/models/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/prompts/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/score/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/score/client.py Outdated Show resolved Hide resolved
langfuse/api/resources/score_configs/client.py 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)

Added support for Anthropic and Vertex media formats in the media manager while updating dependency constraints for better compatibility.

  • Added handling for Anthropic media format with type: base64 and media_type fields in media_manager.py
  • Added support for Vertex media format with type: media and mime_type fields in media_manager.py
  • Updated langchain-google-vertexai dependency to >=2.0.0,<3.0.0 in pyproject.toml
  • Added upper bounds to langchain packages (e.g. <0.3, <0.4) in pyproject.toml for stability

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

pyproject.toml 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)

Based on the provided information, I'll summarize the most recent changes that haven't been covered in previous reviews:

No major changes found since last review. The previous reviews have comprehensively covered:

  1. Media content type expansion
  2. Code formatting improvements
  3. Import path standardization
  4. Anthropic and Vertex media format support
  5. Dependency updates

All recent changes appear to be part of these already documented updates.

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

langfuse/api/reference.md Show resolved Hide resolved
langfuse/api/reference.md 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)

Updated Python version support and import paths across the codebase while maintaining backward compatibility.

  • Dropped Python 3.8 support and added Python 3.12 in .github/workflows/ci.yml
  • Relaxed langchain-google-vertexai dependency from >=2.0.0,<3.0.0 to >=1.0.0,<3.0.0 in pyproject.toml
  • Updated minimum Python version from 3.8.1 to 3.9 in pyproject.toml
  • Excluded langfuse/api/**/*.py from linting in ci.ruff.toml

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

langfuse/api/reference.md Show resolved Hide resolved
pyproject.toml 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)

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

Enhanced model extraction functionality and updated test coverage for new LangChain integrations.

  • Added support for BedrockLLM and OllamaLLM model types in langfuse/extract_model.py
  • Updated model name extraction logic for AzureOpenAI to check invocation_params.model before model_name
  • Moved langchain integration packages (ollama, cohere, huggingface) from dev to main dependencies in pyproject.toml
  • Updated test cases in test_extract_model.py to use latest LangChain package structure and model configurations

These changes focus on improving model support and test coverage, which weren't covered in previous reviews that focused on media types, formatting, and Python version updates.

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

pyproject.toml 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 recent changes shown in the files and previous reviews, here's a summary of the latest updates:

Removed AzureOpenAI test support while maintaining AzureChatOpenAI functionality in test suite.

  • Removed AzureOpenAI test case and import from tests/test_extract_model.py while keeping AzureChatOpenAI tests
  • Maintained comprehensive test coverage for other model types (ChatGroq, Ollama, MistralAI, etc.)
  • Test changes align with recent model extraction logic updates for AzureOpenAI

The changes focus specifically on test suite updates that weren't covered in previous reviews about media types, formatting, Python versions and model extraction logic.

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

tests/test_extract_model.py 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 recent changes, I'll provide a summary focusing only on the most recent updates not covered in previous reviews:

Updated dependency management and test configurations in pyproject.toml for improved package organization.

  • Moved langchain integration packages (ollama, cohere, huggingface) back to dev dependencies in pyproject.toml
  • Added requests and anyio as main dependencies in pyproject.toml
  • Updated langchain-google-vertexai version constraint to >=1.0.0,<3.0.0 for better compatibility
  • Added version constraints for new dependencies: idna ^3.7 and anyio ^4.4.0

The changes focus on dependency organization and version constraints that weren't covered in previous reviews about media types, formatting, Python versions and testing updates.

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 provided information and previous reviews, I'll summarize only the most recent changes not covered in previous reviews:

Updated test configurations and added new media content types for broader format support.

  • Added new media content types in media_content_type.py including audio/oga, audio/aac, audio/mp4, audio/flac
  • Added support for additional document formats: application/msword, application/vnd.ms-excel
  • Added application/octet-stream for generic binary data support
  • Added text/csv and text/css for additional text format support

The changes focus on expanding media format support that wasn't fully covered in previous reviews about dependency management, testing updates, and Python version changes.

No 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 recent changes shown in the files and previous reviews, here's a summary of the latest updates:

Updated test suite to accommodate changes in langchain-openai's behavior and model parameter handling.

  • Modified model name assertions in test_extract_model_langchain_openai.py to use substring checks instead of exact matches
  • Updated max_tokens to max_completion_tokens in test cases to match new ChatOpenAI parameter naming
  • Added refusal: None field in OpenAI function call test expectations in test_langchain.py
  • Updated HumanMessage serialization test in test_json.py to verify full message structure

These changes focus on test suite adaptations that weren't covered in previous reviews about media types, Python versions, and dependency management.

6 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 recent changes and previous reviews, I'll summarize only the most recent updates:

Updated CI configuration and test parameters to align with langchain-openai changes.

  • Changed environment variable from LANGFUSE_READ_FROM_CLICKHOUSE_ONLY to LANGFUSE_RETURN_FROM_CLICKHOUSE_ONLY in ci.yml
  • Added required 'model' parameter to AzureChatOpenAI configuration in test_extract_model_langchain_openai.py
  • Maintained Python version matrix up to 3.11 in CI despite broader version support in package requirements

These changes focus on CI/testing updates that weren't covered in previous reviews about media types, dependency management, and test suite adaptations.

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

tests/test_extract_model_langchain_openai.py 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 recent changes and previous reviews, I'll summarize only the most recent updates:

Fixed environment variable naming in CI configuration for Clickhouse integration.

  • Fixed environment variable from LANGFUSE_RETURN_FROM_CLICKHOUSE_ONLY to LANGFUSE_RETURN_FROM_CLICKHOUSE in .github/workflows/ci.yml for proper Clickhouse integration
  • Maintained Python version matrix (3.9-3.11) in CI configuration despite broader package support
  • Removed commented out SERPAPI_API_KEY from CI environment variables

The changes focus on CI configuration improvements that weren't covered in previous reviews about media types, testing updates, and dependency management.

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

@hassiebp hassiebp merged commit b677d0a into main Nov 28, 2024
9 of 10 checks passed
@hassiebp hassiebp deleted the media-additional-types branch November 28, 2024 13:43
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