fix(observe): preserve streaming context without output capture #1628
Claude / Claude Code Review
completed
Apr 20, 2026 in 13m 36s
Code review found 1 important issue
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | langfuse/_client/observe.py:571-595 |
Span leaks when capture_output=False generator is abandoned |
| 🟡 Nit | tests/unit/test_observe.py:51-53 |
Misleading comment and missing test coverage for Python 3.10 fallback path |
| 🟡 Nit | tests/unit/test_observe.py:51-53 |
test |
Annotations
Check failure on line 595 in langfuse/_client/observe.py
claude / Claude Code Review
Span leaks when capture_output=False generator is abandoned
After this PR, generators are wrapped unconditionally (including when `capture_output=False`), so `is_return_type_generator=True` and the `finally` block skips `span.end()`; if a consumer abandons the generator without exhausting it, the span leaks indefinitely. Before this PR, `capture_output=False` generators were never wrapped, so `span.end()` was always guaranteed by the `finally` block. Adding `close()`/`__del__` (and `aclose()` for async) to the wrapper classes that call `self.span.end()`
Check warning on line 53 in tests/unit/test_observe.py
claude / Claude Code Review
Misleading comment and missing test coverage for Python 3.10 fallback path
The implementation comments in `_ContextPreservedAsyncGeneratorWrapper.__anext__` are wrong: line 639 says "Python 3.10+ approach with context parameter" but `asyncio.create_task(context=...)` was added in Python **3.11**, not 3.10. Consequently, the fallback comment (line 645) should say "Python < 3.11" instead of "Python < 3.10". The test skipif condition `sys.version_info < (3, 11)` is actually correct, but the misleading comments will confuse future maintainers, and the Python 3.10 fallback
Check warning on line 53 in tests/unit/test_observe.py
claude / Claude Code Review
test
The test `test_streaming_response_preserves_context_without_output_capture` has a `skipif` condition of `sys.version_info < (3, 11)`, but the `_ContextPreservedAsyncGeneratorWrapper` code itself comments "Python 3.10+ approach with context parameter", indicating `asyncio.create_task(context=...)` was added in Python 3.10. Since `pyproject.toml` declares `requires-python = ">=3.10,<4.0"`, the threshold should be `sys.version_info < (3, 10)` (or the guard removed entirely), so Python 3.10—the mini
Loading