add tests for fix

[Harshit28j]

follow up for tests #22188

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
litellm	Ready	Preview, Comment	Mar 14, 2026 7:30pm

[greptile-apps]

Greptile Summary

This PR is a follow-up to #22188, adding unit tests to verify that sensitive fields (e.g., api_key, secret_fields, authorization headers) are not leaked into Langfuse traces as modelParameters. It also cleans up unused imports (threading, datetime, MockRouter) and removes stray print debug statements.

Changes:

• Removes unused imports and debug print statements (clean-up).
• Adds test_langfuse_model_parameters_no_secret_leakage: correctly validates that ModelParamHelper.get_standard_logging_model_parameters filters out sensitive fields that are not standard OpenAI API parameters.
• Adds test_langfuse_v2_uses_standard_logging_model_parameters: claims to verify that _log_langfuse_v2 uses sanitized model_parameters from standard_logging_object, but the test is tautological — it never calls _log_langfuse_v2 and only exercises Python's dict.get() on a dict that was explicitly constructed two lines above. This test would pass even if the underlying secret-leakage bug were still fully present in langfuse.py (line 840 still passes optional_params directly as model_parameters).
• Minor formatting changes (quote normalisation, dict/import line wrapping).

Confidence Score: 3/5

• Safe to merge from a functional standpoint, but one of the two new tests does not actually cover the fix it claims to verify.
• The clean-up changes (removing unused imports and prints) are harmless. The first new test (test_langfuse_model_parameters_no_secret_leakage) genuinely exercises ModelParamHelper.get_standard_logging_model_parameters and provides real coverage. However, test_langfuse_v2_uses_standard_logging_model_parameters never calls _log_langfuse_v2; it merely asserts the result of dict.get() on a dict that was populated two lines earlier. This means the test would pass even if the secret-leakage regression it is supposed to guard against were reintroduced, reducing confidence in the test suite as a regression detector.
• tests/logging_callback_tests/test_langfuse_unit_tests.py — specifically the test_langfuse_v2_uses_standard_logging_model_parameters function (lines 555–588).

Important Files Changed

Filename

Overview

tests/logging_callback_tests/test_langfuse_unit_tests.py

Removes unused imports and debug prints (clean), adds two new tests for secret-leakage fix; the first test (test_langfuse_model_parameters_no_secret_leakage) correctly validates ModelParamHelper.get_standard_logging_model_parameters, but the second (test_langfuse_v2_uses_standard_logging_model_parameters) never calls _log_langfuse_v2 and is tautological — it would pass even if the underlying bug were still present.

Sequence Diagram

sequenceDiagram
    participant T as Test
    participant MPH as ModelParamHelper
    participant LFLogger as LangFuseLogger
    participant TG as trace.generation()

    Note over T,TG: test_langfuse_model_parameters_no_secret_leakage (REAL coverage)
    T->>MPH: get_standard_logging_model_parameters(params_with_secrets)
    MPH-->>T: {temperature, top_p, max_tokens, stream} (secrets filtered)
    T->>T: assert secrets absent ✅

    Note over T,TG: test_langfuse_v2_uses_standard_logging_model_parameters (TAUTOLOGICAL)
    T->>T: standard_logging_object["model_parameters"] = safe_params
    T->>T: dict.get("model_parameters", optional_params_with_secrets)
    T-->>T: returns safe_params (trivially, never touches _log_langfuse_v2)
    T->>T: assert secrets absent ✅ (but _log_langfuse_v2 never called!)

    Note over T,TG: What the test SHOULD do
    T->>LFLogger: _log_langfuse_v2(..., optional_params=params_with_secrets, ...)
    LFLogger->>TG: generation(model_parameters=???)
    TG-->>T: capture call_args
    T->>T: assert "api_key" not in call_args["model_parameters"] ✅

Loading

_{Last reviewed commit: d7c9ec6}

[greptile-apps]

Test is tautological and doesn't exercise _log_langfuse_v2

The test name and docstring claim to verify that _log_langfuse_v2 uses sanitized model_parameters from standard_logging_object instead of raw optional_params. However, the test never calls _log_langfuse_v2 at all.

The core assertion:

sanitized = standard_logging_object.get(
    "model_parameters", optional_params_with_secrets
)
assert "api_key" not in sanitized

...is trivially true because standard_logging_object["model_parameters"] was explicitly set to {"temperature": 0.5, "stream": True} two lines earlier. This is just testing Python's dict.get(), not the actual Langfuse logging code.

Crucially, looking at litellm/integrations/langfuse/langfuse.py line 840:

"model_parameters": optional_params,

_log_langfuse_v2 still passes the raw (caller-side filtered) optional_params directly. This test would pass even if the bug described in the docstring were still fully present. To actually verify the fix, the test should mock trace.generation and assert the model_parameters argument does not contain secrets, e.g.:

with patch.object(langfuse_logger.Langfuse, "trace") as mock_trace:
    mock_trace_instance = Mock()
    mock_trace.return_value = mock_trace_instance
    langfuse_logger._log_langfuse_v2(..., optional_params=optional_params_with_secrets, kwargs=kwargs_with_standard_logging_object, ...)
    call_kwargs = mock_trace_instance.generation.call_args.kwargs
    assert "api_key" not in call_kwargs["model_parameters"]