fix(ci): make litellm_internal_staging green (logging test + Bedrock Opus 4.7 self-heal)#29344
Conversation
…edaction PR #28909 hardened log_db_metrics to emit a minimal, non-sensitive event_metadata (only table_name when present, otherwise None) instead of dumping function_name, function_kwargs, and function_args onto the span. The test in test_log_db_redis_services was not updated and still asserted "function_name" in event_metadata, which raised TypeError (argument of type 'NoneType' is not iterable) and turned the logging_testing CI job red on litellm_internal_staging. Update test_log_db_metrics_success to assert event_metadata is None when no table_name is passed, and add test_log_db_metrics_event_metadata_is_safe as a regression guard verifying that only the table name surfaces and that sensitive kwargs (tokens, prisma client) are never dumped.
Greptile SummaryThis PR fixes two failing CircleCI jobs on
Confidence Score: 5/5Safe to merge — both changes are test-only, all new assertions reflect the current production contract, and no production code is modified. All three files are in the test layer and touch no production code. The modified assertion in No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/llm_translation/reasoning_effort_grid/grid_spec.py | Restores fail_reason: Optional[str] = None field to ModelEntry and marks the unentitled bedrock-claude-opus-4-7 cell with an xfail reason; all instances use keyword args so field insertion order is safe. |
| tests/llm_translation/reasoning_effort_grid/test_reasoning_effort_grid.py | Adds pytest.xfail(model.fail_reason) guard before API calls; placement is correct — fires after env-var skip checks and before any network call, so the 8 unentitled Opus 4.7 cells report as xfail instead of failing the suite. |
| tests/logging_callback_tests/test_log_db_redis_services.py | Aligns test_log_db_metrics_success to the post-#28909 contract (event_metadata is None on the no-table_name path) and adds test_log_db_metrics_event_metadata_is_safe as a regression guard that the table name surfaces but sensitive kwargs (token, prisma_client) do not. |
Reviews (1): Last reviewed commit: "test(bedrock): restore opus-4-7 xfail wo..." | Re-trigger Greptile
The bedrock-claude-opus-4-7 converse cells are unentitled on the Bedrock CI account, so they were marked xfail. xfail keeps reporting them as expected failures even after access is granted, so the wire translation never gets verified again. Now the cell makes the call and skips only when Bedrock replies "is not available for this account"; the moment the model is entitled the same cells run their full assertions with no edit. A focused unit test pins the tolerance predicate so any other failure still surfaces loudly and the available path still runs the assertions.
96dc86c to
1015f9e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…Opus 4.7 self-heal) (BerriAI#29344) * test(logging): align DB metrics event_metadata assertions with safe redaction PR BerriAI#28909 hardened log_db_metrics to emit a minimal, non-sensitive event_metadata (only table_name when present, otherwise None) instead of dumping function_name, function_kwargs, and function_args onto the span. The test in test_log_db_redis_services was not updated and still asserted "function_name" in event_metadata, which raised TypeError (argument of type 'NoneType' is not iterable) and turned the logging_testing CI job red on litellm_internal_staging. Update test_log_db_metrics_success to assert event_metadata is None when no table_name is passed, and add test_log_db_metrics_event_metadata_is_safe as a regression guard verifying that only the table name surfaces and that sensitive kwargs (tokens, prisma client) are never dumped. * test(bedrock): self-heal opus-4-7 grid cells when unentitled on CI The bedrock-claude-opus-4-7 converse cells are unentitled on the Bedrock CI account, so they were marked xfail. xfail keeps reporting them as expected failures even after access is granted, so the wire translation never gets verified again. Now the cell makes the call and skips only when Bedrock replies "is not available for this account"; the moment the model is entitled the same cells run their full assertions with no edit. A focused unit test pins the tolerance predicate so any other failure still surfaces loudly and the available path still runs the assertions.
Relevant issues
litellm_internal_stagingCI was red. GitHub Actions was fully green, but the CircleCIbuild_and_testworkflow on staging HEAD (f11c12d, pipeline #79824) had two failing jobs:logging_testingandllm_translation_testing. This PR fixes both. (Supersedes #29341, which was on aclaude/branch the CircleCI branch filter skips; moved here to alitellm_*branch so CircleCI runs.)What this fixes
1.
logging_testing.#28909 hardened
log_db_metricsto emit a minimal, non-sensitiveevent_metadata(onlytable_namewhen present, otherwiseNone) instead of dumpingfunction_name,function_kwargs, andfunction_argsonto the span; the function name still rides on the separatecall_typeargument.test_log_db_redis_services.pywas not updated and still asserted"function_name" in event_metadata, so on the no-table_namepathevent_metadataisNoneand the assertion raisedTypeError: argument of type 'NoneType' is not iterable.This updates
test_log_db_metrics_successto assertevent_metadata is Nonefor that path and addstest_log_db_metrics_event_metadata_is_safe, a regression guard that only the table name surfaces and that sensitive kwargs (tokens, prisma client) are never dumped.2.
llm_translation_testing.All 8 failures were
test_reasoning_effort_grid[bedrock_converse-bedrock-claude-opus-4-7-*]withBedrockException - {"message":"anthropic.claude-opus-4-7 is not available for this account..."}. The revert in #29326 dropped the workaround for the unentitled Opus cells on the assumption that the reactivated Bedrock account (888602223428) carries flagship Opus. Live Bedrockconversecalls with that account (in both us-east-1 and us-west-2) confirmus.anthropic.claude-opus-4-6-v1works butus.anthropic.claude-opus-4-7returns "is not available for this account", so opus-4-7 is genuinely unentitled there and access requires an AWS Sales request rather than self-serve. The CircleCIAWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEYalready point at 888602223428, so no credential rotation was needed.Rather than restore the old
xfail(which keeps reporting the cells as expected failures even after access is granted, so the wire translation never gets verified again), thebedrock-claude-opus-4-7cell now runs the call and skips only when Bedrock replies "is not available for this account". The moment opus-4-7 is entitled, the same cells run their full wire assertions with no test edit. A focused unit test (test_model_unavailable_tolerates_only_the_declared_error) pins the tolerance predicate so any other failure still surfaces loudly and the available path still asserts.Stacked PR
#29327 (Opus 4.8 reasoning-effort grid) branches off this PR. Its Bedrock opus-4-8 cell reuses this self-heal path; its Azure and Vertex opus-4-8 cells use a separate
fail_reasonxfail until a Foundry deployment exists and Vertex availability is confirmed.Pre-Submission checklist
Proof of Fix
CI-only test fix; the proof is the two previously-red CircleCI jobs turning green on this PR's branch. With opus-4-7 still unentitled on the CI account, the 8
bedrock-claude-opus-4-7cells now report as skipped (bedrock-claude-opus-4-7: is not available for this account) instead of failing the suite, andlogging_testingpasses.Before (staging HEAD pipeline #79824):
The CircleCI run on this PR's latest commit is the live proof.
CI (LiteLLM team)
Type
🐛 Bug Fix
✅ Test
Changes
tests/logging_callback_tests/test_log_db_redis_services.py: alignevent_metadataassertions with the redacted contract from #28909 and add a redaction regression test.tests/llm_translation/reasoning_effort_grid/grid_spec.pyandtest_reasoning_effort_grid.py: replace the opus-4-7xfailwith a self-healunavailable_errorguard that skips only on Bedrock's "is not available for this account" reply and otherwise runs the full assertions, plus a unit test for the tolerance predicate.