Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion langfuse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
EvaluatorStats,
MapperFunction,
)
from langfuse.experiment import Evaluation
from langfuse.experiment import Evaluation, RegressionError, RunnerContext

from ._client import client as _client_module
from ._client.attributes import LangfuseOtelSpanAttributes
Expand Down Expand Up @@ -63,6 +63,8 @@
"EvaluatorStats",
"BatchEvaluationResumeToken",
"BatchEvaluationResult",
"RunnerContext",
"RegressionError",
"__version__",
"is_default_export_span",
"is_langfuse_span",
Expand Down
151 changes: 151 additions & 0 deletions langfuse/experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
"""

import asyncio
from datetime import datetime
from typing import (
TYPE_CHECKING,
Any,
Awaitable,
Dict,
Expand All @@ -21,6 +23,10 @@
from langfuse.logger import langfuse_logger as logger
from langfuse.types import ExperimentScoreType

if TYPE_CHECKING:
from langfuse._client.client import Langfuse
from langfuse.batch_evaluation import CompositeEvaluatorFunction


class LocalExperimentItem(TypedDict, total=False):
"""Structure for local experiment data items (not from Langfuse datasets).
Expand Down Expand Up @@ -1049,3 +1055,148 @@
)

return langfuse_evaluator


class RunnerContext:
"""Wraps :meth:`Langfuse.run_experiment` with CI-injected defaults.

Intended for use with the ``langfuse/experiment-action`` GitHub Action
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RunnerContext feels like the right abstraction here, and I think the action side should lean into it as the single public integration path: construct the context, then call experiment(context) explicitly instead of also teaching users to read action-specific env vars. Since this is likely to become the primary authoring model, I’d optimize for that clean contract end-to-end.

(https://github.com/langfuse/experiment-action). The action builds a
``RunnerContext`` before invoking the user's ``experiment(context)``
function. Defaults set here (dataset, name, run name, metadata tags) are
applied when the user omits them on the :meth:`run_experiment` call;
users can override any default by passing the corresponding argument
explicitly.
"""

def __init__(
self,
*,
client: "Langfuse",
data: Optional[ExperimentData] = None,
dataset_version: Optional[datetime] = None,
name: Optional[str] = None,
run_name: Optional[str] = None,
metadata: Optional[Dict[str, str]] = None,
):
"""Build a ``RunnerContext`` populated with defaults for ``run_experiment``.

Typically called by the ``langfuse/experiment-action`` GitHub Action,
not by end users directly. Every field except ``client`` is optional:
fields left as ``None`` simply mean the corresponding argument must be
supplied on the :meth:`run_experiment` call.

Args:
client: Initialized Langfuse SDK client used to execute the
experiment. The action creates this from the
``langfuse_public_key`` / ``langfuse_secret_key`` /
``langfuse_base_url`` inputs.
data: Default dataset items to run the experiment on. Accepts
either ``List[LocalExperimentItem]`` or ``List[DatasetItem]``.
Comment thread
wochinge marked this conversation as resolved.
Injected by the action when ``dataset_name`` is configured.
If ``None``, the user must pass ``data=`` to
:meth:`run_experiment`.
dataset_version: Optional pinned dataset version. Injected by the
action when ``dataset_version`` is configured.
name: Default human-readable experiment name (e.g. the action's
``experiment_name`` input). If ``None``, the user must pass
``name=`` to :meth:`run_experiment`.
run_name: Default exact run name. The action typically derives
this from the commit SHA / PR number so that reruns produce
distinct runs in Langfuse.
metadata: Default metadata attached to every experiment trace and
the dataset run. The action injects GitHub-sourced tags (SHA,
PR link, workflow run link, branch, GH user, etc.). Merged
with any ``metadata`` passed to :meth:`run_experiment`, with
user-supplied keys winning on collision.
"""
self.client = client
self.data = data
self.dataset_version = dataset_version
self.name = name
self.run_name = run_name
self.metadata = metadata

