Skip to content

Conversation

@marliessophie
Copy link
Member

@marliessophie marliessophie commented Nov 26, 2025

Important

Adds methods to Langfuse client for fetching and deleting dataset runs with error handling.

  • Methods Added:
    • get_dataset_run(): Fetches a dataset run by dataset_name and run_name, returning DatasetRunWithItems.
    • get_dataset_runs(): Fetches all runs for a dataset, with pagination support, returning PaginatedDatasetRuns.
    • delete_dataset_run(): Deletes a dataset run by dataset_name and run_name, returning DeleteDatasetRunResponse.
  • Error Handling:
    • All methods use handle_fern_exception() for error handling and re-raise exceptions.

This description was created by Ellipsis for 0d9bd8f. You can customize this summary. It will automatically update as commits are pushed.

Disclaimer: Experimental PR review

Greptile Overview

Greptile Summary

Added three new methods to the Langfuse client for managing dataset runs: get_dataset_run, get_dataset_runs, and delete_dataset_run.

  • get_dataset_run: fetches a single dataset run by dataset name and run name
  • get_dataset_runs: retrieves paginated list of runs for a dataset
  • delete_dataset_run: permanently deletes a dataset run and all its items
  • Added required import statements for DatasetRunWithItems, DeleteDatasetRunResponse, and PaginatedDatasetRuns types

Issues found:

  • Missing is_url_param=True flag in _url_encode() calls for path parameters across all three methods. These parameters are embedded in URL paths and passed to httpx, which handles encoding automatically in v0.28+. This inconsistency with the existing get_dataset method (line 2445) could cause double-encoding issues.

Confidence Score: 3/5

  • This PR can be merged after addressing the URL encoding parameter issue.
  • The implementation follows existing patterns and adds straightforward CRUD operations for dataset runs. However, the missing is_url_param=True flags could cause encoding issues with special characters in dataset/run names. The fix is simple but critical for correctness.
  • Fix the _url_encode() calls in langfuse/_client/client.py to include is_url_param=True for all path parameters

Important Files Changed

File Analysis

Filename Score Overview
langfuse/_client/client.py 3/5 added three dataset run methods (get_dataset_run, get_dataset_runs, delete_dataset_run); missing is_url_param=True flag in URL encoding calls for path parameters

Sequence Diagram

sequenceDiagram
    participant User
    participant Langfuse
    participant API as Langfuse API
    
    Note over User,API: Get Dataset Run
    User->>Langfuse: get_dataset_run(dataset_name, run_name)
    Langfuse->>Langfuse: _url_encode(dataset_name)
    Langfuse->>Langfuse: _url_encode(run_name)
    Langfuse->>API: datasets.get_run(dataset_name, run_name)
    API-->>Langfuse: DatasetRunWithItems
    Langfuse-->>User: DatasetRunWithItems
    
    Note over User,API: Get Dataset Runs (Paginated)
    User->>Langfuse: get_dataset_runs(dataset_name, page, limit)
    Langfuse->>Langfuse: _url_encode(dataset_name)
    Langfuse->>API: datasets.get_runs(dataset_name, page, limit)
    API-->>Langfuse: PaginatedDatasetRuns
    Langfuse-->>User: PaginatedDatasetRuns
    
    Note over User,API: Delete Dataset Run
    User->>Langfuse: delete_dataset_run(dataset_name, run_name)
    Langfuse->>Langfuse: _url_encode(dataset_name)
    Langfuse->>Langfuse: _url_encode(run_name)
    Langfuse->>API: datasets.delete_run(dataset_name, run_name)
    API-->>Langfuse: DeleteDatasetRunResponse
    Langfuse-->>User: DeleteDatasetRunResponse
Loading

Copy link
Contributor

@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.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

raise e

def get_dataset_run(
self, dataset_name: str, *, run_name: str
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have all arguments here as keyword arguments


def get_dataset_runs(
self,
dataset_name: str,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have all arguments here as keyword arguments

raise e

def delete_dataset_run(
self, dataset_name: str, *, run_name: str
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have all arguments here as keyword arguments

try:
return self.api.datasets.get_run(
dataset_name=self._url_encode(dataset_name),
run_name=self._url_encode(run_name),
Copy link
Contributor

Choose a reason for hiding this comment

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

If run_name is being passed as a query param, we would have to set url_param=True in _url_encode to avoid double encoding by newer httpx versions.

In any case, we should add a few tests here to cover all works as expected with the encoding 👍🏾

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.

3 participants