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: update prompts and invalidate cache #1082

Merged
merged 17 commits into from
Jan 28, 2025

Conversation

maxdeichmann
Copy link
Member

@maxdeichmann maxdeichmann commented Jan 20, 2025

Important

Add PromptVersionClient for updating prompt versions and modify Langfuse to support prompt updates and cache invalidation.

  • Behavior:
    • Add PromptVersionClient in client.py to update prompt versions with new labels.
    • Modify Langfuse class to include update_prompt() method for updating prompt versions and invalidating cache.
    • Update trace_tags parameter in ScoreClient to accept str or Sequence[str].
  • Tests:
    • Add test_updating_prompt.py to verify prompt update and cache invalidation.
  • Misc:
    • Bump version to 2.57.13a0 in version.py and pyproject.toml.

This description was created by Ellipsis for bb9528e. 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)

This PR adds functionality to update prompt versions and manage cache invalidation in the Langfuse Python SDK.

  • Adds update_prompt() method in test_updating_prompt.py to modify prompt labels and verify cache updates
  • Implements prompt version update functionality through PromptVersionClient and AsyncPromptVersionClient classes
  • Introduces cache invalidation with invalidate() method in PromptCache to remove cached prompts by name
  • Verifies updated prompt values match expected labels after modification through test assertions

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

tests/test_updating_prompt.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)

This PR adds functionality for updating prompt labels and managing cache invalidation in the Langfuse Python SDK. Here's a focused summary of the key changes:

  • Added test in /tests/test_updating_prompt.py to verify prompt label updates and cache behavior
  • Test creates initial prompt, updates labels, and verifies fetched prompt reflects changes
  • Several test coverage gaps identified around cache invalidation, error cases, and content verification

The test file reveals potential improvements needed:

  • Direct cache invalidation verification missing
  • No error case handling for invalid prompts/versions
  • No validation of reserved 'latest' label handling
  • No verification that original prompt content persists after updates

I've focused only on the most recent changes and avoided repeating points from previous reviews. Let me know if you would like me to elaborate on any specific aspect.

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

langfuse/client.py Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Running this should get rid of all the file diffs due to formatting

poetry run ruff format langfuse/api --config ruff.toml

Copy link
Member Author

Choose a reason for hiding this comment

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

Thx! do you have an idea where this error here might come from?

ImportError while importing test module '/Users/maximiliandeichmann/development/github.com/langfuse/langfuse-python/tests/test_updating_prompt.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../../.pyenv/versions/3.9.1/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_updating_prompt.py:1: in <module>
    from langfuse.client import Langfuse
langfuse/__init__.py:3: in <module>
    from .client import Langfuse  # noqa
