Repository navigation
Conversation
| async def test_chat_completion_captures_refusal( | ||
| langfuse_memory_client, get_span, json_attr, async_client, stream | ||
| ): | ||
| import httpx |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
What does this PR do?
Preserves OpenAI Chat Completions refusal text in generation output, including streamed responses.
When OpenAI returns
content=Noneand arefusal, the SDK consumer receives the refusal but the Langfuse trace loses it. Non-streaming output only containsroleandcontent; 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
refusalinto 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.pypass.Type of change
Verification
Python 3.12, locked dependencies:
test_shutdown_and_flush,test_span_metadata_updates_in_async_context). No test was disabled or weakened for this patch.ruff format --check .reports an existing formatting issue in unchangedtests/unit/test_media.py; left outside this patch.pythonsubprocess and POSIX-path failures. The relevant failures also reproduce on the unmodified base. The Linux run above passes the complete unit suite.Checklist
code_review.md..env.templateif needed. (Not needed: no examples or configuration become stale.)AI assistance: Codex assisted with investigation, implementation, tests and diff self-review. This is not a claim of independent human review.
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.
httpximport 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