Skip to content

fix(langfuse): preserve Responses tool input items - #84186

Open
wo-o wants to merge 1 commit into
NousResearch:mainfrom
wo-o:fix/langfuse-responses-input-items
Open

fix(langfuse): preserve Responses tool input items#84186
wo-o wants to merge 1 commit into
NousResearch:mainfrom
wo-o:fix/langfuse-responses-input-items

Conversation

@wo-o

@wo-o wo-o commented Aug 12, 2026

Copy link
Copy Markdown

Summary

  • preserve OpenAI Responses function_call fields in Langfuse generation inputs
  • preserve matching function_call_output payloads instead of rendering them as role: null, content: null
  • continue applying the existing _safe_value truncation, JSON parsing, depth, and data-URI redaction rules

Root cause

pre_api_request passes the actual Responses API input array to the Langfuse plugin, but _serialize_messages() treated every item as a chat message and retained only role and content. Responses tool items use type, call_id, name, arguments, and output, so those fields were dropped from the generation observation.

Test plan

  • Added regression coverage for function_call and function_call_output serialization
  • HERMES_PYTHON=$(PYENV_VERSION=3.12.12 pyenv which python) scripts/run_tests.sh tests/plugins/test_langfuse_plugin.py -q (25 passed)
  • PYENV_VERSION=3.12.12 python -m ruff check plugins/observability/langfuse/__init__.py tests/plugins/test_langfuse_plugin.py
  • Independent review: no security concerns or logic errors

@wo-o

wo-o commented Aug 12, 2026

Copy link
Copy Markdown
Author

Privacy/history note after tracing the original implementation:

  • This does not appear to restore fields that were intentionally suppressed. Merged PR fix(langfuse): complete observability fix — trace I/O, tool outputs, placeholder credentials (closes #22342, #22763) #26320 explicitly aimed to show the real outbound LLM payload and tool outputs in Langfuse, and the shipped docs currently say generation inputs plus sanitized tool args/results are exported.
  • The null entries are a schema mismatch: _serialize_messages() handles chat-style role/content, while Codex Responses passes function_call / function_call_output items using type, call_id, arguments, and output.
  • The same tool arguments/results are already exported today in separate Tool: ... observations by pre_tool_call / post_tool_call; this PR also preserves them in the generation input, matching the provider request. It therefore duplicates already-exported content rather than introducing a new payload category.

There is still a valid broader privacy concern: #67544 documents that content exports are unredacted, and open PR #64236 proposes explicit none / metadata / content consent with a fail-closed default. This PR should remain compatible with that policy if it lands: Responses tool payloads should only be serialized in its explicit content mode, and omitted in metadata mode.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins provider/openai OpenAI / Codex Responses API telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(langfuse): preserve Responses tool input items

  1. The new function_call / function_call_output branches only preserve call_id, name, and arguments/output. Responses API items also carry an item id and (for outputs) a status_code. If a provider omits call_id or uses the item id as the pairing key, the call/output pair will not link in Langfuse. Consider also capturing id (and optionally status_code) when present.

  2. arguments is passed through _safe_value(..., parse_json_strings=True) — for a function_call whose arguments is not valid JSON this presumably degrades to the raw string, but the new test only covers the well-formed JSON case. A quick test for non-JSON arguments (or a comment stating the fallback) would lock in the behavior.

No blocking issues found. The change is small, focused, and placed before the role-based path so Responses-style items bypass role handling entirely — looks right.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(langfuse): preserve Responses tool input items

  1. plugins/observability/langfuse/__init__.py (_serialize_messages): the new function_call / function_call_output branches export the raw tool arguments and output into the generation input. That content is already exported by pre_tool_call / post_tool_call observations and is unredacted — the same privacy concern tracked in observability/langfuse: traces export unredacted inputs/outputs; no masking hook and no user_id #67544 (unredacted content exports) and fix(langfuse): require explicit export consent #64236 (proposed none / metadata / content consent modes). If those modes land, this serialization should only run in an explicit content mode and be omitted in metadata mode; consider wiring it to the export-mode gate now rather than later.

  2. Other Responses item types that carry no role — e.g. reasoning items or message items in that shape — fall through to the chat-style branch, where role = message.get("role") is None and the item is appended anyway. Suggest an explicit fallthrough: either skip unknown item types or serialize them deliberately, so a future Responses item type doesn't silently emit a role-less entry.

  3. _safe_value(message.get("output"), parse_json_strings=True) assumes output is a JSON string. For tools whose output is already a parsed dict/list, or free text that isn't JSON (terminal output, long logs), confirm _safe_value degrades gracefully (returns the raw value rather than raising or dropping it) — if it doesn't, an unparseable output would be silently omitted from the trace.

  4. Minor: call_id is serialized as-is and may be None for a function_call_output item; if the downstream backfill logic keys on call_id, an output without one will never be matched. A test covering a missing call_id would pin the intended behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants