refactor(e2e): type the claude_code suite and drop its basedpyright exclusion - #33480
Open
mateo-berri wants to merge 2 commits into
Open
refactor(e2e): type the claude_code suite and drop its basedpyright exclusion#33480mateo-berri wants to merge 2 commits into
mateo-berri wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
Resolves caveat 7 of #32548:
tests/e2e/claude_codepredated the zero-error tests/e2e basedpyright gate and was excluded inpyrightconfig.jsonwith ~1,800 strict-mode errors. This PR types the whole suite and drops the exclusion, so the suite is now covered by the same zero-error gate as the rest oftests/e2eLinear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make lint-e2e-basedpyrightgate itself, which now covers the suite at zero errors)@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
This change has no proxy runtime surface; nothing an end user calls changes behavior. The user-visible surface is the lint gate itself, so the proof is the gate flipping from ~1,839 errors to zero with the exclusion removed, while the suite's non-live structural tests pass identically before and after
Before, at base commit ff4a40f with only the
"tests/e2e/claude_code"exclude entry removed frompyrightconfig.json:After, at cfa7f54, the real gate invocation:
Structural (non-live) suites, identical results at ff4a40f and at cfa7f54:
The 2 failures are pre-existing at the base commit and unrelated to this PR:
_publisher_unit_tests/test_run_daily_release_pagination.pyshells out tocurl https://api.github.com/..., which this sandboxed environment blocks; they fail with the sameCalledProcessErrorbefore and afterCollection is also unchanged at both commits:
The repo-wide budget gate is unaffected (its include is scoped to
litellm/), verified at cfa7f54:Type
🧹 Refactoring
Changes
Removes
"tests/e2e/claude_code"from thepyrightconfig.jsonexclude array and brings all 116 files of the suite to zero errors undertypeCheckingMode: strictwithreportAny/reportExplicitAnyas errors. Annotation and boundary-validation work only; no behavior, prompts, model lists, assertion semantics, or public helper names changedA new
tests/e2e/claude_code/json_types.pyprovides the shared vocabulary: a recursiveJSONValuealias (str | int | float | bool | None | list[JSONValue] | dict[str, JSONValue]),JSONObject = dict[str, JSONValue], and pydanticTypeAdapters used to validate every untyped boundary (json.loads,yaml.safe_load, HTTP bodies, CLI stream-json lines) into the typed shape instead of lettingAnyleak. Everything JSON-shaped the suite touches (stream-json events, npm packuments, manifest YAML, results artifacts, rate-limiter state files) flows through it, andisinstancechecks narrow it where the code already defensively branchedCore modules define the typed contract:
cli_driver.pygains aCommandRunnerprotocol for the injected subprocess runner (with a_run_subprocessdefault satisfying it) andDriverResult.events: list[JSONObject];conftest.pytypesCompatResultpayloads asMapping[str, JSONValue], the pytest hooks (pluggy.Result[pytest.TestReport]hookwrapper,pytest.Session,pytest.Function.funcargs), and models the rate-limit summary as aTypedDict;rate_limiter.py,matrix_builder.py,http_probe.py,_basic_messaging.py,pr_gate_version_resolver.py, andcron_vm/build_matrix.pyget fully typed signatures with pydantic validation at their file/wire boundaries. The 75 per-feature provider test files and the four_*_unit_tests/trees were then annotated against that contract (typedcompat_result: CompatResultfixtures,Mapping[str, JSONValue]event helpers, annotated fixture literals, narrowing helpers in the golden-file builder tests)# type: ignorecount is zero. Eight# pyright: ignore[exactRule]suppressions remain, each with a named rule and reason: two tests that deliberately pass a wrong type to assert a runtimeTypeError, two autouse fixtures pytest invokes by name, one private-helper import a unit test exercises on purpose, twopytest.approxcalls (unannotated upstream), and oneurllib.request.urlopenboundary that typeshed types asAnyQA runbook
No e2e test was added and none changed behavior; every diff in
tests/e2e/claude_codeis annotations, isinstance narrowing, or boundary validation with identical runtime semantics, so there are no new manual steps to reproduce. The verification that matters is in the proof section above: the gate at zero errors, the four structural unit-test trees passing identically at the base and head commits, and unchanged test collectionFinal Attestation