Repository navigation
ci: gate undocumented POST routes in the proxy OpenAPI document - #45843
Conversation
Add summaries, descriptions, inlined request body schemas with examples and typed 200 responses to the decision routes and their aliases, so the OpenAPI document and the lazy snapshot show both request formats Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
kerry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
| - name: Install the proxy | ||
| if: matrix.checker == 'openapi-docs' | ||
| run: | | ||
| uv sync --frozen --group proxy-dev --group e2e-dev |
There was a problem hiding this comment.
The new gate imports the proxy, but this install does not select --extra proxy. proxy_server requires apscheduler, which uv.lock includes only through that extra. On a clean runner, the checker fails during import before comparing any counts.
Add the proxy extra to this install and the Python job's install in test-linting.yml. Ensure the local gate target installs it too.
|
|
||
| def _json_schema(container: object) -> object: | ||
| content: Final = _as_mapping(_as_mapping(container).get("content")) | ||
| return _as_mapping(content.get("application/json")).get("schema") |
There was a problem hiding this comment.
_json_schema only accepts application/json, so OA001 rejects a fully documented multipart or form request. The proxy already uses this pattern in create_file, with UploadFile = File(...) and Form(...) parameters. Adding another documented upload route increases the count and fails lint even though Swagger shows its request schema.
Check schemas under the supported content types rather than requiring JSON.
| if not _json_schema(_as_mapping(operation.get("responses")).get("200")): | ||
| yield "OA003" |
There was a problem hiding this comment.
Documented creation routes fail lint
OA003 requires a 200 schema even when a POST documents a different success status. For example, create_user has a typed request, a description, and response_model=SCIMUser with status_code=201. A new route following that pattern still increases OA003 and fails lint.
Accept the route's documented success response instead of requiring a response it never returns.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Merging this PR will not alter performance
|
TLDR
Problem this solves:
request.body()shows up in Swagger UI with no request schema, no description and an empty 200How it solves it:
scripts/check_openapi_docs.pywalksapp.openapi()and reports every POST operation missing one of the three (OA001 body, OA002 description, OA003 200 schema)scripts/openapi_docs_gate.pycounts those per rule at HEAD and at the merge-base, same ratchet as the other four gates, so a new undocumented route fails and existing ones may only shrinkmake lint(somake check), the test-linting.yml python job and the base-counts publisherTargets main; #45838 (the documented decision routes the gate was written against) is merged
Also drops the unused
TypeIsimport inlitellm/proxy/_types.pythat was failing ruff on main and blocking this PR's lint jobUser Flow
Before: an engineer adds a raw-body POST route and nobody finds out it is undocumented until a user opens Swagger
@router.post("/v1/new_thing")withasync def new_thing(request: Request)and open a PRmake checkand the lint CI job passPOST /v1/new_thingwith no description, no request body and200 Successful Responsewith no schemaAfter: the same PR fails lint and names the route and what it is missing
make checkand the lint CI job fail withOA001 ... POST /v1/new_thing,OA002 ...,OA003 ...and a one-paragraph hint on how to document itresponse_model(orresponses={200: ...}), the gate passes, and Swagger shows the full operationPre-Submission checklist
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Probe route appended to
litellm/proxy/proxy_server.pyfor both runs, then reverted:Before (cbd81cc)
Undocumented route added
ls scripts/openapi_docs_gate.pyls: cannot access 'scripts/openapi_docs_gate.py': No such file or directory, nothing inmake lintlooks at the OpenAPI document, so the probe route passes lintAfter (d27236e)
Clean tree
uv run --no-sync python scripts/openapi_docs_gate.py --base litellm_decisions_swagger_docsOK: no OA rule grew past its merge-base count (base litellm_decisions_swagger_docs)(35 s wall clock for the HEAD and merge-base measurements together)Undocumented route added
uv run --no-sync python scripts/openapi_docs_gate.py --base litellm_decisions_swagger_docsFull finding list
uv run --no-sync python scripts/check_openapi_docs.py | cut -d' ' -f3 | sort | uniq -c182 OA001,45 OA002,234 OA003across 352 POST operations, the baseline the ratchet starts fromType
🚄 Infrastructure
Caveats (if any)
Medium
Low
proxyextra for this step only, the other gates keep the smaller installFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/6fb807b73569493f85e18ebd83dfbf6f
Open in Devin Desktop: https://app.devin.ai/desktop/session/6fb807b73569493f85e18ebd83dfbf6f?variant=devin
Requested by: @kerry-berri