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 #493

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

maxdeichmann
Copy link
Member

@maxdeichmann maxdeichmann commented Jan 20, 2025

Important

Adds functionality to update prompt labels and invalidate cache with a new API endpoint and caching mechanism.

  • Behavior:
    • Adds updatePrompt method in LangfuseCore to update prompt labels and invalidate cache.
    • Introduces updatePromptStateless in LangfuseCoreStateless for API interaction.
    • Adds test for updating prompt labels in langfuse-integration-node.spec.ts.
  • API:
    • New PATCH endpoint /api/public/v2/prompts/{promptName}/version/{version} in openapi-server.yaml to update prompt labels.
  • Cache:
    • Adds invalidate method in LangfusePromptCache to clear cache entries by prompt name.

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

Copy link

vercel bot commented Jan 20, 2025

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

Name Status Preview Updated (UTC)
langfuse-js ✅ Ready (Inspect) Visit Preview Jan 21, 2025 9:40am

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 labels and invalidate the cache in the Langfuse SDK. Here are the key changes:

  • Changed HTTP method from PUT to PATCH in updatePromptStateless for more RESTful semantics
  • Added updatePrompt method in LangfuseCore that updates labels and invalidates cache
  • Added test case verifying prompt label updates from ["john"] to ["john", "doe"]
  • Note: Test is marked with .only() which should be removed before merging

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

Comment on lines +445 to +449
const updatedPrompt = await langfuse.updatePrompt({
name: promptName,
version: 1,
newLabels: ["john", "doe"],
});
Copy link

Choose a reason for hiding this comment

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

logic: Test should verify that old labels are preserved when updating with newLabels. Consider testing removal of labels and complete label replacement

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 labels and invalidate the cache in the Langfuse SDK. Here are the key changes:

  • Added new PATCH endpoint /api/public/v2/prompts/{promptName}/version/{version} in openapi-server.yaml for updating prompt labels
  • Added UpdatePromptBody type definition in types.ts for the request payload
  • Added integration test in langfuse-integration-node.spec.ts verifying prompt label updates
  • Added prompt cache invalidation functionality in promptCache.ts

Key points to note:

  • Test is currently marked with .only() which should be removed before merging
  • The PR properly handles cache invalidation when prompt labels are updated
  • The OpenAPI spec changes align with the new 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)

This PR adds functionality to update prompt labels and invalidate caching in the Langfuse SDK. Here are the key changes since the last review:

  • Added invalidate method in LangfusePromptCache to remove cache entries by promptName
  • Added OpenAPI spec for new PATCH endpoint /api/public/v2/prompts/{promptName}/version/{version}
  • Added integration test verifying prompt label updates and cache invalidation
  • Added type definitions for prompt version updates in types.ts

The changes look well-structured and properly tested. The only remaining item is to remove the .only() from the test before merging.

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)

This PR updates the OpenAPI server types to support prompt version updates and cache invalidation. Here are the key changes:

  • Added new PATCH endpoint types in langfuse-core/src/openapi/server.ts for updating prompt version labels at /api/public/v2/prompts/{promptName}/version/{version}
  • Made unit field required in Model schema by removing optional marker
  • Added undefined as valid type for MapValue schema

The changes are focused on type definitions to support the new prompt update functionality while maintaining type safety.

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)

This PR updates the OpenAPI specification and server types to support prompt version updates and cache invalidation in the Langfuse SDK.

  • Added new PATCH endpoint /api/public/v2/prompts/{promptName}/version/{version} in openapi-server.yaml with corresponding type definitions
  • Made unit field optional in Model schema by adding ? to type definition in server.ts
  • Removed undefined as possible type from MapValue schema in server.ts
  • Improved string formatting consistency in OpenAPI spec descriptions

The changes are focused on API specification updates to properly support the new prompt update functionality while maintaining backwards compatibility.

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)

Added new PATCH endpoint in OpenAPI spec for updating prompt version labels and invalidating cache, with key changes:

  • Added /api/public/v2/prompts/{promptName}/version/{version} endpoint in langfuse-core/openapi-spec/openapi-server.yaml with proper authentication and error handling
  • Added invalidate method in langfuse-core/src/prompts/promptCache.ts to remove cache entries by promptName
  • Added UpdatePromptBody type definition in langfuse-core/src/types.ts for request payload validation

The changes provide a clean API for updating prompt labels while ensuring cache consistency.

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

Comment on lines 1489 to 1496
newLabels:
type: array
items:
type: string
description: >-
New labels for the prompt version. Labels are unique across
versions. The 'latest' label is reserved and managed by
Langfuse.
Copy link

Choose a reason for hiding this comment

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

logic: newLabels array should validate that 'latest' is not included since it's a reserved label

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

Added new PATCH endpoint in OpenAPI spec for updating prompt version labels with comprehensive error handling and authentication.

  • Added detailed response codes (200, 400, 401, 403, 404, 405) for /api/public/v2/prompts/{promptName}/version/{version} endpoint
  • Added security requirement for BasicAuth authentication on the new endpoint
  • Added operation ID promptVersion_update for API documentation and client generation
  • Added description "Update labels for a specific prompt version" to clarify endpoint purpose

The changes focus on properly documenting the API contract for the new prompt version update functionality while maintaining security and error handling standards.

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

hassiebp and others added 2 commits January 21, 2025 10:38
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)

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

Removed test-only flag and added proper error handling for the prompt version update endpoint in the OpenAPI specification.

  • Removed .only from test case in integration-test/langfuse-integration-node.spec.ts to enable all tests
  • Added comprehensive HTTP response codes (200, 400, 401, 403, 404, 405) for prompt version endpoint in server.ts
  • Added undefined as valid type for MapValue in OpenAPI schema for optional parameters
  • Added proper authentication requirements for the PATCH endpoint

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

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