fix: expose stable Langfuse trace IO for evaluators - #14089
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughLangfuse tracing now derives root trace input and output from graph boundary components, normalizes message values, supports deterministic selection and fallbacks, and exposes stable evaluator-oriented shapes. Compatibility tests cover marked, unmarked, multi-output, dual-role, and nested-message scenarios. ChangesLangfuse trace IO
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Flow
participant LangFuseTracer
participant Langfuse
Flow->>LangFuseTracer: add traces with boundary vertices
LangFuseTracer->>LangFuseTracer: serialize and select boundary input/output
LangFuseTracer->>Langfuse: update root trace with wrapped IO
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
bug
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/base/langflow/services/tracing/langfuse.py`:
- Around line 54-81: Update _trace_boundary_value to collapse the result based
on the number of serialized values actually collected: return the sole value
when len(values) == 1, otherwise return the values list. Preserve the existing
empty-result behavior, and add a regression test in
test_langfuse_v3_compatibility.py for two marked output components where one
produces an empty dict.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 271079a9-5331-473c-ab7e-e64aade2c41a
📒 Files selected for processing (2)
src/backend/base/langflow/services/tracing/langfuse.pysrc/backend/tests/unit/services/tracing/test_langfuse_v3_compatibility.py
| def _trace_boundary_value( | ||
| component_values: dict[str, Any], | ||
| boundary_traces: dict[str, str], | ||
| *, | ||
| fallback_component_values: dict[str, Any] | None = None, | ||
| prefer_fallback_trace_ids: set[str] | None = None, | ||
| ) -> tuple[bool, Any]: | ||
| """Return marked graph-boundary outputs in deterministic component-id order.""" | ||
| values = [] | ||
| prefer_fallback_trace_ids = prefer_fallback_trace_ids or set() | ||
| for trace_id, trace_name in sorted(boundary_traces.items()): | ||
| sources = ( | ||
| (fallback_component_values, component_values) | ||
| if trace_id in prefer_fallback_trace_ids | ||
| else (component_values, fallback_component_values) | ||
| ) | ||
| for source in sources: | ||
| if source is None or trace_name not in source: | ||
| continue | ||
| component_value = source[trace_name] | ||
| if isinstance(component_value, dict) and not component_value: | ||
| continue | ||
| values.append(_serialize_component_boundary(component_value)) | ||
| break | ||
|
|
||
| if not values: | ||
| return False, None | ||
| return True, values[0] if len(boundary_traces) == 1 else values |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scalar/list collapse should be based on how many values were actually found, not how many components were marked.
values[0] if len(boundary_traces) == 1 else values (Line 81) uses the count of registered boundary trace names, not the count of values that actually survived the empty-dict skip at Line 74-75. If two components are marked on the same side (e.g. two output-marked components) but one of them produces {} for a given execution (common with conditional/router branches), only one value ends up in values, yet the function still returns it wrapped in a single-element list instead of collapsing to a scalar — because len(boundary_traces) == 2. This breaks the "stable evaluator-addressable scalar" contract the PR is meant to guarantee, since the same flow can non-deterministically return a scalar or a one-item list for output/input depending on which branch executed.
None of the added tests exercise this "mixed marked components, one contributes nothing" case, so it currently slips through.
🐛 Proposed fix
if not values:
return False, None
- return True, values[0] if len(boundary_traces) == 1 else values
+ return True, values[0] if len(values) == 1 else valuesConsider also adding a regression test in test_langfuse_v3_compatibility.py covering two marked output components where one yields {}.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _trace_boundary_value( | |
| component_values: dict[str, Any], | |
| boundary_traces: dict[str, str], | |
| *, | |
| fallback_component_values: dict[str, Any] | None = None, | |
| prefer_fallback_trace_ids: set[str] | None = None, | |
| ) -> tuple[bool, Any]: | |
| """Return marked graph-boundary outputs in deterministic component-id order.""" | |
| values = [] | |
| prefer_fallback_trace_ids = prefer_fallback_trace_ids or set() | |
| for trace_id, trace_name in sorted(boundary_traces.items()): | |
| sources = ( | |
| (fallback_component_values, component_values) | |
| if trace_id in prefer_fallback_trace_ids | |
| else (component_values, fallback_component_values) | |
| ) | |
| for source in sources: | |
| if source is None or trace_name not in source: | |
| continue | |
| component_value = source[trace_name] | |
| if isinstance(component_value, dict) and not component_value: | |
| continue | |
| values.append(_serialize_component_boundary(component_value)) | |
| break | |
| if not values: | |
| return False, None | |
| return True, values[0] if len(boundary_traces) == 1 else values | |
| def _trace_boundary_value( | |
| component_values: dict[str, Any], | |
| boundary_traces: dict[str, str], | |
| *, | |
| fallback_component_values: dict[str, Any] | None = None, | |
| prefer_fallback_trace_ids: set[str] | None = None, | |
| ) -> tuple[bool, Any]: | |
| """Return marked graph-boundary outputs in deterministic component-id order.""" | |
| values = [] | |
| prefer_fallback_trace_ids = prefer_fallback_trace_ids or set() | |
| for trace_id, trace_name in sorted(boundary_traces.items()): | |
| sources = ( | |
| (fallback_component_values, component_values) | |
| if trace_id in prefer_fallback_trace_ids | |
| else (component_values, fallback_component_values) | |
| ) | |
| for source in sources: | |
| if source is None or trace_name not in source: | |
| continue | |
| component_value = source[trace_name] | |
| if isinstance(component_value, dict) and not component_value: | |
| continue | |
| values.append(_serialize_component_boundary(component_value)) | |
| break | |
| if not values: | |
| return False, None | |
| return True, values[0] if len(values) == 1 else values |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/base/langflow/services/tracing/langfuse.py` around lines 54 - 81,
Update _trace_boundary_value to collapse the result based on the number of
serialized values actually collected: return the sole value when len(values) ==
1, otherwise return the values list. Preserve the existing empty-result
behavior, and add a regression test in test_langfuse_v3_compatibility.py for two
marked output components where one produces an empty dict.
github-actions
Bot
added
bug
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.11.0 #14089 +/- ##
==================================================
+ Coverage 60.87% 61.19% +0.31%
==================================================
Files 2375 2426 +51
Lines 234025 236020 +1995
Branches 32939 35781 +2842
==================================================
+ Hits 142472 144441 +1969
- Misses 89842 89866 +24
- Partials 1711 1713 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
added
bug
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
1 participant
Summary
release-1.11.0inputandoutputkeys on Langfuse trace IOThe cherry-picked patch is identical to
1ac2e19551f4e0e148eef0ec7ad7acaf861a6a65; the 1.11-only Langfuse__deepcopy__safeguard remains intact.Reported by Naveed Syed (Solis).
Validation
test_langfuse_v3_compatibility.py: 30 passedtest_tracing_service.py: 20 passedtest_langfuse_orphan_generation.py: 9 passedgit diff --checkpassedgit range-diffreports an identical patch to fix: expose stable Langfuse trace IO for evaluators #14087Summary by CodeRabbit