Skip to content

fix(openai): preserve refusal text in traces - #1960

Open
ch-z-hc wants to merge 2 commits into
langfuse:mainfrom
ch-z-hc:fix/openai-refusal-output
Open

ch-z-hc wants to merge 2 commits into
langfuse:mainfrom
ch-z-hc:fix/openai-refusal-output

Conversation

@ch-z-hc

@ch-z-hc ch-z-hc commented Oct 8, 2026 •

Copy link
Copy Markdown

What does this PR do?

Preserves OpenAI Chat Completions refusal text in generation output, including streamed responses.

When OpenAI returns content=None and a refusal, the SDK consumer receives the refusal but the Langfuse trace loses it. Non-streaming output only contains role and content; refusal-only streams have no recorded output. This makes refused requests indistinguishable from empty responses when inspecting traces or evaluating model behavior.

The patch copies non-null refusal into chat output and accumulates streamed refusal deltas. Existing output shapes for responses without a refusal are unchanged. The field is part of the OpenAI Chat Completions streaming contract.

The regression test exercises the actual OpenAI SDK with HTTPX MockTransport and the in-memory exporter, covering sync/async clients and streaming/non-streaming responses. It verifies both the application-visible response and recorded output/usage. Before the fix, all four new cases fail; the other 37 tests in test_openai.py pass.

Type of change

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

Verification

Python 3.12, locked dependencies:

uv sync --locked --python 3.12
# Windows, using the locked virtual environment:
.venv/Scripts/python.exe -m pytest tests/unit/test_openai.py tests/unit/test_openai_prompt_extraction.py -q
# Native Linux, with CI's dummy credentials and PYTEST_XDIST_AUTO_NUM_WORKERS=4:
uv run --frozen pytest -n auto --dist worksteal tests/unit -q
uv run --frozen ruff check .
uv run --frozen mypy langfuse --no-error-summary
uv run --frozen ruff format --check langfuse/openai.py tests/unit/test_openai.py
uv run --frozen pre-commit run --files langfuse/openai.py tests/unit/test_openai.py
git diff --check
  • Affected unit suites: 64 passed on Windows.
  • Full unit suite: 889 passed, 2 skipped on native Linux, using CI's dummy Langfuse/OpenAI credentials and four workers. Both skips are existing explicit skips (test_shutdown_and_flush, test_span_metadata_updates_in_async_context). No test was disabled or weakened for this patch.
  • Ruff, mypy, changed-file formatting and applicable pre-commit hooks pass. The lock hook has no applicable files.
  • Repository-wide ruff format --check . reports an existing formatting issue in unchanged tests/unit/test_media.py; left outside this patch.
  • The first Windows full-suite run had missing-credential errors plus bare-python subprocess and POSIX-path failures. The relevant failures also reproduce on the unmodified base. The Linux run above passes the complete unit suite.
  • E2E/live-provider suites were not run: the SDK parsing and exporter behavior are covered offline, without a running Langfuse server or paid provider calls. Other Python versions remain for CI.

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. (Not needed: no examples or configuration become stale.)
  • I did not hand-edit generated files; if generated files changed, I used the upstream regeneration path.
  • I did not commit secrets or credentials.

AI assistance: Codex assisted with investigation, implementation, tests and diff self-review. This is not a claim of independent human review.

RetriggerConfidence Score: 4/5

The refusal fix looks sound, but the new test must satisfy the repository’s import rule before merging.

Summary

Preserves OpenAI refusal text in completed and streamed chat traces.

  • Adds offline coverage for synchronous and asynchronous clients, with and without streaming.
  • Existing output paths remain unchanged when refusal is absent.
  • Move the test’s httpx import to the module’s import block to satisfy the repository rule.

Reviews (1) · Last reviewed commit: "fix(openai): preserve refusal text in tr..." · Reviewed by Greptile

@CLAassistant

CLAassistant commented Oct 8, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ch-z-hc
ch-z-hc marked this pull request as ready for review October 8, 2026 13:34

@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 tests/unit/test_openai.py Outdated
async def test_chat_completion_captures_refusal(
langfuse_memory_client, get_span, json_attr, async_client, stream
):
import httpx

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Import breaks repository rule

The new test imports httpx inside the function. The repository requires imports at the top of the module. Move this import to the module’s import block before merging.

Rule Used: Move imports to the top of the module instead of placing them within functions or methods. (source)

Learned From
langfuse/langfuse-python#1387

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/test_openai.py
Line: 1377

Comment:
**Import breaks repository rule**

The new test imports `httpx` inside the function. The repository requires imports at the top of the module. Move this import to the module’s import block before merging.

**Rule Used:** Move imports to the top of the module instead of placing them within functions or methods. ([source](https://app.greptile.com/personal-org-4986/-/custom-context?memory=c960fc07-9928-409f-a18b-a780cbdded12))

**Learned From**
[langfuse/langfuse-python#1387](https://github.com/langfuse/langfuse-python/pull/1387)

---

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved httpx to the module import block in 6579cf1; the test behavior and assertions are unchanged.

Validation: the complete Linux/Python 3.12 unit suite passed (889 passed, 2 existing skips), along with Ruff, full-source mypy, changed-file formatting, and applicable commit hooks. Updated with Codex assistance.

This branch has not been deployed

No deployments
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