def run_experiment(
self,
*,
name: Optional[str] = None,
run_name: Optional[str] = None,
description: Optional[str] = None,
data: Optional[ExperimentData] = None,
task: TaskFunction,
evaluators: List[EvaluatorFunction] = [],
composite_evaluator: Optional["CompositeEvaluatorFunction"] = None,
run_evaluators: List[RunEvaluatorFunction] = [],

Check warning on line 1130 in langfuse/experiment.py

View check run for this annotation

Claude / Claude Code Review

Mutable default arguments in RunnerContext.run_experiment

The `evaluators` and `run_evaluators` parameters in `RunnerContext.run_experiment` use mutable list literals (`[]`) as default argument values, which is a Python anti-pattern — the same default object is shared across all calls at definition time. In current code no caller mutates these lists (they are only iterated), so this is not actively broken, but it mirrors the pre-existing anti-pattern in `Langfuse.run_experiment` (client.py:2377,2379) and should be changed to `None` with an in-body subs
Comment thread
wochinge marked this conversation as resolved.
max_concurrency: int = 50,
metadata: Optional[Dict[str, str]] = None,
_dataset_version: Optional[datetime] = None,
) -> ExperimentResult:
resolved_name = name if name is not None else self.name
if resolved_name is None:
raise ValueError(
"`name` must be provided either on the RunnerContext or the run_experiment call"
)

resolved_data = data if data is not None else self.data
if resolved_data is None:
raise ValueError(
"`data` must be provided either on the RunnerContext or the run_experiment call"
)

resolved_run_name = run_name if run_name is not None else self.run_name
resolved_dataset_version = (
_dataset_version if _dataset_version is not None else self.dataset_version
)

Check failure on line 1150 in langfuse/experiment.py

View check run for this annotation

Claude / Claude Code Review

RunnerContext.run_experiment cannot override run_name or _dataset_version back to None

In RunnerContext.run_experiment, run_name and _dataset_version use None as both the "not provided" sentinel and a legitimate override value, so calling ctx.run_experiment(run_name=None) when the context has self.run_name="pr-123-sha-abc" silently ignores the caller intent and uses the context default instead. The class docstring guarantee that "users can override any default by passing the corresponding argument explicitly" is violated for these two parameters when the desired override is None.
Comment thread
wochinge marked this conversation as resolved.
Outdated

merged_metadata: Optional[Dict[str, str]]
if self.metadata is None and metadata is None:
merged_metadata = None
else:
merged_metadata = {**(self.metadata or {}), **(metadata or {})}

return self.client.run_experiment(
name=resolved_name,
run_name=resolved_run_name,
description=description,
data=resolved_data,
task=task,
evaluators=evaluators,
composite_evaluator=composite_evaluator,
run_evaluators=run_evaluators,
max_concurrency=max_concurrency,
metadata=merged_metadata,
_dataset_version=resolved_dataset_version,
)


class RegressionError(Exception):
"""Raised by a user's ``experiment`` function to signal a CI gate failure.

Intended for use with the ``langfuse/experiment-action`` GitHub Action
(https://github.com/langfuse/experiment-action). The action catches this
exception and, when ``should_fail_on_error`` is enabled, fails the
workflow run and renders a callout in the PR comment using
``metric``/``value``/``threshold`` if supplied, otherwise ``str(exc)``.
"""

def __init__(
self,
*,
result: ExperimentResult,
metric: Optional[str] = None,
value: Optional[float] = None,
threshold: Optional[float] = None,
message: Optional[str] = None,
):
self.result = result
self.metric = metric
self.value = value
self.threshold = threshold
if message is not None:
formatted = message
elif metric is not None:
formatted = f"Regression on `{metric}`: {value} (threshold {threshold})"
else:
formatted = "Experiment regression detected"
super().__init__(formatted)

Check warning on line 1202 in langfuse/experiment.py

View check run for this annotation

Claude / Claude Code Review

RegressionError: missing self.message attribute and misleading partial-metric message

Two issues in `RegressionError.__init__`: (1) the `message` parameter is accepted but never stored as `self.message`, so `exc.message` raises `AttributeError` while every other field (`result`, `metric`, `value`, `threshold`) is correctly stored as an instance attribute; (2) when `metric` is provided but `value` and/or `threshold` are `None`, the format string produces `"Regression on `avg_accuracy`: None (threshold None)"` — confusingly showing Python's `None` where numeric data is expected. Fi
Comment thread
wochinge marked this conversation as resolved.
Comment thread
wochinge marked this conversation as resolved.
Loading
Loading