refactor(e2e/claude_code): dedupe tool_use cell body into shared _tool_use module - #33625
refactor(e2e/claude_code): dedupe tool_use cell body into shared _tool_use module#33625mateo-berri wants to merge 1 commit into
Conversation
Greptile SummaryThis PR extracts the duplicated
Confidence Score: 5/5Safe to merge — pure mechanical extraction of identical code into a shared module with no behavioral changes and live test runs confirming all 18 tests pass. Every cell's outcome-assertion loop, constants, and helper functions were byte-verified as identical before extraction. The shared module faithfully preserves the original skip semantics, per-model error messages, and streaming check. Imports were trimmed correctly: files with @pytest.mark.covers keep import pytest; those without do not. The only structural change (TOOL_USE_ARGS list to tuple) is compatible everywhere it is consumed. Live proxy runs across OpenAI, Azure OpenAI, and Anthropic confirm no regressions. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/claude_code/_tool_use.py | New shared module consolidating _has_tool_use_event, _count_input_json_deltas, constants, and run_tool_use_cell; logic is correct and well-documented |
| tests/e2e/claude_code/tool_use/test_anthropic.py | Replaced inline duplicated body with run_tool_use_cell(...); @pytest.mark.covers marker and import pytest retained correctly |
| tests/e2e/claude_code/tool_use/test_azure.py | Replaced inline body with run_tool_use_cell(...); security rationale pointer updated to new module location |
| tests/e2e/claude_code/tool_use/test_azure_openai.py | Replaced inline body with run_tool_use_cell(...); pytest import correctly dropped since this cell has no @pytest.mark.covers decorator |
| tests/e2e/claude_code/tool_use/test_bedrock_converse.py | Replaced inline body with run_tool_use_cell(...); @pytest.mark.covers marker and pytest import retained correctly |
| tests/e2e/claude_code/tool_use/test_bedrock_invoke.py | Replaced inline body with run_tool_use_cell(...); @pytest.mark.covers marker and pytest import retained correctly |
| tests/e2e/claude_code/tool_use/test_bedrock_mantle.py | Replaced inline body with run_tool_use_cell(...); skip_unless_mantle_cells_enabled() gate is preserved before the shared call |
| tests/e2e/claude_code/tool_use/test_openai.py | Replaced inline body with run_tool_use_cell(...); pytest import correctly dropped since no @pytest.mark.covers decorator |
| tests/e2e/claude_code/tool_use/test_vertex_ai.py | Replaced inline body with run_tool_use_cell(...); @pytest.mark.covers marker and pytest import retained correctly |
| tests/e2e/claude_code/tool_use_streaming/test_anthropic.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); @pytest.mark.covers and pytest import retained correctly |
| tests/e2e/claude_code/tool_use_streaming/test_azure.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); @pytest.mark.covers and pytest import retained |
| tests/e2e/claude_code/tool_use_streaming/test_azure_openai.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); pytest import correctly removed since no @pytest.mark.covers decorator present |
| tests/e2e/claude_code/tool_use_streaming/test_bedrock_converse.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); @pytest.mark.covers and pytest import retained |
| tests/e2e/claude_code/tool_use_streaming/test_bedrock_invoke.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); @pytest.mark.covers and pytest import retained |
| tests/e2e/claude_code/tool_use_streaming/test_bedrock_mantle.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); skip_unless_mantle_cells_enabled() gate preserved |
| tests/e2e/claude_code/tool_use_streaming/test_openai.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); pytest import correctly dropped since no @pytest.mark.covers decorator |
| tests/e2e/claude_code/tool_use_streaming/test_vertex_ai.py | Replaced inline streaming body with run_tool_use_cell(..., verify_streaming=True); @pytest.mark.covers and pytest import retained |
Reviews (1): Last reviewed commit: "refactor(e2e/claude_code): dedupe tool_u..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Relevant issues
Follow-up to #33474; Greptile flagged that
_has_tool_use_eventwas copy-pasted across the tool_use cell files. Stacked on #33474 and must merge after it (base is that PR's branch)Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy run against real provider APIs (OpenAI, Azure OpenAI, Anthropic), captured at 62c00fb. The "before" reference run is the parent branch head a017b95, whose identical cells were proven live in #33474's runbook; behavior here is identical by construction (same prompt, CLI args, per-model checks,
compat_resultpayloads, and failure strings, now sourced from one module)Proxy start:
Test collection is unchanged by the refactor (18 tests both before at a017b95 and after at 62c00fb):
Live cells through the shared body (openai and azure_openai each drive the three GPT-5.6 tiers; anthropic covers a legacy Claude column):
Type
🧹 Refactoring
✅ Test
Changes
The 16 live cell files under
tests/e2e/claude_code/tool_use/andtests/e2e/claude_code/tool_use_streaming/each carried an identical copy of_has_tool_use_event, theTOOL_USE_PROMPT/TOOL_USE_ARGSconstants, and the per-model outcome-assertion loop (the streaming files additionally duplicated_count_input_json_deltasand one extra check). This PR extracts all of it intotests/e2e/claude_code/_tool_use.py, mirroring the existing_basic_messaging.pyconvention: each cell file keeps its module docstring, model list, and@pytest.mark.coversmarker, and now calls the sharedrun_tool_use_cell(...)(withverify_streaming=Truefor the streaming feature). The Bash allowlist security rationale moved from a comment intool_use/test_anthropic.pyinto the shared module's docstring, and the mantle cells keep theirskip_unless_mantle_cells_enabled()gate in placeBehavior is identical by construction: same skip semantics, same
compat_result.addpayloads, same failure message formats, and the samepytest.fail(..., pytrace=False)shape; the loop bodies were verified byte-identical across all 16 files (modulo the models constant name) before extraction. Net diff is about 1,300 duplicated lines removedQA runbook
Environment prerequisites are the same as #33474: a proxy booted from
tests/e2e/claude_code/test_config.yaml,ANTHROPIC_API_KEY,OPENAI_API_KEYwith chat quota, andAZURE_API_BASE/AZURE_API_KEYpointing at a resource withgpt-5.6-sol/-terra/-lunadeployments; mantle cells stay opt-in viaCOMPAT_MANTLE_CELLS=1. The<alias>placeholders run once per model in the cell's listtool_use/cell; only the model list differs) - a tool call round-trips to an Anthropictool_useblock through the sharedrun_tool_use_cellbodytool_useblock for each model in the cell's list[<model>] no tool_use content block observed in stream-json events, CLI error, or non-zero exit diagnostics)tool_use_streaming/cell; only the model list differs) - streamed tool arguments arrive as incrementalinput_json_deltaevents via the same shared body withverify_streaming=True--include-partial-messagesaddedtool_useblock plus at least onecontent_block_deltastream event whose delta type isinput_json_delta; zero deltas means the proxy collapsed the streamed tool inputFinal Attestation