refactor(e2e): fold claude_code HTTP probes onto shared ProxyClient methods - #33760
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
|
Greptile SummaryThis PR folds the
Confidence Score: 5/5Safe to merge — this is a pure test-harness refactor with no changes to production code paths; all assertions are preserved intact. The refactor correctly threads dependency injection through all ten migrated cells, the wire shape is preserved (headers, body fields, and serialization all match the old httpx probes), and both previously-flagged gaps from the last review cycle — the missing wildcard arm in No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/e2e/claude_code/http_probe.py | Core refactor: replaces hand-rolled httpx probes with injected ProxyClient methods; wildcard arm and diagnostic key-list both present; clean. |
| tests/e2e/proxy_client.py | Adds count_tokens and messages methods plus _anthropic_headers helper; header construction preserves Authorization and adds anthropic-version correctly. |
| tests/e2e/models.py | Promotes AnthropicMessagesBody/Response to full models with tools, content, choices; adds CountTokensBody/Response and supporting tool schema models; extra="allow" on response for triage diagnostics. |
| tests/e2e/e2e_http.py | Adds AnthropicHeaders extending AuthHeaders with a defaulted anthropic-version field; straightforward addition with no concerns. |
| tests/e2e/claude_code/_env.py | Adds require_proxy_client wrapping require_proxy and build_proxy_client; correctly sets both planes to the same base_url for the single-URL matrix case. |
| tests/e2e/claude_code/count_tokens/test_anthropic.py | Migrated from require_proxy/base_url to require_proxy_client/client; assertions and compat_result reporting unchanged. |
| tests/e2e/claude_code/tool_search/test_anthropic.py | Same mechanical migration as count_tokens cells; assertions and compat_result reporting unchanged. |
| tests/e2e/CLAUDE.md | Documentation updated to reflect that HTTP probes now use the shared transport while the CLI-driving path remains bespoke. |
Reviews (4): Last reviewed commit: "refactor(e2e): fold claude_code HTTP pro..." | Re-trigger Greptile
44b9285 to
7755bd5
Compare
|
@greptileai please review the current head 7755bd5. Addressed both findings: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| ), | ||
| } | ||
| ], | ||
| "tools": [ |
There was a problem hiding this comment.
i'd rather not remove this, this tool list brought up an internally found bug on azure / bedrock. i assigned this to @mateo
There was a problem hiding this comment.
Good call to double-check, but the tool list isn't removed; it's preserved byte-for-byte. The raw "tools": [...] dict you're pointing at moved to the _TOOL_SEARCH_TOOLS pydantic constant at the top of http_probe.py (the tool_search_tool_regex_20251119 discovery tool plus the add_numbers user tool), and probe_tool_search still sends that exact array. The tool_search/test_azure.py, test_bedrock_converse.py, and test_bedrock_invoke.py rows are untouched and keep exercising the same list that surfaced the internal azure / bedrock bug. The serialized request body is identical to what the old httpx probe sent, which the wire-shape check in the PR description confirms. Leaving this open for @mateo to sign off since you assigned it to him
4f4b99b to
260bc32
Compare
|
@greptileai please review the current head 260bc32. Since your last review this was rebased onto litellm_internal_staging and squashed to a single commit; the unit-level harness test ( |
…ethods Migrate tests/e2e/claude_code/http_probe.py off its own httpx client onto the shared transport, and promote count_tokens and native anthropic messages to first-class ProxyClient methods (ProxyClient.count_tokens / ProxyClient.messages) with typed request/response models in the shared models.py so other suites reuse them. The probes now take an injected ProxyClient and issue their request through the shared count_tokens/messages methods, reusing the split control/data-plane routing, timeout, and typed Result handling the rest of tests/e2e uses. The wire shape is preserved: the pydantic bodies serialize byte-for-byte to what the old httpx probes sent, and the anthropic-version header is carried by a small AnthropicHeaders model. httpx is gone from the module.
260bc32 to
78f10dc
Compare
|
@greptileai please review the current head 78f10dc. Rebased onto litellm_internal_staging, which since the last review renamed the shared |
Relevant issues
Linear ticket
Resolves LIT-4556
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This is a harness-internal refactor of two existing HTTP-probe cells; it adds no new asserted product behavior and changes no assertions, so there is no new manual proxy scenario beyond the QA runbook below. The two migrated cells stay real end-to-end rows in the daily compat matrix; the safety argument here is that the request the proxy sees is byte-for-byte what it saw before, plus the harness typecheck and whole-tree collection. Live matrix execution needs the real
claudematrix credentials, which are not available in this environmentWire shape is byte-for-byte identical to the old
httpxprobes (captured at 78f10dc):basedpyrightover the changed shared harness files is clean:The whole
claude_codetree still collects, including the ten migrated cells:Type
🧹 Refactoring
✅ Test
Changes
tests/e2e/claude_code/is a second, parallel e2e harness that ships its own driver, builder, publisher, rate limiter, HTTP probe, and version resolver, and its HTTP probes deliberately hand-rolled their ownhttpxclient. Keeping that probe layer separate from the shared transport is part of the maintenance liability the ticket tracks. This folds the HTTP-probe surface onto the shared transport and goes one step further than a pure lift-and-shift: it promotes the two probed endpoints to first-class sharedProxyClientmethods so any other suite gets them for freehttp_probe.pyused to build its ownhttpxclient and a hand-rolledProbeResultdataclass for the two cells that have no CLI surface (count_tokensandtool_search). It now takes an injectedProxyClientand issues each request throughProxyClient.count_tokens/ProxyClient.messages, reusing the shared split control/data-plane routing, timeout, and typedResultunion the rest oftests/e2e/uses.httpxis gone from the moduleThe two endpoints are now shared
ProxyClientmethods rather than inline probe plumbing.ProxyClient.count_tokens(key, body)posts to/v1/messages/count_tokensandProxyClient.messages(key, body)posts to/v1/messages, both carrying theanthropic-versionheader through a smallAnthropicHeadersmodel and returning a validated typedResult. Their request and response bodies live in the sharedmodels.py(CountTokensBody/CountTokensResponse, and an extendedAnthropicMessagesBody/AnthropicMessagesResponsethat models the tool-search tools array and the content-or-choices response shape), so a future messages suite reuses them instead of re-deriving the wire shapeThe wire shape is preserved exactly. The migrated pydantic bodies serialize byte-for-byte to what the old
httpxprobes sent (model+messagesfor count_tokens;model+max_tokens+messages+ thetool_search_tool_regex_20251119tools array for tool_search), and the header names are HTTP-case-insensitive equivalents of the old ones withContent-Typeset by the transport. Thetool_searchtools array is preserved intact: thetool_search_tool_regex_20251119discovery tool plus the trivialadd_numbersuser tool are now the_TOOL_SEARCH_TOOLSpydantic constant, so the azure / bedrock tool_search rows keep sending exactly the list that surfaced the earlier provider bug. Theassert_count_tokens_shape/assert_tool_search_shapehelpers keep their contract (returnNoneon success, else a diagnostic string) but nowmatchon the typedResult; thestatus 429:wording is preserved so the compat conftest still classifies a rate-limited cell viaRATE_LIMIT_SHAPED_REDependency injection is threaded through the ten
count_tokens/*andtool_search/*cells:_env.pygainsrequire_proxy_client, which resolves the sameLITELLM_PROXY_URL/LITELLM_MASTER_KEYenv and builds the sharedProxyClient, and the cells now grab aProxyClientplus the master key and pass them to the probe instead of rawbase_url/api_keystrings. Their asserts andcompat_resultreporting are untouchedWhat I deliberately left claude-specific, since the ticket scopes those out: the
claudeCLI driver (cli_driver.py), the compat-matrix building (matrix_builder.py), the tagged-unioncompat_resultreporting and its xdist merge inconftest.py, the cross-processrate_limiter.py(the probes still acquire one token per call so probe traffic counts against the same per-provider budget as CLI rows), andpr_gate_version_resolver.py. These have no shared equivalent, andhttp_probe.pywas the only claude_code module doing raw HTTPQA runbook
Both probed cells are existing HTTP-probe rows; the refactor changes how the request is issued, not what it asserts, and the bodies are byte-identical to before. A reviewer can reproduce each by hand against a live proxy that fronts a Claude-style deployment (needs the provider credentials on the proxy side, e.g. an
anthropic/*model registered as aclaude-*name)tests/e2e/claude_code/count_tokens/test_anthropic.py::test_count_tokens_anthropic -
/v1/messages/count_tokensreturns a positiveinput_tokensfor each Claude tier-H "Authorization: Bearer $LITELLM_MASTER_KEY" -H "anthropic-version: 2023-06-01"and body{"model": "claude-haiku-4-5", "messages": [{"role": "user", "content": "hello world"}]}{"input_tokens": <positive int>}tests/e2e/claude_code/tool_search/test_anthropic.py::test_tool_search_anthropic -
/v1/messagesaccepts atool_search_tool_regex_20251119tool and the proxy forwards it without a 400{"model": "claude-haiku-4-5", "max_tokens": 64, "messages": [{"role": "user", "content": "If you have a tool to discover other tools, use it to find one. Otherwise reply with the word 'done'."}], "tools": [{"type": "tool_search_tool_regex_20251119", "name": "tool_search_tool_regex"}, {"name": "add_numbers", "description": "Add two integers", "input_schema": {"type": "object", "properties": {"a": {"type": "integer"}, "b": {"type": "integer"}}, "required": ["a", "b"]}}]}content(Anthropic passthrough) orchoices(OpenAI-normalized), proving the proxy attached the per-provider tool-search beta header and forwarded itFinal Attestation