langfuse/client.py:43: in <module>
    from langfuse.api.resources.prompts.types import (
E   ImportError: cannot import name 'Prompt_Chat' from 'langfuse.api.resources.prompts.types' (/Users/maximiliandeichmann/development/github.com/langfuse/langfuse-python/langfuse/api/resources/prompts/types/__init__.py)

Somehow this fern run changed a lot i think.

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 adds functionality for updating prompt version labels and cache invalidation in the Langfuse Python SDK. Here's a focused summary of the latest changes:

  • Added PromptVersionClient and AsyncPromptVersionClient in /langfuse/api/resources/prompt_version/client.py for managing prompt version updates
  • Implemented PATCH endpoint for updating prompt version labels with proper error handling
  • Added prompt version module to API resources and public interface in __init__.py files
  • Updated API reference documentation in reference.md to include new prompt version update functionality

Key implementation details:

  • Proper error handling for various HTTP status codes (400, 401, 403, 404, 405)
  • Consistent implementation of both sync and async clients
  • Clear documentation of 'latest' label being reserved and managed by Langfuse

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

langfuse/api/resources/prompt_version/client.py Outdated Show resolved Hide resolved
Copy link
Member

@marcklingen marcklingen left a comment

Choose a reason for hiding this comment

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

interface of js and python sdk are inconsistent

langfuse = Langfuse()
langfuse.update_prompt(
    prompt_name="movie-critic",
    prompt_version=1,
    new_labels=["john", "doe"], # assign these labels to the prompt version
)
langfuse = Langfuse();
await langfuse.updatePrompt({
  name: "movie-critic",
  version: 1,
  newLabels: ["john", "doe"],
});

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 updates parameter naming in the Python SDK's client interface for better consistency. Here's a focused summary of the recent changes:

  • Renamed parameters in update_prompt method from prompt_name/prompt_version to name/version in langfuse/client.py for API consistency
  • Updated version to 2.57.13a0 in both version.py and pyproject.toml to reflect interface changes
  • Maintained same functionality while improving parameter naming alignment between Python and TypeScript SDKs

The changes focus on making the interface more consistent across SDKs while preserving the core functionality of updating prompt version labels and cache invalidation.

3 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 focused summary of the recent updates:

Added comprehensive error handling and API documentation for prompt version updates in the Langfuse Python SDK.

  • Added detailed error handling for HTTP status codes (400-405) in /langfuse/api/resources/prompt_version/client.py
  • Documented reserved 'latest' label management by Langfuse in API docstrings
  • Added clear examples and parameter descriptions in /langfuse/api/reference.md for prompt version updates
  • Implemented proper JSON decoding error handling with ApiError class

The changes focus on improving robustness and documentation while avoiding repetition of previously reviewed functionality.

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 recent changes and avoiding repetition from previous reviews, here's a focused summary of the latest updates:

Updated import paths and test utilities in the Langfuse Python SDK for better organization.

  • Refactored import paths from langfuse.api.core to langfuse.core in /langfuse/api/tests/utils/test_http_client.py and test_query_encoding.py
  • Maintained test coverage for query encoding functionality with nested dictionary structures
  • Ensured HTTP client tests continue to verify proper request body handling for JSON, files and additional parameters
  • Updated API reference documentation to reflect consistent parameter naming across endpoints

The changes focus on code organization and import path standardization while maintaining existing test coverage and functionality.

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 and previous reviews, I'll provide a focused summary of the most recent updates:

The changes focus on import path reorganization in test utilities for the Langfuse Python SDK.

  • Updated import path from langfuse.core.query_encoder to langfuse.api.core.query_encoder in test_query_encoding.py
  • Modified import paths in test_http_client.py to use langfuse.api.core instead of langfuse.core
  • Maintained existing test functionality and assertions while improving code organization

These changes are part of a broader refactoring effort to standardize the codebase structure, while ensuring all existing tests continue to validate core functionality correctly.

2 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 focused summary of the recent updates:

Added new prompt version management functionality to the Langfuse Python SDK.

  • Added prompt_version module to API initialization files in /langfuse/api/__init__.py and /langfuse/api/resources/__init__.py
  • Created empty auto-generated initialization file for prompt version module in /langfuse/api/resources/prompt_version/__init__.py
  • Added API reference documentation for prompt version endpoint in /langfuse/api/reference.md
  • Updated version to 2.57.13a0 in both version.py and pyproject.toml to reflect new alpha features

The changes focus on properly exposing and documenting the new prompt version functionality while maintaining consistent code organization.

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

Comment on lines +33 to +35
expected_labels = sorted(["latest", "doe", "production", "john"])
assert sorted(fetched_prompt.labels) == expected_labels
assert sorted(updated_prompt.labels) == expected_labels
Copy link

Choose a reason for hiding this comment

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

logic: Test assumes 'production' label persists after update - verify this is intended behavior

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 focused summary of the recent updates:

Added cache invalidation functionality to the Langfuse Python SDK's prompt management system.

  • Added invalidate() method in /langfuse/prompt_cache.py to remove cached prompts by name
  • Integrated cache invalidation with prompt version updates in Langfuse client
  • Updated test file /tests/test_updating_prompt.py to verify prompt updates and cache behavior
  • Ensured 'production' and 'latest' labels are properly handled in prompt version updates

The changes focus on robust cache management while maintaining data consistency during prompt version updates.

11 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 focused summary of the recent updates:

Simplified type handling and API endpoint paths in the Langfuse Python SDK.

  • Updated API endpoint path from /version/ to /versions/ in langfuse/api/resources/prompt_version/client.py for consistency
  • Simplified trace_tags type annotation in ScoreClient from nested sequence to Union[str, Sequence[str]]
  • Removed unnecessary jsonable_encoder call for trace_tags parameter in API requests
  • Updated example code to demonstrate simpler string-based tag usage

The changes focus on improving API consistency and simplifying type handling while maintaining core functionality.

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

@maxdeichmann maxdeichmann merged commit 0a463cb into main Jan 28, 2025
5 of 10 checks passed
@maxdeichmann maxdeichmann deleted the max/lfe-3587-update-prompt-versions branch January 28, 2025 09:29
@ajram23
Copy link

ajram23 commented Jan 28, 2025

Curious, Will this work address the prompt not found issues I have been having?

@marcklingen
Copy link
Member

Curious, Will this work address the prompt not found issues I have been having?

This feature addresses changes in labels without needing to create a new prompt version. We have until now been unsuccessful in reproducing the issue that you reported afaik.

@ajram23
Copy link

ajram23 commented Jan 29, 2025

Curious, Will this work address the prompt not found issues I have been having?

This feature addresses changes in labels without needing to create a new prompt version. We have until now been unsuccessful in reproducing the issue that you reported afaik.

Oh boy. Let me double check my code to make sure its not an issue at my end cuz I am seeing it regularly.

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.

4 participants