Skip to content

fix(experiments): await future and custom awaitable results - #1869

Open
Bortlesboat wants to merge 3 commits into
langfuse:mainfrom
Bortlesboat:fix/await-experiment-results
Open

Bortlesboat wants to merge 3 commits into
langfuse:mainfrom
Bortlesboat:fix/await-experiment-results

Conversation

@Bortlesboat

@Bortlesboat Bortlesboat commented Sep 8, 2026 •

Copy link
Copy Markdown

What does this PR do?

Experiments now resolve Future, Task and custom awaitable results before recording task output or evaluating scores. Previously, task output could contain the awaitable object itself, and evaluator scores could be dropped, even though the callback protocols allow Awaitable results.

Task callbacks, item evaluators, run evaluators and composite evaluators all use the corrected handling. Direct values and coroutines retain their existing behavior.

Type of change

  • Bug fix

Verification

Rebased onto main 4aa69a3c on October 14, preserving the new warning and JSON imports. Checked on native Windows with Python 3.13 and the frozen dependency lock:

uv run --frozen pytest tests/unit/test_experiment.py -q
uv run --frozen ruff check .
uv run --frozen ruff format --check langfuse/experiment.py langfuse/_client/client.py tests/unit/test_experiment.py
uv run --frozen mypy langfuse --no-error-summary

All 47 experiment tests pass, including public run_experiment coverage for values, coroutines, pending Futures, Tasks and custom awaitables. The awaitable regressions stub the project-ID lookup so they require no DNS or backend. Composite cases assert both returned evaluations and score creation. Ruff, changed-file formatting and mypy pass.

On the same current main with only the regression tests applied, the nine Future/Task/custom-awaitable cases fail and the six value/coroutine cases pass. Restoring the patch makes all 15 pass within the 47-test suite. Self-review against code_review.md found no new actionable issue; no generated files changed.

The full unit suite, server and live-provider suites were not rerun for this rebase. Older full-suite Windows baseline failures remain historical evidence, not current-head results. Hosted CI still needs maintainer approval; local checks do not establish a hosted pass.

Checklist

  • Self-reviewed the diff using code_review.md.
  • Added tests for behavior changes.
  • Checked docs, examples and .env.template; callback protocols already document awaitable results.
  • Did not hand-edit generated files.
  • Did not commit secrets or credentials.

No service deployment or new monitoring is needed for this SDK result-handling fix.

Agent-assisted: Codex prepared the patch, refreshed the branch and ran the local checks listed above.


Greptile Summary

This PR broadens experiment callback result handling from coroutine-only detection to all Python awaitables.

  • Resolves Futures, Tasks, and custom awaitables returned by experiment tasks.
  • Applies the same handling to item and run evaluators.
  • Adds parameterized unit coverage for direct values, coroutines, Futures, Tasks, and custom awaitables.
  • Leaves the separate composite-evaluator path using coroutine-only detection.

Confidence Score: 4/5

The PR is not yet safe to merge because composite evaluators can still silently lose valid Future, Task, and custom-awaitable results.

Item and run evaluator results are now resolved correctly, but the third supported experiment evaluation layer bypasses the corrected helper and retains the same coroutine-only behavior this PR is intended to fix.

Files Needing Attention: langfuse/experiment.py and langfuse/_client/client.py

Prompt To Fix All With AI
### Issue 1
langfuse/experiment.py:1010
**Composite awaitables remain unresolved**

The new awaitable handling only covers item and run evaluators. Composite evaluators use a separate coroutine-only check, so a composite evaluator returning a Future, Task, or custom awaitable allowed by its callback protocol passes the unresolved object to normalization. The result is silently discarded, and its composite evaluation and score are missing.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(experiments): await future and custo..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Sep 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread langfuse/experiment.py
Bortlesboat added a commit to Bortlesboat/langfuse-python that referenced this pull request Sep 8, 2026
Address review feedback on langfuse#1869 and preserve composite scores from Future, Task and custom awaitable callbacks.

Note: pre-existing failures in prompt, prompt atexit, and Windows path tests are not addressed by this PR.
@Bortlesboat

Copy link
Copy Markdown
Author

The current experiments patch resolves Future/Task/custom-awaitable results in composite evaluators as well as task, item and run callbacks; that includes the composite path raised in the earlier review. CI run 35680552545 for 5002c98 is still action_required, so the refreshed head has not received hosted test coverage.

Could a maintainer approve the current CI run? The related helper-thread exception fix #1868 has the same gate on run 35680551195 at dc56653. Its cancellation/exit propagation and this PR's callback-result changes have separate regressions. The PR bodies retain the local Windows baseline failures and unrun server/provider limits; CLA success is not a test-suite result.

Address review feedback on langfuse#1869 and preserve composite scores from Future, Task and custom awaitable callbacks.

Note: pre-existing failures in prompt, prompt atexit, and Windows path tests are not addressed by this PR.
@Bortlesboat
Bortlesboat force-pushed the fix/await-experiment-results branch from 5002c98 to 60bb24e Compare October 14, 2026 17:11

This branch has not been deployed

No deployments
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