Repository navigation
fix(experiments): await future and custom awaitable results - #1869
Open
Bortlesboat wants to merge 3 commits into
Open
Bortlesboat wants to merge 3 commits into
Bortlesboat wants to merge 3 commits into
Conversation
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.
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
force-pushed
the
fix/await-experiment-results
branch
from
October 14, 2026 17:11
5002c98 to
60bb24e
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Experiments now resolve
Future,Taskand 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 allowAwaitableresults.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
Verification
Rebased onto main
4aa69a3con 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-summaryAll 47 experiment tests pass, including public
run_experimentcoverage 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.mdfound 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
code_review.md..env.template; callback protocols already document awaitable results.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.
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
Reviews (1): Last reviewed commit: "fix(experiments): await future and custo..." | Re-trigger Greptile
Context used: