Skip to content

fix(openai): treat openai Omit sentinels like NotGiven when logging - #1805

Merged
hassiebp merged 5 commits into
langfuse:mainfrom
DavidTraina:fix/openai-omit-sentinels
Aug 11, 2026
Merged

fix(openai): treat openai Omit sentinels like NotGiven when logging#1805
hassiebp merged 5 commits into
langfuse:mainfrom
DavidTraina:fix/openai-omit-sentinels

Conversation

@DavidTraina

@DavidTraina DavidTraina commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Addresses langfuse/langfuse#10057

openai v2 ships the Omit sentinel alongside NotGiven for unset params, and callers commonly forward it: the openai-agents SDK (>= 0.4.0) passes Omit for unset params such as temperature, top_p, tool_choice, parallel_tool_calls, and instructions (see _non_null_or_omit in agents/models/openai_responses.py), and langchain-openai does too (the linked issue reproduces with langchain-openai + openai 2.6.1, no agents SDK involved). The isinstance(metadata, NotGiven) TypeError crash in the linked issue's traceback is already guarded on current main; what remains is that the OpenAI instrumentation only filtered NotGiven, so Omit sentinels leaked into logged generations as junk like "<openai.Omit object at 0x...>" (str-repr paths) or {} (EventSerializer output):

  • _extract_responses_prompt: an Omit instructions fabricated a system message whose content was the Omit repr; Omit tool_choice / parallel_tool_calls / tools / input leaked into the logged input.
  • Model-parameter extraction in _get_langfuse_data_from_kwargs: Omit temperature, top_p, etc. were logged as sentinel objects instead of falling back to defaults.
  • _serialize_openai_value / metadata handling: Omit serialized to its str() repr.
  • _extract_chat_prompt (Chat Completions path): functions / function_call / tools were only checked against None, so both NotGiven and Omit sentinels leaked into the logged input.

This PR treats Omit exactly like NotGiven on all logging paths: _is_not_given now checks against (NotGiven, Omit), all direct isinstance(..., NotGiven) checks route through _is_not_given, and _extract_chat_prompt filters sentinels too.

Relation to #1426: that PR only patches the metadata validation path in _get_langfuse_data_from_kwargs (which no longer raises on current main). This PR is a superset — it covers that metadata case plus the prompt/input fields, model parameters, and the chat-completions path, with unit tests.

Type of change

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

Verification

List the main commands you ran:

uv sync --locked
uv run --frozen ruff check .            # All checks passed!
uv run --frozen ruff format .           # no-op on changed files
uv run --frozen mypy langfuse --no-error-summary  # exit 0
uv run --frozen pytest tests/unit/test_openai_prompt_extraction.py  # 23 passed (9 of the new cases fail without the fix)
uv run --frozen pytest tests/unit/test_openai.py tests/unit/test_openai_prompt_extraction.py  # 57 passed
uv run --frozen pytest -n auto --dist worksteal tests/unit  # 685 passed, 2 skipped

Note: uv run --frozen ruff format --check . flags tests/unit/test_media.py, but that drift pre-exists on main and is untouched here. e2e / live_provider suites were not run (unit-testable instrumentation change).

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.

🤖 Generated with Claude Code

Greptile Summary

This PR updates the OpenAI integration’s prompt, metadata, model-parameter, serialization, and raw-response extraction paths to treat Omit as an unset sentinel alongside NotGiven.

  • Centralizes both sentinel checks in _is_not_given.
  • Filters unset tool-related fields from Responses and Chat Completions logging.
  • Adds unit coverage for top-level, nested, prompt, metadata, and model-parameter handling.

Confidence Score: 4/5

This PR should not merge until the Omit import is made compatible with supported older OpenAI SDK versions.

The sentinel filtering itself is consistently implemented and tested, but importing the integration now fails before its existing version-specific compatibility paths can run when Omit is unavailable.

Files Needing Attention: langfuse/openai.py

Prompt To Fix All With AI
### Issue 1
langfuse/openai.py:35
**Legacy SDK import breaks**

When `langfuse.openai` is imported with a supported OpenAI SDK version that predates `Omit`, the unconditional `openai._types.Omit` import raises `ImportError` before the existing version-specific compatibility paths run, preventing all OpenAI instrumentation from loading.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(openai): treat openai Omit sentinels..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

@DavidTraina
DavidTraina force-pushed the fix/openai-omit-sentinels branch from 4c14706 to fa50c2f Compare August 10, 2026 22:33
@DavidTraina
DavidTraina marked this pull request as ready for review August 10, 2026 22:45

@claude claude Bot left a comment

Copy link
Copy Markdown

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.

Comment thread langfuse/openai.py Outdated
@hassiebp
hassiebp merged commit 0bc39fb into langfuse:main Aug 11, 2026
15 of 18 checks passed
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