fix(observe): preserve current observation type on span update#1705
Conversation
|
@claude review |
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
langfuse/_client/client.py:1377-1391— The sister methodupdate_current_generation(lines 1304-1309, just above the diff hunk) was not migrated and still unconditionally constructsLangfuseGeneration(otel_span=..., langfuse_client=self), which forceslangfuse.observation.typeback to"generation". BecauseObservationTypeGenerationLike = Literal["generation", "embedding"], an@observe(as_type="embedding", capture_output=False)function that callslangfuse.update_current_generation(output=...)silently downgrades the exported type from"embedding"to"generation"— the exact #1676 symptom on the sibling method. Fix by applying the same dispatch you just added: readLangfuseOtelSpanAttributes.OBSERVATION_TYPEoff the OTEL span and route throughself._get_span_class(...)(the signature widening tostralready supports this).Extended reasoning...
What is wrong
This PR establishes a contract — "
update_current_*must preserve the active observation type" — and implements it forupdate_current_spanatlangfuse/_client/client.py:1377-1393. But the sister methodupdate_current_generationdirectly above it (client.py:1304-1309) was not migrated and still runs:generation = LangfuseGeneration( otel_span=current_otel_span, langfuse_client=self )
LangfuseGeneration.__init__chains toLangfuseObservationWrapper.__init__withas_type="generation", and that base__init__unconditionally rewrites the attribute (langfuse/_client/span.py:120-123):self._otel_span = otel_span self._otel_span.set_attribute( LangfuseOtelSpanAttributes.OBSERVATION_TYPE, as_type )
So every call to
update_current_generationre-stamps the attribute to"generation", regardless of what it was before.Why existing code does not prevent it
ObservationTypeGenerationLikeinconstants.py:15-18isLiteral["generation", "embedding"]—embeddingis a first-class generation-like type the SDK exposes via@observe(as_type="embedding", ...). Withcapture_output=False, the decorator skips its own final output write, so the only output write is whatever the user does inside the function body. If the user reaches forupdate_current_generation(the natural method name for a generation-like observation), the type gets clobbered with no fallback to fix it up.The new
update_current_spanpath guards this by reading the existing OBSERVATION_TYPE attribute off the OTEL span and dispatching through the widened_get_span_class(which now acceptsstr,client.py:1071). That same dispatch infrastructure already supports the embedding/generation case — the call site at line 1307 just was not migrated.Impact
After this PR, the type-preservation contract is half-applied:
update_current_spanpreserves all seven span-like types plus the two generation-like ones, butupdate_current_generationstill downgradesembedding→generation. The PR description says "Fixes #1676", but #1676's root cause (wrapper construction overwriting OBSERVATION_TYPE during an update) is left open on the sibling method.Step-by-step proof
- User code:
@observe(as_type="embedding", capture_output=False) def embed_text(text): vec = compute_embedding(text) langfuse.update_current_generation(output=vec) return vec
- The decorator opens the span with
as_type="embedding".LangfuseObservationWrapper.__init__callsset_attribute(OBSERVATION_TYPE, "embedding"). ✓ - Inside the body,
update_current_generation(output=vec)runs. Atclient.py:1307-1309it constructsLangfuseGeneration(otel_span=current_otel_span, langfuse_client=self). LangfuseGeneration.__init__(span.py:1347-1348) callssuper().__init__(as_type="generation", ...).LangfuseObservationWrapper.__init__atspan.py:121-123unconditionally callsself._otel_span.set_attribute(LangfuseOtelSpanAttributes.OBSERVATION_TYPE, "generation")— overwriting the"embedding"from step 2.capture_output=Falsemeans the decorator's exit path does not re-write OBSERVATION_TYPE either.- Exported span attribute
langfuse.observation.type="generation"instead of"embedding".
How to fix
Apply the same pattern from
update_current_spanat theupdate_current_generationcall site:existing_observation_type = ( current_otel_span.attributes.get( LangfuseOtelSpanAttributes.OBSERVATION_TYPE, "generation" ) if current_otel_span.is_recording() else "generation" ) span_class = self._get_span_class(existing_observation_type) generation = span_class(otel_span=current_otel_span, langfuse_client=self)
A regression test mirroring the new
test_capture_output_false_preserves_type_when_current_span_is_updatedbut withas_type="embedding"andupdate_current_generationwould lock this in. - User code:
What changed
update_current_span(...)is called.@observe(as_type="guardrail", capture_output=False)when the function body sets output viaupdate_current_span(...).Why
update_current_span(...)wrapped the active OTEL span as a genericLangfuseSpan, which rewrotelangfuse.observation.typetospan. Withcapture_output=False, the decorator skips its final output update, so typed observations such asguardrailstayed downgraded tospan.Fixes #1676.
Verification
uv run --frozen ruff check .uv run --frozen mypy langfuse --no-error-summaryuv run --frozen pytest tests/unit/test_observe.py tests/unit/test_otel.py::TestBasicSpans::test_start_as_current_observation_types -qGreptile Summary
This PR fixes a bug where calling
update_current_span(...)on a typed observation (e.g.,as_type="guardrail") would silently downgrade the span'slangfuse.observation.typeattribute to"span"becauseupdate_current_spanunconditionally wrapped the active OTEL span in a plainLangfuseSpan. The issue was most visible withcapture_output=Falsesince the decorator skips its final output write, leaving the clobbered type in place.langfuse/_client/client.py:update_current_spannow reads the existingOBSERVATION_TYPEattribute from the active span before wrapping it, then delegates to the correct typed class via_get_span_class. The_get_span_classsignature is loosened fromObservationTypeLiteraltostrto accommodate runtime attribute values.tests/unit/test_observe.py: Adds a focused async regression test for@observe(as_type="guardrail", capture_output=False)combined with an in-bodyupdate_current_span(output=...)call, asserting both type and output are preserved.Confidence Score: 5/5
The change is safe to merge — it makes a minimal, targeted fix to update_current_span with a direct regression test, and the fallback path for unknown types matches prior behavior exactly.
The fix reads one attribute from the active span and routes to the already-existing _get_span_class helper — no new logic is introduced. The typed subclasses force their own as_type in init, so the round-trip write back to the span attribute is always consistent. The added test directly exercises the previously broken path and confirms both the type and output attributes are preserved.
No files require special attention.
Sequence Diagram
sequenceDiagram participant D as @observe decorator participant F as guardrail_check() participant C as Langfuse.update_current_span() participant S as OTEL Span D->>S: "start span, set OBSERVATION_TYPE="guardrail"" D->>F: invoke function body F->>C: "update_current_span(output={"verdict": "manually set"})" C->>S: read OBSERVATION_TYPE → "guardrail" C->>C: _get_span_class("guardrail") → LangfuseGuardrail C->>S: "LangfuseGuardrail.__init__ sets OBSERVATION_TYPE="guardrail" (preserved)" C->>S: "span.update(output=...) writes output attribute" F-->>D: return True Note over D: capture_output=False → skip final output write D->>S: end span Note over S: OBSERVATION_TYPE="guardrail" ✓, output preserved ✓Reviews (1): Last reviewed commit: "Merge branch 'main' into codex/fix-curre..." | Re-trigger Greptile