Skip to content

fix(test): prevent flaky failure in test_log_langfuse_v2_handles_null_usage_values#21419

Merged
jquinter merged 1 commit intomainfrom
fix/langfuse-test-supports-prompt-flakiness
Feb 17, 2026
Merged

fix(test): prevent flaky failure in test_log_langfuse_v2_handles_null_usage_values#21419
jquinter merged 1 commit intomainfrom
fix/langfuse-test-supports-prompt-flakiness

Conversation

@jquinter
Copy link
Contributor

Summary

This test has repeatedly failed in CI (previously fixed in #17599, #20475, #21093) with:

AssertionError: Expected '_add_prompt_to_generation_params' to have been called once. Called 0 times.
  • _add_prompt_to_generation_params is only reached when _supports_prompt() returns True
  • Under cross-test state contamination in CI (parallel workers running in the same process), langfuse_sdk_version can end up in an unexpected state
  • _supports_prompt() then returns False, silently skipping the call (the outer try/except Exception in _log_langfuse_v2 swallows any exception)
  • The previous fix used reset_mock() without side_effect=True, so setUp's trace.side_effect persisted and overrode the return_value set afterward

Fixes:

  • Use reset_mock(side_effect=True) to clear setUp's side effects so the explicit return_value assignments actually take effect
  • Add patch.object(self.logger, "_supports_prompt", return_value=True) to make the _add_prompt_to_generation_params assertion fully independent of SDK version state contamination

Test plan

  • poetry run pytest tests/test_litellm/integrations/test_langfuse.py::TestLangfuseUsageDetails -v — all 7 tests pass

🤖 Generated with Claude Code

…_usage_values

This test has failed repeatedly in CI with:
  'Expected _add_prompt_to_generation_params to have been called once. Called 0 times.'

Root cause: _add_prompt_to_generation_params is only called when _supports_prompt()
returns True. Under cross-test state contamination in CI (parallel workers),
langfuse_sdk_version can be in an unexpected state, causing _supports_prompt() to
return False and silently skip the call (exception swallowed by the outer try/except).

Fixes:
- Use reset_mock(side_effect=True) so setUp's trace side_effect is cleared and the
  explicit return_value assignment actually takes effect
- Patch _supports_prompt on the logger instance to always return True, making the
  _add_prompt_to_generation_params assertion independent of SDK version state

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 17, 2026 10:19pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

This PR fixes a repeatedly flaky test (test_log_langfuse_v2_handles_null_usage_values) that has failed intermittently in CI due to cross-test state contamination. Two root causes are addressed:

  • reset_mock(side_effect=True): The previous reset_mock() calls (without side_effect=True) did not clear the side_effect set in setUp(), so the subsequent return_value assignments on lines 281-284 were silently ignored. This caused trace() to not return the expected mock, leading to downstream assertion failures.
  • patch.object(self.logger, "_supports_prompt", return_value=True): The _add_prompt_to_generation_params assertion depends on _supports_prompt() returning True, which in turn depends on langfuse_sdk_version. Under parallel CI workers, this global state could be contaminated. Patching _supports_prompt directly makes the assertion fully independent of SDK version state.

Both fixes are minimal, well-targeted, and don't affect any production code. The test file is mock-only, consistent with the project rule requiring no real network calls in this test directory.

Confidence Score: 5/5

  • This PR is safe to merge — it only modifies a single test method with well-understood mock behavior fixes.
  • The changes are limited to a single test method, address clearly documented flaky behavior (referenced in 3 prior PRs), and both fixes are correct: reset_mock(side_effect=True) properly clears side_effect to allow return_value, and patching _supports_prompt isolates the test from global state. No production code is touched.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/test_litellm/integrations/test_langfuse.py Two targeted fixes to prevent flaky test: (1) reset_mock(side_effect=True) clears setUp's side_effect so return_value assignments take effect, (2) patch.object(self.logger, "_supports_prompt", return_value=True) isolates the test from SDK version state contamination. Both changes are correct and well-scoped.

Flowchart

flowchart TD
    A[test_log_langfuse_v2_handles_null_usage_values] --> B[reset_mock - side_effect=True]
    B --> C[Re-setup mock chain with return_value]
    C --> D[patch _supports_prompt → True]
    D --> E[_log_langfuse_v2 called]
    E --> F{_supports_prompt?}
    F -->|True - patched| G[_add_prompt_to_generation_params called]
    G --> H[trace.generation called]
    H --> I[Assert usage values converted None → 0]
    I --> J[Assert _add_prompt_to_generation_params called once]

    style B fill:#90EE90,stroke:#333
    style D fill:#90EE90,stroke:#333
    style F fill:#FFD700,stroke:#333
Loading

Last reviewed commit: 703f02b

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

This PR fixes a repeatedly flaky test (test_log_langfuse_v2_handles_null_usage_values) that has failed in CI multiple times due to cross-test state contamination when running with parallel workers.

  • reset_mock(side_effect=True): The setUp method sets mock_langfuse_client.trace.side_effect to _trace_side_effect. Previously, reset_mock() was called without side_effect=True, which left the side effect in place, overriding the return_value assignments that followed. This is fixed by passing side_effect=True to all three reset_mock() calls.
  • patch.object(self.logger, "_supports_prompt", return_value=True): The _add_prompt_to_generation_params function is only called when _supports_prompt() returns True, which depends on langfuse_sdk_version. Under parallel CI, this attribute can be contaminated by other tests. Patching _supports_prompt directly makes the assertion independent of global state.
  • Minor whitespace cleanup (trailing whitespace removed on one line).

The changes are minimal, well-targeted, and correctly address the two root causes of the flaky test.

Confidence Score: 5/5

  • This PR is safe to merge — it only modifies test code with two well-reasoned fixes for a known flaky test.
  • The changes are limited to a single test file, are minimal in scope, and correctly address two documented root causes of CI flakiness. No production code is modified. The fixes are consistent with Python unittest.mock semantics.
  • No files require special attention.

Important Files Changed

Filename Overview
tests/test_litellm/integrations/test_langfuse.py Two targeted fixes for flaky test: (1) reset_mock(side_effect=True) to properly clear setUp's _trace_side_effect so return_value assignments take effect, and (2) patch.object on _supports_prompt to decouple the _add_prompt_to_generation_params assertion from global SDK version state. Both changes are correct and well-scoped.

Flowchart

flowchart TD
    A["test_log_langfuse_v2_handles_null_usage_values"] --> B["reset_mock(side_effect=True)"]
    B --> C["Clear setUp's _trace_side_effect"]
    C --> D["Set return_value on trace mock"]
    D --> E["patch _supports_prompt → True"]
    E --> F["Call _log_langfuse_v2"]
    F --> G{"_supports_prompt()"}
    G -->|"True (patched)"| H["_add_prompt_to_generation_params called"]
    H --> I["Assert generation called with correct usage values"]
    I --> J["Assert _add_prompt_to_generation_params called once"]
    G -->|"False (unpatched, flaky)"| K["_add_prompt_to_generation_params skipped"]
    K --> L["❌ AssertionError: Called 0 times"]
Loading

Last reviewed commit: 703f02b

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@jquinter jquinter merged commit 6d5f9b5 into main Feb 17, 2026
17 of 25 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.

1 participant