fix(ci): ignore-list recursive form-field flatteners in recursive_detector - #38149
Conversation
…ector The recursive_detector code-quality gate fails on litellm_internal_staging because _flatten_form_field and _flatten_form_data_field in llm_request_utils.py are recursive but absent from IGNORE_FUNCTIONS. Both are bounded structural recursion over an already-parsed JSON-shaped request body (a finite tree, no cycles possible), matching the existing ignored walkers, so add them to the ignore list with a justification comment.
Greptile SummaryThis PR restores the recursive-detector CI gate by exempting two finite structural form-field flatteners.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| tests/code_coverage_tests/recursive_detector.py | Adds the two form-field flatteners to the recursive-function allowlist; no eligible blocking follow-up issue remains. |
Reviews (2): Last reviewed commit: "fix(ci): ignore-list recursive form-fiel..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5d34b12. Configure here.
tin-berri
left a comment
There was a problem hiding this comment.
Approving — this is the red I was chasing across #38144, #38114 and #38136, so landing it first turns three PRs green.
One note on the justification, since the ignore list is load-bearing. Every neighbouring entry cites an explicit bound the function enforces itself ("max depth set (MAX_STRUCTURED_CONTENT_SCAN_DEPTH)", "fails closed by raising at the cap"). These two enforce nothing — they recurse until the input stops nesting. What actually makes that safe isn't "no cycles possible"; a self-referential dict handed to flatten_form_field_values would still spin. It's that the only thing reaching these is an already-parsed request body, and json.loads caps nesting itself, so a body deep enough to matter fails to parse before it ever gets here.
Worth saying that in the comment rather than "no cycles possible" — it names the actual invariant, and it tells the next person what would break it (feeding these a hand-built Python mapping instead of a parsed body).
TLDR
Problem this solves:
code-qualitycheck is red on every PRrecursive_detectorstep fails onlitellm_internal_stagingitselfHow it solves it:
User Flow
Before: any contributor opening a PR sees the
code-qualitycheck go red through no fault of their own, because the base branch itself fails the detector steplitellm_internal_stagingrecursive_detectorstep exits 1 withUnignored recursive functions found in ./litellm/litellm_core_utils/llm_request_utils.py: ['_flatten_form_field', '_flatten_form_data_field'], so thecode-qualitycheck shows a red XAfter: the same contributor opens a PR and the
code-qualitycheck passes, reflecting only their own changelitellm_internal_stagingrecursive_detectorstep exits 0 (the two flatteners are now recognized as bounded structural recursion), so thecode-qualitycheck shows a green checkRelevant issues
Linear ticket
Resolves LIT-6066
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<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)The
recursive_detectorgate is itself the regression test here: it fails whenever a recursive function inlitellm/is not ignore-listed, and it now passes with these two entries added, so the same breakage cannot recur silently.Screenshots / Proof of Fix
This is a CI/lint-config fix with no runtime behavior change, so the live-proxy curl QA rule does not apply. The proof is the
recursive_detectorgate itself: it exits 1 before (naming the two functions) and exits 0 after. CI runs it asuv run --no-sync python ./tests/code_coverage_tests/recursive_detector.py; the script only importsastandos, sopython3reproduces CI exactly.Before (47c988e)
python3 ./tests/code_coverage_tests/recursive_detector.py ; echo EXIT=$?After (5d34b12)
python3 ./tests/code_coverage_tests/recursive_detector.py ; echo EXIT=$?The two functions now appear in the
IGNORED RECURSIVE FUNCTIONSlist instead.Type
🚄 Infrastructure
Caveats (if any)
Final Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
5d34b12 passes /live-pr-risk
Note
Low Risk
CI allowlist-only change with no runtime or security behavior; recursion bounds are documented as structural on parsed JSON.
Overview
Fixes a base-branch failure in the
code-qualityworkflow’srecursive_detectorstep, which was flagging_flatten_form_fieldand_flatten_form_data_fieldinllm_request_utils.pyas unlisted recursive functions and failing every PR.Adds both names to
IGNORE_FUNCTIONSinrecursive_detector.pywith rationale that they recurse only over the nesting depth of an already-parsed request body (finite JSON, no cycles)—the same pattern as other bounded structural walkers on the list. No application logic changes; only the AST-based gate passes again.Reviewed by Cursor Bugbot for commit 5d34b12. Bugbot is set up for automated code reviews on this repo. Configure here.