Skip to content

fix(propagation): coerce non-string metadata values to string#1681

Open
aryxnn wants to merge 1 commit into
langfuse:mainfrom
aryxnn:fix-dataset-double-encoding
Open

fix(propagation): coerce non-string metadata values to string#1681
aryxnn wants to merge 1 commit into
langfuse:mainfrom
aryxnn:fix-dataset-double-encoding

Conversation

@aryxnn
Copy link
Copy Markdown

@aryxnn aryxnn commented May 31, 2026

Allows propagate_attributes to accept and automatically convert non-string metadata values (int, float, bool) to strings instead of silently dropping them. This resolves tracing compatibility issues with LangGraph integrations.

What does this PR do?

PR title must follow Conventional Commits, for example feat: add dataset scoring helper or fix(openai): preserve trace context.

Fixes #

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Refactor
  • Documentation update
  • Tooling, CI, or repo maintenance

Verification

List the main commands you ran:

Checklist

  • I self-reviewed the diff using code_review.md.
  • I added or updated tests for behavior changes.
  • I updated docs, examples, or .env.template if needed.
  • I did not hand-edit generated files; if generated files changed, I used the upstream regeneration path.
  • I did not commit secrets or credentials.

Greptile Summary

This PR fixes propagate_attributes to accept non-string metadata values (int, float, bool) and coerce them to strings rather than silently dropping them, improving compatibility with LangGraph integrations that pass typed metadata.

  • propagation.py: Widens the metadata type hint from Dict[str, str] to Dict[str, Any], adds an explicit None-skip guard, and converts non-string values with str() before the existing _validate_string_value check.
  • test_propagate_attributes.py: Adds a new test covering int, float, and bool coercion; note that the asserted "True" for bool_val matches Python's str(True) behaviour, not JSON-canonical "true".

Confidence Score: 4/5

Safe to merge with minor follow-up consideration around bool stringification format.

The coercion logic is straightforward and an improvement over silently dropping values. The main open question is whether str(True) → 'True' is the right representation for downstream consumers that expect JSON-canonical 'true', and whether complex types (dict, list) should be explicitly rejected rather than stringified with Python repr. Neither concern blocks existing functionality — they are edge-case surprises rather than regressions.

The coercion path in langfuse/_client/propagation.py lines 287-291 deserves a second look specifically for bool and complex-type handling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[metadata dict entry] --> B{value is None?}
    B -- yes --> C[skip / drop]
    B -- no --> D{isinstance str?}
    D -- yes --> E[use as-is]
    D -- no --> F["str(value) — coerce to string\n(bool→'True', int→'42', dict→repr)"]
    E --> G[_validate_string_value]
    F --> G
    G -- len > 200 chars --> H[warn + drop]
    G -- not a str --> I[warn + drop]
    G -- valid --> J[add to validated_metadata]
    J --> K[set propagated attribute on context]
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
langfuse/_client/propagation.py:289-291
**`str()` on complex types produces Python repr, not JSON**

The new type signature accepts `Dict[str, Any]`, so callers who pass `dict`, `list`, or arbitrary objects will get Python repr strings (e.g. `"{'nested': 'val'}"`) rather than JSON (`'{"nested": "val"}'`). These are also likely to exceed the 200-character limit and be silently dropped. The PR description and tests only cover scalar types (int, float, bool); consider restricting the accepted value types to those with documented coercion behavior, or adding a JSON fallback for container types.

### Issue 2 of 3
langfuse/_client/propagation.py:289
**`str(True)` gives `"True"`, not JSON-canonical `"true"`**

Python's `str()` renders booleans as `"True"`/`"False"` (capital first letter), which differs from the JSON representation (`"true"`/`"false"`) that many integrations, including LangGraph, typically expect. The test explicitly asserts `"True"`, so this will stay silently inconsistent with JSON-based consumers. A simple guard can emit the JSON-canonical form.

```suggestion
            if isinstance(value, bool):
                string_value = "true" if value else "false"
            elif isinstance(value, str):
                string_value = value
            else:
                string_value = str(value)
```

### Issue 3 of 3
tests/unit/test_propagate_attributes.py:489-492
Missing blank line between the preceding test method and the new one (PEP 8 / existing style in this file), and trailing whitespace on the last assertion line.

```suggestion
        self.verify_missing_attribute(
            child_span, f"{LangfuseOtelSpanAttributes.TRACE_METADATA}.invalid_key"
        )

    def test_metadata_coercion_of_non_string_values(self, langfuse_client, memory_exporter):
```

Reviews (1): Last reviewed commit: "fix(propagation): coerce non-string meta..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Copy link
Copy Markdown

@claude claude Bot left a comment

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
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Aryan Srivastava seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread langfuse/_client/propagation.py Outdated
Comment thread langfuse/_client/propagation.py Outdated
@aryxnn aryxnn force-pushed the fix-dataset-double-encoding branch from 076e5a2 to 1671682 Compare May 31, 2026 22:29
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